Primary Email update about civicrm-wp-profile-sync HOT 39 CLOSED

@christianwach I understand that the UFMatch and the attribute comments would be a core issue/change. I was just commenting and rationalizing against this issue I added in. I am not as familiar with core history and code as you are and really thought the uf_name was something utilized more but apparently it isn't.

I appreciate the time on this and enjoy having the community around core that help make this more useful.

from civicrm-wp-profile-sync.

@rbaugh Sorry for the late reply - holidays eh?

Shouldn't the UFMatch.name be updated as well since this is the email that ideally is associated with the WP user?

Yes, it should be updated.

Are you raising the issue here because this plugin doesn't update it?
Or because your CiviRule doesn't update it - and when it doesn't, then sync via this plugin fails?

from civicrm-wp-profile-sync.

@christianwach NP, I understand.

I was raising the issue as the UFMatch.name is not getting updated for me. So I am having to update the email in Civi, double check the WP profile and finally update UFMatch through API.

I only mentioned the WPCiviRules in that we use it in email templates to remind people of their login at times and when it doesn't get updated, it causes issues. Plus changing a primary email for someone and this not getting updated gives us issues when someone wants to get back in and uses the new email for a forgot password.

from civicrm-wp-profile-sync.

It doesn't happen often, but does occur at times.

@rbaugh Do you have any idea of the context in which this happens? Is there anything related in your logs?

from civicrm-wp-profile-sync.

@rbaugh FYI this sounds really hard to "blind debug"... I'd recommend putting code into your system that logs when the UFMatch table is updated and there's no email remaining in the record. The backtrace from that should tell you where the issue stems from.

One final thought - do you have any CiviRules that are unfinished? If so, delete them or finish configuring then - I've seen this cause all sorts of problems in various installs.

from civicrm-wp-profile-sync.

@christianwach I will check logs and see if I can find anything.

Overall, whenever I change the primary email for a contact, the WP profile is updated but not the UFMatch. This is consistently failing on the update, but I will need to see if there is anything in the logs. I will also see if I can debug this and if I find a culprit, I can look to submit an update or PR

from civicrm-wp-profile-sync.

@rbaugh Thanks for this - I'll check later at my end and try to replicate.

from civicrm-wp-profile-sync.

@rbaugh Okay, I've replicated this now - thanks for the report. I suspect this hasn't come up before because the UFMatch Email is rarely used - people probably simply use the Email attached to the Contact record. Indeed, in multi-domain situations, there are multiple UFMatch records and it then becomes tricky to decide which one to use.

I'll see if I can implement a quick fix - however it's a bit of a can of worms, this one... I'm not actually happy with the way that the CiviCRM Primary Email syncs with the WordPress User Email. I actually prefer the idea that this plugin allows an Email Location Type to be chosen specifying the Email to be synced - the same way it does for the Website Type. In many cases, it is not desirable for the WordPress User Email to be changed when changing the a Contact's Primary Email.

from civicrm-wp-profile-sync.

FWIW I'd also suggest that the WPCiviRules Extension's token logic is somewhat shaky:

• It doesn't use standard WordPress functions (e.g. wp_generate_password() thus bypassing the random_password filter.
• It conflates uf_username with uf_name which are definitely not the same thing - the former is an actual username (e.g. johndoe) while the latter is an Email.
• The $ufmatch data retrieved from the CiviCRM API contains the uf_id through which the canonical data could be retrieved from WordPress.

That aside, I still agree that this plugin should update the uf_name value correctly.

from civicrm-wp-profile-sync.

@christianwach I am glad you caught the issue. I could use another option for the token, but had already set it up and realized the problem as the email address in the body of the email was not matching the WP account.

I can see your point about npt updating WP, but if a member is updated in Civi and then tries to do a PW reset with the new email, it wouldn't work. One of our clients goes in and updates the member records as they do change emails because of schooling so having things sync and not have to make multiple edits is nice.

I can also create my own token for this, but was just using what was available. It wasn't until I had an issue that I realized it was pulling from the UFMatch and not as you stated by using the contact_id to derive the uf_id and pull the data from WP. This also becomes an issue when you merge contacts as the UF_Match doesn't update to use the primary either. Seems like it should just not set anything and just be a lookup only.

from civicrm-wp-profile-sync.

@rbaugh My issue with the Primary Email being synced to the WordPress User is that it doesn't properly take into account the logic in CiviCRM that is used in Mailings. Someone may wish to change the Email they want to receive Mailings at but not wish to change their WordPress Email. My goal here is to suggest people use a custom Location Type (e.g. "CMS" or "WordPress" or similar) and then sync that Email, leaving the CiviCRM Mailing logic out of the equation.

from civicrm-wp-profile-sync.

And then there's the added complication of the "Bulk Email" attribute 😆 - as I said, it's a can of worms!

from civicrm-wp-profile-sync.

The logic appears to be this...

If you are using the CiviMail component to send mailings to contacts, this field (the Bulk Mailings checkbox) provides additional control over which email address is used. By default, CiviMail sends mail to each contact's preferred email address. If the contact has multiple locations, then the preferred email of the primary location is used. However, if the contact prefers to have CiviMail ('bulk') mailings set to an alternate email address - check the 'Use for Bulk Mailings' box next to that email address.

And of course multiple Emails can be selected for Bulk Mailings... which makes me think that the custom Location Type sync is probably the lightest touch sync. Your CiviCRM admins would still be able to update the WordPress User Email from CiviCRM, they'd just be doing it without affecting the Contact's choices regarding Mailings.

from civicrm-wp-profile-sync.

This also becomes an issue when you merge contacts as the UF_Match doesn't update to use the primary either.

@rbaugh Just swerving back to this for a moment... do you mean this a result of this plugin overlooking the UFMatch Email update or is this something that happens in CiviCRM more generally?

from civicrm-wp-profile-sync.

The mailings itself isn't the issue, at least not what sent got me to this issue ticket. We have an email template that is using the CiviRule token in the footer as a reminder to someone what email they used for when they are logging into their account. While the email address this message is being sent to may be the same email address, it also may not be and we are just adding it in as a reminder.

The issue for us was that the UFMatch wasn't updated and the token was pulling from UFMatch and providing the old email their WP profile was connected to. So if they used that email to do a PW reset, it wouldn't work as the email had been updated through Civi and synced to the WP profile.

We were using the WPCiviRules to create the WP account and so the member doesn't really know their UN and only knows their email. When the email changes, they likely won't think to check old emails for WP reset so keeping the email in sync is key. Having a new Location Type, or setting, to specify which email should be used for the CMS account would be good and what I think you are getting at.

from civicrm-wp-profile-sync.

Thanks for the clear explanation of your scenario 👍

I'm on the case with a fix that works on WordPress single site, multisite and multi-domain.

from civicrm-wp-profile-sync.

@rbaugh My issue with the Primary Email being synced to the WordPress User is that it doesn't properly take into account the logic in CiviCRM that is used in Mailings. Someone may wish to change the Email they want to receive Mailings at but not wish to change their WordPress Email. My goal here is to suggest people use a custom Location Type (e.g. "CMS" or "WordPress" or similar) and then sync that Email, leaving the CiviCRM Mailing logic out of the equation.

Didn't catch this reply earlier. I get what you mean by the sync of primary to WP/CMS. Having a flag for CMS like the is primary would be useful. Only one can be used and could be set as desired. If only one email, then is becomes primary and CMS user email.

Appreciate getting a patch in for this. I haven't had a chance to get to debugging and looking into the code.

from civicrm-wp-profile-sync.

@rbaugh I've added a branch with what I think is a fix:

https://github.com/christianwach/civicrm-wp-profile-sync/tree/ufmatch

Can you check if that fixes things for you?

@kcristiano @danaskallman I suspect this thread may be of interest to you both since my fix:

• Updates all UFMatch records for a User regardless of Domain ID
• Syncs the Contact Primary Email to the WordPress User Email even when the Contact being edited does not have a UFMatch record for the current Domain
• Excludes sync of ACF Fields to WordPress User Profiles when the Contact being edited does not have a UFMatch record for the current Domain

Thoughts?

from civicrm-wp-profile-sync.

@christianwach I'll test this out later today.

from civicrm-wp-profile-sync.

@christianwach

I do think we should respect the domain id - why is it updating regardless of the domain id? CiviCRM MultiDomain does not have to be a WP multi-Site. I've even had WP and Drupal sites connected with MultiDomain and that would surely break that configuration,

The second point I also see an issue with depending on the configuration.

I do think we have to respect the domain id with uf_match. This may work for a specific WP Multisite config, but how would it work with WP Multi-Site connected to multiple CiviCRM DB (no multi-Domain)

I am unsure of the 3rd point - can you share an example?

from civicrm-wp-profile-sync.

I do think we should respect the domain id - why is it updating regardless of the domain id?

@kcristiano Here's a scenario:

• WordPress Multisite
• Multiple Domains in CiviCRM
• A CiviCRM admin visits the CiviCRM back-end and edits a Contact's Primary Email
• That Contact is not in the current Domain Group and has no UFMatch record for that Domain ID
• The WordPress User is not found because there is no UFMatch for that Domain
• The Primary Email is changed but the WordPress User Email is not

Now we have data corruption.

If the UFMatch record is retrieved without reference to the Domain (or searches all Domains) then the WordPress User is found and their Email is updated.

from civicrm-wp-profile-sync.

and if we update all domains and that is not the scenario wouldn't we have data corruption?

I think this needs testing before merging/releasing - certainly before releasing. Happy to help I just need to get past a few other issues first

from civicrm-wp-profile-sync.

Having mulled this over all evening, I'm wondering what use there is in multiple entries in the uf_match table (with different Domain IDs) linking the Contact and the User?

If the purpose of the uf_match table is to look up the connection between Contact and User, then the only use case I can see (as you say @kcristiano) is where each entry connects the Contact to a User in a different CMS. I'm currently struggling to think of another purpose.

if we update all domains and that is not the scenario wouldn't we have data corruption?

Hmm, well if I'm reading the code right, it seems that the Multisite Extension itself implements a blanket update of the uf_name value on all UFMatch records when a UFMatch record is updated. Which suggests that this plugin actually only needs to update one and the Multisite Extension will handle updating the rest!

If there is purpose to UFMatch beyond discovering the Contact-to-User link, I can't seem to identify an important one - even though I've scanned every usage of UFMatch records in CiviCRM Core. The only examples of any significance that I can find where the uf_name value is used is in:

• The isDuplicateUser() method which seems to be marked as unused.
• The PCP formRule() method which checks for an existing User with that Email Address.

It seems to me that the first can be ignored... and the logic in the second method could be shifted to querying via the CRM_Utils_System_* classes. I proposed removing the unused language column from the uf_match table a while back... I wonder if it's worth proposing the removal of uf_name too?

from civicrm-wp-profile-sync.

how would it work with WP Multi-Site connected to multiple CiviCRM DB (no multi-Domain)?

Good question. I'll look into this when I get a chance.

from civicrm-wp-profile-sync.

The primary use case is One CiviCRM Multi-Domain connected to multiple single site installs (regardless of the CMS) with different users. That is the exception I am thinking of.

I also don't see much value in uf_name either

from civicrm-wp-profile-sync.

@christianwach @kcristiano
If the uf_name isn't really used anywhere else, then I think maybe this doesn't need to be part of the table since it can be derived anyways. Not sure if the purpose was to limit another lookup, but can be handled if this field wasn't used anymore. This wouldn't be a problem anymore either as we wouldn't need to worry about updating it when the CMS profile email is updated. Not sure if any other extensions are using this field or not.

I had just been using the WPCiviRules token since it was there, but I don't have an issue creating my own token to get the CMS user email to be able to add it to emails.

I do like having another attribute on an email address to signify which email is to be used for the CMS. This way it isn't always contingent on just the primary email although others may not agree with it.

from civicrm-wp-profile-sync.

I do like having another attribute on an email address to signify which email is to be used for the CMS.

@rbaugh To be clear, I'm not proposing an additional attribute (that would require changes in CiviCRM Core) but instead propose to map a Location Type so that the Email with that Location Type is synced to the CMS - the same way this plugin does for the Website Type. People can then use an existing Location Type or create a custom one.

from civicrm-wp-profile-sync.

Not sure if any other extensions are using this field or not.

Yes, this could be a problem - which is why (for the time being) I'm looking at the best way to do this. There'll be an update to the ufmatch branch in due course when I've figured out the interaction with the CiviCRM Multisite Extension.

from civicrm-wp-profile-sync.

The primary use case is one CiviCRM Multi-Domain connected to multiple single site installs (regardless of the CMS) with different users.

@kcristiano Okay that seems a reasonable assumption... I've been checking my Multisite/MultiDomain installs and I can't find an example where there are multiple UFMatch entries. However, that's not to say that there cannot be multiple UFMatch entries - I still think the Multisite use case I laid out above remains problematic, i.e. when the CiviCRM Domains are tied to WordPress Subsites in a Multisite context and a admin edits a Contact in CiviCRM where the Contact doesn't have a UFMatch record for that Domain.

from civicrm-wp-profile-sync.

I've been looking again at this yesterday and today as I was keen to get it in to the next release, but have come across some oddities that may mean postponing it - mainly to do with the way the CiviCRM Multisite Extension handles UFMatch updates.

The commit that I force-pushed to the ufmatch branch should "just work" for installations without the CiviCRM Multisite Extension, but for those with, they will need the updated code in this Merge Request.

@rbaugh & @kcristiano I'd really appreciate it if you could test this in your context(s) before I do the next release 👍

from civicrm-wp-profile-sync.

I've tested this in multisite setting with both PR's. I did not see the email update in multisite when a user had 2 uf_match entries domain 1 and domain 21 (only a user on domain 21) when doing any of the following:

• Merging on domain 1 with user record on left side of merge screen (trashed, but moved all related data, moved primary email on uf_match entry as primary (overwrite)
• Saving primary email on civi contact on domain 1
• Saving primary email on civi contact on domain 23

No email update on domain 21 in this case.

However, contact_id, does successfully update as I've observed previously a uf_match mismatch if a user record is merged on a domain other than the one it is a user on. That's critical for obvious reasons if the linkage is severed, e.g. civicrm groups sync, etc will fail to work.

However, that's not to say that there cannot be multiple UFMatch entries - I still think the Multisite use case I laid out above remains problematic, i.e. when the CiviCRM Domains are tied to WordPress Subsites in a Multisite context and a admin edits a Contact in CiviCRM where the Contact doesn't have a UFMatch record for that Domain.

This relates to the "Merging a uf_match record on a domain ID the user is not a user on" issue that I've never opened up :/ (usually the parent domain ID) does not update the contact_id uf_match entry for the domain the user exists on. I observe that this is NOT the case with 2 PR's. This appears to resolve that.

Before merge

After merge

from civicrm-wp-profile-sync.

I did not see the email update in multisite when a user had 2 uf_match entries domain 1 and domain 21 (only a user on domain 21) when doing any of the following:

@andyburnsco Hi Andy, thanks for testing, much appreciated. Before I dive in with further testing myself based on your results, can I check with you that your "Sync CMS Email" setting is set to "No" on all the Domains you're working with?

I discovered recently that this plugin doesn't properly override this in WordPress Multisite/Multi-Domain installations and that the Profile Sync settings need to be saved on each Sub-site because CiviCRM settings are specific to each Domain. FWIW it's hard to see how this plugin can automate this because switch_to_blog() does not switch the CiviCRM Domain ID - it has to be done for each Domain/Sub-site - and so looping through Sub-sites programatically won't work.

One other point to mention is that I haven't tested any of this with Contact merging but have put this on my to-do list.

from civicrm-wp-profile-sync.

For the 2 domains tested, in UI it showed "No". I do see it is not consistent throughout all of the domains.

Though upon re-saving those 2 domains, I see in the db it changed from

to

I've now set an override in civicrm.settings.php to achieve consistency:
$civicrm_setting['CiviCRM Preferences']['syncCMSEmail'] = '0';

I tried inspecting wp_options for both domains to see if the profile sync settings was already saved but I'm not seeing an any entries. I know I've saved at network level: https://example.org/wp-admin/network/settings.php?page=cwps_network and likely domain 1 but not others.

Unfortunately, I can't do more testing till the 13th, I'm about to head out for vacation right now and not bringing the PC :) Can resume then if this is still ongoing at that time.

from civicrm-wp-profile-sync.

Thanks @andyburnsco that's very useful. When the setting was updated, did the UFMatch values change as expected?

I tried inspecting wp_options for both domains to see if the profile sync settings was already saved but I'm not seeing an any entries.

When network-activated, you'll find the Profile Sync settings under the cwps_settings meta key in wp_sitemeta. These propagate to each Sub-site Profile Sync Settings Page, but (as I said above) don't automatically update CiviCRM's settings unless saved on each Sub-site. I'll try and address this alongside these UFMatch issues.

I've now set an override in civicrm.settings.php

Ah, now this is a good idea 👍

Have a great holiday.

from civicrm-wp-profile-sync.

@christianwach I was able to test the UFMatch branch and it looks like it is working as expected for me. I was able to switch the primary email used as well as making updates to the email itself and the changes were updated for the WP profile and the UFMatch to match the CiviCRM changes.

When I updated the email from the WP side, the CiviCRM record is updated but the UFMatch is not carrying across.

So CiviCRM changes are showing correctly for UFMatch, but WP changes are not.

from civicrm-wp-profile-sync.

I had to do a release yesterday but didn't include this as I think it still needs a lot more testing.

from civicrm-wp-profile-sync.

When I updated the email from the WP side, the CiviCRM record is updated but the UFMatch is not carrying across.

@rbaugh I've added some commits to master that should update UFMatch records when sync happen in that direction.

from civicrm-wp-profile-sync.

@christianwach, Thanks for continuing the work on this. I will check this out and see how things work.

from civicrm-wp-profile-sync.

@christianwach It seems I didn't report back on this over the past year. But this has been working and updating as expected.

from civicrm-wp-profile-sync.