Problem/Motivation

Error: Call to a member function getRoles() on null in Drupal\posthog\UserAttributesProvider->shouldUserBeIdentified() (Zeile 42 in /web/modules/contrib/posthog/src/UserAttributesProvider.php).

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork posthog-3586046

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

anybody created an issue. See original summary.

anybody opened merge request !49

  • anybody committed 3b6fe3bb on 2.x 
    feat: #3586046 Error: Call to a member function getRoles() on null in...
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil
Status: Active » Needs review

Creating a 2.1.2 hotfix release. @grevil please check if the correction is fine afterwards.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue tags: +Needs tests

The error appeared on login. Do we already have a test testing the login in combination with the selected "always identify" roles and / or the advanced identification?

If not, we should add both as separate and combined tests with only "authenticated" and with a custom role.

grevil’s picture

grevil commented
Status: Needs review » Needs work

Thanks @anybody!

The old implementation of "shouldUserBeIdentified", looks really prone to errors... No idea what I thought when I implemented this... Best thing is, that "shouldUserBeIdentified" is never called with the user argument 😅.

Your change LGTM and totally makes sense. It just worked before, because "getUserDistinctIdBySetting" used currentUser as a fallback itself, when we passed the NULL user in the past.

Going to add a simple login test for posthog_php

grevil’s picture

grevil commented
Assigned: grevil » Unassigned
Status: Needs work » Fixed
Issue tags: -Needs tests

I'll add the test through #3586186: "identify_users_on_login" setting form element missing.

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.