Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#1558 Data conversion for non-standard setting. #16424

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions CRM/Core/SelectValues.php
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,27 @@ public static function geoProvider() {
return $geo;
}

/**
* Get options for displaying tax.
*
* @return array
*
* @throws \CRM_Core_Exception
*/
public function taxDisplayOptions() {
return [
'Do_not_show' => ts('Do not show breakdown, only show total - i.e %1', [
1 => CRM_Utils_Money::format(120),
]),
'Inclusive' => ts('Show [tax term] inclusive price - i.e. %1', [
1 => ts('%1 (includes [tax term] of %2)', [1 => CRM_Utils_Money::format(120), 2 => CRM_Utils_Money::format(20)]),
]),
'Exclusive' => ts('Show [tax term] exclusive price - i.e. %1', [
1 => ts('%1 + %2 [tax term]', [1 => CRM_Utils_Money::format(120), 2 => CRM_Utils_Money::format(20)]),
]),
];
}

/**
* Get the Address Standardization Providers from available plugins.
*
Expand Down
4 changes: 3 additions & 1 deletion CRM/Invoicing/Utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class CRM_Invoicing_Utils {
* @param bool $oldValue
* @param bool $newValue
* @param array $metadata
*
* @throws \CiviCRM_API3_Exception
*/
public static function onToggle($oldValue, $newValue, $metadata) {
if ($oldValue == $newValue) {
Expand All @@ -32,7 +34,7 @@ public static function onToggle($oldValue, $newValue, $metadata) {
$existingUserViewOptions = civicrm_api3('Setting', 'get', ['return' => 'user_dashboard_options'])['values'][CRM_Core_Config::domainID()]['user_dashboard_options'];
$optionValues = civicrm_api3('Setting', 'getoptions', ['field' => 'user_dashboard_options'])['values'];
$invoiceKey = array_search('Invoices / Credit Notes', $optionValues);
$existingIndex = in_array($invoiceKey, $existingUserViewOptions);
$existingIndex = array_search($invoiceKey, $existingUserViewOptions);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was removing key 1 not key 10 - see below we do a strict comparison on FALSE so a return of 0 is fine.

This was the cause of some weird test errors we occasionally saw


if ($newValue && $existingIndex === FALSE) {
$existingUserViewOptions[] = $invoiceKey;
Expand Down
17 changes: 12 additions & 5 deletions CRM/Upgrade/Incremental/Base.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
+--------------------------------------------------------------------+
*/

use Civi\Core\SettingsBag;

/**
* Base class for incremental upgrades
*/
Expand Down Expand Up @@ -195,11 +197,16 @@ public static function updateMessageTemplates($ctx, $version) {
* @return bool
*/
public static function updateContributeSettings($ctx) {
$settings = Civi::settings()->get('contribution_invoice_settings');
$metadata = \Civi\Core\SettingsMetadata::getMetadata();
$conversions = array_intersect_key((array) $settings, $metadata);
foreach ($conversions as $key => $conversion) {
Civi::settings()->set($key, $conversion);
// Use a direct query as api now does some handling on this.
$settings = CRM_Core_DAO::executeQuery("SELECT value, domain_id FROM civicrm_setting WHERE name = 'contribution_invoice_settings'");

while ($settings->fetch()) {
$contributionSettings = (array) CRM_Utils_String::unserialize($settings->value);
foreach (array_merge(SettingsBag::getContributionInvoiceSettingKeys(), ['deferred_revenue_enabled' => 'deferred_revenue_enabled']) as $possibleKeyName => $settingName) {
if (!empty($contributionSettings[$possibleKeyName]) && empty(Civi::settings($settings->domain_id)->getExplicit($settingName))) {
Civi::settings($settings->domain_id)->set($settingName, $contributionSettings[$possibleKeyName]);
}
}
}
return TRUE;
}
Expand Down
58 changes: 58 additions & 0 deletions Civi/Core/SettingsBag.php
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ 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;
Expand Down Expand Up @@ -253,12 +254,51 @@ public function revert($key) {
* @return SettingsBag
*/
public function set($key, $value) {
if ($key === 'contribution_invoice_settings') {
$this->setContributionSettings($value);
return $this;
}
$this->setDb($key, $value);
$this->values[$key] = $value;
$this->combined = NULL;
return $this;
}

/**
* Temporary handling for phasing out contribution_invoice_settings.
*
* Until we have transitioned we need to handle setting & retrieving
* contribution_invoice_settings.
*
* Once removed from core we will add deprecation notices & then remove this.
*
* https://lab.civicrm.org/dev/core/issues/1558
*
* @param array $value
*/
public function setContributionSettings($value) {
foreach (SettingsBag::getContributionInvoiceSettingKeys() as $possibleKeyName => $settingName) {
$keyValue = $value[$possibleKeyName] ?? '';
$this->set($settingName, $keyValue);
}
$this->values['contribution_invoice_settings'] = $this->getContributionSettings();
}

/**
* Temporary function to handle returning the contribution_settings key despite it being deprecated.
*
* See more in comment block on previous function.
*
* @return array
*/
public function getContributionSettings() {
$contributionSettings = [];
foreach (SettingsBag::getContributionInvoiceSettingKeys() as $keyName => $settingName) {
$contributionSettings[$keyName] = $this->values[$settingName] ?? '';
}
return $contributionSettings;
}

/**
* @return \CRM_Utils_SQL_Select
*/
Expand Down Expand Up @@ -378,4 +418,22 @@ protected function setDb($name, $value) {
}
}

/**
* @return array
*/
public static function getContributionInvoiceSettingKeys(): array {
$convertedKeys = [
'credit_notes_prefix' => 'credit_notes_prefix',
'invoice_prefix' => 'invoice_prefix',
'due_date' => 'invoice_due_date',
'due_date_period' => 'invoice_due_date_period',
'notes' => 'invoice_notes',
'is_email_pdf' => 'invoice_is_email_pdf',
'tax_term' => 'tax_term',
'tax_display_settings' => 'tax_display_settings',
'invoicing' => 'invoicing',
];
return $convertedKeys;
}

}
93 changes: 92 additions & 1 deletion settings/Contribute.setting.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
'tax_display_settings' => 'Inclusive',
],
'add' => '4.7',
'title' => ts('Contribution Invoice Settings'),
'title' => ts('Deprecated setting'),
'is_domain' => 1,
'is_contact' => 0,
'help_text' => NULL,
Expand All @@ -71,6 +71,97 @@
'CRM_Invoicing_Utils::onToggle',
],
],
'credit_notes_prefix' => [
'group_name' => 'Contribute Preferences',
'group' => 'contribute',
'name' => 'credit_notes_prefix',
'html_type' => 'text',
'quick_form_type' => 'Element',
'add' => '5.23',
'type' => CRM_Utils_Type::T_STRING,
'title' => ts('Credit Notes Prefix'),
'is_domain' => 1,
'is_contact' => 0,
'description' => ts('Prefix to be prepended to credit note ids'),
'default' => 'CN_',
'help_text' => ts('The credit note ID is generated when a contribution is set to Refunded, Cancelled or Chargeback. It is visible on invoices, if invoices are enabled'),
],
'invoice_prefix' => [
'html_type' => 'text',
'name' => 'invoice_prefix',
'add' => '5.23',
'type' => CRM_Utils_Type::T_STRING,
'title' => ts('Invoice Prefix'),
'description' => ts('Enter prefix to be be preprended when creating an invoice number'),
'is_domain' => 1,
'is_contact' => 0,
],
'invoice_due_date' => [
'name' => 'invoice_due_date',
'html_type' => 'text',
'title' => ts('Due Date'),
'add' => '5.23',
'type' => CRM_Utils_Type::T_INT,
'is_domain' => 1,
'is_contact' => 0,
],
'invoice_due_date_period' => [
'html_type' => 'select',
'name' => 'invoice_due_date_period',
'title' => ts('For transmission'),
'weight' => 4,
'add' => '5.23',
'type' => CRM_Utils_Type::T_STRING,
'is_domain' => 1,
'is_contact' => 0,
'description' => ts('Select the interval for due date.'),
'options' => [
'select' => ts('- select -'),
'days' => ts('Days'),
'months' => ts('Months'),
'years' => ts('Years'),
],
],
'invoice_notes' => [
'name' => 'invoice_notes',
'html_type' => 'wysiwyg',
'title' => ts('Notes or Standard Terms'),
'type' => CRM_Utils_Type::T_STRING,
'add' => '5.23',
'is_domain' => 1,
'is_contact' => 0,
'description' => ts('Enter note or message to be displayed on PDF invoice or credit notes '),
'attributes' => ['rows' => 2, 'cols' => 40],
],
'invoice_is_email_pdf' => [
'name' => 'invoice_is_email_pdf',
'html_type' => 'checkbox',
'add' => '5.23',
'type' => CRM_Utils_Type::T_BOOLEAN,
'is_domain' => 1,
'is_contact' => 0,
'title' => ts('Automatically email invoice when user purchases online'),
'description' => ts('Should a pdf invoice be emailed automatically?'),
],
'tax_term' => [
'name' => 'tax_term',
'html_type' => 'text',
'add' => '5.23',
'title' => ts('Tax Term'),
'type' => CRM_Utils_Type::T_STRING,
'is_domain' => 1,
'is_contact' => 0,
],
'tax_display_settings' => [
'html_type' => 'select',
'name' => 'tax_display_settings',
'type' => CRM_Utils_Type::T_STRING,
'add' => '5.23',
'title' => ts('Tax Display Settings'),
'is_domain' => 1,
'is_contact' => 0,
'pseudoconstant' => ['callback' => 'CRM_Core_SelectValues::taxDisplayOptions'],
],
'acl_financial_type' => [
'group_name' => 'Contribute Preferences',
'group' => 'contribute',
Expand Down
3 changes: 3 additions & 0 deletions tests/phpunit/CRM/Contact/Page/View/UserDashBoardTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ public function testDashboardContentContributionsWithInvoicingEnabled() {

/**
* Test the content of the dashboard.
*
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
public function testDashboardContentContributions() {
$this->contributionCreate(['contact_id' => $this->contactID]);
Expand Down
66 changes: 56 additions & 10 deletions tests/phpunit/CRM/Core/BAO/SettingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,31 +29,77 @@ public function tearDown() {
parent::tearDown();
}

/**
* Test that enabling a valid component works.
*/
public function testEnableComponentValid() {
$config = CRM_Core_Config::singleton(TRUE, TRUE);

CRM_Core_Config::singleton(TRUE, TRUE);
$result = CRM_Core_BAO_ConfigSetting::enableComponent('CiviCampaign');

$this->assertTrue($result);
}

/**
* Test that we get a success result if we try to enable an enabled component.
*/
public function testEnableComponentAlreadyPresent() {
$config = CRM_Core_Config::singleton(TRUE, TRUE);

$result = CRM_Core_BAO_ConfigSetting::enableComponent('CiviCampaign');
CRM_Core_Config::singleton(TRUE, TRUE);
CRM_Core_BAO_ConfigSetting::enableComponent('CiviCampaign');
$result = CRM_Core_BAO_ConfigSetting::enableComponent('CiviCampaign');

$this->assertTrue($result);
}

/**
* Test that we get a false result if we try to enable an invalid component.
*/
public function testEnableComponentInvalid() {
$config = CRM_Core_Config::singleton(TRUE, TRUE);

CRM_Core_Config::singleton(TRUE, TRUE);
$result = CRM_Core_BAO_ConfigSetting::enableComponent('CiviFake');

$this->assertFalse($result);
}

/**
* Test temporary retrieval & setting of converted settings.
*
* As a transitional measure we allow the settings that were munged into
* contribution_invoice_setting. This tests that the current method of getting via the 'old' key
* works. This will be deprecated & removed over the next few versions but
* 1) some extensions use these settings &
* 2) there is a lot of work to fix this mess in core so a transitional method makes sense.
*
* https://lab.civicrm.org/dev/core/issues/1558
*
* @throws \CRM_Core_Exception
*/
public function testHandlingOfContributionInvoiceSetting() {
$contributionSettings = [
'invoice_prefix' => 'G_',
'credit_notes_prefix' => 'XX_',
'due_date' => '20',
'due_date_period' => 'weeks',
'notes' => '<p>Give me money</p>',
'tax_term' => 'Extortion',
'tax_display_settings' => 'Exclusive',
'invoicing' => 1,
'is_email_pdf' => '1',
];
Civi::settings()->set('contribution_invoice_settings', $contributionSettings);
$settingsFromGet = Civi::settings()->get('contribution_invoice_settings');
$settingsFromAPI = $this->callAPISuccess('Setting', 'get', ['return' => 'contribution_invoice_settings'])['values'][CRM_Core_Config::domainID()]['contribution_invoice_settings'];
$getVersion = $this->callAPISuccessGetValue('Setting', ['name' => 'contribution_invoice_settings']);
$this->assertEquals($settingsFromAPI, $settingsFromGet);
$this->assertAPIArrayComparison($getVersion, $settingsFromGet);
$this->assertEquals($contributionSettings, $settingsFromGet);

// These are the preferred retrieval methods.
$this->assertEquals('G_', Civi::settings()->get('invoice_prefix'));
$this->assertEquals('XX_', Civi::settings()->get('credit_notes_prefix'));
$this->assertEquals('20', Civi::settings()->get('invoice_due_date'));
$this->assertEquals('weeks', Civi::settings()->get('invoice_due_date_period'));
$this->assertEquals('<p>Give me money</p>', Civi::settings()->get('invoice_notes'));
$this->assertEquals('Extortion', Civi::settings()->get('tax_term'));
$this->assertEquals('Exclusive', Civi::settings()->get('tax_display_settings'));
}

/**
* Ensure that overrides in $civicrm_setting apply when
* using getItem($group,$name).
Expand Down
4 changes: 3 additions & 1 deletion tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ public function tearDown() {
*
* The scenario is that a pending contribution exists and the IPN call will update it to completed.
* And also if Tax and Invoicing is enabled, this unit test ensure that invoice pdf is attached with email recipet
*
* @throws \CRM_Core_Exception
*/
public function testInvoiceSentOnIPNPaymentSuccess() {
$this->enableTaxAndInvoicing();
Expand All @@ -81,7 +83,7 @@ public function testInvoiceSentOnIPNPaymentSuccess() {
$_REQUEST = ['q' => CRM_Utils_System::url('civicrm/payment/ipn/' . $this->_paymentProcessorID)] + $this->getPaypalTransaction();

$mut = new CiviMailUtils($this, TRUE);
$paymentProcesors = civicrm_api3('PaymentProcessor', 'getsingle', ['id' => $this->_paymentProcessorID]);
$paymentProcesors = $this->callAPISuccessGetSingle('PaymentProcessor', ['id' => $this->_paymentProcessorID]);
$payment = Civi\Payment\System::singleton()->getByProcessor($paymentProcesors);
$payment->handlePaymentNotification();

Expand Down
2 changes: 1 addition & 1 deletion tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ public function testBalanceDueIfDeferredRevenueEnabled() {
]);
$balance = CRM_Contribute_BAO_Contribution::getContributionBalance($contribution['id'], $totalAmount);
$this->assertEquals(0.0, $balance);
Civi::settings()->revert('contribution_invoice_settings');
Civi::settings()->set('deferred_revenue_enabled', FALSE);
}

/**
Expand Down
Loading