APIv4 - Add activity contacts to APIv4 field spec

[colemanw]

Overview

The BAO supports these pseudo-fields during save operations, this exposes them to the api explorer.

Before

Activity-contact params not discoverable.

After

Activity-contact params discoverable.

Comments

This was based on discussion in https://chat.civicrm.org/civicrm/pl/wnuxuef3ppgt8q5jtdsa5kjsqh

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

[aydun]

I've tested creating activities specifying targets and assignees and all works as expected.

Although targets and assignees are specified as arrays, both also work with integer values when adding a single target or assignee which is handy in some situations. So addValue('target_contact_id', [ 3 ]) and addValue('target_contact_id', 3) both work. Can we get that documented as supported behaviour? Or if we don't want to support it, enforce from the beginning that it must be an array.

[colemanw]

I'm thinking about the relative merits of enforcing data type on input fields in general.
For some data types like numbers, it makes sense for the api to accept 123, but I would argue strings like "123" are also acceptable.
For boolean fields, I think there's too much historical acceptance (and encouragement in basically all the APIv3 docs) of strings "1" and "0" as interchangeable for true and false so I'd be very reluctant to enforce data types there.
And for arrays that could also take a single value... dunno. We should probably add a unit test though.

[aydun]

I prefer the liberal approach so accepting a single value in place of an array is fine for me. I had a look at the existing unit tests and tried changing api_v3_ActivityTest::testActivityReturnTargetAssignee() to use @dataProvider versionThreeAndFour. The creation works but the test fails since target and assignee are not returned by Activity.get in Api4. Can we add those?

[eileenmcnaughton]

@colemanw I don't have any concerns here- please merge once you and @aydun are in agreement...

[colemanw]

I like the symmetry of having those pseudofields available for get but don't like much else about it. The api3 implementation is problematic and in general I'm trying to avoid klunky stuff in apiv4. Working on a different approach that will be more search-builder friendly.

[eileenmcnaughton]

@colemanw is there test cover? (I'm going through tagging ones with tests & ok without tests - not sure where this fits)

[colemanw]

This passes half the api3 test. I guess we could just include the part that works... I'll add that.

[colemanw]

Ok I've added test coverage for what we've got. This should be mergable now.

[colemanw]

Retest this please

[colemanw]

Unrelated test fail