fix: Use data instead of parsed data for recording analytics #6196
Conversation
existentialcoder
requested review from
Zaimwa9
and removed request for
a team
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on November 10. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
@existentialcoder is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
|
@matthewelwell Apologies for the delay. Let me know if this would work! |
|
Hi @existentialcoder, thanks for the contribution! Can you make sure that the tests pass (see here) and I'll review it once that's done. Thanks! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6196 +/- ##
==========================================
+ Coverage 97.99% 98.01% +0.01%
==========================================
Files 1278 1278
Lines 44927 45151 +224
==========================================
+ Hits 44028 44253 +225
+ Misses 899 898 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
existentialcoder
changed the title
existentialcoder
changed the title
Addressed. I guess it's only the vercel complaints now |
Were you able to check? @matthewelwell |
existentialcoder
force-pushed
the
iss-5906
branch
from
efd8a29 to
c3bbcff
Compare
existentialcoder
force-pushed
the
iss-5906
branch
from
27d218d to
2221473
Compare
for more information, see https://pre-commit.ci
Might have fixed the build one more time. Can you check again, sorry :) @Zaimwa9 |
Just a reminder. Thanks :) |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
There was a problem hiding this comment.
@existentialcoder thanks again for the contribution, and sorry for the delay in getting back to you after the last review. I've added a few comments in on top of previous requests from @Zaimwa9 .
| @pytest.fixture() | ||
| def feature_with_dots(project: Project) -> Feature: | ||
| return Feature.objects.create(name="feature.with.dots", project=project) # type: ignore[no-any-return] |
There was a problem hiding this comment.
This doesn't need to be a fixture since it's only used in a single test. I'd rather that we just instantiate the feature directly in the test.
| # validated_data splits out request body with '.' in feature name (e.g a.b.c). | ||
| # Instead, it's safe to use self.data as keys are not altered. |
There was a problem hiding this comment.
This comment doesn't make sense to me. Surely we want the "." in the feature name. Can we be more explicit here about what the difference is between validated_data and data in this scenario?
Also, is there a neater way to solve this in the serializer rather than the view? I wonder if skipping validated_data here might lead to other problems down the line.
3 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Changes
.righly (Refer/v1/analytics/flagsparses data incorrectly for feature names that include dots #5906 for the exact error)initial_datainstead ofvalidated_datawhich has the wrong parsed formatPlease describe.
How did you test this code?
Manual API testing