From 997b6bb3ceab1ad8108ce2d84e87b417bce7b672 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Wed, 20 May 2026 16:21:08 -0700 Subject: [PATCH 1/6] ref(issues): Remove grouping store The merged fingerprints tab was using a Reflux store even though every real caller lived in the same issue details area. This moves the endpoint query and local UI state into groupMerged so the feature owns its own data flow. The local model now matches the hashes endpoint response instead of carrying the old store's dead fields around. Co-authored-by: Codex GPT-5 --- static/app/stores/groupingStore.spec.tsx | 415 ----------------- static/app/stores/groupingStore.tsx | 406 ----------------- .../issueDetails/groupMerged/index.spec.tsx | 13 +- .../views/issueDetails/groupMerged/index.tsx | 96 ++-- .../groupMerged/mergedIssuesDrawer.spec.tsx | 3 +- .../issueDetails/groupMerged/mergedItem.tsx | 115 ++--- .../issueDetails/groupMerged/mergedList.tsx | 9 +- .../groupMerged/mergedToolbar.tsx | 22 +- .../groupMerged/useGroupMerged.spec.tsx | 137 ++++++ .../groupMerged/useGroupMerged.tsx | 423 ++++++++++++++++++ 10 files changed, 643 insertions(+), 996 deletions(-) delete mode 100644 static/app/stores/groupingStore.spec.tsx delete mode 100644 static/app/stores/groupingStore.tsx create mode 100644 static/app/views/issueDetails/groupMerged/useGroupMerged.spec.tsx create mode 100644 static/app/views/issueDetails/groupMerged/useGroupMerged.tsx diff --git a/static/app/stores/groupingStore.spec.tsx b/static/app/stores/groupingStore.spec.tsx deleted file mode 100644 index dca6aa0ba69c..000000000000 --- a/static/app/stores/groupingStore.spec.tsx +++ /dev/null @@ -1,415 +0,0 @@ -import {GroupingStore} from 'sentry/stores/groupingStore'; - -describe('Grouping Store', () => { - let trigger!: jest.SpyInstance; - - beforeAll(() => { - MockApiClient.asyncDelay = 1; - }); - - afterAll(() => { - MockApiClient.asyncDelay = undefined; - }); - - beforeEach(() => { - GroupingStore.init(); - trigger = jest.spyOn(GroupingStore, 'trigger'); - MockApiClient.addMockResponse({ - url: '/organizations/org-slug/issues/groupId/hashes/', - body: [ - { - latestEvent: { - eventID: 'event-1', - }, - state: 'locked', - id: '1', - }, - { - latestEvent: { - eventID: 'event-2', - }, - state: 'unlocked', - id: '2', - mergedBySeer: true, - }, - { - latestEvent: { - eventID: 'event-3', - }, - state: 'unlocked', - id: '3', - }, - { - latestEvent: { - eventID: 'event-4', - }, - state: 'unlocked', - id: '4', - mergedBySeer: true, - }, - { - latestEvent: { - eventID: 'event-5', - }, - state: 'locked', - id: '5', - }, - ], - }); - }); - - afterEach(() => { - MockApiClient.clearMockResponses(); - jest.resetAllMocks(); - jest.restoreAllMocks(); - }); - - describe('onFetch()', () => { - beforeEach(() => GroupingStore.init()); - - it('initially gets called with correct state values', () => { - GroupingStore.onFetch([]); - - expect(trigger).toHaveBeenCalled(); - expect(trigger).toHaveBeenCalledWith( - expect.objectContaining({ - error: false, - loading: true, - mergedItems: [], - mergedLinks: '', - unmergeState: new Map(), - }) - ); - }); - - it('fetches list of hashes', () => { - const promise = GroupingStore.onFetch([ - {dataKey: 'merged', endpoint: '/organizations/org-slug/issues/groupId/hashes/'}, - ]); - - expect(trigger).toHaveBeenCalled(); - const calls = trigger.mock.calls; - return promise.then(() => { - const arg = calls[calls.length - 1][0]; - expect(arg.mergedItems).toHaveLength(5); - expect(arg).toMatchObject({ - loading: false, - error: false, - unmergeState: new Map([ - ['1', {busy: true}], - ['2', {busy: false}], - ['3', {busy: false}], - ['4', {busy: false}], - ['5', {busy: true}], - ]), - }); - }); - }); - - it('handles fingerprints with seer merging information', async () => { - await GroupingStore.onFetch([ - {dataKey: 'merged', endpoint: '/organizations/org-slug/issues/groupId/hashes/'}, - ]); - - expect(trigger).toHaveBeenCalled(); - const mergedItems = GroupingStore.getState().mergedItems; - - // Check that fingerprints with metadata are properly handled - const fingerprintWithSeer = mergedItems.find((item: any) => item.id === '2'); - expect(fingerprintWithSeer).toBeDefined(); - expect(fingerprintWithSeer?.mergedBySeer).toBe(true); - - const fingerprintWithSeerV2 = mergedItems.find((item: any) => item.id === '4'); - expect(fingerprintWithSeerV2).toBeDefined(); - expect(fingerprintWithSeerV2?.mergedBySeer).toBe(true); - - // Check that fingerprints without seer merging are still handled correctly - const fingerprintWithoutSeer = mergedItems.find((item: any) => item.id === '3'); - expect(fingerprintWithoutSeer).toBeDefined(); - expect(fingerprintWithoutSeer?.mergedBySeer).toBeUndefined(); - }); - - it('unsuccessfully fetches list of hashes items', () => { - MockApiClient.clearMockResponses(); - MockApiClient.addMockResponse({ - url: '/organizations/org-slug/issues/groupId/hashes/', - statusCode: 500, - body: {message: 'failed'}, - }); - - const promise = GroupingStore.onFetch([ - {dataKey: 'merged', endpoint: '/organizations/org-slug/issues/groupId/hashes/'}, - ]); - - expect(trigger).toHaveBeenCalled(); - const calls = trigger.mock.calls; - return promise.then(() => { - const arg = calls[calls.length - 1][0]; - expect(arg).toMatchObject({ - loading: false, - error: true, - mergedItems: [], - unmergeState: new Map(), - }); - }); - }); - }); - - describe('Hashes list (to be unmerged)', () => { - let unmergeList: (typeof GroupingStore)['state']['unmergeList']; - let unmergeState: (typeof GroupingStore)['state']['unmergeState']; - - beforeEach(async () => { - GroupingStore.init(); - unmergeList = new Map(); - unmergeState = new Map(); - await GroupingStore.onFetch([ - {dataKey: 'merged', endpoint: '/organizations/org-slug/issues/groupId/hashes/'}, - ]); - - trigger.mockClear(); - unmergeState = new Map([...GroupingStore.getState().unmergeState]); - }); - - // WARNING: all the tests in this describe block are not running in isolated state. - // There is a good chance that moving them around will break them. To simulate an isolated state, - // add a beforeEach(() => GroupingStore.init()) - describe('onToggleUnmerge (checkbox state for hashes)', () => { - // Attempt to check first item but its "locked" so should not be able to do anything - it('can not check locked item', () => { - GroupingStore.onToggleUnmerge('1'); - - expect(GroupingStore.getState().unmergeList).toEqual(unmergeList); - expect(GroupingStore.getState().unmergeState).toEqual(unmergeState); - expect(trigger).not.toHaveBeenCalled(); - }); - - it('can check and uncheck unlocked items', () => { - // Check - GroupingStore.onToggleUnmerge(['2', 'event-2']); - unmergeList.set('2', 'event-2'); - unmergeState.set('2', {busy: false, checked: true}); - - expect(GroupingStore.getState().unmergeList).toEqual(unmergeList); - expect(GroupingStore.getState().unmergeState).toEqual(unmergeState); - - // Uncheck - GroupingStore.onToggleUnmerge(['2', 'event-2']); - unmergeList.delete('2'); - unmergeState.set('2', {busy: false, checked: false}); - - expect(GroupingStore.getState().unmergeList).toEqual(unmergeList); - expect(GroupingStore.getState().unmergeState).toEqual(unmergeState); - - // Check - GroupingStore.onToggleUnmerge(['2', 'event-2']); - unmergeList.set('2', 'event-2'); - unmergeState.set('2', {busy: false, checked: true}); - - expect(GroupingStore.getState().unmergeList).toEqual(unmergeList); - expect(GroupingStore.getState().unmergeState).toEqual(unmergeState); - - expect(trigger).toHaveBeenLastCalledWith( - expect.objectContaining({ - enableFingerprintCompare: false, - unmergeLastCollapsed: false, - unmergeDisabled: false, - unmergeList, - unmergeState, - }) - ); - }); - - it('should have Compare button enabled only when two fingerprints are checked', () => { - expect(GroupingStore.getState().enableFingerprintCompare).toBe(false); - - GroupingStore.onToggleUnmerge(['2', 'event-2']); - GroupingStore.onToggleUnmerge(['3', 'event-3']); - expect(GroupingStore.getState().enableFingerprintCompare).toBe(true); - - GroupingStore.onToggleUnmerge(['2', 'event-2']); - expect(GroupingStore.getState().enableFingerprintCompare).toBe(false); - }); - - it('selecting all available checkboxes should disable the unmerge button and re-enable when unchecking', () => { - GroupingStore.onToggleUnmerge(['2', 'event-2']); - GroupingStore.onToggleUnmerge(['3', 'event-3']); - GroupingStore.onToggleUnmerge(['4', 'event-4']); - unmergeList.set('2', 'event-2'); - unmergeList.set('3', 'event-3'); - unmergeList.set('4', 'event-4'); - unmergeState.set('2', {busy: false, checked: true}); - unmergeState.set('3', {busy: false, checked: true}); - unmergeState.set('4', {busy: false, checked: true}); - - expect(GroupingStore.getState().unmergeList).toEqual(unmergeList); - expect(GroupingStore.getState().unmergeState).toEqual(unmergeState); - expect(GroupingStore.getState().unmergeDisabled).toBe(true); - - // Unchecking - GroupingStore.onToggleUnmerge(['4', 'event-4']); - unmergeList.delete('4'); - unmergeState.set('4', {busy: false, checked: false}); - - expect(GroupingStore.getState().unmergeList).toEqual(unmergeList); - expect(GroupingStore.getState().unmergeState).toEqual(unmergeState); - expect(GroupingStore.getState().unmergeDisabled).toBe(false); - - expect(trigger).toHaveBeenLastCalledWith({ - enableFingerprintCompare: true, - unmergeLastCollapsed: false, - unmergeDisabled: false, - unmergeList, - unmergeState, - }); - }); - }); - - // WARNING: all the tests in this describe block are not running in isolated state. - // There is a good chance that moving them around will break them. To simulate an isolated state, - // add a beforeEach(() => GroupingStore.init()) - describe('onUnmerge', () => { - beforeEach(() => { - MockApiClient.clearMockResponses(); - MockApiClient.addMockResponse({ - method: 'PUT', - url: '/organizations/org-slug/issues/groupId/hashes/', - }); - }); - - it('can not toggle unmerge for a locked item', () => { - // Event 1 is locked - GroupingStore.onToggleUnmerge(['1', 'event-1']); - unmergeState.set('1', {busy: true}); - - // trigger does NOT get called because an item returned via API is in a "locked" state - expect(trigger).not.toHaveBeenCalled(); - - GroupingStore.onUnmerge({ - orgSlug: 'org-slug', - groupId: 'groupId', - }); - - expect(trigger).toHaveBeenCalledWith({ - enableFingerprintCompare: false, - unmergeLastCollapsed: false, - unmergeDisabled: true, - unmergeList, - unmergeState, - }); - }); - - it('disables rows to be merged', async () => { - GroupingStore.onToggleUnmerge(['2', 'event-2']); - unmergeList.set('2', 'event-2'); - unmergeState.set('2', {checked: true, busy: false}); - - // trigger does NOT get called because an item returned via API is in a "locked" state - expect(trigger).toHaveBeenCalledWith({ - enableFingerprintCompare: false, - unmergeLastCollapsed: false, - unmergeDisabled: false, - unmergeList, - unmergeState, - }); - - const promise = GroupingStore.onUnmerge({ - orgSlug: 'org-slug', - groupId: 'groupId', - }); - - unmergeState.set('2', {checked: false, busy: true}); - expect(trigger).toHaveBeenCalledWith({ - enableFingerprintCompare: false, - unmergeLastCollapsed: false, - unmergeDisabled: true, - unmergeList, - unmergeState, - }); - - await promise; - - // Success - unmergeState.set('2', {checked: false, busy: true}); - unmergeList.delete('2'); - expect(trigger).toHaveBeenLastCalledWith({ - enableFingerprintCompare: false, - unmergeLastCollapsed: false, - unmergeDisabled: false, - unmergeList, - unmergeState, - }); - }); - - it('keeps rows in "busy" state and unchecks after successfully adding to unmerge queue', async () => { - GroupingStore.onToggleUnmerge(['2', 'event-2']); - unmergeList.set('2', 'event-2'); - unmergeState.set('2', {checked: true, busy: false}); - - const promise = GroupingStore.onUnmerge({ - groupId: 'groupId', - orgSlug: 'org-slug', - }); - - unmergeState.set('2', {checked: false, busy: true}); - expect(trigger).toHaveBeenCalledWith({ - enableFingerprintCompare: false, - unmergeLastCollapsed: false, - unmergeDisabled: true, - unmergeList, - unmergeState, - }); - - await promise; - - expect(trigger).toHaveBeenLastCalledWith({ - enableFingerprintCompare: false, - unmergeLastCollapsed: false, - unmergeDisabled: false, - unmergeList: new Map(), - unmergeState, - }); - }); - - it('resets busy state and has same items checked after error when trying to merge', async () => { - MockApiClient.clearMockResponses(); - MockApiClient.addMockResponse({ - method: 'PUT', - url: '/organizations/org-slug/issues/groupId/hashes/', - statusCode: 500, - body: {}, - }); - - GroupingStore.onToggleUnmerge(['2', 'event-2']); - unmergeList.set('2', 'event-2'); - - const promise = GroupingStore.onUnmerge({ - groupId: 'groupId', - orgSlug: 'org-slug', - }); - - unmergeState.set('2', {checked: false, busy: true}); - expect(trigger).toHaveBeenCalledWith( - expect.objectContaining({ - enableFingerprintCompare: false, - unmergeLastCollapsed: false, - unmergeDisabled: true, - unmergeList, - unmergeState, - }) - ); - - await promise; - - unmergeState.set('2', {checked: true, busy: false}); - expect(trigger).toHaveBeenLastCalledWith({ - enableFingerprintCompare: false, - unmergeLastCollapsed: false, - unmergeDisabled: false, - unmergeList, - unmergeState, - }); - }); - }); - }); -}); diff --git a/static/app/stores/groupingStore.tsx b/static/app/stores/groupingStore.tsx deleted file mode 100644 index 65cec454c5b0..000000000000 --- a/static/app/stores/groupingStore.tsx +++ /dev/null @@ -1,406 +0,0 @@ -import pick from 'lodash/pick'; -import {createStore} from 'reflux'; - -import { - addErrorMessage, - addLoadingMessage, - addSuccessMessage, -} from 'sentry/actionCreators/indicator'; -import {Client} from 'sentry/api'; -import type {Event} from 'sentry/types/event'; -import type {Group} from 'sentry/types/group'; -import type {Organization} from 'sentry/types/organization'; -import {toArray} from 'sentry/utils/array/toArray'; - -import type {StrictStoreDefinition} from './types'; - -type State = { - enableFingerprintCompare: boolean; - error: boolean; - loading: boolean; - // List of fingerprints that belong to issue - mergedItems: Fingerprint[]; - mergedLinks: string; - // Disabled state of "Unmerge" button in "Merged" tab (for Issues) - unmergeDisabled: boolean; - // If "Collapse All" was just used, this will be true - unmergeLastCollapsed: boolean; - // Map of {[fingerprint]: Array} that is selected to be unmerged - unmergeList: Map; - // Map of state for each fingerprint (i.e. "collapsed") - unmergeState: Readonly< - Map> - >; -}; - -type ApiFingerprint = { - id: string; - latestEvent: Event; - childId?: string; - childLabel?: string; - eventCount?: number; - label?: string; - lastSeen?: string; - parentId?: string; - parentLabel?: string; - state?: string; -}; - -type ChildFingerprint = { - childId: string; - childLabel?: string; - eventCount?: number; - lastSeen?: string; - latestEvent?: Event; -}; - -export type Fingerprint = { - children: ChildFingerprint[]; - eventCount: number; - id: string; - latestEvent: Event; - label?: string; - lastSeen?: string; - mergedBySeer?: boolean; - parentId?: string; - parentLabel?: string; - state?: string; -}; - -type IdState = { - busy?: boolean; - checked?: boolean; - collapsed?: boolean; -}; - -type UnmergeResponse = Pick< - State, - | 'unmergeDisabled' - | 'unmergeState' - | 'unmergeList' - | 'enableFingerprintCompare' - | 'unmergeLastCollapsed' ->; - -interface GroupingStoreDefinition extends StrictStoreDefinition { - api: Client; - getInitialState(): State; - init(): void; - isAllUnmergedSelected(): boolean; - onFetch( - toFetchArray: Array<{ - dataKey: 'merged'; - endpoint: string; - queryParams?: Record; - }> - ): Promise; - onToggleCollapseFingerprint(fingerprint: string): void; - onToggleCollapseFingerprints(): void; - onToggleUnmerge(props: [string, string] | string): void; - onUnmerge(props: { - groupId: Group['id']; - orgSlug: Organization['slug']; - errorMessage?: string; - loadingMessage?: string; - successMessage?: string; - }): Promise; - /** - * Updates unmergeState - */ - setStateForId( - stateProperty: 'unmergeState', - idOrIds: string[] | string, - newState: IdState - ): void; - triggerFetchState(): Readonly< - Pick - >; - triggerUnmergeState(): Readonly; -} - -const storeConfig: GroupingStoreDefinition = { - // This will be populated on init - state: {} as State, - api: new Client(), - - init() { - // XXX: Do not use `this.listenTo` in this store. We avoid usage of reflux - // listeners due to their leaky nature in tests. - - this.state = this.getInitialState(); - }, - - getInitialState() { - return { - // List of fingerprints that belong to issue - mergedItems: [], - // Map of {[fingerprint]: Array} that is selected to be unmerged - unmergeList: new Map(), - // Map of state for each fingerprint (i.e. "collapsed") - unmergeState: new Map(), - // Disabled state of "Unmerge" button in "Merged" tab (for Issues) - unmergeDisabled: true, - // If "Collapse All" was just used, this will be true - unmergeLastCollapsed: false, - // "Compare" button state - enableFingerprintCompare: false, - mergedLinks: '', - loading: true, - error: false, - }; - }, - - setStateForId(stateProperty, idOrIds, newState) { - const ids = toArray(idOrIds); - const newMap = new Map(this.state[stateProperty]); - - ids.forEach(id => { - const state = newMap.get(id) ?? {}; - const mergedState = {...state, ...newState}; - newMap.set(id, mergedState); - }); - this.state = {...this.state, [stateProperty]: newMap}; - }, - - isAllUnmergedSelected() { - const lockedItems = - (Array.from(this.state.unmergeState.values()) as IdState[]).filter( - ({busy}) => busy - ) || []; - return ( - this.state.unmergeList.size === - this.state.mergedItems.filter(({latestEvent}) => !!latestEvent).length - - lockedItems.length - ); - }, - - // Fetches data - onFetch(toFetchArray) { - // Reset state and trigger update - this.init(); - this.triggerFetchState(); - - const promises = toFetchArray.map( - ({endpoint}) => - new Promise((resolve, reject) => { - this.api.request(endpoint, { - method: 'GET', - success: (data, _, resp) => { - resolve({ - data, - links: resp ? resp.getResponseHeader('Link') : null, - }); - }, - error: err => { - const error = err.responseJSON?.detail || true; - // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors - reject(error); - }, - }); - }) - ); - - const processMerged = (items: ApiFingerprint[]): Fingerprint[] => { - const newItemsMap: Record = {}; - const newItems: Fingerprint[] = []; - - items.forEach(item => { - if (!newItemsMap[item.id]) { - const newItem = { - eventCount: 0, - children: [], - // lastSeen and latestEvent properties are correct - // since the server returns items in - // descending order of lastSeen - ...item, - }; - // Check for locked items - this.setStateForId('unmergeState', item.id, { - busy: item.state === 'locked', - }); - - newItemsMap[item.id] = newItem; - newItems.push(newItem); - } - - const newItem = newItemsMap[item.id]!; - const {childId, childLabel, eventCount, lastSeen, latestEvent} = item; - - if (eventCount) { - newItem.eventCount += eventCount; - } - - if (childId) { - newItem.children.push({ - childId, - childLabel, - lastSeen, - latestEvent, - eventCount, - }); - } - }); - - return newItems; - }; - - return Promise.all(promises).then( - resultsArray => { - (resultsArray as Array<{data: ApiFingerprint[]; links: string | null}>).forEach( - ({data, links}) => { - const items = processMerged(data); - this.state = { - ...this.state, - mergedItems: items, - mergedLinks: links ?? '', - }; - } - ); - - this.state = {...this.state, loading: false, error: false}; - this.triggerFetchState(); - return resultsArray; - }, - () => { - this.state = {...this.state, loading: false, error: true}; - this.triggerFetchState(); - return []; - } - ); - }, - - // Toggle unmerge check box - onToggleUnmerge([fingerprint, eventId]) { - let checked = false; - - // Uncheck an item to unmerge - const state = this.state.unmergeState.get(fingerprint); - - if (state?.busy === true) { - return; - } - - const newUnmergeList = new Map(this.state.unmergeList); - if (newUnmergeList.has(fingerprint)) { - newUnmergeList.delete(fingerprint); - } else { - newUnmergeList.set(fingerprint, eventId); - checked = true; - } - this.state = {...this.state, unmergeList: newUnmergeList}; - - // Update "checked" state for row - this.setStateForId('unmergeState', fingerprint!, {checked}); - - // Unmerge should be disabled if 0 or all items are selected, or if there's - // only one item to select - const unmergeDisabled = - this.state.mergedItems.length === 1 || - this.state.unmergeList.size === 0 || - this.isAllUnmergedSelected(); - - const enableFingerprintCompare = this.state.unmergeList.size === 2; - this.state = {...this.state, unmergeDisabled, enableFingerprintCompare}; - - this.triggerUnmergeState(); - }, - - onUnmerge({groupId, loadingMessage, orgSlug, successMessage, errorMessage}) { - const grouphashIds = Array.from(this.state.unmergeList.keys()) as string[]; - - return new Promise((resolve, reject) => { - if (this.isAllUnmergedSelected()) { - reject(new Error('Not allowed to unmerge ALL events')); - return; - } - - // Disable unmerge button - this.state = {...this.state, unmergeDisabled: true}; - - // Disable rows - this.setStateForId('unmergeState', grouphashIds, {checked: false, busy: true}); - this.triggerUnmergeState(); - addLoadingMessage(loadingMessage); - - this.api.request(`/organizations/${orgSlug}/issues/${groupId}/hashes/`, { - method: 'PUT', - query: { - id: grouphashIds, - }, - success: () => { - addSuccessMessage(successMessage); - - // Busy rows after successful Unmerge - this.setStateForId('unmergeState', grouphashIds, {checked: false, busy: true}); - this.state.unmergeList.clear(); - }, - error: error => { - errorMessage = error?.responseJSON?.detail || errorMessage; - addErrorMessage(errorMessage); - this.setStateForId('unmergeState', grouphashIds, {checked: true, busy: false}); - }, - complete: () => { - this.state = {...this.state, unmergeDisabled: false}; - resolve(this.triggerUnmergeState()); - }, - }); - }); - }, - - // Toggle collapsed state of all fingerprints - onToggleCollapseFingerprints() { - this.setStateForId( - 'unmergeState', - this.state.mergedItems.map(({id}) => id), - { - collapsed: !this.state.unmergeLastCollapsed, - } - ); - - this.state = { - ...this.state, - unmergeLastCollapsed: !this.state.unmergeLastCollapsed, - }; - - this.trigger({ - unmergeLastCollapsed: this.state.unmergeLastCollapsed, - unmergeState: this.state.unmergeState, - }); - }, - - onToggleCollapseFingerprint(fingerprint) { - const collapsed = this.state.unmergeState.get(fingerprint)?.collapsed; - this.setStateForId('unmergeState', fingerprint, {collapsed: !collapsed}); - this.trigger({unmergeState: this.state.unmergeState}); - }, - - triggerFetchState() { - const state = pick(this.state, [ - 'mergedItems', - 'mergedLinks', - 'unmergeState', - 'loading', - 'error', - ]); - this.trigger(state); - return state; - }, - - triggerUnmergeState() { - const state = pick(this.state, [ - 'unmergeDisabled', - 'unmergeState', - 'unmergeList', - 'enableFingerprintCompare', - 'unmergeLastCollapsed', - ]); - this.trigger(state); - return state; - }, - - getState(): State { - return this.state; - }, -}; - -export const GroupingStore = createStore(storeConfig); diff --git a/static/app/views/issueDetails/groupMerged/index.spec.tsx b/static/app/views/issueDetails/groupMerged/index.spec.tsx index bf7c0f4854e6..abb0eb8fc7a2 100644 --- a/static/app/views/issueDetails/groupMerged/index.spec.tsx +++ b/static/app/views/issueDetails/groupMerged/index.spec.tsx @@ -4,7 +4,6 @@ import {GroupFixture} from 'sentry-fixture/group'; import {initializeOrg} from 'sentry-test/initializeOrg'; import {render, screen} from 'sentry-test/reactTestingLibrary'; -import {GroupingStore} from 'sentry/stores/groupingStore'; import {GroupMergedView} from 'sentry/views/issueDetails/groupMerged'; describe('Issues -> Merged View', () => { @@ -14,23 +13,20 @@ describe('Issues -> Merged View', () => { merged: [ { latestEvent: events[0], - state: 'unlocked', id: '2c4887696f708c476a81ce4e834c4b02', mergedBySeer: true, }, { latestEvent: events[1], - state: 'unlocked', id: 'e05da55328a860b21f62e371f0a7507d', }, ], }; beforeEach(() => { - GroupingStore.init(); MockApiClient.clearMockResponses(); MockApiClient.addMockResponse({ - url: `/organizations/org-slug/issues/${group.id}/hashes/?limit=50&query=`, + url: `/organizations/org-slug/issues/${group.id}/hashes/`, body: mockData.merged, }); }); @@ -49,15 +45,12 @@ describe('Issues -> Merged View', () => { } ); - // Wait for the component to load - await screen.findByText('Fingerprints included in this issue'); + const links = await screen.findAllByRole('button', {name: 'View latest event'}); + expect(links).toHaveLength(mockData.merged.length); const title = await screen.findByText('Fingerprints included in this issue'); expect(title.parentElement).toHaveTextContent( 'Fingerprints included in this issue (2)' ); - - const links = await screen.findAllByRole('button', {name: 'View latest event'}); - expect(links).toHaveLength(mockData.merged.length); }); }); diff --git a/static/app/views/issueDetails/groupMerged/index.tsx b/static/app/views/issueDetails/groupMerged/index.tsx index 77e6e8595ebc..8fbcd32602c1 100644 --- a/static/app/views/issueDetails/groupMerged/index.tsx +++ b/static/app/views/issueDetails/groupMerged/index.tsx @@ -1,21 +1,18 @@ -import {Fragment, useCallback, useEffect, useState} from 'react'; +import {Fragment} from 'react'; import styled from '@emotion/styled'; -import {useQuery} from '@tanstack/react-query'; import type {Location} from 'history'; -import * as qs from 'query-string'; import {LoadingError} from 'sentry/components/loadingError'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {QueryCount} from 'sentry/components/queryCount'; import {t, tct} from 'sentry/locale'; -import type {Fingerprint} from 'sentry/stores/groupingStore'; -import {GroupingStore} from 'sentry/stores/groupingStore'; import type {Group} from 'sentry/types/group'; import type {Project} from 'sentry/types/project'; import {trackAnalytics} from 'sentry/utils/analytics'; import {useOrganization} from 'sentry/utils/useOrganization'; import {MergedList} from './mergedList'; +import {GroupMergedProvider, useGroupMerged} from './useGroupMerged'; type Props = { groupId: Group['id']; @@ -25,74 +22,49 @@ type Props = { export function GroupMergedView(props: Props) { const organization = useOrganization(); - const [mergedItems, setMergedItems] = useState([]); - const [isLoading, setIsLoading] = useState(true); - const [error, setError] = useState(false); - const [mergedLinks, setMergedLinks] = useState(undefined); - const {project, groupId, location} = props; + const {groupId, location} = props; - const onGroupingChange = useCallback( - ({ - mergedItems: items, - mergedLinks: links, - loading: l, - error: e, - }: ReturnType) => { - if (items) { - setMergedItems(items); - setMergedLinks(links); - setIsLoading(l === undefined ? false : l); - setError(e === undefined ? false : e); - } - }, - [] + return ( + + + ); +} - useEffect(() => { - const unsubscribe = GroupingStore.listen(onGroupingChange, undefined); - return () => { - unsubscribe(); - }; - }, [onGroupingChange]); - - const {refetch} = useQuery({ - queryKey: [ - `/organizations/${organization.slug}/issues/${groupId}/hashes/`, - {query: {...location.query, limit: 50, query: location.query.query ?? ''}}, - ] as const, - queryFn: ({queryKey}) => { - // Not sure why query params are encoded into the "endpoint", but keeping behavior the same - const endpoint = `${queryKey[0]}?${qs.stringify(queryKey[1].query)}`; - // TODO: GroupingStore.onFetch is a nightmare, useQuery here is helping convert from class component. - return GroupingStore.onFetch([{endpoint, dataKey: 'merged'}]); - }, - staleTime: 30_000, - retry: false, - }); +function GroupMergedContent({project, groupId}: Props) { + const organization = useOrganization(); + const { + error, + fingerprints, + fingerprintsWithLatestEvent, + loading, + pageLinks, + refetch, + selectedEventIds, + toggleAllCollapsed, + unmerge, + } = useGroupMerged(); const handleUnmerge = () => { - GroupingStore.onUnmerge({ - groupId, - orgSlug: organization.slug, + unmerge({ loadingMessage: t('Unmerging events\u2026'), successMessage: t('Events successfully queued for unmerging.'), errorMessage: t('Unable to queue events for unmerging.'), }); - const unmergeKeys = [...GroupingStore.getState().unmergeList.values()]; trackAnalytics('issue_details.merged_tab.unmerge_clicked', { organization, group_id: groupId, - event_ids_unmerged: unmergeKeys.join(','), - total_unmerged: unmergeKeys.length, + event_ids_unmerged: selectedEventIds.join(','), + total_unmerged: selectedEventIds.length, }); }; - const isError = error && !isLoading; - const isLoadedSuccessfully = !isError && !isLoading; - - const fingerprintsWithLatestEvent = mergedItems.filter( - ({latestEvent}) => !!latestEvent - ); + const isError = error && !loading; + const isLoadedSuccessfully = !isError && !loading; return ( @@ -113,7 +85,7 @@ export function GroupMergedView(props: Props) { - {isLoading && } + {loading && } {isError && ( )} diff --git a/static/app/views/issueDetails/groupMerged/mergedIssuesDrawer.spec.tsx b/static/app/views/issueDetails/groupMerged/mergedIssuesDrawer.spec.tsx index 9a197e1685a6..a3c6dabf4ad0 100644 --- a/static/app/views/issueDetails/groupMerged/mergedIssuesDrawer.spec.tsx +++ b/static/app/views/issueDetails/groupMerged/mergedIssuesDrawer.spec.tsx @@ -22,11 +22,10 @@ describe('MergedIssuesDrawer', () => { GroupStore.init(); mockMergedIssues = MockApiClient.addMockResponse({ - url: `/organizations/${organization.slug}/issues/${group.id}/hashes/?limit=50&query=`, + url: `/organizations/${organization.slug}/issues/${group.id}/hashes/`, body: [ { latestEvent: event, - state: 'unlocked', id: '2c4887696f708c476a81ce4e834c4b02', mergedBySeer: true, }, diff --git a/static/app/views/issueDetails/groupMerged/mergedItem.tsx b/static/app/views/issueDetails/groupMerged/mergedItem.tsx index c968ccb83592..ea614c54fbfa 100644 --- a/static/app/views/issueDetails/groupMerged/mergedItem.tsx +++ b/static/app/views/issueDetails/groupMerged/mergedItem.tsx @@ -1,4 +1,3 @@ -import {useEffect, useState} from 'react'; import {useTheme} from '@emotion/react'; import styled from '@emotion/styled'; @@ -10,14 +9,14 @@ import {Tooltip} from '@sentry/scraps/tooltip'; import {IconChevron, IconLink} from 'sentry/icons'; import {t} from 'sentry/locale'; -import type {Fingerprint} from 'sentry/stores/groupingStore'; -import {GroupingStore} from 'sentry/stores/groupingStore'; import {useLocation} from 'sentry/utils/useLocation'; import {useOrganization} from 'sentry/utils/useOrganization'; import {createIssueLink} from 'sentry/views/issueList/utils'; +import {type FingerprintWithLatestEvent, useGroupMerged} from './useGroupMerged'; + interface Props { - fingerprint: Fingerprint; + fingerprint: FingerprintWithLatestEvent; totalFingerprint: number; } @@ -25,36 +24,14 @@ export function MergedItem({fingerprint, totalFingerprint}: Props) { const theme = useTheme(); const organization = useOrganization(); const location = useLocation(); - const [busy, setBusy] = useState(false); - const [collapsed, setCollapsed] = useState(false); - const [checked, setChecked] = useState(false); - - function onGroupChange({unmergeState}: any) { - if (!unmergeState) { - return; - } - - const stateForId = unmergeState.has(fingerprint.id) - ? unmergeState.get(fingerprint.id) - : undefined; - - if (!stateForId) { - return; - } - - Object.keys(stateForId).forEach(key => { - if (key === 'collapsed') { - setCollapsed(Boolean(stateForId[key])); - } else if (key === 'checked') { - setChecked(Boolean(stateForId[key])); - } else if (key === 'busy') { - setBusy(Boolean(stateForId[key])); - } - }); - } + const {state, toggleCollapsed, toggleSelected} = useGroupMerged(); + const stateForId = state.fingerprintState.get(fingerprint.id); + const busy = Boolean(stateForId?.busy); + const collapsed = Boolean(stateForId?.collapsed); + const checked = Boolean(stateForId?.checked); function handleToggleEvents() { - GroupingStore.onToggleCollapseFingerprint(fingerprint.id); + toggleCollapsed(fingerprint.id); } function handleToggle() { @@ -65,7 +42,7 @@ export function MergedItem({fingerprint, totalFingerprint}: Props) { } // clicking anywhere in the row will toggle the checkbox - GroupingStore.onToggleUnmerge([fingerprint.id, latestEvent.id]); + toggleSelected(fingerprint.id, latestEvent.id); } function handleCheckClick() { @@ -73,38 +50,16 @@ export function MergedItem({fingerprint, totalFingerprint}: Props) { // we handle change via row click } - function renderFingerprint(id: string, label?: string) { - if (!label) { - return id; - } - - return ( - - {label} - - ); - } - - useEffect(() => { - const teardown = GroupingStore.listen((data: any) => onGroupChange(data), undefined); - return () => { - teardown(); - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - - const {latestEvent, id, label} = fingerprint; + const {latestEvent, id} = fingerprint; const checkboxDisabled = busy || totalFingerprint === 1; - const issueLink = latestEvent - ? createIssueLink({ - organization, - location, - data: latestEvent, - eventId: latestEvent.id, - referrer: 'merged-item', - }) - : null; + const issueLink = createIssueLink({ + organization, + location, + data: latestEvent, + eventId: latestEvent.id, + referrer: 'merged-item', + }); // `latestEvent` can be null if last event w/ fingerprint is not within retention period return ( @@ -128,7 +83,7 @@ export function MergedItem({fingerprint, totalFingerprint}: Props) { size="xs" /> - {renderFingerprint(id, label)} + {id} {fingerprint.mergedBySeer && ' (merged by Sentry)'} @@ -145,24 +100,22 @@ export function MergedItem({fingerprint, totalFingerprint}: Props) { {!collapsed && ( - {issueLink ? ( - - } - tooltipProps={{title: t('View latest event')}} - aria-label={t('View latest event')} - variant="transparent" - size="xs" - style={{marginLeft: theme.space.md}} - /> - - - {latestEvent.title} - - + + } + tooltipProps={{title: t('View latest event')}} + aria-label={t('View latest event')} + variant="transparent" + size="xs" + style={{marginLeft: theme.space.md}} + /> + + + {latestEvent.title} + - ) : null} + )} diff --git a/static/app/views/issueDetails/groupMerged/mergedList.tsx b/static/app/views/issueDetails/groupMerged/mergedList.tsx index 10e31e8bc1a4..84cea0133cf2 100644 --- a/static/app/views/issueDetails/groupMerged/mergedList.tsx +++ b/static/app/views/issueDetails/groupMerged/mergedList.tsx @@ -6,18 +6,15 @@ import {EmptyStateWarning} from 'sentry/components/emptyStateWarning'; import {Panel} from 'sentry/components/panels/panel'; import {PanelBody} from 'sentry/components/panels/panelBody'; import {t} from 'sentry/locale'; -import type {Fingerprint} from 'sentry/stores/groupingStore'; import type {Group} from 'sentry/types/group'; import type {Project} from 'sentry/types/project'; import {MergedItem} from './mergedItem'; import {MergedToolbar} from './mergedToolbar'; +import {hasLatestEvent, type Fingerprint} from './useGroupMerged'; type Props = { groupId: Group['id']; - /** - * From GroupingStore.onToggleCollapseFingerprints - */ onToggleCollapse: () => void; /** * From GroupMergedView -> handleUnmerge @@ -36,9 +33,7 @@ export function MergedList({ groupId, project, }: Props) { - const fingerprintsWithLatestEvent = fingerprints.filter( - ({latestEvent}) => !!latestEvent - ); + const fingerprintsWithLatestEvent = fingerprints.filter(hasLatestEvent); const hasResults = fingerprintsWithLatestEvent.length > 0; if (!hasResults) { return ( diff --git a/static/app/views/issueDetails/groupMerged/mergedToolbar.tsx b/static/app/views/issueDetails/groupMerged/mergedToolbar.tsx index f238289cf8b4..65144028ff62 100644 --- a/static/app/views/issueDetails/groupMerged/mergedToolbar.tsx +++ b/static/app/views/issueDetails/groupMerged/mergedToolbar.tsx @@ -5,11 +5,11 @@ import {openDiffModal} from 'sentry/actionCreators/modal'; import {Confirm} from 'sentry/components/confirm'; import {PanelHeader} from 'sentry/components/panels/panelHeader'; import {t, tct} from 'sentry/locale'; -import {GroupingStore} from 'sentry/stores/groupingStore'; -import {useLegacyStore} from 'sentry/stores/useLegacyStore'; import type {Group} from 'sentry/types/group'; import type {Project} from 'sentry/types/project'; +import {isAllUnmergedSelected, useGroupMerged} from './useGroupMerged'; + type Props = { groupId: Group['id']; onToggleCollapse: () => void; @@ -18,15 +18,11 @@ type Props = { }; export function MergedToolbar({groupId, project, onUnmerge, onToggleCollapse}: Props) { - const { - unmergeList, - mergedItems, - unmergeLastCollapsed, - unmergeDisabled, - enableFingerprintCompare, - } = useLegacyStore(GroupingStore); + const {fingerprints, state, enableFingerprintCompare, unmergeDisabled} = + useGroupMerged(); + const {unmergeLastCollapsed, unmergeList} = state; - const unmergeCount = unmergeList?.size ?? 0; + const unmergeCount = unmergeList.size; function handleShowDiff(event: React.MouseEvent) { event.stopPropagation(); @@ -53,11 +49,11 @@ export function MergedToolbar({groupId, project, onUnmerge, onToggleCollapse}: P } const unmergeDisabledReason = - mergedItems.length <= 1 + fingerprints.length <= 1 ? t('To unmerge, the list must contain 2 or more items') : unmergeList.size === 0 ? t('To unmerge, 1 or more items must be selected') - : GroupingStore.isAllUnmergedSelected() + : isAllUnmergedSelected(state, fingerprints) ? t('We are unable to unmerge all items at once') : undefined; @@ -72,7 +68,7 @@ export function MergedToolbar({groupId, project, onUnmerge, onToggleCollapse}: P )} >