[native] Introduce `StaffContextProvider`
ClosedPublic
Actions

Authored by atul on Oct 31 2022, 2:00 PM.

Details

Reviewers

tomek
ginsu
rohan
ashoat
marcin

Commits

rCOMM22ac9645d0eb: [native] Introduce `StaffContextProvider`

Summary

Context: https://linear.app/comm/issue/ENG-2134/have-a-flag-that-indicates-whether-the-build-is-a-staff-release

We want to maintain a staffUserHasBeenLoggedIn value so we know if a staff member is currently, or has previously logged in.

This provider will set staffUserHasBeenLoggedIn to true, but won't do the reverse. Once it's been set to true it will remain true indefinitely.

Test Plan

Log value of provider
Log into non-staff account
Ensure that staffUserHasBeenLoggedIn is false
Log into a staff account
Ensure that staffUserHasBeenLoggedIn is true
Log back into a non-staff account
Ensure that staffUserHasBeenLoggedIn is still true

Diff Detail

Repository

rCOMM Comm

Branch

master

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Oct 31 2022, 2:00 PM

Herald added a subscriber: • abosh. · View Herald TranscriptOct 31 2022, 2:00 PM

Harbormaster completed remote builds in B13115: Diff 18004.Oct 31 2022, 2:14 PM

atul requested review of this revision.Oct 31 2022, 2:14 PM

Added @ashoat as blocking since he has context, feel free to resign if you don't think it makes sense to be first-pass reviewer

Add StaffContextProvider to root.react.js

No point in making this a separate diff

atul edited the test plan for this revision. (Show Details)Oct 31 2022, 2:30 PM

Harbormaster completed remote builds in B13116: Diff 18005.Oct 31 2022, 2:38 PM

Was thinking about this more, and I think it would useful if the "button" in LoggedOutModal actually showed an Alert when pressed (similar thing could be done in "Build info") that explained eg.:

Advanced logging enabled
Since a staff user was logged in earlier, advanced logging is still enabled. To disable, kill the app and reopen.

OR:

Advanced logging enabled
Since this is a debug build, advanced logged is enabled.

OR:

Advanced logging enabled
Since this is a beta build, advanced logged is enabled.

In order to figure out which message we should display, we'd want to know which of the scenarios apply. So perhaps we should expand the context here to include a bit more info?

Accepting since it's not a hard requirement and could be handled in a later diff.

This revision is now accepted and ready to land.Nov 1 2022, 5:24 AM

For the first message, if the user popped up the Alert from a logged-in state (BuildInfo) we'd actually want to say:

Advanced logging enabled
Since a staff user is logged in, advanced logging is enabled. To disable, log out and then kill the app and reopen.

ashoat mentioned this in D5514: [native] Add `staffUserHasBeenLoggedInIndicator` to `LoggedInModal`.Nov 1 2022, 5:28 AM

In D5513#163029, @ashoat wrote:

Was thinking about this more, and I think it would useful if the "button" in LoggedOutModal actually showed an Alert when pressed (similar thing could be done in "Build info") that explained eg.:

Advanced logging enabled
Since a staff user was logged in earlier, advanced logging is still enabled. To disable, kill the app and reopen.

OR:

Advanced logging enabled
Since this is a debug build, advanced logged is enabled.

OR:

Advanced logging enabled
Since this is a beta build, advanced logged is enabled.

In order to figure out which message we should display, we'd want to know which of the scenarios apply. So perhaps we should expand the context here to include a bit more info?

Accepting since it's not a hard requirement and could be handled in a later diff.