Paths

Table of Contentst

[lib] Add `invariant` to suppress `flow` issue in `getPushUserInfo`
AbandonedPublic
Actions

Authored by atul on Jul 10 2024, 2:42 PM.

Details

Reviewers

ashoat
marcin

Summary

There are new client-side member permission checks in getPushUserInfo which I believe (and hope...) are only relevant for ThickRawThreadInfos.

getPushUserInfo gets called by preparePushNotifs gets called by usePreparePushNotifs which isn't used anywhere at this point.

Putting in invariants instead of $FlowIgnore so that it's harder to miss if any ThinRawThreadInfos make it to this point... which should be an error case because ThinRawThreadInfos will not have any permissions.

Fixing things here is tracked in https://linear.app/comm/issue/ENG-8809/modify-functions-in-send-utilsjs-in-to-operate-only-on

Depends on D12597

Test Plan

flow

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Jul 10 2024, 2:42 PM

Herald added a subscriber: tomek. · View Herald TranscriptJul 10 2024, 2:42 PM

atul published this revision for review.Jul 10 2024, 2:44 PM

Harbormaster failed remote builds in B30273: Diff 42214!Jul 10 2024, 2:55 PM

Gonna leave this one for @marcin to confirm:

I believe (and hope...) are only relevant for ThickRawThreadInfos.

Also, there appear to be ESLint issues here that would need to be resolved prior to landing

This function is supposed to be used exclusively with thick threads.

This revision is now accepted and ready to land.Jul 12 2024, 2:09 AM

Requesting changes for ESLint

This revision now requires changes to proceed.Jul 12 2024, 7:22 AM

atul added a child revision: D12733: [lib] Deprecate `minimallyEncodeRawThreadInfo`.Jul 15 2024, 11:31 AM

address ESLint issues

Harbormaster failed remote builds in B30336: Diff 42298!Jul 15 2024, 11:58 AM

There is a Flow issue. Can you please either pass this diff back to me when it's fully green, or else explain why there need to be CI issues?

lib/shared/thread-utils.js
88 ↗	(On Diff #42298)	Why did you put this here? Can you please move this fix to where it belongs? Presumably you have an earlier diff in the stack that introduced this ESLint issue – it should be resolved there

This revision now requires changes to proceed.Jul 17 2024, 12:07 PM

Looks like the Flow issue is resolved in D12755, but still would like to see the ESLint fix moved to the diff where the issue is introduced. Let me know if I'm off-base or missing something

still would like to see the ESLint fix moved to the diff where the issue is introduced.

The fix has been moved to D12597.

rebase

Harbormaster failed remote builds in B30435: Diff 42411!Jul 17 2024, 2:29 PM

Looks like D12784 has been landed, so this can be abandoned after you rebase

This revision now requires changes to proceed.Jul 18 2024, 10:17 AM

Can drop this now, thanks @marcin !

Revision Contents
Changeset List

Path

Size

lib/

push/

send-utils.js

7 lines

Diff 42411

View Options

[lib] Add `invariant` to suppress `flow` issue in `getPushUserInfo`AbandonedPublicActions