[lib] Add `siweAuthActionTypes.success` check everywhere there's a `logInActionTypes.success` check
ClosedPublic
Actions

Authored by atul on Dec 29 2022, 1:24 AM.

Details

Reviewers

ashoat
tomek

Commits

rCOMM50274a5fdc6d: [lib] Add `siweAuthActionTypes.success` check everywhere there's a…

Summary

We want to make sure that we're handling siweAuthActionTypes.success everywhere there's a logInActionTypes.success check to ensure we're matching the existing login behavior.

Depends on D6085

Test Plan

Will be tested implicitly by subsequent diffs. Won't land until login/registration working end to end.

Known issue: SIWE registration leads to client in correct state, subsequent SIWE login has missing threadInfos. Currently investing.

Diff Detail

Repository

rCOMM Comm

Branch

master

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Dec 29 2022, 1:24 AM

Harbormaster completed remote builds in B14881: Diff 20296.Dec 29 2022, 1:37 AM

atul requested review of this revision.Dec 29 2022, 1:37 AM

Sounds reasonable. But now I'm wondering if introducing new action was a good idea. Maybe dispatching the old action (possibly with a new field e.g. type) might be easier? I don't think we should change it now, though.

This revision is now accepted and ready to land.Dec 29 2022, 4:27 AM

ashoat accepted this revision.Dec 29 2022, 9:25 AM

In D6086#182693, @tomek wrote:

Sounds reasonable. But now I'm wondering if introducing new action was a good idea. Maybe dispatching the old action (possibly with a new field e.g. type) might be easier? I don't think we should change it now, though.

My understanding is that there's usually a 1:1 relationship between endpoints and actions? There could definitely be "1 action to many endpoints" relationships in the codebase, but I didn't look too hard.

rebase before landing

Harbormaster completed remote builds in B14953: Diff 20399.Dec 29 2022, 2:28 PM

Closed by commit rCOMM50274a5fdc6d: [lib] Add `siweAuthActionTypes.success` check everywhere there's a… (authored by atul). · Explain WhyDec 29 2022, 3:36 PM

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM50274a5fdc6d: [lib] Add `siweAuthActionTypes.success` check everywhere there's a….

In D6086#183275, @atul wrote:

In D6086#182693, @tomek wrote:

Sounds reasonable. But now I'm wondering if introducing new action was a good idea. Maybe dispatching the old action (possibly with a new field e.g. type) might be easier? I don't think we should change it now, though.

My understanding is that there's usually a 1:1 relationship between endpoints and actions? There could definitely be "1 action to many endpoints" relationships in the codebase, but I didn't look too hard.

I think that there's nothing which stops introducing 1:N relationship, but it might make the code more fragile and less readable, so let's keep this solution.