Account creation (issue #250) #293

JStein92 · 2018-09-20T16:06:51Z

This PR is a:

[x] New feature
[ ] Enhancement/Optimization
[ ] Refactor
[ ] Bugfix
[ ] Test for existing code
[ ] Documentation

Summary

This pull request is an extension to PR #281 - login feature.

Addresses issue #250

When this pull request is merged, in addition to the changes of #281, it will enable users to create accounts through Magento's REST api.

New components

CreateAccount
Checkbox
Form

Additional information

User is automatically logged in after creating an account through getUserDetails() in the root index.js
signin_token expires after some time but stays in local storage, so it's possible to receive a 401 Unauthorized when the root index.js tries to get user details with an expired token. Automatic token refresh should be addressed in the future.
Guest cart is merged into newly created account, but the carts will not show up because getCartDetails still only looks for guest cart ids. Changing this seemed out of scope for this issue so we left it as is.
Style changes for Input component to better match Invision designs.
Automatic email availability check with debounce using Magento isEmailAvailable endpoint.

… into login

- Style form to match mockup - Fix misimport of user reducer

- Rename log in to sign in to match mockups - Style error messages in sign in form

… account

- Transferring cart to a newly created customer account created an issue where a guest cart could not be made. getCartDetails now checks if a user is signed in.

…ith redux

zetlen

This is an excellent addition and I'm excited to see it merged. I have a few comments, a couple suggestions, and a couple necessary changes. Please let me know what you think.

It also may require a significant back-merge, but that's just where we are at right now. I'll try to conduct the merge for you--since you have lots of tests, it should be easy to detect regressions.

zetlen · 2018-09-26T04:09:56Z

packages/venia-concept/package.json

@@ -7,7 +7,7 @@
    "main": "src/index.js",
    "repository": "github:magento-research/pwa-studio",
    "bugs": {
-      "url": "https://github.com/magento-research/pwa-studio/issues"
+        "url": "https://github.com/magento-research/pwa-studio/issues"


Are your editors doing four-space indentation on package.json files? If so, our prettier should be catching this. Whoopsie!

Our editors are set to follow the editor config - not sure why it wasn't caught in the prettier. Thanks!

zetlen · 2018-09-26T04:12:02Z

packages/venia-concept/src/actions/cart.js

+        const [dispatch, getState] = args;
+        const { user } = getState();
+        if (user.isSignedIn) {
+            ///////////////////////////////////////////


This is a good default behavior for now, but could you please add a "TODO: handle logged-in cart retrieval" to this comment? The comment is already helpful, but the string TODO is searchable and helps us find tech debt in the future.

zetlen · 2018-09-26T14:53:31Z

packages/venia-concept/src/actions/user.js

@@ -0,0 +1,121 @@
+import { RestApi } from '@magento/peregrine';
+import BrowserPersistence from 'src/util/simplePersistence.js';


Since you're using localStorage in Peregrine and the BrowserPersistence abstraction here (that later might use Cache or IndexedDB or ServiceWorker or whatever), maybe it's time to move BrowserPersistence to Peregrine and pull it from there!

Feel like you could do that quickly with this PR, or should we save that for an upcoming PR?

Done! We're exporting it the same way as RestAPI is exported from Peregrine. 👍

zetlen · 2018-09-26T14:55:43Z

packages/venia-concept/src/actions/user.js

+    };
+
+function setToken(token) {
+    localStorage.setItem('signin_token', token);


Since you're using a BrowserPersistence instance above, maybe use it here.

zetlen · 2018-09-26T14:58:56Z

packages/venia-concept/src/components/CreateAccount/createAccount.js

+        return this.state.password !== this.state.passwordConfirm;
+    }
+
+    get disableAccountCreation() {


I did a double take on these method names. hasEmailError is great, because the wording of the method name says it's a pure method that checks state and returns a boolean. But passwordConfirmError doesn't follow that convention; seems like it should be called hasPasswordConfirmError.

Same with disableAccountCreation. It might be better called isIncompleteOrInvalid.

zetlen · 2018-09-26T15:11:14Z

packages/venia-concept/src/components/SignIn/signIn.js

+                    <Input
+                        onChange={this.updateUsername}
+                        helpText={'example help text'}
+                        label={'Username or Email'}


Suggestion: If the user has filled out this field and then clicks "Create Account" instead, it should copy this value into the CreateAccount counterpart field.

zetlen · 2018-09-26T15:11:39Z

packages/venia-concept/src/index.js

+});
+
+const authLink = setContext((_, { headers }) => {
+    // get the authentication token from local storage if it exists


Here's another place where you would want to use a BrowserPersistence instance.

zetlen · 2018-09-26T15:11:58Z

packages/venia-concept/src/index.js


-const apolloClient = new ApolloClient();
+const apolloClient = new ApolloClient({
+    link: authLink.concat(httpLink),


Excellent job using Apollo Client exactly as intended.

zetlen · 2018-09-26T15:12:59Z

packages/venia-concept/src/reducers/mockAccount.json

@@ -0,0 +1,73 @@
+{


Please move this and all other mocks/fixtures into a tests subfolder like fixtures. Add to the Jest exclude pattern if you have to.

zetlen · 2018-09-26T15:13:30Z

packages/venia-concept/src/reducers/mockAccount.json

+    "created_at": "2018-09-13T18:26:34.662Z",
+    "updated_at": "2018-09-13T18:26:34.662Z",
+    "created_in": "string",
+    "dob": "1/1/2010",


Your customer created their account when they were eight years old. Adorable!

…orage

…on/pwa-studio into account_creation

… into account_creation

JStein92 · 2018-10-03T17:45:25Z

@zetlen Ok, this pull request has been updated to meet the release/2.0 standards. : ) Also now includes tests (just basic coverage) for user actions/asyncActions.

That was quite a merge!

zetlen · 2018-10-03T19:11:32Z

It sure was, @JStein92 . It sure was. Thank you so, so much. I guess this closes #250 now!

JStein92 and others added 30 commits September 7, 2018 07:28

create generic input component

174ddd0

ignore storybook-dist in eslint

25dcc9d

create login component, action and reducer catch

ac482dc

Hook login form to utilize and store token

3ef5bb1

Add navigation animation and user icon

2d9e7f7

add auth to REST requests

3561f71

Merge branch 'login' of https://github.com/bargreenellingson/pwa-studio…

9e3ab0c

… into login

Add user icon

5673948

Merge branch 'login' of https://github.com/bargreenellingson/pwa-studio…

614bd1d

… into login

add auth header login_token to graphql queries

2b0d825

Change animation, add user to store

455b7d3

Move user logic into separate action/reducer

ece2eec

Add token to auth headers when token is available (rest)

9ad855d

Style sign in form, fix user reducer

ec87f4f

- Style form to match mockup - Fix misimport of user reducer

Rename log in -> sign in. Wire up sign in form.

03bea9b

- Rename log in to sign in to match mockups - Style error messages in sign in form

Change login to signin

491dfdc

Add proptypes, clean up css

f109d81

Prettier pass

293c523

write test for signIn and input, small style and prop fixes

fcfe28e

Merge branch 'master' into login

aa88455

lint and prettier pass

4fc83b7

reword a c ouple tests

28a8e51

fix dependency issue and clean up navigation component

240fc98

Add redux actions to create account and transfer guest cart to user's…

710eb4d

… account

add form, navheader and create account components, update styles

9d4e52e

Refactor createAccount, handle transferring cart

2d0d31b

- Transferring cart to a newly created customer account created an issue where a guest cart could not be made. getCartDetails now checks if a user is signed in.

Merge

6835654

add debounce email check

6f2d9ae

merge branches

5c70a74

add error display and checkbox components, wire up account creation w…

35dd2be

…ith redux

zetlen added the 1.x release label Sep 25, 2018

zetlen suggested changes Sep 26, 2018

View reviewed changes

zetlen changed the base branch from release/1.0 to release/2.0 September 26, 2018 21:05

zetlen added the enhancement New feature or request label Sep 27, 2018

JStein92 and others added 20 commits September 28, 2018 11:14

use browser persistence instead of interacting directly with local st…

d859a10

…orage

add TODO and fix indents

63c6420

Rename accountCreation variables

8828635

add autocomplete to input

8e1669d

Remove mock account

1710e58

set default username and replace Form component with Informed's Form

fcd4a25

Merge branch 'account_creation' of https://github.com/bargreenellings…

9e1c58c

…on/pwa-studio into account_creation

start switch to Text component from Informed

16a22e2

switch to dot lookup in navigation states

6f65e6f

Set initial value of CreateAccount (inform)

b1d9b2d

prettier pass and some test fixes

b309e7e

Merge / Prettier / Lint

7586c42

Merge branch 'master' of https://github.com/magento-research/pwa-studio…

81e92c0

… into account_creation

merge test fixes

c43b7c9

start merge with 2.0 release

d28700d

remove debugger

83b9f45

modify user actions/reducers to be in release 2.0 format

970f749

Resolve conflicts, prettier/lint

a32f2e1

get tests running for cart actions

0947576

write tests for async User actions

7b83f45

zetlen approved these changes Oct 3, 2018

View reviewed changes

zetlen merged commit 2aebb5e into magento:release/2.0 Oct 3, 2018

pcvonz mentioned this pull request Oct 18, 2018

Implement basic login for Venia (Issue #249) #281

Closed

sharkySharks mentioned this pull request Jun 21, 2023

[Snyk] Security upgrade execa from 1.0.0 to 2.1.0 sharkySharks/pwa-studio#74

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Account creation (issue #250) #293

Account creation (issue #250) #293

JStein92 commented Sep 20, 2018 •

edited

Loading

zetlen left a comment

zetlen Sep 26, 2018

JStein92 Sep 28, 2018

zetlen Sep 26, 2018

zetlen Sep 26, 2018

JStein92 Sep 28, 2018

zetlen Sep 26, 2018

zetlen Sep 26, 2018

zetlen Sep 26, 2018

zetlen Sep 26, 2018

zetlen Sep 26, 2018

zetlen Sep 26, 2018

zetlen Sep 26, 2018

JStein92 commented Oct 3, 2018

zetlen commented Oct 3, 2018

		@@ -0,0 +1,121 @@
		import { RestApi } from '@magento/peregrine';
		import BrowserPersistence from 'src/util/simplePersistence.js';

Account creation (issue #250) #293

Account creation (issue #250) #293

Conversation

JStein92 commented Sep 20, 2018 • edited Loading

This PR is a:

Summary

New components

Additional information

zetlen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JStein92 commented Oct 3, 2018

zetlen commented Oct 3, 2018

JStein92 commented Sep 20, 2018 •

edited

Loading