Merge pull request #17199 from seamuslee001/new_master

5.25
civicrm · Apr 30, 2020 · 1fbf24b · 1fbf24b
2 parents 45f4a87 + c5e2eef
commit 1fbf24b
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 25 deletions.
diff --git a/CRM/Upgrade/Incremental/php/FiveTwentyFive.php b/CRM/Upgrade/Incremental/php/FiveTwentyFive.php
@@ -79,6 +79,11 @@ public function upgrade_5_25_alpha1($rev) {
     $this->addTask('Convert Report Form dates from jcalander to datepicker', 'convertReportsJcalendarToDatePicker');
   }
 
+  public function upgrade_5_25_beta3($rev) {
+    $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev);
+    $this->addTask('Convert CiviContribute settings', 'updateContributeSettings');
+  }
+
   /**
    * Convert date fields stored in civicrm_report_instance to that format for datepicker
    */

diff --git a/CRM/Upgrade/Incremental/sql/5.25.beta3.mysql.tpl b/CRM/Upgrade/Incremental/sql/5.25.beta3.mysql.tpl
@@ -0,0 +1 @@
+{* file to handle db changes in 5.25.beta3 during upgrade *}
diff --git a/Civi/Core/SettingsBag.php b/Civi/Core/SettingsBag.php
@@ -136,7 +136,6 @@ public function loadValues() {
       while ($dao->fetch()) {
         $this->values[$dao->name] = ($dao->value !== NULL) ? \CRM_Utils_String::unserialize($dao->value) : NULL;
       }
-      $dao->values['contribution_invoice_settings'] = $this->getContributionSettings();
     }
 
     return $this;
@@ -167,6 +166,10 @@ public function all() {
       $this->combined = $this->combine(
         [$this->defaults, $this->values, $this->mandatory]
       );
+      // computeVirtual() depends on completion of preceding pass.
+      $this->combined = $this->combine(
+        [$this->combined, $this->computeVirtual()]
+      );
     }
     return $this->combined;
   }
@@ -254,8 +257,7 @@ public function revert($key) {
    * @return SettingsBag
    */
   public function set($key, $value) {
-    if ($key === 'contribution_invoice_settings') {
-      $this->setContributionSettings($value);
+    if ($this->updateVirtual($key, $value)) {
       return $this;
     }
     $this->setDb($key, $value);
@@ -265,6 +267,8 @@ public function set($key, $value) {
   }
 
   /**
+   * Update a virtualized/deprecated setting.
+   *
    * Temporary handling for phasing out contribution_invoice_settings.
    *
    * Until we have transitioned we need to handle setting & retrieving
@@ -274,29 +278,44 @@ public function set($key, $value) {
    *
    * https://lab.civicrm.org/dev/core/issues/1558
    *
+   * @param string $key
    * @param array $value
+   * @return bool
+   *   TRUE if $key is a virtualized setting. FALSE if it is a normal setting.
    */
-  public function setContributionSettings($value) {
-    foreach (SettingsBag::getContributionInvoiceSettingKeys() as $possibleKeyName => $settingName) {
-      $keyValue = $value[$possibleKeyName] ?? '';
-      $this->set($settingName, $keyValue);
+  public function updateVirtual($key, $value) {
+    if ($key === 'contribution_invoice_settings') {
+      foreach (SettingsBag::getContributionInvoiceSettingKeys() as $possibleKeyName => $settingName) {
+        $keyValue = $value[$possibleKeyName] ?? '';
+        if ($possibleKeyName === 'invoicing' && is_array($keyValue)) {
+          $keyValue = $keyValue['invoicing'];
+        }
+        $this->set($settingName, $keyValue);
+      }
+      return TRUE;
     }
-    $this->values['contribution_invoice_settings'] = $this->getContributionSettings();
+    return FALSE;
   }
 
   /**
-   * Temporary function to handle returning the contribution_settings key despite it being deprecated.
-   *
-   * See more in comment block on previous function.
+   * Determine the values of any virtual/computed settings.
    *
    * @return array
    */
-  public function getContributionSettings() {
+  public function computeVirtual() {
     $contributionSettings = [];
     foreach (SettingsBag::getContributionInvoiceSettingKeys() as $keyName => $settingName) {
-      $contributionSettings[$keyName] = $this->values[$settingName] ?? '';
+      switch ($keyName) {
+        case 'invoicing':
+          $contributionSettings[$keyName] = $this->get($settingName) ? [$keyName => 1] : 0;
+          break;
+
+        default:
+          $contributionSettings[$keyName] = $this->get($settingName);
+          break;
+      }
     }
-    return $contributionSettings;
+    return ['contribution_invoice_settings' => $contributionSettings];
   }
 
   /**

diff --git a/settings/Contribute.setting.php b/settings/Contribute.setting.php
@@ -41,17 +41,8 @@
     'group' => 'contribute',
     'name' => 'contribution_invoice_settings',
     'type' => 'Array',
-    'default' => [
-      'invoice_prefix' => 'INV_',
-      'credit_notes_prefix' => 'CN_',
-      'due_date' => '10',
-      'due_date_period' => 'days',
-      'notes' => '',
-      'tax_term' => 'Sales Tax',
-      'tax_display_settings' => 'Inclusive',
-    ],
     'add' => '4.7',
-    'title' => ts('Deprecated setting'),
+    'title' => ts('Deprecated, virtualized setting'),
     'is_domain' => 1,
     'is_contact' => 0,
     'help_text' => NULL,
@@ -74,6 +65,7 @@
     'settings_pages' => ['contribute' => ['weight' => 90]],
   ],
   'invoice_prefix' => [
+    'default' => 'INV_',
     'html_type' => 'text',
     'name' => 'invoice_prefix',
     'add' => '5.23',
@@ -84,6 +76,7 @@
     'is_contact' => 0,
   ],
   'invoice_due_date' => [
+    'default' => '10',
     'name' => 'invoice_due_date',
     'html_type' => 'text',
     'title' => ts('Due Date'),
@@ -93,6 +86,7 @@
     'is_contact' => 0,
   ],
   'invoice_due_date_period' => [
+    'default' => 'days',
     'html_type' => 'select',
     'name' => 'invoice_due_date_period',
     'title' => ts('For transmission'),
@@ -110,6 +104,7 @@
     ],
   ],
   'invoice_notes' => [
+    'default' => '',
     'name' => 'invoice_notes',
     'html_type' => 'wysiwyg',
     'title' => ts('Notes or Standard Terms'),
@@ -131,6 +126,7 @@
     'description' => ts('Should a pdf invoice be emailed automatically?'),
   ],
   'tax_term' => [
+    'default' => 'Sales Tax',
     'name' => 'tax_term',
     'html_type' => 'text',
     'add' => '5.23',
@@ -140,6 +136,7 @@
     'is_contact' => 0,
   ],
   'tax_display_settings' => [
+    'default' => 'Inclusive',
     'html_type' => 'select',
     'name' => 'tax_display_settings',
     'type' => CRM_Utils_Type::T_STRING,

diff --git a/tests/phpunit/CRM/Core/BAO/SettingTest.php b/tests/phpunit/CRM/Core/BAO/SettingTest.php
@@ -91,6 +91,7 @@ public function testHandlingOfContributionInvoiceSetting() {
       'notes' => '<p>Give me money</p>',
       'tax_term' => 'Extortion',
       'tax_display_settings' => 'Exclusive',
+      // NOTE: This form of `invoicing` is accepted, but it may be normalized to the idiomatic form with a nested array.
       'invoicing' => 1,
       'is_email_pdf' => '1',
     ];
@@ -100,7 +101,7 @@ public function testHandlingOfContributionInvoiceSetting() {
     $getVersion = $this->callAPISuccessGetValue('Setting', ['name' => 'contribution_invoice_settings']);
     $this->assertEquals($settingsFromAPI, $settingsFromGet);
     $this->assertAPIArrayComparison($getVersion, $settingsFromGet);
-    $this->assertEquals($contributionSettings, $settingsFromGet);
+    $this->assertEquals(['invoicing' => ['invoicing' => 1]] + $contributionSettings, $settingsFromGet);
 
     // These are the preferred retrieval methods.
     $this->assertEquals('G_', Civi::settings()->get('invoice_prefix'));

diff --git a/tests/phpunit/Civi/Core/SettingsBagTest.php b/tests/phpunit/Civi/Core/SettingsBagTest.php
@@ -30,4 +30,50 @@ public function testInnoDbFTS() {
     $this->assertEquals(0, $settingsBag->get('enable_innodb_fts'));
   }
 
+  /**
+   * The setting "contribution_invoice_settings" is actually a virtual value built on other settings.
+   * Check that various updates work as expected.
+   */
+  public function testVirtualContributionSetting_explicit() {
+    $s = \Civi::settings();
+
+    $this->assertEquals(10, $s->get('contribution_invoice_settings')['due_date']);
+    $this->assertEquals(10, $s->get('invoice_due_date'));
+    $this->assertEquals(NULL, $s->getExplicit('invoice_due_date'));
+
+    $s->set('invoice_due_date', 20);
+    $this->assertEquals(20, $s->get('contribution_invoice_settings')['due_date']);
+    $this->assertEquals(20, $s->get('invoice_due_date'));
+    $this->assertEquals(20, $s->getExplicit('invoice_due_date'));
+
+    $s->set('contribution_invoice_settings', array_merge($s->get('contribution_invoice_settings'), [
+      'due_date' => 30,
+    ]));
+    $this->assertEquals(30, $s->get('contribution_invoice_settings')['due_date']);
+    $this->assertEquals(30, $s->get('invoice_due_date'));
+    $this->assertEquals(30, $s->getExplicit('invoice_due_date'));
+
+    $s->revert('invoice_due_date');
+    $this->assertEquals(10, $s->get('contribution_invoice_settings')['due_date']);
+    $this->assertEquals(10, $s->get('invoice_due_date'));
+    $this->assertEquals(NULL, $s->getExplicit('invoice_due_date'));
+  }
+
+  /**
+   * The setting "contribution_invoice_settings" is actually a virtual value built on other settings.
+   * Check that mandatory values ($civicrm_settings) are respected.
+   */
+  public function testVirtualContributionSetting_mandatory() {
+    $s = \Civi::settings();
+    $this->assertEquals(10, $s->get('contribution_invoice_settings')['due_date']);
+    $this->assertEquals(10, $s->get('invoice_due_date'));
+    $this->assertEquals(NULL, $s->getExplicit('invoice_due_date'));
+
+    $s->loadMandatory(['invoice_due_date' => 30]);
+
+    $this->assertEquals(30, $s->get('contribution_invoice_settings')['due_date']);
+    $this->assertEquals(30, $s->get('invoice_due_date'));
+    $this->assertEquals(NULL, $s->getExplicit('invoice_due_date'));
+  }
+
 }
diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
@@ -258,6 +258,8 @@ public function testCreateCheckContribution() {
 
   /**
    * Test the 'return' param works for all fields.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testGetContributionReturnFunctionality() {
     $params = $this->_params;
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		{* file to handle db changes in 5.25.beta3 during upgrade *}