dev/core#5308 : APIv4 - Add Order.validate

[monishdeb]

Overview

Add (skeletal) Order.validate APIv4 action. This adds the generic validate action that can be inherited by all entities for validating the parameters of an entity before doing CRUD operation.

Ref - https://lab.civicrm.org/dev/core/-/issues/5308

Comments

ping @JoeMurray @eileenmcnaughton @colemanw @jensschuppe @seamuslee001

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers...

• ➡️ PR Demo site (can be rebuilt with the command test this please).
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[civibot]

No issue was found matching the number given in the pull request title. Please check the issue number.

[eileenmcnaughton]

@monishdeb I like the look of this - I think we want to add a unit test to get it merged - perhaps the easiest thing to cover in a unit test would be ensuring lifetime cannot have an end_date -eg

(based on testCreateOrderForMembership)

price_field_value_id' => $this->ids['PriceFieldValue']['membershprice_field_value_id' => $this->ids['PriceFieldValue']['membership_first'],ip_first'],

    $contribution = Order::validate()
      ->setContributionValues([
        'contact_id' => $this->individualCreate(),
        'receive_date' => '2010-01-20',
        'financial_type_id:name' => 'Member Dues',
      ])
      ->addLineItem([
        'entity_id.membership_type_id:name' => 'Lifetime',
        'entity_id.join_date' => '2006-01-21',
        'entity_id.start_date' => '2006-01-21',
        'entity_id.end_date' => '2006-12-21',
        'entity_id.source' => 'Payment',
      ])
      ->execute()->first();

[eileenmcnaughton]

@monishdeb I took another look at this & I think there is a lot to like here :-) I feel like in terms of the order of how we procede through this project getting this merged, with some tests, will help us structure the rest of the work

I want to kick the tires a bit on how it works in practice. Some thoughts

• this assumes multiple records - the order api takes just one order at a time
• the order API has 2 types or records - contribution & line items. They are not mapped to 'values' like other other API do but are top level properties
• when validating events there will be a need for the function to validate ALL the line items in one place - because perhaps there are 2 spaces left on the event & the order has 2 line items taking 2 spaces each

Given that I wonder

1. can order share a generic validate with other entities - or should it have it's own

2. I assume the validate listener will need both line items and the contribution

3. what does the result look like - it seems it will throw an exception at the moment - I wonder if it is more appropriate to return an array of values - but if we do how does that look? Some examples

4. Membership back office form - submits lifetime membership with end_date. Quick form wants to know the end_date field needs to be highlighted in red as having failed validation

5. Front end event form goes over the places limit - in this case the message is at the top level - ie 'the line items in combination are wrong'

How do we accomodate both of these in the way we return the results?

Some other thoughts

• we should probably also 'interpret' the line items ie if only price_field_value is set then populate the rest of the line item before passing out, as we do with the Order.create)
• tests
• assuming order api 'goes it's own way' - do we want to add the generic validate now or let it perculate for a bit

[monishdeb]

@eileenmcnaughton I forgot to mention the other PR which is on implementing Membership.validate() APIv4 #30710. To answer each question:

1. can order share a generic validate with other entities - or should it have it's own
   You are right that Order.validate should be have its own signature to gather contribution and line-item parameters unlike other entity's validate action , say for example membership.validate here. Also because Order entity is a pseudo-entity unlike other entities so we have to manually design its pieces.

2. I assume the validate listener will need both line items and the contribution

Extending pseudocode in your previous comment there should be separate setter functions for setting contribution values and line-item values. And the line-item values should be pass through a formatter function before getting validated by respective entity.validate. To summarize :

namespace \Civi\Api4\Order;

class validate {
public function _run(\Civi\Api4\Generic\Result $result) {
 // extract and format the contribution parameters before validating the parameters
 $contributionValues = $this->format($this->getContributionValues(...), 'Contribution');
 $errors['contributions'] = Contribution.validate($contributionValues);
 foreach ($this->getLineItems() as $key => $lineItem) {
     $entity = getEntityFromEntityTable($lineItem['entity_table']);
     // $entity could be in Contribution, Event or Membership
     $errors['lineitems'][$key] = $entity.validate($this->format($lineItem, $entity));
   }
   $result[] = $errors;
 }
 
}

3. what does the result look like - it seems it will throw an exception at the moment - I wonder if it is more appropriate to return an array of values - but if we do how does that look? Some examples
   This would be something similar like Membership.validate shown here :

So Order.validate result in case of errors could something like:

return [
  [
    [
      'record' => 'contribution values',
      'fields' => [
        'contact_id',
        'receive_date',
      ],
      'name' => 'mandatory_missing',
      'message' => 'Mandatory values missing from Api4 Contribution::validate: contact_id, receive_date',
    ],
    [
      'lineitems' => [
         [
            'record' => $lineItemKey,
             'fields' => [
              'status_id',
         ],
         'name' => 'empty_status_id',
         'message' => 'There is no valid Membership Status available for selected membership dates.',
       ],
       ...
    ]
];

4. Membership back office form - submits lifetime membership with end_date. Quick form wants to know the end_date field needs to be highlighted in red as having failed validation

Isn't it apt to write the unit tests in this PR #30710 ? To use Membership.validate as in:

After #30710 got merged we can work on the Order.validate signature and add a unit tests to validate Order's (contribution) parameters and it's entity's parameter (say Membership using Membership.validate as per the pseudocode mentioned above)

[mattwire]

How about making this into a subscriber to civi.api4.validate rather than calling directly? If it has a service name and uses IsActiveTrait it could even be disabled by custom implementations of civi.api4.validate and would serve as a good in-core example of how to implement your own.