Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Here the permission is called "update terms in":
/**
* Implements \Drupal\Core\Entity\EntityAccessControllerInterface::updateAccess().
*/
public function updateAccess(EntityInterface $entity, $langcode = LANGUAGE_DEFAULT, User $account = NULL) {
return user_access("update terms in {$entity->bundle()}", $account) || user_access('administer taxonomy', $account);
}
and here it's called "edit terms in":
$permissions += array(
'edit terms in ' . $vocabulary->id() => array(
'title' => t('Edit terms in %vocabulary', array('%vocabulary' => $vocabulary->name)),
),
);
shouldn't this be both update?
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | 1966614-16.patch | 6.32 KB | jibran |
| #16 | 1966614-16-test-only-patch.patch | 5.45 KB | jibran |
| #16 | interdiff.txt | 735 bytes | jibran |
| #13 | 1966614-13-test-only-patch.patch | 5.45 KB | jibran |
| #13 | 1966614-13.patch | 6.31 KB | jibran |
Comments
Comment #1
Johnny vd Laar commentedIssue was found in this issue:
https://drupal.org/node/1862758#comment-7261618
http://drupal.org/node/1848686#comment-7261878
A patch that fixes this and adds extra test coverage is added here:
http://drupal.org/node/1848686#comment-7261878
Comment #2
Johnny vd Laar commentedI guess this should be a major issue because now the permissions don't work properly?
Comment #3
berdirAs that other issue is a feature request, which are blocked by thresholds, it's basically blocking itself with this issue :)
You should extract the fix and a test for it and provide a patch here so that we can get it asap. Maybe the permission check should be updated and not hook_permission() ?
Comment #4
Johnny vd Laar commentedNode module also has "edit" so I guess it should be "edit" here as well. I'll add a patch here that also adds tests.
Comment #5
Johnny vd Laar commentedAdded is a patch that should fix the permission check and that adds the proper tests.
Comment #6
berdirShould be Contains \Drupal\... now according to the current coding standards (there are a lot of old classes that don't follow this yet)
You can use checkPermissions(array(), TRUE) to avoid resetting the complete static cache.
Comment is outdated now :)
Tests look nice otherwise, and thorough. Can you also upload a patch that only contains the tests so that we can see that it does fail and covers the bug.
Comment #7
Johnny vd Laar commentedSorry can you explain what this means:
Should be Contains \Drupal\... now according to the current coding standards (there are a lot of old classes that don't follow this yet)
Attached is the test that should fail
Comment #8
berdirIn the @file docblock, write "Contains \Drupal\Core\..." instead of "Definition of Drupal\Core"... that's all :)
Comment #10
Johnny vd Laar commentedOk here is the modified patch with 2 changed comments.
Comment #11
Johnny vd Laar commentedComment #12
berdirSee above, this should call checkPermissions() so that it only resets the permission static cache.
Otherwise, this looks good. Please also upload an interdiff so that it's visible what changed between two patches.
Comment #13
jibranUpdated as per #12
Comment #15
berdir$this->checkPermissions(), it's a method, not a function :)
Comment #16
jibranThanks @Berdir for pointing that out. I should have run the test locally first. It is passing on local now.
Comment #17
jibranSending for test.
Comment #19
berdirFixes a bug and comes with good test coverage. Looks good to me :)
Edit: it was set to needs work because of the test-only patch. If you upload the test-only patch first, then, in most cases, the issue will stay at needs review because only the last-returned patch in an issue changes the status.
Comment #20
webchickWow. Good catch, AND that's awesome to have so many tests to make sure that it never, ever happens again! :D
Committed and pushed to 8.x. Thanks!