Skip to content

Commit

Permalink
fix(api): validate before creating projects based on current subscrip…
Browse files Browse the repository at this point in the history
…tion (#2869)

Co-authored-by: Matthew Elwell <[email protected]>
  • Loading branch information
tushar5526 and matthewelwell authored Nov 20, 2023
1 parent 5f3482c commit f32159e
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 14 deletions.
2 changes: 2 additions & 0 deletions api/app/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@

HOSTED_SEATS_LIMIT = env.int("HOSTED_SEATS_LIMIT", default=0)

MAX_PROJECTS_IN_FREE_PLAN = 1

# Google Analytics Configuration
GOOGLE_ANALYTICS_KEY = env("GOOGLE_ANALYTICS_KEY", default="")
GOOGLE_SERVICE_ACCOUNT = env("GOOGLE_SERVICE_ACCOUNT", default=None)
Expand Down
2 changes: 1 addition & 1 deletion api/app/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

# We dont want to track tests
ENABLE_TELEMETRY = False

MAX_PROJECTS_IN_FREE_PLAN = 10
REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"] = {
"login": "100/min",
"mfa_code": "5/min",
Expand Down
3 changes: 1 addition & 2 deletions api/organisations/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
CHARGEBEE,
FREE_PLAN_ID,
MAX_API_CALLS_IN_FREE_PLAN,
MAX_PROJECTS_IN_FREE_PLAN,
MAX_SEATS_IN_FREE_PLAN,
SUBSCRIPTION_BILLING_STATUSES,
SUBSCRIPTION_PAYMENT_METHODS,
Expand Down Expand Up @@ -287,7 +286,7 @@ def get_subscription_metadata(self) -> BaseSubscriptionMetadata:
return metadata or BaseSubscriptionMetadata(
seats=self.max_seats,
api_calls=self.max_api_calls,
projects=MAX_PROJECTS_IN_FREE_PLAN,
projects=settings.MAX_PROJECTS_IN_FREE_PLAN,
)

def add_single_seat(self):
Expand Down
7 changes: 4 additions & 3 deletions api/organisations/subscriptions/constants.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
from enum import Enum

from django.conf import settings

from organisations.subscriptions.metadata import BaseSubscriptionMetadata

MAX_SEATS_IN_FREE_PLAN = 1
MAX_API_CALLS_IN_FREE_PLAN = 50000
MAX_PROJECTS_IN_FREE_PLAN = 1
SUBSCRIPTION_DEFAULT_LIMITS = (
MAX_API_CALLS_IN_FREE_PLAN,
MAX_SEATS_IN_FREE_PLAN,
MAX_PROJECTS_IN_FREE_PLAN,
settings.MAX_PROJECTS_IN_FREE_PLAN,
)

CHARGEBEE = "CHARGEBEE"
Expand All @@ -35,7 +36,7 @@
FREE_PLAN_SUBSCRIPTION_METADATA = BaseSubscriptionMetadata(
seats=MAX_SEATS_IN_FREE_PLAN,
api_calls=MAX_API_CALLS_IN_FREE_PLAN,
projects=MAX_PROJECTS_IN_FREE_PLAN,
projects=settings.MAX_PROJECTS_IN_FREE_PLAN,
)
FREE_PLAN_ID = "free"

Expand Down
15 changes: 14 additions & 1 deletion api/projects/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,22 @@ def has_permission(self, request, view):
if view.action == "create" and request.user.belongs_to(
int(request.data.get("organisation"))
):
organisation = Organisation.objects.get(
organisation = Organisation.objects.select_related("subscription").get(
id=int(request.data.get("organisation"))
)

# Allow project creation based on the active subscription
subscription_metadata = (
organisation.subscription.get_subscription_metadata()
)
total_projects_created = Project.objects.filter(
organisation=organisation
).count()
if (
subscription_metadata.projects
and total_projects_created >= subscription_metadata.projects
):
return False
if organisation.restrict_project_create_to_admin:
return request.user.is_organisation_admin(organisation.pk)
return request.user.has_organisation_permission(
Expand Down
30 changes: 28 additions & 2 deletions api/projects/tests/test_permissions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from unittest import TestCase, mock

import pytest
from django.conf import settings

from organisations.models import Organisation, OrganisationRole
from projects.models import (
Expand All @@ -16,8 +17,8 @@
)
from users.models import FFAdminUser, UserPermissionGroup

mock_request = mock.MagicMock
mock_view = mock.MagicMock
mock_request = mock.MagicMock()
mock_view = mock.MagicMock()


@pytest.mark.django_db
Expand Down Expand Up @@ -371,3 +372,28 @@ def test_regular_user_has_no_destroy_permission(self):

# Then - exception thrown
assert not result


@pytest.mark.django_db
def test_free_plan_has_only_fixed_projects_permission():
# Given
organisation = Organisation.objects.create(name="Test organisation")

user = FFAdminUser.objects.create(email="[email protected]")

user.add_organisation(organisation, OrganisationRole.ADMIN)

project_permissions = ProjectPermissions()

mock_view = mock.MagicMock(action="create", detail=False)
mock_request = mock.MagicMock(
data={"name": "Test", "organisation": organisation.id}, user=user
)

# When
for i in range(settings.MAX_PROJECTS_IN_FREE_PLAN):
assert project_permissions.has_permission(mock_request, mock_view)
Project.objects.create(name=f"Test project{i}", organisation=organisation)

# Then - free projects limit should be exhausted
assert not project_permissions.has_permission(mock_request, mock_view)
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from django.conf import settings

from organisations.chargebee.metadata import ChargebeeObjMetadata
from organisations.subscriptions.constants import (
CHARGEBEE,
MAX_API_CALLS_IN_FREE_PLAN,
MAX_PROJECTS_IN_FREE_PLAN,
MAX_SEATS_IN_FREE_PLAN,
XERO,
)
Expand All @@ -20,7 +21,7 @@ def test_get_subscription_metadata_returns_default_values_if_org_does_not_have_s
# Then
assert subscription_metadata.api_calls == MAX_API_CALLS_IN_FREE_PLAN
assert subscription_metadata.seats == MAX_SEATS_IN_FREE_PLAN
assert subscription_metadata.projects == MAX_PROJECTS_IN_FREE_PLAN
assert subscription_metadata.projects == settings.MAX_PROJECTS_IN_FREE_PLAN
assert subscription_metadata.payment_source is None


Expand Down
7 changes: 4 additions & 3 deletions api/tests/unit/organisations/test_unit_organisation_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
from organisations.subscriptions.constants import (
CHARGEBEE,
MAX_API_CALLS_IN_FREE_PLAN,
MAX_PROJECTS_IN_FREE_PLAN,
MAX_SEATS_IN_FREE_PLAN,
SUBSCRIPTION_BILLING_STATUS_ACTIVE,
SUBSCRIPTION_BILLING_STATUS_DUNNING,
Expand Down Expand Up @@ -928,7 +927,9 @@ def test_get_subscription_metadata_returns_200_if_the_organisation_have_no_paid_
assert response.data == {
"chargebee_email": None,
"max_api_calls": 50000,
"max_projects": 1,
# MAX_PROJECTS_IN_FREE_PLAN is set to 10 in tests, as there are tests that needs to create more
# than 1 project within a single organisation using the default subscription
"max_projects": settings.MAX_PROJECTS_IN_FREE_PLAN,
"max_seats": 1,
"payment_source": None,
}
Expand All @@ -954,7 +955,7 @@ def test_get_subscription_metadata_returns_defaults_if_chargebee_error(
assert response.json() == {
"max_seats": MAX_SEATS_IN_FREE_PLAN,
"max_api_calls": MAX_API_CALLS_IN_FREE_PLAN,
"max_projects": MAX_PROJECTS_IN_FREE_PLAN,
"max_projects": settings.MAX_PROJECTS_IN_FREE_PLAN,
"payment_source": None,
"chargebee_email": None,
}
Expand Down

3 comments on commit f32159e

@vercel
Copy link

@vercel vercel bot commented on f32159e Nov 20, 2023

Choose a reason for hiding this comment

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

@vercel
Copy link

@vercel vercel bot commented on f32159e Nov 20, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

docs – ./docs

docs-git-main-flagsmith.vercel.app
docs-flagsmith.vercel.app
docs.bullet-train.io
docs.flagsmith.com

@vercel
Copy link

@vercel vercel bot commented on f32159e Nov 20, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.