[keyserver] Update usage of threads.default_role in thread-creator.js
ClosedPublic
Actions

Authored by rohan on Nov 24 2023, 9:58 AM.

Details

Reviewers

atul
ginsu
tomek

Commits

rCOMM6f6bf02f683d: [keyserver] Update usage of threads.default_role in thread-creator.js

Summary

We need to get rid of usages of threads.default_role throughout the codebase so we can eventually drop the column.

This diff handles the usage in thread-creator.js

Existing code: When creating a thread, the roles from createInitialRolesForNewThread are used to populate the role ID into threads.default_role for that specific row. We should no longer populate this field.

Depends on D9980

Addresses part of ENG-5833

Test Plan

Ran ALTER TABLE threads MODIFY COLUMN default_role bigint(20) NULL; to allow NULL values in the column for now (until the column is dropped)
Created a new thread and checked to make sure that no role got inserted into default_role

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rohan created this revision.Nov 24 2023, 9:58 AM

Herald added a subscriber: ashoat. · View Herald TranscriptNov 24 2023, 9:58 AM

rohan edited the summary of this revision. (Show Details)Nov 24 2023, 10:01 AM

rohan edited the test plan for this revision. (Show Details)Nov 24 2023, 10:04 AM

rohan added a child revision: D9982: [keyserver] Drop the default_role column from threads.Nov 24 2023, 10:34 AM

Harbormaster completed remote builds in B24408: Diff 33607.Nov 24 2023, 11:08 AM

rohan requested review of this revision.Nov 24 2023, 11:08 AM

Is the default_role column nullable at this point?

This revision is now accepted and ready to land.Nov 27 2023, 3:19 AM

In D9981#291960, @tomek wrote:

Is the default_role column nullable at this point?

This query would attempt to insert a NULL value into default_role, but without running ALTER TABLE threads MODIFY COLUMN default_role bigint(20) NULL, you'd see an error because it still expects a non-null value.

I felt like adding a migration to allow it to be nullable wouldn't be very useful since the very next diff drops the column entirely anyways

Closed by commit rCOMM6f6bf02f683d: [keyserver] Update usage of threads.default_role in thread-creator.js (authored by rohan). · Explain WhyNov 27 2023, 10:36 AM

This revision was automatically updated to reflect the committed changes.

rohan added a commit: rCOMM6f6bf02f683d: [keyserver] Update usage of threads.default_role in thread-creator.js.