From 02e9693c32d548e89182401847e36304c4a9d0f0 Mon Sep 17 00:00:00 2001 From: niklastheman Date: Mon, 3 Apr 2023 09:21:09 +0200 Subject: [PATCH] Bug/SK-414 | Limit apps per project should not be based on user (#115) --- helpers/get_apps_limit_per_user.py | 22 --------- helpers/get_apps_per_project_limit.py | 26 +++++++++++ models.py | 5 +-- tests/test_create_app_view.py | 64 ++++++++++++++++++++++++--- tests/test_helpers.py | 22 ++++----- 5 files changed, 96 insertions(+), 43 deletions(-) delete mode 100644 helpers/get_apps_limit_per_user.py create mode 100644 helpers/get_apps_per_project_limit.py diff --git a/helpers/get_apps_limit_per_user.py b/helpers/get_apps_limit_per_user.py deleted file mode 100644 index 2e2b6bf07..000000000 --- a/helpers/get_apps_limit_per_user.py +++ /dev/null @@ -1,22 +0,0 @@ -from django.conf import settings - - -def get_apps_limit_per_user(slug): - """get_apps_limit_per_user - - Args: - slug (App.slug): slug for the app type - - Returns: - Integer or None: returns the limit or None if not set - """ - try: - apps_per_user_limit = ( - settings.APPS_PER_USER_LIMIT - if settings.APPS_PER_USER_LIMIT is not None - else {} - ) - except Exception: - apps_per_user_limit = {} - - return apps_per_user_limit[slug] if slug in apps_per_user_limit else None diff --git a/helpers/get_apps_per_project_limit.py b/helpers/get_apps_per_project_limit.py new file mode 100644 index 000000000..0d073c400 --- /dev/null +++ b/helpers/get_apps_per_project_limit.py @@ -0,0 +1,26 @@ +from django.conf import settings + + +def get_apps_per_project_limit(slug): + """get_apps_per_project_limit + + Args: + slug (App.slug): slug for the app type + + Returns: + Integer or None: returns the limit or None if not set + """ + try: + apps_per_project_limit = ( + settings.APPS_PER_PROJECT_LIMIT + if settings.APPS_PER_PROJECT_LIMIT is not None + else {} + ) + except Exception: + apps_per_project_limit = {} + + return ( + apps_per_project_limit[slug] + if slug in apps_per_project_limit + else None + ) diff --git a/models.py b/models.py index 4f4ee1845..eac7ef6ea 100644 --- a/models.py +++ b/models.py @@ -6,7 +6,7 @@ from guardian.shortcuts import assign_perm, remove_perm from tagulous.models import TagField -from apps.helpers.get_apps_limit_per_user import get_apps_limit_per_user +from apps.helpers.get_apps_per_project_limit import get_apps_per_project_limit class AppCategories(models.Model): @@ -58,10 +58,9 @@ def __str__(self): class AppInstanceManager(models.Manager): def user_can_create(self, user, project, app_slug): - limit = get_apps_limit_per_user(app_slug) + limit = get_apps_per_project_limit(app_slug) num_of_app_instances = self.filter( - Q(owner=user), ~Q(state="Deleted"), app__slug=app_slug, project=project, diff --git a/tests/test_create_app_view.py b/tests/test_create_app_view.py index 42ee88972..22909329d 100644 --- a/tests/test_create_app_view.py +++ b/tests/test_create_app_view.py @@ -46,7 +46,7 @@ def get_data(self, user=None): return project @override_settings( - APPS_PER_USER_LIMIT={ + APPS_PER_PROJECT_LIMIT={ "jupyter-lab": 1, } ) @@ -68,7 +68,7 @@ def test_has_permission(self): self.assertEqual(response.status_code, 200) - @override_settings(APPS_PER_USER_LIMIT={"jupyter-lab": 0}) + @override_settings(APPS_PER_PROJECT_LIMIT={"jupyter-lab": 0}) def test_has_reached_app_limit(self): c = Client() @@ -93,7 +93,7 @@ def test_has_reached_app_limit(self): self.assertEqual(response.status_code, 403) - @override_settings(APPS_PER_USER_LIMIT={"jupyter-lab": 1}) + @override_settings(APPS_PER_PROJECT_LIMIT={"jupyter-lab": 1}) def test_missing_access_to_project(self): c = Client() @@ -121,7 +121,7 @@ def test_missing_access_to_project(self): self.assertEqual(response.status_code, 403) @override_settings( - APPS_PER_USER_LIMIT={ + APPS_PER_PROJECT_LIMIT={ "jupyter-lab": None, } ) @@ -143,7 +143,7 @@ def test_has_permission_when_none(self): self.assertEqual(response.status_code, 200) - @override_settings(APPS_PER_USER_LIMIT={}) + @override_settings(APPS_PER_PROJECT_LIMIT={}) def test_has_permission_when_not_specified(self): c = Client() @@ -163,7 +163,7 @@ def test_has_permission_when_not_specified(self): self.assertEqual(response.status_code, 200) @override_settings( - APPS_PER_USER_LIMIT={ + APPS_PER_PROJECT_LIMIT={ "jupyter-lab": 1, } ) @@ -207,7 +207,7 @@ def test_has_permission_project_level(self): self.assertEqual(response.status_code, 403) - @override_settings(APPS_PER_USER_LIMIT={"jupyter-lab": 0}) + @override_settings(APPS_PER_PROJECT_LIMIT={"jupyter-lab": 0}) def test_permission_overrides_reached_app_limit(self): c = Client() @@ -249,3 +249,53 @@ def test_permission_overrides_reached_app_limit(self): ) self.assertEqual(response.status_code, 200) + + @override_settings(APPS_PER_PROJECT_LIMIT={"jupyter-lab": 1}) + def test_app_limit_is_per_project(self): + c = Client() + + project = self.get_data() + + response = c.post( + "/accounts/login/", {"username": "foo1", "password": "bar"} + ) + response.status_code + + self.assertEqual(response.status_code, 302) + + response = c.get( + f"/{self.user.username}/{project.slug}/apps/create/jupyter-lab" + ) + + self.assertEqual(response.status_code, 200) + + user2 = User.objects.create_user("foo123", "foo123@test.com", "bar123") + + project.authorized.add(user2) + project.save() + + response = c.get( + f"/{user2.username}/{project.slug}/apps/create/jupyter-lab" + ) + + self.assertEqual(response.status_code, 200) + + _ = AppInstance.objects.create( + access="private", + owner=self.user, + name="test_app_instance_private", + app=self.app, + project=project, + ) + + response = c.get( + f"/{self.user.username}/{project.slug}/apps/create/jupyter-lab" + ) + + self.assertEqual(response.status_code, 403) + + response = c.get( + f"/{user2.username}/{project.slug}/apps/create/jupyter-lab" + ) + + self.assertEqual(response.status_code, 403) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 2cec8b296..12752b3f2 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,11 +1,11 @@ from django.test import TestCase, override_settings -from apps.helpers.get_apps_limit_per_user import get_apps_limit_per_user +from apps.helpers.get_apps_per_project_limit import get_apps_per_project_limit class HelpersTestCase(TestCase): @override_settings( - APPS_PER_USER_LIMIT={ + APPS_PER_PROJECT_LIMIT={ "vscode": 1, "volumeK8s": 2, "pytorch-serve": 0, @@ -13,27 +13,27 @@ class HelpersTestCase(TestCase): } ) def test_get_apps_limit_per_user(self): - result = get_apps_limit_per_user("vscode") + result = get_apps_per_project_limit("vscode") self.assertEqual(1, result) - result = get_apps_limit_per_user("volumeK8s") + result = get_apps_per_project_limit("volumeK8s") self.assertEqual(2, result) - result = get_apps_limit_per_user("pytorch-serve") + result = get_apps_per_project_limit("pytorch-serve") self.assertEqual(0, result) - result = get_apps_limit_per_user("tensorflow-serve") + result = get_apps_per_project_limit("tensorflow-serve") self.assertIsNone(result) - result = get_apps_limit_per_user("no-app") + result = get_apps_per_project_limit("no-app") self.assertIsNone(result) - @override_settings(APPS_PER_USER_LIMIT={}) + @override_settings(APPS_PER_PROJECT_LIMIT={}) def test_get_apps_limit_per_user_handle_empty(self): - result = get_apps_limit_per_user("vscode") + result = get_apps_per_project_limit("vscode") self.assertIsNone(result) - @override_settings(APPS_PER_USER_LIMIT=None) + @override_settings(APPS_PER_PROJECT_LIMIT=None) def test_get_apps_limit_per_user_handle_none(self): - result = get_apps_limit_per_user("vscode") + result = get_apps_per_project_limit("vscode") self.assertIsNone(result)