Let's start to convert all calls to user_access() with the new AccountInterface::hasPermission() method.
Update: These should be rolled into only three patches, combining
Steps:
1. The first patch (comment 1) should just target places where we can easily retrieve the current user. Otherwise we would need to access the Request to retrieve the active account from that which would probably dramatically increase the patch size.
2. Create sub issues for all modules and core directories.
3. Create patches for all sub issues, get RTBC, wait for core commits (view table below).
4. Remove deprecated function user_access.
Needs works issues
Committed and pushed to 8.x.
Comment | File | Size | Author |
---|---|---|---|
#21 | drupal_core_replace_user_access_204817121.patch | 74.09 KB | InternetDevels |
Comments
Comment #1
fubhy CreditAttribution: fubhy commentedThis was done solely with a regex and replaces all occurrences of
user_access\((.*), (.*)\)
(so, just those with two arguments).Anything more complex than that most likely needs some extra code except for those cases where we have the Request object available. Will take another look (manually) for those next. But that can be a separate patch.
Comment #2
fubhy CreditAttribution: fubhy commentedAdding novice tag. This patch is so small that we should consider also covering invocations of user_access() in controllers (where we have $account always available through the request object).
Comment #4
jibranLet's postpone this till #2032553: The _account attribute on the Request is not available during web tests lands.
Comment #5
merdekiti CreditAttribution: merdekiti commentedGood issue for Code Sprint CIS
Comment #6
Berdir#1: 2048171-1.patch queued for re-testing.
Comment #7.0
(not verified) CreditAttribution: commentedadd sub issues
Comment #8
dawehnerAdd tag.
Comment #9
larowlanNote #2062805: Add a \Drupal::userAccess() to replace user_access instead of \Drupal::request()->attributes->get('_account')->hasPermission()
Comment #10
larowlanPostponed based on #2062805-11: Add a \Drupal::userAccess() to replace user_access instead of \Drupal::request()->attributes->get('_account')->hasPermission()
Comment #11
Zemelia CreditAttribution: Zemelia commented#2062805: Add a \Drupal::userAccess() to replace user_access instead of \Drupal::request()->attributes->get('_account')->hasPermission() has "closed" status because of #2062151: Create a current user service to ensure that current account is always available, so I think we can proceed with this one.
Comment #12
webchickSo I am currently spending about 50-60% of my core committing time just keeping up on these patches. :(
Since these seem to be all find/replace, with the exception of introducing a $user = Drupal::currentUser() if it's not already there, could we take the approach of one massive patch that just does them all in one go?
Comment #13
andypost@webchick only 9 was commited, most of all other issues depends on #2048223: Add $account argument to AccessCheckInterface::access() method and use the current_user service or require to inject current_user service so each one could be big for example #2061971-5: Replace user_access() calls with $account->hasPermission() in block module
I'd like have sandbox but after sprint random people file patches so I have no connection to them, I'm just reviewing patches
Comment #13.0
andypostadd steps to description
Comment #14
andypostComment #15
bohartIssue summary updated.
Sub-issues was grouped in two tables (that already done and that needs work).
Comment #16
andypostComment #17
xjmThanks for all your work on this so far! Looking over the RTBC queue, these patches are very tiny, way too small to merit one per module. It's a burden on core maintainer time. Let's combine them. If you really want to break them down into separate patches I'd suggest one for user.module, one for all modules except user.module, one for everything else. I'll mark the existing issues as duplicates of one of the outstanding ones.
Comment #18
xjmRescoping to three child issues.
Comment #19
andypostNeeds to compile a commit message from all closed issues to give commit credit to all contributors
Comment #20
webchickThat's fine; just add it to the issue summary, the core committers can copy/paste it from there.
Comment #21
InternetDevels CreditAttribution: InternetDevels commentedComment #22
andypost@Internetdevels please use sub issues for 3 patches.
The compiled commit message for modules except user in #2061977-10: Replace user_access() calls with $account->hasPermission() in all core modules except user
Comment #23
ianthomas_ukI've just marked 15 issues as duplicates of the two sub issues of this. They all had patches though, so if we find we've missed some cases there may already be patches ready. See https://drupal.org/project/issues/search/drupal?version[]=8.x&issue_tags...
Comment #24
ianthomas_ukAll child issues are fixed, and there is an RTBC issue to remove the function: #2306429: Remove user_access()