diff --git a/docs/source/specs/typespec/main.tsp b/docs/source/specs/typespec/main.tsp index 3da15b9d7..95d07f40b 100644 --- a/docs/source/specs/typespec/main.tsp +++ b/docs/source/specs/typespec/main.tsp @@ -98,7 +98,7 @@ namespace Workspaces { model Workspace { @key uuid: UUID; - parent?: UUID; + parent_id?: UUID; ...BasicWorkspace; ...Timestamps; } diff --git a/docs/source/specs/v2/openapi.v2.yaml b/docs/source/specs/v2/openapi.v2.yaml index 99e18285e..b6eaf9dff 100644 --- a/docs/source/specs/v2/openapi.v2.yaml +++ b/docs/source/specs/v2/openapi.v2.yaml @@ -666,7 +666,7 @@ components: properties: uuid: $ref: '#/components/schemas/UUID' - parent: + parent_id: $ref: '#/components/schemas/UUID' name: type: string diff --git a/rbac/management/migrations/0049_alter_workspace_parent.py b/rbac/management/migrations/0049_alter_workspace_parent.py new file mode 100644 index 000000000..126776fb2 --- /dev/null +++ b/rbac/management/migrations/0049_alter_workspace_parent.py @@ -0,0 +1,26 @@ +# Generated by Django 4.2.14 on 2024-08-29 19:42 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("management", "0048_outbox"), + ] + + operations = [ + migrations.AlterField( + model_name="workspace", + name="parent", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="children", + to="management.workspace", + to_field="uuid", + ), + ), + ] diff --git a/rbac/management/workspace/model.py b/rbac/management/workspace/model.py index 5e901c362..0de8bf594 100644 --- a/rbac/management/workspace/model.py +++ b/rbac/management/workspace/model.py @@ -29,7 +29,9 @@ class Workspace(TenantAwareModel): name = models.CharField(max_length=255) uuid = models.UUIDField(default=uuid4, editable=False, unique=True, null=False) - parent = models.UUIDField(null=True, blank=True, editable=True) + parent = models.ForeignKey( + "self", to_field="uuid", on_delete=models.PROTECT, related_name="children", null=True, blank=True + ) description = models.CharField(max_length=255, null=True, blank=True, editable=True) created = models.DateTimeField(default=timezone.now) modified = AutoDateTimeField(default=timezone.now) diff --git a/rbac/management/workspace/serializer.py b/rbac/management/workspace/serializer.py index d5a187d08..8bbbf5172 100644 --- a/rbac/management/workspace/serializer.py +++ b/rbac/management/workspace/serializer.py @@ -27,20 +27,17 @@ class WorkspaceSerializer(serializers.ModelSerializer): name = serializers.CharField(required=False, max_length=255) uuid = serializers.UUIDField(read_only=True, required=False) description = serializers.CharField(allow_null=True, required=False, max_length=255) - parent = serializers.UUIDField(allow_null=True, required=False) + parent_id = serializers.UUIDField(allow_null=True, required=False) class Meta: """Metadata for the serializer.""" model = Workspace - fields = ("name", "uuid", "parent", "description") + fields = ("name", "uuid", "parent_id", "description") def create(self, validated_data): """Create the workspace object in the database.""" - name = validated_data.pop("name") - description = validated_data.pop("description", "") - tenant = self.context["request"].tenant - parent = validated_data.pop("parent", None) + validated_data["tenant"] = self.context["request"].tenant - workspace = Workspace.objects.create(name=name, description=description, parent=parent, tenant=tenant) + workspace = Workspace.objects.create(**validated_data) return workspace diff --git a/rbac/management/workspace/view.py b/rbac/management/workspace/view.py index 2b92a0673..bda272026 100644 --- a/rbac/management/workspace/view.py +++ b/rbac/management/workspace/view.py @@ -27,8 +27,8 @@ from .model import Workspace from .serializer import WorkspaceSerializer -VALID_PATCH_FIELDS = ["name", "description", "parent"] -REQUIRED_PUT_FIELDS = ["name", "description", "parent"] +VALID_PATCH_FIELDS = ["name", "description", "parent_id"] +REQUIRED_PUT_FIELDS = ["name", "description", "parent_id"] REQUIRED_CREATE_FIELDS = ["name"] @@ -66,7 +66,7 @@ def retrieve(self, request, *args, **kwargs): def destroy(self, request, *args, **kwargs): """Delete a workspace.""" instance = self.get_object() - if Workspace.objects.filter(parent=instance.uuid, tenant=instance.tenant).exists(): + if Workspace.objects.filter(parent=instance, tenant=instance.tenant).exists(): message = "Unable to delete due to workspace dependencies" error = {"workspace": [_(message)]} raise serializers.ValidationError(error) @@ -94,9 +94,9 @@ def partial_update(self, request, *args, **kwargs): def update_validation(self, request): """Validate a workspace for update.""" instance = self.get_object() - parent = request.data.get("parent") - if str(instance.uuid) == parent: - message = "Parent and UUID can't be same" + parent_id = request.data.get("parent_id") + if str(instance.uuid) == parent_id: + message = "Parent ID and UUID can't be same" error = {"workspace": [_(message)]} raise serializers.ValidationError(error) @@ -110,18 +110,19 @@ def validate_required_fields(self, request, required_fields): def validate_workspace(self, request, action="create"): """Validate a workspace.""" - parent = request.data.get("parent") + parent_id = request.data.get("parent_id") tenant = request.tenant if action == "create": self.validate_required_fields(request, REQUIRED_CREATE_FIELDS) else: self.validate_required_fields(request, REQUIRED_PUT_FIELDS) - if parent is None: - message = "Field 'parent' can't be null." + if parent_id is None: + message = "Field 'parent_id' can't be null." error = {"workspace": [_(message)]} raise serializers.ValidationError(error) - validate_uuid(parent) - if not Workspace.objects.filter(uuid=parent, tenant=tenant).exists(): - message = f"Parent workspace '{parent}' doesn't exist in tenant" + if parent_id: + validate_uuid(parent_id) + if not Workspace.objects.filter(uuid=parent_id, tenant=tenant).exists(): + message = f"Parent workspace '{parent_id}' doesn't exist in tenant" error = {"workspace": [message]} raise serializers.ValidationError(error) diff --git a/tests/management/workspace/test_model.py b/tests/management/workspace/test_model.py new file mode 100644 index 000000000..4667fec86 --- /dev/null +++ b/tests/management/workspace/test_model.py @@ -0,0 +1,47 @@ +# +# Copyright 2024 Red Hat, Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +"""Test the workspace model.""" +from management.models import Workspace +from tests.identity_request import IdentityRequest + +from django.db.models import ProtectedError + + +class WorkspaceModelTests(IdentityRequest): + """Test the workspace model.""" + + def setUp(self): + """Set up the workspace model tests.""" + super().setUp() + + def tearDown(self): + """Tear down workspace model tests.""" + Workspace.objects.update(parent=None) + Workspace.objects.all().delete() + + def test_child_parent_relations(self): + """Test that workspaces can add/have parents as well as children""" + parent = Workspace.objects.create(name="Parent", tenant=self.tenant) + child = Workspace.objects.create(name="Child", tenant=self.tenant, parent=parent) + self.assertEqual(child.parent, parent) + self.assertEqual(list(parent.children.all()), [child]) + + def test_delete_fails_when_children(self): + """Test that workspaces will not be deleted when children exist""" + parent = Workspace.objects.create(name="Parent", tenant=self.tenant) + child = Workspace.objects.create(name="Child", tenant=self.tenant, parent=parent) + self.assertRaises(ProtectedError, parent.delete) diff --git a/tests/management/workspace/test_serializer.py b/tests/management/workspace/test_serializer.py new file mode 100644 index 000000000..0afbf806e --- /dev/null +++ b/tests/management/workspace/test_serializer.py @@ -0,0 +1,65 @@ +# +# Copyright 2024 Red Hat, Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +from django.test import TestCase +from unittest.mock import Mock +from api.models import Tenant +from management.models import Workspace +from management.workspace.serializer import WorkspaceSerializer +import uuid + + +class WorkspaceSerializerTest(TestCase): + """Test the workspace serializer""" + + def setUp(self): + """Set up workspace serializer tests.""" + tenant = Tenant.objects.get(tenant_name="public") + self.parent = Workspace.objects.create( + name="Parent", description="Parent desc", tenant=tenant, uuid=uuid.uuid4() + ) + self.child = Workspace.objects.create( + name="Child", description="Child desc", tenant=tenant, parent=self.parent, uuid=uuid.uuid4() + ) + + def tearDown(self): + """Tear down workspace serializer tests.""" + Workspace.objects.update(parent=None) + Workspace.objects.all().delete() + + def test_get_workspace_detail_child(self): + """Return GET /workspace// serializer response for child""" + serializer = WorkspaceSerializer(self.child) + expected_data = { + "uuid": str(self.child.uuid), + "name": self.child.name, + "description": self.child.description, + "parent_id": str(self.parent.uuid), + } + + self.assertDictEqual(serializer.data, expected_data) + + def test_get_workspace_detail_parent(self): + """Return GET /workspace// serializer response for parent""" + serializer = WorkspaceSerializer(self.parent) + expected_data = { + "uuid": str(self.parent.uuid), + "name": self.parent.name, + "description": self.parent.description, + "parent_id": None, + } + + self.assertDictEqual(serializer.data, expected_data) diff --git a/tests/management/workspace/test_view.py b/tests/management/workspace/test_view.py index 0caf0d8ec..45a11b290 100644 --- a/tests/management/workspace/test_view.py +++ b/tests/management/workspace/test_view.py @@ -42,6 +42,7 @@ def setUp(self): def tearDown(self): """Tear down group model tests.""" + Workspace.objects.update(parent=None) Workspace.objects.all().delete() def test_create_workspace(self): @@ -50,11 +51,11 @@ def test_create_workspace(self): "name": "New Workspace", "description": "New Workspace - description", "tenant_id": self.tenant.id, - "parent": "cbe9822d-cadb-447d-bc80-8bef773c36ea", + "parent_id": "cbe9822d-cadb-447d-bc80-8bef773c36ea", } parent_workspace = Workspace.objects.create(**workspace_data) - workspace = {"name": "New Workspace", "description": "Workspace", "parent": parent_workspace.uuid} + workspace = {"name": "New Workspace", "description": "Workspace", "parent_id": parent_workspace.uuid} url = reverse("workspace-list") client = APIClient() @@ -125,12 +126,12 @@ def test_duplicate_create_workspace(self): "name": "New Workspace", "description": "New Workspace - description", "tenant_id": self.tenant.id, - "parent": self.init_workspace.uuid, + "parent_id": self.init_workspace.uuid, } Workspace.objects.create(**workspace_data) - test_data = {"name": "New Workspace", "parent": self.init_workspace.uuid} + test_data = {"name": "New Workspace", "parent_id": self.init_workspace.uuid} url = reverse("workspace-list") client = APIClient() @@ -143,7 +144,7 @@ def test_update_workspace(self): "name": "New Workspace", "description": "New Workspace - description", "tenant_id": self.tenant.id, - "parent": self.init_workspace.uuid, + "parent_id": self.init_workspace.uuid, } workspace = Workspace.objects.create(**workspace_data) @@ -153,7 +154,7 @@ def test_update_workspace(self): workspace_data["name"] = "Updated name" workspace_data["description"] = "Updated description" - workspace_data["parent"] = workspace.parent + workspace_data["parent_id"] = workspace.parent_id response = client.put(url, workspace_data, format="json", **self.headers) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.data @@ -174,7 +175,7 @@ def test_partial_update_workspace_with_put_method(self): "name": "New Workspace", "description": "New Workspace - description", "tenant_id": self.tenant.id, - "parent": "cbe9822d-cadb-447d-bc80-8bef773c36ea", + "parent_id": "cbe9822d-cadb-447d-bc80-8bef773c36ea", } workspace = Workspace.objects.create(**workspace_data) @@ -201,7 +202,7 @@ def test_update_workspace_same_parent(self): "name": "New Workspace", "description": "New Workspace - description", "tenant_id": self.tenant.id, - "parent": "cbe9822d-cadb-447d-bc80-8bef773c36ea", + "parent_id": "cbe9822d-cadb-447d-bc80-8bef773c36ea", } parent_workspace = Workspace.objects.create(**parent_workspace_data) @@ -210,7 +211,7 @@ def test_update_workspace_same_parent(self): "name": "New Workspace", "description": "New Workspace - description", "tenant_id": self.tenant.id, - "parent": parent_workspace.uuid, + "parent_id": parent_workspace.uuid, } workspace = Workspace.objects.create(**workspace_data) @@ -218,7 +219,7 @@ def test_update_workspace_same_parent(self): url = reverse("workspace-detail", kwargs={"uuid": workspace.uuid}) client = APIClient() - workspace_request_data = {"name": "New Workspace", "parent": workspace.uuid, "description": "XX"} + workspace_request_data = {"name": "New Workspace", "parent_id": workspace.uuid, "description": "XX"} response = client.put(url, workspace_request_data, format="json", **self.headers) @@ -226,7 +227,7 @@ def test_update_workspace_same_parent(self): status_code = response.data.get("status") detail = response.data.get("detail") self.assertIsNotNone(detail) - self.assertEqual(detail, "Parent and UUID can't be same") + self.assertEqual(detail, "Parent ID and UUID can't be same") self.assertEqual(status_code, 400) def test_update_workspace_parent_doesnt_exist(self): @@ -246,7 +247,7 @@ def test_update_workspace_parent_doesnt_exist(self): parent = "cbe9822d-cadb-447d-bc80-8bef773c36ea" workspace_request_data = { "name": "New Workspace", - "parent": parent, + "parent_id": parent, "description": "XX", } @@ -267,7 +268,7 @@ def test_partial_update_workspace(self): "name": "New Workspace", "description": "New Workspace - description", "tenant_id": self.tenant.id, - "parent": None, + "parent_id": None, } workspace = Workspace.objects.create(**workspace_data) @@ -310,7 +311,7 @@ def test_update_duplicate_workspace(self): "name": "New Duplicate Workspace", "description": "New Duplicate Workspace - description", "tenant_id": self.tenant.id, - "parent": self.init_workspace.uuid, + "parent_id": self.init_workspace.uuid, } Workspace.objects.create(**workspace_data) @@ -319,7 +320,7 @@ def test_update_duplicate_workspace(self): "name": "New Duplicate Workspace for Update", "description": "New Duplicate Workspace - description", "tenant_id": self.tenant.id, - "parent": self.init_workspace.uuid, + "parent_id": self.init_workspace.uuid, } workspace_for_update = Workspace.objects.create(**workspace_data_for_update) @@ -330,7 +331,7 @@ def test_update_duplicate_workspace(self): workspace_data_for_put = { "name": "New Duplicate Workspace", "description": "New Duplicate Workspace - description", - "parent": self.init_workspace.uuid, + "parent_id": self.init_workspace.uuid, } response = client.put(url, workspace_data_for_put, format="json", **self.headers) @@ -454,5 +455,5 @@ def test_get_workspace_list(self): self.assertEqual(payload.get("meta").get("count"), Workspace.objects.count()) for keyname in ["meta", "links", "data"]: self.assertIn(keyname, payload) - for keyname in ["name", "uuid", "parent", "description"]: + for keyname in ["name", "uuid", "parent_id", "description"]: self.assertIn(keyname, payload.get("data")[0])