Skip to content
This repository has been archived by the owner on Jul 7, 2021. It is now read-only.

Conversation

ahed-compucorp
Copy link
Contributor

@ahed-compucorp ahed-compucorp commented Mar 31, 2021

Overview

This PR fixed the issue when adding more than one line item at a time on CiviCRM 5.35.1 .

Before

Peek 2021-03-29 15-34

After

Peek 2021-03-29 15-37

Technical Details

This extension will create price field for each new line item inside the foreach loop.

foreach ($lineItemParams as $key => $lineItem) {
  if ($lineItem['price_field_value_id'] == 'new') {
    list($lineItem['price_field_id'], $lineItem['price_field_value_id']) = CRM_Lineitemedit_Util::createPriceFieldByContributionID($entityID);
  }
      
  ...
  $newLineItem[] = civicrm_api3('LineItem', 'create', $newLineItemParams)['id'];
}

When creating the line item CiviCRM internally will validate each price field and one of the validation steps is fetching the potential values/options to check against - see utils.php#L2276.

$options = civicrm_api($entity, 'getoptions', $options_lookup_params);

CiviCRM will cache getoptions result in PseudoConstant.php#L278

\Civi::$statics[__CLASS__][$cacheKey] = $output;

Everything is fine for one line item creating but when we create two items or more in one go, the price fields ids was cached in the first iteration during the creating of the first line item and CiviCRM will throw error when creating the second line item because the price field is invalid.

To overcome this issue, I moved the creation of line items outside of the foreach loop. That way the main foreach loop will prepare the params and create the required price fields and then will create the line items in the new foreach loop.

@JoeMurray
Copy link
Member

@seamuslee001 could you look at this? I don't recall this being a problem but maybe we were not testing this workflow.

@JoeMurray JoeMurray assigned JoeMurray and seamuslee001 and unassigned JoeMurray Mar 31, 2021
@ahed-compucorp ahed-compucorp force-pushed the MAE-508-fix-creating-two-line-items-at-a-time branch from d425dd5 to 8cee004 Compare March 31, 2021 12:14
@ahed-compucorp
Copy link
Contributor Author

Thank you @JoeMurray.
@seamuslee001 I've just edited the commit message.

@seamuslee001
Copy link
Member

I tried replicating this locally but couldn't but the code looks sensible and UTs work

@seamuslee001 seamuslee001 merged commit ff83773 into JMAConsulting:master Apr 15, 2021
@JoeMurray
Copy link
Member

Many thanks, @ahed-compucorp !

@ahed-compucorp
Copy link
Contributor Author

@seamuslee001 Thank you.

Can I ask when will be the next release/tag?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants