CRM-20847 Support custom api with composite primary keys

[eileenmcnaughton]

This adds a couple of checks in the api to not add the id field and adds unit tests on a custom api (albeit somewhat hacked into a single file). This is useful because in the past the hook_civicrm_EntityTypes became mandatory and broke custom api using basic_get, so valuable testing added.

I also confirmed it's possible to define an fk join on a field other than id & added a test. It could have been declared in the DAO but I used a spec function to define mailing_identifier joining to civicrm_mailing.hash per code below

/**
 * Metadata for MailingProviderData.get API
 *
 * @param array $params
 *
 * @throws API_Exception
 */
function _civicrm_api3_mailing_provider_data_get_spec(&$params) {
  $params['mailing_identifier']['FKClassName'] = 'CRM_Mailing_BAO_Mailing';
  $params['mailing_identifier']['FKApiName'] = 'Mailing';
  $params['mailing_identifier']['FKKeyColumn'] = 'hash';
}

Note that Grant is an example of an api which uses a uniqueName on
id & I checked manually that is still returned. SyntaxConformance
tests cover this a lot, but I still also added one check to the grant test.

---

• CRM-20847: Support custom api with composite primary keys

[mickadoo]

should this comment be updated if it doesn't always select the ID?

[eileenmcnaughton]

yes!

[eileenmcnaughton]

fixed now

[monishdeb]

👍 for this improvement. Tested, works fine

[eileenmcnaughton]

Hmm - you should probably have merged this - now I'm going to have to merge this based on your review - which is arguably a less good process

[eileenmcnaughton]

(thanks for the review though - note that I did discuss this with @totten on MM & he was pretty agnostic about it)

[jackrabbithanna]

Oh yeah Eileen the "merge police" are watching :) I'm just messing with you. Maybe its just a matter of setting some new habits...CT thumbs up on MM is "good enough" maybe, but it doesn't give anyone else the chance to see documentation of it, or at least its hard, especially as the time goes from merge to somebody looking back at the PR. If everyone sees a Core Team thumbs up on these PRs or on Jira, it definitely takes away the ambiguity. There's got to be a balance between efficiency and bureaucracy that we can strike. Nice PR here! if you ever need a "merger" let me know!

[eileenmcnaughton]

Yep - well I think like a lot of people I'll be just taking a back seat for a while since the big blow up was about a change that had been OK'd on JIRA a few months ago by Coleman. I think if anyone doesn't feel very nervous about doing anything right now they are braver than I!

[jackrabbithanna]

I"m sorry you see it as a blowup...I think you can tell that there is a lot of support for this in the community.

[jackrabbithanna]

And maybe a little nervousness is good, shouldn't it lead to higher quality code, more review, and less regressions?

[eileenmcnaughton]

It's possible that's how it will play out - I can also see some quite different scenarios - but I don't wish to discuss any further at this time

[jackrabbithanna]

@eileenmcnaughton when you feel like, please do share your concerns and the difference scenarios you foresee, in the partners channel sometime. Your knowledge is deep, and your input both necessary and important.