feat: Clone identity flag states #3773
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Uffizzi Preview |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3773 +/- ##
==========================================
+ Coverage 95.89% 95.93% +0.04%
==========================================
Files 1102 1122 +20
Lines 34707 35656 +949
==========================================
+ Hits 33282 34208 +926
- Misses 1425 1448 +23 ☔ View full report in Codecov by Sentry. |
|
I am tempted to create one or a couple of fixtures that return all the assets used in the integration tests since they both, API Core and the EDdge API tests, are very similar, even the asserts are almost the same. |
|
@novakzaballa @zachaysan can you make sure all outstanding conversations on this PR are responded to or resolved please? |
Apply additional code optimisations.
novakzaballa
force-pushed
the
chore/clone-identity-flag-states
branch
from
9112d97 to
81e6e55
Compare
api/edge_api/identities/views.py
Outdated
| master_api_key=getattr(request, "master_api_key", None), | ||
| ) | ||
|
|
||
| # Prepare response |
There was a problem hiding this comment.
Couldn't we just do return self.all(request, *args, **kwargs) ? Also, bear in mind that the documentation for this endpoint needs updating as well!
api/features/views.py
Outdated
| target_identity=target_identity, source_identity=source_identity | ||
| ) | ||
|
|
||
| # Prepare response |
There was a problem hiding this comment.
Again, can we use return self.all(request, *args, **kwargs) here?
There was a problem hiding this comment.
Right. Done. Also updated the autogenerate swagger command
| for feature_state in identity_flags: | ||
| overridden_feature_states[feature_state.feature.id] = feature_state | ||
| return overridden_feature_states |
There was a problem hiding this comment.
No, my point is that we should use feature_id instead of feature.id this will provide a small performance optimisation. I'll add a suggestion to the actual line in another comment to make it clear.
| :return: dict[int, FeatureState] - Key: feature ID. Value: Overridden feature_state. | ||
| """ | ||
|
|
||
| return {fs.feature.id: fs for fs in self.identity_features.all()} |
There was a problem hiding this comment.
| return {fs.feature.id: fs for fs in self.identity_features.all()} | |
| return {fs.feature_id: fs for fs in self.identity_features.all()} |
Related thread: #3773 (comment)
There was a problem hiding this comment.
Right, we don't need to load the whole related feature but just the FK which is already in the feature_sate data.
Done
Thanks for submitting a PR! Please check the boxes below:
pre-committo check lintingdocs/if required so people know about the feature!Changes
Add Core and Edge edpoints for clonning identity's flag states.
How did you test this code?
Integrations tests added