Fix line item 'title' determination
authorEileen McNaughton <emcnaughton@wikimedia.org>
Tue, 2 Aug 2022 06:39:34 +0000 (18:39 +1200)
committerEileen McNaughton <emcnaughton@wikimedia.org>
Thu, 4 Aug 2022 06:34:01 +0000 (18:34 +1200)
CRM/Financial/BAO/Order.php
CRM/Price/BAO/PriceField.php
tests/phpunit/CRM/Contribute/Form/ContributionTest.php

index 2397f483ced5a9ab8d2f4270fc1fd36d3deafc18..941f65e9e4dcbde3a23d471803b84ff7ab6f934a 100644 (file)
@@ -458,12 +458,6 @@ class CRM_Financial_BAO_Order {
       foreach ($this->getPriceOptions() as $fieldID => $valueID) {
         $this->setPriceSetIDFromSelectedField($fieldID);
       }
-      if (!$this->priceSetID && $this->getTemplateContributionID()) {
-        // Load the line items from the contribution.
-        foreach ($this->getLineItems() as $lineItem) {
-          return $lineItem['price_field_id.price_set_id'];
-        }
-      }
     }
     return $this->priceSetID;
   }
@@ -580,7 +574,7 @@ class CRM_Financial_BAO_Order {
    * @return array
    */
   public function getPriceFieldSpec(int $id) :array {
-    return $this->getPriceFieldsMetadata()[$id];
+    return $this->getPriceFieldsMetadata()[$id] ?? $this->getPriceFieldMetadata($id);
   }
 
   /**
@@ -613,6 +607,28 @@ class CRM_Financial_BAO_Order {
     return $this->priceFieldMetadata;
   }
 
+  /**
+   * Get the metadata for the given price field.
+   *
+   * Note this uses a different method to getPriceFieldMetadata.
+   *
+   * There is an assumption in the code currently that all purchases
+   * are within a single price set. However, discussions have been around
+   * the idea that when form-builder supports contributions price sets will
+   * not be used as form-builder in itself is a configuration unit.
+   *
+   * Currently there are couple of unit tests that mix & match & rather than
+   * updating the tests to avoid notices when orders are loaded for receipting,
+   * the migration to this new method is starting....
+   *
+   * @param int $id
+   *
+   * @return array
+   */
+  public function getPriceFieldMetadata(int $id): array {
+    return CRM_Price_BAO_PriceField::getPriceField($id);
+  }
+
   /**
    * Set the metadata for the order.
    *
@@ -819,6 +835,10 @@ class CRM_Financial_BAO_Order {
     $params['financial_type_id'] = 0;
     if ($this->getTemplateContributionID()) {
       $lineItems = $this->getLinesFromTemplateContribution();
+      // Set the price set ID from the first line item (we need to set this here
+      // to prevent a loop later when we retrieve the price field metadata to
+      // set the 'title' (as accessed from workflow message templates).
+      $this->setPriceSetID($lineItems[0]['price_field_id.price_set_id']);
     }
     else {
       foreach ($this->getPriceOptions() as $fieldID => $valueID) {
@@ -855,7 +875,7 @@ class CRM_Financial_BAO_Order {
       elseif ($taxRate) {
         $lineItem['tax_amount'] = ($taxRate / 100) * $lineItem['line_total'];
       }
-      $lineItem['title'] = $lineItem['label'];
+      $lineItem['title'] = $this->getLineItemTitle($lineItem);
     }
     return $lineItems;
   }
@@ -1001,15 +1021,7 @@ class CRM_Financial_BAO_Order {
       }
     }
     if (empty($lineItem['title'])) {
-      // Title is used in output for workflow templates.
-      $htmlType = !empty($this->priceFieldMetadata) ? $this->getPriceFieldSpec($lineItem['price_field_id'])['html_type'] : NULL;
-      $lineItem['title'] = (!$htmlType || $htmlType === 'Text') ? $lineItem['label'] : $this->getPriceFieldSpec($lineItem['price_field_id'])['label'] . ' : ' . $lineItem['label'];
-      if (!empty($lineItem['price_field_value_id'])) {
-        $description = $this->priceFieldValueMetadata[$lineItem['price_field_value_id']]['description'] ?? '';
-        if ($description) {
-          $lineItem['title'] .= ' ' . CRM_Utils_String::ellipsify($description, 30);
-        }
-      }
+      $lineItem['title'] = $this->getLineItemTitle($lineItem);
     }
     $this->lineItems[$index] = $lineItem;
   }
@@ -1211,4 +1223,27 @@ class CRM_Financial_BAO_Order {
     $lineItem = array_merge($defaults, $lineItem);
   }
 
+  /**
+   * Get a 'title' for the line item.
+   *
+   * This descriptor is used in message templates. It could conceivably
+   * by used elsewhere but if so determination would likely move to the api.
+   *
+   * @param array $lineItem
+   *
+   * @return string
+   */
+  private function getLineItemTitle(array $lineItem): string {
+    // Title is used in output for workflow templates.
+    $htmlType = $this->getPriceFieldSpec($lineItem['price_field_id'])['html_type'] ?? NULL;
+    $lineItemTitle = (!$htmlType || $htmlType === 'Text') ? $lineItem['label'] : $this->getPriceFieldSpec($lineItem['price_field_id'])['label'] . ' - ' . $lineItem['label'];
+    if (!empty($lineItem['price_field_value_id'])) {
+      $description = $this->priceFieldValueMetadata[$lineItem['price_field_value_id']]['description'] ?? '';
+      if ($description) {
+        $lineItemTitle .= ' ' . CRM_Utils_String::ellipsify($description, 30);
+      }
+    }
+    return $lineItemTitle;
+  }
+
 }
index aa03d74aeefeacd030be1c9d75d1b5b4883879f9..9a3d87b76263db4a630b37b53e45a46e160f9139 100644 (file)
@@ -9,6 +9,8 @@
  +--------------------------------------------------------------------+
  */
 
+use Civi\Api4\PriceField;
+
 /**
  * Business objects for managing price fields.
  *
@@ -164,6 +166,7 @@ class CRM_Price_BAO_PriceField extends CRM_Price_DAO_PriceField {
     }
 
     $transaction->commit();
+    Civi::cache('metadata')->flush();
     return $priceField;
   }
 
@@ -872,4 +875,22 @@ WHERE  id IN (" . implode(',', array_keys($priceFields)) . ')';
     return 0;
   }
 
+  /**
+   * Get a specific price field (leveraging the cache).
+   *
+   * @param int $id
+   *
+   * @return array
+   * @noinspection PhpUnhandledExceptionInspection
+   * @noinspection PhpDocMissingThrowsInspection
+   */
+  public static function getPriceField(int $id): array {
+    $cacheString = __CLASS__ . __FUNCTION__ . $id . CRM_Core_Config::domainID() . '_' . CRM_Core_I18n::getLocale();
+    if (!Civi::cache('metadata')->has($cacheString)) {
+      $field = PriceField::get(FALSE)->addWhere('id', '=', $id)->execute()->first();
+      Civi::cache('metadata')->set($cacheString, $field);
+    }
+    return Civi::cache('metadata')->get($cacheString);
+  }
+
 }
index 50febfabb8b0317a737ba15b8bf6956f04d2f610..e09880b2906b593b202b88e1ac4d2d85dae4d1d5 100644 (file)
@@ -712,7 +712,7 @@ Financial Type: Donation
 ---------------------------------------------------------
 Item                             Qty       Each    Subtotal Tax Rate Tax Amount       Total
 ----------------------------------------------------------
-Price Field 1                      1    $100.00    $100.00  10.00 %       $10.00        $110.00
+Price Field - Price Field 1        1    $100.00    $100.00  10.00 %       $10.00        $110.00
 
 
 Amount before Tax : $100.00
@@ -737,7 +737,7 @@ Financial Type: Donation
 ---------------------------------------------------------
 Item                             Qty       Each       Total
 ----------------------------------------------------------
-Price Field 1                      1    $100.00       $100.00
+Price Field - Price Field 1        1    $100.00       $100.00