dev/core#731 Add upgrade routine to convert on_hold to an array for sites with #13731
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
| @@ -75,6 +75,18 @@ public function setPostUpgradeMessage(&$postUpgradeMessage, $rev) { | |||
| public function upgrade_5_11_alpha1($rev) { | |||
| $this->addTask(ts('Upgrade DB to %1: SQL', array(1 => $rev)), 'runSql', $rev); | |||
| $this->addTask('Update smart groups where jcalendar fields have been converted to datepicker', 'updateSmartGroups', $rev); | |||
| if (Civi::settings()->get('civimail_multiple_bulk_emails')) { | |||
There was a problem hiding this comment.
@eileenmcnaughton shouldn't this be in 5_11_beta1 as it is in the RC?
There was a problem hiding this comment.
@seamuslee001 I guess - I can move it there if you think that is better
There was a problem hiding this comment.
@seamuslee001 ok updated
eileenmcnaughton
changed the title
eileenmcnaughton
force-pushed
the
5.11
branch
3 times, most recently
from
27e58bb to
5d7371c
Compare
civimail_multiple_bulk_emails set.
with that set we get a select box which needs an array rather than a checkbox.
Note from my testing there is problem loading the defaults due to the field name being
wrong - this aligns groups created before & after the 5.9 upgrade but does not resolve that.
// preferred communication method
if (Civi::settings()->get('civimail_multiple_bulk_emails')) {
->addSelect('email_on_hold',
array('entity' => 'email', 'multiple' => 'multiple', 'label' => ts('Email On Hold'), 'options' => CRM_Core_PseudoConstant::emailOnHoldOptions()));
}
else {
->add('advcheckbox', 'email_on_hold', ts('Email On Hold')
|
test this please |
1 similar comment
|
test this please |
|
Jenkins re test this please |
|
test this please |
|
This looks fine to me and is well covered by the tests adding merge on pass |
|
thanks @seamuslee001 - is it also your understanding that there is still an outstanding issue in the regression q even with this merged relating to changes to this field? |
|
unrelated fail |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Adds an upgrade routine for smart groups affected by the change of field type for on_hold emails
where civimail_multiple_bulk_emails is enabled
Before
Smart groups need to be manually resaved
To replicate create a smart group for contacts with email on hold with the setting civimail_multiple_bulk_emails enabled on 5.8 codebase, then switch to 5.11 & create one - note the format differences ie email_on_hold is a string in the 5.8 group & an array in the 5.11 group
After
Upgrading to 5.11 updates the groups
Run the upgrade - both groups should be the same
Technical Details
For sites with civimail_multiple_bulk_emails set we get a select box which needs an array rather than a checkbox, the saved groups need to reflect that.
Comments
Note from my testing there is problem loading the defaults on an existing saved search due to the field name having a discrepancy
wrong - this aligns groups created before & after the 5.9 upgrade but does not resolve that.
@twomice ping
https://lab.civicrm.org/dev/core/issues/731