Page MenuHomePhabricator

[lib] add identity password log in action
ClosedPublic

Authored by varun on Jan 25 2024, 11:01 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 7, 1:26 AM
Unknown Object (File)
Fri, Oct 18, 3:36 PM
Unknown Object (File)
Fri, Oct 18, 3:36 PM
Unknown Object (File)
Fri, Oct 18, 3:36 PM
Unknown Object (File)
Fri, Oct 18, 3:36 PM
Unknown Object (File)
Fri, Oct 18, 3:36 PM
Unknown Object (File)
Fri, Oct 18, 6:55 AM
Unknown Object (File)
Mon, Oct 14, 10:53 PM
Subscribers
None

Details

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

I'm not going to touch tempIdentityLoginActionTypes for now

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

ashoat requested changes to this revision.Jan 25 2024, 5:55 PM

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
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

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

This revision is now accepted and ready to land.Feb 7 2024, 10:08 AM
This revision was automatically updated to reflect the committed changes.