[REF] Code cleanup on location entities for the Contact Summary screen

[eileenmcnaughton]

Overview

Builds on #22966 & cleans up location fetching with a view to reducing complexity & notices

Before

2 problems
1) the code is really confusing - handling the loading in 2 places
2) not all keys are loaded - resulting in enotices at the tpl layer.

After

most - not yet address - location entities loaded in one function with mapping done in the same function, all fields loaded so less notices

Technical Details

Comments

[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]

@demeritcowboy Can I get you to check this one (I just rebased to see if it passes first) - it prevents a really annoying set of notices in smarty secure

[eileenmcnaughton]

@demeritcowboy Can I get you to check this one (I just rebased to see if it passes first) - it prevents a really annoying set of notices in smarty secure

[eileenmcnaughton]

ug - it failed - on your new test I think - yay

testContactSummary

[demeritcowboy]

There is a lot going on in here. This might take a while.

[eileenmcnaughton]

@demeritcowboy yeah - hopefully it's not as crazy as it looks - but I did take a while to figure out what the original code was doing (there is a bunch more of the original stuff that should be removable after this with a bit more work)

[demeritcowboy]

jenkins retest this please

[demeritcowboy]

Even though it's not a big PR in terms of lines of code, there was a lot to look at here and I had to do it in pieces over a couple days. I'm not sure if there's a way to make that easier for similar PRs, but anyway see comments below.

• General standards
  • [r-explain] PASS
  • [r-user] PASS
  • [r-doc] PASS
  • [r-run] PASS
    • Ok.
    • The custom fields also still seem to work ok.
• Developer standards
  • [r-tech] PASS with comments
    • It switches to api4 which changes the permissions model to check_permission=true, but I can't think how it matters here.
    • It bypasses the DAO level string "null" handling, but other than it displaying "null" I can't see how it matters for these fields since I'm not sure how they would be "null". And if something is setting them to string null then should probably address that.
    • It changes the ordering and the indexing of the arrays, but I can't see where it matters.
    • It no longer runs the "lookup" loop for some of the types but I can't find any differences.
  • [r-maint] PASS
  • [r-code] PASS
  • [r-test] PASS

[eileenmcnaughton]

Thanks @demeritcowboy - I appreciate you following me into this rather nasty code!