Representing AuthComponent state using ADT

[dingyuchen]

Overview

Algebraic Data Types (ADTs) can be viewed as enum with data. Currently, application state in the AuthComponent is represented by a collection of booleans (isAuthenticating, isOutdatedVersion, etc) which does not appropriately and intuitively translate into the state of the application. State is determined by programmatically checking for some collection of booleans, (!isOutdatedVersion && !isUserAuthenticated) which may miss out edge cases, are hard to parse for new contributors, and susceptible to regression bugs.

Proposal

ADTs can be accomplished in typescript 2.0 with interfaces and a "disciminant". as an exmaple, we can have

type State = Initial | Auth | Current
type Auth = Authenticating | Authenticated | NotAuthenticated | Error
type Current = UpToDate | Outdated | Error

Implementation detail can be found here as a reference.

Notes

Start after #723 is merged.

[anubh-v]

For the booleans used to track version-related state, I suggest either leaving them unmodified or removing them entirely. We could simply use notification pop-ups (as shown below) to communicate version-related errors, which removes the need for these boolean variables.

[AudreyFelicio]

@dingyuchen @anubh-v Can I try to take on this issue after #723 is merged?

[dingyuchen]

For the booleans used to track version-related state, I suggest either leaving them unmodified or removing them entirely. We could simply use notification pop-ups (as shown below) to communicate version-related errors, which removes the need for these boolean variables.

Would you suggest that we block the "Continue as {{username}}" button if the app version is out of date? If that's so we would still need some flag to keep track of whether an error has occurred. Saw your comment in the PR

@dingyuchen @anubh-v Can I try to take on this issue after #723 is merged?

Feel free to assign yourself, but I think you should hold off development until the scope and requirements of the PR has been finalized.

[anubh-v]

@dingyuchen we've removed many of the booleans used to track version-related state in #723 Let us know how this issue should proceed

[dingyuchen]

I think the application state is a lot simpler to reason now and I think there is no longer any need for this issue. I'll close this issue but feel free to reopen if anyone has any suggestions