feat: add authentication and validate DB results with Pydantic schemas #60
Conversation
…sults with Pydantic schemas
| @@ -58,96 +52,62 @@ async def create_project_with_project_info( | |||
| }, | |||
| ) | |||
|
|
|||
| if not new_project_id: | |||
| if not project_id: | |||
| raise HTTPException(status_code=500, detail="Project could not be created") | |||
| # Fetch the newly created project using the returned ID | |||
| select_query = f""" | |||
There was a problem hiding this comment.
This seems unnecessary after the insert above.
You could just add the fields you need to the RETURNING clause and save a database call 👍
There was a problem hiding this comment.
Thank you for the suggestion! I have updated the query to include only the necessary fields in the RETURNING clause, eliminating the extra database call. Appreciate the feedback! 👍
| projects.name, | ||
| projects.short_description, | ||
| projects.description, | ||
| projects.per_task_instructions, |
There was a problem hiding this comment.
More of a db schema related question, but I don't understand this variable name.
Isn't it just the instructions for the project?
It could probably just be projects.instructions?
There was a problem hiding this comment.
@spwoodcock I am returning only the necessary fields of the projects model instead of the whole model, and those fields are also validated using Pydantic schemas.
Regarding the variable name, I agree it could probably be just projects.instructions instead of projects.per_task_instructions.
| def delete_project_by_id( | ||
| project_id: uuid.UUID, | ||
| db: Session = Depends(database.get_db), | ||
| AuthUser=Depends(login_required), |
There was a problem hiding this comment.
This is missing a param name:
user: AuthUser = Depends(login_required)
as auth user is just the param type
There was a problem hiding this comment.
@spwoodcock I have just fixed it by adding the missing parameter name. Thanks for pointing that out!
| @@ -83,6 +93,7 @@ async def upload_project_task_boundaries( | |||
| project_id: uuid.UUID, | |||
| task_geojson: UploadFile = File(...), | |||
| db: Database = Depends(database.encode_db), | |||
| AuthUser=Depends(login_required), | |||
There was a problem hiding this comment.
Same here.
Another endpoint has it right with
user_data: AuthUser =
| project_geojson: UploadFile = File(...), dimension: int = Form(100) | ||
| project_geojson: UploadFile = File(...), | ||
| dimension: int = Form(100), | ||
| AuthUser=(login_required), |
The pull request I created here encountered numerous merge conflicts and merge commits, which made the codebase difficult to manage and review. In order to ensure a clean and manageable integration, I decided to create a new pull request.
This new PR consolidates all the necessary changes without the clutter of previous conflicts. I
@nrjadkry @spwoodcock Please review this PR.
What type of PR is this? (check all applicable)
Describe this PR
This PR introduces the following features:
Screenshots
Review Guide
To test this change: