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

Restore civicrm_action_schedule.limit_to (5.50) #23497

Merged
merged 7 commits into from
May 19, 2022

Conversation

totten
Copy link
Member

@totten totten commented May 19, 2022

Overview

(Updated variant of @monishdeb's #23478. Equivalent to #23490 on 5.49-stable.)

Restore the schema for field civicrm_action_schedule.limit_to to allow NULL values. Attempt to reinstate NULL values (where possible). Warn about any records which may have changed.

See also:

Before

  • limit_to is not nullable
  • limit_to cannot be saved with null
  • The upgrade (5.48.2 ==> 5.49.1) will munge data; it coerces from limit_to=null to 0.

After

  • limit_to is nullable (as in <=5.48)
  • limit_to can be saved with null
  • Upgrade behavior varies for these perspectives:
    • If you upgrade directly from 5.48.2 ==> 5.49.2, it properly preserves data.
    • If you upgrade indirectly from 5.48.2 ==> 5.49.1 ==> 5.49.2, then the first upgrade already munged data. The second upgrade remediates by:
      • Cleaning up some records (ie anything with limit_to=0 AND group_id=NULL AND recipient_manual=NULL)
      • Showing a message to highlight records that could be inaccurate. (Some records may still have limit_to=0 even though their 5.48

Comment

This does NOT accommodate current/recent RC testers. Anyone who has deployed 5.50-rc on would need to re-run the upgrade, starting from 5.49-stable (or earlier). (Mattermost discussion)

monishdeb and others added 7 commits May 18, 2022 19:20
I'm not sure what it even means to limit a scheduled-reminder to a "soft_credit_type".

But it's a thing - I can use the 5.48 GUI to create it. Here's how it looks in the DB (in 5.48):

+----+------------------------------------------+------------------+----------+------------------+-------------------+----------+
| id | title                                    | recipient        | limit_to | recipient_manual | recipient_listing | group_id |
+----+------------------------------------------+------------------+----------+------------------+-------------------+----------+
| 11 | Limit To - Soft Credit                   | soft_credit_type |        1 | NULL             | in_memory_of      |     NULL |
+----+------------------------------------------+------------------+----------+------------------+-------------------+----------+

The `changeBooleanColumnLimitTo()` was coercing down to `limit_to=null` because it lacked `recipient_manual` and `group_id`.
1. This fixes a problem where the message shows at the opposite-of-correct time
   (ie it should display on builds that have already run 5.49.{beta,0,1}, but
   it actually displayed on builds that have not run 5.49.{beta,0,1})

2. This makes the pre+post messages match.
@civibot
Copy link

civibot bot commented May 19, 2022

(Standard links)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants