dev/core#2486 Add v4 Mailing api (+ MailingJob)

[eileenmcnaughton]

Overview

dev/core#2486 Add v4 Mailing api (+ MailingJob)

Before

No v4 api for Mailing

After

Tada

Technical Details

@colemanw @seamuslee001 @totten this is a starting point but there are some bizarre things regarding the Mailing create function

1. there is some permissions stuff in the v3 api layer - we could probably just move this to the BAO
2. weird voodoo number 1 _evil_bao_validator_ - this is where either flexmailer or the legacy method validate
3. weird voodoo number 2 - if you just want to create the entity & not have it start firing stuff out you have to pass in 'skip_evil_bao_auto_schedule' and _skip_evil_bao_auto_recipients_ ..... - UPDATE agreed setPopulateReceipients - default to FALSE

I'm not quite sure how best to deal with 2 & 3. Note we did talk about moving the legacy Mailing stuff to an extension - which probably would have made this easier at least WRT 2

Comments

https://lab.civicrm.org/dev/core/-/issues/2486

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

[monishdeb]

@eileenmcnaughton there is a related test failure:

api\v4\Entity\ConformanceTest::testConformance with data set "MailingJob" ('MailingJob')
API_Exception: At least one Mailing is required in test

/home/jenkins/bknix-dfl/build/core-22624-62lv4/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php:155
/home/jenkins/bknix-dfl/build/core-22624-62lv4/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php:99
/home/jenkins/bknix-dfl/build/core-22624-62lv4/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php:59
/home/jenkins/bknix-dfl/build/core-22624-62lv4/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Entity/ConformanceTest.php:298
/home/jenkins/bknix-dfl/build/core-22624-62lv4/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Entity/ConformanceTest.php:168
/home/jenkins/bknix-dfl/extern/phpunit8/phpunit8.phar:671

[eileenmcnaughton]

thanks @monishdeb - could be worse :-)

[totten]

Should we make MailingJob readonly?

[eileenmcnaughton]

done

[colemanw]

@eileenmcnaughton the @readonly annotation isn't right. Instead you have to use the ReadOnlyEntity trait.

[colemanw]

@eileenmcnaughton ^

[eileenmcnaughton]

@colemanw thanks - fixed

[eileenmcnaughton]

@colemanw I'm not sure why MailingJob doesn't create the 'Mailing' it requires in testConformance.

Note the only reason we need MailingJob is that once we want to start to get other details - eg. bounces we need to join through MailingJob to MailingEventQueue to MailingEventBounce

[colemanw]

@eileenmcnaughton I fixed the conformance test which triggered a new test run. But the previous run showed other problems: https://test.civicrm.org/job/CiviCRM-Core-PR/47154/#showFailuresLink

[colemanw]

@eileenmcnaughton looks like just one test error left...

[totten]

@colemanw @eileenmcnaughton - The failures in ValidatorTest are complaining because the signature changed (DAO => array).

It should be straight-forward to update the test:

diff --git a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php
index 35541e5e67..642d4527b2 100644
--- a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php
+++ b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php
@@ -86,9 +86,7 @@ class ValidatorTest extends \CiviUnitTestCase {
    * @dataProvider getExamples
    */
   public function testExamples($mailingData, $expectedErrors): void {
-    $mailing = new \CRM_Mailing_DAO_Mailing();
-    $mailing->copyValues($mailingData);
-    $actualErrors = Validator::createAndRun($mailing);
+    $actualErrors = Validator::createAndRun($mailingData);
     $this->assertEquals(
       array_keys($actualErrors),
       array_keys($expectedErrors)

Grepped universe for createAndRun -- I don't see anything outside of civicrm-core that uses it.

[eileenmcnaughton]

Thanks @totten @colemanw

@colemanw ideally we would expose bounces etc in search kit next.