Merge pull request #2607 from getsentry/fix-merge-again

Fix bulk merge (#2585)

Author: benvinegar

package.json

@@ -61,7 +61,7 @@
     "eslint-plugin-react": "3.11.2",
     "gettext-parser": "1.1.1",
     "js-cookie": "2.0.4",
-    "karma": "0.13.15",
+    "karma": "0.13.19",
     "karma-chai": "0.1.0",
     "karma-mocha": "0.1.10",
     "karma-phantomjs-launcher": "0.1.4",

src/sentry/static/sentry/app/api.jsx

@@ -14,6 +14,18 @@ export class Request {
   }
 }
 
+/**
+ * Converts input parameters to API-compatible query arguments
+ * @param params
+ */
+export function paramsToQueryArgs(params) {
+  return params.itemIds
+      ? {id: params.itemIds}    // items matching array of itemids
+      : params.query
+        ? {query: params.query} // items matching search query
+        : undefined;            // all items
+}
+
 export class Client {
   constructor(options) {
     if (typeof options === 'undefined') {
@@ -119,7 +131,7 @@ export class Client {
 
   bulkDelete(params, options) {
     let path = '/projects/' + params.orgId + '/' + params.projectId + '/issues/';
-    let query = (params.itemIds ? {id: params.itemIds} : undefined);
+    let query = paramsToQueryArgs(params);
     let id = this.uniqueId();
 
     GroupActions.delete(id, params.itemIds);
@@ -138,14 +150,7 @@ export class Client {
 
   bulkUpdate(params, options) {
     let path = '/projects/' + params.orgId + '/' + params.projectId + '/issues/';
-
-    let query =
-      params.itemIds
-        ? {id: params.itemIds}
-        : params.query
-          ? {query: params.query}
-          : undefined;
-
+    let query = paramsToQueryArgs(params);
     let id = this.uniqueId();
 
     GroupActions.update(id, params.itemIds, params.data);
@@ -165,7 +170,7 @@ export class Client {
 
   merge(params, options) {
     let path = '/projects/' + params.orgId + '/' + params.projectId + '/issues/';
-    let query = (params.itemIds ? {id: params.itemIds} : undefined);
+    let query = paramsToQueryArgs(params);
     let id = this.uniqueId();
 
     GroupActions.merge(id, params.itemIds);

tests/js/spec/api.spec.jsx

@@ -1,6 +1,5 @@
 import $ from 'jquery';
-import {Client, Request} from 'app/api';
-import GroupActions from 'app/actions/groupActions';
+import {Client, Request, paramsToQueryArgs} from 'app/api';
 
 describe('api', function () {
   beforeEach(function () {
@@ -9,6 +8,30 @@ describe('api', function () {
     this.api = new Client();
   });
 
+
+  describe('paramsToQueryArgs()', function () {
+    it('should convert itemIds properties to id array', function () {
+      expect(paramsToQueryArgs({
+        itemIds: [1, 2, 3],
+        query: 'is:unresolved' // itemIds takes precedence
+      })).to.eql({id: [1, 2, 3]});
+    });
+
+    it('should extract query property if no itemIds', function () {
+      expect(paramsToQueryArgs({
+        query: 'is:unresolved',
+        foo: 'bar'
+      })).to.eql({query: 'is:unresolved'});
+    });
+
+    it('should convert params w/o itemIds or query to undefined', function () {
+      expect(paramsToQueryArgs({
+        foo: 'bar',
+        bar: 'baz' // paramsToQueryArgs ignores these
+      })).to.be.undefined;
+    });
+  });
+
   describe('Client', function () {
     beforeEach(function () {
       this.sandbox.stub($, 'ajax');
@@ -56,8 +79,6 @@ describe('api', function () {
     });
 
     it('should use query as query if itemIds are absent', function () {
-      this.sandbox.stub(GroupActions, 'update');
-
       this.api.bulkUpdate({
         orgId: '1337',
         projectId: '1337',
@@ -71,4 +92,40 @@ describe('api', function () {
       expect(requestArgs.query).to.eql({query: 'is:resolved'});
     });
   });
+
+  describe('merge()', function () {
+    // TODO: this is totally copypasta from the test above. We need to refactor
+    //       these API methods/tests.
+    beforeEach(function () {
+      this.sandbox.stub(this.api, '_wrapRequest');
+    });
+
+    it('should use itemIds as query if provided', function () {
+      this.api.merge({
+        orgId: '1337',
+        projectId: '1337',
+        itemIds: [1,2,3],
+        data: {status: 'unresolved'},
+        query: 'is:resolved'
+      });
+
+      expect(this.api._wrapRequest.calledOnce).to.be.ok;
+      let requestArgs = this.api._wrapRequest.getCall(0).args[1];
+      expect(requestArgs.query).to.eql({id: [1, 2, 3]});
+    });
+
+    it('should use query as query if itemIds are absent', function () {
+      this.api.merge({
+        orgId: '1337',
+        projectId: '1337',
+        itemIds: null,
+        data: {status: 'unresolved'},
+        query: 'is:resolved'
+      });
+
+      expect(this.api._wrapRequest.calledOnce).to.be.ok;
+      let requestArgs = this.api._wrapRequest.getCall(0).args[1];
+      expect(requestArgs.query).to.eql({query: 'is:resolved'});
+    });
+  });
 });