Split ThreadFollower into separate models for posts and threads
Potentially irreversible change
This code change must not be merged without running the migration, because the code refers to a table that does not yet exist. The movement of data makes this change irreversible (at least for Rails' automatic rollback), and the fact that we use MySQL means that if the migration fails part way through it will be left in an intermediate state (for example, with a table created but no data moved). Separate up and down methods have been added to the migration, designed to handle rerunning or undoing the migration after an only partial success. However, please ensure backups are up to date before running this migration.
The ThreadFollower model was created to keep track of users following comment threads. Later it had a post_id field added to keep track of users following new threads on posts. This pull request splits out this later addition into a separate model NewThreadFollower.
Following discussion in Discord the migration file now covers all of:
- Create the
new_thread_followerstable . - Move all rows containing a
post_idfromthread_followersto the newnew_thread_followerstable. - Remove the now unused
post_idcolumn from thethread_followerstable.
I'm making this change so that #1548 can add a large number of rows (one per post) to the new_thread_followers table, rather than adding to the current combined table thread_followers. Waiting until after that large number of rows is added before splitting the table would require a much larger data movement between tables later (unless it is decided to never split the table, which I'm uncomfortable with since there's no reason for it to serve two independent purposes).
I'm not experienced in migrations so if there are obvious things that don't need to be mentioned in review, please mention them anyway.
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 80.42%. Comparing base (44f1d22) to head (6d8e9a2).
Additional details and impacted files
| Components | Coverage Δ | |
|---|---|---|
| controllers | 74.69% <100.00%> (+0.03%) |
:arrow_up: |
| helpers | 85.39% <ø> (+0.48%) |
:arrow_up: |
| jobs | 79.24% <ø> (ø) |
|
| models | 92.91% <100.00%> (+2.90%) |
:arrow_up: |
| tasks | 61.11% <ø> (ø) |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
I tested down and up after the latest changes and it seemed to work, but also, my dev environment got a little messy from the earlier changes, so somebody else should provide final approval.
LGTM but want @ArtOfCode- to review. Also, hold merge until after we deploy; we have some other small things ready to ship, so let's get those out and then merge this just in case there are any issues that would require rollback. (I don't think there are, but this isn't user-facing so it doesn't need to be deployed right away.)
I'm avoiding merging
I'm waiting until potential problems in the develop branch are resolved / ruled out:
https://discord.com/channels/634104110131445811/668890340199235613/1452154459223228518