Problem/Motivation

HelpTest and HelpTopicTest both contain the following $anyUser property:

  /**
   * The anonymous user that will be created.
   *
   * @var \Drupal\user\Entity\User|false
   */
  protected $anyUser;

But this user is not and never was anonymous. The docblock was written incorrectly three years after the property's addition. See the Git archaeology in comment #6.

Proposed resolution

  • Update the short description of the properties.
  • Delete incorrect @var annotations in the HelpTest and convert them to type hints (#7).

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3617563

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

joachim created an issue. See original summary.

charlliequadros made their first commit to this issue’s fork.

charlliequadros opened merge request !16747

charlliequadros’s picture

charlliequadros commented

Hi @joachim

When it sets the current user, there is a comment saying // Log in the regular user.. This indicates that the test checks whether a regular user receives a 403 error when trying to access pages they do not have permission to access. So I updated the comments to make the test's intent clearer.

charlliequadros’s picture

charlliequadros commented
Status: Active » Needs review
dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community

I was able to perform some successful Git archaeology on this property. The original $any_user property was added to the Help simpletest by Dries back in 2008. Aside from getting moved around, changed to camelCase, and copied to the help topic test; it and the access check that uses it have remained unchanged since then. The docblock was added in 2011 as part of an effort to fix Help module API documentation. Unfortunately, they got this one wrong. It never was an anonymous user. @charlliequadros's changes are correct.

mstrelan’s picture

mstrelan commented
Status: Reviewed & tested by the community » Needs work

While we're at it let's fix the @var type. Since #3560202: UserCreationTrait::create* methods can never return false $this->createUser() cannot return FALSE (it never could anyway). Plus $this->setCurrentUser() doesn't accept FALSE either.

If we're fixing this @var type we should fix the same for $adminUser. I would also go as far to say we should swap these to be native typed properties instead of @var docs.

mstrelan’s picture

mstrelan commented
Priority: Major » Normal
charlliequadros’s picture

charlliequadros commented
Status: Needs work » Needs review
mstrelan’s picture

mstrelan commented

Looks great. Can we update the IS to match?

dcam’s picture

dcam commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Can we update the IS to match?

I took care of it.

Looks good to me too. Back to RTBC.

  • godotislate committed 7156a086 on main 
    fix: #3617563 HelpTest docs say it's working with the anonymous user,...
godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Fixed

Not sure this is worth going to 11.x, so committed 7156a08 and pushed to main. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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