[DOP-19926] Provide guest role when changing group owner

Author: Ilyas Gasanov

docs/changelog/next_release/135.feature.rst

@@ -0,0 +1 @@
+Grant read-only rights for the previous group owner when ownership is transferred

syncmaster/db/repositories/group.py

@@ -186,9 +186,11 @@ async def update(
         description: str,
         owner_id: int,
     ) -> Group:
+        group = await self.read_by_id(group_id=group_id)
+        previous_owner_id = group.owner_id
         args = [Group.id == group_id, Group.is_deleted.is_(False)]
         try:
-            return await self._update(
+            updated_group = await self._update(
                 *args,
                 name=name,
                 description=description,
@@ -199,6 +201,21 @@ async def update(
         except IntegrityError as e:
             self._raise_error(e)
 
+        if previous_owner_id != owner_id:
+            previous_user_group = await self._session.get(
+                UserGroup,
+                {
+                    "group_id": group_id,
+                    "user_id": previous_owner_id,
+                },
+            )
+            if previous_user_group:
+                await self.update_member_role(group_id=group_id, user_id=previous_owner_id, role=GroupMemberRole.Guest)
+            else:
+                await self.add_user(group_id=group_id, new_user_id=previous_owner_id, role=GroupMemberRole.Guest)
+
+        return updated_group
+
     async def get_member_role(self, group_id: int, user_id: int) -> GroupMemberRole:
         user_group = await self._session.get(
             UserGroup,

tests/test_unit/test_connections/connection_fixtures/group_connections_fixture.py

@@ -41,7 +41,6 @@ async def group_connections(
             )
         elif conn_type in [
             ConnectionType.POSTGRES,
-            ConnectionType.ORACLE,
             ConnectionType.CLICKHOUSE,
             ConnectionType.MSSQL,
             ConnectionType.MYSQL,

tests/test_unit/test_groups/test_update_group_by_id.py

@@ -192,24 +192,87 @@ async def test_validation_on_update_group(
 
 
 async def test_owner_change_group_owner(client: AsyncClient, empty_group: MockGroup, simple_user: MockUser):
+    # Arrange
+    previous_owner = empty_group.owner
+    user = empty_group.get_member_of_role(UserTestRoles.Owner)
     # Act
-    result = await client.patch(
+    patch_result = await client.patch(
         f"v1/groups/{empty_group.id}",
-        headers={"Authorization": f"Bearer {empty_group.get_member_of_role(UserTestRoles.Owner).token}"},
+        headers={"Authorization": f"Bearer {user.token}"},
         json={
             "name": empty_group.name,
             "owner_id": simple_user.id,
             "description": empty_group.description,
         },
     )
+    group_users_result = await client.get(
+        f"v1/groups/{empty_group.id}/users",
+        headers={"Authorization": f"Bearer {user.token}"},
+    )
     # Assert
-    assert result.json() == {
+    assert patch_result.status_code == 200
+    assert patch_result.json() == {
         "id": empty_group.id,
         "name": empty_group.name,
         "owner_id": simple_user.id,
         "description": empty_group.description,
     }
-    assert result.status_code == 200
+    assert group_users_result.status_code == 200
+    assert group_users_result.json()["items"] == [
+        {
+            "id": previous_owner.id,
+            "username": previous_owner.username,
+            "role": UserTestRoles.Guest,
+        },
+    ]
+
+
+async def test_owner_with_existing_role_change_group_owner(
+    client: AsyncClient,
+    empty_group: MockGroup,
+    simple_user: MockUser,
+    role_maintainer_or_below: UserTestRoles,
+):
+    # Arrange
+    previous_owner = empty_group.owner
+    user = empty_group.get_member_of_role(UserTestRoles.Owner)
+    # Act
+    await client.post(
+        f"v1/groups/{empty_group.id}/users/{empty_group.owner_id}",
+        headers={"Authorization": f"Bearer {user.token}"},
+        json={
+            "role": role_maintainer_or_below,
+        },
+    )
+    patch_result = await client.patch(
+        f"v1/groups/{empty_group.id}",
+        headers={"Authorization": f"Bearer {user.token}"},
+        json={
+            "name": empty_group.name,
+            "owner_id": simple_user.id,
+            "description": empty_group.description,
+        },
+    )
+    group_users_result = await client.get(
+        f"v1/groups/{empty_group.id}/users",
+        headers={"Authorization": f"Bearer {user.token}"},
+    )
+    # Assert
+    assert patch_result.status_code == 200
+    assert patch_result.json() == {
+        "id": empty_group.id,
+        "name": empty_group.name,
+        "owner_id": simple_user.id,
+        "description": empty_group.description,
+    }
+    assert group_users_result.status_code == 200
+    assert group_users_result.json()["items"] == [
+        {
+            "id": previous_owner.id,
+            "username": previous_owner.username,
+            "role": UserTestRoles.Guest,
+        },
+    ]
 
 
 async def test_maintainer_or_below_cannot_change_group_owner(