Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ProjectUserDict usage and standardise geojson parsing/usage throughout #1659

Merged
merged 17 commits into from
Jul 17, 2024

Conversation

spwoodcock
Copy link
Member

@spwoodcock spwoodcock commented Jul 16, 2024

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation
  • πŸ§‘β€πŸ’» Refactor
  • βœ… Test
  • πŸ€– Build or CI
  • ❓ Other (please specify)

Related Issue

Related to #1657

Describe this PR

  • Fixes issue where we no longer use AuthUser for the mapper endpoints, but instead a dict ProjectUserDict, where the DbUser and DbProject must be obtained from keys user and project respectively.
  • I also refactored the project_crud and project_routes logic to take this into account and be more efficient (preventing double db access to get the project twice, as we now have it available).
  • TODO also need to update the logic / fix for task_routes and submission_routes. @Sujanadh please pick up where I left off πŸ™
  • I also took the opportunity to do a big refactor of our geojson handling, as it started to spraw a bit.
    • I tried to reuse the same functions as much as possible, handling the geojson as FeatureCollection where possible.
    • I also separated usage of jazzband/geojson and geojson_pydantic: geojson_pydantic is used for the pydantic models only, then all other usage defers to .geojson.
    • Please test endpoints thoroughly after the refactor πŸ˜„
    • The endpoints I updated are working & project creation / viewing also seems to work fine for me.

@Sujanadh the remaining task:

  • Replace current_user: AuthUser = Depends(mapper) with project_user: ProjectUserDict = Depends(mapper) in submission_routes and task_routes.
  • In all cases where this is replaced, ensure the logic uses .get("project") if required
  • If the project is got via another function, try to remove that logic and refer to project_user.get("project") where possible.

Review Guide

Test the things!

Checklist before requesting a review

[optional] What gif best describes this PR or how it makes you feel?

@Sujanadh
Copy link
Collaborator

Thats a huge refactor πŸ™Œ

@Sujanadh
Copy link
Collaborator

looks all good I tried creating project, made submissions, filter, review status, update entity status all seems working pretty good πŸ‘

src/backend/app/auth/roles.py Outdated Show resolved Hide resolved
@spwoodcock spwoodcock marked this pull request as ready for review July 17, 2024 12:30
@Sujanadh
Copy link
Collaborator

I added back your code from your last commit to better handle invalid multipolygon geoms which was removed when merging conflicts.

@spwoodcock spwoodcock merged commit 018605b into development Jul 17, 2024
7 checks passed
@spwoodcock spwoodcock deleted the refactor/geojson-handling branch July 17, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to backend code frontend Related to frontend code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants