Page MenuHomePhabricator

[native] Update `updateRolesAndPermissions` to only handle `LegacyRawThreadInfos`
ClosedPublic

Authored by atul on Dec 10 2023, 4:11 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 1, 8:29 AM
Unknown Object (File)
Tue, Nov 26, 8:07 PM
Unknown Object (File)
Tue, Nov 26, 7:27 PM
Unknown Object (File)
Nov 23 2024, 4:34 AM
Unknown Object (File)
Nov 23 2024, 1:23 AM
Unknown Object (File)
Nov 6 2024, 1:54 PM
Unknown Object (File)
Nov 5 2024, 9:43 PM
Unknown Object (File)
Nov 5 2024, 9:33 PM
Subscribers
None

Details

Summary

Similar to what we did in D10283, we're "lying" to flow and saying that updatedRolesAndPermissions (which we also rename legacyUpdateRolesAndPermissions) accepts RawThreadInfos instead of just LegacyRawThreadInfos. However, we call assertAllThreadInfosAreLegacy right at the start of legacyUpdateRolesAndPermissions to ensure that only LegacyRawThreadInfos are passed to this function.

It could be argued that this is worse than D10283, because it's unlikely that we'd want to run the convertThreadStoreThreadInfosToNewIDSchema migration again, whereas it's VERY likely that we'll want to run updateRolesAndPermissions in the future. I will introduce a new version of updateRolesAndPermissions that'll handle MinimallyEncodedRawThreadInfos (by going through and converting all to LegacyRawThreadInfos which is what the logic in this migration requires since that's what eg getRolePermissionBlobs and makePermissionsBlob deal with. I'll put that up as a followup diff in this stack (probably before landing anything).

However, as of this diff we're now at 0 flow issues in native/web/keyserver/lib.


Depends on D10283

Test Plan

CI, flow, etc. I think in the future it would be nice to have unit tests for migrations.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul edited the summary of this revision. (Show Details)
atul published this revision for review.Dec 10 2023, 4:22 PM
atul added inline comments.
lib/shared/thread-utils.js
1761 ↗(On Diff #34464)

Actually this is bad, instead I will update legacyUpdateRolesAndPermissions to create a "writeable" copy.

lib/shared/thread-utils.js
1761 ↗(On Diff #34464)

This isn't bad if you accept my change in D10281, is it?

This revision is now accepted and ready to land.Dec 11 2023, 11:17 AM
This revision was landed with ongoing or failed builds.Dec 14 2023, 12:49 PM
This revision was automatically updated to reflect the committed changes.