fix(versioning): fix cloning environments using v2 versioning #3999
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: api/features/versioning/models.py
Did you find this useful? React with a 👍 or 👎 |
|
Uffizzi Preview |
matthewelwell
requested review from
a team and
zachaysan
and removed request for
a team
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3999 +/- ##
=======================================
Coverage 96.21% 96.22%
=======================================
Files 1142 1142
Lines 36724 36760 +36
=======================================
+ Hits 35335 35371 +36
Misses 1389 1389 ☔ View full report in Codecov by Sentry. |
api/features/versioning/managers.py
Outdated
| return self.filter( | ||
| uuid__in=[ | ||
| efv.uuid | ||
| for efv in EnvironmentFeatureVersion.objects.get_latest_versions(self) | ||
| ] | ||
| ) |
There was a problem hiding this comment.
I'm not particularly happy with this, but the alternative was to update the sql to return all the fields and then (in the Environment.clone method) manually create the EnvironmentFeatureVersion, rather than using a clone method on the model class. To keep things consistent with other models, this seemed like the lesser of 2 evils.
There was a problem hiding this comment.
Why not write a second sql with all the fields?
manually create the EnvironmentFeatureVersion, rather than using a clone method on the model class
I don't think so as long as the model instances were correctly mapped?
There was a problem hiding this comment.
I think that "Cloning environments" is not used frequently enough to make this a real issue.
There was a problem hiding this comment.
I think that "Cloning environments" is not used frequently enough to make this a real issue.
Well, I wish this were true. But the FE made the decision to remove the functionality to 'create' an environment, and now all environments are cloned from another.
Why not write a second sql with all the fields?
I'd still rather avoid this to avoid code duplication or hacky ways of concatenating SQL strings.
matthewelwell
force-pushed
the
fix(versioning)/clone-environment
branch
from
ba65782 to
655a9cf
Compare
Thanks for submitting a PR! Please check the boxes below:
pre-committo check lintingdocs/if required so people know about the feature!Changes
This fixes an issue found in testing when cloning an environment that has enabled v2 versioning.
How did you test this code?
Added a unit test for cloning an environment using v2 versioning.