fail permission check if we fail to fetch community root
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
Feed All Stories
All Stories
All Stories
Jul 8 2024
Jul 8 2024
atul updated the diff for D12543: [keyserver] Use `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck` instead of `threadFrozenDueToBlock` in `checkThreadsFrozen`.
atul updated the diff for D12543: [keyserver] Use `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck` instead of `threadFrozenDueToBlock` in `checkThreadsFrozen`.
partially address feedback (add comment explaining nullish coalescing w threadID)
ginsu committed rCOMM4176f25c2bba: [landing/lib/native/web] update copy for thread notifications for the rest of… (authored by ginsu).
[landing/lib/native/web] update copy for thread notifications for the rest of…
ginsu committed rCOMM1d4adf7c55f0: [native] fix empty media gallery in thread settings (authored by ginsu).
[native] fix empty media gallery in thread settings
ginsu retitled D12684: [landing/lib/native/web] update copy for thread notifications for the rest of the app from [landing] update copy for thread notifications on landing to [landing/lib/native/web] update copy for thread notifications for the rest of the app.
ginsu updated the diff for D12684: [landing/lib/native/web] update copy for thread notifications for the rest of the app.
rebase before landing
rebase before landing
[lib] Permissions for thick threads
ashoat accepted D12684: [landing/lib/native/web] update copy for thread notifications for the rest of the app.
Looks like threadSettingsNotificationsCopy has already been updated
Don't we now need to consider NO_FID_METADATA in all places we fetch the FID? It seems like it would need to be checked at every place the FID is fetched.
ashoat added inline comments to D12695: Make client handlers that call authenticated identity RPC's gated on CSAT.
Similar to D12676, I'd encourage reviewing this one carefully
If the network issue gets triggered by the keyserver connection status cycling, then we'll want to add some condition to eg. make sure it only runs once per app start / foreground event. If that's the case, please re-request review. Otherwise it looks good to land!
Worried that once !shouldCheckCurrentFIDRef.current, we'll be executing lines 171 and on, even after our FID is already set locally
ashoat requested changes to D12543: [keyserver] Use `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck` instead of `threadFrozenDueToBlock` in `checkThreadsFrozen`.
Solution to community root issue looks good! Passing back to figure out what to do if communityMembersToRole isn't present. Might be worth also considering a scenario where it's present, but there is no m.id key
We should be really careful in reviewing this diff. It seems complex, and notif storage in C++ has historically been an area where we've often introduced very serious issues
ginsu added 1 blocking reviewer(s) for D12684: [landing/lib/native/web] update copy for thread notifications for the rest of the app: ashoat.
Making @ashoat blocking since this is a copy change diff
shouldSkipPushPermissionAlert should probably be updated - if I understand correctly this diff makes it impossible for totalAlerts to exceed 1, is that correct?
ashoat requested review of D12697: [lib][native] Introduce isLoggedInToIdentityAndAuthoritativeKeyserver.
ashoat requested review of D12698: [native] Have BackgroundIdentityLoginHandler wait until LogInHandler.
marcin requested review of D12695: Make client handlers that call authenticated identity RPC's gated on CSAT.
ashoat committed rCOMM713982ce488d: [native][web] Stop clearing user input on unknown error (authored by ashoat).
[native][web] Stop clearing user input on unknown error
Rename onUnknownErrorAlertAcknowledged to onOtherErrorAlertAcknowledged
ashoat committed rCOMMbd12ab53cebf: [lib] Don't query keyserver for empty string from useThreadListSearch (authored by ashoat).
[lib] Don't query keyserver for empty string from useThreadListSearch
ashoat added a reverting change for D12674: [lib] Fix missing usernames: rCOMMdeb4a0ed9d3e: [lib] Fix crash when pressing message from anonymous author.
ashoat committed rCOMMdeb4a0ed9d3e: [lib] Fix crash when pressing message from anonymous author (authored by ashoat).
[lib] Fix crash when pressing message from anonymous author
ashoat committed rCOMM7c82a5b2ea9a: [native] Limit BackgroundIdentityLoginHandler to one try per startup (authored by ashoat).
[native] Limit BackgroundIdentityLoginHandler to one try per startup
ashoat committed rCOMM1419a4bd0741: [native] Don't bother with keyserver auth in BackgroundIdentityLoginHandler (authored by ashoat).
[native] Don't bother with keyserver auth in BackgroundIdentityLoginHandler
ashoat added a comment to D12679: [native] Limit BackgroundIdentityLoginHandler to one try per startup.
In D12679#359225, @tomek wrote:one try per startup
Why do we want to do this only once? Is there a downside to setting loginAttemptedRef.current to false after the login is done?
ashoat added a comment to D12678: [native] Don't bother with keyserver auth in BackgroundIdentityLoginHandler.
In D12678#359222, @tomek wrote:we're already auth'd with the authoritative keyserver
Why is it guaranteed?
Harbormaster failed remote builds in B30196: Diff 42108 for D12650: Update extractKeyserverIDFromID so that it returns null for DMs!
tomek added inline comments to D12650: Update extractKeyserverIDFromID so that it returns null for DMs.
Squash
Extract a function
will requested review of D12691: [terraform] include REDIS_URL as part of keyserver terraform config.
will requested review of D12688: [terraform] make aws rds instance class configurable for self-hosting.
shouldSkipPushPermissionAlert should probably be updated - if I understand correctly this diff makes it impossible for totalAlerts to exceed 1, is that correct?
set to false initially
Thank you for fixing this!
one try per startup
Why do we want to do this only once? Is there a downside to setting loginAttemptedRef.current to false after the login is done?
we're already auth'd with the authoritative keyserver
Why is it guaranteed?
ginsu added inline comments to D12569: [lib/web] introduce handleCurrentUserFID to FarcasterDataHandler.
When do we want handleCurrentUserFID to run?
ginsu updated the summary of D12569: [lib/web] introduce handleCurrentUserFID to FarcasterDataHandler.
atul updated the diff for D12543: [keyserver] Use `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck` instead of `threadFrozenDueToBlock` in `checkThreadsFrozen`.
update communityMembersToRole within for loop to fallback to threadID if threadInfo.community is null (which means it's a community root itself).
atul updated the diff for D12543: [keyserver] Use `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck` instead of `threadFrozenDueToBlock` in `checkThreadsFrozen`.
construct communityRootMembersToRole with both threadInfos and communityThreadInfos to handle scenario described here: https://phab.comm.dev/D12543?id=41737#inline-73658 where community root is one of the threadIDs passed into checkThreadsFrozen.
ginsu added inline comments to D12684: [landing/lib/native/web] update copy for thread notifications for the rest of the app.
ginsu updated the diff for D12569: [lib/web] introduce handleCurrentUserFID to FarcasterDataHandler.
update
Jul 7 2024
Jul 7 2024
ginsu committed rCOMM335064c59cf1: [native] update copy for thread notification settings (authored by ginsu).
[native] update copy for thread notification settings
rebase before landing
atul updated the diff for D12543: [keyserver] Use `threadIsWithBlockedUserOnlyWithoutAdminRoleCheck` instead of `threadFrozenDueToBlock` in `checkThreadsFrozen`.
address simple communityRootThread -> communityRootThreadID rename feedback
address comments
We ended up going with a very different approach from this stack. Abandoning to tidy things up.
atul abandoned D11311: [lib] Separate `serverThreadFrozenDueToBlock` and `clientThreadFrozenDueToBlock`.
We ended up going with a very different approach from this stack. Abandoning to tidy things up.
atul abandoned D11317: [lib] Update `clientThreadFrozenDueToBlock` and `getMinimallyEncodedCurrentUser` signatures to include `communityThreadInfo`.
We ended up going with a very different approach from this stack. Abandoning to tidy things up.
atul abandoned D11318: [lib] Introduce `threadInfoFromRawThreadInfo` wrapper that computes `currentUser` and passes to `baseThreadInfoFromRawThreadInfo`.
We ended up going with a very different approach from this stack. Abandoning to tidy things up.
We ended up going with a very different approach from this stack. Abandoning to tidy things up.
Jul 6 2024
Jul 6 2024
ashoat updated the summary of D12681: [lib] Don't query keyserver for empty string from useThreadListSearch.
ashoat requested review of D12681: [lib] Don't query keyserver for empty string from useThreadListSearch.