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.

Sign up for GitHub

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

Jump to bottom

Fix overwrite custom field value with default value during redirection #7595

Merged
merged 5 commits into from Mar 18, 2020
Merged

Fix overwrite custom field value with default value during redirection #7595

merged 5 commits into from Mar 18, 2020

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented Jun 11, 2019

Please be sure you are submitting this against the staging branch.

Q A
Bug fix?
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

Noticed issue with contact field default value.
If Mautic create contact and already have contact in url (ct), Mautic create new anonymouse contact first and then merge it with founded contact.
This make mess, because If our founded contact already exist and we modified custom field, then during the merge method, these two contact are merged and value is overwritten to default value.

Steps to reproduce the bug:

  1. Create custom field with default value
  2. Create contact (default values is filled)
  3. Change default value
  4. Create email with anchor link to http://mautic.org
  5. Send email to contact
  6. Copy redirect link from email
  7. Open this link in incognito window
  8. See value of custom field was set to default value (what we don't expect)

Steps to test this PR:

  1. Load up this PR
  2. Repeat all steps
  3. See If value of custom field is not overwrite to default value

@kuzmany kuzmany added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels Jun 11, 2019
@kuzmany kuzmany added this to the 2.16.0 milestone Jun 11, 2019
@kuzmany kuzmany changed the title Fix getContactFromQuery If contact exist in query Fix getContactFromQuery If contact exists in query Jun 11, 2019
@kuzmany kuzmany changed the title Fix getContactFromQuery If contact exists in query Fix overwrite custom field value with default value during redirection Jun 11, 2019
@florentpetitjean
Copy link

tested on Mautix box and it works (the value does not reset to default)

@kuzmany
Copy link
Member Author

kuzmany commented Jun 14, 2019

@florentpetitjean Can you retest with last changes?
I hope new version should be much better to figure out this issue

@florentpetitjean
Copy link

Works fine with new patch

@npracht npracht added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels Jun 25, 2019
@npracht npracht added this to Ready to Test (confirmation) in Mautic 2 Aug 15, 2019
@RCheesley RCheesley mentioned this pull request Dec 9, 2019
Closed
@npracht npracht modified the milestone: 2.16.0 Jan 23, 2020
@RCheesley RCheesley added this to the 2.16.1 milestone Mar 9, 2020
@npracht npracht added this to Needs a second test/review in Mautic 2 Mar 10, 2020
@RCheesley
Copy link
Sponsor Member

Ooh, nasty bug! Tested this on Mautibox with a PR that was only unit tests, so close as possible to vanilla 2.16 and could reproduce the bug.

Tested the PR on Mautibox and the field is not overwritten with the default content.

@RCheesley RCheesley added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Mar 17, 2020
@RCheesley RCheesley moved this from Needs a second test/review to Ready to Commit (passed testing) in Mautic 2 Mar 17, 2020
dennisameling
dennisameling approved these changes Mar 18, 2020
View reviewed changes
Copy link
Member

@dennisameling dennisameling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuzmany You're a hero! Was actually experiencing this myself on my Mautic instance (some contacts would mysteriously have some custom field values set back to their defaults sometimes), haven't tested it yet but based on the comments above I'm merging this PR 🎉

@dennisameling dennisameling merged commit db61301 into mautic:staging Mar 18, 2020
Mautic 2 automation moved this from Ready to Commit (passed testing) to Merged Mar 18, 2020
@dennisameling dennisameling mentioned this pull request Mar 18, 2020
Merged
@mautibot
Copy link

This pull request has been mentioned on Mautic Community Forums. There might be relevant details there:

https://forum.mautic.org/t/announcing-mautic-2-16-1-beta/13438/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dennisameling dennisameling dennisameling approved these changes

Assignees
No one assigned
Labels
bug Issues or PR's relating to bugs ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
No open projects
Mautic 2
  
Merged
Milestone
2.16.1
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@kuzmany @florentpetitjean @RCheesley @mautibot @dennisameling @npracht