Problem/Motivation
I need to be able to access user information for use within user.html.twig. Currently only custom fields (admin/config/people/accounts/fields) are available. Core fields like uid and username are not available.
Solution
Taking a look into user.html.twig's preprocess function template_preprocess_user(), account info is stored in $account on the first line, but nothing is being done at this point.
I have 2 propositions:
#1) proposed-solution-1_user_pages-2385243-1.patch:
Add uid, name_raw, name, email_raw, email to content (similarly to template_preprocess_username())
#2) proposed-solution-2_user_pages-2385243-1.patch:
Expose entire account object
Remaining tasks
Make user object available to twig templateWrite automated tests- Update docblocks of all user.html.twig files
Beta phase evaluation
Issue category | Bug because only custom fields (admin/config/people/accounts/fields) are available in user.html.twig , core fields like uid and username are not available. |
---|---|
Issue priority | Not critical because the bug is not a blocker, and can be pushed later. |
Unfrozen changes | Unfrozen because it will only add user information in user.html.twig . Adds core fields like uid and username. |
Prioritized changes | The main goal of this issue is improving developer experience. After pushing the code, developers will be able to access core user fields like uid, username in user.html.twig |
Comment | File | Size | Author |
---|---|---|---|
#55 | patch-2385243-52-applied.jpg | 126.86 KB | Petr Illek |
#52 | interdiff-2385243-47-52.txt | 564 bytes | subhojit777 |
#52 | make_core_user_fields-2385243-52.patch | 5.04 KB | subhojit777 |
#47 | interdiff-2385243-41-47.txt | 587 bytes | subhojit777 |
#47 | make_core_user_fields-2385243-47.patch | 5.04 KB | subhojit777 |
Comments
Comment #1
spesic CreditAttribution: spesic commentedComment #2
spesic CreditAttribution: spesic commentedComment #3
spesic CreditAttribution: spesic commentedComment #4
steveoliver CreditAttribution: steveoliver commentedProposed solution #2 looks good to me. As $account was unused I think $variables['account'] = $variables['elements']['#user'] is what line 1 was supposed to be in the first place.
Comment #5
davidhernandezIn this previous issue field_attach_preprocess() was removed. This orphaned the $account variable. #2157153: Remove field_attach_preprocess()
Comment #7
star-szrNice find! This should get some test coverage.
Comment #8
laughnanThis (#2) looks good, @spesic!
Comment #9
spesic CreditAttribution: spesic commentedComment #10
spesic CreditAttribution: spesic commentedHide proposed solution #1 patch in favor of #2, trigger tests...
Comment #12
star-szrContributor documentation on writing automated tests:
https://www.drupal.org/contributor-tasks/write-tests
Comment #13
iMiksuI start with the automated test.
Comment #14
iMiksuActually, I did not as #2407489: Remove user.pages.inc got in by removing whole
user.pages.inc
, so I re-rolled this. It seems that they dropped of the variable completely in comment #6.Comment #15
iMiksuComment #16
BerdirOur user/account naming is a mess, but the entity type is user, the template is called user.html.twig, I would suggest to make this available as user, not account.
Pretty sure we need to update the documentation in user.html.twig as well?
Comment #17
aneek CreditAttribution: aneek commented@Berdir, I don't think that we need all the fields that are stored in the
$variables['elements']['#user']
array.Refer to the paste for the dump. So any thoughts?
I am writing a test case as per our little discussion over the IRC and following simplenews module for references on the test theme. But I have run to an issue. Described below,
Created a test theme "user_test_theme" in "/core/modules/user/tests/themes/" and in the test case activated the theme as
The theme is loaded I can see that with
$themes = \Drupal::service('theme_handler')->listInfo();
&$templates = $this->container->get('theme.registry')->getRuntime();
dumps but the text printed in user.html.twig in the test theme is not printed in test result verbose message.I am attaching a difftext for more reference. Help please :-)
Comment #18
aneek CreditAttribution: aneek commentedComment #19
aneek CreditAttribution: aneek commentedAs per discussion with @Berdir, uploading a test case that will validate the user fields available in twig template.
Please review if the bot returns green. :-)
Comment #20
aneek CreditAttribution: aneek commentedComment #22
BerdirContains \Drupal\...
Missing empty line
Looks good to me otherwise.
Comment #23
aneek CreditAttribution: aneek commented@Berdir, fixed.
Comment #24
aneek CreditAttribution: aneek commentedComment #26
star-szrThanks all.
First - It looks like we should be updating the docblocks in all our user.html.twig templates to indicate the new variable is available. Other than that, some nits:
I don't think we should add calls to deprecated functions.
\Drupal::service('renderer')->render
please.This isn't the default theme implementation, please change this to indicate it's for testing :)
Please remove @ingroup themeable, this is for a test so it shouldn't be part of the themeable group: https://api.drupal.org/api/drupal/core%21modules%21system%21theme.api.ph...
Comment #27
iMiksuComment #28
iMiksuHere's changes to #26.1, #26.2, #26.3.
I'm actually struggling to dump out variables from twig template, so no changes related to updating variables to docblocks.
Comment #29
iMiksuForgot attaching the patches in #28 :S Here they are
Comment #30
lauriiiComment #31
star-szrThe docs don't have to be super extensive, just documenting the new top level variable would be an improvement. Thanks!
Comment #32
lauriiiThanks for working on this issue. I found few things that might be good to change still:
New array syntax FTW
Use "" instead to avoid unnecessay escaping \'
Comment #33
tohesi CreditAttribution: tohesi commentedComment #34
tohesi CreditAttribution: tohesi as a volunteer commentedUpdated docblocks indicating the new user variable and updated UserFieldsTest.php as per lauriii's comments.
Comment #35
lauriiiThere is test coverage. Last patch fixes my nits and adds the documentation (even though that is not on interdiff).
Comment #36
star-szrSome points below from my review, new patch and interdiff please, then I think it's ready to fly!
Minor: Setup is a noun, what we're looking for here is "Set up".
Minor: Missing blank line between last method and end of class per https://www.drupal.org/node/608152#indenting.
"for testing the presence of all user data" would be a bit less terse here.
These lines should be removed, they have nothing to do with what is being tested as far as I can tell.
Comment #37
deepakaryan1988Comment #38
deepakaryan1988@Cottser
Address in the points which you mentioned for #2385243: Make core user fields available for twig templates
I have only change the point #1,2,4 as #3 I am not able to understand. Please explain that.
Comment #40
subhojit777Working on the failing tests.
Comment #41
subhojit777Seems like patch in #38 is created incorrectly. Changes suggested in #36 are made.
Comment #42
lauriiiComment #43
subhojit777Comment #44
star-szrMissing a word still: "…the presence of…"
Other than that the changes look great, thank you @subhojit777.
Comment #45
subhojit777Comment #46
subhojit777Comment #47
subhojit777Comment #48
subhojit777Comment #49
davidhernandezAre these whitespace modifiers necessary, especially given this template is just for the test?
Comment #50
joelpittetThey are not necessary and given that the tags around them have no white space before or after it will not do anything.
FYI those modifiers don't trim inside the variable. If the variable contains whitespace it will persist even with the modifiers
Nice to see more tests! woo:)
Comment #51
subhojit777Comment #52
subhojit777Comment #53
subhojit777Comment #54
deepakaryan1988I am applied patch which is in #52 and it's working fine.
Please someone else review it also.
Comment #55
Petr IllekI've applied the patch in #52 and its working for me too.
See attached screenshot with before&after state.
Setting to RTBC.
Comment #56
rteijeiro CreditAttribution: rteijeiro at Tieto commentedRTBC++
Tagging as DrupalCampCS sprint.
Great job reviewing Petr!
Comment #57
lauriiiJust as a clarification for a commiter this patch doesn't include any visual changes. The pictures on #55 have been taken with a modified template where data is tried to be accessed through user object.
Comment #58
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. At first look this appears to be more a task but #5 explains why it is. Committed ec03242 and pushed to 8.0.x. Thanks!