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

[CHE-199] Create Tests and Error Handling for userController registerUser #156

Conversation

seantokuzo
Copy link
Contributor

Description

As part of the CHE-73/BE-Controller-Files story, this PR aims to:

  • Abstract out registerUser middleware into it's own file (in new server/controllers/userController/ directory)
  • Refactor registerUser to make use of new error handling
  • Add test coverage for registerUser

Other things included with the changes above:

  • Create reusable utility functions for validating email and password inputs
  • Create a reusable utility function for attaching auth cookies to responses
  • Simplify imports of utilities
  • update registerUser import in userRouter
  • Remove unused registerUser middleware from userController.ts
  • Remove registerUser tests that have been replaced

Jira Task

JIRA TICKET

Testing Instructions

  • run npm run docker-test:solo register to run new unit tests on registerUser middleware
  • manual review of server/controllers/userController/registerUser/registerUser.test.ts to make sure all possible request cases are covered

Checklist

All Team Members

  • I added a descriptive title to this PR.
  • I filled out the Description, Jira Task, and Testing Instructions sections above.
  • I added or updated [Jest unit tests]for any changes to components, server-side controllers, etc.
  • I ran npm run docker-build-check in my local environment to check that this PR passes all linting and unit tests.
  • I did a quick check to make sure my code changes follow the recommended style guide.

@brok3turtl3 brok3turtl3 added the enhancement New feature or request label Jun 28, 2024
Copy link
Contributor

@brok3turtl3 brok3turtl3 left a comment

Choose a reason for hiding this comment

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

@seantokuzo Great job on your first refactor and test ticket. 🎉

👍 I like the utility separation as well.

🛑 Left a couple comments and one request to delete the tests file. I think you can probably just go ahead and delete all the test files in that folder. Most of them wouldn't be recognizable after the reactors anyway.

__tests__/userController.tests.ts Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

@seantokuzo If we end up using this in the login flow as well, we will have to figure out how to deal with existing users that have passwords that don't meet this criteria.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we'll need to use it for login, just enforcing stronger passwords when created. It's kind of overkill to abstract it out with just the 'password.length > 7' check, but good if we decide to enforce more conditions

server/utils/index.ts Show resolved Hide resolved
…to CHE-199/subtask/Create-Tests-and-Error-Handling-for-userController-registerUser
Copy link
Contributor

@brok3turtl3 brok3turtl3 left a comment

Choose a reason for hiding this comment

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

@seantokuzo Great work 🎉

@brok3turtl3 brok3turtl3 merged commit f53b887 into dev Jul 1, 2024
1 check passed
@brok3turtl3 brok3turtl3 deleted the CHE-199/subtask/Create-Tests-and-Error-Handling-for-userController-registerUser branch July 1, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants