refactor: create AccountServices module and refactor access controller #194
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Moved code for getting account into new service layer module in models/services/account.ts and added unit tests for getAccount
app.ts had a build error due to the User type mismatching between the passport and passport-local-mongoose packages. To fix this, I modified our custom Express.user type to extend UserType. This commit also cleans up User and the previous workaround.
hiyaryan
requested changes
Nov 19, 2024
Integrate code review comments for getAccount refactor.
Moved code for updating journal into service layer function and added unit tests for updateJournal
Move business logic for updateAccount into account services. Split into updateProfile, updatePassword, and updateConfig. Add unit tests.
Refactor deleteItem into deleteConfig and deleteAccount service functions, and write unit tests.
Refactor controller function to move account creation into createAccount service function. Move validateJournal call into earlier in middleware chain. Add unit test for createAccount.
Refactor verifyEmail into verifyEmail service function, and add unit tests for verifyEmail service function.
Refactor resetPassword in access controller into resetPassword service function and add unit tests.
Have controller check if error thrown by service function and pass to error handler instead of creating 500 ExpressError.
Refactor forgotPassword into forgotPassword service function and rewrite using async/await instead of callback style. Add unit tests.
Refactor login controller handler into several functions and add unit tests for ensureUserJournal.
davidzlu
changed the title
Refactor tokenLogin in access controller.
The arguments for AccountServices.resetPassword were passed in the wrong order for the resetPassword controller handler. This commit corrects the order of the arguments.
hiyaryan
requested changes
Dec 15, 2024
There was a problem hiding this comment.
Really fine work @davidzlu! 🚀 I have tried my best to carefully review everything over the last 3 days and left some suggestions. Please take a look and let me know if you have any questions or would like to discuss anything. The access controller is looking pristine with this service layer! ✨
Co-authored-by: Ryan James Meneses <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: This PR was originally for refactoring just the get operations in the access controller before it was expanded to refactoring the entire access controller. This is why the branch name is still
139-get-access-operations.This PR:
There was a tsc error where the user type in the passport and and passport-local-mongoose modules didn't match. This was caused by the way Express.User was getting defined in our custom type declaration file. Express.User now extends the CDJ UserType in models to resolve the build error.