Browse Source

ref(js): Remove store listener in SelectedGroupStore (#39726)

Evan Purkhiser 2 years ago
parent
commit
5e7308202a
2 changed files with 31 additions and 28 deletions
  1. 30 22
      static/app/stores/groupStore.tsx
  2. 1 6
      static/app/stores/selectedGroupStore.tsx

+ 30 - 22
static/app/stores/groupStore.tsx

@@ -12,6 +12,7 @@ import {
   GroupStats,
 } from 'sentry/types';
 
+import SelectedGroupStore from './selectedGroupStore';
 import {CommonStoreDefinition} from './types';
 
 function showAlert(msg: string, type: Indicator['type']) {
@@ -38,6 +39,7 @@ interface InternalDefinition {
   removeActivity: (groupId: string, id: string) => number;
   statuses: Record<string, Record<string, boolean>>;
   updateActivity: (groupId: string, id: string, data: Partial<Activity>) => void;
+  updateItems: (itemIds: ItemIds) => void;
 }
 
 interface GroupStoreDefinition extends CommonStoreDefinition<Item[]>, InternalDefinition {
@@ -113,6 +115,12 @@ const storeConfig: GroupStoreDefinition = {
     this.trigger(itemIds);
   },
 
+  updateItems(itemIds: ItemIds) {
+    const idSet = new Set(itemIds);
+    this.trigger(idSet);
+    SelectedGroupStore.onGroupChange(idSet);
+  },
+
   mergeItems(items: Item[]) {
     const itemsById = items.reduce((acc, item) => ({...acc, [item.id]: item}), {});
 
@@ -142,7 +150,7 @@ const storeConfig: GroupStoreDefinition = {
 
     this.items = [...this.items, ...newItems];
 
-    this.trigger(new Set(items.map(item => item.id)));
+    this.updateItems(items.map(item => item.id));
   },
 
   /**
@@ -157,7 +165,7 @@ const storeConfig: GroupStoreDefinition = {
 
     this.items = [...items, ...this.items.filter(item => !itemMap[item.id])];
 
-    this.trigger(new Set(items.map(item => item.id)));
+    this.updateItems(items.map(item => item.id));
   },
 
   /**
@@ -170,7 +178,7 @@ const storeConfig: GroupStoreDefinition = {
   remove(itemIds) {
     this.items = this.items.filter(item => !itemIds?.includes(item.id));
 
-    this.trigger(new Set(itemIds));
+    this.updateItems(itemIds);
   },
 
   addStatus(id, status) {
@@ -221,7 +229,7 @@ const storeConfig: GroupStoreDefinition = {
       group.numComments++;
     }
 
-    this.trigger(new Set([id]));
+    this.updateItems([id]);
   },
 
   updateActivity(groupId, id, data) {
@@ -239,7 +247,7 @@ const storeConfig: GroupStoreDefinition = {
     // into the existing `data` object. This effectively
     // allows passing in an object of only changes.
     group.activity[index].data = Object.assign(group.activity[index].data, data);
-    this.trigger(new Set([group.id]));
+    this.updateItems([group.id]);
   },
 
   removeActivity(groupId, id) {
@@ -259,7 +267,7 @@ const storeConfig: GroupStoreDefinition = {
       group.numComments--;
     }
 
-    this.trigger(new Set([group.id]));
+    this.updateItems([group.id]);
     return index;
   },
 
@@ -300,7 +308,7 @@ const storeConfig: GroupStoreDefinition = {
 
   onAssignTo(_changeId, itemId, _data) {
     this.addStatus(itemId, 'assignTo');
-    this.trigger(new Set([itemId]));
+    this.updateItems([itemId]);
   },
 
   // TODO(dcramer): This is not really the best place for this
@@ -316,13 +324,13 @@ const storeConfig: GroupStoreDefinition = {
     }
     item.assignedTo = response.assignedTo;
     this.clearStatus(itemId, 'assignTo');
-    this.trigger(new Set([itemId]));
+    this.updateItems([itemId]);
   },
 
   onDelete(_changeId, itemIds) {
     const ids = this.itemIdsOrAll(itemIds);
     ids.forEach(itemId => this.addStatus(itemId, 'delete'));
-    this.trigger(new Set(ids));
+    this.updateItems(ids);
   },
 
   onDeleteError(_changeId, itemIds, _response) {
@@ -333,7 +341,7 @@ const storeConfig: GroupStoreDefinition = {
     }
 
     itemIds.forEach(itemId => this.clearStatus(itemId, 'delete'));
-    this.trigger(new Set(itemIds));
+    this.updateItems(itemIds);
   },
 
   onDeleteSuccess(_changeId, itemIds, _response) {
@@ -352,18 +360,18 @@ const storeConfig: GroupStoreDefinition = {
       this.clearStatus(itemId, 'delete');
     });
     this.items = this.items.filter(item => !itemIdSet.has(item.id));
-    this.trigger(new Set(ids));
+    this.updateItems(ids);
   },
 
   onDiscard(_changeId, itemId) {
     this.addStatus(itemId, 'discard');
-    this.trigger(new Set([itemId]));
+    this.updateItems([itemId]);
   },
 
   onDiscardError(_changeId, itemId, _response) {
     this.clearStatus(itemId, 'discard');
     showAlert(t('Unable to discard event. Please try again.'), 'error');
-    this.trigger(new Set([itemId]));
+    this.updateItems([itemId]);
   },
 
   onDiscardSuccess(_changeId, itemId, _response) {
@@ -371,7 +379,7 @@ const storeConfig: GroupStoreDefinition = {
     this.clearStatus(itemId, 'discard');
     this.items = this.items.filter(item => item.id !== itemId);
     showAlert(t('Similar events will be filtered and discarded.'), 'success');
-    this.trigger(new Set([itemId]));
+    this.updateItems([itemId]);
   },
 
   onMerge(_changeId, itemIds) {
@@ -380,7 +388,7 @@ const storeConfig: GroupStoreDefinition = {
     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(ids));
+    this.updateItems(ids);
   },
 
   onMergeError(_changeId, itemIds, _response) {
@@ -388,7 +396,7 @@ const storeConfig: GroupStoreDefinition = {
 
     ids.forEach(itemId => this.clearStatus(itemId, 'merge'));
     showAlert(t('Unable to merge events. Please try again.'), 'error');
-    this.trigger(new Set(ids));
+    this.updateItems(ids);
   },
 
   onMergeSuccess(_changeId, itemIds, response) {
@@ -411,7 +419,7 @@ const storeConfig: GroupStoreDefinition = {
       showAlert(t(`Merged ${ids.length} Issues`), 'success');
     }
 
-    this.trigger(new Set(ids));
+    this.updateItems(ids);
   },
 
   onUpdate(changeId, itemIds, data) {
@@ -422,7 +430,7 @@ const storeConfig: GroupStoreDefinition = {
     });
     this.pendingChanges.set(changeId, {itemIds: ids, data});
 
-    this.trigger(new Set(ids));
+    this.updateItems(ids);
   },
 
   onUpdateError(changeId, itemIds, failSilently) {
@@ -435,7 +443,7 @@ const storeConfig: GroupStoreDefinition = {
       showAlert(t('Unable to update events. Please try again.'), 'error');
     }
 
-    this.trigger(new Set(ids));
+    this.updateItems(ids);
   },
 
   onUpdateSuccess(changeId, itemIds, response) {
@@ -451,7 +459,7 @@ const storeConfig: GroupStoreDefinition = {
       }
     });
     this.pendingChanges.delete(changeId);
-    this.trigger(new Set(ids));
+    this.updateItems(ids);
   },
 
   onPopulateStats(itemIds, response) {
@@ -469,7 +477,7 @@ const storeConfig: GroupStoreDefinition = {
         };
       }
     });
-    this.trigger(new Set(itemIds));
+    this.updateItems(itemIds);
   },
 
   onPopulateReleases(itemId, releaseData) {
@@ -481,7 +489,7 @@ const storeConfig: GroupStoreDefinition = {
         };
       }
     });
-    this.trigger(new Set([itemId]));
+    this.updateItems([itemId]);
   },
 };
 

+ 1 - 6
static/app/stores/selectedGroupStore.tsx

@@ -1,7 +1,6 @@
 import {createStore} from 'reflux';
 
 import GroupStore from 'sentry/stores/groupStore';
-import {makeSafeRefluxStore} from 'sentry/utils/makeSafeRefluxStore';
 
 import {CommonStoreDefinition} from './types';
 
@@ -43,10 +42,6 @@ const storeConfig: SelectedGroupStoreDefinition = {
 
   init() {
     this.reset();
-
-    this.unsubscribeListeners.push(
-      this.listenTo(GroupStore, this.onGroupChange, this.onGroupChange)
-    );
   },
 
   reset() {
@@ -165,5 +160,5 @@ const storeConfig: SelectedGroupStoreDefinition = {
   },
 };
 
-const SelectedGroupStore = createStore(makeSafeRefluxStore(storeConfig));
+const SelectedGroupStore = createStore(storeConfig);
 export default SelectedGroupStore;