fix(api): handle invalid project IDs when creating environments#6874
fix(api): handle invalid project IDs when creating environments#6874MartinLam12 wants to merge 6 commits intoFlagsmith:mainfrom
Conversation
|
@MartinLam12 is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
for more information, see https://pre-commit.ci
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
api/tests/unit/environments/permissions/test_unit_environments_permissions.py
Outdated
Show resolved
Hide resolved
|
Hi @MartinLam12 , thanks for the contribution. Could you please update the PR title as per this failing CI check, and also resolve the comment review from Cursor? |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
MartinLam12
changed the title
|
I updated the PR title and also resolved the comment review from Cursor. |
|
@matthewelwell Hello, can you take another look? Thanks |
api/tests/unit/environments/permissions/test_unit_environments_permissions.py
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6874 +/- ##
==========================================
- Coverage 98.31% 98.24% -0.08%
==========================================
Files 1335 1335
Lines 49730 49617 -113
==========================================
- Hits 48892 48746 -146
- Misses 838 871 +33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Matthew Elwell <mjelwell89@gmail.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| project = Project.objects.get(project_lookup) | ||
| return request.user.has_project_permission(CREATE_ENVIRONMENT, project) | ||
| except Project.DoesNotExist: | ||
| # We catch ValueError and TypeError here to resolve previous issues with invalid project IDs |
There was a problem hiding this comment.
PR commentary left permanently in production code
Low Severity
The comment # We catch ValueError and TypeError here to resolve previous issues with invalid project IDs reads as PR rationale rather than code documentation. It describes the historical motivation for the change rather than explaining the code's behavior. The except clause itself is self-documenting. The project maintainer already flagged this in review but it remains in the diff.
2 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to
Please describe.
Handle invalid project values in POST /api/v1/environments/ by catching ValueError and TypeError in EnvironmentPermissions.has_permission(). Previously, passing a non-integer value (e.g., "") caused an unhandled ValueError resulting in a 500 error. The permission check now returns False for invalid project IDs, returning a 403 instead of a 500
How did you test this code?
Please describe.
Added two unit tests in test_unit_environments_permissions.py: test_create_environment__invalid_project_id_string__returns_false — verifies a non-numeric string project ID returns False test_create_environment__none_project_id__returns_false — verifies a missing/null project ID returns False