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

Afform - Update correct existing email,phone,address & prevent deletion of others #24172

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Aug 7, 2022

Overview

This fixes a bug introduced in #21254 which caused existing contacts' email, address, phone, im to be updated incorrectly.

(The bug applies to Phone / Email / Address blocks with a singular location-type. Multi-address blocks are not affected.)

Before

To reproduce:

  1. Log in and update your own contact record with > 1 email address
  2. Set up an afform which pre-populates the current user
  3. Add an email to the form. Set it to update "Primary" instead of "Multiple"
    image
  4. Use the form - your primary email will be updated but other emails will be deleted!

After

Fixed.

…ate,

and prevent deletion of other records when updating a single one
(primary or by location type).
@civibot
Copy link

civibot bot commented Aug 7, 2022

(Standard links)

@totten
Copy link
Member

totten commented Aug 8, 2022

Did some r-run with a few different styles of "Email" and "Address" widgets:

  • The "Multiple" style worked before and after patch.
  • The "Primary" and "Billing" style were both problematic (deleting records from other locations) before the patch. After the patch, updates+deletes were properly focused on the specific location-type.
    • FWIW, if you have multiple records with the same location-type (eg two "Work" emails), then the outcome is somewhat arbitrary - eg for loading, the widget will show one; for saving, it will save the visible one and delete the other. It's kinda grey/debatable - it differs from the backend UI, but allowing only have one address per location-type is an easier mental-model... Regardless, the patch's behavior is clearly better than the prior behavior.

Tested on both 5.52-stable and 5.53-master.

Merging!

@totten totten merged commit d5b2b3f into civicrm:master Aug 8, 2022
@totten totten deleted the afformLocationFix branch August 8, 2022 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants