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 template
  • Write automated tests
  • Update docblocks of all user.html.twig files

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

spesic’s picture

spesic’s picture

spesic’s picture

Issue summary: View changes
steveoliver’s picture

Status: Active » Needs review
Issue tags: +Twig

Proposed 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.

davidhernandez’s picture

Category: Feature request » Bug report

In this previous issue field_attach_preprocess() was removed. This orphaned the $account variable. #2157153: Remove field_attach_preprocess()

Status: Needs review » Needs work

The last submitted patch, 1: proposed-solution-1_user_pages-2385243-1.patch, failed testing.

star-szr’s picture

Issue tags: +Needs tests

Nice find! This should get some test coverage.

laughnan’s picture

This (#2) looks good, @spesic!

spesic’s picture

spesic’s picture

Status: Needs work » Needs review

Hide proposed solution #1 patch in favor of #2, trigger tests...

star-szr’s picture

Status: Needs review » Needs work
Issue tags: +Novice

Contributor documentation on writing automated tests:

https://www.drupal.org/contributor-tasks/write-tests

iMiksu’s picture

Assigned: Unassigned » iMiksu

I start with the automated test.

iMiksu’s picture

Assigned: iMiksu » Unassigned
FileSize
580 bytes

Actually, 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.

iMiksu’s picture

Berdir’s picture

Our 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?

aneek’s picture

@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

\Drupal::service('theme_handler')->install(array('user_test_theme'));
\Drupal::theme()->setActiveTheme(\Drupal::service('theme.initialization')->initTheme('user_test_theme'));
$this->container->set('theme.registry', NULL);

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 :-)

aneek’s picture

Assigned: Unassigned » aneek
aneek’s picture

As 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. :-)

aneek’s picture

Status: Needs work » Needs review

The last submitted patch, 19: core_user_fields_available-test_only_fail-2385243-19.patch, failed testing.

Berdir’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/user/src/Tests/UserFieldsTest.php
    @@ -0,0 +1,55 @@
    + * @file
    + * Definition of Drupal\user\Tests\UserFieldsTest.
    

    Contains \Drupal\...

  2. +++ b/core/modules/user/src/Tests/UserFieldsTest.php
    @@ -0,0 +1,55 @@
    +class UserFieldsTest extends KernelTestBase {
    +  /**
    

    Missing empty line

Looks good to me otherwise.

aneek’s picture

aneek’s picture

Status: Needs work » Needs review

star-szr’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Thanks 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:

  1. +++ b/core/modules/user/src/Tests/UserFieldsTest.php
    @@ -0,0 +1,56 @@
    +    $output = drupal_render($build);
    

    I don't think we should add calls to deprecated functions. \Drupal::service('renderer')->render please.

  2. +++ b/core/modules/user/tests/themes/user_test_theme/user.html.twig
    @@ -0,0 +1,33 @@
    + * Default theme implementation to present all user data.
    

    This isn't the default theme implementation, please change this to indicate it's for testing :)

  3. +++ b/core/modules/user/tests/themes/user_test_theme/user.html.twig
    @@ -0,0 +1,33 @@
    + * @ingroup themeable
    

    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...

iMiksu’s picture

Assigned: aneek » iMiksu
iMiksu’s picture

Assigned: iMiksu » Unassigned
Issue summary: View changes
Issue tags: -Needs issue summary update

Here'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.

iMiksu’s picture

Forgot attaching the patches in #28 :S Here they are

lauriii’s picture

Status: Needs work » Needs review
star-szr’s picture

The docs don't have to be super extensive, just documenting the new top level variable would be an improvement. Thanks!

lauriii’s picture

Status: Needs review » Needs work

Thanks for working on this issue. I found few things that might be good to change still:

  1. +++ b/core/modules/user/src/Tests/UserFieldsTest.php
    @@ -0,0 +1,56 @@
    +  public static $modules = array('user', 'system');
    

    New array syntax FTW

  2. +++ b/core/modules/user/src/Tests/UserFieldsTest.php
    @@ -0,0 +1,56 @@
    +    $this->assertText($userEmail, 'User\'s mail field is found in the twig template');
    

    Use "" instead to avoid unnecessay escaping \'

tohesi’s picture

Assigned: Unassigned » tohesi
tohesi’s picture

Assigned: tohesi » Unassigned
Status: Needs work » Needs review
FileSize
5.13 KB
832 bytes

Updated docblocks indicating the new user variable and updated UserFieldsTest.php as per lauriii's comments.

lauriii’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

There is test coverage. Last patch fixes my nits and adds the documentation (even though that is not on interdiff).

star-szr’s picture

Status: Reviewed & tested by the community » Needs work

Some points below from my review, new patch and interdiff please, then I think it's ready to fly!

  1. +++ b/core/modules/user/src/Tests/UserFieldsTest.php
    @@ -0,0 +1,56 @@
    +    // Setup a test theme that prints the user's mail field.
    

    Minor: Setup is a noun, what we're looking for here is "Set up".

  2. +++ b/core/modules/user/src/Tests/UserFieldsTest.php
    @@ -0,0 +1,56 @@
    +  }
    +}
    

    Minor: Missing blank line between last method and end of class per https://www.drupal.org/node/608152#indenting.

  3. +++ b/core/modules/user/tests/themes/user_test_theme/user.html.twig
    @@ -0,0 +1,32 @@
    + * Theme override for testing presence all user data.
    

    "for testing the presence of all user data" would be a bit less terse here.

  4. +++ b/core/modules/user/tests/themes/user_test_theme/user_test_theme.info.yml
    @@ -0,0 +1,11 @@
    +stylesheets-remove:
    +  - system.module.css
    +regions:
    +  content: Content
    +  left: Left
    +  right: Right
    

    These lines should be removed, they have nothing to do with what is being tested as far as I can tell.

deepakaryan1988’s picture

Assigned: Unassigned » deepakaryan1988
deepakaryan1988’s picture

Assigned: deepakaryan1988 » Unassigned
Status: Needs work » Needs review
FileSize
11.32 KB
11.97 KB

@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.

Status: Needs review » Needs work

The last submitted patch, 38: core_user_fields_available-2385243-38.patch, failed testing.

subhojit777’s picture

Assigned: Unassigned » subhojit777

Working on the failing tests.

subhojit777’s picture

Assigned: subhojit777 » Unassigned
Status: Needs work » Needs review
FileSize
2 KB
5.04 KB

Seems like patch in #38 is created incorrectly. Changes suggested in #36 are made.

lauriii’s picture

Status: Needs review » Needs work
Issue tags: +Needs beta evaluation
subhojit777’s picture

Assigned: Unassigned » subhojit777
star-szr’s picture

+++ b/core/modules/user/tests/themes/user_test_theme/user.html.twig
@@ -1,7 +1,7 @@
- * Theme override for testing presence all user data.
+ * Theme override for testing the presence all user data.

Missing a word still: "…the presence of…"

Other than that the changes look great, thank you @subhojit777.

subhojit777’s picture

Issue summary: View changes
subhojit777’s picture

Issue summary: View changes
subhojit777’s picture

subhojit777’s picture

Assigned: subhojit777 » Unassigned
Status: Needs work » Needs review
davidhernandez’s picture

+++ b/core/modules/user/tests/themes/user_test_theme/user.html.twig
@@ -0,0 +1,32 @@
+    <p>{{- user.mail.value -}}</p>

Are these whitespace modifiers necessary, especially given this template is just for the test?

joelpittet’s picture

Status: Needs review » Needs work

They 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:)

subhojit777’s picture

Assigned: Unassigned » subhojit777
subhojit777’s picture

Assigned: subhojit777 » Unassigned
Status: Needs work » Needs review
FileSize
5.04 KB
564 bytes
subhojit777’s picture

Issue summary: View changes
deepakaryan1988’s picture

I am applied patch which is in #52 and it's working fine.

Please someone else review it also.

Petr Illek’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
126.86 KB

I've applied the patch in #52 and its working for me too.
See attached screenshot with before&after state.
Setting to RTBC.

rteijeiro’s picture

Issue tags: +drupalcampcs

RTBC++

Tagging as DrupalCampCS sprint.

Great job reviewing Petr!

lauriii’s picture

Just 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.

alexpott’s picture

Category: Bug report » Task
Status: Reviewed & tested by the community » Fixed

This 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!

  • alexpott committed ec03242 on 8.0.x
    Issue #2385243 by aneek, subhojit777, iMiksu, spesic, deepakaryan1988,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.