Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Force environments to be different names #5058

Merged
merged 7 commits into from
Jan 31, 2025

Conversation

zachaysan
Copy link
Contributor

Changes

This forces new environments to have unique names. This was not intentionally done at the model level because some existing integrations rely on environment names and could break if they were updated to different names to solve existing conflicts.

How did you test this code?

One new test.

@zachaysan zachaysan requested a review from a team as a code owner January 29, 2025 20:55
@zachaysan zachaysan requested review from gagantrivedi and removed request for a team January 29, 2025 20:55
Copy link

vercel bot commented Jan 29, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
flagsmith-frontend-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 31, 2025 2:25pm
flagsmith-frontend-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 31, 2025 2:25pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) Visit Preview Jan 31, 2025 2:25pm

@github-actions github-actions bot added api Issue related to the REST API fix labels Jan 29, 2025
Copy link
Contributor

github-actions bot commented Jan 29, 2025

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-api-test:pr-5058 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-5058 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-api:pr-5058 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-private-cloud:pr-5058 Finished ✅ Results
ghcr.io/flagsmith/flagsmith:pr-5058 Finished ✅ Results

Copy link
Contributor

github-actions bot commented Jan 29, 2025

Uffizzi Preview deployment-60419 was deleted.

Copy link

codecov bot commented Jan 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.43%. Comparing base (b0d0a80) to head (d3dd6b2).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5058   +/-   ##
=======================================
  Coverage   97.43%   97.43%           
=======================================
  Files        1214     1214           
  Lines       42253    42264   +11     
=======================================
+ Hits        41169    41180   +11     
  Misses       1084     1084           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions github-actions bot added fix and removed fix labels Jan 30, 2025
Comment on lines 143 to 148
existing_environment = Environment.objects.filter(
name=serializer.validated_data["name"],
project=serializer.validated_data["project"],
)
if existing_environment:
raise ValidationError("Existing environment for given name.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can do this in a much simpler way using the serializer as per the PR I linked on the issue (see here)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed the linked PR! I've updated the code to handle this in a similar manner to what you posted.

@matthewelwell matthewelwell merged commit 22223da into main Jan 31, 2025
37 checks passed
@matthewelwell matthewelwell deleted the fix/force_environments_to_be_different_names branch January 31, 2025 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issue related to the REST API fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants