fix: ignore old versions when validating segment override limit #4618
Conversation
matthewelwell
requested review from
zachaysan
and removed request for
a team
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
…imits # Conflicts: # api/tests/unit/features/versioning/test_unit_versioning_views.py
Docker builds report
|
|
Uffizzi Preview |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4618 +/- ##
========================================
Coverage 97.16% 97.17%
========================================
Files 1160 1162 +2
Lines 40159 40267 +108
========================================
+ Hits 39022 39130 +108
Misses 1137 1137 ☔ View full report in Codecov by Sentry. |
| segment_ids_to_create_overrides: list[int] = None, | ||
| segment_ids_to_delete_overrides: list[int] = None, |
There was a problem hiding this comment.
I understand it's setting them equal to empty list below, but I still think the typing should match the default arguments.
| segment_ids_to_create_overrides: list[int] = None, | |
| segment_ids_to_delete_overrides: list[int] = None, | |
| segment_ids_to_create_overrides: list[int] | None = None, | |
| segment_ids_to_delete_overrides: list[int] | None = None, |
There was a problem hiding this comment.
Yep, that's a fair point, updated.
| if _check( | ||
| segment_override_count + extra, | ||
| environment.project.max_segment_overrides_allowed, | ||
| ): | ||
| return True | ||
|
|
||
| return False |
There was a problem hiding this comment.
| if _check( | |
| segment_override_count + extra, | |
| environment.project.max_segment_overrides_allowed, | |
| ): | |
| return True | |
| return False | |
| return _check( | |
| segment_override_count + extra, | |
| environment.project.max_segment_overrides_allowed, | |
| ) |
There was a problem hiding this comment.
Good catch, updated.
| # We create a new version, without making any changes, we shouldn't receive | ||
| # any errors. |
There was a problem hiding this comment.
I'm a little confused by this comment because a status.HTTP_400_BAD_REQUEST is returned below. Is a 400 response not considered an error?
There was a problem hiding this comment.
Sorry, this was a copy pasta error.
…imits # Conflicts: # api/features/versioning/serializers.py
Changes
Ensure that old versions aren't included when validating segment override limit.
How did you test this code?
Added tests.