Let's start to convert all calls to user_access() with the new AccountInterface::hasPermission() method.

Part of #2048171: [meta] Replace user_access() calls with $account->hasPermission() wherever possible.

Change records for this issue:

CommentFileSizeAuthor
#16 drupal_2062039_16.patch17.84 KBxano
#16 interdiff.txt970 bytesxano
#14 drupal-user_replace_user_access-20620397-14.patch18.06 KBh3rj4n
#14 interdiff.txt1.33 KBh3rj4n
#7 drupal-user_replace_user_access-20620397.patch18.16 KBinternetdevels
#7 interdiff-2062039-5-7.txt10.04 KBinternetdevels
#5 drupal_2062039_5.patch22.78 KBxano
#3 drupal-user_replace_user_access-2062039-3.patch29.71 KBinternetdevels
#1 drupal-user_replace_user_access-2062039.patch27.54 KBinternetdevels

Comments

internetdevels’s picture

internetdevels commented
Status: Active » Needs review
StatusFileSize
new drupal-user_replace_user_access-2062039.patch27.54 KB

Here is the patch. It still has some unresolved issues, but lets see what testbot says.

Status: Needs review » Needs work

The last submitted patch, drupal-user_replace_user_access-2062039.patch, failed testing.

internetdevels’s picture

internetdevels commented
Status: Needs work » Needs review
StatusFileSize
new drupal-user_replace_user_access-2062039-3.patch29.71 KB

corrected

Status: Needs review » Needs work
Issue tags: -CodeSprintCIS

The last submitted patch, drupal-user_replace_user_access-2062039-3.patch, failed testing.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new drupal_2062039_5.patch22.78 KB

Re-roll.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Needs work
Issue tags: +CodeSprintCIS, +Needs reroll
Parent issue: » #2048171: [meta] Replace user_access() calls with $account->hasPermission() wherever possible.
internetdevels’s picture

internetdevels commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2062039-5-7.txt10.04 KB
new drupal-user_replace_user_access-20620397.patch18.16 KB

Status: Needs review » Needs work

The last submitted patch, 7: drupal-user_replace_user_access-20620397.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

7: drupal-user_replace_user_access-20620397.patch queued for re-testing.

EDIT HEAD was broken

xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Minor » Normal
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll 
+++ b/core/modules/user/lib/Drupal/user/Tests/UserPermissionsTest.php
@@ -34,7 +34,7 @@ function setUp() {
-   * Change user permissions and check user_access().
+   * Change user permissions and check permissions.

it was ugly before, no idea how to improve

herom’s picture

herom commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/modules/user/lib/Drupal/user/RegisterFormController.php
@@ -18,11 +18,9 @@ class RegisterFormController extends AccountFormController {
-    global $user;
+    $user = \Drupal::currentUser();

    shouldn't we use $this->currentUser() ? 

  2. +++ b/core/modules/user/lib/Drupal/user/Tests/UserPermissionsTest.php
@@ -34,7 +34,7 @@ function setUp() {
-   * Change user permissions and check user_access().
+   * Change user permissions and check permissions.

    "Test changing user permissions through the (UI|permissions page).", maybe?

andypost’s picture

andypost
he/him
Primary language Russian
commented

both nitpicks makes sense

h3rj4n’s picture

h3rj4n commented
Status: Needs work » Needs review
Issue tags: -CodeSprintCIS
StatusFileSize
new interdiff.txt1.33 KB
new drupal-user_replace_user_access-20620397-14.patch18.06 KB

As suggested in comment #12 changed the two things.

herom’s picture

herom commented
Status: Needs review » Needs work 
+++ b/core/modules/user/lib/Drupal/user/AccountFormController.php
@@ -252,6 +252,7 @@ public function form(array $form, array &$form_state) {
+      '#access' => $show_admin_language && $account->hasPermission('access administration pages'),

this line isn't necessary after last reroll. please remove.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt970 bytes
new drupal_2062039_16.patch17.84 KB
herom’s picture

herom commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
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.