Refactor CRM_Contact_BAO_Relationship::add to autoload missing params from existing record

[mattwire]

Overview

This is a pre-requisite to #15030. The BAO add method needed updating so it loads missing parameters directly using standard methods rather than requiring that they are all passed in to the function.

Before

Have to pass a lot of parameters to the CRM_Relationship_BAO_Relationship::add() function. Existing parameters not found by the function.

After

Per other standard functions, add() uses standard methods to find an existing record and use the params directly. This only happens if an ID is passed into the function.

Technical Details

Use standard DAO find() method.

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).

[demeritcowboy]

Thanks @mattwire - have taken a quick look and looks good so far. Give me a bit and I'll do a bit more.

[eileenmcnaughton]

@mattwire I'd rather only do the load if we are missing specific params we need for the next processing steps - this could be part of a big batch update.

[mattwire]

@mattwire I'd rather only do the load if we are missing specific params we need for the next processing steps - this could be part of a big batch update.

@eileenmcnaughton Ok I've done that

[eileenmcnaughton]

test this please

[eileenmcnaughton]

This seems ok to me - will see if @demeritcowboy has further comments

[demeritcowboy]

• General standards
  • [r-explain] PASS
  • [r-user] PASS
  • [r-doc] PASS
  • [r-run] PASS
    • Looks good.
    • It's pre-existing but the head of household thing doesn't work because 6 is the wrong id, at least on the test site here. I'm not sure that feature would work properly anyway as written in all cases.
    • I don't currently see a way to test in the UI the situation where you haven't passed in enough params, which is partly what this was about. See also r-maint below.
• Developer standards
  • [r-tech] PASS
  • [r-code] PASS
  • [r-maint] PASS
    • I notice there are no existing tests on the add() function directly. The api relationship-create is well-tested, so indirectly add() is tested, just as we know the api does more manipulation after calling add(), so some of those tests might be really testing that later code not add(). But it seems tested enough that it shouldn't hold this up. I don't mind writing a test about the lack of params to demonstrate the before and after, but you don't have to wait for me.
  • [r-test] PASS

[demeritcowboy]

See test at #15121

[mattwire]

It's pre-existing but the head of household thing doesn't work because 6 is the wrong id, at least on the test site here. I'm not sure that feature would work properly anyway as written in all cases.

I didn't like that there was a hardcoded ID there - but resisted the urge to fix because I couldn't find a an existing example and.. more PRs.. but as it's actually broken on your site here's a PR: #15123

[demeritcowboy]

Thanks @mattwire. While I've always been a huge fan of the number 6 it's time for it to go. It's the wrong id on dmaster too and any stock new install.