From aa0d6576ded74abcd35577e16fd8a35df8129c25 Mon Sep 17 00:00:00 2001 From: "Matthew Wire (MJW)" Date: Tue, 28 Jul 2026 17:30:17 +0100 Subject: [PATCH] Use Order API to create recur directly for Formbuilder payments Formbuilder's Afform contribution submission (CreateContribution::saveNewContribution) used to create the Contribution via Order API and then create the ContributionRecur separately afterwards, manually copying contact_id/amount/currency/is_test across from the just-created Contribution. Order API now supports recurring contributions directly, so pass the recur values straight into Order::create() and let it create both records. Order::calculateSharedValues() reconciles currency/is_test between the Contribution and ContributionRecur when only one side provides a value, so callers don't have to specify both. It runs before the ContributionRecur is created (calculateContributionRecurValues() creates it immediately when there isn't an existing one), otherwise a value provided only on the Contribution side would be too late to reach it. --- CRM/Financial/BAO/Order.php | 33 ++++++- .../Contribute/Service/CreateContribution.php | 47 +++------- tests/phpunit/CRM/Financial/BAO/OrderTest.php | 90 +++++++++++++++++++ 3 files changed, 132 insertions(+), 38 deletions(-) diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php index 2c632e9eebb0..a1fd045a394b 100644 --- a/CRM/Financial/BAO/Order.php +++ b/CRM/Financial/BAO/Order.php @@ -1635,6 +1635,30 @@ private function calculateContributionValues() { } } + /** + * Reconcile the params shared between Contribution & ContributionRecur + * ('currency', 'is_test') so both end up with the same value: if only one + * side provides a value, copy it to the other. + * + * Note: this only reconciles values passed in for this create; it does not + * look up values from an existing Contribution/ContributionRecur, since + * nothing currently calls validate() with an existing id set (see the + * "existing contributions" TODO on CreateContribution::getSubscribedEvents). + */ + private function calculateSharedValues(): void { + foreach (['currency', 'is_test'] as $field) { + $recurValue = $this->contributionRecurValues[$field] ?? NULL; + $contributionValue = $this->contributionValues[$field] ?? NULL; + + if ($recurValue !== NULL && $contributionValue === NULL) { + $this->contributionValues[$field] = $recurValue; + } + elseif ($contributionValue !== NULL && $recurValue === NULL) { + $this->contributionRecurValues[$field] = $contributionValue; + } + } + } + /** * @return $this * @@ -1644,7 +1668,14 @@ private function calculateContributionValues() { * @throws \Civi\API\Exception\UnauthorizedException */ public function validate(): CRM_Financial_BAO_Order { - // First we calculate remaining parameters for Contribution/ContributionRecur + // Reconcile currency/is_test between Contribution & ContributionRecur before + // calculateContributionRecurValues() below creates & saves the ContributionRecur + // (when there's no existing one) - otherwise it's too late to share a value + // provided only on the Contribution side. + if (!empty($this->contributionRecurValues)) { + $this->calculateSharedValues(); + } + // Now we calculate remaining parameters for Contribution/ContributionRecur $this->calculateContributionRecurValues(); $this->calculateContributionValues(); // Then we get/calculate the lineitems - they won't have related entity IDs Membership/Participant etc. for new records. diff --git a/ext/civi_contribute/Civi/Contribute/Service/CreateContribution.php b/ext/civi_contribute/Civi/Contribute/Service/CreateContribution.php index 826e15b3a392..576d7e453b6d 100644 --- a/ext/civi_contribute/Civi/Contribute/Service/CreateContribution.php +++ b/ext/civi_contribute/Civi/Contribute/Service/CreateContribution.php @@ -183,18 +183,16 @@ public function saveNewContribution(AfformSubmitEvent $event) { } // use order to create the contribution record - $savedContribution = \Civi\Api4\Order::create(FALSE) + $orderAPI = \Civi\Api4\Order::create(FALSE) ->setContributionValues($contribution) - ->setLineItems($lineItems) - ->execute() - ->first(); - - $event->setEntityId(0, $savedContribution['id']); - + ->setLineItems($lineItems); if ($contribution['recur_period'] ?? NULL) { - $this->createContributionRecur($savedContribution['id'], $contribution['recur_period']); + $orderAPI->setContributionRecurValues($this->getContributionRecurValues($contribution['recur_period'])); } + $savedContribution = $orderAPI->execute() + ->first(); + $event->setEntityId(0, $savedContribution['id']); } /** @@ -220,14 +218,7 @@ private function getLineItemsForRecord(string $entityType, array $values, array /** * For a recurring contribution, create a ContributionRecur record as well */ - public function createContributionRecur(int $contributionId, string $recurPeriod) { - // get values we need to reuse from the contribution record - $contribution = \Civi\Api4\Contribution::get(FALSE) - ->addSelect('contact_id', 'total_amount', 'currency', 'is_test') - ->addWhere('id', '=', $contributionId) - ->execute() - ->single(); - + private function getContributionRecurValues(string $recurPeriod): array { // unpack recurPeriod parameter // TODO: provide extendable options (option group) for this $recurParams = match($recurPeriod) { @@ -243,27 +234,9 @@ public function createContributionRecur(int $contributionId, string $recurPeriod }; // calculate the next scheduled date - $nextSched = (new DateTime("+ {$recurParams['frequency_interval']} {$recurParams['frequency_unit']}"))->format('Y-m-d'); - - $recurRecordId = \Civi\Api4\ContributionRecur::create(FALSE) - ->addValue('contact_id', $contribution['contact_id']) - ->addValue('amount', $contribution['total_amount']) - ->addValue('currency', $contribution['currency']) - ->addValue('is_test', $contribution['is_test']) - ->addValue('frequency_unit', $recurParams['frequency_unit']) - ->addValue('frequency_interval', $recurParams['frequency_interval']) - ->addValue('next_sched_contribution_date', $nextSched) - ->execute() - ->single()['id']; - - // attach the existing contribution to the recurring record - \Civi\Api4\Contribution::update(FALSE) - ->addWhere('id', '=', $contributionId) - ->addValue('contribution_recur_id', $recurRecordId) - ->execute(); - - // TODO: do we need to copy the first contribution as a template? - // or will it be used anyway if no template contribution exists + // @todo: Don't think we need this as it should be calculated automatically by BAO/ContributionRecur + $recurParams['next_sched_contribution_date'] = (new DateTime("+ {$recurParams['frequency_interval']} {$recurParams['frequency_unit']}"))->format('Y-m-d'); + return $recurParams; } public function onAfformEntitySort(AfformEntitySortEvent $e): void { diff --git a/tests/phpunit/CRM/Financial/BAO/OrderTest.php b/tests/phpunit/CRM/Financial/BAO/OrderTest.php index b8f7e58c8b98..f72756fc524a 100644 --- a/tests/phpunit/CRM/Financial/BAO/OrderTest.php +++ b/tests/phpunit/CRM/Financial/BAO/OrderTest.php @@ -217,6 +217,96 @@ public function testCreateRecurringOrderForMembership(): void { $this->assertEquals(0, $contributionRecur['is_email_receipt']); } + /** + * The 'currency' and 'is_test' params should be copied across between + * Contribution & ContributionRecur when only one side provides them. + * + * @throws \CRM_Core_Exception + */ + public function testCreateRecurringOrderSharesCurrencyAndIsTestValues(): void { + $this->setUpMembershipPriceSet(); + $contactID = $this->individualCreate(); + + // Provided on the Contribution only -> should be copied to the ContributionRecur. + $contribution = Order::create() + ->setContributionValues([ + 'contact_id' => $contactID, + 'financial_type_id:name' => 'Member Dues', + 'currency' => 'NZD', + 'is_test' => 1, + ]) + ->setContributionRecurValues(['frequency_unit' => 'year']) + ->addLineItem([ + 'price_field_value_id' => $this->ids['PriceFieldValue']['membership_first'], + '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(); + $contributionRecur = \Civi\Api4\ContributionRecur::get(FALSE) + ->addWhere('id', '=', $contribution['contribution_recur_id']) + ->execute()->single(); + $this->assertEquals('NZD', $contributionRecur['currency']); + $this->assertEquals(1, $contributionRecur['is_test']); + + // Provided on the ContributionRecur only -> should be copied to the Contribution. + $contribution = Order::create() + ->setContributionValues([ + 'contact_id' => $contactID, + 'financial_type_id:name' => 'Member Dues', + ]) + ->setContributionRecurValues([ + 'frequency_unit' => 'year', + 'currency' => 'EUR', + 'is_test' => 1, + ]) + ->addLineItem([ + 'price_field_value_id' => $this->ids['PriceFieldValue']['membership_first'], + 'entity_id.join_date' => '2007-01-21', + 'entity_id.start_date' => '2007-01-21', + 'entity_id.end_date' => '2007-12-21', + 'entity_id.source' => 'Payment', + ]) + ->execute()->first(); + $this->assertEquals('EUR', $contribution['currency']); + $this->assertEquals(1, $contribution['is_test']); + } + + /** + * If both sides explicitly provide a different currency, neither should + * be overridden by the other (there's no reason to prioritise one entity + * over the other). + * + * @throws \CRM_Core_Exception + */ + public function testCreateRecurringOrderDoesNotOverrideExplicitSharedValues(): void { + $this->setUpMembershipPriceSet(); + $contribution = Order::create() + ->setContributionValues([ + 'contact_id' => $this->individualCreate(), + 'financial_type_id:name' => 'Member Dues', + 'currency' => 'NZD', + ]) + ->setContributionRecurValues([ + 'frequency_unit' => 'year', + 'currency' => 'EUR', + ]) + ->addLineItem([ + 'price_field_value_id' => $this->ids['PriceFieldValue']['membership_first'], + '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(); + $this->assertEquals('NZD', $contribution['currency']); + $contributionRecur = \Civi\Api4\ContributionRecur::get(FALSE) + ->addWhere('id', '=', $contribution['contribution_recur_id']) + ->execute()->single(); + $this->assertEquals('EUR', $contributionRecur['currency']); + } + /** * Test creating an order containing items from 2 price sets plus an ad hoc amount. *