diff --git a/CRM/Upgrade/Incremental/php/FiveTwentyFive.php b/CRM/Upgrade/Incremental/php/FiveTwentyFive.php index 892053f1a6ae..e7f269ed06e9 100644 --- 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 new file mode 100644 index 000000000000..1d9fd478e2d5 --- /dev/null +++ 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 index 28f3ef36869e..be51cc6ae415 100644 --- 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 index 57b57eda2a6e..be0c94a8ef31 100644 --- 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 index c986c6c6ea73..34cf4a8f47d7 100644 --- a/tests/phpunit/CRM/Core/BAO/SettingTest.php +++ b/tests/phpunit/CRM/Core/BAO/SettingTest.php @@ -91,6 +91,7 @@ public function testHandlingOfContributionInvoiceSetting() { 'notes' => '

Give me money

', '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 index 54993ff03184..6271337b29c0 100644 --- 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 index d18afe4039d6..68495f0d72de 100644 --- 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;