343 add validation to user signup form #363

marvinsjsu · 2023-08-02T19:08:44Z

Dev Summary

This adds validation to the fields in the User Sign Up form. It also refactors the UI into multiple step components and updates the display of errors to be under the field instead of under the label. This also adds redux for state management for user entity, and redux-thunk for sending the POST request to the backend to create the user account.

The work was starting to have scope-creep, so created some follow-up tickets:

Test Plan

To test, go to the User Signup form via http://localhost:3000/signup-citizen.

try to go through the form by entering invalid or missing values and see if proper error messages are displayed
verify that when all fields are filled with valid information, the user reaches the email verification step

Note:
During account creation on my local, I was getting an error regarding SendGrid -

web-app/server/src/acccount-manager/account-manager.controller.ts

Line 105 in cc54d36

await this.sendgridService.send(mail);

... I don't think I have a valid SENDGRID_API_KEY in my .env.

React-query tutorial: https://www.youtube.com/watch?v=r8Dg0KVnfMA

marvinsjsu · 2023-08-02T19:38:27Z

client/src/components/Users/Auth/SignUpCitizen/StepOne.tsx

+
+  Object.keys(initialFormData).forEach((key) => {
+    //@ts-ignore
+    initialFormData[key].value = initData[key];


I've used // @ts-ignore on multiple lines as an escape hatch as I'm not sure how to get this to work with TypeScript. If you guys have any ideas on how I could/should do this, that'd be awesome.

EtoKruto · 2023-08-03T07:25:09Z

Regarding Passwords in the Video:

See video - I noticed that going back to the first password may still allow proceeding. Though this might be a minor correction since it's not typical user behavior, I thought it might be worth looking into. Nice work, looks clean

Screen.Recording.2023-08-03.at.12.06.56.AM.mov

EtoKruto · 2023-08-03T07:26:34Z

On Titles and Styles(all pages):
I observed that the purple titles on each page don't exactly match the styles, but I like the direction you've taken with this, looks to be an improvement on the mock UI so I say keep it

About State/City Screen (Page 2):
The State/City screen in step 2 doesn't quite align with the mock, but I actually prefer it this way. Would make sense to add some validation with the Zip Code input but this could be a later add-on

EtoKruto · 2023-08-03T07:28:32Z

Concerning the Upload Button Overlay:
I spotted an overlay issue with the upload button on the profile photo. I know there's another story for this HERE, so it may be out of scope, but I thought it worth mentioning

EtoKruto · 2023-08-03T07:28:53Z

Console Log Clean-Up:
Once ready, clean up any console.log entries

EtoKruto · 2023-08-03T07:31:31Z

Error with 'Bio' Column in UsersService:
I encountered an error related to the 'bio' column of the 'users' relation when clicking next on from the profile icon/about me page.

ERROR [UsersService] column "bio" of relation "users" does not exist: 
QueryFailedError: column "bio" of relation "users" does not exist
    at PostgresQueryRunner.query

In addition this also overlayed the words "Loading" over the back button (see screenshot)

Since I'm not as familiar with the backend, I feel it might be beneficial to have someone more familiar with the backend take a look at this part of the project

Overall Great job, really like what you did here and there is a lot of good work here!

esteban-gs

@marvinsjsu , thanks for working on this. If we want to use redux going forward, we'll probably need to update other forms and remove other libraries (probably for another PR).

esteban-gs · 2023-08-04T21:06:58Z

client/package.json

@@ -9,6 +9,7 @@
 		"@mui/base": "^5.0.0-beta.2",
 		"@mui/icons-material": "^5.11.16",
 		"@mui/material": "^5.13.2",
+		"@reduxjs/toolkit": "^1.9.5",


@marvinsjsu , do we want to settle on redux without any other form library? If so, we'll also want to add some cards to remove react-query and react-hooks-form. There are a few forms (signupnonprofit) using it. I like Redux' separation of concerns, but one downside is all the boilerplate.

Either way, I'm down with it.

client/src/components/Users/Auth/SignUpCitizen/StepOne.tsx

…e last step, StepFive component

marvinsjsu · 2023-09-06T21:45:18Z

hi @esteban-gs and @EtoKruto, I've updated this PR based on the feedback, but I'm going to work on the styling in the other ticket I have - https://github.com/orgs/Nonprofit-Exchange-Hub/projects/14?pane=issue&itemId=33617416 (since this PR is getting kinda big and the other ticket is more for styling changes anyways). I think it's ready for another look. Thanks guys!

…ack/react-query-devtools to dev dep

esteban-gs

@marvinsjsu , thank you for tackling this! I think we are getting close to being ready. I left a couple of comments and one blocking comment about the last step not working as intended.

esteban-gs · 2023-09-08T05:35:46Z

client/src/components/Users/Auth/SignUpCitizen/StepFour.tsx

+          <Button type="submit" color="primary" variant="outlined">
+            Next
+          </Button>


Step 4 data (bio) isn't getting merged with the rest of the data. Maybe you need to call the handleNext parent method as well.

esteban-gs · 2023-09-08T05:38:39Z

client/src/components/Users/Auth/SignUpCitizen/StepOne.tsx

+  const { status, data } = useQuery({
+    queryKey: ['user-email-exists', formData['email'].value],
+    queryFn: () => Endpoints.checkUserEmail(formData['email'].value),
+    enabled: userEmailQueryEnabled,
+  });


I think we should leave this query out of this iteration until we define this further. What do you think?

esteban-gs · 2023-09-08T05:39:31Z

client/src/components/Users/Auth/SignUpCitizen/StepThree.tsx

+
+  const makeChips = () => {
+    return interests.map((interest) => {
+      // TODO: toggle chip style when interest is chosen


Suggested change

// TODO: toggle chip style when interest is chosen

esteban-gs · 2023-09-08T05:41:32Z

server/src/acccount-manager/account-manager.controller.ts

+  @Get('user-email-exists/:email')
+  @ApiOperation({ summary: 'Check if a user email already exists.' })
+  async userEmailExists(@Param('email') email: string) {
+    try {
+      const user = await this.usersService.findByEmail(email);
+      return Boolean(user);
+    } catch (error) {
+      return false;
+    }
+  }
+


My suggestion is to leave this new public endpoint out of this PR until we solve this issue further.

Suggested change

@Get('user-email-exists/:email')

@ApiOperation({ summary: 'Check if a user email already exists.' })

async userEmailExists(@Param('email') email: string) {

try {

const user = await this.usersService.findByEmail(email);

return Boolean(user);

} catch (error) {

return false;

}

}

…rop types

marvinsjsu · 2023-09-10T01:53:26Z

@esteban, thanks for catching the bug with bio. This should be fixed now and I've also removed the email check. This should be ready for another look.

esteban-gs

There is one linting rule exception comment that's tripping up the build. LGTM after that's addressed 🚢

esteban-gs · 2023-09-10T04:31:11Z

client/src/components/Users/Auth/SignUpCitizen/SignUpCitizen.tsx

+    // eslint-disable-next-line prettier/prettier
+    doSubmit
+      ? setSubmitForm(true)
+      : setActiveStep((prevActiveStep) => prevActiveStep + 1);


prettier doesn't like this in-line warning disabling comment. See the build errors.

You should be able to just format it in a single line to get rid of the lint warning.

Suggested change

// eslint-disable-next-line prettier/prettier

doSubmit

? setSubmitForm(true)

: setActiveStep((prevActiveStep) => prevActiveStep + 1);

doSubmit ? setSubmitForm(true) : setActiveStep((prevActiveStep) => prevActiveStep + 1);

garimacha · 2023-09-11T21:05:57Z

@marvinsjsu This PR is under review but its not under the In progress vertical. Shouldn't this be moved under in progress?

marvinsjsu added 13 commits July 28, 2023 13:24

refactor: create StepOne component

474de00

refactor: EmailInput and NameInput

712baec

finish validation for step one form

6d361ad

refactor: add StepTwo component

d43420a

refactor: add StepThree, StepFour, and StepFive components

a9c5711

set initData on form

4c823cd

add style for selected chip

b690218

add styling for buttons

24db3ed

add passwordConfirm to type

4c1d380

add redux-thunk for user signup

6030afd

un-comment sendgrid send

3fe366a

add StyledLink import on StepOne

cffac22

fix bug for StepTwo next button disabled

d268616

marvinsjsu mentioned this pull request Aug 2, 2023

Finish User Sign Up Form #343

Closed

marvinsjsu requested review from EtoKruto, taoliu12 and denniswangcodes August 2, 2023 19:35

marvinsjsu commented Aug 2, 2023

View reviewed changes

esteban-gs requested changes Aug 4, 2023

View reviewed changes

Marvin Mante and others added 6 commits August 25, 2023 15:53

Merge branch 'main' into 343-add-validation-to-user-signup-form

17b0141

bugfix: always apply password match validation

c467a66

refactor: simplify user actions

3550260

refactor: add utils for validation helpers

78c348a

cleanup: remove console.logs

816c42a

refactor: feature-based folder structure

d2ca68e

marvinsjsu added 6 commits September 6, 2023 13:19

removed Redux and use react-query for user registration

04607b8

display loading and error state placeholders

458a633

display the email, firstName, lastName or last_name of the user in th…

df04b15

…e last step, StepFive component

cleanup: remove console.log

cd2cdfb

cleanup: remove unused folder/file backend.ts

e5fddb7

cleanup: re-arrange props

13d7021

add validation for existing email

d694b4b

This was referenced Sep 7, 2023

Add profile img upload on Sign Up form #364

Closed

Form validation demo #372

Closed

marvinsjsu added Back End Front End labels Sep 7, 2023

marvinsjsu self-assigned this Sep 7, 2023

marvinsjsu marked this pull request as ready for review September 7, 2023 17:30

marvinsjsu added 2 commits September 7, 2023 10:43

remove react-query package and use @tanstack/react-query, move @tanst…

16f976b

…ack/react-query-devtools to dev dep

remove @tanstack/react-query references

672cec4

marvinsjsu linked an issue Sep 8, 2023 that may be closed by this pull request

Finish User Sign Up Form #343

Closed

esteban-gs requested changes Sep 8, 2023

View reviewed changes

marvinsjsu added 5 commits September 9, 2023 12:49

removed call to check email existence from StepOne

e00261c

removed endpoint for checking email existence

3c7ed6c

bugfix: ensure bio is included in formData on form submission

3884285

refactor: use type instead of interface for functional approach for p…

c29fa03

…rop types

refactor: use type instead of interface for prop types

f3335d1

esteban-gs approved these changes Sep 10, 2023

View reviewed changes

marvinsjsu added 2 commits September 10, 2023 07:00

fix linting issue

f4d02e6

un-comment sending email verification

8330910

marvinsjsu merged commit 427becc into main Sep 11, 2023
3 checks passed

marvinsjsu deleted the 343-add-validation-to-user-signup-form branch September 11, 2023 00:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

343 add validation to user signup form #363

343 add validation to user signup form #363

marvinsjsu commented Aug 2, 2023 •

edited

Loading

marvinsjsu Aug 2, 2023

EtoKruto commented Aug 3, 2023

EtoKruto commented Aug 3, 2023 •

edited

Loading

EtoKruto commented Aug 3, 2023

EtoKruto commented Aug 3, 2023

EtoKruto commented Aug 3, 2023

esteban-gs left a comment

esteban-gs Aug 4, 2023

marvinsjsu commented Sep 6, 2023

esteban-gs left a comment

esteban-gs Sep 8, 2023

esteban-gs Sep 8, 2023

esteban-gs Sep 8, 2023

esteban-gs Sep 8, 2023

marvinsjsu commented Sep 10, 2023 •

edited

Loading

esteban-gs left a comment

esteban-gs Sep 10, 2023

garimacha commented Sep 11, 2023

343 add validation to user signup form #363

343 add validation to user signup form #363

Conversation

marvinsjsu commented Aug 2, 2023 • edited Loading

Dev Summary

Test Plan

marvinsjsu Aug 2, 2023

Choose a reason for hiding this comment

EtoKruto commented Aug 3, 2023

EtoKruto commented Aug 3, 2023 • edited Loading

EtoKruto commented Aug 3, 2023

EtoKruto commented Aug 3, 2023

EtoKruto commented Aug 3, 2023

esteban-gs left a comment

Choose a reason for hiding this comment

esteban-gs Aug 4, 2023

Choose a reason for hiding this comment

marvinsjsu commented Sep 6, 2023

esteban-gs left a comment

Choose a reason for hiding this comment

esteban-gs Sep 8, 2023

Choose a reason for hiding this comment

esteban-gs Sep 8, 2023

Choose a reason for hiding this comment

esteban-gs Sep 8, 2023

Choose a reason for hiding this comment

esteban-gs Sep 8, 2023

Choose a reason for hiding this comment

marvinsjsu commented Sep 10, 2023 • edited Loading

esteban-gs left a comment

Choose a reason for hiding this comment

esteban-gs Sep 10, 2023

Choose a reason for hiding this comment

garimacha commented Sep 11, 2023

marvinsjsu commented Aug 2, 2023 •

edited

Loading

EtoKruto commented Aug 3, 2023 •

edited

Loading

marvinsjsu commented Sep 10, 2023 •

edited

Loading