Page MenuHomePhabricator
Differential D12722

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

Authored by atul on Jul 10 2024, 2:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Sep 6, 1:57 PM
Unknown Object (File)
Mon, Sep 2, 5:57 AM
Unknown Object (File)
Fri, Aug 30, 8:51 PM
Unknown Object (File)
Thu, Aug 29, 8:56 AM
Unknown Object (File)
Wed, Aug 28, 1:11 PM
Unknown Object (File)
Wed, Aug 28, 3:28 AM
Unknown Object (File)
Wed, Aug 28, 3:27 AM
Unknown Object (File)
Wed, Aug 28, 3:27 AM
View All 46 Files
Subscribers
ashoat
tomek

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
ashoat resigned from this revision.Jul 11 2024, 5:16 PM

Gonna leave this one for @marcin to confirm:

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

ashoat added a subscriber: ashoat.Jul 11 2024, 5:18 PM

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

marcin accepted this revision.Jul 12 2024, 2:09 AM

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
ashoat requested changes to this revision.Jul 12 2024, 7:22 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
atul updated this revision to Diff 42298.Jul 15 2024, 11:45 AM

address ESLint issues

Harbormaster failed remote builds in B30336: Diff 42298!Jul 15 2024, 11:58 AM
ashoat requested changes to this revision.Jul 17 2024, 12:07 PM

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
ashoat added a comment.Jul 17 2024, 12:31 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

atul requested review of this revision.Jul 17 2024, 2:14 PM

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

The fix has been moved to D12597.

atul updated this revision to Diff 42411.Jul 17 2024, 2:17 PM

rebase

Harbormaster failed remote builds in B30435: Diff 42411!Jul 17 2024, 2:29 PM
ashoat requested changes to this revision.Jul 18 2024, 10:17 AM

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
atul abandoned this revision.Jul 18 2024, 12:39 PM

Can drop this now, thanks @marcin !

Revision Contents
Changeset List

PathSize
lib/
push/
7 lines

Diff 42411

View Options

lib/push/send-utils.js

Loading...