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

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