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(api): validate before creating projects based on current subscription #2869

Merged
merged 8 commits into from
Nov 20, 2023

Conversation

tushar5526
Copy link
Contributor

@tushar5526 tushar5526 commented Oct 22, 2023

Closes #2835
Thanks for submitting a PR! Please check the boxes below:

  • I have run pre-commit to check linting
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

Subscription metadata contains the number of projects allowed in a subscription, use that to validate and create project requests.

PS: All the tests are written assuming that the default subscription allows the creation of multiple projects, therefore I had to allow up to the creation of 10 projects in the default subscription for tests. Another way could to be change the default subscription for every failing test that requires the creation of multiple projects.

How did you test this code?

Created a test ORG and tried to create multiple projects via API.

@vercel
Copy link

vercel bot commented Oct 22, 2023

@tushar5526 is attempting to deploy a commit to the Flagsmith Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 22, 2023

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

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 20, 2023 4:56pm
flagsmith-frontend-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 20, 2023 4:56pm

@github-actions
Copy link
Contributor

github-actions bot commented Oct 22, 2023

Uffizzi Preview deployment-39138 was deleted.

@tushar5526 tushar5526 changed the title Fix: validate before creating projects based on current subscription fix(api): validate before creating projects based on current subscription Oct 22, 2023
api/projects/serializers.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Oct 24, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (b0ef013) 95.60% compared to head (89adb29) 95.61%.
Report is 79 commits behind head on main.

❗ Current head 89adb29 differs from pull request most recent head d169e96. Consider uploading reports for the commit d169e96 to get more accurate results

Files Patch % Lines
api/integrations/launch_darkly/tasks.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2869   +/-   ##
=======================================
  Coverage   95.60%   95.61%           
=======================================
  Files        1009     1009           
  Lines       28904    28936   +32     
=======================================
+ Hits        27634    27666   +32     
  Misses       1270     1270           

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

…tion

fix: Move max project constant to settings for tests
api/app/settings/common.py Outdated Show resolved Hide resolved
Copy link
Contributor

@matthewelwell matthewelwell left a comment

Choose a reason for hiding this comment

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

Thanks again for this @tushar5526 ! I've added a few (mostly minor) comments but this looks good on the whole.

api/projects/permissions.py Outdated Show resolved Hide resolved
api/projects/permissions.py Outdated Show resolved Hide resolved
api/projects/tests/test_permissions.py Outdated Show resolved Hide resolved
api/projects/tests/test_permissions.py Outdated Show resolved Hide resolved
api/projects/tests/test_permissions.py Show resolved Hide resolved
api/app/settings/common.py Outdated Show resolved Hide resolved
Copy link
Contributor

@matthewelwell matthewelwell left a comment

Choose a reason for hiding this comment

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

I've added a couple of minor comments but on the whole, this looks great. Thanks again @tushar5526 .

api/organisations/subscriptions/constants.py Outdated Show resolved Hide resolved
api/projects/tests/test_permissions.py Outdated Show resolved Hide resolved
@matthewelwell matthewelwell added this pull request to the merge queue Nov 20, 2023
Merged via the queue into Flagsmith:main with commit f32159e Nov 20, 2023
18 of 20 checks passed
@tushar5526 tushar5526 deleted the 2835 branch November 20, 2023 19:26
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Projects can be created via API under the free plan
4 participants