Skip to content

Conversation

@allisonsibrian
Copy link
Contributor

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

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

Related Issue

Fixes #686

Describe this PR

This PR resolves the 403/404 errors encountered in the backend tests. It resolves database conflicts, fixes permission logic, and ensures all backend tests pass in CI.

Fixed background issue in app/projects/project_logic.py:

  • Updated process_task_metrics to handle cases where Psycopg 3 returns points as a dictionary wrapper instead of a GeoJSON string. This prevents a TypeError in geojson.loads.

Fixed permission logic in app/auth/permissions.py:

  • Restructured ordering for has_permission, where the system validates user.is_superuser before checking if the target object exists. This fixes 403 errors on "Create" endpoints where the object is None.

Conftest.py fixes:

  • Updated project_info to generate UUID-based project names and prevents "Project name already exists" issues.
  • Added is_superuser=True via SQL to ensure Admin role.
  • Added an override for login_dependency, as this allows the permission system to accept the test user.

Other fixes:

  • Corrected URL paths in test_projects_routes and test_users_routes
  • Updated test_reset_password_success to generate a valid token for the test user.

Review Guide

Run backend tests and verify all 7 tests pass.

Checklist before requesting a review

  • πŸ“– Read the HOT Code of Conduct: https://docs.hotosm.org/code-of-conduct
  • πŸ‘·β€β™€οΈ Create small PRs. In most cases, this will be possible.
  • βœ… Provide tests for your changes.
  • πŸ“ Use descriptive commit messages.
  • πŸ“— Update any related documentation and include any relevant screenshots.

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

@github-actions github-actions bot added bug Something isn't working backend Related to backend code labels Dec 16, 2025
@allisonsibrian allisonsibrian marked this pull request as ready for review December 16, 2025 00:47
@spwoodcock
Copy link
Member

Nice, fantastic to see the tests operational!

I can't seem to comment on specific lines via the github app currently, so will review all together.

1
For the is_superuser check, if user is None, then checking user.is_superuser first would AttributeError no?

2
Also for the db.execute to update the user to a superuser, couldn't the user initially be set with the is_superuser field set to True, instead of having to update after?

@allisonsibrian
Copy link
Contributor Author

For the is_superuser check, if user is None, then checking user.is_superuser first would AttributeError no?

2 Also for the db.execute to update the user to a superuser, couldn't the user initially be set with the is_superuser field set to True, instead of having to update after?

  1. Thank you for catching this. I didn't realize when I swapped the blocks, but I'll update this!
  2. You're right that initially setting it is better. I originally added the SQL to handle already existing test users that didn't have any Admin privileges, but it's much simpler to set AuthUser to accept is_superuser=True and still keep db_user.is_superuser = True to handle those cases.

@spwoodcock
Copy link
Member

Looks great! πŸ’«

@spwoodcock spwoodcock merged commit 2812aa8 into hotosm:dev Dec 17, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Related to backend code bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants