Просмотр исходного кода

ref(groupStore): Remove GroupActions and improve typing (#37951)

Evan Purkhiser 2 лет назад
Родитель
Сommit
fd6508abc8

+ 23 - 24
static/app/actionCreators/group.tsx

@@ -1,7 +1,6 @@
 import * as Sentry from '@sentry/react';
 import isNil from 'lodash/isNil';
 
-import GroupActions from 'sentry/actions/groupActions';
 import {Client, RequestCallbacks, RequestOptions} from 'sentry/api';
 import GroupStore from 'sentry/stores/groupStore';
 import {Actor, Group, Member, Note, User} from 'sentry/types';
@@ -26,7 +25,7 @@ export function assignToUser(params: AssignToUserParams) {
 
   const id = uniqueId();
 
-  GroupActions.assignTo(id, params.id, {
+  GroupStore.onAssignTo(id, params.id, {
     email: (params.member && params.member.email) || '',
   });
 
@@ -43,10 +42,10 @@ export function assignToUser(params: AssignToUserParams) {
 
   request
     .then(data => {
-      GroupActions.assignToSuccess(id, params.id, data);
+      GroupStore.onAssignToSuccess(id, params.id, data);
     })
     .catch(data => {
-      GroupActions.assignToError(id, params.id, data);
+      GroupStore.onAssignToError(id, params.id, data);
     });
 
   return request;
@@ -59,7 +58,7 @@ export function clearAssignment(groupId: string, assignedBy: AssignedBy) {
 
   const id = uniqueId();
 
-  GroupActions.assignTo(id, groupId, {
+  GroupStore.onAssignTo(id, groupId, {
     email: '',
   });
 
@@ -74,10 +73,10 @@ export function clearAssignment(groupId: string, assignedBy: AssignedBy) {
 
   request
     .then(data => {
-      GroupActions.assignToSuccess(id, groupId, data);
+      GroupStore.onAssignToSuccess(id, groupId, data);
     })
     .catch(data => {
-      GroupActions.assignToError(id, groupId, data);
+      GroupStore.onAssignToError(id, groupId, data);
     });
 
   return request;
@@ -98,9 +97,9 @@ export function assignToActor({id, actor, assignedBy}: AssignToActorParams) {
   const endpoint = `/issues/${id}/`;
 
   const guid = uniqueId();
-  let actorId;
+  let actorId = '';
 
-  GroupActions.assignTo(guid, id, {email: ''});
+  GroupStore.onAssignTo(guid, id, {email: ''});
 
   switch (actor.type) {
     case 'user':
@@ -124,10 +123,10 @@ export function assignToActor({id, actor, assignedBy}: AssignToActorParams) {
       data: {assignedTo: actorId, assignedBy},
     })
     .then(data => {
-      GroupActions.assignToSuccess(guid, id, data);
+      GroupStore.onAssignToSuccess(guid, id, data);
     })
     .catch(data => {
-      GroupActions.assignToError(guid, id, data);
+      GroupStore.onAssignToSuccess(guid, id, data);
     });
 }
 
@@ -180,9 +179,9 @@ export function updateNote(
 }
 
 type ParamsType = {
-  environment?: string | Array<string> | null;
-  itemIds?: Array<number> | Array<string>;
-  project?: Array<number> | null;
+  environment?: string | string[] | null;
+  itemIds?: string[];
+  project?: number[] | null;
   query?: string;
 };
 
@@ -283,7 +282,7 @@ export function bulkDelete(
   const query: QueryArgs = paramsToQueryArgs(params);
   const id = uniqueId();
 
-  GroupActions.delete(id, itemIds);
+  GroupStore.onDelete(id, itemIds);
 
   return wrapRequest(
     api,
@@ -292,10 +291,10 @@ export function bulkDelete(
       query,
       method: 'DELETE',
       success: response => {
-        GroupActions.deleteSuccess(id, itemIds, response);
+        GroupStore.onDeleteSuccess(id, itemIds, response);
       },
       error: error => {
-        GroupActions.deleteError(id, itemIds, error);
+        GroupStore.onDeleteError(id, itemIds, error);
       },
     },
     options
@@ -318,7 +317,7 @@ export function bulkUpdate(
   const query: QueryArgs = paramsToQueryArgs(params);
   const id = uniqueId();
 
-  GroupActions.update(id, itemIds, data);
+  GroupStore.onUpdate(id, itemIds, data);
 
   return wrapRequest(
     api,
@@ -328,10 +327,10 @@ export function bulkUpdate(
       method: 'PUT',
       data,
       success: response => {
-        GroupActions.updateSuccess(id, itemIds, response);
+        GroupStore.onUpdateSuccess(id, itemIds, response);
       },
-      error: error => {
-        GroupActions.updateError(id, itemIds, error, failSilently);
+      error: () => {
+        GroupStore.onUpdateError(id, itemIds, !!failSilently);
       },
     },
     options
@@ -351,7 +350,7 @@ export function mergeGroups(
   const query: QueryArgs = paramsToQueryArgs(params);
   const id = uniqueId();
 
-  GroupActions.merge(id, itemIds);
+  GroupStore.onMerge(id, itemIds);
 
   return wrapRequest(
     api,
@@ -361,10 +360,10 @@ export function mergeGroups(
       method: 'PUT',
       data: {merge: 1},
       success: response => {
-        GroupActions.mergeSuccess(id, itemIds, response);
+        GroupStore.onMergeSuccess(id, itemIds, response);
       },
       error: error => {
-        GroupActions.mergeError(id, itemIds, error);
+        GroupStore.onMergeError(id, itemIds, error);
       },
     },
     options

+ 0 - 24
static/app/actions/groupActions.tsx

@@ -1,24 +0,0 @@
-import {createActions} from 'reflux';
-
-// TODO(dcramer): we should probably just make every parameter update
-// work on bulk groups
-const GroupActions = createActions([
-  'assignTo',
-  'assignToError',
-  'assignToSuccess',
-  'delete',
-  'deleteError',
-  'deleteSuccess',
-  'discard',
-  'discardError',
-  'discardSuccess',
-  'update',
-  'updateError',
-  'updateSuccess',
-  'merge',
-  'mergeError',
-  'mergeSuccess',
-  'populateStats',
-]);
-
-export default GroupActions;

+ 6 - 2
static/app/plugins/components/issueActions.tsx

@@ -1,9 +1,9 @@
-import GroupActions from 'sentry/actions/groupActions';
 import PluginComponentBase from 'sentry/components/bases/pluginComponentBase';
 import {Form, FormState} from 'sentry/components/deprecatedforms';
 import LoadingError from 'sentry/components/loadingError';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {t} from 'sentry/locale';
+import GroupStore from 'sentry/stores/groupStore';
 import {Group, Organization, Plugin, Project} from 'sentry/types';
 
 type Field = {
@@ -301,7 +301,11 @@ class IssueActions extends PluginComponentBase<Props, State> {
   }
 
   onSuccess(data) {
-    GroupActions.updateSuccess(null, [this.getGroup().id], {stale: true});
+    // TODO(ts): This needs a better approach. We splice in this attribute to trigger
+    // a refetch in GroupDetails
+    type StaleGroup = Group & {stale?: boolean};
+
+    GroupStore.onUpdateSuccess('', [this.getGroup().id], {stale: true} as StaleGroup);
     this.props.onSuccess && this.props.onSuccess(data);
   }
 

+ 79 - 83
static/app/stores/groupStore.tsx

@@ -2,7 +2,6 @@ import isArray from 'lodash/isArray';
 import {createStore, StoreDefinition} from 'reflux';
 
 import {Indicator} from 'sentry/actionCreators/indicator';
-import GroupActions from 'sentry/actions/groupActions';
 import {t} from 'sentry/locale';
 import IndicatorStore from 'sentry/stores/indicatorStore';
 import {
@@ -28,6 +27,8 @@ type Change = {
 
 type Item = BaseGroup | Group | GroupCollapseRelease;
 
+type ItemIds = string[] | undefined;
+
 interface InternalDefinition {
   addActivity: (groupId: string, data: Activity, index?: number) => void;
   indexOfActivity: (groupId: string, id: string) => number;
@@ -43,46 +44,47 @@ interface GroupStoreDefinition extends StoreDefinition, InternalDefinition {
   add: (items: Item[]) => void;
   addStatus: (id: string, status: string) => void;
   clearStatus: (id: string, status: string) => void;
+
   get: (id: string) => Item | undefined;
   getAllItemIds: () => string[];
   getAllItems: () => Item[];
+
   hasStatus: (id: string, status: string) => boolean;
   init: () => void;
+
+  itemIdsOrAll(itemIds: ItemIds): string[];
+
   loadInitialData: (items: Item[]) => void;
+
   onAssignTo: (changeId: string, itemId: string, data: any) => void;
   onAssignToError: (changeId: string, itemId: string, error: Error) => void;
   onAssignToSuccess: (changeId: string, itemId: string, response: any) => void;
-  onDelete: (changeId: string, itemIds: string[]) => void;
-  onDeleteError: (changeId: string, itemIds: string[], error: Error) => void;
-  onDeleteSuccess: (changeId: string, itemIds: string[], response: any) => void;
+
+  onDelete: (changeId: string, itemIds: ItemIds) => void;
+  onDeleteError: (changeId: string, itemIds: ItemIds, error: Error) => void;
+  onDeleteSuccess: (changeId: string, itemIds: ItemIds, response: any) => void;
+
   onDiscard: (changeId: string, itemId: string) => void;
   onDiscardError: (changeId: string, itemId: string, response: any) => void;
   onDiscardSuccess: (changeId: string, itemId: string, response: any) => void;
-  onMerge: (changeId: string, itemIds: string[]) => void;
-  onMergeError: (changeId: string, itemIds: string[], response: any) => void;
-  onMergeSuccess: (changeId: string, itemIds: string[], response: any) => void;
+
+  onMerge: (changeId: string, itemIds: ItemIds) => void;
+  onMergeError: (changeId: string, itemIds: ItemIds, response: any) => void;
+  onMergeSuccess: (changeId: string, itemIds: ItemIds, response: any) => void;
+
   onPopulateReleases: (itemId: string, releaseData: GroupRelease) => void;
-  onPopulateStats: (itemIds: string[], response: GroupStats[]) => void;
-  onUpdate: (changeId: string, itemIds: string[], data: any) => void;
-  onUpdateError: (
-    changeId: string,
-    itemIds: string[],
-    error: Error,
-    silent: boolean
-  ) => void;
-  onUpdateSuccess: (
-    changeId: string,
-    itemIds: string[],
-    response: Partial<Group>
-  ) => void;
-
-  remove: (itemIds: string[]) => void;
+  onPopulateStats: (itemIds: ItemIds, response: GroupStats[]) => void;
+
+  onUpdate: (changeId: string, itemIds: ItemIds, data: any) => void;
+  onUpdateError: (changeId: string, itemIds: ItemIds, silent: boolean) => void;
+  onUpdateSuccess: (changeId: string, itemIds: ItemIds, response: Partial<Group>) => void;
+
+  remove: (itemIds: ItemIds) => void;
 
   reset: () => void;
 }
 
 const storeConfig: GroupStoreDefinition = {
-  listenables: [GroupActions],
   pendingChanges: new Map(),
   items: [],
   statuses: {},
@@ -140,8 +142,15 @@ const storeConfig: GroupStoreDefinition = {
     this.trigger(itemIds);
   },
 
+  /**
+   * If itemIds is undefined, returns all ids in the store
+   */
+  itemIdsOrAll(itemIds: ItemIds) {
+    return itemIds === undefined ? this.getAllItemIds() : itemIds;
+  },
+
   remove(itemIds) {
-    this.items = this.items.filter(item => !itemIds.includes(item.id));
+    this.items = this.items.filter(item => !itemIds?.includes(item.id));
 
     this.trigger(new Set(itemIds));
   },
@@ -287,11 +296,9 @@ const storeConfig: GroupStoreDefinition = {
   },
 
   onDelete(_changeId, itemIds) {
-    itemIds = this._itemIdsOrAll(itemIds);
-    itemIds.forEach(itemId => {
-      this.addStatus(itemId, 'delete');
-    });
-    this.trigger(new Set(itemIds));
+    const ids = this.itemIdsOrAll(itemIds);
+    ids.forEach(itemId => this.addStatus(itemId, 'delete'));
+    this.trigger(new Set(ids));
   },
 
   onDeleteError(_changeId, itemIds, _response) {
@@ -301,29 +308,27 @@ const storeConfig: GroupStoreDefinition = {
       return;
     }
 
-    itemIds.forEach(itemId => {
-      this.clearStatus(itemId, 'delete');
-    });
+    itemIds.forEach(itemId => this.clearStatus(itemId, 'delete'));
     this.trigger(new Set(itemIds));
   },
 
   onDeleteSuccess(_changeId, itemIds, _response) {
-    itemIds = this._itemIdsOrAll(itemIds);
+    const ids = this.itemIdsOrAll(itemIds);
 
-    if (itemIds.length > 1) {
-      showAlert(t(`Deleted ${itemIds.length} Issues`), 'success');
+    if (ids.length > 1) {
+      showAlert(t(`Deleted ${ids.length} Issues`), 'success');
     } else {
-      const shortId = itemIds.map(item => GroupStore.get(item)?.shortId).join('');
+      const shortId = ids.map(item => GroupStore.get(item)?.shortId).join('');
       showAlert(t(`Deleted ${shortId}`), 'success');
     }
 
-    const itemIdSet = new Set(itemIds);
-    itemIds.forEach(itemId => {
+    const itemIdSet = new Set(ids);
+    ids.forEach(itemId => {
       delete this.statuses[itemId];
       this.clearStatus(itemId, 'delete');
     });
     this.items = this.items.filter(item => !itemIdSet.has(item.id));
-    this.trigger(new Set(itemIds));
+    this.trigger(new Set(ids));
   },
 
   onDiscard(_changeId, itemId) {
@@ -346,35 +351,29 @@ const storeConfig: GroupStoreDefinition = {
   },
 
   onMerge(_changeId, itemIds) {
-    itemIds = this._itemIdsOrAll(itemIds);
+    const ids = this.itemIdsOrAll(itemIds);
 
-    itemIds.forEach(itemId => {
-      this.addStatus(itemId, 'merge');
-    });
+    ids.forEach(itemId => this.addStatus(itemId, 'merge'));
     // XXX(billy): Not sure if this is a bug or not but do we need to publish all itemIds?
     // Seems like we only need to publish parent id
-    this.trigger(new Set(itemIds));
+    this.trigger(new Set(ids));
   },
 
   onMergeError(_changeId, itemIds, _response) {
-    itemIds = this._itemIdsOrAll(itemIds);
+    const ids = this.itemIdsOrAll(itemIds);
 
-    itemIds.forEach(itemId => {
-      this.clearStatus(itemId, 'merge');
-    });
+    ids.forEach(itemId => this.clearStatus(itemId, 'merge'));
     showAlert(t('Unable to merge events. Please try again.'), 'error');
-    this.trigger(new Set(itemIds));
+    this.trigger(new Set(ids));
   },
 
-  onMergeSuccess(_changeId, mergedIds, response) {
-    mergedIds = this._itemIdsOrAll(mergedIds); // everything on page
+  onMergeSuccess(_changeId, itemIds, response) {
+    const ids = this.itemIdsOrAll(itemIds); // everything on page
 
-    mergedIds.forEach(itemId => {
-      this.clearStatus(itemId, 'merge');
-    });
+    ids.forEach(itemId => this.clearStatus(itemId, 'merge'));
 
     // Remove all but parent id (items were merged into this one)
-    const mergedIdSet = new Set(mergedIds);
+    const mergedIdSet = new Set(ids);
 
     // Looks like the `PUT /api/0/projects/:orgId/:projectId/issues/` endpoint
     // actually returns a 204, so there is no `response` body
@@ -384,45 +383,42 @@ const storeConfig: GroupStoreDefinition = {
         (response && response.merge && item.id === response.merge.parent)
     );
 
-    showAlert(t(`Merged ${mergedIds.length} Issues`), 'success');
-    this.trigger(new Set(mergedIds));
-  },
+    if (ids.length > 0) {
+      showAlert(t(`Merged ${ids.length} Issues`), 'success');
+    }
 
-  /**
-   * If itemIds is undefined, returns all ids in the store
-   */
-  _itemIdsOrAll(itemIds) {
-    return itemIds === undefined ? this.getAllItemIds() : itemIds;
+    this.trigger(new Set(ids));
   },
 
   onUpdate(changeId, itemIds, data) {
-    itemIds = this._itemIdsOrAll(itemIds);
+    const ids = this.itemIdsOrAll(itemIds);
 
-    itemIds.forEach(itemId => {
+    ids.forEach(itemId => {
       this.addStatus(itemId, 'update');
       this.pendingChanges.set(changeId, {itemId, data});
     });
-    this.trigger(new Set(itemIds));
+
+    this.trigger(new Set(ids));
   },
 
-  onUpdateError(changeId, itemIds, _error, failSilently) {
-    itemIds = this._itemIdsOrAll(itemIds);
+  onUpdateError(changeId, itemIds, failSilently) {
+    const ids = this.itemIdsOrAll(itemIds);
 
     this.pendingChanges.delete(changeId);
-    itemIds.forEach(itemId => {
-      this.clearStatus(itemId, 'update');
-    });
+    ids.forEach(itemId => this.clearStatus(itemId, 'update'));
+
     if (!failSilently) {
       showAlert(t('Unable to update events. Please try again.'), 'error');
     }
-    this.trigger(new Set(itemIds));
+
+    this.trigger(new Set(ids));
   },
 
   onUpdateSuccess(changeId, itemIds, response) {
-    itemIds = this._itemIdsOrAll(itemIds);
+    const ids = this.itemIdsOrAll(itemIds);
 
     this.items.forEach((item, idx) => {
-      if (itemIds.indexOf(item.id) !== -1) {
+      if (ids.includes(item.id)) {
         this.items[idx] = {
           ...item,
           ...response,
@@ -431,28 +427,28 @@ const storeConfig: GroupStoreDefinition = {
       }
     });
     this.pendingChanges.delete(changeId);
-    this.trigger(new Set(itemIds));
+    this.trigger(new Set(ids));
   },
 
-  onPopulateStats(itemIds: string[], response: GroupStats[]) {
+  onPopulateStats(itemIds, response) {
     // Organize stats by id
-    const groupStatsMap = response.reduce((map, stats) => {
-      map[stats.id] = stats;
-      return map;
-    }, {});
+    const groupStatsMap = response.reduce<Record<string, GroupStats>>(
+      (map, stats) => ({...map, [stats.id]: stats}),
+      {}
+    );
 
     this.items.forEach((item, idx) => {
-      if (itemIds.includes(item.id)) {
+      if (itemIds?.includes(item.id)) {
         this.items[idx] = {
           ...item,
           ...groupStatsMap[item.id],
         };
       }
     });
-    this.trigger(new Set(this.items.map(item => item.id)));
+    this.trigger(new Set(itemIds));
   },
 
-  onPopulateReleases(itemId: string, releaseData: GroupRelease) {
+  onPopulateReleases(itemId, releaseData) {
     this.items.forEach((item, idx) => {
       if (item.id === itemId) {
         this.items[idx] = {

+ 1 - 1
static/app/stores/groupingStore.tsx

@@ -518,7 +518,7 @@ const storeConfig: GroupingStoreDefinition = {
         {
           orgId,
           projectId: projectId || params.projectId,
-          itemIds: [...ids, groupId] as Array<number>,
+          itemIds: [...ids, groupId],
           query,
         },
         {

+ 1 - 2
static/app/views/issueList/overview.tsx

@@ -19,7 +19,6 @@ import {
   resetSavedSearches,
 } from 'sentry/actionCreators/savedSearches';
 import {fetchTagValues, loadOrganizationTags} from 'sentry/actionCreators/tags';
-import GroupActions from 'sentry/actions/groupActions';
 import {Client} from 'sentry/api';
 import * as Layout from 'sentry/components/layouts/thirds';
 import LoadingError from 'sentry/components/loadingError';
@@ -411,7 +410,7 @@ class IssueListOverview extends Component<Props, State> {
         if (!data) {
           return;
         }
-        GroupActions.populateStats(groups, data);
+        GroupStore.onPopulateStats(groups, data);
       },
       error: err => {
         this.setState({

+ 4 - 4
static/app/views/organizationGroupDetails/actions/index.tsx

@@ -14,7 +14,6 @@ import {
   openModal,
   openReprocessEventModal,
 } from 'sentry/actionCreators/modal';
-import GroupActions from 'sentry/actions/groupActions';
 import {Client} from 'sentry/api';
 import Access from 'sentry/components/acl/access';
 import Feature from 'sentry/components/acl/feature';
@@ -28,6 +27,7 @@ import DropdownMenuControl from 'sentry/components/dropdownMenuControl';
 import Tooltip from 'sentry/components/tooltip';
 import {IconEllipsis} from 'sentry/icons';
 import {t} from 'sentry/locale';
+import GroupStore from 'sentry/stores/groupStore';
 import space from 'sentry/styles/space';
 import {
   Group,
@@ -254,17 +254,17 @@ class Actions extends Component<Props, State> {
     const id = uniqueId();
     addLoadingMessage(t('Discarding event\u2026'));
 
-    GroupActions.discard(id, group.id);
+    GroupStore.onDiscard(id, group.id);
 
     api.request(`/issues/${group.id}/`, {
       method: 'PUT',
       data: {discard: true},
       success: response => {
-        GroupActions.discardSuccess(id, group.id, response);
+        GroupStore.onDiscardSuccess(id, group.id, response);
         browserHistory.push(`/${organization.slug}/${project.slug}/`);
       },
       error: error => {
-        GroupActions.discardError(id, group.id, error);
+        GroupStore.onDiscardError(id, group.id, error);
       },
       complete: clearIndicators,
     });

+ 5 - 5
tests/js/spec/actionCreators/group.spec.jsx

@@ -1,6 +1,6 @@
 import {bulkUpdate, mergeGroups, paramsToQueryArgs} from 'sentry/actionCreators/group';
-import GroupActions from 'sentry/actions/groupActions';
 import {Client} from 'sentry/api';
+import GroupStore from 'sentry/stores/groupStore';
 
 describe('group', () => {
   let api;
@@ -82,7 +82,7 @@ describe('group', () => {
   describe('bulkUpdate()', function () {
     beforeEach(function () {
       jest.spyOn(api, 'request');
-      jest.spyOn(GroupActions, 'update'); // stub GroupActions.update call from update
+      jest.spyOn(GroupStore, 'onUpdate'); // stub GroupStore.onUpdate call from update
     });
 
     it('should use itemIds as query if provided', function () {
@@ -105,7 +105,7 @@ describe('group', () => {
       bulkUpdate(api, {
         orgId: '1337',
         projectId: '1337',
-        itemIds: null,
+        itemIds: undefined,
         data: {status: 'unresolved'},
         query: 'is:resolved',
       });
@@ -138,7 +138,7 @@ describe('group', () => {
     //       these API methods/tests.
     beforeEach(function () {
       jest.spyOn(api, 'request');
-      jest.spyOn(GroupActions, 'merge'); // stub GroupActions.merge call from mergeGroups
+      jest.spyOn(GroupStore, 'onMerge'); // stub GroupStore.onMerge call from mergeGroups
     });
 
     it('should use itemIds as query if provided', function () {
@@ -161,7 +161,7 @@ describe('group', () => {
       mergeGroups(api, {
         orgId: '1337',
         projectId: '1337',
-        itemIds: null,
+        itemIds: undefined,
         data: {status: 'unresolved'},
         query: 'is:resolved',
       });

+ 0 - 166
tests/js/spec/stores/groupStore.spec.jsx

@@ -1,166 +0,0 @@
-import GroupStore from 'sentry/stores/groupStore';
-
-describe('GroupStore', function () {
-  beforeEach(function () {
-    GroupStore.reset();
-  });
-
-  describe('add()', function () {
-    it('should add new entries', function () {
-      GroupStore.items = [];
-      GroupStore.add([{id: 1}, {id: 2}]);
-
-      expect(GroupStore.items).toEqual([{id: 1}, {id: 2}]);
-    });
-
-    it('should update matching existing entries', function () {
-      GroupStore.items = [{id: 1}, {id: 2}];
-
-      GroupStore.add([{id: 1, foo: 'bar'}, {id: 3}]);
-
-      expect(GroupStore.items).toEqual([{id: 1, foo: 'bar'}, {id: 2}, {id: 3}]);
-    });
-
-    it('should attempt to preserve order of ids', function () {
-      GroupStore.add([{id: 2}, {id: 1}, {id: 3}]);
-      expect(GroupStore.getAllItemIds()).toEqual([2, 1, 3]);
-    });
-  });
-
-  describe('remove()', function () {
-    it('should remove entry', function () {
-      GroupStore.items = [{id: 1}, {id: 2}];
-      GroupStore.remove([1]);
-
-      expect(GroupStore.items).toEqual([{id: 2}]);
-    });
-
-    it('should remove multiple entries', function () {
-      GroupStore.items = [{id: 1}, {id: 2}, {id: 3}];
-      GroupStore.remove([1, 2]);
-
-      expect(GroupStore.items).toEqual([{id: 3}]);
-    });
-
-    it('should not remove already removed item', function () {
-      GroupStore.items = [{id: 1}, {id: 2}];
-      GroupStore.remove([0]);
-
-      expect(GroupStore.items).toEqual([{id: 1}, {id: 2}]);
-    });
-  });
-
-  describe('onMergeSuccess()', function () {
-    it('should remove the non-parent merged ids', function () {
-      GroupStore.items = [{id: 1}, {id: 2}, {id: 3}, {id: 4}];
-
-      GroupStore.onMergeSuccess(
-        null,
-        [2, 3, 4], // items merged
-        {merge: {parent: 3}} // merge API response
-      );
-
-      expect(GroupStore.items).toEqual([
-        {id: 1},
-        {id: 3}, // parent
-      ]);
-    });
-  });
-
-  describe('onPopulateStats()', function () {
-    const stats = {auto: [[1611576000, 10]]};
-    beforeAll(function () {
-      jest.spyOn(GroupStore, 'trigger');
-    });
-    beforeEach(function () {
-      GroupStore.trigger.mockReset();
-      GroupStore.items = [{id: 1}, {id: 2}, {id: 3}];
-    });
-
-    it('should merge stats into existing groups', function () {
-      GroupStore.onPopulateStats(
-        [1, 2, 3],
-        [
-          {id: 1, stats},
-          {id: 2, stats},
-          {id: 3, stats},
-        ]
-      );
-      expect(GroupStore.getAllItems()[0].stats).toEqual(stats);
-      expect(GroupStore.trigger).toHaveBeenCalledWith(new Set([1, 2, 3]));
-    });
-
-    it('should not change current item ids', function () {
-      GroupStore.onPopulateStats(
-        [2, 3],
-        [
-          {id: 2, stats},
-          {id: 3, stats},
-        ]
-      );
-      expect(GroupStore.trigger).toHaveBeenCalledWith(new Set([1, 2, 3]));
-      expect(GroupStore.getAllItems()[0].stats).toBeUndefined();
-      expect(GroupStore.getAllItems()[1].stats).toEqual(stats);
-    });
-  });
-
-  describe('getAllItems()', function () {
-    it('Merges pending changes into items', function () {
-      GroupStore.items = [];
-      GroupStore.add([{id: 1}, {id: 2}]);
-
-      GroupStore.onUpdate(1337, [1], {someChange: true});
-
-      expect(GroupStore.getAllItems()).toEqual([{id: 1, someChange: true}, {id: 2}]);
-    });
-  });
-
-  describe('update methods', function () {
-    beforeAll(function () {
-      jest.spyOn(GroupStore, 'trigger');
-    });
-    beforeEach(function () {
-      GroupStore.trigger.mockReset();
-    });
-
-    beforeEach(function () {
-      GroupStore.items = [{id: 1}, {id: 2}, {id: 3}];
-    });
-
-    describe('onUpdate()', function () {
-      it("should treat undefined itemIds argument as 'all'", function () {
-        GroupStore.onUpdate(1337, undefined, 'somedata');
-
-        expect(GroupStore.trigger).toHaveBeenCalledTimes(1);
-        expect(GroupStore.trigger).toHaveBeenCalledWith(new Set([1, 2, 3]));
-      });
-    });
-
-    describe('onUpdateSuccess()', function () {
-      it("should treat undefined itemIds argument as 'all'", function () {
-        GroupStore.onUpdateSuccess(1337, undefined, 'somedata');
-
-        expect(GroupStore.trigger).toHaveBeenCalledTimes(1);
-        expect(GroupStore.trigger).toHaveBeenCalledWith(new Set([1, 2, 3]));
-      });
-    });
-
-    describe('onUpdateError()', function () {
-      it("should treat undefined itemIds argument as 'all'", function () {
-        GroupStore.onUpdateError(1337, undefined, 'something failed', false);
-
-        expect(GroupStore.trigger).toHaveBeenCalledTimes(1);
-        expect(GroupStore.trigger).toHaveBeenCalledWith(new Set([1, 2, 3]));
-      });
-    });
-
-    describe('onDeleteSuccess()', function () {
-      it("should treat undefined itemIds argument as 'all'", function () {
-        GroupStore.onDeleteSuccess(1337, undefined, 'somedata');
-
-        expect(GroupStore.trigger).toHaveBeenCalledTimes(1);
-        expect(GroupStore.trigger).toHaveBeenCalledWith(new Set([1, 2, 3]));
-      });
-    });
-  });
-});

+ 181 - 0
tests/js/spec/stores/groupStore.spec.tsx

@@ -0,0 +1,181 @@
+import GroupStore from 'sentry/stores/groupStore';
+import {Group, GroupStats, TimeseriesValue} from 'sentry/types';
+
+const g = (id: string, params?: Partial<Group>) => TestStubs.Group({id, ...params});
+
+describe('GroupStore', function () {
+  beforeEach(function () {
+    GroupStore.reset();
+  });
+
+  describe('add()', function () {
+    it('should add new entries', function () {
+      GroupStore.items = [];
+      GroupStore.add([g('1'), g('2')]);
+
+      expect(GroupStore.items).toEqual([g('1'), g('2')]);
+    });
+
+    it('should update matching existing entries', function () {
+      GroupStore.items = [g('1'), g('2')];
+
+      GroupStore.add([{id: '1', foo: 'bar'}, g('3')]);
+
+      expect(GroupStore.getAllItemIds()).toEqual(['1', '2', '3']);
+      expect(GroupStore.items[0]).toEqual(expect.objectContaining({id: '1', foo: 'bar'}));
+    });
+
+    it('should attempt to preserve order of ids', function () {
+      GroupStore.add([g('2'), g('1'), g('3')]);
+      expect(GroupStore.getAllItemIds()).toEqual(['2', '1', '3']);
+    });
+  });
+
+  describe('remove()', function () {
+    it('should remove entry', function () {
+      GroupStore.items = [g('1'), g('2')];
+      GroupStore.remove(['1']);
+
+      expect(GroupStore.items).toEqual([g('2')]);
+    });
+
+    it('should remove multiple entries', function () {
+      GroupStore.items = [g('1'), g('2'), g('3')];
+      GroupStore.remove(['1', '2']);
+
+      expect(GroupStore.items).toEqual([g('3')]);
+    });
+
+    it('should not remove already removed item', function () {
+      GroupStore.items = [g('1'), g('2')];
+      GroupStore.remove(['0']);
+
+      expect(GroupStore.items).toEqual([g('1'), g('2')]);
+    });
+  });
+
+  describe('onMergeSuccess()', function () {
+    it('should remove the non-parent merged ids', function () {
+      GroupStore.items = [g('1'), g('2'), g('3'), g('4')];
+
+      GroupStore.onMergeSuccess(
+        '',
+        ['2', '3', '4'], // items merged
+        {merge: {parent: '3'}} // merge API response
+      );
+
+      expect(GroupStore.items).toEqual([
+        g('1'),
+        g('3'), // parent
+      ]);
+    });
+  });
+
+  describe('onPopulateStats()', function () {
+    let triggerSpy: jest.SpyInstance;
+
+    const stats: Record<string, TimeseriesValue[]> = {auto: [[1611576000, 10]]};
+
+    beforeAll(function () {
+      triggerSpy = jest.spyOn(GroupStore, 'trigger');
+    });
+    beforeEach(function () {
+      triggerSpy.mockReset();
+      GroupStore.items = [g('1'), g('2'), g('3')];
+    });
+
+    it('should merge stats into existing groups', function () {
+      GroupStore.onPopulateStats(
+        ['1', '2', '3'],
+        [
+          {id: '1', stats} as GroupStats,
+          {id: '2', stats} as GroupStats,
+          {id: '3', stats} as GroupStats,
+        ]
+      );
+
+      const group = GroupStore.getAllItems()[0] as Group;
+
+      expect(group.stats).toEqual(stats);
+      expect(GroupStore.trigger).toHaveBeenCalledWith(new Set(['1', '2', '3']));
+    });
+
+    it('should not change current item ids', function () {
+      GroupStore.onPopulateStats(
+        ['2', '3'],
+        [{id: '2', stats} as GroupStats, {id: '3', stats} as GroupStats]
+      );
+
+      const group1 = GroupStore.getAllItems()[0] as Group;
+      const group2 = GroupStore.getAllItems()[1] as Group;
+
+      expect(GroupStore.trigger).toHaveBeenCalledWith(new Set(['2', '3']));
+      expect(group1.stats).not.toEqual(stats);
+      expect(group2.stats).toEqual(stats);
+    });
+  });
+
+  describe('getAllItems()', function () {
+    it('Merges pending changes into items', function () {
+      GroupStore.items = [];
+      GroupStore.add([g('1'), g('2')]);
+
+      GroupStore.onUpdate('1337', ['1'], {someChange: true});
+
+      expect(GroupStore.get('1')).toEqual(
+        expect.objectContaining({id: '1', someChange: true})
+      );
+    });
+  });
+
+  describe('update methods', function () {
+    let triggerSpy: jest.SpyInstance;
+
+    beforeAll(function () {
+      triggerSpy = jest.spyOn(GroupStore, 'trigger');
+    });
+    beforeEach(function () {
+      triggerSpy.mockReset();
+    });
+
+    beforeEach(function () {
+      GroupStore.items = [g('1'), g('2'), g('3')];
+    });
+
+    describe('onUpdate()', function () {
+      it("should treat undefined itemIds argument as 'all'", function () {
+        GroupStore.onUpdate('1337', undefined, {});
+
+        expect(GroupStore.trigger).toHaveBeenCalledTimes(1);
+        expect(GroupStore.trigger).toHaveBeenCalledWith(new Set(['1', '2', '3']));
+      });
+    });
+
+    describe('onUpdateSuccess()', function () {
+      it("should treat undefined itemIds argument as 'all'", function () {
+        GroupStore.onUpdateSuccess('1337', undefined, {});
+
+        expect(GroupStore.trigger).toHaveBeenCalledTimes(1);
+        expect(GroupStore.trigger).toHaveBeenCalledWith(new Set(['1', '2', '3']));
+      });
+    });
+
+    describe('onUpdateError()', function () {
+      it("should treat undefined itemIds argument as 'all'", function () {
+        GroupStore.onUpdateError('1337', undefined, false);
+
+        expect(GroupStore.trigger).toHaveBeenCalledTimes(1);
+        expect(GroupStore.trigger).toHaveBeenCalledWith(new Set(['1', '2', '3']));
+      });
+    });
+
+    describe('onDeleteSuccess()', function () {
+      it("should treat undefined itemIds argument as 'all'", function () {
+        GroupStore.onDeleteSuccess('1337', undefined, {});
+
+        expect(GroupStore.trigger).toHaveBeenCalledTimes(1);
+        expect(GroupStore.trigger).toHaveBeenCalledWith(new Set(['1', '2', '3']));
+      });
+    });
+  });
+});