From 9c6ddf48466255d72cdf6ea6bfa958330943dcb2 Mon Sep 17 00:00:00 2001 From: akochari Date: Mon, 21 Oct 2024 15:14:44 +0200 Subject: [PATCH 01/28] bugfix SS-1191 - environment and flavor deletion --- .../0017_alter_streamlitinstance_port.py | 17 ++++++++ projects/views.py | 41 +++++++++++++++++-- templates/projects/settings.html | 14 +++---- 3 files changed, 61 insertions(+), 11 deletions(-) create mode 100644 apps/migrations/0017_alter_streamlitinstance_port.py diff --git a/apps/migrations/0017_alter_streamlitinstance_port.py b/apps/migrations/0017_alter_streamlitinstance_port.py new file mode 100644 index 00000000..bdb3c8a7 --- /dev/null +++ b/apps/migrations/0017_alter_streamlitinstance_port.py @@ -0,0 +1,17 @@ +# Generated by Django 5.1.1 on 2024-10-21 13:12 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("apps", "0016_tagulous_streamlitinstance_tags_streamlitinstance"), + ] + + operations = [ + migrations.AlterField( + model_name="streamlitinstance", + name="port", + field=models.IntegerField(default=8000), + ), + ] diff --git a/projects/views.py b/projects/views.py index 170a8a76..f185e291 100644 --- a/projects/views.py +++ b/projects/views.py @@ -5,6 +5,7 @@ import requests as r from django.apps import apps from django.conf import settings as django_settings +from django.contrib import messages from django.contrib.auth import get_user_model from django.contrib.auth.decorators import login_required from django.core.exceptions import FieldDoesNotExist @@ -102,7 +103,10 @@ def settings(request, project_slug): ) environments = Environment.objects.filter(project=project) - apps = Apps.objects.all().order_by("slug", "-revision").distinct("slug") + # apps = Apps.objects.all().order_by("slug", "-revision").distinct("slug") + apps_with_environment_option = ( + Apps.objects.filter(environment__isnull=False).order_by("slug", "-revision").distinct("slug") + ) flavors = Flavor.objects.filter(project=project) @@ -207,7 +211,23 @@ def delete_environment(request, project_slug): pk = request.POST.get("environment_pk") # TODO: Check that the user has permission to delete this environment. environment = Environment.objects.get(pk=pk, project=project) - environment.delete() + + can_environment_be_deleted = True + for model_class in APP_REGISTRY.iter_orm_models(): + if hasattr(model_class, "environment"): + queryset = model_class.objects.filter(environment=environment) + logger.error(f"{queryset}") + if queryset: + messages.error( + request, + "Environment cannot be deleted because it is currently used by at least one app \ + (can also be a deleted app).", + ) + can_environment_be_deleted = False + break + + if can_environment_be_deleted: + environment.delete() return HttpResponseRedirect( reverse( @@ -259,7 +279,22 @@ def delete_flavor(request, project_slug): pk = request.POST.get("flavor_pk") # TODO: Check that the user has permission to delete this flavor. flavor = Flavor.objects.get(pk=pk, project=project) - flavor.delete() + + can_flavor_be_deleted = True + for model_class in APP_REGISTRY.iter_orm_models(): + if hasattr(model_class, "flavor"): + queryset = model_class.objects.filter(flavor=flavor) + if queryset: + messages.error( + request, + "Flavor cannot be deleted because it is currently used by at least one app \ + (can also be a deleted app).", + ) + can_flavor_be_deleted = False + break + + if can_flavor_be_deleted: + flavor.delete() return HttpResponseRedirect( reverse( diff --git a/templates/projects/settings.html b/templates/projects/settings.html index 46067d0c..f58fad17 100644 --- a/templates/projects/settings.html +++ b/templates/projects/settings.html @@ -54,6 +54,8 @@

Project settings

+ {% include 'common/flash_messages.html' %} +
@@ -147,7 +149,6 @@
Flavors
-
@@ -187,23 +188,23 @@
Environments
+ placeholder="Lab X" class="form-control" />
+ placeholder="docker.io" value="" class="form-control" />
+ placeholder="user/image:tag" class="form-control" />
@@ -215,9 +216,6 @@
Environments
- - - {% endif %} {% if request.user.pk == project.owner.pk or request.user.is_superuser %}
From 91bc0b64352137a0c85b8e46be944c06a7e23f15 Mon Sep 17 00:00:00 2001 From: Arnold Kochari Date: Tue, 22 Oct 2024 09:35:42 +0200 Subject: [PATCH 02/28] better naming in the loop Co-authored-by: Nikita Churikov <8545082+churnikov@users.noreply.github.com> --- projects/views.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/projects/views.py b/projects/views.py index f185e291..2c28a959 100644 --- a/projects/views.py +++ b/projects/views.py @@ -281,9 +281,9 @@ def delete_flavor(request, project_slug): flavor = Flavor.objects.get(pk=pk, project=project) can_flavor_be_deleted = True - for model_class in APP_REGISTRY.iter_orm_models(): - if hasattr(model_class, "flavor"): - queryset = model_class.objects.filter(flavor=flavor) + for app_orm in APP_REGISTRY.iter_orm_models(): + if hasattr(app_orm, "flavor"): + queryset = app_orm.objects.filter(flavor=flavor) if queryset: messages.error( request, From 4d0eaeb68fa00deac1eb6f85540d82170fc8bc67 Mon Sep 17 00:00:00 2001 From: Arnold Kochari Date: Tue, 22 Oct 2024 09:36:50 +0200 Subject: [PATCH 03/28] better naming in the loop Co-authored-by: Nikita Churikov <8545082+churnikov@users.noreply.github.com> --- projects/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/projects/views.py b/projects/views.py index 2c28a959..eb94dafb 100644 --- a/projects/views.py +++ b/projects/views.py @@ -213,9 +213,9 @@ def delete_environment(request, project_slug): environment = Environment.objects.get(pk=pk, project=project) can_environment_be_deleted = True - for model_class in APP_REGISTRY.iter_orm_models(): + for app_orm in APP_REGISTRY.iter_orm_models(): if hasattr(model_class, "environment"): - queryset = model_class.objects.filter(environment=environment) + queryset = app_orm.objects.filter(environment=environment) logger.error(f"{queryset}") if queryset: messages.error( From 83d860c6d8ba1435bcaf227d6ea78d07402d4256 Mon Sep 17 00:00:00 2001 From: Arnold Kochari Date: Tue, 22 Oct 2024 09:38:15 +0200 Subject: [PATCH 04/28] remove commented out line Co-authored-by: Nikita Churikov <8545082+churnikov@users.noreply.github.com> --- projects/views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/projects/views.py b/projects/views.py index eb94dafb..8e560feb 100644 --- a/projects/views.py +++ b/projects/views.py @@ -103,7 +103,6 @@ def settings(request, project_slug): ) environments = Environment.objects.filter(project=project) - # apps = Apps.objects.all().order_by("slug", "-revision").distinct("slug") apps_with_environment_option = ( Apps.objects.filter(environment__isnull=False).order_by("slug", "-revision").distinct("slug") ) From 9ca5daf697343bf322abf1bdb9f7b9bc3fe66f7c Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 22 Oct 2024 16:06:41 +0200 Subject: [PATCH 05/28] remove unneccesary logging statement --- projects/views.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/projects/views.py b/projects/views.py index 8e560feb..205f4f12 100644 --- a/projects/views.py +++ b/projects/views.py @@ -213,9 +213,8 @@ def delete_environment(request, project_slug): can_environment_be_deleted = True for app_orm in APP_REGISTRY.iter_orm_models(): - if hasattr(model_class, "environment"): + if hasattr(app_orm, "environment"): queryset = app_orm.objects.filter(environment=environment) - logger.error(f"{queryset}") if queryset: messages.error( request, From c9418bc53ae9b8ec526e58c97ed3f46729355a10 Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 19 Nov 2024 10:52:52 +0100 Subject: [PATCH 06/28] allow instructions for local docker for gradio and streamlit apps (small correction of previous PR) --- templates/common/app_card.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/common/app_card.html b/templates/common/app_card.html index 5b2c0f9f..62b90081 100644 --- a/templates/common/app_card.html +++ b/templates/common/app_card.html @@ -78,7 +78,7 @@
{{ app.name }}
{% else %} Open {% endif %} - {% if app.app.slug in 'shinyapp,shinyproxyapp,dashapp,customapp' %} + {% if app.app.slug in 'shinyapp,shinyproxyapp,dashapp,customapp,gradio,streamlit' %} {% if app.pvc == None %} From 576069afc0798ed80bfcf02f64516839592d05b1 Mon Sep 17 00:00:00 2001 From: akochari Date: Thu, 21 Nov 2024 11:36:24 +0100 Subject: [PATCH 07/28] prohibit deleting environments when they are in use by an app --- apps/models/app_types/jupyter.py | 2 +- apps/models/app_types/rstudio.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/models/app_types/jupyter.py b/apps/models/app_types/jupyter.py index bc21b8de..5b0d99ef 100644 --- a/apps/models/app_types/jupyter.py +++ b/apps/models/app_types/jupyter.py @@ -16,7 +16,7 @@ class JupyterInstance(BaseAppInstance): ) volume = models.ManyToManyField("VolumeInstance", blank=True) access = models.CharField(max_length=20, default="project", choices=ACCESS_TYPES) - environment: Environment = models.ForeignKey(Environment, on_delete=models.DO_NOTHING, null=True, blank=True) + environment: Environment = models.ForeignKey(Environment, on_delete=models.RESTRICT, null=True, blank=True) def get_k8s_values(self): k8s_values = super().get_k8s_values() diff --git a/apps/models/app_types/rstudio.py b/apps/models/app_types/rstudio.py index 58b62296..6d01929d 100644 --- a/apps/models/app_types/rstudio.py +++ b/apps/models/app_types/rstudio.py @@ -16,7 +16,7 @@ class RStudioInstance(BaseAppInstance): ) volume = models.ManyToManyField("VolumeInstance", blank=True) access = models.CharField(max_length=20, default="project", choices=ACCESS_TYPES) - environment: Environment = models.ForeignKey(Environment, on_delete=models.DO_NOTHING, null=True, blank=True) + environment: Environment = models.ForeignKey(Environment, on_delete=models.RESTRICT, null=True, blank=True) def get_k8s_values(self): k8s_values = super().get_k8s_values() From e0233566907730592cbc8a387412eb7f002c4173 Mon Sep 17 00:00:00 2001 From: akochari Date: Thu, 21 Nov 2024 11:59:49 +0100 Subject: [PATCH 08/28] general function for checking if flavor or env can be deleted --- projects/views.py | 60 ++++++++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/projects/views.py b/projects/views.py index 97362639..ad48ef26 100644 --- a/projects/views.py +++ b/projects/views.py @@ -174,6 +174,28 @@ def change_description(request, project_slug): ) +def can_model_instance_be_deleted(request, model, field_name, instance): + """ + Check if a model instance can be deleted by ensuring no app in APP_REGISTRY + references it via the specified field. + + Args: + request: The HTTP request object or None if called outside of request context. + model: The model class (e.g., Environment or Flavor). + field_name: The name of the field to check in APP_REGISTRY models. + instance: The instance to check. + + Returns: + bool: True if the instance can be safely deleted, False otherwise. + """ + for app_orm in APP_REGISTRY.iter_orm_models(): + if hasattr(app_orm, field_name): + queryset = app_orm.objects.filter(**{field_name: instance}) + if queryset.exists(): + return False + return True + + @login_required @permission_required_or_403("can_view_project", (Project, "slug", "project_slug")) def create_environment(request, project_slug): @@ -211,21 +233,16 @@ def delete_environment(request, project_slug): # TODO: Check that the user has permission to delete this environment. environment = Environment.objects.get(pk=pk, project=project) - can_environment_be_deleted = True - for app_orm in APP_REGISTRY.iter_orm_models(): - if hasattr(app_orm, "environment"): - queryset = app_orm.objects.filter(environment=environment) - if queryset: - messages.error( - request, - "Environment cannot be deleted because it is currently used by at least one app \ - (can also be a deleted app).", - ) - can_environment_be_deleted = False - break + can_environment_be_deleted = can_model_instance_be_deleted(request, Environment, "environment", pk) if can_environment_be_deleted: environment.delete() + else: + messages.error( + request, + "Environment cannot be deleted because it is currently used by at least one app \ + (can also be a deleted app).", + ) return HttpResponseRedirect( reverse( @@ -278,21 +295,16 @@ def delete_flavor(request, project_slug): # TODO: Check that the user has permission to delete this flavor. flavor = Flavor.objects.get(pk=pk, project=project) - can_flavor_be_deleted = True - for app_orm in APP_REGISTRY.iter_orm_models(): - if hasattr(app_orm, "flavor"): - queryset = app_orm.objects.filter(flavor=flavor) - if queryset: - messages.error( - request, - "Flavor cannot be deleted because it is currently used by at least one app \ - (can also be a deleted app).", - ) - can_flavor_be_deleted = False - break + can_flavor_be_deleted = can_model_instance_be_deleted(request, Flavor, "flavor", pk) if can_flavor_be_deleted: flavor.delete() + else: + messages.error( + request, + "Flavor cannot be deleted because it is currently used by at least one app \ + (can also be a deleted app).", + ) return HttpResponseRedirect( reverse( From 9a93db47c15b11cda429b3011186771e5a745ca2 Mon Sep 17 00:00:00 2001 From: akochari Date: Thu, 21 Nov 2024 16:01:36 +0100 Subject: [PATCH 09/28] add e2e tests for flavor and environment deletion functionality --- cypress.config.js | 2 + .../test-superuser-functionality.cy.js | 113 ++++++++++-------- 2 files changed, 64 insertions(+), 51 deletions(-) diff --git a/cypress.config.js b/cypress.config.js index b7205d7f..ae2816ea 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -10,6 +10,8 @@ module.exports = defineConfig({ e2e: { baseUrl: 'http://studio.127.0.0.1.nip.io:8080', //baseUrl: 'https://serve-dev.scilifelab.se', + experimentalSessionAndOrigin: true, + setupNodeEvents(on, config) { // implement node event listeners here diff --git a/cypress/e2e/ui-tests/test-superuser-functionality.cy.js b/cypress/e2e/ui-tests/test-superuser-functionality.cy.js index a30a6170..efd90dc0 100644 --- a/cypress/e2e/ui-tests/test-superuser-functionality.cy.js +++ b/cypress/e2e/ui-tests/test-superuser-functionality.cy.js @@ -65,7 +65,6 @@ describe("Test superuser access", () => { cy.get('input[name=name]').type(project_name) cy.get('textarea[name=description]').type(project_description) cy.get("input[name=save]").contains('Create project').click() - //cy.wait(5000) // sometimes it takes a while to create a project. Not needed because of cypress retryability. cy.get('h3', {timeout: longCmdTimeoutMs}).should('contain', project_name) cy.get('.card-text').should('contain', project_description) @@ -139,13 +138,11 @@ describe("Test superuser access", () => { cy.get('#submit-id-submit').contains('Submit').click() cy.get('tr:contains("' + private_app_name_2 + '")').should('exist') // regular user's private app now has a different name - //cy.wait(10000) // Not needed because of the retryability built into cypress. cy.get('tr:contains("' + private_app_name_2 + '")', {timeout: longCmdTimeoutMs}).find('span', {timeout: longCmdTimeoutMs}).should('contain', 'Running') // add this because to make sure the app is running before deleting otherwise it gives an error, cy.logf("Deleting a regular user's private app", Cypress.currentTest) cy.get('tr:contains("' + private_app_name_2 + '")').find('i.bi-three-dots-vertical').click() cy.get('tr:contains("' + private_app_name_2 + '")').find('a.confirm-delete').click() cy.get('button').contains('Delete').click() - //cy.wait(5000) // Not needed because of the retryability built into cypress. cy.get('tr:contains("' + private_app_name_2 + '")', {timeout: longCmdTimeoutMs}).find('span', {timeout: longCmdTimeoutMs}).should('contain', 'Deleted') cy.logf("Deleting a regular user's project", Cypress.currentTest) @@ -163,7 +160,9 @@ describe("Test superuser access", () => { // Names of objects to create const project_name = "e2e-proj-flavor-env-test" const new_flavor_name = "4 CPU, 8 GB RAM" + const new_flavor_name_unused = "Unused flavor" const new_environment_name = "e2e test environment" + const new_environment_name_unused = "Unused environment" cy.logf("Logging in as a regular user and creating a project", Cypress.currentTest) cy.fixture('users.json').then(function (data) { @@ -175,7 +174,6 @@ describe("Test superuser access", () => { cy.get("a").contains('Create').first().click() cy.get('input[name=name]').type(project_name) cy.get("input[name=save]").contains('Create project').click() - //cy.wait(5000) // sometimes it takes a while to create a project. Not needed because of cypress retryability. cy.get('h3').should('contain', project_name) Cypress.session.clearAllSavedSessions() @@ -185,7 +183,7 @@ describe("Test superuser access", () => { cy.loginViaUI(users.superuser.email, users.superuser.password) }) - cy.logf("Creating a new flavor in the regular user's project", Cypress.currentTest) + cy.logf("Creating new flavors in the regular user's project", Cypress.currentTest) cy.visit("/projects/") cy.contains('.card-title', project_name).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click() cy.get('[data-cy="settings"]').click() @@ -197,7 +195,15 @@ describe("Test superuser access", () => { cy.get('input[name="mem_lim"]').clear().type("8Gi") cy.get('button').contains("Create flavor").click() - cy.logf("Creating a new Jupyter Lab environment in the regular user's project", Cypress.currentTest) + cy.get('.list-group').find('a').contains('Flavors').click() + cy.get('input[name="flavor_name"]').type(new_flavor_name_unused) + cy.get('input[name="cpu_req"]').clear().type("500m") + cy.get('input[name="cpu_lim"]').clear().type("4000m") + cy.get('input[name="mem_req"]').clear().type("2Gi") + cy.get('input[name="mem_lim"]').clear().type("8Gi") + cy.get('button').contains("Create flavor").click() + + cy.logf("Creating new Jupyter Lab environments in the regular user's project", Cypress.currentTest) cy.visit("/projects/") cy.contains('.card-title', project_name).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click() cy.get('[data-cy="settings"]').click() @@ -208,6 +214,13 @@ describe("Test superuser access", () => { cy.get('#environment_app').select('Jupyter Lab') cy.get('button').contains("Create environment").click() + cy.get('.list-group').find('a').contains('Environments').click() + cy.get('input[name="environment_name"]').type(new_environment_name_unused) + cy.get('input[name="environment_repository"]').clear().type("docker.io") + cy.get('input[name="environment_image"]').clear().type("jupyter/minimal-notebook:latest") + cy.get('#environment_app').select('Jupyter Lab') + cy.get('button').contains("Create environment").click() + Cypress.session.clearAllSavedSessions() cy.logf("Logging back in as a regular user and using the new flavor and environment", Cypress.currentTest) const createResources = Cypress.env('create_resources'); @@ -286,6 +299,48 @@ describe("Test superuser access", () => { cy.get('tr:contains("' + app_name_env + '")').find('a').contains('Settings').click() cy.get('#id_environment').find(':selected').should('contain', new_environment_name) + cy.logf("Checking that admin cannot delete flavor or environment currently in use", Cypress.currentTest) + Cypress.session.clearAllSavedSessions() + cy.logf("Logging in as a superuser", Cypress.currentTest) + cy.fixture('users.json').then(function (data) { + users = data + cy.loginViaUI(users.superuser.email, users.superuser.password) + }) + + cy.logf("Deleting a flavor that was used", Cypress.currentTest) + cy.visit("/projects/") + cy.contains('.card-title', project_name).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click() + cy.get('[data-cy="settings"]').click() + cy.logf("Deleting a flavor that was used", Cypress.currentTest) + cy.get('.list-group').find('a').contains('Flavors').click() + cy.get('#flavor_pk').select(new_flavor_name) + cy.get('button').contains("Delete flavor").click() + cy.get('div.alert-danger').contains('Flavor cannot be deleted').should('exist') + + cy.logf("Deleting a flavor that was not used", Cypress.currentTest) + cy.logf("Trying flavor deletion", Cypress.currentTest) + cy.get('.list-group').find('a').contains('Flavors').click() + cy.get('#flavor_pk').select(new_flavor_name_unused) + cy.get('button').contains("Delete flavor").click() + cy.get('.list-group').find('a').contains('Flavors').click() + cy.get('#flavor_pk').contains(new_flavor_name_unused).should("not.exist") + + cy.logf("Deleting an environment that was used", Cypress.currentTest) + cy.visit("/projects/") + cy.contains('.card-title', project_name).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click() + cy.get('[data-cy="settings"]').click() + cy.logf("Deleting a flavor that was used", Cypress.currentTest) + cy.get('.list-group').find('a').contains('Environments').click() + cy.get('#environment_pk').select(new_environment_name) + cy.get('button').contains("Delete environment").click() + cy.get('div.alert-danger').contains('Environment cannot be deleted').should('exist') + + cy.logf("Deletion of an environment that was not used", Cypress.currentTest) + cy.get('.list-group').find('a').contains('Environments').click() + cy.get('#environment_pk').select(new_environment_name_unused) + cy.get('button').contains("Delete environment").click() + cy.get('.list-group').find('a').contains('Environments').click() + cy.get('#environment_pk').contains(new_environment_name_unused).should("not.exist") } else { cy.logf('Skipped because create_resources is not true', Cypress.currentTest); @@ -302,55 +357,11 @@ describe("Test superuser access", () => { }) }) - it.skip("can create a persistent volume", () => { - // This test is not used, since creating PVCs like this is not the correct way any more. - // The correct way is to create a volume in the admin panel. - - // Names of objects to create - const project_name_pvc = "e2e-superuser-pvc-test" - const volume_name = "e2e-project-vol" - - cy.logf("Creating a blank project", Cypress.currentTest) - cy.createBlankProject(project_name_pvc) - - cy.logf("Creating a persistent volume", Cypress.currentTest) - cy.visit("/projects/") - cy.contains('.card-title', project_name_pvc).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click() - - cy.get('div.card-body:contains("Persistent Volume")').find('a:contains("Create")').click() - cy.get('#id_name').type(volume_name) - cy.get('#submit-id-submit').contains('Submit').click() - cy.get('tr:contains("' + volume_name + '")').should('exist') // persistent volume has been created - - // This does not work in our CI. Disabled for now, needs to be enabled for runs against an instance of Serve running on the cluster - /* - cy.get('tr:contains("' + volume_name + '")').find('span').should('contain', 'Installed') // confirm the volume is working - - cy.log("Deleting the created persistent volume") - cy.get('tr:contains("' + volume_name + '")').find('i.bi-three-dots-vertical').click() - cy.get('tr:contains("' + volume_name + '")').find('a.confirm-delete').click() - cy.get('button').contains('Delete').click() - cy.get('tr:contains("' + volume_name + '")').find('span').should('contain', 'Deleted') // confirm the volume has been deleted - */ - - cy.logf("Deleting the created project", Cypress.currentTest) - cy.visit("/projects/") - cy.contains('.card-title', project_name_pvc).parents('.card-body').siblings('.card-footer').find('.confirm-delete').click() - .then((href) => { - cy.get('div#modalConfirmDelete').should('have.css', 'display', 'block') - cy.get("h1#modalConfirmDeleteLabel").then(function($elem) { - cy.get('div#modalConfirmDeleteFooter').find('button').contains('Delete').click() - cy.contains(project_name_pvc).should('not.exist') // confirm the project has been deleted - }) - }) - - }) - it("can bypass N projects limit", () => { // Names of projects to create const project_name = "e2e-superuser-proj-limits-test" - cy.logf("Create 10 projects (current limit for regular users)", Cypress.currentTest) + cy.logf("Create 10 projects (current limit for regular users) with the same name (currently not possible for regular users to use the same name)", Cypress.currentTest) Cypress._.times(10, () => { // better to write this out rather than use the createBlankProject command because then we can do a 5000 ms pause only once cy.visit("/projects/") From c1897ddf58602a9a20ab8917158070f545bd84a8 Mon Sep 17 00:00:00 2001 From: akochari Date: Thu, 21 Nov 2024 16:04:14 +0100 Subject: [PATCH 10/28] precommit fix --- cypress.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress.config.js b/cypress.config.js index ae2816ea..3cea1855 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -11,7 +11,7 @@ module.exports = defineConfig({ baseUrl: 'http://studio.127.0.0.1.nip.io:8080', //baseUrl: 'https://serve-dev.scilifelab.se', experimentalSessionAndOrigin: true, - + setupNodeEvents(on, config) { // implement node event listeners here From d4c39febc1300caf4cd06ba046b64f6cb2cd8d20 Mon Sep 17 00:00:00 2001 From: akochari Date: Fri, 22 Nov 2024 07:32:46 +0100 Subject: [PATCH 11/28] manual logout and login where cypress doesn't want to get back to a session --- cypress.config.js | 2 -- .../test-superuser-functionality.cy.js | 22 +++++++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cypress.config.js b/cypress.config.js index 3cea1855..b7205d7f 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -10,8 +10,6 @@ module.exports = defineConfig({ e2e: { baseUrl: 'http://studio.127.0.0.1.nip.io:8080', //baseUrl: 'https://serve-dev.scilifelab.se', - experimentalSessionAndOrigin: true, - setupNodeEvents(on, config) { // implement node event listeners here diff --git a/cypress/e2e/ui-tests/test-superuser-functionality.cy.js b/cypress/e2e/ui-tests/test-superuser-functionality.cy.js index efd90dc0..23081365 100644 --- a/cypress/e2e/ui-tests/test-superuser-functionality.cy.js +++ b/cypress/e2e/ui-tests/test-superuser-functionality.cy.js @@ -221,12 +221,12 @@ describe("Test superuser access", () => { cy.get('#environment_app').select('Jupyter Lab') cy.get('button').contains("Create environment").click() - Cypress.session.clearAllSavedSessions() - cy.logf("Logging back in as a regular user and using the new flavor and environment", Cypress.currentTest) const createResources = Cypress.env('create_resources'); if (createResources === true) { + Cypress.session.clearAllSavedSessions() + cy.logf("Logging back in as a regular user and using the new flavor and environment", Cypress.currentTest) cy.fixture('users.json').then(function (data) { users = data cy.loginViaUI(users.superuser_testuser.email, users.superuser_testuser.password) @@ -300,14 +300,22 @@ describe("Test superuser access", () => { cy.get('#id_environment').find(':selected').should('contain', new_environment_name) cy.logf("Checking that admin cannot delete flavor or environment currently in use", Cypress.currentTest) - Cypress.session.clearAllSavedSessions() cy.logf("Logging in as a superuser", Cypress.currentTest) + // I do this logout and login manually (rather than using Cypress sessions) because Cypress + // refused to do one more session for this user here for some reason + cy.get('button.btn-profile').contains("Profile").click() + cy.get('li.btn-group').find('button').contains("Sign out").click() + cy.get("title").should("have.text", "Logout | SciLifeLab Serve (beta)") cy.fixture('users.json').then(function (data) { users = data - cy.loginViaUI(users.superuser.email, users.superuser.password) + cy.visit('/accounts/login/') + cy.get('input[name=username]').type(users.superuser.email) + cy.get('input[name=password]').type(`${users.superuser.password}{enter}`, { log: false }) + cy.url().should('include', '/projects') + cy.get('h3').should('contain', 'My projects') }) - cy.logf("Deleting a flavor that was used", Cypress.currentTest) + cy.logf("Trying to delete a flavor that was used", Cypress.currentTest) cy.visit("/projects/") cy.contains('.card-title', project_name).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click() cy.get('[data-cy="settings"]').click() @@ -325,7 +333,7 @@ describe("Test superuser access", () => { cy.get('.list-group').find('a').contains('Flavors').click() cy.get('#flavor_pk').contains(new_flavor_name_unused).should("not.exist") - cy.logf("Deleting an environment that was used", Cypress.currentTest) + cy.logf("Trying to delete an environment that was used", Cypress.currentTest) cy.visit("/projects/") cy.contains('.card-title', project_name).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click() cy.get('[data-cy="settings"]').click() @@ -335,7 +343,7 @@ describe("Test superuser access", () => { cy.get('button').contains("Delete environment").click() cy.get('div.alert-danger').contains('Environment cannot be deleted').should('exist') - cy.logf("Deletion of an environment that was not used", Cypress.currentTest) + cy.logf("Deleting an environment that was not used", Cypress.currentTest) cy.get('.list-group').find('a').contains('Environments').click() cy.get('#environment_pk').select(new_environment_name_unused) cy.get('button').contains("Delete environment").click() From 0fc16298b4cf92e75dfe274f7a2c035062e37ef9 Mon Sep 17 00:00:00 2001 From: akochari Date: Fri, 22 Nov 2024 07:33:16 +0100 Subject: [PATCH 12/28] migration for changes to environment field --- ...er_jupyterinstance_environment_and_more.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 apps/migrations/0019_alter_jupyterinstance_environment_and_more.py diff --git a/apps/migrations/0019_alter_jupyterinstance_environment_and_more.py b/apps/migrations/0019_alter_jupyterinstance_environment_and_more.py new file mode 100644 index 00000000..89e61eba --- /dev/null +++ b/apps/migrations/0019_alter_jupyterinstance_environment_and_more.py @@ -0,0 +1,28 @@ +# Generated by Django 5.1.1 on 2024-11-21 15:08 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("apps", "0018_customappinstance_default_url_subpath"), + ("projects", "0008_project_deleted_on"), + ] + + operations = [ + migrations.AlterField( + model_name="jupyterinstance", + name="environment", + field=models.ForeignKey( + blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, to="projects.environment" + ), + ), + migrations.AlterField( + model_name="rstudioinstance", + name="environment", + field=models.ForeignKey( + blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, to="projects.environment" + ), + ), + ] From fdcc6b50ef7e4593e67f3ee41130f632ae04c6ce Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 26 Nov 2024 10:35:07 +0100 Subject: [PATCH 13/28] remove unused bits of code --- .../test_grant_access_to_project_view.py | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/projects/tests/test_grant_access_to_project_view.py b/projects/tests/test_grant_access_to_project_view.py index 2b96f5e7..57fbecdb 100644 --- a/projects/tests/test_grant_access_to_project_view.py +++ b/projects/tests/test_grant_access_to_project_view.py @@ -9,14 +9,11 @@ test_user_2 = {"username": "foo2", "email": "foo2@test.com", "password": "bar"} -test_client = {"username": "client1", "email": "foo3@test.com", "password": "bar"} - class GrantAccessToProjectViewTestCase(TestCase): def setUp(self) -> None: self.user = User.objects.create_user(test_user_1["username"], test_user_1["email"], test_user_1["password"]) self.user2 = User.objects.create_user(test_user_2["username"], test_user_2["email"], test_user_2["password"]) - self.user3 = User.objects.create_user(test_client["username"], test_client["email"], test_client["password"]) self.client = Client() def get_data(self, user=None): @@ -100,29 +97,3 @@ def test_grant_access_to_non_existing_user(self): authorized = project.authorized.all() self.assertEqual(len(authorized), 0) - - """ - THIS TEST FAILS ON SCALEOUT DUE TO is_client does not exist - def test_grant_access_to_client(self): - response = self.client.post( - "/accounts/login/", {"username": test_user_1["email"], "password": test_user_1["password"]} - ) - response.status_code - - self.assertEqual(response.status_code, 302) - - project = self.get_data() - - response = self.client.post( - f"/{self.user.username}/{project.slug}/project/access/grant", - {"selected_user":test_client["email"]}, - ) - - self.assertEqual(response.status_code, 302) - - project = Project.objects.get(name="test-perm") - - authorized = project.authorized.all() - - self.assertEquals(len(authorized), 0) - """ From 989d1be176b236fd59b9dde7374076c9b1fce509 Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 26 Nov 2024 14:40:07 +0100 Subject: [PATCH 14/28] add unit tests for flavor creation --- projects/tests/test_create_remove_flavors.py | 65 ++++++++++++++++++++ projects/views.py | 48 ++++++++------- 2 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 projects/tests/test_create_remove_flavors.py diff --git a/projects/tests/test_create_remove_flavors.py b/projects/tests/test_create_remove_flavors.py new file mode 100644 index 00000000..8ebdb4e2 --- /dev/null +++ b/projects/tests/test_create_remove_flavors.py @@ -0,0 +1,65 @@ +from django.contrib.auth import get_user_model +from django.test import TestCase + +from projects.models import Project, Flavor + +User = get_user_model() + +test_user = {"username": "foo1", "email": "foo@test.com", "password": "bar"} +test_superuser = {"username": "superuser", "email": "superuser@test.com", "password": "bar"} + +class FlavorTestCase(TestCase): + def setUp(self): + user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) + _ = Project.objects.create_project(name="test-flavor", owner=user, description="") + superuser = User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) + #self.client.login(username=test_user["email"], password=test_user["password"]) + + def test_forbidden_flavor_creation(self): + """ + Test regular user not allowed to create flavor + """ + self.client.login(username=test_user["email"], password=test_user["password"]) + + project = Project.objects.get(name="test-flavor") + + response = self.client.post( + f"/projects/{project.slug}/createflavor/", + {"flavor_name": "new-flavor", + "cpu_req": "n", + "mem_req": "n", + "ephmem_req": "n", + "cpu_lim": "n", + "mem_lim": "n", + "ephmem_lim": "n" + }, + ) + + self.assertEqual(response.status_code, 403) + + flavors = Flavor.objects.all() + self.assertEqual(len(flavors), 0) + + + def test_allowed_flavor_creation(self): + """ + Test superuser is allowed to create flavor + """ + self.client.login(username=test_superuser["email"], password=test_superuser["password"]) + project = Project.objects.get(name="test-flavor") + response = self.client.post( + f"/projects/{project.slug}/createflavor/", + {"flavor_name": "new-flavor", + "cpu_req": "n", + "mem_req": "n", + "ephmem_req": "n", + "cpu_lim": "n", + "mem_lim": "n", + "ephmem_lim": "n" + }, + ) + + self.assertEqual(response.status_code, 302) + + flavors = Flavor.objects.all() + self.assertEqual(len(flavors), 1) diff --git a/projects/views.py b/projects/views.py index 28f72669..d87f52ed 100644 --- a/projects/views.py +++ b/projects/views.py @@ -258,29 +258,31 @@ def delete_environment(request, project_slug): @login_required @permission_required_or_403("can_view_project", (Project, "slug", "project_slug")) def create_flavor(request, project_slug): - # TODO: Ensure that user is allowed to create flavor in this project. - if request.method == "POST": - # TODO: Check input - project = Project.objects.get(slug=project_slug) - logger.info(request.POST) - name = request.POST.get("flavor_name") - cpu_req = request.POST.get("cpu_req") - mem_req = request.POST.get("mem_req") - ephmem_req = request.POST.get("ephmem_req") - cpu_lim = request.POST.get("cpu_lim") - mem_lim = request.POST.get("mem_lim") - ephmem_lim = request.POST.get("ephmem_lim") - flavor = Flavor( - name=name, - project=project, - cpu_req=cpu_req, - mem_req=mem_req, - cpu_lim=cpu_lim, - mem_lim=mem_lim, - ephmem_req=ephmem_req, - ephmem_lim=ephmem_lim, - ) - flavor.save() + project = Project.objects.get(slug=project_slug) + if not request.user.is_superuser: + return HttpResponseForbidden() + else: + if request.method == "POST": + # TODO: Check input + logger.info(request.POST) + name = request.POST.get("flavor_name") + cpu_req = request.POST.get("cpu_req") + mem_req = request.POST.get("mem_req") + ephmem_req = request.POST.get("ephmem_req") + cpu_lim = request.POST.get("cpu_lim") + mem_lim = request.POST.get("mem_lim") + ephmem_lim = request.POST.get("ephmem_lim") + flavor = Flavor( + name=name, + project=project, + cpu_req=cpu_req, + mem_req=mem_req, + cpu_lim=cpu_lim, + mem_lim=mem_lim, + ephmem_req=ephmem_req, + ephmem_lim=ephmem_lim, + ) + flavor.save() return HttpResponseRedirect( reverse( "projects:settings", From bdc61d14477cf307cad2a9c9be777c3f4b6b1abf Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 26 Nov 2024 15:40:14 +0100 Subject: [PATCH 15/28] simplify function to check if ok to delete --- projects/views.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/projects/views.py b/projects/views.py index d87f52ed..8a4aa704 100644 --- a/projects/views.py +++ b/projects/views.py @@ -177,7 +177,7 @@ def change_description(request, project_slug): ) -def can_model_instance_be_deleted(request, model, field_name, instance): +def can_model_instance_be_deleted(field_name, instance): """ Check if a model instance can be deleted by ensuring no app in APP_REGISTRY references it via the specified field. @@ -236,7 +236,7 @@ def delete_environment(request, project_slug): # TODO: Check that the user has permission to delete this environment. environment = Environment.objects.get(pk=pk, project=project) - can_environment_be_deleted = can_model_instance_be_deleted(request, Environment, "environment", pk) + can_environment_be_deleted = can_model_instance_be_deleted("environment", pk) if can_environment_be_deleted: environment.delete() @@ -300,7 +300,7 @@ def delete_flavor(request, project_slug): # TODO: Check that the user has permission to delete this flavor. flavor = Flavor.objects.get(pk=pk, project=project) - can_flavor_be_deleted = can_model_instance_be_deleted(request, Flavor, "flavor", pk) + can_flavor_be_deleted = can_model_instance_be_deleted("flavor", pk) if can_flavor_be_deleted: flavor.delete() From 476c1bc6e1d54e04b4fcd499e26be8b6d43d3c4e Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 26 Nov 2024 15:40:44 +0100 Subject: [PATCH 16/28] add flavor deletion tests --- projects/tests/test_create_remove_flavors.py | 36 ++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/projects/tests/test_create_remove_flavors.py b/projects/tests/test_create_remove_flavors.py index 8ebdb4e2..f33a9a2f 100644 --- a/projects/tests/test_create_remove_flavors.py +++ b/projects/tests/test_create_remove_flavors.py @@ -2,6 +2,8 @@ from django.test import TestCase from projects.models import Project, Flavor +from projects.views import can_model_instance_be_deleted +from apps.models import Apps, CustomAppInstance User = get_user_model() @@ -25,7 +27,7 @@ def test_forbidden_flavor_creation(self): response = self.client.post( f"/projects/{project.slug}/createflavor/", - {"flavor_name": "new-flavor", + {"flavor_name": "new-flavor-user", "cpu_req": "n", "mem_req": "n", "ephmem_req": "n", @@ -49,7 +51,7 @@ def test_allowed_flavor_creation(self): project = Project.objects.get(name="test-flavor") response = self.client.post( f"/projects/{project.slug}/createflavor/", - {"flavor_name": "new-flavor", + {"flavor_name": "new-flavor-superuser", "cpu_req": "n", "mem_req": "n", "ephmem_req": "n", @@ -63,3 +65,33 @@ def test_allowed_flavor_creation(self): flavors = Flavor.objects.all() self.assertEqual(len(flavors), 1) + + """ + Test it is allowed to delete flavor that is not in use + """ + user = User.objects.get(email=test_superuser["email"]) + flavor = Flavor.objects.get(name="new-flavor-superuser") + + can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor.pk) + self.assertTrue(can_flavor_be_deleted) + + """ + Test it is not allowed to delete flavor that is in use + """ + app = Apps.objects.create(name="Some App", slug="customapp") + self.app_instance = CustomAppInstance.objects.create( + access="public", + owner=user, + name="test_app_instance_flavor", + description="some app description", + app=app, + project=project, + k8s_values={ + "environment": {"pk": ""}, + }, + flavor=flavor + ) + + can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor.pk) + self.assertFalse(can_flavor_be_deleted) + From 3120ce40e369ce87ed4421bf49ed42509f1310e9 Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 26 Nov 2024 15:52:09 +0100 Subject: [PATCH 17/28] pre-commit fix --- projects/tests/test_create_remove_flavors.py | 48 ++++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/projects/tests/test_create_remove_flavors.py b/projects/tests/test_create_remove_flavors.py index f33a9a2f..07792c01 100644 --- a/projects/tests/test_create_remove_flavors.py +++ b/projects/tests/test_create_remove_flavors.py @@ -1,21 +1,21 @@ from django.contrib.auth import get_user_model from django.test import TestCase -from projects.models import Project, Flavor -from projects.views import can_model_instance_be_deleted from apps.models import Apps, CustomAppInstance +from projects.models import Flavor, Project +from projects.views import can_model_instance_be_deleted User = get_user_model() test_user = {"username": "foo1", "email": "foo@test.com", "password": "bar"} test_superuser = {"username": "superuser", "email": "superuser@test.com", "password": "bar"} + class FlavorTestCase(TestCase): def setUp(self): user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) _ = Project.objects.create_project(name="test-flavor", owner=user, description="") - superuser = User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) - #self.client.login(username=test_user["email"], password=test_user["password"]) + User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) def test_forbidden_flavor_creation(self): """ @@ -27,14 +27,15 @@ def test_forbidden_flavor_creation(self): response = self.client.post( f"/projects/{project.slug}/createflavor/", - {"flavor_name": "new-flavor-user", - "cpu_req": "n", - "mem_req": "n", - "ephmem_req": "n", - "cpu_lim": "n", - "mem_lim": "n", - "ephmem_lim": "n" - }, + { + "flavor_name": "new-flavor-user", + "cpu_req": "n", + "mem_req": "n", + "ephmem_req": "n", + "cpu_lim": "n", + "mem_lim": "n", + "ephmem_lim": "n", + }, ) self.assertEqual(response.status_code, 403) @@ -42,7 +43,6 @@ def test_forbidden_flavor_creation(self): flavors = Flavor.objects.all() self.assertEqual(len(flavors), 0) - def test_allowed_flavor_creation(self): """ Test superuser is allowed to create flavor @@ -51,21 +51,22 @@ def test_allowed_flavor_creation(self): project = Project.objects.get(name="test-flavor") response = self.client.post( f"/projects/{project.slug}/createflavor/", - {"flavor_name": "new-flavor-superuser", - "cpu_req": "n", - "mem_req": "n", - "ephmem_req": "n", - "cpu_lim": "n", - "mem_lim": "n", - "ephmem_lim": "n" - }, + { + "flavor_name": "new-flavor-superuser", + "cpu_req": "n", + "mem_req": "n", + "ephmem_req": "n", + "cpu_lim": "n", + "mem_lim": "n", + "ephmem_lim": "n", + }, ) self.assertEqual(response.status_code, 302) flavors = Flavor.objects.all() self.assertEqual(len(flavors), 1) - + """ Test it is allowed to delete flavor that is not in use """ @@ -89,9 +90,8 @@ def test_allowed_flavor_creation(self): k8s_values={ "environment": {"pk": ""}, }, - flavor=flavor + flavor=flavor, ) can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor.pk) self.assertFalse(can_flavor_be_deleted) - From 06182479e6c99b771c8641373da045cd0249c487 Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 26 Nov 2024 17:03:11 +0100 Subject: [PATCH 18/28] include more flavor deletion tests --- ...avors.py => test_create_delete_flavors.py} | 64 ++++++++++++++----- 1 file changed, 47 insertions(+), 17 deletions(-) rename projects/tests/{test_create_remove_flavors.py => test_create_delete_flavors.py} (59%) diff --git a/projects/tests/test_create_remove_flavors.py b/projects/tests/test_create_delete_flavors.py similarity index 59% rename from projects/tests/test_create_remove_flavors.py rename to projects/tests/test_create_delete_flavors.py index 07792c01..90c6bcc1 100644 --- a/projects/tests/test_create_remove_flavors.py +++ b/projects/tests/test_create_delete_flavors.py @@ -1,7 +1,7 @@ from django.contrib.auth import get_user_model from django.test import TestCase -from apps.models import Apps, CustomAppInstance +from apps.models import Apps, CustomAppInstance, Subdomain from projects.models import Flavor, Project from projects.views import can_model_instance_be_deleted @@ -14,10 +14,11 @@ class FlavorTestCase(TestCase): def setUp(self): user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) - _ = Project.objects.create_project(name="test-flavor", owner=user, description="") + project = Project.objects.create_project(name="test-flavor", owner=user, description="") User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) + Flavor.objects.create(name="flavor-to-be-deleted", project=project) - def test_forbidden_flavor_creation(self): + def test_flavor_creation_user(self): """ Test regular user not allowed to create flavor """ @@ -25,6 +26,7 @@ def test_forbidden_flavor_creation(self): project = Project.objects.get(name="test-flavor") + n_flavors_before = len(Flavor.objects.all()) response = self.client.post( f"/projects/{project.slug}/createflavor/", { @@ -37,18 +39,28 @@ def test_forbidden_flavor_creation(self): "ephmem_lim": "n", }, ) - self.assertEqual(response.status_code, 403) + n_flavors_after = len(Flavor.objects.all()) - flavors = Flavor.objects.all() - self.assertEqual(len(flavors), 0) + self.assertEqual(n_flavors_before, n_flavors_after) - def test_allowed_flavor_creation(self): + """ + Test regular user not allowed to delete flavor + """ + flavor_to_be_deleted = Flavor.objects.get(name="flavor-to-be-deleted") + n_flavors_before = len(Flavor.objects.all()) + response = self.client.post(f"/projects/{project.slug}/deleteflavor/", {"flavor_pk": flavor_to_be_deleted.pk}) + self.assertEqual(response.status_code, 403) + n_flavors_after = len(Flavor.objects.all()) + self.assertEqual(n_flavors_before, n_flavors_after) + + def test_flavor_creation_deletion_superuser(self): """ Test superuser is allowed to create flavor """ self.client.login(username=test_superuser["email"], password=test_superuser["password"]) project = Project.objects.get(name="test-flavor") + n_flavors_before = len(Flavor.objects.all()) response = self.client.post( f"/projects/{project.slug}/createflavor/", { @@ -61,25 +73,19 @@ def test_allowed_flavor_creation(self): "ephmem_lim": "n", }, ) - self.assertEqual(response.status_code, 302) + n_flavors_after = len(Flavor.objects.all()) - flavors = Flavor.objects.all() - self.assertEqual(len(flavors), 1) + self.assertEqual(n_flavors_before + 1, n_flavors_after) """ - Test it is allowed to delete flavor that is not in use + Test it is not allowed to delete flavor that is in use """ user = User.objects.get(email=test_superuser["email"]) flavor = Flavor.objects.get(name="new-flavor-superuser") - can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor.pk) - self.assertTrue(can_flavor_be_deleted) - - """ - Test it is not allowed to delete flavor that is in use - """ app = Apps.objects.create(name="Some App", slug="customapp") + subdomain = Subdomain.objects.create(subdomain="test_internal") self.app_instance = CustomAppInstance.objects.create( access="public", owner=user, @@ -91,7 +97,31 @@ def test_allowed_flavor_creation(self): "environment": {"pk": ""}, }, flavor=flavor, + subdomain=subdomain, ) can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor.pk) self.assertFalse(can_flavor_be_deleted) + + n_flavors_before = len(Flavor.objects.all()) + response = self.client.post(f"/projects/{project.slug}/deleteflavor/", {"flavor_pk": flavor.pk}) + self.assertEqual(response.status_code, 302) + n_flavors_after = len(Flavor.objects.all()) + + self.assertEqual(n_flavors_before, n_flavors_after) + + """ + Test it is allowed to delete flavor that is not in use + """ + + flavor_to_be_deleted = Flavor.objects.get(name="flavor-to-be-deleted") + + can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor_to_be_deleted.pk) + self.assertTrue(can_flavor_be_deleted) + + n_flavors_before = len(Flavor.objects.all()) + response = self.client.post(f"/projects/{project.slug}/deleteflavor/", {"flavor_pk": flavor_to_be_deleted.pk}) + self.assertEqual(response.status_code, 302) + n_flavors_after = len(Flavor.objects.all()) + + self.assertEqual(n_flavors_before - 1, n_flavors_after) From d2289843f02d0c62e744ae5fe6b4b535a82311af Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 26 Nov 2024 17:04:21 +0100 Subject: [PATCH 19/28] prohibit flavor and environment editing for regular users --- projects/views.py | 99 +++++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 46 deletions(-) diff --git a/projects/views.py b/projects/views.py index 8a4aa704..aa660c88 100644 --- a/projects/views.py +++ b/projects/views.py @@ -202,23 +202,26 @@ def can_model_instance_be_deleted(field_name, instance): @login_required @permission_required_or_403("can_view_project", (Project, "slug", "project_slug")) def create_environment(request, project_slug): - # TODO: Ensure that user is allowed to create environment in this project. - if request.method == "POST": - project = Project.objects.get(slug=project_slug) - name = request.POST.get("environment_name") - repo = request.POST.get("environment_repository") - image = request.POST.get("environment_image") - app_pk = request.POST.get("environment_app") - app = Apps.objects.get(pk=app_pk) - environment = Environment( - name=name, - slug=name, - project=project, - repository=repo, - image=image, - app=app, - ) - environment.save() + project = Project.objects.get(slug=project_slug) + if not request.user.is_superuser: + return HttpResponseForbidden() + else: + if request.method == "POST": + # TODO: check input data + name = request.POST.get("environment_name") + repo = request.POST.get("environment_repository") + image = request.POST.get("environment_image") + app_pk = request.POST.get("environment_app") + app = Apps.objects.get(pk=app_pk) + environment = Environment( + name=name, + slug=name, + project=project, + repository=repo, + image=image, + app=app, + ) + environment.save() return HttpResponseRedirect( reverse( "projects:settings", @@ -230,23 +233,24 @@ def create_environment(request, project_slug): @login_required @permission_required_or_403("can_view_project", (Project, "slug", "project_slug")) def delete_environment(request, project_slug): - if request.method == "POST": - project = Project.objects.get(slug=project_slug) - pk = request.POST.get("environment_pk") - # TODO: Check that the user has permission to delete this environment. - environment = Environment.objects.get(pk=pk, project=project) - - can_environment_be_deleted = can_model_instance_be_deleted("environment", pk) + project = Project.objects.get(slug=project_slug) + if not request.user.is_superuser: + return HttpResponseForbidden() + else: + if request.method == "POST": + pk = request.POST.get("environment_pk") + environment = Environment.objects.get(pk=pk, project=project) - if can_environment_be_deleted: - environment.delete() - else: - messages.error( - request, - "Environment cannot be deleted because it is currently used by at least one app \ - (can also be a deleted app).", - ) + can_environment_be_deleted = can_model_instance_be_deleted("environment", pk) + if can_environment_be_deleted: + environment.delete() + else: + messages.error( + request, + "Environment cannot be deleted because it is currently used by at least one app \ + (can also be a deleted app).", + ) return HttpResponseRedirect( reverse( "projects:settings", @@ -294,22 +298,25 @@ def create_flavor(request, project_slug): @login_required @permission_required_or_403("can_view_project", (Project, "slug", "project_slug")) def delete_flavor(request, project_slug): - if request.method == "POST": - project = Project.objects.get(slug=project_slug) - pk = request.POST.get("flavor_pk") - # TODO: Check that the user has permission to delete this flavor. - flavor = Flavor.objects.get(pk=pk, project=project) + project = Project.objects.get(slug=project_slug) + if not request.user.is_superuser: + return HttpResponseForbidden() + else: + if request.method == "POST": + project = Project.objects.get(slug=project_slug) + pk = request.POST.get("flavor_pk") + flavor = Flavor.objects.get(pk=pk, project=project) - can_flavor_be_deleted = can_model_instance_be_deleted("flavor", pk) + can_flavor_be_deleted = can_model_instance_be_deleted("flavor", pk) - if can_flavor_be_deleted: - flavor.delete() - else: - messages.error( - request, - "Flavor cannot be deleted because it is currently used by at least one app \ - (can also be a deleted app).", - ) + if can_flavor_be_deleted: + flavor.delete() + else: + messages.error( + request, + "Flavor cannot be deleted because it is currently used by at least one app \ + (can also be a deleted app).", + ) return HttpResponseRedirect( reverse( From dc7e5949372239e8fea570a300a7987dbfe154ba Mon Sep 17 00:00:00 2001 From: akochari Date: Wed, 27 Nov 2024 07:58:24 +0100 Subject: [PATCH 20/28] add unit tests for environment creation and deletion --- .../tests/test_create_delete_environments.py | 129 ++++++++++++++++++ projects/tests/test_create_delete_flavors.py | 7 +- 2 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 projects/tests/test_create_delete_environments.py diff --git a/projects/tests/test_create_delete_environments.py b/projects/tests/test_create_delete_environments.py new file mode 100644 index 00000000..b77dc04e --- /dev/null +++ b/projects/tests/test_create_delete_environments.py @@ -0,0 +1,129 @@ +from django.contrib.auth import get_user_model +from django.contrib.messages import get_messages +from django.test import TestCase + +from apps.models import Apps, JupyterInstance, Subdomain +from projects.models import Environment, Project +from projects.views import can_model_instance_be_deleted + +User = get_user_model() + +test_user = {"username": "foo1", "email": "foo@test.com", "password": "bar"} +test_superuser = {"username": "superuser", "email": "superuser@test.com", "password": "bar"} + + +class EnvironmentTestCase(TestCase): + def setUp(self): + user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) + project = Project.objects.create_project(name="test-env", owner=user, description="") + User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) + app = Apps.objects.create(name="Some App", slug="someapp") + Environment.objects.create(app=app, name="env-to-be-deleted", project=project) + + def test_environment_creation_user(self): + """ + Test regular user not allowed to create environment + """ + self.client.login(username=test_user["email"], password=test_user["password"]) + + project = Project.objects.get(name="test-env") + app = Apps.objects.get(slug="someapp") + + n_envs_before = len(Environment.objects.all()) + response = self.client.post( + f"/projects/{project.slug}/createenvironment/", + { + "environment_name": "new-env-user", + "environment_repository": "n", + "environment_image": "n", + "environment_app": "n", + "app": app.pk, + }, + ) + self.assertEqual(response.status_code, 403) + n_envs_after = len(Environment.objects.all()) + + self.assertEqual(n_envs_before, n_envs_after) + + """ + Test regular user not allowed to delete environment + """ + env_to_be_deleted = Environment.objects.get(name="env-to-be-deleted") + n_envs_before = len(Environment.objects.all()) + response = self.client.post( + f"/projects/{project.slug}/deleteenvironment/", {"environment_pk": env_to_be_deleted.pk} + ) + self.assertEqual(response.status_code, 403) + n_envs_after = len(Environment.objects.all()) + self.assertEqual(n_envs_before, n_envs_after) + + def test_environment_creation_deletion_superuser(self): + """ + Test superuser is allowed to create environment + """ + self.client.login(username=test_superuser["email"], password=test_superuser["password"]) + project = Project.objects.get(name="test-env") + app = Apps.objects.get(slug="someapp") + + n_envs_before = len(Environment.objects.all()) + response = self.client.post( + f"/projects/{project.slug}/createenvironment/", + { + "environment_name": "new-env-superuser", + "environment_repository": "n", + "environment_image": "n", + "environment_app": app.pk, + }, + ) + self.assertEqual(response.status_code, 302) + n_envs_after = len(Environment.objects.all()) + + self.assertEqual(n_envs_before + 1, n_envs_after) + + """ + Test it is not allowed to delete environment that is in use + """ + user = User.objects.get(email=test_superuser["email"]) + env = Environment.objects.get(name="new-env-superuser") + + subdomain = Subdomain.objects.create(subdomain="test_internal") + self.app_instance = JupyterInstance.objects.create( + access="public", + owner=user, + name="test_app_instance_env", + app=app, + project=project, + environment=env, + subdomain=subdomain, + ) + + can_env_be_deleted = can_model_instance_be_deleted("environment", env.pk) + self.assertFalse(can_env_be_deleted) + + n_envs_before = len(Environment.objects.all()) + response = self.client.post(f"/projects/{project.slug}/deleteenvironment/", {"environment_pk": env.pk}) + self.assertEqual(response.status_code, 302) + messages = list(get_messages(response.wsgi_request)) + self.assertEqual(len(messages), 1) + self.assertIn("cannot be deleted", str(messages[0])) + n_envs_after = len(Environment.objects.all()) + + self.assertEqual(n_envs_before, n_envs_after) + + """ + Test it is allowed to delete environment that is not in use + """ + + env_to_be_deleted = Environment.objects.get(name="env-to-be-deleted") + + can_env_be_deleted = can_model_instance_be_deleted("environment", env_to_be_deleted.pk) + self.assertTrue(can_env_be_deleted) + + n_env_before = len(Environment.objects.all()) + response = self.client.post( + f"/projects/{project.slug}/deleteenvironment/", {"environment_pk": env_to_be_deleted.pk} + ) + self.assertEqual(response.status_code, 302) + n_envs_after = len(Environment.objects.all()) + + self.assertEqual(n_envs_before - 1, n_envs_after) diff --git a/projects/tests/test_create_delete_flavors.py b/projects/tests/test_create_delete_flavors.py index 90c6bcc1..5d94c002 100644 --- a/projects/tests/test_create_delete_flavors.py +++ b/projects/tests/test_create_delete_flavors.py @@ -1,4 +1,5 @@ from django.contrib.auth import get_user_model +from django.contrib.messages import get_messages from django.test import TestCase from apps.models import Apps, CustomAppInstance, Subdomain @@ -93,9 +94,6 @@ def test_flavor_creation_deletion_superuser(self): description="some app description", app=app, project=project, - k8s_values={ - "environment": {"pk": ""}, - }, flavor=flavor, subdomain=subdomain, ) @@ -106,6 +104,9 @@ def test_flavor_creation_deletion_superuser(self): n_flavors_before = len(Flavor.objects.all()) response = self.client.post(f"/projects/{project.slug}/deleteflavor/", {"flavor_pk": flavor.pk}) self.assertEqual(response.status_code, 302) + messages = list(get_messages(response.wsgi_request)) + self.assertEqual(len(messages), 1) + self.assertIn("cannot be deleted", str(messages[0])) n_flavors_after = len(Flavor.objects.all()) self.assertEqual(n_flavors_before, n_flavors_after) From 96e384322116f73c07842b9ce1b5c985fe52809b Mon Sep 17 00:00:00 2001 From: akochari Date: Fri, 6 Dec 2024 08:45:46 +0100 Subject: [PATCH 21/28] add type hints for the new function --- .../tests/test_create_delete_environments.py | 24 +++++++++---------- projects/tests/test_create_delete_flavors.py | 24 +++++++++---------- projects/views.py | 15 +++++------- 3 files changed, 30 insertions(+), 33 deletions(-) diff --git a/projects/tests/test_create_delete_environments.py b/projects/tests/test_create_delete_environments.py index b77dc04e..5e429ca5 100644 --- a/projects/tests/test_create_delete_environments.py +++ b/projects/tests/test_create_delete_environments.py @@ -29,7 +29,7 @@ def test_environment_creation_user(self): project = Project.objects.get(name="test-env") app = Apps.objects.get(slug="someapp") - n_envs_before = len(Environment.objects.all()) + n_envs_before = Environment.objects.count() response = self.client.post( f"/projects/{project.slug}/createenvironment/", { @@ -41,7 +41,7 @@ def test_environment_creation_user(self): }, ) self.assertEqual(response.status_code, 403) - n_envs_after = len(Environment.objects.all()) + n_envs_after = Environment.objects.count() self.assertEqual(n_envs_before, n_envs_after) @@ -49,12 +49,12 @@ def test_environment_creation_user(self): Test regular user not allowed to delete environment """ env_to_be_deleted = Environment.objects.get(name="env-to-be-deleted") - n_envs_before = len(Environment.objects.all()) + n_envs_before = Environment.objects.count() response = self.client.post( f"/projects/{project.slug}/deleteenvironment/", {"environment_pk": env_to_be_deleted.pk} ) self.assertEqual(response.status_code, 403) - n_envs_after = len(Environment.objects.all()) + n_envs_after = Environment.objects.count() self.assertEqual(n_envs_before, n_envs_after) def test_environment_creation_deletion_superuser(self): @@ -65,7 +65,7 @@ def test_environment_creation_deletion_superuser(self): project = Project.objects.get(name="test-env") app = Apps.objects.get(slug="someapp") - n_envs_before = len(Environment.objects.all()) + n_envs_before = Environment.objects.count() response = self.client.post( f"/projects/{project.slug}/createenvironment/", { @@ -76,7 +76,7 @@ def test_environment_creation_deletion_superuser(self): }, ) self.assertEqual(response.status_code, 302) - n_envs_after = len(Environment.objects.all()) + n_envs_after = Environment.objects.count() self.assertEqual(n_envs_before + 1, n_envs_after) @@ -97,16 +97,16 @@ def test_environment_creation_deletion_superuser(self): subdomain=subdomain, ) - can_env_be_deleted = can_model_instance_be_deleted("environment", env.pk) + can_env_be_deleted = can_model_instance_be_deleted("environment", env) self.assertFalse(can_env_be_deleted) - n_envs_before = len(Environment.objects.all()) + n_envs_before = Environment.objects.count() response = self.client.post(f"/projects/{project.slug}/deleteenvironment/", {"environment_pk": env.pk}) self.assertEqual(response.status_code, 302) messages = list(get_messages(response.wsgi_request)) self.assertEqual(len(messages), 1) self.assertIn("cannot be deleted", str(messages[0])) - n_envs_after = len(Environment.objects.all()) + n_envs_after = Environment.objects.count() self.assertEqual(n_envs_before, n_envs_after) @@ -116,14 +116,14 @@ def test_environment_creation_deletion_superuser(self): env_to_be_deleted = Environment.objects.get(name="env-to-be-deleted") - can_env_be_deleted = can_model_instance_be_deleted("environment", env_to_be_deleted.pk) + can_env_be_deleted = can_model_instance_be_deleted("environment", env_to_be_deleted) self.assertTrue(can_env_be_deleted) - n_env_before = len(Environment.objects.all()) + n_env_before = Environment.objects.count() response = self.client.post( f"/projects/{project.slug}/deleteenvironment/", {"environment_pk": env_to_be_deleted.pk} ) self.assertEqual(response.status_code, 302) - n_envs_after = len(Environment.objects.all()) + n_envs_after = Environment.objects.count() self.assertEqual(n_envs_before - 1, n_envs_after) diff --git a/projects/tests/test_create_delete_flavors.py b/projects/tests/test_create_delete_flavors.py index 5d94c002..6c8e299b 100644 --- a/projects/tests/test_create_delete_flavors.py +++ b/projects/tests/test_create_delete_flavors.py @@ -27,7 +27,7 @@ def test_flavor_creation_user(self): project = Project.objects.get(name="test-flavor") - n_flavors_before = len(Flavor.objects.all()) + n_flavors_before = Flavor.objects.count() response = self.client.post( f"/projects/{project.slug}/createflavor/", { @@ -41,7 +41,7 @@ def test_flavor_creation_user(self): }, ) self.assertEqual(response.status_code, 403) - n_flavors_after = len(Flavor.objects.all()) + n_flavors_after = Flavor.objects.count() self.assertEqual(n_flavors_before, n_flavors_after) @@ -49,10 +49,10 @@ def test_flavor_creation_user(self): Test regular user not allowed to delete flavor """ flavor_to_be_deleted = Flavor.objects.get(name="flavor-to-be-deleted") - n_flavors_before = len(Flavor.objects.all()) + n_flavors_before = Flavor.objects.count() response = self.client.post(f"/projects/{project.slug}/deleteflavor/", {"flavor_pk": flavor_to_be_deleted.pk}) self.assertEqual(response.status_code, 403) - n_flavors_after = len(Flavor.objects.all()) + n_flavors_after = Flavor.objects.count() self.assertEqual(n_flavors_before, n_flavors_after) def test_flavor_creation_deletion_superuser(self): @@ -61,7 +61,7 @@ def test_flavor_creation_deletion_superuser(self): """ self.client.login(username=test_superuser["email"], password=test_superuser["password"]) project = Project.objects.get(name="test-flavor") - n_flavors_before = len(Flavor.objects.all()) + n_flavors_before = Flavor.objects.count() response = self.client.post( f"/projects/{project.slug}/createflavor/", { @@ -75,7 +75,7 @@ def test_flavor_creation_deletion_superuser(self): }, ) self.assertEqual(response.status_code, 302) - n_flavors_after = len(Flavor.objects.all()) + n_flavors_after = Flavor.objects.count() self.assertEqual(n_flavors_before + 1, n_flavors_after) @@ -98,16 +98,16 @@ def test_flavor_creation_deletion_superuser(self): subdomain=subdomain, ) - can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor.pk) + can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor) self.assertFalse(can_flavor_be_deleted) - n_flavors_before = len(Flavor.objects.all()) + n_flavors_before = Flavor.objects.count() response = self.client.post(f"/projects/{project.slug}/deleteflavor/", {"flavor_pk": flavor.pk}) self.assertEqual(response.status_code, 302) messages = list(get_messages(response.wsgi_request)) self.assertEqual(len(messages), 1) self.assertIn("cannot be deleted", str(messages[0])) - n_flavors_after = len(Flavor.objects.all()) + n_flavors_after = Flavor.objects.count() self.assertEqual(n_flavors_before, n_flavors_after) @@ -117,12 +117,12 @@ def test_flavor_creation_deletion_superuser(self): flavor_to_be_deleted = Flavor.objects.get(name="flavor-to-be-deleted") - can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor_to_be_deleted.pk) + can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor_to_be_deleted) self.assertTrue(can_flavor_be_deleted) - n_flavors_before = len(Flavor.objects.all()) + n_flavors_before = Flavor.objects.count() response = self.client.post(f"/projects/{project.slug}/deleteflavor/", {"flavor_pk": flavor_to_be_deleted.pk}) self.assertEqual(response.status_code, 302) - n_flavors_after = len(Flavor.objects.all()) + n_flavors_after = Flavor.objects.count() self.assertEqual(n_flavors_before - 1, n_flavors_after) diff --git a/projects/views.py b/projects/views.py index aa660c88..ba692877 100644 --- a/projects/views.py +++ b/projects/views.py @@ -9,7 +9,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.decorators import login_required from django.core.exceptions import FieldDoesNotExist -from django.db.models import Q +from django.db.models import Model, Q from django.http import ( HttpResponse, HttpResponseBadRequest, @@ -34,7 +34,6 @@ logger = logging.getLogger(__name__) Apps = apps.get_model(app_label=django_settings.APPS_MODEL) AppCategories = apps.get_model(app_label=django_settings.APPCATEGORIES_MODEL) -Model = apps.get_model(app_label=django_settings.MODELS_MODEL) User = get_user_model() @@ -177,16 +176,14 @@ def change_description(request, project_slug): ) -def can_model_instance_be_deleted(field_name, instance): +def can_model_instance_be_deleted(field_name: str, instance: Model) -> bool: """ Check if a model instance can be deleted by ensuring no app in APP_REGISTRY references it via the specified field. Args: - request: The HTTP request object or None if called outside of request context. - model: The model class (e.g., Environment or Flavor). - field_name: The name of the field to check in APP_REGISTRY models. - instance: The instance to check. + field_name (str): The name of the field to check in APP_REGISTRY models. + instance (Model): The model instance to check. Returns: bool: True if the instance can be safely deleted, False otherwise. @@ -241,7 +238,7 @@ def delete_environment(request, project_slug): pk = request.POST.get("environment_pk") environment = Environment.objects.get(pk=pk, project=project) - can_environment_be_deleted = can_model_instance_be_deleted("environment", pk) + can_environment_be_deleted = can_model_instance_be_deleted("environment", environment) if can_environment_be_deleted: environment.delete() @@ -307,7 +304,7 @@ def delete_flavor(request, project_slug): pk = request.POST.get("flavor_pk") flavor = Flavor.objects.get(pk=pk, project=project) - can_flavor_be_deleted = can_model_instance_be_deleted("flavor", pk) + can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor) if can_flavor_be_deleted: flavor.delete() From 001a489566cac8dbe9db78d76d6db41cf5d71df3 Mon Sep 17 00:00:00 2001 From: akochari Date: Mon, 9 Dec 2024 13:35:38 +0100 Subject: [PATCH 22/28] updated serve logo --- static/images/scilifelab_serve_logo.svg | 124 +++++++++++++++++++++++- 1 file changed, 123 insertions(+), 1 deletion(-) diff --git a/static/images/scilifelab_serve_logo.svg b/static/images/scilifelab_serve_logo.svg index 9cf9f9ab..8f364229 100644 --- a/static/images/scilifelab_serve_logo.svg +++ b/static/images/scilifelab_serve_logo.svg @@ -1 +1,123 @@ - + + + + + + + + + + + + + + + + + + + + + + + + + + + From 8c984fa00189fdc711f0590997b4866526ca2037 Mon Sep 17 00:00:00 2001 From: akochari Date: Mon, 9 Dec 2024 13:56:21 +0100 Subject: [PATCH 23/28] remove extra migrations file --- ...er_jupyterinstance_environment_and_more.py | 28 ------------------- 1 file changed, 28 deletions(-) delete mode 100644 apps/migrations/0019_alter_jupyterinstance_environment_and_more.py diff --git a/apps/migrations/0019_alter_jupyterinstance_environment_and_more.py b/apps/migrations/0019_alter_jupyterinstance_environment_and_more.py deleted file mode 100644 index 89e61eba..00000000 --- a/apps/migrations/0019_alter_jupyterinstance_environment_and_more.py +++ /dev/null @@ -1,28 +0,0 @@ -# Generated by Django 5.1.1 on 2024-11-21 15:08 - -import django.db.models.deletion -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - ("apps", "0018_customappinstance_default_url_subpath"), - ("projects", "0008_project_deleted_on"), - ] - - operations = [ - migrations.AlterField( - model_name="jupyterinstance", - name="environment", - field=models.ForeignKey( - blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, to="projects.environment" - ), - ), - migrations.AlterField( - model_name="rstudioinstance", - name="environment", - field=models.ForeignKey( - blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, to="projects.environment" - ), - ), - ] From 0d2c73f4830eeaf3ee3a2aad22056b22758d15a0 Mon Sep 17 00:00:00 2001 From: akochari Date: Mon, 9 Dec 2024 16:41:59 +0100 Subject: [PATCH 24/28] split unit tests into smaller tests --- .../tests/test_create_delete_environments.py | 74 +++++++++---------- projects/tests/test_create_delete_flavors.py | 58 +++++++-------- 2 files changed, 62 insertions(+), 70 deletions(-) diff --git a/projects/tests/test_create_delete_environments.py b/projects/tests/test_create_delete_environments.py index 5e429ca5..e9bac32a 100644 --- a/projects/tests/test_create_delete_environments.py +++ b/projects/tests/test_create_delete_environments.py @@ -12,96 +12,93 @@ test_superuser = {"username": "superuser", "email": "superuser@test.com", "password": "bar"} -class EnvironmentTestCase(TestCase): +class EnvironmentTestCaseRegularUser(TestCase): def setUp(self): user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) - project = Project.objects.create_project(name="test-env", owner=user, description="") + self.project = Project.objects.create_project(name="test-env", owner=user, description="") User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) - app = Apps.objects.create(name="Some App", slug="someapp") - Environment.objects.create(app=app, name="env-to-be-deleted", project=project) + self.app = Apps.objects.create(name="Some App", slug="someapp") + self.env_to_be_deleted = Environment.objects.create(app=self.app, name="env-to-be-deleted", project=self.project) + self.client.login(username=test_user["email"], password=test_user["password"]) - def test_environment_creation_user(self): + def test_environment_creation_regular_user(self): """ Test regular user not allowed to create environment """ - self.client.login(username=test_user["email"], password=test_user["password"]) - - project = Project.objects.get(name="test-env") - app = Apps.objects.get(slug="someapp") - n_envs_before = Environment.objects.count() response = self.client.post( - f"/projects/{project.slug}/createenvironment/", + f"/projects/{self.project.slug}/createenvironment/", { "environment_name": "new-env-user", "environment_repository": "n", "environment_image": "n", "environment_app": "n", - "app": app.pk, + "app": self.app.pk, }, ) self.assertEqual(response.status_code, 403) n_envs_after = Environment.objects.count() - self.assertEqual(n_envs_before, n_envs_after) + def test_environment_deletion_regular_user(self): """ Test regular user not allowed to delete environment """ - env_to_be_deleted = Environment.objects.get(name="env-to-be-deleted") n_envs_before = Environment.objects.count() response = self.client.post( - f"/projects/{project.slug}/deleteenvironment/", {"environment_pk": env_to_be_deleted.pk} + f"/projects/{self.project.slug}/deleteenvironment/", {"environment_pk": self.env_to_be_deleted.pk} ) self.assertEqual(response.status_code, 403) n_envs_after = Environment.objects.count() self.assertEqual(n_envs_before, n_envs_after) - def test_environment_creation_deletion_superuser(self): +class EnvironmentTestCaseSuperUser(TestCase): + def setUp(self): + self.user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) + self.project = Project.objects.create_project(name="test-env", owner=self.user, description="") + User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) + self.app = Apps.objects.create(name="Some App", slug="someapp") + self.env_to_be_deleted = Environment.objects.create(app=self.app, name="env-to-be-deleted", project=self.project) + self.client.login(username=test_superuser["email"], password=test_superuser["password"]) + + def test_environment_creation_superuser(self): """ Test superuser is allowed to create environment """ - self.client.login(username=test_superuser["email"], password=test_superuser["password"]) - project = Project.objects.get(name="test-env") - app = Apps.objects.get(slug="someapp") - n_envs_before = Environment.objects.count() response = self.client.post( - f"/projects/{project.slug}/createenvironment/", + f"/projects/{self.project.slug}/createenvironment/", { "environment_name": "new-env-superuser", "environment_repository": "n", "environment_image": "n", - "environment_app": app.pk, + "environment_app": self.app.pk, }, ) self.assertEqual(response.status_code, 302) n_envs_after = Environment.objects.count() - self.assertEqual(n_envs_before + 1, n_envs_after) + def test_environment_deletion_inuse_superuser(self): """ Test it is not allowed to delete environment that is in use """ - user = User.objects.get(email=test_superuser["email"]) - env = Environment.objects.get(name="new-env-superuser") - + env_cannot_be_deleted = Environment.objects.create(app=self.app, name="env-cannot-be-deleted", project=self.project) subdomain = Subdomain.objects.create(subdomain="test_internal") self.app_instance = JupyterInstance.objects.create( access="public", - owner=user, + owner=self.user, name="test_app_instance_env", - app=app, - project=project, - environment=env, + app=self.app, + project=self.project, + environment=env_cannot_be_deleted, subdomain=subdomain, ) - - can_env_be_deleted = can_model_instance_be_deleted("environment", env) + can_env_be_deleted = can_model_instance_be_deleted("environment", env_cannot_be_deleted) self.assertFalse(can_env_be_deleted) n_envs_before = Environment.objects.count() - response = self.client.post(f"/projects/{project.slug}/deleteenvironment/", {"environment_pk": env.pk}) + response = self.client.post(f"/projects/{self.project.slug}/deleteenvironment/", {"environment_pk": env_cannot_be_deleted.pk}) self.assertEqual(response.status_code, 302) messages = list(get_messages(response.wsgi_request)) self.assertEqual(len(messages), 1) @@ -110,18 +107,15 @@ def test_environment_creation_deletion_superuser(self): self.assertEqual(n_envs_before, n_envs_after) + def test_environment_deletion_notinuse_superuser(self): """ Test it is allowed to delete environment that is not in use """ - - env_to_be_deleted = Environment.objects.get(name="env-to-be-deleted") - - can_env_be_deleted = can_model_instance_be_deleted("environment", env_to_be_deleted) + can_env_be_deleted = can_model_instance_be_deleted("environment", self.env_to_be_deleted) self.assertTrue(can_env_be_deleted) - - n_env_before = Environment.objects.count() + n_envs_before = Environment.objects.count() response = self.client.post( - f"/projects/{project.slug}/deleteenvironment/", {"environment_pk": env_to_be_deleted.pk} + f"/projects/{self.project.slug}/deleteenvironment/", {"environment_pk": self.env_to_be_deleted.pk} ) self.assertEqual(response.status_code, 302) n_envs_after = Environment.objects.count() diff --git a/projects/tests/test_create_delete_flavors.py b/projects/tests/test_create_delete_flavors.py index 6c8e299b..8a450acb 100644 --- a/projects/tests/test_create_delete_flavors.py +++ b/projects/tests/test_create_delete_flavors.py @@ -12,24 +12,20 @@ test_superuser = {"username": "superuser", "email": "superuser@test.com", "password": "bar"} -class FlavorTestCase(TestCase): +class FlavorTestCaseRegularUser(TestCase): def setUp(self): user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) - project = Project.objects.create_project(name="test-flavor", owner=user, description="") - User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) - Flavor.objects.create(name="flavor-to-be-deleted", project=project) + self.project = Project.objects.create_project(name="test-flavor", owner=user, description="") + self.flavor_to_be_deleted = Flavor.objects.create(name="flavor-to-be-deleted", project=self.project) + self.client.login(username=test_user["email"], password=test_user["password"]) - def test_flavor_creation_user(self): + def test_flavor_creation_regular_user(self): """ Test regular user not allowed to create flavor """ - self.client.login(username=test_user["email"], password=test_user["password"]) - - project = Project.objects.get(name="test-flavor") - n_flavors_before = Flavor.objects.count() response = self.client.post( - f"/projects/{project.slug}/createflavor/", + f"/projects/{self.project.slug}/createflavor/", { "flavor_name": "new-flavor-user", "cpu_req": "n", @@ -42,28 +38,33 @@ def test_flavor_creation_user(self): ) self.assertEqual(response.status_code, 403) n_flavors_after = Flavor.objects.count() - self.assertEqual(n_flavors_before, n_flavors_after) + def test_flavor_deletion_regular_user(self): """ Test regular user not allowed to delete flavor """ - flavor_to_be_deleted = Flavor.objects.get(name="flavor-to-be-deleted") n_flavors_before = Flavor.objects.count() - response = self.client.post(f"/projects/{project.slug}/deleteflavor/", {"flavor_pk": flavor_to_be_deleted.pk}) + response = self.client.post(f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": self.flavor_to_be_deleted.pk}) self.assertEqual(response.status_code, 403) n_flavors_after = Flavor.objects.count() self.assertEqual(n_flavors_before, n_flavors_after) - def test_flavor_creation_deletion_superuser(self): +class FlavorTestCaseSuperUser(TestCase): + def setUp(self): + self.user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) + self.project = Project.objects.create_project(name="test-flavor", owner=self.user, description="") + User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) + self.flavor_to_be_deleted = Flavor.objects.create(name="flavor-to-be-deleted", project=self.project) + self.client.login(username=test_superuser["email"], password=test_superuser["password"]) + + def test_flavor_creation_superuser(self): """ Test superuser is allowed to create flavor """ - self.client.login(username=test_superuser["email"], password=test_superuser["password"]) - project = Project.objects.get(name="test-flavor") n_flavors_before = Flavor.objects.count() response = self.client.post( - f"/projects/{project.slug}/createflavor/", + f"/projects/{self.project.slug}/createflavor/", { "flavor_name": "new-flavor-superuser", "cpu_req": "n", @@ -76,33 +77,32 @@ def test_flavor_creation_deletion_superuser(self): ) self.assertEqual(response.status_code, 302) n_flavors_after = Flavor.objects.count() - self.assertEqual(n_flavors_before + 1, n_flavors_after) + def test_flavor_deletion_inuse_superuser(self): """ Test it is not allowed to delete flavor that is in use """ - user = User.objects.get(email=test_superuser["email"]) - flavor = Flavor.objects.get(name="new-flavor-superuser") + flavor_cannot_be_deleted = Flavor.objects.create(name="flavor-cannot-be-deleted", project=self.project) app = Apps.objects.create(name="Some App", slug="customapp") subdomain = Subdomain.objects.create(subdomain="test_internal") self.app_instance = CustomAppInstance.objects.create( access="public", - owner=user, + owner=self.user, name="test_app_instance_flavor", description="some app description", app=app, - project=project, - flavor=flavor, + project=self.project, + flavor=flavor_cannot_be_deleted, subdomain=subdomain, ) - can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor) + can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor_cannot_be_deleted) self.assertFalse(can_flavor_be_deleted) n_flavors_before = Flavor.objects.count() - response = self.client.post(f"/projects/{project.slug}/deleteflavor/", {"flavor_pk": flavor.pk}) + response = self.client.post(f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": flavor_cannot_be_deleted.pk}) self.assertEqual(response.status_code, 302) messages = list(get_messages(response.wsgi_request)) self.assertEqual(len(messages), 1) @@ -111,17 +111,15 @@ def test_flavor_creation_deletion_superuser(self): self.assertEqual(n_flavors_before, n_flavors_after) + def test_flavor_deletion_notinuse_superuser(self): """ Test it is allowed to delete flavor that is not in use """ - - flavor_to_be_deleted = Flavor.objects.get(name="flavor-to-be-deleted") - - can_flavor_be_deleted = can_model_instance_be_deleted("flavor", flavor_to_be_deleted) + can_flavor_be_deleted = can_model_instance_be_deleted("flavor", self.flavor_to_be_deleted) self.assertTrue(can_flavor_be_deleted) n_flavors_before = Flavor.objects.count() - response = self.client.post(f"/projects/{project.slug}/deleteflavor/", {"flavor_pk": flavor_to_be_deleted.pk}) + response = self.client.post(f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": self.flavor_to_be_deleted.pk}) self.assertEqual(response.status_code, 302) n_flavors_after = Flavor.objects.count() From a322fb925e5f27819ceca31ad1f4ea9ad0815f15 Mon Sep 17 00:00:00 2001 From: akochari Date: Mon, 9 Dec 2024 17:31:57 +0100 Subject: [PATCH 25/28] fix precommit --- .../tests/test_create_delete_environments.py | 17 +++++++++++++---- projects/tests/test_create_delete_flavors.py | 13 ++++++++++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/projects/tests/test_create_delete_environments.py b/projects/tests/test_create_delete_environments.py index e9bac32a..f03eff24 100644 --- a/projects/tests/test_create_delete_environments.py +++ b/projects/tests/test_create_delete_environments.py @@ -18,7 +18,9 @@ def setUp(self): self.project = Project.objects.create_project(name="test-env", owner=user, description="") User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) self.app = Apps.objects.create(name="Some App", slug="someapp") - self.env_to_be_deleted = Environment.objects.create(app=self.app, name="env-to-be-deleted", project=self.project) + self.env_to_be_deleted = Environment.objects.create( + app=self.app, name="env-to-be-deleted", project=self.project + ) self.client.login(username=test_user["email"], password=test_user["password"]) def test_environment_creation_regular_user(self): @@ -52,13 +54,16 @@ def test_environment_deletion_regular_user(self): n_envs_after = Environment.objects.count() self.assertEqual(n_envs_before, n_envs_after) + class EnvironmentTestCaseSuperUser(TestCase): def setUp(self): self.user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) self.project = Project.objects.create_project(name="test-env", owner=self.user, description="") User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) self.app = Apps.objects.create(name="Some App", slug="someapp") - self.env_to_be_deleted = Environment.objects.create(app=self.app, name="env-to-be-deleted", project=self.project) + self.env_to_be_deleted = Environment.objects.create( + app=self.app, name="env-to-be-deleted", project=self.project + ) self.client.login(username=test_superuser["email"], password=test_superuser["password"]) def test_environment_creation_superuser(self): @@ -83,7 +88,9 @@ def test_environment_deletion_inuse_superuser(self): """ Test it is not allowed to delete environment that is in use """ - env_cannot_be_deleted = Environment.objects.create(app=self.app, name="env-cannot-be-deleted", project=self.project) + env_cannot_be_deleted = Environment.objects.create( + app=self.app, name="env-cannot-be-deleted", project=self.project + ) subdomain = Subdomain.objects.create(subdomain="test_internal") self.app_instance = JupyterInstance.objects.create( access="public", @@ -98,7 +105,9 @@ def test_environment_deletion_inuse_superuser(self): self.assertFalse(can_env_be_deleted) n_envs_before = Environment.objects.count() - response = self.client.post(f"/projects/{self.project.slug}/deleteenvironment/", {"environment_pk": env_cannot_be_deleted.pk}) + response = self.client.post( + f"/projects/{self.project.slug}/deleteenvironment/", {"environment_pk": env_cannot_be_deleted.pk} + ) self.assertEqual(response.status_code, 302) messages = list(get_messages(response.wsgi_request)) self.assertEqual(len(messages), 1) diff --git a/projects/tests/test_create_delete_flavors.py b/projects/tests/test_create_delete_flavors.py index 8a450acb..721aaeb0 100644 --- a/projects/tests/test_create_delete_flavors.py +++ b/projects/tests/test_create_delete_flavors.py @@ -45,11 +45,14 @@ def test_flavor_deletion_regular_user(self): Test regular user not allowed to delete flavor """ n_flavors_before = Flavor.objects.count() - response = self.client.post(f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": self.flavor_to_be_deleted.pk}) + response = self.client.post( + f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": self.flavor_to_be_deleted.pk} + ) self.assertEqual(response.status_code, 403) n_flavors_after = Flavor.objects.count() self.assertEqual(n_flavors_before, n_flavors_after) + class FlavorTestCaseSuperUser(TestCase): def setUp(self): self.user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) @@ -102,7 +105,9 @@ def test_flavor_deletion_inuse_superuser(self): self.assertFalse(can_flavor_be_deleted) n_flavors_before = Flavor.objects.count() - response = self.client.post(f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": flavor_cannot_be_deleted.pk}) + response = self.client.post( + f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": flavor_cannot_be_deleted.pk} + ) self.assertEqual(response.status_code, 302) messages = list(get_messages(response.wsgi_request)) self.assertEqual(len(messages), 1) @@ -119,7 +124,9 @@ def test_flavor_deletion_notinuse_superuser(self): self.assertTrue(can_flavor_be_deleted) n_flavors_before = Flavor.objects.count() - response = self.client.post(f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": self.flavor_to_be_deleted.pk}) + response = self.client.post( + f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": self.flavor_to_be_deleted.pk} + ) self.assertEqual(response.status_code, 302) n_flavors_after = Flavor.objects.count() From 8a9af69786b4137457495035c71195195faa2191 Mon Sep 17 00:00:00 2001 From: Arnold Kochari Date: Tue, 10 Dec 2024 13:30:49 +0100 Subject: [PATCH 26/28] uppercase names of global variables Co-authored-by: Nikita Churikov <8545082+churnikov@users.noreply.github.com> --- projects/tests/test_create_delete_environments.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/projects/tests/test_create_delete_environments.py b/projects/tests/test_create_delete_environments.py index f03eff24..41f1e8f7 100644 --- a/projects/tests/test_create_delete_environments.py +++ b/projects/tests/test_create_delete_environments.py @@ -8,8 +8,8 @@ User = get_user_model() -test_user = {"username": "foo1", "email": "foo@test.com", "password": "bar"} -test_superuser = {"username": "superuser", "email": "superuser@test.com", "password": "bar"} +TEST_USER = {"username": "foo1", "email": "foo@test.com", "password": "bar"} +TEST_SUPERUSER = {"username": "superuser", "email": "superuser@test.com", "password": "bar"} class EnvironmentTestCaseRegularUser(TestCase): From 02b6b50f78c955f873300bfab0a71b5edc2472be Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 10 Dec 2024 13:34:00 +0100 Subject: [PATCH 27/28] uppercase names for global variables --- projects/tests/test_create_delete_environments.py | 12 ++++++------ projects/tests/test_create_delete_flavors.py | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/projects/tests/test_create_delete_environments.py b/projects/tests/test_create_delete_environments.py index 41f1e8f7..b7dc1043 100644 --- a/projects/tests/test_create_delete_environments.py +++ b/projects/tests/test_create_delete_environments.py @@ -14,14 +14,14 @@ class EnvironmentTestCaseRegularUser(TestCase): def setUp(self): - user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) + user = User.objects.create_user(TEST_USER["username"], TEST_USER["email"], TEST_USER["password"]) self.project = Project.objects.create_project(name="test-env", owner=user, description="") - User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) + User.objects.create_superuser(TEST_SUPERUSER["username"], TEST_SUPERUSER["email"], TEST_SUPERUSER["password"]) self.app = Apps.objects.create(name="Some App", slug="someapp") self.env_to_be_deleted = Environment.objects.create( app=self.app, name="env-to-be-deleted", project=self.project ) - self.client.login(username=test_user["email"], password=test_user["password"]) + self.client.login(username=TEST_USER["email"], password=TEST_USER["password"]) def test_environment_creation_regular_user(self): """ @@ -57,14 +57,14 @@ def test_environment_deletion_regular_user(self): class EnvironmentTestCaseSuperUser(TestCase): def setUp(self): - self.user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) + self.user = User.objects.create_user(TEST_USER["username"], TEST_USER["email"], TEST_USER["password"]) self.project = Project.objects.create_project(name="test-env", owner=self.user, description="") - User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) + User.objects.create_superuser(TEST_SUPERUSER["username"], TEST_SUPERUSER["email"], TEST_SUPERUSER["password"]) self.app = Apps.objects.create(name="Some App", slug="someapp") self.env_to_be_deleted = Environment.objects.create( app=self.app, name="env-to-be-deleted", project=self.project ) - self.client.login(username=test_superuser["email"], password=test_superuser["password"]) + self.client.login(username=TEST_SUPERUSER["email"], password=TEST_SUPERUSER["password"]) def test_environment_creation_superuser(self): """ diff --git a/projects/tests/test_create_delete_flavors.py b/projects/tests/test_create_delete_flavors.py index 721aaeb0..5f13ea63 100644 --- a/projects/tests/test_create_delete_flavors.py +++ b/projects/tests/test_create_delete_flavors.py @@ -8,16 +8,16 @@ User = get_user_model() -test_user = {"username": "foo1", "email": "foo@test.com", "password": "bar"} -test_superuser = {"username": "superuser", "email": "superuser@test.com", "password": "bar"} +TEST_USER = {"username": "foo1", "email": "foo@test.com", "password": "bar"} +TEST_SUPERUSER = {"username": "superuser", "email": "superuser@test.com", "password": "bar"} class FlavorTestCaseRegularUser(TestCase): def setUp(self): - user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) + user = User.objects.create_user(TEST_USER["username"], TEST_USER["email"], TEST_USER["password"]) self.project = Project.objects.create_project(name="test-flavor", owner=user, description="") self.flavor_to_be_deleted = Flavor.objects.create(name="flavor-to-be-deleted", project=self.project) - self.client.login(username=test_user["email"], password=test_user["password"]) + self.client.login(username=TEST_USER["email"], password=TEST_USER["password"]) def test_flavor_creation_regular_user(self): """ @@ -55,11 +55,11 @@ def test_flavor_deletion_regular_user(self): class FlavorTestCaseSuperUser(TestCase): def setUp(self): - self.user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"]) + self.user = User.objects.create_user(TEST_USER["username"], TEST_USER["email"], TEST_USER["password"]) self.project = Project.objects.create_project(name="test-flavor", owner=self.user, description="") - User.objects.create_superuser(test_superuser["username"], test_superuser["email"], test_superuser["password"]) + User.objects.create_superuser(TEST_SUPERUSER["username"], TEST_SUPERUSER["email"], TEST_SUPERUSER["password"]) self.flavor_to_be_deleted = Flavor.objects.create(name="flavor-to-be-deleted", project=self.project) - self.client.login(username=test_superuser["email"], password=test_superuser["password"]) + self.client.login(username=TEST_SUPERUSER["email"], password=TEST_SUPERUSER["password"]) def test_flavor_creation_superuser(self): """ From fafe0db30c303722294c87df135524ac209ced9f Mon Sep 17 00:00:00 2001 From: akochari Date: Tue, 10 Dec 2024 13:48:52 +0100 Subject: [PATCH 28/28] follow redirects in tests --- projects/tests/test_create_delete_environments.py | 15 ++++++++++----- projects/tests/test_create_delete_flavors.py | 11 ++++++----- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/projects/tests/test_create_delete_environments.py b/projects/tests/test_create_delete_environments.py index b7dc1043..026ad41b 100644 --- a/projects/tests/test_create_delete_environments.py +++ b/projects/tests/test_create_delete_environments.py @@ -79,8 +79,9 @@ def test_environment_creation_superuser(self): "environment_image": "n", "environment_app": self.app.pk, }, + follow=True, ) - self.assertEqual(response.status_code, 302) + self.assertEqual(response.status_code, 200) n_envs_after = Environment.objects.count() self.assertEqual(n_envs_before + 1, n_envs_after) @@ -106,9 +107,11 @@ def test_environment_deletion_inuse_superuser(self): n_envs_before = Environment.objects.count() response = self.client.post( - f"/projects/{self.project.slug}/deleteenvironment/", {"environment_pk": env_cannot_be_deleted.pk} + f"/projects/{self.project.slug}/deleteenvironment/", + {"environment_pk": env_cannot_be_deleted.pk}, + follow=True, ) - self.assertEqual(response.status_code, 302) + self.assertEqual(response.status_code, 200) messages = list(get_messages(response.wsgi_request)) self.assertEqual(len(messages), 1) self.assertIn("cannot be deleted", str(messages[0])) @@ -124,9 +127,11 @@ def test_environment_deletion_notinuse_superuser(self): self.assertTrue(can_env_be_deleted) n_envs_before = Environment.objects.count() response = self.client.post( - f"/projects/{self.project.slug}/deleteenvironment/", {"environment_pk": self.env_to_be_deleted.pk} + f"/projects/{self.project.slug}/deleteenvironment/", + {"environment_pk": self.env_to_be_deleted.pk}, + follow=True, ) - self.assertEqual(response.status_code, 302) + self.assertEqual(response.status_code, 200) n_envs_after = Environment.objects.count() self.assertEqual(n_envs_before - 1, n_envs_after) diff --git a/projects/tests/test_create_delete_flavors.py b/projects/tests/test_create_delete_flavors.py index 5f13ea63..bd81f256 100644 --- a/projects/tests/test_create_delete_flavors.py +++ b/projects/tests/test_create_delete_flavors.py @@ -77,8 +77,9 @@ def test_flavor_creation_superuser(self): "mem_lim": "n", "ephmem_lim": "n", }, + follow=True, ) - self.assertEqual(response.status_code, 302) + self.assertEqual(response.status_code, 200) n_flavors_after = Flavor.objects.count() self.assertEqual(n_flavors_before + 1, n_flavors_after) @@ -106,9 +107,9 @@ def test_flavor_deletion_inuse_superuser(self): n_flavors_before = Flavor.objects.count() response = self.client.post( - f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": flavor_cannot_be_deleted.pk} + f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": flavor_cannot_be_deleted.pk}, follow=True ) - self.assertEqual(response.status_code, 302) + self.assertEqual(response.status_code, 200) messages = list(get_messages(response.wsgi_request)) self.assertEqual(len(messages), 1) self.assertIn("cannot be deleted", str(messages[0])) @@ -125,9 +126,9 @@ def test_flavor_deletion_notinuse_superuser(self): n_flavors_before = Flavor.objects.count() response = self.client.post( - f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": self.flavor_to_be_deleted.pk} + f"/projects/{self.project.slug}/deleteflavor/", {"flavor_pk": self.flavor_to_be_deleted.pk}, follow=True ) - self.assertEqual(response.status_code, 302) + self.assertEqual(response.status_code, 200) n_flavors_after = Flavor.objects.count() self.assertEqual(n_flavors_before - 1, n_flavors_after)