Page MenuHomePhabricator

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

Authored by rohan on Nov 24 2023, 9:53 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Oct 27, 9:55 AM
Unknown Object (File)
Oct 2 2024, 4:13 AM
Unknown Object (File)
Oct 2 2024, 4:13 AM
Unknown Object (File)
Oct 2 2024, 4:12 AM
Unknown Object (File)
Oct 2 2024, 4:09 AM
Unknown Object (File)
Aug 28 2024, 11:54 AM
Unknown Object (File)
Jul 30 2024, 7:01 PM
Unknown Object (File)
Jul 30 2024, 7:01 PM
Subscribers

Details

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-updaters.js

Depends on D9976

Addresses part of ENG-5833

Test Plan

Tested both use cases of threads.default_role separately in this file:

  • Attempts to fetch the role permissions for the default role in the thread (expected return result size is one row)
    • Ran the query before and after in my MariaDB on my local database and saw the same results printed out
  • Fetches the user, role and default_role from memberships to determine whether the user is a 'default role user'
    • Ran the query before and after in my MariaDB on my local database and saw the same results printed out

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

keyserver/src/updaters/thread-updaters.js
211 ↗(On Diff #33603)

Makes me wonder if we should update the thread index on roles to be a thread, special_role index

tomek added inline comments.
keyserver/src/updaters/thread-updaters.js
211 ↗(On Diff #33603)

Makes sense, but shouldn't matter too much, as we probably don't expect more than a couple of roles per thread

This revision is now accepted and ready to land.Nov 27 2023, 1:53 AM
keyserver/src/updaters/thread-updaters.js
211 ↗(On Diff #33603)

Addressed in D9961!