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)
Wed, Feb 19, 12:31 PM
Unknown Object (File)
Wed, Feb 19, 9:30 AM
Unknown Object (File)
Wed, Jan 29, 5:47 PM
Unknown Object (File)
Jan 20 2025, 12:20 PM
Unknown Object (File)
Jan 20 2025, 12:20 PM
Unknown Object (File)
Jan 20 2025, 12:11 PM
Unknown Object (File)
Jan 20 2025, 11:36 AM
Unknown Object (File)
Dec 31 2024, 4:26 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!