fix(6905): replace dynamic serializer with SDKAnalyticsFlagsV1Serializer#6908
Merged
gagantrivedi merged 3 commits intomainfrom Mar 11, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
Contributor
Docker builds report
|
Contributor
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
|
Contributor
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6908 +/- ##
==========================================
- Coverage 98.34% 98.31% -0.03%
==========================================
Files 1335 1335
Lines 49707 49717 +10
==========================================
- Hits 48882 48879 -3
- Misses 825 838 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c39d427 to
a5e89f9
Compare
Contributor
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
|
Contributor
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
|
The V1 SDKAnalyticsFlags view dynamically generated a DRF serializer inside get_serializer_class. Extract this into a proper SDKAnalyticsFlagsV1Serializer so the view mirrors the clean V2 pattern of serializer_class / get_serializer / is_valid / save.
a7e4367 to
b8edc1d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Free Tier Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Boolean values pass
isinstance(count, int)check- Changed the V1 analytics serializer to accept only exact
intvalues and added a regression test confirming boolean counts are skipped.
- Changed the V1 analytics serializer to accept only exact
Or push these changes by commenting:
@cursor push 97bade19b0
Preview (97bade19b0)
diff --git a/api/app_analytics/serializers.py b/api/app_analytics/serializers.py
--- a/api/app_analytics/serializers.py
+++ b/api/app_analytics/serializers.py
@@ -137,7 +137,7 @@
return {
name: count
for name, count in data.items()
- if isinstance(name, str) and isinstance(count, int)
+ if isinstance(name, str) and type(count) is int
}
def validate(self, attrs: dict[str, int]) -> dict[str, int]:
diff --git a/api/tests/unit/app_analytics/test_unit_app_analytics_views.py b/api/tests/unit/app_analytics/test_unit_app_analytics_views.py
--- a/api/tests/unit/app_analytics/test_unit_app_analytics_views.py
+++ b/api/tests/unit/app_analytics/test_unit_app_analytics_views.py
@@ -110,6 +110,31 @@
)
+def test_sdk_analytics_flags_v1__boolean_count__is_skipped(
+ mocker: MockerFixture,
+ environment: Environment,
+ feature: Feature,
+ api_client: APIClient,
+) -> None:
+ # Given
+ api_client.credentials(HTTP_X_ENVIRONMENT_KEY=environment.api_key)
+ data = {feature.name: True}
+ mocked_feature_eval_cache = mocker.patch(
+ "app_analytics.views.feature_evaluation_cache"
+ )
+
+ url = reverse("api-v1:analytics-flags")
+
+ # When
+ response = api_client.post(
+ url, data=json.dumps(data), content_type="application/json"
+ )
+
+ # Then
+ assert response.status_code == status.HTTP_200_OK
+ mocked_feature_eval_cache.track_feature_evaluation.assert_not_called()
+
+
def test_get_usage_data(mocker, admin_client, organisation): # type: ignore[no-untyped-def]
# Given
url = reverse("api-v1:organisations:usage-data", args=[organisation.id])This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
dcd9372 to
d422a84
Compare
for more information, see https://pre-commit.ci
Zaimwa9
approved these changes
Mar 11, 2026
Contributor
Zaimwa9
left a comment
There was a problem hiding this comment.
Looks good! Indeed, I can get why linking the sentry issue with this code was sneaky
This was referenced Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
docs/if required so people know about the feature.Changes
Closes #6905, #5906
The V1
SDKAnalyticsFlagsview dynamically generated a DRF serializer insideget_serializer_class. This caused dotted feature names (e.g.org.app.module:handler) to be split into nested dicts by DRF'sset_value.This PR replaces the dynamic serializer with a proper
SDKAnalyticsFlagsV1Serializer.Unknown feature names were already silently dropped by the old code (only known names had fields). The new serializer preserves this behaviour explicitly in
validate. Non-integer values are now also silently skipped — the old code would have raised an unhandledAssertionErrorin this case sinceis_valid()was called withoutraise_exception=True, leavingvalidated_dataunpopulated. This never surfaced because SDKs always send integers.How did you test this code?
Existing and new unit tests:
All 22 tests pass (20 passed, 2 skipped due to no analytics database configured).