All places in Comment module should replace global $user and $GLOBALS['user'].

CommentFileSizeAuthor
#34 2061899-34-global-user-comment.patch4.69 KBjoelpittet
#32 2061899-32-global-user-comment.patch4.66 KBjoelpittet
#19 2061899-19-global-user-comment.patch3.7 KBjoelpittet
#19 interdiff.txt5.39 KBjoelpittet
#18 interdiff.txt1.59 KBjoelpittet
#18 2061899-18-global-user-comment.patch3.7 KBjoelpittet
#16 2061899-16-global-user-comment.patch6.84 KBjoelpittet
#16 interdiff.txt1.59 KBjoelpittet
#13 2061899-13-global-user-comment.patch6.84 KBjoelpittet
#11 2061899-11-global-user-comment-reroll.patch2.64 KBjoelpittet
#7 comment-remove-global-user-from-comment-module-2061899-7.patch3.54 KBm1r1k
#2 comment-remove-global-user-from-comment-module-2061899-2.patch4.31 KBm1r1k
#1 comment-remove-global-user-from-comment-module-2061899-1.patch4.31 KBm1r1k
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

m1r1k’s picture

m1r1k CreditAttribution: m1r1k commented
Status: Active » Needs review
FileSize
comment-remove-global-user-from-comment-module-2061899-1.patch4.31 KB

Here is the patch

m1r1k’s picture

m1r1k CreditAttribution: m1r1k commented
FileSize
comment-remove-global-user-from-comment-module-2061899-2.patch4.31 KB

Removed '\' form functional code
Depends on #2062051: The request does not contain the _account on comment permalink pages

Status: Needs review » Needs work

The last submitted patch, comment-remove-global-user-from-comment-module-2061899-2.patch, failed testing.

yanniboi’s picture

yanniboi CreditAttribution: yanniboi commented 
+++ b/core/modules/comment/lib/Drupal/comment/Plugin/Core/Entity/Comment.php
@@ -225,7 +225,7 @@ public static function preCreate(EntityStorageControllerInterface $storage_contr
+    $user = \Drupal::request()->attributes->get('_account');

This is where the BCDecorator Test fails.

After debugging I fount the '_account' is not among the attributes (only parameters is available..). Not sure how to fix it, but I thought I'd post what I've found.

yanniboi’s picture

yanniboi CreditAttribution: yanniboi commented

Looks like the ENTITY CRUD HOOKS test fails for the same reason.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented 
+++ b/core/modules/comment/comment.module
@@ -1323,7 +1323,7 @@ function comment_load($cid, $reset = FALSE) {
 function comment_num_new($nid, $timestamp = 0) {
-  global $user;
+  $user = Drupal::request()->attributes->get('_account');

Probably this one should use Drupal::currentUser()

m1r1k’s picture

m1r1k CreditAttribution: m1r1k commented
Status: Needs work » Needs review
FileSize
comment-remove-global-user-from-comment-module-2061899-7.patch3.54 KB

Rerolling after #2062151: Create a current user service to ensure that current account is always available. Also using this small but important fix - #2076411: Remove the request scope from the current user service

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs work » Needs review 
+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/NodeNewComments.php
@@ -93,7 +93,7 @@ public function query() {
+    $user = \Drupal::currentUser();

only questionable because better to inject this service

Status: Needs review » Needs work

The last submitted patch, comment-remove-global-user-from-comment-module-2061899-7.patch, failed testing.

m1r1k’s picture

m1r1k CreditAttribution: m1r1k commented

Seems it depends on patches for #2061935: Remove calls to deprecated global $user in forum module, #2078057: Remove references to global $user in Content Translation module and other similar issues.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Assigned: m1r1k » Unassigned
Issue tags: -CodeSprintCIS
FileSize
2061899-11-global-user-comment-reroll.patch2.64 KB

Re-rolled #7

Status: Needs review » Needs work

The last submitted patch, 2061899-11-global-user-comment-reroll.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
FileSize
2061899-13-global-user-comment.patch6.84 KB

curious about @m1r1k said in #10 so I applied both of those patches to #11 to see what testbot does.

This is a test only. Use #11 for the basis of any future patches/rerolls.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 2061899-13-global-user-comment.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs work » Needs review
FileSize
interdiff.txt1.59 KB
2061899-16-global-user-comment.patch6.84 KB

Changed for this: #2053489: Standardize on \Drupal throughout core
Probably will still break.

Status: Needs review » Needs work

The last submitted patch, 2061899-16-global-user-comment.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs work » Needs review
FileSize
2061899-18-global-user-comment.patch3.7 KB
interdiff.txt1.59 KB

I think I got some extra stuff in the last so I undid that content_translation stuff and shuffled some stuff around a bit to see if I can get some better results.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
FileSize
interdiff.txt5.39 KB
2061899-19-global-user-comment.patch3.7 KB

better interdiff.

Status: Needs review » Needs work

The last submitted patch, 2061899-19-global-user-comment.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs work » Needs review

#19: 2061899-19-global-user-comment.patch queued for re-testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

#18: 2061899-18-global-user-comment.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2061899-19-global-user-comment.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

18 and 19 are identical but there are two more fails:S I retested both and same diff.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs work » Needs review

19: 2061899-19-global-user-comment.patch queued for re-testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

18: 2061899-18-global-user-comment.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 19: 2061899-19-global-user-comment.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs work » Needs review

19: 2061899-19-global-user-comment.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 19: 2061899-19-global-user-comment.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs work » Needs review

19: 2061899-19-global-user-comment.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 19: 2061899-19-global-user-comment.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Issue summary: View changes
Status: Needs work » Needs review
Parent issue: » #2047951: [META] Remove calls to deprecated global $user and $GLOBALS['user']
FileSize
2061899-32-global-user-comment.patch4.66 KB

Re-roll

areke’s picture

areke CreditAttribution: areke commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

This doesn't apply anymore; it needs to be re-rolled again.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2061899-34-global-user-comment.patch4.69 KB

Re-rolled again.

ryantremblay’s picture

ryantremblay CreditAttribution: ryantremblay commented
Status: Needs review » Reviewed & tested by the community

Looks good. Passes test bot.

areke’s picture

areke CreditAttribution: areke commented
Status: Needs review » Reviewed & tested by the community

RTBC'd again.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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