Solution #5
Solution #5
Conversation
There was a problem hiding this comment.
Great job on implementing the user-related operations and ensuring the core functionalities are in place! 🎉 While there are some minor improvements suggested, such as using EmailStr for better email validation and providing more specific error messages, these do not critically impact the functionality of your code. Keep up the good work, and consider these suggestions for future enhancements to make your code even more robust and user-friendly! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -25,7 +25,7 @@ jobs: | |||
|
|
|||
| - name: Run flake8 | |||
| run: | | |||
| poetry run flake8 src | |||
| poetry run flake8 src --exclude=src/database/migrations/* | |||
There was a problem hiding this comment.
The flake8 command in the test-accounts job excludes the src/database/migrations/* directory. Ensure that this exclusion is intentional and aligns with your project's linting requirements. If you want to lint the entire src directory without exclusions, remove the --exclude=src/database/migrations/* option.
| detail=f"A user with this email {user.email} already exists." | ||
| ) | ||
|
|
||
| user_group = db.query(UserGroupModel).filter(UserGroupModel.name == UserGroupEnum.USER).first() |
There was a problem hiding this comment.
Ensure that user_group is not None before accessing user_group.id. If the UserGroupEnum.USER group does not exist, this will raise an AttributeError. Consider adding a check to handle this case.
| db: Session = Depends(get_db) | ||
| ): | ||
| db_user = db.query(UserModel).filter(UserModel.email == user.email).first() | ||
| if db_user.is_active: |
There was a problem hiding this comment.
Ensure that db_user is not None before accessing db_user.is_active. If the user does not exist, this will raise an AttributeError. Consider adding a check to handle this case.
| db: Session = Depends(get_db) | ||
| ): | ||
| db_user = db.query(UserModel).filter(UserModel.email == user.email).first() | ||
| if not db_user or not db_user.is_active: |
There was a problem hiding this comment.
The condition if not db_user or not db_user.is_active: might be misleading. If db_user is None, accessing db_user.is_active will raise an AttributeError. Consider separating the checks for db_user existence and is_active status.
| ): | ||
| db_user = db.query(UserModel).filter_by(email=user_data.email).first() | ||
|
|
||
| if not db_user or not db_user.verify_password(raw_password=user_data.password): |
There was a problem hiding this comment.
Ensure that db_user is not None before calling db_user.verify_password. If the user does not exist, this will raise an AttributeError. Consider adding a check to handle this case.
| @@ -208,7 +208,7 @@ def create_movie( | |||
| return MovieDetailSchema.model_validate(movie) | |||
| except IntegrityError: | |||
| db.rollback() | |||
| raise HTTPException(status_code=400, detail=f"Invalid input data.") | |||
| raise HTTPException(status_code=400, detail="Invalid input data.") | |||
There was a problem hiding this comment.
The error message 'Invalid input data.' is quite generic. Consider providing more specific details about what might be wrong with the input data to help clients debug their requests.
| @@ -394,6 +394,6 @@ def update_movie( | |||
| db.refresh(movie) | |||
| except IntegrityError: | |||
| db.rollback() | |||
| raise HTTPException(status_code=400, detail=f"Invalid input data.") | |||
| raise HTTPException(status_code=400, detail="Invalid input data.") | |||
There was a problem hiding this comment.
Similar to the create_movie function, the error message 'Invalid input data.' is generic. Consider providing more specific details about what might be wrong with the input data to help clients debug their requests.
|
|
||
| class UserRegistrationResponseSchema(BaseModel): | ||
| id: int | ||
| email: str |
There was a problem hiding this comment.
Consider using EmailStr for the email field in UserRegistrationResponseSchema to maintain consistency with the UserRegistrationRequestSchema and ensure that the email format is validated.
|
|
||
|
|
||
| class UserActivationRequestSchema(BaseModel): | ||
| email: str |
There was a problem hiding this comment.
Consider using EmailStr for the email field in UserActivationRequestSchema to ensure that the email format is validated.
|
|
||
|
|
||
| class PasswordResetRequestSchema(BaseModel): | ||
| email: str |
There was a problem hiding this comment.
Consider using EmailStr for the email field in PasswordResetRequestSchema to ensure that the email format is validated.
No description provided.