Problem/Motivation
#2097123: Deprecate comment_num_new() in favour of method on CommentManager deprecated comment_num_new()
Proposed resolution
Remove it when the time is right.
Remaining tasks
User interface changes
No.
API changes
Removal of comment_num_new()
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | interdiff.txt | 1.49 KB | MKorostoff |
| #6 | drupal8-remove-comment-num-new-2296839-6.patch | 2.88 KB | MKorostoff |
| #4 | Working As Expected.png | 140.89 KB | MKorostoff |
| #3 | drupal8-remove-comment-num-new-2296839-3.patch | 1.4 KB | pushpinderchauhan |
Comments
Comment #1
yesct commentedtechnically postponed until the other is committed I guess.
Comment #2
marcingy commentedOther issue is in so unpostponing,.
Comment #3
pushpinderchauhan commentedPlease review.
Comment #4
MKorostoff commentedConfirming that the patch in #3 applies cleanly for me, and that the "new comment count" continues to function as expected when commenting on articles.
To test this I did the following
However, one small item I noticed that could use a bit of additional work: in core/modules/comment/src/Tests/CommentLinksTest.php on lines 234 and 246 there are docblocks which reference comment_num_new(). The overall test remains relevant and functional, but this documentation should be updated to reflect the new function name. I'll update and post a new patch in a moment.
Comment #5
MKorostoff commentedComment #6
MKorostoff commentedThis patch 1) removes the deprecated function comment_num_new() and 2) replaces all docblock references to comment_num_new() with the new method \Drupal::service('comment.manager')->getCountNewComments()
Comment #7
MKorostoff commentedComment #8
MKorostoff commentedHere's an interdiff for the above patch
Comment #9
yesct commentedI wonder if that comment is still accurate.
Comment #10
MKorostoff commentedYup, that docblock is indeed still accurate.
Here's the behavior that docblock is describing:
I've tested this behavior by creating dummy user accounts and dummy content, and it is indeed preserved in getCountNewComments()
Comment #11
yesct commentedok. I read all the lines of the patch.
did
ag "comment_num_new" core/*
this is getting them all as far as I can see, and the edited comments seem fine (no out of scope changes, and wrapped for the longer reference)
Comment #12
alexpottCommitted 46a9a49 and pushed to 8.x. Thanks!