Refactor the Pulse/Alert/DashboardSubscription tables & model code

[camsaul]

Background

Originally we only had normal Pulses, with a model diagram that looked something like this (uninterested columns omitted):

A Pulse can have zero or more Cards, and a Card can be in zero or more Pulses; this relationship is represented by the PulseCard table. A Pulse has one or more PulseChannels that represent Slack channels, Users, or email addresses the Pulse can be sent to. PulseChannelRecipient is a 1tM Table for User<->PulseChannel.

Pretty reasonable. Later on, we added Alerts, which were a special subtype of Pulse. The models then looked something like this:

(note that Alert and "NormalPulse" are not separate tables in the application DB; they're subtypes of Pulse, and the pulse table has all of the listed columns.)

The main differences between a "NormalPulse" and an "Alert" are that

1. Alerts have exactly 1 card, whereas NormalPulses can have any number of Cards
2. Alerts have several special columns such as alert_condition to decide whether or not the Alert should be sent, and if the Alert should be deleted after the first time it is triggered.

At this point, things were still pretty reasonable, altho in hindsight it may have been better to make Alert a separate table from the get go. (Or maybe even better would have been to use table inheritance, if MySQL supported it.)

Things got a little more complicated when we added Collections -- both a Pulse and its Cards belong to Collections, altho not neccesarily the same ones. Still, manageable and not too hard to work with.

Current State of the World

I think things got out of control when we added DashboardSubscriptions, which are another subtype of Pulse, this time tied to a specific Dashboard. Dashboards themselves are have zero or more Cards, represented by DashboardCard.

For DashboardSubscriptions we create a Pulse and associated PulseCards that have to be programatically kept in sync with the corresponding Dashboard and DashboardCards. The model diagram now looks something like:

(actually this is a little simpler than reality since it excludes "NormalPulses" which are deprecated and scheduled for removal, although not yet removed).

Major Issues

Because the models are unnecessarily complicated it makes working on things and implementing new features much harder than it needs to be and has lead to numerous bugs; we've spent a significant part of the 0.41 cycle working on tasks indirectly caused by the current architecture (#17658, #17537, #17531, #17436, and lots more)

• There are multiple redundant collection_id FKs floating around. The two remaining subtypes of Pulses, DashboardSubscriptions and Alerts, are intrinsically tied to a specific Dashboards and Cards, respectively. Both Dashboards and Cards belong to a specific Collection. Since Pulse itself also belongs to a Collection we have a situation where these can get out of sync. For DashboardSubscriptions we manually keep the collection_ids in sync, but Alerts can currently exist in different Collections from the Card they're associated with.
• PulseCard card_id presents a similar issue since it has both a direct card_id and an indirect card_id via DashboardCard. PulseCard also has multiple paths toDashboard (Pulse.dashboard_id and DashboardCard.dashboard_id)
• Several columns are completely redundant depending on the Pulse type; for example PulseCard position is not needed for Alerts since there's only one Card.
• The graph for an Alert is much too complicated for what we're representing (Card -> Pulse -> PulseCard -> Card as opposed Card -> Alert)

Proposal

• Split out DashboardSubscription and Alert into their own Tables. Remove the PulseCard table entirely.
• Alerts belong directly to a Cards without the Pulse or PulseCard in between and combines the columns we need from these Tables. We can drop several unneeded columns such as PulseCard position and Pulse name.
• For DashboardSubscriptions the only thing we really use PulseCard for is recording whether individual DashboardCards should be attached as XLSX/CSV files or not; this is something we can store in a JSON blob instead. DashboardSubscription can drop several unused columns from Pulse, e.g. collection_id and name
• Represent 1tM relationships for Alert<->SubscriptionChannel with new AlertChannel table,
• DashboardSubscription can only have one channel, so we can have a subscription_channel_id as FK on DashboardSubcription

Proposed model diagram:

[noahmoss]

Makes sense to me. Maybe it would be smart to bundle this in with (normal) pulse deprecation, since we'll be converting existing pulses to dashboard subscriptions. We could just fill in dashboard_id for existing pulse rows, then rename the entire pulse table & model.

[camsaul]

@flamber this is just a proposal, not sure it warrants a release notes mention? We wouldn't do it as part of this cycle at any rate

[flamber]

@camsaul I'm just adding the label, so if the issue is closed with a milestone, then we can remember to include a note. Since a change like this would likely break some peoples scripts, but also possible make it impossible to downgrade without reverting to backup.

Related to #13834

[camsaul]

@flamber RE making downgrade impossible I was thinking we could copy everything over to the new Tables but leave the old Pulse tables in place, unused, for a release cycle. Stuff you created after the upgrade wouldn't show up, but things wouldn't be totally broken if you downgraded that way.

[camsaul]

So DashboardSubscriptions can actually only have a single channel currently (not enforced on the schema but the UI will only show a single channel) so maybe DashboardSubscription can have an FK directly to a SubscriptionChannel or something like that

[qnkhuat]

So DashboardSubscriptions can actually only have a single channel currently (not enforced on the schema but the UI will only show a single channel) so maybe DashboardSubscription can have an FK directly to a SubscriptionChannel or something like that

yes

I've updated the diagram in the description to reflect that