Deprecate comment_uri()

Updated the issue title, this is just a straightforward deprecation now. Needs a re-roll updating the deprecation version.

Filed CR https://www.drupal.org/node/3384294 →

and updated patch, looks there's no test for uri_callback annotation in comment entity

Also it needs update hook to clear entity definition cache, and let's see if something will be broken with deprecation only

Looking for better wording as Comment::permalink() is useless 📌 Rename Comment::permalink() to not be ambiguous with ::uri() Needs work

That's reason why issue used to have such title

updated IS

Comment::permalink() isn't useless, it's necessary to link to a comment in the context of the thread (with the correct page and comment fragment). I think we can just tell people to use that until it's renamed. However why not recommend $comment->uri() here?

samit.310@gmail.com → made their first commit to this issue’s fork.

Hello,

The comment_uri function is only been used in uri_callbackcomment entity's annotation.

As per https://www.drupal.org/project/drupal/issues/2667040 📌 Deprecate uri_callback in routes for entities Needs work ticket the uri_callback is also depricated.

In template_preprocess_comment hook $comment->permalink(); is already been used to generate Comment link.

Thanks
Samit K.

Title says to deprecate the function so that still needs to be done

Hi @smustgrave,

As per the changes record [#3384294] it is deprecated in 10.2.0 and will removed with 11.x, so i removed it for 11.x branch.

Thanks
Samit K.

Yes but it got missed so now needs to be properly deprecated

Hi @smustgrave,

Updated the code based on your suggestion. Also updated the changes record [#3384294] → .

Thanks
Samit K.

Thanks only thing missing is a quick deprecation test

Hi @smustgrave,,

Test also added, Please review.

Thanks
Samit K.

Couple of minor questions/nits on MR, thanks for keeping this moving

Looking into the MR feedback.

I have reviewed all the feedback and made the necessary updates. After making the changes, I re-ran the tests, and This is working as expected. Moving this to NR for your review.

Left another small comment on MR.

@smustgrave Thank you for the review & feedback , I have updated the test file name and comment for the test class, Accordingly updated the Phpstan baseline . Re-run test after these changes and working as expected. and pipeline is green so moving this again for your review.
Kindly review.

Left 1 more comment on the thread started by @larowlan

Moving this for your review.

Removed the comment on the return statement, as I believe it was not appropriate.
Kindly review.

This function is still referenced on the Comment entity type attribute as the uri callback, it should be removed there. https://git.drupalcode.org/project/drupal/-/blob/010bbc7476df8c23b0170f5...

@berdir Thank you for the feedback , I have removed the uri_callback and tested the functionality. It is working as expected. However, I am a bit unsure if there is anything else that needs to be updated related to this.

Moving this for NR , Kindly review.

Yes, uri_callback has not actually been used in a very, very long time. If it would have been called, it would have triggered deprecation errors in the tests. Looking at \Drupal\Core\Entity\EntityBase::toUrl(), it only falls back to that if there are no link templates defined. Which isn't the case for the comment entity (or any other entity in core AFAIK).

@berdir, Thanks you for the Information :)

Believe all feedback for this one has been addressed.

I read the IS, comments and the MR. This all looks good but I found some things that need work, so setting to NW. See the comments in the MR.

I am not sure about the removal of uri_callback: 'comment_uri',. I did read the comment from @berdir but it is used in entity.api.php and other files. I think that needs some investigation.

the proposed solution in the iS does need an update, but #72 explains why it's not called. uri_callback is a weird leftover from before we had link templates.

Entirely deprecating that concept is done in 📌 Deprecate uri_callback in routes for entities Needs work , this is just one specific dead configuration that is not used.