Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: DAH-2565 Create Account Errors #2285

Merged
merged 12 commits into from
Sep 12, 2024
Merged

Conversation

chadbrokaw
Copy link
Collaborator

@chadbrokaw chadbrokaw commented Sep 6, 2024

Description

This change introduces error handling to the Create Account page and also improves how we handle errors across the create account and account settings pages.

Jira ticket

DAH-2565

Checklist before requesting review

Version Control

  • branch name begins with angular if it contains updates to Angular code
  • branch name contains the Jira ticket number
  • PR name follows type: TICKET-NUMBER Description format, e.g. feat: DAH-123 New Feature

Code quality

Review instructions

  • instructions specify which environment(s) it applies to
  • instructions have already been performed at least once
  • instructions can be followed by PA testers
  • instructions specify if it can only be followed by an engineer

Request review

  • PR has needs review label
  • Use Housing Eng group to automatically assign reviewers, and/or assign specific engineers
  • If time sensitive, notify engineers in Slack

@chadbrokaw chadbrokaw force-pushed the DAH-2565_create_acct_errors branch from adbf0b1 to ef3c33a Compare September 6, 2024 21:37
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 6, 2024 21:37 Inactive
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 6, 2024 21:52 Inactive
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 6, 2024 21:54 Inactive
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 6, 2024 22:00 Inactive
@chadbrokaw chadbrokaw force-pushed the DAH-2565_create_acct_errors branch from e337400 to 0349ff2 Compare September 9, 2024 13:23
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 9, 2024 13:23 Inactive
@chadbrokaw chadbrokaw changed the title Dah 2565 create acct errors feat: DAH-2565 Create Account Errors Sep 9, 2024
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 9, 2024 16:05 Inactive
@chadbrokaw chadbrokaw requested review from a team, cliu02 and tallulahkay and removed request for a team September 9, 2024 16:18
@chadbrokaw chadbrokaw marked this pull request as ready for review September 9, 2024 16:18
) {
if (error?.response?.data?.errors?.firstName) {
// In the case that the name contains invalid characters (http, www, .), we will show the generic server error
if (error.response.data?.errors?.full_messages.includes("Firstname is empty")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the complexity is coming from a lot of duplicated logic, so the logic can instead be reused for each error type

As just an example, could instead be something like:

export const handleNameServerErrors = (
  key: string,
  error: AxiosError<{
    errors: {
      email: string[]
      password: string[]
      DOB: string[]
      full_messages: string[]
      firstName: string[]
      lastName: string[]
    }
  }>
) => {
  if (error?.response?.status === 422 && error?.response?.data?.errors?.full_messages.length > 0) {
    if (error?.response?.data?.errors?.[key]) {
      // In the case that the name contains invalid characters (http, www, .), we will show the generic server error
      return error.response.data?.errors?.full_messages.includes("is empty")
        ? { message: `name:${key}`, shouldFocus: true }
        : { message: "name:server:generic", shouldFocus: true }
    }

    return { message: "name:server:generic", shouldFocus: true }
  }
}

and then in account-settings.tsx

    if (error.response?.data?.errors?.firstName) {
      setError("firstName", handleNameServerErrors("firstName", error))
    } else if (error.response?.data?.errors?.lastName) {
      setError("lastName", handleNameServerErrors("lastName", error))
    }

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tallulahkay Thank you for that suggestion! I pushed an update to the handleNameServerErrors function. I pretty much used your solution but I kept setError within the function

@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 10, 2024 15:00 Inactive
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 10, 2024 15:06 Inactive
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 10, 2024 15:38 Inactive
@@ -96,7 +109,27 @@ const CreateAccountFooter = () => {
)
}

const onSubmit = (data) => {
const handleCreateAccountErrors =
(setError: (name: string, error: ErrorOption) => void) => (error: ExpandedAccountAxiosError) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 10, 2024 15:55 Inactive
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 10, 2024 19:00 Inactive
Copy link
Contributor

@tallulahkay tallulahkay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making those updates! Works well

@chadbrokaw chadbrokaw force-pushed the DAH-2565_create_acct_errors branch from c37ec89 to d77089c Compare September 10, 2024 20:14
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 10, 2024 20:14 Inactive
@chadbrokaw chadbrokaw force-pushed the DAH-2565_create_acct_errors branch from d77089c to d313740 Compare September 11, 2024 13:23
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 11, 2024 15:08 Inactive
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 11, 2024 16:16 Inactive
@chadbrokaw chadbrokaw force-pushed the DAH-2565_create_acct_errors branch from 01eb138 to 59dd51b Compare September 12, 2024 20:27
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 12, 2024 20:28 Inactive
@chadbrokaw chadbrokaw force-pushed the DAH-2565_create_acct_errors branch from 59dd51b to 6e73e1f Compare September 12, 2024 21:07
@chadbrokaw chadbrokaw temporarily deployed to dahlia-webapp-pr-2285 September 12, 2024 21:07 Inactive
@chadbrokaw chadbrokaw merged commit 475bdea into main Sep 12, 2024
18 checks passed
@chadbrokaw chadbrokaw deleted the DAH-2565_create_acct_errors branch September 12, 2024 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants