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

5.25 #17199

Merged
merged 10 commits into from
Apr 30, 2020
Merged

5.25 #17199

merged 10 commits into from
Apr 30, 2020

Conversation

seamuslee001
Copy link
Contributor

No description provided.

eileenmcnaughton and others added 10 commits April 28, 2020 19:38
It seems that, depending on the order in which settings are loaded, this is not always populated. Moving it here
addresses, although medium term we should address all references to it
…irtual

The `SettingsBag` uses the `$combined` property to (locally) cache a full
view of the active settings (based on combining different layers of data --
default-values, explicit-values, and mandatory-values). This design is
good for read-mostly data.

This patch changes the `__ContributionSettings` to be another layer in the
construction of `$combined` -- the virtual-values layer.  It fixes issues
wherein:

1. The virtual value is recomputed during every call to `get($key)`.

2. The virtual value is only based on explicit-values -- it disregards
   default-values and mandatory-values.
The default values were previously stored in 'contribution_invoice_settings', but
the updates are stored in individual settings, which makes it difficult to reconcile
the default values and explicit values.

With this revision, 'contribution_invoice_settings' is strictly virtual, and
all the other settings (like 'invoice_due_date') are canonical.
Consider this use-case:

* Install 5.22
* Configure `CiviContribute Component Settings`
* Upgrade to 5.25.beta

The settings are lost during the upgrade.
…_settings.invoicing'

In the old setting `contribution_invoice_settings`, the `invoicing`
property (aka "CiviContribute Component Settings" => "Enable Tax and
Invoicing") could take one of two values:

1. If enabled, it's an array with trueish subkey: `['invoicing'=>1]`
2. If disabled, it's a numeric zero.

This encoding is weird, and `contribution_invoice_settings` is generally deprecated
and has been replaced by a virtual rendition which needs to be consistent with the
old representation -- so that a couple-dozen things continue working.

For testing, it helped me to try these commands at different points:

```
cv ev 'return CRM_Invoicing_Utils::isInvoicingEnabled();'
cv api setting.get return=invoicing
cv api setting.get return=contribution_invoice_settings

select * from civicrm_setting where name like 'contrib%' or name like 'invoic%';
```
…rtion

In practice, for the old setting `contribution_invoice_settings`, the
`invoicing` property could take one of two values:

1. If enabled, the value is an array with trueish subkey: `['invoicing'=>['invoicing'=>1]]`
2. If disabled, the value is a numeric `0`: `['invoicing'=>0]`

Following this means better compatibility.

The format is counter-intuitive, and the unit-test sets the value to enabled
using a more intuitive format (`['invoicing'=>1]`).  It's currently valid to
`set()` this way, but `get()`ting the value produces the
more-compatible/historical format.
 Fix loading, saving, and upgrade of CiviContribute settings
@civibot
Copy link

civibot bot commented Apr 30, 2020

(Standard links)

@civibot civibot bot added the master label Apr 30, 2020
@seamuslee001 seamuslee001 merged commit 1fbf24b into civicrm:master Apr 30, 2020
@seamuslee001 seamuslee001 deleted the new_master branch April 30, 2020 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants