-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
regression: Do not throw an error on custom fields validation when migrating visitors to contacts #34030
regression: Do not throw an error on custom fields validation when migrating visitors to contacts #34030
Conversation
…r to contact migration
Looks like this PR is ready to merge! 🎉 |
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release-7.1.0 #34030 +/- ##
==============================================
Coverage 75.79% 75.79%
==============================================
Files 510 510
Lines 22063 22063
Branches 5385 5385
==============================================
Hits 16723 16723
Misses 4694 4694
Partials 646 646
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls fix tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If an invalid value is already saved on the visitor, we shouldn't just discard it.
Let's register that value as a conflict for the custom field instead.
Also: we probably need to do the same thing on the merge process.
I brought this idea to our product lead and we decided to not create any custom field conflict as of now. Let's hold all of them for later so that they can be implemented and handled consistently
For the same reason I stated above, we decided to not create any custom field conflict on contact merge. Only custom fields that are "missing" in the base contact will be added on merge -- all other ones will be discarded. |
This PR currently has a merge conflict. Please resolve this and then re-add the |
…grating visitors to contacts (#34030)
Proposed changes (including videos or screenshots)
Introduced here: #32727
Issue(s)
Steps to test or reproduce
Further comments
SCI-179