[REF] Further cleanup on address handling in merge code.

[eileenmcnaughton]

Overview

This is a follow on from #15949

This doesn't make any change - it just improves legibility

Before

Code less readable

After

Code more readable

Technical Details

The biggest change here is the extraction of getContactPortionOfGreeting and a comment block with some explanatory comments

Comments

This has good test cover - notably in testMergeSameAddressGreetingOption

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@monishdeb this is towards making the code legible enough to make the new change but when I do this I see the following discussion point (which is non-blocking on this PR)

1. as you indicated shared-address-merging isn't currently working but
2. in fixing it we need to check the logic makes sense - the current logic is that if you have the same address as me the addressee will be 'Dear Eileen & Monish' - but from my read of the code (which is not changed by this cleanup but is clear-ish after this is merged) if my address is your master_id address then it will say 'Dear Eileen' - that seems odd but perhaps when it was written only orgs could share their addresses & it certainly doesn't make sense to say

Dear CiviCRM & Josh & Coleman & Tim. &......

[monishdeb]

I agree to disagree with the current code how it munging the addressees which make no sense when it involves multiple contacts. But maybe it was implemented in this order to indicate which are the contacts which are merged into one record. Should we bring this discussion in dev channel? For now this PR looks good and doesn't break the existing workflow.

[eileenmcnaughton]

@monishdeb - does that mean this is mergeable?

Regarding the current code - I pinged you on dev channel but I guess a gitlab should be created for visibility

[monishdeb]

Yes, it is. Did a quick test, and looks ok to me.

[eileenmcnaughton]

@monishdeb I think my proposal on share-same-address is that we make it append other addresses unless an individual is sharing with an Org. We actually have to resolve the 'correct' behaviour before we fix the bug because it will feel like a regression to some otherwise I think. Will log in gitlab