Page MenuHomePhabricator

[lib] Update existing checks on isDefault in thread-utils.js
ClosedPublic

Authored by atul on Nov 30 2023, 1:20 PM.
Tags
None
Referenced Files
Unknown Object (File)
Apr 8 2024, 11:20 PM
Unknown Object (File)
Apr 8 2024, 11:20 PM
Unknown Object (File)
Apr 8 2024, 11:20 PM
Unknown Object (File)
Apr 8 2024, 11:20 PM
Unknown Object (File)
Apr 8 2024, 11:20 PM
Unknown Object (File)
Apr 8 2024, 11:20 PM
Unknown Object (File)
Mar 5 2024, 3:24 PM
Unknown Object (File)
Mar 5 2024, 2:00 AM
Subscribers

Details

Summary

We now want to make sure that we get rid of checks on role.isDefault and replace them with checks on role.specialRole:

  • For default roles, this is `role.specialRole === ${specialRoles.DEFAULT_ROLE}
  • For admin roles, this is `role.specialRole === ${specialRoles.ADMIN_ROLE}

Introducing a new utility function roleIsDefaultRole and updating roleIsAdminRole in this diff so we no longer have such hacky checks (and so I can update other callsites of role.isDefault with roleIsDefaultRole)

Part of ENG-5993

Depends on D10115

Test Plan
  • Confirmed that callsites of roleIsAdminRole had no difference in behavior (i.e. the old and new roleIsAdminRole returned the same value)
  • Confirmed that roleIsDefaultRole was correct by doing a logical read through the code and manually calling the function myself to test the return value on roles

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan held this revision as a draft.
rohan published this revision for review.Nov 30 2023, 1:34 PM
lib/shared/thread-utils.js
1865 ↗(On Diff #34075)

Is it used in later diffs?

lib/shared/thread-utils.js
1865 ↗(On Diff #34075)

Yeah it's also used in D10117, D10118, and D10119

This revision is now accepted and ready to land.Dec 4 2023, 12:27 AM
atul edited reviewers, added: rohan; removed: atul.