[lib] add identity password log in action
ClosedPublic
Actions

Authored by varun on Jan 25 2024, 11:01 AM.

Details

Reviewers

ashoat
inka
tomek

Commits

rCOMM805c1b14fde7: [lib] add identity password log in action

Summary

adding a password log in action that will be dispatched in the next diff

Depends on D10816

Test Plan

will be tested in next diff by actually dispatching the action

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

varun created this revision.Jan 25 2024, 11:01 AM

I'm not going to touch tempIdentityLoginActionTypes for now

Harbormaster completed remote builds in B26160: Diff 36137.Jan 25 2024, 11:19 AM

varun requested review of this revision.Jan 25 2024, 11:19 AM

varun added a child revision: D10821: [web] add identity password login to traditional-login-form.react.js.Jan 25 2024, 12:07 PM

varun added inline comments.Jan 25 2024, 12:42 PM

lib/actions/user-actions.js
434–438 ↗	(On Diff #36137)	i'll rename this later to make it reusable for wallet login too. see this comment

In D10817#311772, @varun wrote:

I'm not going to touch tempIdentityLoginActionTypes for now

Why not?

lib/actions/user-actions.js
434–438 ↗	(On Diff #36137)	Passing back to you to make this change (or alternately just renaming `tempIdentityLoginActionTypes`?)
440 ↗	(On Diff #36137)	I actually think the noun form makes more sense here, and lines up with `tempIdentityLoginActionTypes`

This revision now requires changes to proceed.Jan 25 2024, 5:55 PM

varun added inline comments.Feb 1 2024, 10:07 PM

lib/actions/user-actions.js
434–438 ↗	(On Diff #36137)	renamed `tempIdentityLoginActionTypes`
440 ↗	(On Diff #36137)	we use the verb form for other hooks like `useIdentityRegister()`, `useClaimUsername()`, etc.
440 ↗	(On Diff #36137)	I'm going to keep the verb form from now but can change it if you feel strongly. just don't think it makes sense given all the other hooks use the verb form, including the legacy `useLogIn` hook

address feedback

Harbormaster completed remote builds in B26452: Diff 36570.Feb 1 2024, 10:32 PM

ashoat requested changes to this revision.Feb 2 2024, 7:34 AM

ashoat added inline comments.

lib/types/redux-types.js
345–360 ↗	(On Diff #36570)	I'm confused – why are these being deleted instead of renamed?

This revision now requires changes to proceed.Feb 2 2024, 7:34 AM

varun requested review of this revision.Feb 2 2024, 1:02 PM

varun added inline comments.

lib/types/redux-types.js
345–360 ↗	(On Diff #36570)	@inka removed all references to `tempIdentityLoginActionTypes` in D10868 in the next diff in the stack (D10821) i've added the new `BaseAction` variants along with where we first use them in `dispatchActionPromise`. i can add them here instead, if you prefer

varun planned changes to this revision.Feb 2 2024, 1:03 PM

varun added inline comments.

lib/types/redux-types.js
345–360 ↗	(On Diff #36570)	actually this will take 2 seconds, so i'll just do it now

Ah, missed that. The type names don't match, so you'll need to rebase anyways to fix them... at this point, might as well move the types to this diff. Since this where you define the constants, it's easier to compare and notice discrepancies

address feedback

Harbormaster completed remote builds in B26604: Diff 36761.Feb 6 2024, 9:18 PM

ashoat accepted this revision.Feb 7 2024, 10:08 AM

This revision is now accepted and ready to land.Feb 7 2024, 10:08 AM

Closed by commit rCOMM805c1b14fde7: [lib] add identity password log in action (authored by varun). · Explain WhyFeb 7 2024, 9:57 PM

This revision was automatically updated to reflect the committed changes.

varun added a commit: rCOMM805c1b14fde7: [lib] add identity password log in action.