WIP Update user registration flow #168

pdotsani · 2020-11-21T16:58:39Z

What type of PR is this? (check all applicable)

Context

This PR closes issue #161

Screenshots/Recordings (if there are UI changes)

Placeholder example: ![screenshot of new resource page](https://user-images.githubusercontent.com/43/23.png)

Note: you can upload a screenshot onto your PR and put it here.

Implementation Details - what was your thought process as you changed the code?

Update the POST request when a user registers to hit POST /api/v1/auth/registration/
Remove the first name and last name fields
Change the login endpoint to be POST /api/v1/auth/login/
Change the logout endpoint to be POST api/v1/auth/logout
Create a api/v1/auth/registration/verify-email?key={KEY} page that upon load, makes a POST request to /verify-email and passes in the key from the params in the URL

Related Tickets & Documents (Optional)

This relates to #161

Added tests?

👍 yes (updated tests)
🙅 no, because they aren't needed
🙋 no, because I need help

Added to documentation (readme.md or contributing.md)?

📜 readme.md
📜 contributing.md
🙅 no documentation needed
🙋 I'd like someone to help write documentation, and will file a new issue for it

stale · 2020-12-21T17:49:17Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2021-01-28T02:48:12Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

BethanyG · 2021-01-28T18:15:02Z

@lpatmo @pdotsani -- is this still a WIP? Should we be removing the stale tag?

pdotsani · 2021-01-28T18:22:12Z

Yupp. Still WIP. We're still trying to find a time to get together to finish it.

lpatmo · 2021-01-31T02:09:50Z

@pdotsani @peoray Thanks for working on this this morning! Re: the bugs that you ran into...

DoesNotExist at /api/v1/auth/login/ EmailAddress matching query does not exist. seems to be and api issue. I tried using preloaded user data generated by script.

I'm not sure that the preloaded user data generated by the script should work against that endpoint; you should be using a new user you registered.

Verification email is not being sent. This happens when I try to login with existing user credentials and when I try to create new ones.

This is because in dev, the registration email appears in the logs when you run docker-compose up (mailhog). You won't get a real email.

=========

This is the process that worked for me:

In your backend repo, run docker-compose up (without the -d flag so that you're not hiding the logs)
In SignupForm.js, change the POST URL to .post('http://localhost:8000/api/v1/auth/registration/', data). Make sure it's http:localhost:8000, not https.

(In the meantime, I'll look into figuring out how we can sidestep hardcoding this, so that it's automatically http://localhost:8000 if someone is running this locally and https://api-staging.codebuddies.org when this is deployed.)

On the frontend repo, run npm run start to start the app. Go to https://localhost:3000/signup and register a new user.

Make sure the password is something not too common; otherwise you'll get an error. fwiw, I used codebuddies as my fake password and that worked fine.

After you successfully register a new user, you'll be redirected to this page:

And the logs will look like this:

Next, you can log out. You'll see this screen:
When you try to log in, you'll see "Email is not verified":

If you remember the logs, they pointed to a URL that said:

http://localhost:8000/api/v1/auth/registration/verify-email/?key=MTI:1l624W:Wa-4hXDvgMRyRtfdy9C06oBR9rU

We can fix the hostname in the BE later (I believe there is a ticket for that on github.com/codebuddies/backend), but https://localhost:3000/api/v1/auth/registration/verify-email/?key={KEY} is the page we need to create on the FE side, and it needs to make a POST request to the http://localhost:8000/api/v1/auth/registration/verify-email/ endpoint.

For example, if I paste my key into my BE app and click on the POST button:

...that's a successful response. :)

Let me know if you need more screenshots/if any part of this doesn't make sense!

lpatmo · 2021-02-16T08:19:27Z

@pdotsani Thank you for updating the tests and the error messages! :) I think it's fine to comment out the lines in the test that aren't relevant anymore (e.g. the first and last name).

Looks like this is the main TODO that remains, since logout is taken care of via localStorage.clear('tokens');:

Create a api/v1/auth/registration/verify-email?key={KEY} page that upon load, makes a POST request to /verify-email and passes in the key from the params in the URL

Can work on making that route if you want help!

pdotsani · 2021-02-16T16:04:06Z

@pdotsani Thank you for updating the tests and the error messages! :) I think it's fine to comment out the lines in the test that aren't relevant anymore (e.g. the first and last name).

Looks like this is the main TODO that remains, since logout is taken care of via localStorage.clear('tokens');:

Create a api/v1/auth/registration/verify-email?key={KEY} page that upon load, makes a POST request to /verify-email and passes in the key from the params in the URL

Can work on making that route if you want help!

@lpatmo Ah ok. Didn't have enough time to get to this last week. Should be finishing up in the week or so.

lpatmo · 2021-02-17T03:51:08Z

@pdotsani Thank you!!

pdotsani · 2021-03-13T19:25:41Z

Thanks for the help @lpatmo. The new view and post request are working, and should be ready for review. The only additional change needed is that the url on the verification email needs to be updated to the client (localhost:3000 on local development).

src/components/Auth/VerifyEmail.js

src/components/Auth/SignUpForm.js

src/components/Auth/VerifyEmail.js

lpatmo · 2021-03-14T01:43:12Z

@pdotsani Sweet, thank you so much! I added a couple of suggestions re: things like redirecting the user to a "Verify your email" page after signing up and only showing the "Email verified" text if successful... let me know if I can clarify further or if you need any help!

src/components/Auth/SignUpForm.js

lpatmo · 2021-04-13T07:00:18Z

@pdotsani Thanks for pushing up your commits! Finally got around to reviewing. :) I pushed up 60f4594 which makes sure there is a dependency array in the useEffect; otherwise it caused some errors in the build. Also pushed up a commit 56b2602 so that users who are redirected to the verify-email page see a notice to check their email instead of only the error.

Otherwise, looks great! I'll make a note in the backend ticket that we need to replace localhost:8000 with localhost:3000 and api-staging.codebuddies.org.

…agination component when there is no search term, and moved it down on the page

Bumps [axios](https://github.com/axios/axios) from 0.19.0 to 0.21.1. **This update includes a security fix.** - [Release notes](https://github.com/axios/axios/releases) - [Changelog](https://github.com/axios/axios/blob/v0.21.1/CHANGELOG.md) - [Commits](axios/axios@v0.19.0...v0.21.1) Signed-off-by: dependabot-preview[bot] <[email protected]>

… host URL

Co-authored-by: Linda <[email protected]>

notswedish · 2021-04-13T16:48:25Z

src/pages/Resources/Resources.js

+            variant="outlined"
+            shape="rounded"
+            size="large"
+            count={Math.floor(count / 10)}


small nitpick: I would move computations like these out of the rendering of components and before the return statement

Good suggestion -- thanks @turretd!

notswedish · 2021-04-13T16:50:14Z

src/helpers/constants.js

+  API_URL: 'http://localhost:8000',
+};
+
+export const config = process.env.NODE_ENV === 'development' ? dev : prod;


really love this approach 🙌

pdotsani changed the title ~~Update user registration flow~~ WIP Update user registration flow Nov 21, 2020

stale bot added the stale label Dec 21, 2020

stale bot closed this Dec 28, 2020

lpatmo reopened this Dec 29, 2020

stale bot removed the stale label Dec 29, 2020

stale bot added the stale label Jan 28, 2021

stale bot removed the stale label Jan 28, 2021

pdotsani force-pushed the feature/update-registration-flow branch 4 times, most recently from cc6ded1 to df9fa10 Compare February 13, 2021 19:48

pdotsani marked this pull request as ready for review February 13, 2021 19:49

lpatmo requested changes Mar 14, 2021

View reviewed changes

src/components/Auth/VerifyEmail.js Outdated Show resolved Hide resolved

src/components/Auth/SignUpForm.js Show resolved Hide resolved

src/components/Auth/VerifyEmail.js Outdated Show resolved Hide resolved

lpatmo reviewed Mar 14, 2021

View reviewed changes

src/components/Auth/SignUpForm.js Outdated Show resolved Hide resolved

lpatmo approved these changes Apr 13, 2021

View reviewed changes

pdotsani and others added 4 commits April 13, 2021 00:08

Added pagination component with useQuery call connected

c00bc85

tested on mock data, working without useeffect

a51d55e

Pagination cleanup - ermoved Resources/Resources.js file, only show p…

da0d2a9

…agination component when there is no search term, and moved it down on the page

register and login api updated

446e669

pdotsani and others added 9 commits April 13, 2021 00:08

login and registration apis working now, error handling tmp patch

39bd218

Add constants file and differentiate between dev and prod for the API…

17a91b4

… host URL

fixed tests

6ac2361

verify email post request working

b43d75d

error validation on verify email screen

df55168

Update src/components/Auth/SignUpForm.js

e0c229d

Co-authored-by: Linda <[email protected]>

Add key to dependencies array for useEffect

bae0123

Added explanatory text re: verifying email

a637bbf

lpatmo force-pushed the feature/update-registration-flow branch from 56b2602 to a637bbf Compare April 13, 2021 07:09

notswedish reviewed Apr 13, 2021

View reviewed changes

notswedish approved these changes Apr 13, 2021

View reviewed changes

pdotsani force-pushed the feature/update-registration-flow branch from a637bbf to d29540f Compare April 15, 2021 00:05

lpatmo force-pushed the feature/update-registration-flow branch from d29540f to a637bbf Compare April 18, 2021 01:11

lpatmo merged commit 9112cc3 into main Apr 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP Update user registration flow #168

WIP Update user registration flow #168

pdotsani commented Nov 21, 2020 •

edited

Loading

stale bot commented Dec 21, 2020

stale bot commented Jan 28, 2021

BethanyG commented Jan 28, 2021

pdotsani commented Jan 28, 2021

lpatmo commented Jan 31, 2021 •

edited

Loading

lpatmo commented Feb 16, 2021

pdotsani commented Feb 16, 2021

lpatmo commented Feb 17, 2021

pdotsani commented Mar 13, 2021

lpatmo commented Mar 14, 2021

lpatmo commented Apr 13, 2021

notswedish Apr 13, 2021 •

edited

Loading

lpatmo Apr 18, 2021

notswedish Apr 13, 2021

WIP Update user registration flow #168

WIP Update user registration flow #168

Conversation

pdotsani commented Nov 21, 2020 • edited Loading

What type of PR is this? (check all applicable)

Context

Screenshots/Recordings (if there are UI changes)

Implementation Details - what was your thought process as you changed the code?

Related Tickets & Documents (Optional)

Added tests?

Added to documentation (readme.md or contributing.md)?

stale bot commented Dec 21, 2020

stale bot commented Jan 28, 2021

BethanyG commented Jan 28, 2021

pdotsani commented Jan 28, 2021

lpatmo commented Jan 31, 2021 • edited Loading

This is the process that worked for me:

lpatmo commented Feb 16, 2021

pdotsani commented Feb 16, 2021

lpatmo commented Feb 17, 2021

pdotsani commented Mar 13, 2021

lpatmo commented Mar 14, 2021

lpatmo commented Apr 13, 2021

notswedish Apr 13, 2021 • edited Loading

Choose a reason for hiding this comment

lpatmo Apr 18, 2021

Choose a reason for hiding this comment

notswedish Apr 13, 2021

Choose a reason for hiding this comment

pdotsani commented Nov 21, 2020 •

edited

Loading

lpatmo commented Jan 31, 2021 •

edited

Loading

notswedish Apr 13, 2021 •

edited

Loading