Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
block.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
10 Aug 2013 at 09:07 UTC
Updated:
29 Jul 2014 at 22:45 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
internetdevels commentedHere is the patch. $request haven't contained information about user, so I've used Drupal::request()->attributes->get('_account') instead.
Comment #2
andypostUse Drupal::currentUser() service https://drupal.org/node/2032447
Comment #3
naveenvalechaThe #1 was no longer applied.So I rewrote the patch.Got testbot go !
Comment #4
naveenvalechaReviewed my patch and found that there are white space errors.
So rerolled the patch again after removing white space errors.
Comment #5
andypostForms now have helper method, use $this->getCurrentUser()
This is a plugin, so 'current_user' service needs injection.
Just add $container->get('current_user') to create.
And AccountInterface $account to __construct()
This could be solved after #2048223: Add $account argument to AccessCheckInterface::access() method and use the current_user service
I see no reason
Comment #6
andypostI'd like to postpone this on #5.3 but please roll patches for injection
Comment #7
naveenvalecha@andypost Thanks for checking me out.Here's the updated patch and interdiff.
Comment #9
andypostOverall looks good, just somehow tests broken
Just a nutpick:
needs space after ',' and remove trailing white space
Comment #10
naveenvalechaAdded the suggestions of #9 in the new patch.I think for this little change interdiff is not required.
@andypost, I think my IDE needs some configuration that is showing trailing space errors.Also let me know if there is other IDE which is better than Netbeans.
Comment #11
naveenvalechaComment #13
internetdevels commentednext one :)
Comment #14
andypost#13: drupal-block_replace_user_access_2061971-13.patch queued for re-testing.
Comment #15
benjy commentedNeeds reroll.
Comment #16
tim.plunkettThis will be removed shortly in #2115077: Remove _block_themes_access()
All of this will be handled by #1951386: Extend BlockPluginInterface::access to allow passing in an account
When would this be empty?
Comment #17
herom commentedrerolled + removed the
!empty($this->account)condition (#16.3)1. and 2. both got in, so the patch size is pretty smaller.
Comment #18
benjy commentedDoes $account need a default of null?
Comment #19
herom commentedprobably not. The added doc mentions the parameter as "optional", but the code doesn't seem to provide a default, and it doesn't seem necessary.
Comment #20
benjy commentedLooks good to me.
Comment #21
xano19: drupal-block_replace_user_access_2061971-19.patch queued for re-testing.
Comment #22
xano19: drupal-block_replace_user_access_2061971-19.patch queued for re-testing.
Comment #23
catch19: drupal-block_replace_user_access_2061971-19.patch queued for re-testing.
Comment #25
herom commentedreroll (auto-merged).
the changes are minimal, so keeping the rtbc status.
strange though that the failed test didn't move it back to "needs work". it's gonna take some time to tame the d.o horse again.
Comment #26
herom commentedlet's "needs review" this. maybe it will start testing.
Comment #27
andypostback to rtbc
Comment #28
webchickCommitted and pushed to 8.x. Thanks!