Problem/Motivation

hook_rest_type_uri_alter() and hook_rest_relation_uri_alter() were deprecated in #2758897: Move rest module's "link manager" services to serialization module

That was before we had the ability to @trigger_error() for deprecated hooks: https://www.drupal.org/node/2881531

Proposed resolution

Use ModuleHandlerInterface::invokeDeprecated() and alterDeprecated() to invoke these hooks.

Modify any tests to include @expectedDeprecation if they use implementations of these hooks.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23 created an issue. See original summary.

Wim Leers’s picture

Status: Active » Needs review
FileSize
1.61 KB

YES!

I'd tag this as novice, but I first want to get some experience with this myself.

Status: Needs review » Needs work

The last submitted patch, 2: 2934994-2.patch, failed testing. View results

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Wim Leers’s picture

Status: Needs work » Needs review
FileSize
3.32 KB
3.3 KB
Mile23’s picture

Status: Needs review » Needs work
+++ b/core/modules/hal/src/LinkManager/RelationLinkManager.php
@@ -73,7 +73,7 @@ public function getRelationUri($entity_type, $bundle, $field_name, $context = []
     // @deprecated in Drupal 8.3.x and will be removed before Drupal 9.0.0. This
     // hook is invoked to maintain backwards compatibility
...
+    $this->moduleHandler->alterDeprecated('Implement hook_hal_relation_uri_alter() instead.', 'rest_relation_uri', $uri, $context);

I think the triggered message and the @deprecated docblock message should be the same. Telling someone what to implement instead is much more useful, so we should change the @deprecated message to reflect that.

Wim Leers’s picture

Status: Needs work » Needs review
FileSize
4.41 KB
4.08 KB

Done.

borisson_’s picture

It doesn't look like anything else should be fixed in this issue. But I'm not sure, so not setting to RTBC. If Wim can confirm that nothing else should be fixed here, this is RTBC for me.

Wim Leers’s picture

I confirm!

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

Awesome, sorry for not setting it to RTBC earlier, it looked so simple that I wasn't sure. Thank you!

Wim Leers’s picture

np at all :)

Thank you for the review/RTBC!

catch’s picture

Committed 5b43b3d and pushed to 8.6.x. Thanks!

catch’s picture

Status: Reviewed & tested by the community » Fixed

  • catch committed 5b43b3d on 8.6.x
    Issue #2934994 by Wim Leers, Mile23: Complete deprecation of...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.