From f32159e3fd821c6dc7bfbd50a0c3d22374f1b558 Mon Sep 17 00:00:00 2001 From: Tushar <30565750+tushar5526@users.noreply.github.com> Date: Mon, 20 Nov 2023 22:29:30 +0530 Subject: [PATCH] fix(api): validate before creating projects based on current subscription (#2869) Co-authored-by: Matthew Elwell --- api/app/settings/common.py | 2 ++ api/app/settings/test.py | 2 +- api/organisations/models.py | 3 +- api/organisations/subscriptions/constants.py | 7 +++-- api/projects/permissions.py | 15 +++++++++- api/projects/tests/test_permissions.py | 30 +++++++++++++++++-- .../test_unit_subscription_service.py | 5 ++-- .../test_unit_organisation_views.py | 7 +++-- 8 files changed, 57 insertions(+), 14 deletions(-) diff --git a/api/app/settings/common.py b/api/app/settings/common.py index 0d6c66a68db5..40f56d1b8d7d 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -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) diff --git a/api/app/settings/test.py b/api/app/settings/test.py index 0bd851a28829..50bd5effc573 100644 --- a/api/app/settings/test.py +++ b/api/app/settings/test.py @@ -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", diff --git a/api/organisations/models.py b/api/organisations/models.py index 1af730203ae3..cda5ce696dc8 100644 --- a/api/organisations/models.py +++ b/api/organisations/models.py @@ -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, @@ -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): diff --git a/api/organisations/subscriptions/constants.py b/api/organisations/subscriptions/constants.py index 9c1eef5e3e43..204ca6c488eb 100644 --- a/api/organisations/subscriptions/constants.py +++ b/api/organisations/subscriptions/constants.py @@ -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" @@ -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" diff --git a/api/projects/permissions.py b/api/projects/permissions.py index c0f170f1c139..0ffa08b7b1ea 100644 --- a/api/projects/permissions.py +++ b/api/projects/permissions.py @@ -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( diff --git a/api/projects/tests/test_permissions.py b/api/projects/tests/test_permissions.py index add8e0043301..972d9f3b18dd 100644 --- a/api/projects/tests/test_permissions.py +++ b/api/projects/tests/test_permissions.py @@ -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 ( @@ -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 @@ -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="admin@test.com") + + 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) diff --git a/api/tests/unit/organisations/subscriptions/test_unit_subscription_service.py b/api/tests/unit/organisations/subscriptions/test_unit_subscription_service.py index 57d57fd39b50..d286decd680a 100644 --- a/api/tests/unit/organisations/subscriptions/test_unit_subscription_service.py +++ b/api/tests/unit/organisations/subscriptions/test_unit_subscription_service.py @@ -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, ) @@ -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 diff --git a/api/tests/unit/organisations/test_unit_organisation_views.py b/api/tests/unit/organisations/test_unit_organisation_views.py index 9d7b2b4554bc..8f7aa977d82e 100644 --- a/api/tests/unit/organisations/test_unit_organisation_views.py +++ b/api/tests/unit/organisations/test_unit_organisation_views.py @@ -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, @@ -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, } @@ -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, }