Page MenuHomePhabricator

[lib] Introduce `decodeRolePermissionBitmask`
ClosedPublic

Authored by atul on Oct 31 2023, 3:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Oct 27, 7:13 AM
Unknown Object (File)
Tue, Oct 22, 1:07 PM
Unknown Object (File)
Tue, Oct 22, 1:07 PM
Unknown Object (File)
Tue, Oct 22, 1:07 PM
Unknown Object (File)
Tue, Oct 22, 1:07 PM
Unknown Object (File)
Tue, Oct 22, 1:07 PM
Unknown Object (File)
Tue, Oct 22, 1:07 PM
Unknown Object (File)
Tue, Oct 22, 1:07 PM
Subscribers

Details

Summary

To reverse rolePermissionToBitmaskHex introduced in D9662.


Depends on D9662

Test Plan

Unit tests, will add a few more.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul published this revision for review.Oct 31 2023, 3:58 PM

fix flow suppression label

Please make sure to address the last two comments before landing

lib/permissions/minimally-encoded-thread-permissions.js
115–129 ↗(On Diff #32573)

Seems like a bit of code duplication here... I wonder if it would be worth defining a generic inverseMap function in lib/utils/objects.js. Not sure how hard the Flow types would be

132 ↗(On Diff #32573)

Wondering – why'd you skip the & BigInt(63) part here?

142–147 ↗(On Diff #32573)

Some typos here, you're passing 4 params instead of 2

This revision is now accepted and ready to land.Nov 1 2023, 10:21 AM
lib/permissions/minimally-encoded-thread-permissions.js
142–147 ↗(On Diff #32573)

Dang, really should've caught this. Intended to copy/paste the first line and replace the , with &&.

lib/permissions/minimally-encoded-thread-permissions.js
132 ↗(On Diff #32573)

We don't really need to, but it doesn't hurt to add in case we're given malformed input (bitmask exceeds 12 bits).

d3964e.png (882×812 px, 147 KB)

When we shift right 4, we're "pushing" the propagation bits and filter bits off the edge.. so we're only left with the base bits.

However, when we shift right 2 to get propagation bits, and shift 0 to get filter bits (c and d in attached snippet)... we have "extraneous" more significant bits that we want to "mask away."

resolve merge conflicts

lib/permissions/minimally-encoded-thread-permissions.js
115–129 ↗(On Diff #32573)

Ran into some flow issues, made a linear task to unblock landing this diff: https://linear.app/comm/issue/ENG-5640/consider-introducing-invertobjecttomap-utility-fn

This revision was landed with ongoing or failed builds.Nov 1 2023, 7:12 PM
This revision was automatically updated to reflect the committed changes.