Details

Reviewers

Commits

Summary

ThreadCurrentUserInfo type w/ permissions as string instead of ThreadPermissions.

Next diffs:

Introduce MinimallyEncodedRawThreadInfo
Higher level utilities for translating back/forth from RawThreadInfo <=> MinimallyEncodedRawThreadInfo. Found this to be cleaner API than encoding/decoding RawThreadInfo.currentUser.permissions and RawThreadInfo.members[memberID].permissions "manually" a bunch of different places.
Native refactoring + migrations
Web refactoring
Flipping the switch

Depends on D9731

Test Plan

Unit tests, will be tested implicitly in subsequent diffs as well.

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D9734 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

atul published this revision for review.Nov 6 2023, 11:12 AM

Confused by the word boundaries... passing back but let me know if I'm missing something

lib/permissions/minimally-encoded-thread-permissions.js
207 ↗	(On Diff #32822)	Why the word boundaries? Is `{1,16}` really what we want? If we add more permissions in the future, it seems like older clients might reject the longer permission strings. Wouldn't it be better to just use `+`?

This revision now requires changes to proceed.Nov 6 2023, 11:41 AM

fix regex boundary and add failing test

update regex from {1,16} to +

lib/permissions/minimally-encoded-thread-permissions.js
207 ↗	(On Diff #32822)	Was experimenting w/ `\b` when doing the "multiple 12-bit chunks in one bitmask" approach and failed to update the boundaries Intention was that things should start breaking when the 65th permission is added to `baseRolePermissionEncoding` to surface to developer that there's an issue... making this validator break may not be the best way to achieve that. Updated this diff to address feedback.