Solution-py-fastapi-homework-3-task #22
Conversation
There was a problem hiding this comment.
Great job on implementing the user-related operations! 🎉 Your code meets the task requirements and constraints, and you've handled edge cases well. While there are some areas for improvement, such as ensuring password hashing and consistent email validation, these are minor and do not impact the core functionality. Keep up the good work and consider the feedback for future enhancements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| detail=f"A user with this email {register_data.email} already exists." | ||
| ) | ||
|
|
||
| user_group = db.query(UserGroupModel).filter_by(name=UserGroupEnum.USER).first() |
There was a problem hiding this comment.
The filter_by(name=UserGroupEnum.USER) assumes that UserGroupEnum.USER is a valid enum value. Ensure that this enum value exists and matches the expected user group name in the database.
| ) | ||
|
|
||
| try: | ||
| user.password = password_data.password |
There was a problem hiding this comment.
Assigning user.password = password_data.password directly may not be secure if the password is stored in plain text. Ensure that the password is hashed before storing it in the database.
| db.add(refresh_token_model) | ||
| db.commit() | ||
| db.refresh(refresh_token_model) | ||
| except SQLAlchemyError: |
There was a problem hiding this comment.
Consider adding db.rollback() here to ensure the transaction is rolled back in case of an error during the login process.
| refresh_token_record = ( | ||
| db.query(RefreshTokenModel).filter_by(token=token_data.refresh_token).first() | ||
| ) | ||
| if not refresh_token_record: |
There was a problem hiding this comment.
The check for refresh_token_record should also ensure that the token is not expired. Consider adding a check for the token's expiration date if applicable.
| @@ -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.
Consider providing more specific error details in the HTTPException for invalid input data to help clients understand what went wrong.
| @@ -394,6 +394,6 @@ | |||
| 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.
Consider providing more specific error details in the HTTPException for invalid input data to help clients understand what went wrong during the update process.
|
|
||
|
|
||
| class UserActivationRequestSchema(BaseModel): | ||
| email: str |
There was a problem hiding this comment.
Consider using EmailStr for the email field in UserActivationRequestSchema to ensure consistent email validation across all schemas.
|
|
||
|
|
||
| class PasswordResetRequestSchema(BaseModel): | ||
| email: str |
There was a problem hiding this comment.
Consider using EmailStr for the email field in PasswordResetRequestSchema to ensure consistent email validation across all schemas.
| @@ -146,6 +146,7 @@ class MovieListResponseSchema(BaseModel): | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| class MovieCreateSchema(BaseModel): | |||
| name: str | |||
There was a problem hiding this comment.
Consider adding a max_length constraint to the name field in MovieCreateSchema to ensure consistency with MovieBaseSchema.
No description provided.