fix(members): drop the duplicate user refetch after a role change

The hook invalidated the user query while EditMemberDrawer also refetches
it on every save and retry path, so a role change fired two identical
GET /api/v2/users/{id} calls. This was hidden before, when the hook owned
a separate roles query.

The drawer already refreshes on all four paths, so ownership stays there.

Assisted-by: Claude Fable 5

frontend/src/components/EditMemberDrawer/__tests__/EditMemberDrawer.test.tsx

@@ -5,10 +5,9 @@ import {
 	useCreateResetPasswordToken,
 	useDeleteUser,
 	useGetResetPasswordToken,
-	useGetRolesByUserID,
+	useCreateUserRole,
+	useDeleteUserRole,
 	useGetUser,
-	useRemoveUserRoleByUserIDAndRoleID,
-	useSetRoleByUserID,
 	useUpdateMyUserV2,
 	useUpdateUser,
 } from 'api/generated/services/users';
@@ -25,15 +24,14 @@ import EditMemberDrawer, { EditMemberDrawerProps } from '../EditMemberDrawer';
 jest.mock('api/generated/services/users', () => ({
 	useDeleteUser: jest.fn(),
 	useGetUser: jest.fn(),
-	useGetRolesByUserID: jest.fn(),
-	useRemoveUserRoleByUserIDAndRoleID: jest.fn(),
+	useDeleteUserRole: jest.fn(),
 	useUpdateUser: jest.fn(),
 	useUpdateMyUserV2: jest.fn(),
-	useSetRoleByUserID: jest.fn(),
+	useCreateUserRole: jest.fn(),
 	useGetResetPasswordToken: jest.fn(),
 	useCreateResetPasswordToken: jest.fn(),
-	getGetRolesByUserIDQueryKey: ({ id }: { id: string }): string[] => [
-		`/api/v2/users/${id}/roles`,
+	getGetUserQueryKey: ({ id }: { id: string }): string[] => [
+		`/api/v2/users/${id}`,
 	],
 }));
 
@@ -194,11 +192,7 @@ describe('EditMemberDrawer', () => {
 			isLoading: false,
 			refetch: jest.fn(),
 		});
-		(useGetRolesByUserID as jest.Mock).mockReturnValue({
-			data: { data: [managedRoles[0]] },
-			isLoading: false,
-		});
-		(useRemoveUserRoleByUserIDAndRoleID as jest.Mock).mockReturnValue({
+		(useDeleteUserRole as jest.Mock).mockReturnValue({
 			mutateAsync: mockRemoveMutateAsync.mockResolvedValue({}),
 			isLoading: false,
 		});
@@ -210,7 +204,7 @@ describe('EditMemberDrawer', () => {
 			mutateAsync: jest.fn().mockResolvedValue({}),
 			isLoading: false,
 		});
-		(useSetRoleByUserID as jest.Mock).mockReturnValue({
+		(useCreateUserRole as jest.Mock).mockReturnValue({
 			mutateAsync: jest.fn().mockResolvedValue({}),
 			isLoading: false,
 		});
@@ -312,12 +306,12 @@ describe('EditMemberDrawer', () => {
 		expect(onClose).not.toHaveBeenCalled();
 	});
 
-	it('adding a new role calls setRole without removing existing ones', async () => {
+	it('adding a new role creates a user role without removing existing ones', async () => {
 		const onComplete = jest.fn();
 		const user = userEvent.setup({ pointerEventsCheck: 0 });
 		const mockSet = jest.fn().mockResolvedValue({});
 
-		(useSetRoleByUserID as jest.Mock).mockReturnValue({
+		(useCreateUserRole as jest.Mock).mockReturnValue({
 			mutateAsync: mockSet,
 			isLoading: false,
 		});
@@ -334,15 +328,14 @@ describe('EditMemberDrawer', () => {
 
 		await waitFor(() => {
 			expect(mockSet).toHaveBeenCalledWith({
-				pathParams: { id: 'user-1' },
-				data: { name: 'signoz-editor' },
+				data: { userId: 'user-1', roleId: managedRoles[1].id },
 			});
 			expect(mockRemoveMutateAsync).not.toHaveBeenCalled();
 			expect(onComplete).toHaveBeenCalled();
 		});
 	});
 
-	it('deselecting a role calls removeRole with the role id', async () => {
+	it('deselecting a role deletes the user role by its assignment id', async () => {
 		const onComplete = jest.fn();
 		const user = userEvent.setup({ pointerEventsCheck: 0 });
 
@@ -361,7 +354,7 @@ describe('EditMemberDrawer', () => {
 
 		await waitFor(() => {
 			expect(mockRemoveMutateAsync).toHaveBeenCalledWith({
-				pathParams: { id: 'user-1', roleId: managedRoles[0].id },
+				pathParams: { id: 'ur-1' },
 			});
 			expect(onComplete).toHaveBeenCalled();
 		});

frontend/src/hooks/member/useMemberRoleManager.ts

@@ -1,16 +1,13 @@
 import { useCallback, useMemo } from 'react';
-import { useQueryClient } from 'react-query';
 import type { AuthtypesGettableRoleDTO } from 'api/generated/services/sigNoz.schemas';
 import {
-	getGetRolesByUserIDQueryKey,
-	useGetRolesByUserID,
-	useRemoveUserRoleByUserIDAndRoleID,
-	useSetRoleByUserID,
+	useCreateUserRole,
+	useDeleteUserRole,
+	useGetUser,
 } from 'api/generated/services/users';
 import { retryOn429 } from 'utils/errorUtils';
 
 const enum PromiseStatus {
-	Fulfilled = 'fulfilled',
 	Rejected = 'rejected',
 }
 
@@ -33,31 +30,34 @@ export function useMemberRoleManager(
 	userId: string,
 	enabled: boolean,
 ): UseMemberRoleManagerResult {
-	const queryClient = useQueryClient();
-
-	const { data, isLoading } = useGetRolesByUserID(
+	const { data, isLoading } = useGetUser(
 		{ id: userId },
 		{ query: { enabled: !!userId && enabled } },
 	);
 
+	const userRoles = useMemo(
+		() => data?.data?.userRoles ?? [],
+		[data?.data?.userRoles],
+	);
+
 	const currentRoles = useMemo<AuthtypesGettableRoleDTO[]>(
-		() => data?.data ?? [],
-		[data?.data],
+		() => userRoles.map((userRole) => userRole.role),
+		[userRoles],
 	);
 
-	const { mutateAsync: setRole } = useSetRoleByUserID({
-		mutation: { retry: retryOn429 },
-	});
-	const { mutateAsync: removeRole } = useRemoveUserRoleByUserIDAndRoleID({
-		mutation: { retry: retryOn429 },
-	});
-
-	const invalidateRoles = useCallback(
-		() =>
-			queryClient.invalidateQueries(getGetRolesByUserIDQueryKey({ id: userId })),
-		[userId, queryClient],
+	// DELETE /api/v2/user_roles/{id} is keyed by the user_role join row, not the role.
+	const assignmentIdByRoleId = useMemo(
+		() => new Map(userRoles.map((userRole) => [userRole.roleId, userRole.id])),
+		[userRoles],
 	);
 
+	const { mutateAsync: createUserRole } = useCreateUserRole({
+		mutation: { retry: retryOn429 },
+	});
+	const { mutateAsync: deleteUserRole } = useDeleteUserRole({
+		mutation: { retry: retryOn429 },
+	});
+
 	const applyDiff = useCallback(
 		async (
 			localRoleIds: string[],
@@ -80,30 +80,33 @@ export function useMemberRoleManager(
 			const allOperations = [
 				...addedRoles.map((role) => ({
 					role,
-					run: (): ReturnType<typeof setRole> =>
-						setRole({
-							pathParams: { id: userId },
-							data: { name: role.name ?? '' },
-						}),
-				})),
-				...removedRoles.map((role) => ({
-					role,
-					run: (): ReturnType<typeof removeRole> =>
-						removeRole({ pathParams: { id: userId, roleId: role.id ?? '' } }),
+					run: (): ReturnType<typeof createUserRole> =>
+						createUserRole({ data: { userId, roleId: role.id ?? '' } }),
 				})),
+				...removedRoles
+					.map((role) => ({
+						role,
+						assignmentId: assignmentIdByRoleId.get(role.id ?? ''),
+					}))
+					.filter(
+						(
+							entry,
+						): entry is {
+							role: AuthtypesGettableRoleDTO;
+							assignmentId: string;
+						} => !!entry.assignmentId,
+					)
+					.map(({ role, assignmentId }) => ({
+						role,
+						run: (): ReturnType<typeof deleteUserRole> =>
+							deleteUserRole({ pathParams: { id: assignmentId } }),
+					})),
 			];
 
 			const results = await Promise.allSettled(
 				allOperations.map((op) => op.run()),
 			);
 
-			const successCount = results.filter(
-				(r) => r.status === PromiseStatus.Fulfilled,
-			).length;
-			if (successCount > 0) {
-				await invalidateRoles();
-			}
-
 			const failures: MemberRoleUpdateFailure[] = [];
 			results.forEach((result, index) => {
 				if (result.status === PromiseStatus.Rejected) {
@@ -113,7 +116,6 @@ export function useMemberRoleManager(
 						error: result.reason,
 						onRetry: async (): Promise<void> => {
 							await run();
-							await invalidateRoles();
 						},
 					});
 				}
@@ -121,7 +123,7 @@ export function useMemberRoleManager(
 
 			return failures;
 		},
-		[userId, currentRoles, setRole, removeRole, invalidateRoles],
+		[userId, currentRoles, assignmentIdByRoleId, createUserRole, deleteUserRole],
 	);
 
 	return { currentRoles, isLoading, applyDiff };