fix(feedback): Remove duplicate assignment request (#83669)

getsentry · Jan 22, 2025 · 9c1efc8 · 9c1efc8
1 parent fdca387
commit 9c1efc8
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 36 deletions.
diff --git a/static/app/actionCreators/group.tsx b/static/app/actionCreators/group.tsx
@@ -61,7 +61,7 @@ export function clearAssignment(
   groupId: string,
   orgSlug: string,
   assignedBy: AssignedBy
-) {
+): Promise<Group> {
   const api = new Client();
 
   const endpoint = `/organizations/${orgSlug}/issues/${groupId}/`;
@@ -84,9 +84,11 @@ export function clearAssignment(
   request
     .then(data => {
       GroupStore.onAssignToSuccess(id, groupId, data);
+      return data;
     })
     .catch(data => {
       GroupStore.onAssignToError(id, groupId, data);
+      throw data;
     });
 
   return request;
@@ -102,7 +104,12 @@ type AssignToActorParams = {
   orgSlug: string;
 };
 
-export function assignToActor({id, actor, assignedBy, orgSlug}: AssignToActorParams) {
+export function assignToActor({
+  id,
+  actor,
+  assignedBy,
+  orgSlug,
+}: AssignToActorParams): Promise<Group> {
   const api = new Client();
 
   const endpoint = `/organizations/${orgSlug}/issues/${id}/`;
@@ -135,9 +142,11 @@ export function assignToActor({id, actor, assignedBy, orgSlug}: AssignToActorPar
     })
     .then(data => {
       GroupStore.onAssignToSuccess(guid, id, data);
+      return data;
     })
     .catch(data => {
       GroupStore.onAssignToSuccess(guid, id, data);
+      throw data;
     });
 }
 

diff --git a/static/app/components/feedback/feedbackItem/feedbackAssignedTo.spec.tsx b/static/app/components/feedback/feedbackItem/feedbackAssignedTo.spec.tsx
@@ -0,0 +1,93 @@
+import {EventFixture} from 'sentry-fixture/event';
+import {FeedbackIssueFixture} from 'sentry-fixture/feedbackIssue';
+import {MemberFixture} from 'sentry-fixture/member';
+import {OrganizationFixture} from 'sentry-fixture/organization';
+import {ProjectFixture} from 'sentry-fixture/project';
+import {UserFixture} from 'sentry-fixture/user';
+
+import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
+
+import MemberListStore from 'sentry/stores/memberListStore';
+import type {Group} from 'sentry/types/group';
+
+import FeedbackAssignedTo from './feedbackAssignedTo';
+
+describe('FeedbackAssignedTo', () => {
+  const user = UserFixture();
+  const organization = OrganizationFixture();
+  const feedbackIssue = FeedbackIssueFixture({}) as unknown as Group;
+  const feedbackEvent = EventFixture();
+  const project = ProjectFixture();
+
+  beforeEach(() => {
+    MemberListStore.reset();
+
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/users/`,
+      body: [MemberFixture({user})],
+    });
+    MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/${project.slug}/events/${feedbackEvent.id}/owners/`,
+      body: {
+        owners: [],
+        rules: [],
+      },
+    });
+  });
+
+  it('should assign to user', async () => {
+    const assignMock = MockApiClient.addMockResponse({
+      method: 'PUT',
+      url: `/organizations/${organization.slug}/issues/${feedbackIssue.id}/`,
+      body: {...feedbackIssue, assignedTo: {id: user.id, type: 'user', name: user.name}},
+    });
+
+    render(
+      <FeedbackAssignedTo feedbackIssue={feedbackIssue} feedbackEvent={feedbackEvent} />
+    );
+
+    await userEvent.click(await screen.findByLabelText('Modify issue assignee'));
+    await userEvent.click(screen.getByText(`${user.name} (You)`));
+
+    await waitFor(() =>
+      expect(assignMock).toHaveBeenLastCalledWith(
+        `/organizations/${organization.slug}/issues/${feedbackIssue.id}/`,
+        expect.objectContaining({
+          data: {assignedTo: `user:${user.id}`, assignedBy: 'assignee_selector'},
+        })
+      )
+    );
+    expect(assignMock).toHaveBeenCalledTimes(1);
+  });
+
+  it('should clear assignee', async () => {
+    const assignMock = MockApiClient.addMockResponse({
+      method: 'PUT',
+      url: `/organizations/${organization.slug}/issues/${feedbackIssue.id}/`,
+      body: {...feedbackIssue, assignedTo: null},
+    });
+
+    render(
+      <FeedbackAssignedTo
+        feedbackIssue={{
+          ...feedbackIssue,
+          assignedTo: {id: user.id, type: 'user', name: user.name},
+        }}
+        feedbackEvent={feedbackEvent}
+      />
+    );
+
+    await userEvent.click(await screen.findByLabelText('Modify issue assignee'));
+    await userEvent.click(await screen.findByRole('button', {name: 'Clear'}));
+
+    await waitFor(() =>
+      expect(assignMock).toHaveBeenLastCalledWith(
+        `/organizations/${organization.slug}/issues/${feedbackIssue.id}/`,
+        expect.objectContaining({
+          data: {assignedBy: 'assignee_selector', assignedTo: ''},
+        })
+      )
+    );
+    expect(assignMock).toHaveBeenCalledTimes(1);
+  });
+});
diff --git a/static/app/components/feedback/feedbackItem/feedbackAssignedTo.tsx b/static/app/components/feedback/feedbackItem/feedbackAssignedTo.tsx
@@ -1,14 +1,13 @@
 import {useEffect} from 'react';
 
 import {fetchOrgMembers} from 'sentry/actionCreators/members';
-import useMutateFeedback from 'sentry/components/feedback/useMutateFeedback';
+import useFeedbackCache from 'sentry/components/feedback/useFeedbackCache';
 import type {EventOwners} from 'sentry/components/group/assignedTo';
 import {getOwnerList} from 'sentry/components/group/assignedTo';
 import {
   AssigneeSelector,
   useHandleAssigneeChange,
 } from 'sentry/components/group/assigneeSelector';
-import type {Actor} from 'sentry/types/core';
 import type {Group} from 'sentry/types/group';
 import type {FeedbackEvent} from 'sentry/utils/feedback/types';
 import {useApiQuery} from 'sentry/utils/queryClient';
@@ -38,15 +37,13 @@ export default function FeedbackAssignedTo({feedbackIssue, feedbackEvent}: Props
       enabled: Boolean(feedbackEvent),
     }
   );
+  const {updateCached} = useFeedbackCache();
   const {handleAssigneeChange, assigneeLoading} = useHandleAssigneeChange({
     organization,
     group: feedbackIssue,
-  });
-
-  const {assign} = useMutateFeedback({
-    feedbackIds: [feedbackIssue.id],
-    organization,
-    projectIds: [feedbackIssue.project.id],
+    onSuccess: assignedTo => {
+      updateCached([feedbackIssue.id], {assignedTo});
+    },
   });
 
   const owners = getOwnerList([], eventOwners, feedbackIssue.assignedTo);
@@ -57,7 +54,6 @@ export default function FeedbackAssignedTo({feedbackIssue, feedbackEvent}: Props
       owners={owners}
       assigneeLoading={assigneeLoading}
       handleAssigneeChange={e => {
-        assign(e?.assignee as Actor);
         handleAssigneeChange(e);
       }}
     />

diff --git a/static/app/components/feedback/useMutateFeedback.tsx b/static/app/components/feedback/useMutateFeedback.tsx
@@ -79,19 +79,8 @@ export default function useMutateFeedback({
     [mutation, feedbackIds]
   );
 
-  const assign = useCallback(
-    (
-      assignedTo: Actor | undefined,
-      options?: MutateOptions<TData, TError, TVariables, TContext>
-    ) => {
-      mutation.mutate([feedbackIds, {assignedTo}], options);
-    },
-    [mutation, feedbackIds]
-  );
-
   return {
     markAsRead,
     resolve,
-    assign,
   };
 }
diff --git a/static/app/components/group/assigneeSelector.tsx b/static/app/components/group/assigneeSelector.tsx
@@ -14,7 +14,6 @@ import type {Group} from 'sentry/types/group';
 import type {Organization} from 'sentry/types/organization';
 import type {User} from 'sentry/types/user';
 import {useMutation} from 'sentry/utils/queryClient';
-import type RequestError from 'sentry/utils/requestError/requestError';
 
 interface AssigneeSelectorProps {
   assigneeLoading: boolean;
@@ -29,36 +28,31 @@ export function useHandleAssigneeChange({
   organization,
   group,
   onAssign,
+  onSuccess,
 }: {
   group: Group;
   organization: Organization;
   onAssign?: OnAssignCallback;
+  onSuccess?: (assignedTo: Group['assignedTo']) => void;
 }) {
-  const {mutate: handleAssigneeChange, isPending: assigneeLoading} = useMutation<
-    AssignableEntity | null,
-    RequestError,
-    AssignableEntity | null
-  >({
-    mutationFn: async (
-      newAssignee: AssignableEntity | null
-    ): Promise<AssignableEntity | null> => {
+  const {mutate: handleAssigneeChange, isPending: assigneeLoading} = useMutation({
+    mutationFn: (newAssignee: AssignableEntity | null): Promise<Group> => {
       if (newAssignee) {
-        await assignToActor({
+        return assignToActor({
           id: group.id,
           orgSlug: organization.slug,
           actor: {id: newAssignee.id, type: newAssignee.type},
           assignedBy: 'assignee_selector',
         });
-        return Promise.resolve(newAssignee);
       }
 
-      await clearAssignment(group.id, organization.slug, 'assignee_selector');
-      return Promise.resolve(null);
+      return clearAssignment(group.id, organization.slug, 'assignee_selector');
     },
-    onSuccess: (newAssignee: AssignableEntity | null) => {
+    onSuccess: (updatedGroup, newAssignee) => {
       if (onAssign && newAssignee) {
         onAssign(newAssignee.type, newAssignee.assignee, newAssignee.suggestedAssignee);
       }
+      onSuccess?.(updatedGroup.assignedTo);
     },
     onError: () => {
       addErrorMessage('Failed to update assignee');