diff --git a/docs/changelog/next_release/134.feature.rst b/docs/changelog/next_release/134.feature.rst new file mode 100644 index 00000000..d6565d5a --- /dev/null +++ b/docs/changelog/next_release/134.feature.rst @@ -0,0 +1 @@ +Provide read-only rights for the previous group owner when ownership is transferred \ No newline at end of file diff --git a/syncmaster/db/repositories/group.py b/syncmaster/db/repositories/group.py index 8c593c76..1c6e212f 100644 --- a/syncmaster/db/repositories/group.py +++ b/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, diff --git a/tests/test_unit/test_connections/connection_fixtures/group_connections_fixture.py b/tests/test_unit/test_connections/connection_fixtures/group_connections_fixture.py index 3c599207..b9b2527e 100644 --- a/tests/test_unit/test_connections/connection_fixtures/group_connections_fixture.py +++ b/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, diff --git a/tests/test_unit/test_groups/test_update_group_by_id.py b/tests/test_unit/test_groups/test_update_group_by_id.py index 865cc690..d84ce9c1 100644 --- a/tests/test_unit/test_groups/test_update_group_by_id.py +++ b/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(