[REF+test] clean up code for getting labels for merge screen, stdise #14260
Conversation
|
(Standard links)
|
eileenmcnaughton
changed the title
|
I will try to QA this, but it looks good from the screenshots. The 'addressee ID' thing is a bit weird; but 'better' than just displaying a meaningless ID I suppose. |
|
Yes, the changes look good to me. I tested the display, and the merging, and it worked as expected. However - I did come across a new bug while testing this: But it is not a regression, so I recommend merging this PR providing the tests pass (I did not test the tests). |
CRM/Dedupe/Merger.php
Outdated
| @@ -1092,14 +1090,22 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio | |||
| // CRM-15681 don't display sub-types in UI | |||
| continue; | |||
| } | |||
| foreach (['main', 'other'] as $moniker) { | |||
| $contact = &$$moniker; | |||
|
@eileenmcnaughton this needs rebasing. |
…data This is preliminary to a code cleanup. m
…splay This code was really confusing - all that 'special values' stuff. Turned out it didn't have to be....
|
@colemanw done |
|
test this please |
|
I'm going to merge based on @JKingsnorth 's review but @eileenmcnaughton i assume you will want to follow up on that other issue @JKingsnorth has reported |
|
@seamuslee001 dying to :-) But yeah it's on my radar |
Overview
Primarily a code clean up & test add this has the side effect of rendering labels for preferred language & the greeting IDs - which seems to me like a positive side effect
Before
After
Technical Details
The whole getSpecialValues function was a convoluted way of
It was deeply confusing & hence had to be cleaned up
Comments
@JKingsnorth