feat(workflow): Remove CommitterStore for useQuery (#42365)

static/app/actionCreators/committers.spec.jsx

@@ -1,98 +0,0 @@
-import {getCommitters} from 'sentry/actionCreators/committers';
-import CommitterStore, {getCommitterStoreKey} from 'sentry/stores/committerStore';
-
-describe('CommitterActionCreator', function () {
-  const organization = TestStubs.Organization();
-  const project = TestStubs.Project();
-  const event = TestStubs.Event();
-
-  const storeKey = getCommitterStoreKey(organization.slug, project.slug, event.id);
-  const endpoint = `/projects/${organization.slug}/${project.slug}/events/${event.id}/committers/`;
-
-  const api = new MockApiClient();
-  const mockData = {
-    committers: [
-      {
-        author: TestStubs.CommitAuthor(),
-        commits: [TestStubs.Commit()],
-      },
-    ],
-  };
-  let mockResponse;
-
-  beforeEach(() => {
-    MockApiClient.clearMockResponses();
-    mockResponse = MockApiClient.addMockResponse({
-      url: endpoint,
-      body: mockData,
-    });
-
-    CommitterStore.init();
-
-    jest.restoreAllMocks();
-    jest.spyOn(CommitterStore, 'load');
-    jest.spyOn(CommitterStore, 'loadSuccess');
-
-    /**
-     * XXX(leedongwei): We would want to ensure that Store methods are not
-     * called to be 100% sure that the short-circuit is happening correctly.
-     *
-     * However, it seems like we cannot attach a listener to the method
-     * See: https://github.com/reflux/refluxjs/issues/139#issuecomment-64495623
-     */
-    // jest.spyOn(CommitterStore, 'load');
-    // jest.spyOn(CommitterStore, 'loadSuccess');
-  });
-
-  /**
-   * XXX(leedongwei): I wanted to separate the ticks and run tests to assert the
-   * state change at every tick but it is incredibly flakey.
-   */
-  it('fetches a Committer and emits actions', async () => {
-    getCommitters(api, {
-      orgSlug: organization.slug,
-      projectSlug: project.slug,
-      eventId: event.id,
-    }); // Fire Action.load
-    expect(CommitterStore.load).toHaveBeenCalledWith(
-      organization.slug,
-      project.slug,
-      event.id
-    );
-    expect(CommitterStore.loadSuccess).not.toHaveBeenCalled();
-
-    await tick(); // Run Store.load and fire Action.loadSuccess
-    await tick(); // Run Store.loadSuccess
-
-    expect(mockResponse).toHaveBeenCalledWith(endpoint, expect.anything());
-    expect(CommitterStore.loadSuccess).toHaveBeenCalledWith(
-      organization.slug,
-      project.slug,
-      event.id,
-      mockData.committers,
-      undefined
-    );
-
-    expect(CommitterStore.state).toEqual({
-      [storeKey]: {
-        committers: mockData.committers,
-        committersLoading: false,
-        committersError: undefined,
-      },
-    });
-  });
-
-  it('short-circuits the JS event loop', () => {
-    expect(CommitterStore.state.committersLoading).toEqual(undefined);
-
-    getCommitters(api, {
-      orgSlug: organization.slug,
-      projectSlug: project.slug,
-      eventId: event.id,
-    }); // Fire Action.load
-
-    expect(CommitterStore.load).toHaveBeenCalled();
-    // expect(CommitterStore.load).not.toHaveBeenCalled();
-    expect(CommitterStore.state[storeKey].committersLoading).toEqual(true); // Short-circuit
-  });
-});

static/app/actionCreators/committers.tsx

@@ -1,44 +0,0 @@
-import type {Client} from 'sentry/api';
-import CommitterStore, {getCommitterStoreKey} from 'sentry/stores/committerStore';
-import type {Committer, ReleaseCommitter} from 'sentry/types';
-
-type ParamsGet = {
-  eventId: string;
-  orgSlug: string;
-  projectSlug: string;
-};
-
-export function getCommitters(api: Client, params: ParamsGet) {
-  const {orgSlug, projectSlug, eventId} = params;
-  const path = `/projects/${orgSlug}/${projectSlug}/events/${eventId}/committers/`;
-
-  // HACK(leedongwei): Actions fired by the ActionCreators are queued to
-  // the back of the event loop, allowing another getRepo for the same
-  // repo to be fired before the loading state is updated in store.
-  // This hack short-circuits that and update the state immediately.
-  const storeKey = getCommitterStoreKey(orgSlug, projectSlug, eventId);
-  CommitterStore.state[storeKey] = {
-    ...CommitterStore.state[storeKey],
-    committersLoading: true,
-  };
-  CommitterStore.load(orgSlug, projectSlug, eventId);
-
-  return api
-    .requestPromise(path, {
-      method: 'GET',
-    })
-    .then((res: {committers: Committer[]; releaseCommitters: ReleaseCommitter[]}) => {
-      CommitterStore.loadSuccess(
-        orgSlug,
-        projectSlug,
-        eventId,
-        res.committers,
-        res.releaseCommitters
-      );
-    })
-    .catch(err => {
-      // NOTE: Do not captureException here as EventFileCommittersEndpoint returns
-      // 404 Not Found if the project did not setup Releases or Commits
-      CommitterStore.loadError(orgSlug, projectSlug, eventId, err);
-    });
-}

static/app/components/events/eventCause.spec.jsx

@@ -1,8 +1,7 @@
-import {act, render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
 import {Client} from 'sentry/api';
 import EventCause from 'sentry/components/events/eventCause';
-import CommitterStore from 'sentry/stores/committerStore';
 
 import {CommitRow} from '../commitRow';
 import {QuickContextCommitRow} from '../discover/quickContextCommitRow';
@@ -15,11 +14,9 @@ describe('EventCause', function () {
 
   afterEach(function () {
     Client.clearMockResponses();
-    act(() => CommitterStore.reset());
   });
 
   beforeEach(function () {
-    CommitterStore.init();
     Client.addMockResponse({
       method: 'GET',
       url: `/projects/${organization.slug}/${project.slug}/events/${event.id}/committers/`,

static/app/components/events/eventCause.tsx

@@ -25,13 +25,14 @@ interface Props {
 function EventCause({group, eventId, project, commitRow: CommitRow}: Props) {
   const organization = useOrganization();
   const [isExpanded, setIsExpanded] = useState(false);
-  const {committers, fetching} = useCommitters({
+  const {data, isLoading} = useCommitters({
     eventId,
     projectSlug: project.slug,
   });
+  const committers = data?.committers ?? [];
 
   useRouteAnalyticsParams({
-    fetching,
+    fetching: isLoading,
     num_suspect_commits: committers.length,
   });
 

static/app/components/group/suggestedOwners/suggestedOwners.spec.jsx

@@ -2,7 +2,6 @@ import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrar
 
 import {Client} from 'sentry/api';
 import SuggestedOwners from 'sentry/components/group/suggestedOwners/suggestedOwners';
-import CommitterStore from 'sentry/stores/committerStore';
 import MemberListStore from 'sentry/stores/memberListStore';
 import TeamStore from 'sentry/stores/teamStore';
 
@@ -16,7 +15,6 @@ describe('SuggestedOwners', function () {
   const endpoint = `/projects/${organization.slug}/${project.slug}/events/${event.id}`;
 
   beforeEach(function () {
-    CommitterStore.init();
     TeamStore.init();
     MemberListStore.loadInitialData([user, TestStubs.CommitAuthor()]);
     Client.addMockResponse({
@@ -130,39 +128,6 @@ describe('SuggestedOwners', function () {
     );
   });
 
-  it('displays release committers', async function () {
-    const team1 = TestStubs.Team({slug: 'team-1', id: '1'});
-    const team2 = TestStubs.Team({slug: 'team-2', id: '2'});
-    TeamStore.loadInitialData([team1, team2], false, null);
-
-    Client.addMockResponse({
-      url: `${endpoint}/committers/`,
-      body: {
-        committers: [],
-        releaseCommitters: [
-          {
-            author: TestStubs.CommitAuthor(),
-            commits: [TestStubs.Commit()],
-            release: TestStubs.Release(),
-          },
-        ],
-      },
-    });
-
-    Client.addMockResponse({
-      url: `${endpoint}/owners/`,
-      body: {owners: [], rules: []},
-    });
-
-    render(<SuggestedOwners project={project} group={group} event={event} />, {
-      organization,
-    });
-    userEvent.hover(await screen.findByTestId('suggested-assignee'));
-
-    expect(await screen.findByText('Suspect Release')).toBeInTheDocument();
-    expect(screen.getByText('last committed')).toBeInTheDocument();
-  });
-
   it('hides when there are no suggestions', async () => {
     Client.addMockResponse({
       url: `${endpoint}/committers/`,

static/app/components/group/suggestedOwners/suggestedOwners.tsx

@@ -7,7 +7,6 @@ import type {
   Group,
   Organization,
   Project,
-  ReleaseCommitter,
 } from 'sentry/types';
 import type {Event} from 'sentry/types/event';
 import useCommitters from 'sentry/utils/useCommitters';
@@ -24,7 +23,6 @@ type Props = {
   organization: Organization;
   project: Project;
   committers?: Committer[];
-  releaseCommitters?: ReleaseCommitter[];
 } & AsyncComponent['props'];
 
 type State = {
@@ -100,11 +98,9 @@ class SuggestedOwners extends AsyncComponent<Props, State> {
    */
   getOwnerList(): OwnerList {
     const committers = this.props.committers ?? [];
-    const releaseCommitters = this.props.releaseCommitters ?? [];
-    const owners: OwnerList = [...committers, ...releaseCommitters].map(commiter => ({
+    const owners: OwnerList = committers.map(commiter => ({
       actor: {...commiter.author, type: 'user'},
       commits: commiter.commits,
-      release: (commiter as ReleaseCommitter).release,
       source: 'suspectCommit',
     }));
 
@@ -186,16 +182,18 @@ class SuggestedOwners extends AsyncComponent<Props, State> {
 
 function SuggestedOwnersWrapper(props: Omit<Props, 'committers' | 'organization'>) {
   const organization = useOrganization();
-  const {committers, releaseCommitters} = useCommitters({
-    eventId: props.event.id,
-    projectSlug: props.project.slug,
-  });
+  const {data} = useCommitters(
+    {
+      eventId: props.event.id,
+      projectSlug: props.project.slug,
+    },
+    {notifyOnChangeProps: ['data']}
+  );
 
   return (
     <SuggestedOwners
       organization={organization}
-      committers={committers}
-      releaseCommitters={releaseCommitters}
+      committers={data?.committers ?? []}
       {...props}
     />
   );

static/app/stores/committerStore.tsx

@@ -1,125 +0,0 @@
-import {createStore} from 'reflux';
-
-import type {Committer, ReleaseCommitter} from 'sentry/types';
-
-type State = {
-  // Use `getCommitterStoreKey` to generate key
-  [key: string]: {
-    committers?: Committer[];
-    committersError?: Error;
-    committersLoading?: boolean;
-    releaseCommitters?: ReleaseCommitter[];
-  };
-};
-
-interface CommitterStoreDefinition extends Reflux.StoreDefinition {
-  get(
-    orgSlug: string,
-    projectSlug: string,
-    eventId: string
-  ): {
-    committers?: Committer[];
-    committersError?: Error;
-    committersLoading?: boolean;
-  };
-  getState(): State;
-
-  init(): void;
-
-  load(orgSlug: string, projectSlug: string, eventId: string): void;
-  loadError(orgSlug: string, projectSlug: string, eventId: string, error: Error): void;
-  loadSuccess(
-    orgSlug: string,
-    projectSlug: string,
-    eventId: string,
-    committers: Committer[],
-    releaseCommitters?: ReleaseCommitter[]
-  ): void;
-
-  state: State;
-}
-
-export const storeConfig: CommitterStoreDefinition = {
-  state: {},
-
-  init() {
-    // XXX: Do not use `this.listenTo` in this store. We avoid usage of reflux
-    // listeners due to their leaky nature in tests.
-
-    this.reset();
-  },
-
-  reset() {
-    this.state = {};
-    this.trigger(this.state);
-  },
-
-  load(orgSlug: string, projectSlug: string, eventId: string) {
-    const key = getCommitterStoreKey(orgSlug, projectSlug, eventId);
-    this.state = {
-      ...this.state,
-      [key]: {
-        committers: undefined,
-        committersLoading: true,
-        committersError: undefined,
-      },
-    };
-
-    this.trigger(this.state);
-  },
-
-  loadError(orgSlug: string, projectSlug: string, eventId: string, err: Error) {
-    const key = getCommitterStoreKey(orgSlug, projectSlug, eventId);
-    this.state = {
-      ...this.state,
-      [key]: {
-        committers: undefined,
-        committersLoading: false,
-        committersError: err,
-      },
-    };
-
-    this.trigger(this.state);
-  },
-
-  loadSuccess(
-    orgSlug: string,
-    projectSlug: string,
-    eventId: string,
-    committers: Committer[],
-    releaseCommitters?: ReleaseCommitter[]
-  ) {
-    const key = getCommitterStoreKey(orgSlug, projectSlug, eventId);
-    this.state = {
-      ...this.state,
-      [key]: {
-        committers,
-        releaseCommitters,
-        committersLoading: false,
-        committersError: undefined,
-      },
-    };
-
-    this.trigger(this.state);
-  },
-
-  get(orgSlug: string, projectSlug: string, eventId: string) {
-    const key = getCommitterStoreKey(orgSlug, projectSlug, eventId);
-    return {...this.state[key]};
-  },
-
-  getState() {
-    return this.state;
-  },
-};
-
-export function getCommitterStoreKey(
-  orgSlug: string,
-  projectSlug: string,
-  eventId: string
-): string {
-  return `${orgSlug} ${projectSlug} ${eventId}`;
-}
-
-const CommitterStore = createStore(storeConfig);
-export default CommitterStore;

static/app/types/integrations.tsx

@@ -9,7 +9,7 @@ import type {
 } from 'sentry/views/organizationIntegrations/constants';
 
 import type {Avatar, Choice, Choices, ObjectStatus, Scope} from './core';
-import type {BaseRelease, Release} from './release';
+import type {BaseRelease} from './release';
 import type {User} from './user';
 
 export type PermissionValue = 'no-access' | 'read' | 'write' | 'admin';
@@ -96,10 +96,6 @@ export type Committer = {
   commits: Commit[];
 };
 
-export interface ReleaseCommitter extends Committer {
-  release: Release;
-}
-
 export type CommitAuthor = {
   email?: string;
   name?: string;

static/app/utils/useCommitters.spec.tsx

@@ -1,97 +0,0 @@
-import {reactHooks} from 'sentry-test/reactTestingLibrary';
-
-import CommitterStore from 'sentry/stores/committerStore';
-import useCommitters from 'sentry/utils/useCommitters';
-import {OrganizationContext} from 'sentry/views/organizationContext';
-
-describe('useCommitters hook', function () {
-  const organization = TestStubs.Organization();
-  const wrapper = ({children}: {children?: React.ReactNode}) => (
-    <OrganizationContext.Provider value={organization}>
-      {children}
-    </OrganizationContext.Provider>
-  );
-  const project = TestStubs.Project();
-  const event = TestStubs.Event();
-  let mockApiEndpoint: ReturnType<typeof MockApiClient.addMockResponse>;
-
-  const endpoint = `/projects/${organization.slug}/${project.slug}/events/${event.id}/committers/`;
-
-  const mockData = {
-    committers: [
-      {
-        author: TestStubs.CommitAuthor(),
-        commits: [TestStubs.Commit()],
-      },
-    ],
-  };
-
-  beforeEach(() => {
-    mockApiEndpoint = MockApiClient.addMockResponse({
-      url: endpoint,
-      body: mockData,
-    });
-
-    CommitterStore.init();
-  });
-
-  afterEach(() => {
-    MockApiClient.clearMockResponses();
-    jest.clearAllMocks();
-  });
-
-  it('returns committers', async () => {
-    const {result, waitFor} = reactHooks.renderHook(useCommitters, {
-      initialProps: {eventId: event.id, projectSlug: project.slug},
-      wrapper,
-    });
-
-    await waitFor(() => expect(result.current.committers).toEqual(mockData.committers));
-    expect(result.current.fetching).toBe(false);
-    expect(mockApiEndpoint).toHaveBeenCalledTimes(1);
-  });
-
-  it('prevents repeated calls', async () => {
-    const {result, waitFor} = reactHooks.renderHook(useCommitters, {
-      initialProps: {eventId: event.id, projectSlug: project.slug},
-      wrapper,
-    });
-
-    await waitFor(() => expect(result.current.committers).toEqual(mockData.committers));
-
-    reactHooks.renderHook(useCommitters, {
-      initialProps: {eventId: event.id, projectSlug: project.slug},
-      wrapper,
-    });
-    reactHooks.renderHook(useCommitters, {
-      initialProps: {eventId: event.id, projectSlug: project.slug},
-      wrapper,
-    });
-
-    expect(mockApiEndpoint).toHaveBeenCalledTimes(1);
-  });
-
-  /**
-   * Same as 'prevents repeated calls', but with the async fetch/checks
-   * happening on same tick.
-   *
-   * Additionally, this test checks that withCommitters.fetchCommitters does
-   * not check for (store.orgSlug !== orgSlug) as the short-circuit does not
-   * change the value for orgSlug
-   */
-  it('prevents simultaneous calls', async () => {
-    // Mount and run duplicates
-    reactHooks.renderHook(useCommitters, {
-      initialProps: {eventId: event.id, projectSlug: project.slug},
-      wrapper,
-    });
-    const {result, waitFor} = reactHooks.renderHook(useCommitters, {
-      initialProps: {eventId: event.id, projectSlug: project.slug},
-      wrapper,
-    });
-
-    await waitFor(() => expect(result.current.committers).toEqual(mockData.committers));
-
-    expect(mockApiEndpoint).toHaveBeenCalledTimes(1);
-  });
-});

static/app/utils/useCommitters.tsx

@@ -1,66 +1,35 @@
-import {useCallback, useEffect} from 'react';
-
-import {getCommitters} from 'sentry/actionCreators/committers';
-import {Client} from 'sentry/api';
-import CommitterStore, {getCommitterStoreKey} from 'sentry/stores/committerStore';
-import {useLegacyStore} from 'sentry/stores/useLegacyStore';
-import type {Committer, Organization, ReleaseCommitter} from 'sentry/types';
-import useApi from 'sentry/utils/useApi';
+import type {Committer} from 'sentry/types';
+import {QueryKey, useQuery, UseQueryOptions} from 'sentry/utils/queryClient';
 
 import useOrganization from './useOrganization';
 
-interface Props {
+interface UseCommittersProps {
   eventId: string;
   projectSlug: string;
 }
 
-interface Result {
+interface CommittersResponse {
   committers: Committer[];
-  fetching: boolean;
-  // TODO(scttcper): Not optional on GA of release-committer-assignees flag
-  releaseCommitters?: ReleaseCommitter[];
 }
 
-async function fetchCommitters(
-  api: Client,
-  organization: Organization,
+const makeCommittersQueryKey = (
+  orgSlug: string,
   projectSlug: string,
   eventId: string
-) {
-  const repoData = CommitterStore.get(organization.slug, projectSlug, eventId);
-
-  if ((!repoData.committers && !repoData.committersLoading) || repoData.committersError) {
-    await getCommitters(api, {
-      orgSlug: organization.slug,
-      projectSlug,
-      eventId,
-    });
-  }
-}
-
-function useCommitters({eventId, projectSlug}: Props): Result {
-  const api = useApi();
-  const organization = useOrganization();
-  const store = useLegacyStore(CommitterStore);
+): QueryKey => [`/projects/${orgSlug}/${projectSlug}/events/${eventId}/committers/`];
 
-  const loadCommitters = useCallback(async () => {
-    await fetchCommitters(api, organization!, projectSlug, eventId);
-  }, [api, organization, projectSlug, eventId]);
-
-  useEffect(() => {
-    if (!organization) {
-      return;
+function useCommitters(
+  {eventId, projectSlug}: UseCommittersProps,
+  options: UseQueryOptions<CommittersResponse> = {}
+) {
+  const org = useOrganization();
+  return useQuery<CommittersResponse>(
+    makeCommittersQueryKey(org.slug, projectSlug, eventId),
+    {
+      staleTime: Infinity,
+      ...options,
     }
-
-    loadCommitters();
-  }, [eventId, loadCommitters, organization]);
-
-  const key = getCommitterStoreKey(organization?.slug ?? '', projectSlug, eventId);
-  return {
-    committers: store[key]?.committers ?? [],
-    releaseCommitters: store[key]?.releaseCommitters ?? [],
-    fetching: store[key]?.committersLoading ?? false,
-  };
+  );
 }
 
 export default useCommitters;