Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
API page: https://api.drupal.org/api/drupal/core%21modules%21user%21user.api.php/f...
> \Drupal\Core\Session\AccountInterface $account: The user object on which the operation is being performed.
That's not true -- AccountInterface is not a 'user object'.
Also, the docs should mention that this hook is invoked both for user entities and the anonymous user session object.
Comment | File | Size | Author |
---|---|---|---|
#16 | fix-doc-3000240-16.diff | 1.49 KB | govind.maloo |
#11 | fix-doc-hook_user_format_name_alter-3000240-12.patch | 1.52 KB | msankhala |
#4 | interdiff_3_4.txt | 838 bytes | philipnorton42 |
#4 | document_hook_user_format_name_alter-3000240-4.patch | 869 bytes | philipnorton42 |
#3 | document_hook_user_format_name_alter-3000240-3.patch | 713 bytes | izus |
Comments
Comment #2
gnugetComment #3
izus CreditAttribution: izus commentedhi,
here is a patch for it
Thanks
Comment #4
philipnorton42 CreditAttribution: philipnorton42 as a volunteer commentedAfter reviewing where places where this hook is called from the $account object is either a session object (implementing \Drupal\Core\Session\AccountInterface) or a user object (implementing \Drupal\user\UserInterface). So this will not necessarily be an anonymous user object.
Updated the docs for this hook.
Comment #5
scott_euser CreditAttribution: scott_euser as a volunteer and at Soapbox Communications Ltd commentedThis change looks good to me, but perhaps we should clarify that ultimately either case is an AccountInterface? Or is that implied? In the end the issue is that the description says this is a user object always when it isn't so this change solves it +1 for RTBC for that reason.
Comment #6
philipnorton42 CreditAttribution: philipnorton42 as a volunteer commentedGood point, but I think it's important to have a distinction between AccountInterface and UserInterface as they have different functions available and as such should be documented well.
When you look at the user profile page this hook is called twice, once for the user profile on the page (which receives UserInterface) and once for the username in the toolbar (which receives AccountInterface). So an example of something that might break is using isActive() to show the status of a user in this hook. This function doesn't exist in AccountInterface and as such shouldn't be relied on in all circumstances.
Comment #7
scott_euser CreditAttribution: scott_euser as a volunteer and at Soapbox Communications Ltd commentedMakes sense! So specifying both hints to the developer appropriately that they should not expect all methods to be there (like isActive()). Changing to RTBC
Comment #9
philipnorton42 CreditAttribution: philipnorton42 as a volunteer commentedThe tests failed due to Selenium falling over, which caused the status to go back to "Needs work". This didn't make sense as I hadn't actually changed any code so I re-ran the tests.
Rerunning the tests allowed them to pass :)
Popping it back to RTBC.
Comment #10
alexpottI think this part of the change is a bit odd. Implementations should only ever rely on
Drupal\Core\Session\AccountInterface
methods. I can see some benefit in making it clear that you might want to fo something likeif instanceof UserInterace
... /me shrugs.It's not a user session object. The docs from AccountInterace are
Defines an account interface which represents the current user.
.I think maybe we can do better here.
Comment #11
msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commentedHere is an updated patch.
Comment #12
linichalexey CreditAttribution: linichalexey at EPAM Systems commentedComment #13
linichalexey CreditAttribution: linichalexey at EPAM Systems commentedComment #14
linichalexey CreditAttribution: linichalexey at EPAM Systems commentedPatch serves the purpose as suggested in #10
Comment #15
catchThe patch doesn't resolve the feedback from #10, it's still talking about 'current user session object'. Also found a couple of other issues:
Should be 'an implementation'.
Also 'user entity methods'.
Comment #16
govind.maloo CreditAttribution: govind.maloo commentedComment #17
govind.maloo CreditAttribution: govind.maloo commentedComment #19
govind.maloo CreditAttribution: govind.maloo commentedComment #20
msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commentedThe patch in #16 LGTM. It fixes the problem mentioned in #15. @govind.maloo Nice work. Just a note whenever you are uploading interdiff use the .txt extension instead of .diff or .patch so that testbot do not run a test for interdiff.
Comment #21
alexpottLooking close, I think
anonymous user session object
is okay. As basically this is passed the result of\Drupal\Core\Session\AccountProxy::getAccount()
. Which does:Comment #22
alexpottAs @catch said
user entity methods
The indentation here should be:
Fixed the above on commit.
Committed and pushed b6208e5225 to 8.7.x and ad9f72e1bd to 8.6.x. Thanks!
Backported to 8.6.x as an docs update.
Comment #25
govind.maloo CreditAttribution: govind.maloo at Salsa Digital commented