-
-
Notifications
You must be signed in to change notification settings - Fork 824
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
CRM-21773 Fix merging multi-value custom fields #11691
Conversation
The test should FAIL - proving the issue. |
It failed, good: not ok 988 - Error: CRM_Dedupe_MergerTest::testMigrationOfSomeCustomDataOnEmptyCustomRecord |
Now, with the fix, it should pass. |
@eileenmcnaughton I think this is failing because \CRM_Dedupe_Merger::relTables is being cached in a static variable (https://github.com/JKingsnorth/civicrm-core/blob/06f4969c149039c74940604b75d5f44e44de7bfc/CRM/Dedupe/Merger.php#L42), so it's not loading up the new group I'm creating during the test. Can you advise me on how to clear that static variable cache? I'm not sure it's even possible since the scope of the static variable is limited to the function itself? How do other unit tests get around these issues? |
@JKingsnorth search for Civi::statics - that is a static var that is cleared between test runs & can be cleared from outside the function |
Thanks @eileenmcnaughton - I'll try that out. |
d68970f
to
335cead
Compare
335cead
to
641a8cb
Compare
@eileenmcnaughton @systopia This is ready for review now! I had to refactor the static variable caching to get the tests to work. Thanks for the tip Eileen. I confirm that I have reviewed @systopia 's patch, and it resolves the issue. If someone could check my unit test then this should be good to merge. |
thanks @JKingsnorth & @systopia |
Thanks all. |
Thanks @eileenmcnaughton and @JKingsnorth ! |
Overview
Multi-value custom fields are ignored during the merge process, when they should be merged provided we tick the box to move them during the process.
Before
Multi-value fields not migrated.
After
Multi-value fields are migrated if required.