Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syncing first/last name fields on an existing user without a fullName mapping will not update fullName #32

Open
Braedan opened this issue Jun 4, 2024 · 2 comments

Comments

@Braedan
Copy link

Braedan commented Jun 4, 2024

Describe the bug

When syncing an existing user profile, if fullName is set and there's no mapping set on the provider (which is the case for Google, and likely others), it can fall out of sync with the firstName and lastName fields.

Craft handles this on User::afterSave via NameTrait::prepareNamesForSave(), but As of 4.9 only if fullName or first/last name is null.

(Sorry in advance if this should be a feature request; its hard to tell what the expected behaviour should be)

Steps to reproduce

  1. Set populateProfile and syncProfile to true in social settings
  2. Create a user that will share an email with an SSO login, set Full Name to fName lName (assuming SSO first/last name isn't the same)
  3. Log in via SSO to any provider that has fieldMapping for firstName and lastName but not fullName (eg; Google provider)
  4. Updated User element will save, but Full Name will not be synced with new first name / last name values

Craft CMS version

4.9.5

Plugin version

1.0.15

Multi-site?

No

Additional context

To fix, could use User::setAttributes() instead of manually assigning values onto the user object in services\Users::_syncUserProfile(). The function handles firstName / lastName logic (see: https://github.com/craftcms/cms/blob/5.x/src/elements/User.php#L995-L1006).

Alternatively, adding an event onto _syncUserProfile where I could manually set fullName = null would be handy, as right now I'm resolving by emulating the setAttributes conditional on User::EVENT_BEFORE_SAVE

@engram-design
Copy link
Member

I suppose this is true for anything when syncProfile is true - where should be the source of truth? If it's always the offsite provider, then yes we should overwrite the firstName, lastName and fullName.

The reason we use a stepped process in _syncUserProfile() is so that we can handle and issues with applying attributes or fields. Using setAttributes() means it's all-or-nothing.

All too happy to add an event here to help.

@Braedan
Copy link
Author

Braedan commented Jun 5, 2024

Hey thanks for the swift response! I think you're likely right, if syncProfile is true, or even populateProfile I'd assume the source of truth should be coming from the SSO provider.

I agree it would suck to lose the current benefits _syncUserProfile() offers. I guess the alternative would be implementing the same conditional logic Craft uses in setAttributes(), but understandable it that's not something that's warranted.

An event would definitely help my use-case, though my current workaround works well enough that I'm not desperate for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants