Improve error handling for user lookup by email#1766
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Pull request overview
This PR refactors the user lookup function to improve error handling consistency. The changes rename getUserByEmailOrFail to getUserByEmailOrThrow and replace the repository's built-in error handling with explicit error checking and custom error throwing.
Key changes:
- Renamed function from
getUserByEmailOrFailtogetUserByEmailOrThrowfor clearer semantics - Replaced
findOneByOrFailwith explicit null checking and customApplicationErrorthrowing - Simplified function signature to accept email string directly instead of object parameter
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/server/api/src/app/user/user-service.ts | Refactored getUserByEmail function with explicit error handling and renamed to getUserByEmailOrThrow |
| packages/server/api/src/app/openops-tables/auth-admin-tables.ts | Updated function call to use new getUserByEmailOrThrow with simplified parameter |
| packages/server/api/src/app/authentication/new-user/organization-assignment.ts | Updated function call to use new getUserByEmailOrThrow with simplified parameter |
| packages/server/api/test/unit/openops-tables/auth-admin-tables.test.ts | Updated mock function name to match the renamed getUserByEmailOrThrow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
|
||
| async getUserByEmailOrFail({ email }: { email: string }): Promise<User> { | ||
| return userRepo().findOneByOrFail({ email }); | ||
| async getUserByEmailOrThrow(email: string): Promise<User> { |
There was a problem hiding this comment.
This one will result in status 404 in global exception handlers, which is preferable to status 500.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Part of OPS-3009.