Browse Source

Fix onUpdateError not handling undefined itemIds

Ben Vinegar 9 years ago
parent
commit
14b2b83e0e

+ 14 - 4
src/sentry/static/sentry/app/stores/groupStore.jsx

@@ -281,10 +281,19 @@ const GroupStore = Reflux.createStore({
     this.trigger(new Set(mergedIds));
   },
 
-  onUpdate(changeId, itemIds, data) {
+  /**
+   * If itemIds is undefined, returns all ids in the store
+   */
+  _itemIdsOrAll(itemIds) {
     if (typeof itemIds === 'undefined') {
       itemIds = this.items.map(item => item.id);
     }
+    return itemIds;
+  },
+
+  onUpdate(changeId, itemIds, data) {
+    itemIds = this._itemIdsOrAll(itemIds);
+
     itemIds.forEach(itemId => {
       this.addStatus(itemId, 'update');
       this.pendingChanges.push(changeId, itemId, data);
@@ -293,6 +302,8 @@ const GroupStore = Reflux.createStore({
   },
 
   onUpdateError(changeId, itemIds, error, failSilently) {
+    itemIds = this._itemIdsOrAll(itemIds);
+
     this.pendingChanges.remove(changeId);
     itemIds.forEach(itemId => {
       this.clearStatus(itemId, 'update');
@@ -304,9 +315,8 @@ const GroupStore = Reflux.createStore({
   },
 
   onUpdateSuccess(changeId, itemIds, response) {
-    if (typeof itemIds === 'undefined') {
-      itemIds = this.items.map(item => item.id);
-    }
+    itemIds = this._itemIdsOrAll(itemIds);
+
     this.items.forEach((item, idx) => {
       if (itemIds.indexOf(item.id) !== -1) {
         this.items[idx] = jQuery.extend(true, {}, item, response);

+ 26 - 19
tests/js/spec/stores/groupStore.spec.jsx

@@ -31,36 +31,43 @@ describe('GroupStore', function () {
     });
   });
 
-  describe('onUpdate()', function () {
-    it('should treat undefined itemIds argument as \'all\'', function () {
+  describe('update methods', function () {
+    beforeEach(function () {
       GroupStore.items = [
         {id: 1},
         {id: 2},
         {id: 3},
       ];
+    });
 
-      this.sandbox.stub(GroupStore, 'trigger');
-      GroupStore.onUpdate(1337, undefined, 'somedata');
-
+    describe('onUpdate()', function () {
+      it('should treat undefined itemIds argument as \'all\'', function () {
+        this.sandbox.stub(GroupStore, 'trigger');
+        GroupStore.onUpdate(1337, undefined, 'somedata');
 
-      expect(GroupStore.trigger.calledOnce).to.be.ok;
-      expect(GroupStore.trigger.firstCall.args[0]).to.eql(new Set([1,2,3]));
+        expect(GroupStore.trigger.calledOnce).to.be.ok;
+        expect(GroupStore.trigger.firstCall.args[0]).to.eql(new Set([1,2,3]));
+      });
     });
-  });
 
-  describe('onUpdateSuccess()', function () {
-    it('should treat undefined itemIds argument as \'all\'', function () {
-      GroupStore.items = [
-        {id: 1},
-        {id: 2},
-        {id: 3},
-      ];
+    describe('onUpdateSuccess()', function () {
+      it('should treat undefined itemIds argument as \'all\'', function () {
+        this.sandbox.stub(GroupStore, 'trigger');
+        GroupStore.onUpdateSuccess(1337, undefined, 'somedata');
+
+        expect(GroupStore.trigger.calledOnce).to.be.ok;
+        expect(GroupStore.trigger.firstCall.args[0]).to.eql(new Set([1,2,3]));
+      });
+    });
 
-      this.sandbox.stub(GroupStore, 'trigger');
-      GroupStore.onUpdateSuccess(1337, undefined, 'somedata');
+    describe('onUpdateError()', function () {
+      it('should treat undefined itemIds argument as \'all\'', function () {
+        this.sandbox.stub(GroupStore, 'trigger');
+        GroupStore.onUpdateError(1337, undefined, 'something failed', false);
 
-      expect(GroupStore.trigger.calledOnce).to.be.ok;
-      expect(GroupStore.trigger.firstCall.args[0]).to.eql(new Set([1,2,3]));
+        expect(GroupStore.trigger.calledOnce).to.be.ok;
+        expect(GroupStore.trigger.firstCall.args[0]).to.eql(new Set([1,2,3]));
+      });
     });
   });
 });