refactor: use typed patch instance for group updates (#487)

See #485.

---------

Co-authored-by: Ralf Grubenmann <[email protected]>

components/renku_data_services/namespace/blueprints.py

@@ -15,6 +15,7 @@
 from renku_data_services.base_models.validation import validate_and_dump, validated_json
 from renku_data_services.errors import errors
 from renku_data_services.namespace import apispec, models
+from renku_data_services.namespace.core import validate_group_patch
 from renku_data_services.namespace.db import GroupRepository
 
 
@@ -87,8 +88,8 @@ def patch(self) -> BlueprintFactoryResponse:
         async def _patch(
             _: Request, user: base_models.APIUser, slug: str, body: apispec.GroupPatchRequest
         ) -> JSONResponse:
-            body_dict = body.model_dump(exclude_none=True)
-            res = await self.group_repo.update_group(user=user, slug=slug, payload=body_dict)
+            group_patch = validate_group_patch(body)
+            res = await self.group_repo.update_group(user=user, slug=slug, patch=group_patch)
             return validated_json(apispec.GroupResponse, res)
 
         return "/groups/<slug:renku_slug>", ["PATCH"], _patch

components/renku_data_services/namespace/core.py

@@ -0,0 +1,12 @@
+"""Business logic for groups and namespaces."""
+
+from renku_data_services.namespace import apispec, models
+
+
+def validate_group_patch(patch: apispec.GroupPatchRequest) -> models.GroupPatch:
+    """Validate the update to a group."""
+    return models.GroupPatch(
+        slug=patch.slug,
+        name=patch.name,
+        description=patch.description,
+    )

components/renku_data_services/namespace/db.py

@@ -171,40 +171,41 @@ async def get_group_members(
     @with_db_transaction
     @dispatch_message(GroupUpdated)
     async def update_group(
-        self, user: base_models.APIUser, slug: str, payload: dict[str, str], *, session: AsyncSession | None = None
+        self, user: base_models.APIUser, slug: str, patch: models.GroupPatch, *, session: AsyncSession | None = None
     ) -> models.Group:
         """Update a group in the DB."""
         if not session:
             raise errors.ProgrammingError(message="A database session is required")
         group, _ = await self._get_group(session, user, slug)
-        if user.id != group.created_by and not user.is_admin:
-            raise errors.ForbiddenError(message="Only the owner and admins can modify groups")
         if group.namespace.slug != slug.lower():
             raise errors.UpdatingWithStaleContentError(
                 message=f"You cannot update a group by using its old slug {slug}.",
                 detail=f"The latest slug is {group.namespace.slug}, please use this for updates.",
             )
-        for k, v in payload.items():
-            match k:
-                case "slug":
-                    new_slug_str = v.lower()
-                    if group.namespace.slug == new_slug_str:
-                        # The slug has not changed at all.
-                        # NOTE that the continue will work only because of the enclosing loop over the payload
-                        continue
-                    new_slug_already_taken = await session.scalar(
-                        select(schemas.NamespaceORM).where(schemas.NamespaceORM.slug == new_slug_str)
-                    )
-                    if new_slug_already_taken:
-                        raise errors.ValidationError(
-                            message=f"The slug {v} is already in use, please try a different one"
-                        )
-                    session.add(schemas.NamespaceOldORM(slug=group.namespace.slug, latest_slug_id=group.namespace.id))
-                    group.namespace.slug = new_slug_str
-                case "description":
-                    group.description = v
-                case "name":
-                    group.name = v
+
+        authorized = await self.authz.has_permission(user, ResourceType.group, group.id, Scope.DELETE)
+        if not authorized:
+            raise errors.MissingResourceError(
+                message=f"Group with slug '{slug}' does not exist or you do not have access to it."
+            )
+
+        new_slug_str = patch.slug.lower() if patch.slug is not None else None
+        if new_slug_str is not None and new_slug_str != group.namespace.slug:
+            # Only update if the slug has changed.
+            new_slug_already_taken = await session.scalar(
+                select(schemas.NamespaceORM).where(schemas.NamespaceORM.slug == new_slug_str)
+            )
+            if new_slug_already_taken:
+                raise errors.ValidationError(
+                    message=f"The slug {new_slug_str} is already in use, please try a different one"
+                )
+            session.add(schemas.NamespaceOldORM(slug=group.namespace.slug, latest_slug_id=group.namespace.id))
+            group.namespace.slug = new_slug_str
+        if patch.name is not None:
+            group.name = patch.name
+        if patch.description is not None:
+            group.description = patch.description if patch.description else None
+
         return group.dump()
 
     @with_db_transaction

components/renku_data_services/namespace/models.py

@@ -75,6 +75,15 @@ class Namespace:
     name: str | None = None
 
 
+@dataclass(frozen=True, eq=True, kw_only=True)
+class GroupPatch:
+    """Model for changes requested on a group."""
+
+    slug: str | None
+    name: str | None
+    description: str | None
+
+
 @dataclass
 class GroupUpdate:
     """Information about the update of a group."""