Page MenuHomePhabricator

[keyserver] Add special_role column in migration 52
ClosedPublic

Authored by rohan on Dec 1 2023, 5:47 AM.
Tags
None
Referenced Files
Unknown Object (File)
Apr 8 2024, 10:35 PM
Unknown Object (File)
Apr 8 2024, 6:03 PM
Unknown Object (File)
Apr 8 2024, 6:03 PM
Unknown Object (File)
Apr 8 2024, 6:03 PM
Unknown Object (File)
Apr 8 2024, 6:03 PM
Unknown Object (File)
Feb 22 2024, 5:54 PM
Unknown Object (File)
Feb 22 2024, 4:58 PM
Unknown Object (File)
Feb 22 2024, 3:16 PM
Subscribers

Details

Summary

For those who incrementally went from migration 50 --> 52 --> 56 (or any number after 52), all of the migrations should have succeeded since the updateRolesAndPermissionsForAllThreads migration would have been run on queries that used threads.default_role, not roles.special_role.

However, following some new code in this stack, we updated the queries to use the special_role field.

For those who did not update their migration version incrementally and attempted to go from an older version like 50 straight to 56, the updateRolesAndPermissionsForAllThreads would fail because the queries attempt to fetch a roles.special_role columm, but that doesn't yet exist in their database state.

Here, we prefix migration 52 with the ALTER TABLE statement so we can add in a special_role column before the updateRolesAndPermissionsForAllThreads so the user's DB state has it before attempting to run any queries with it

Addresses ENG-5999

Test Plan

Ran the following queries to 'reset' what my DB would have looked like from migration 50:

ALTER TABLE threads ADD COLUMN default_role bigint(20) NOT NULL;
ALTER TABLE invite_links DROP COLUMN blob_holder;
ALTER TABLE roles DROP COLUMN special_role;

Following that, attempting to start up keyserver resulted in the same error as ENG-5999.

(node:90468) DB version: 50
(node:90468) migration 51 succeeded.
recalculating permissions for threads with depth 0
updating roles for 1
updating roles for 83880
updating roles for 83894
updating roles for 83903
updating roles for 83912
updating roles for 83961
(node:90468) migration 52 failed.
Unknown column 'special_role' in 'field list'
[nodemon] app crashed - waiting for file changes before starting...

After making these changes, I attempted to start up keyserver again and saw the migrations succeeded

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan edited the test plan for this revision. (Show Details)

Update migration #52 instead. Repeated the test plan to make sure this works for me

keyserver/src/database/migration-config.js
694–696 ↗(On Diff #34106)

I kept this here in case someone is on migration version #53 and tries to upgrade, as they won't have the special_role field added from migration #52

rohan retitled this revision from [keyserver] Change migration order to prevent them failing to [keyserver] Add special_role column in migration 52.Dec 1 2023, 5:59 AM
rohan edited the summary of this revision. (Show Details)
rohan published this revision for review.Dec 1 2023, 6:34 AM

Requesting review in case there's anything I need to address since CI is taking some time.

Won't land until it passes though

This revision is now accepted and ready to land.Dec 1 2023, 6:48 AM