[SIP-4] replace SQL Lab ajax calls with SupersetClient (#5896)

* [superset-client] replace sqllab ajax calls with SupersetClient * [superset-client][sqllab] replace more misc ajax calls * [superset-client][tests] call setupSupersetClient() in test shim * [superset-client] replace more sqllab ajax calls and fix tests * [superset-client][tests] remove commented lines * [sqllab][superset-client] fix eslint and tests, add better error handling tests. * [superset-client] fix tests from rebase * [cypress][sqllab][superset-client] fix * [superset-client] use Promises not callbacks in getShortUrl calls * [superset-client][short-url] don't stringify POST * [superset-client][short-url][cypress] add data-test attribute for more reliable test * [cypress] remove .only() call
apache · Oct 18, 2018 · e163dfe · e163dfe
1 parent 273991f
commit e163dfe
Show file tree

Hide file tree

Showing 23 changed files with 880 additions and 757 deletions.
diff --git a/superset/assets/cypress/integration/explore/link.test.js b/superset/assets/cypress/integration/explore/link.test.js
@@ -26,19 +26,22 @@ describe('Test explore links', () => {
   });
 
   it('Visit short link', () => {
+    cy.route('POST', 'r/shortner/').as('getShortUrl');
+
     cy.visitChartByName('Growth Rate');
     cy.verifySliceSuccess({ waitAlias: '@getJson' });
 
     cy.get('[data-test=short-link-button]').click();
-    cy.get('#shorturl-popover').within(() => {
-      cy.get('i[title="Copy to clipboard"]')
-        .siblings()
-        .first()
-        .invoke('text')
-        .then((text) => {
-          cy.visit(text);
+
+    // explicitly wait for the url response
+    cy.wait('@getShortUrl');
+
+    cy.wait(100);
+
+    cy.get('#shorturl-popover [data-test="short-url"]').invoke('text')
+      .then((text) => {
+        cy.visit(text);
       });
-    });
     cy.verifySliceSuccess({ waitAlias: '@getJson' });
   });
 

diff --git a/superset/assets/cypress/integration/sqllab/query.js b/superset/assets/cypress/integration/sqllab/query.js
@@ -40,6 +40,7 @@ export default () => {
 
     it('successfully saves a query', () => {
       cy.route('savedqueryviewapi/**').as('getSavedQuery');
+      cy.route('superset/tables/**').as('getTables');
 
       const query = 'SELECT ds, gender, name, num FROM main.birth_names ORDER BY name LIMIT 3';
       const savedQueryTitle = `CYPRESS TEST QUERY ${shortid.generate()}`;
@@ -83,7 +84,7 @@ export default () => {
       cy.get('table tr:first-child a[href*="savedQueryId"').click();
 
       // will timeout without explicitly waiting here
-      cy.wait('@getSavedQuery');
+      cy.wait(['@getSavedQuery', '@getTables']);
 
       // run the saved query
       cy.get('#js-sql-toolbar button')

diff --git a/superset/assets/spec/helpers/shim.js b/superset/assets/spec/helpers/shim.js
@@ -5,6 +5,8 @@ import jsdom from 'jsdom';
 import { configure } from 'enzyme';
 import Adapter from 'enzyme-adapter-react-16';
 
+import setupSupersetClient from './setupSupersetClient';
+
 configure({ adapter: new Adapter() });
 
 const exposedProperties = ['window', 'navigator', 'document'];
@@ -45,3 +47,5 @@ global.window.XMLHttpRequest = global.XMLHttpRequest;
 global.window.location = { href: 'about:blank' };
 global.window.performance = { now: () => new Date().getTime() };
 global.$ = require('jquery')(global.window);
+
+setupSupersetClient();
diff --git a/superset/assets/spec/javascripts/explore/chartActions_spec.js b/superset/assets/spec/javascripts/explore/chartActions_spec.js
@@ -2,7 +2,6 @@ import fetchMock from 'fetch-mock';
 import sinon from 'sinon';
 
 import { Logger } from '../../../src/logger';
-import setupSupersetClient from '../../helpers/setupSupersetClient';
 import * as exploreUtils from '../../../src/explore/exploreUtils';
 import * as actions from '../../../src/chart/chartAction';
 
@@ -17,7 +16,6 @@ describe('chart actions', () => {
   };
 
   beforeAll(() => {
-    setupSupersetClient();
     setupDefaultFetchMock();
   });
 

diff --git a/superset/assets/spec/javascripts/explore/components/SaveModal_spec.jsx b/superset/assets/spec/javascripts/explore/components/SaveModal_spec.jsx
@@ -10,7 +10,6 @@ import fetchMock from 'fetch-mock';
 import * as exploreUtils from '../../../../src/explore/exploreUtils';
 import * as saveModalActions from '../../../../src/explore/actions/saveModalActions';
 import SaveModal from '../../../../src/explore/components/SaveModal';
-import setupSupersetClient from '../../../helpers/setupSupersetClient';
 
 describe('SaveModal', () => {
   const middlewares = [thunk];
@@ -182,7 +181,6 @@ describe('SaveModal', () => {
     const saveEndpoint = `glob:*/dashboardasync/api/read?_flt_0_owners=${1}`;
 
     beforeAll(() => {
-      setupSupersetClient();
       fetchMock.get(saveEndpoint, mockDashboardData);
     });
 

diff --git a/superset/assets/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx b/superset/assets/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx
@@ -4,8 +4,8 @@ import thunk from 'redux-thunk';
 
 import { shallow } from 'enzyme';
 import sinon from 'sinon';
+import fetchMock from 'fetch-mock';
 
-import $ from 'jquery';
 import shortid from 'shortid';
 import { queries, queryWithBadColumns } from './fixtures';
 import { sqlLabReducer } from '../../../src/SqlLab/reducers';
@@ -58,10 +58,10 @@ describe('ExploreResultsButton', () => {
     requiresTime: true,
     value: 'bar',
   };
-  const getExploreResultsButtonWrapper = (props = mockedProps) => (
+  const getExploreResultsButtonWrapper = (props = mockedProps) =>
     shallow(<ExploreResultsButton {...props} />, {
       context: { store },
-    }).dive());
+    }).dive();
 
   it('renders', () => {
     expect(React.isValidElement(<ExploreResultsButton />)).toBe(true);
@@ -151,64 +151,71 @@ describe('ExploreResultsButton', () => {
       datasourceName: 'mockDatasourceName',
     });
 
-    let ajaxSpy;
-    let datasourceSpy;
+    const visualizeURL = '/superset/sqllab_viz/';
+    const visualizeEndpoint = `glob:*${visualizeURL}`;
+    const visualizationPayload = { table_id: 107 };
+    fetchMock.post(visualizeEndpoint, visualizationPayload);
+
     beforeEach(() => {
-      ajaxSpy = sinon.spy($, 'ajax');
-      sinon.stub(JSON, 'parse').callsFake(() => ({ table_id: 107 }));
-      sinon.stub(exploreUtils, 'getExploreUrlAndPayload').callsFake(() => ({ url: 'mockURL', payload: { datasource: '107__table' } }));
+      sinon
+        .stub(exploreUtils, 'getExploreUrlAndPayload')
+        .callsFake(() => ({ url: 'mockURL', payload: { datasource: '107__table' } }));
       sinon.spy(exploreUtils, 'exportChart');
-      sinon.stub(wrapper.instance(), 'buildVizOptions').callsFake(() => (mockOptions));
-      datasourceSpy = sinon.stub(actions, 'createDatasource');
+      sinon.stub(wrapper.instance(), 'buildVizOptions').callsFake(() => mockOptions);
     });
     afterEach(() => {
-      ajaxSpy.restore();
-      JSON.parse.restore();
       exploreUtils.getExploreUrlAndPayload.restore();
       exploreUtils.exportChart.restore();
       wrapper.instance().buildVizOptions.restore();
-      datasourceSpy.restore();
+      fetchMock.reset();
     });
 
-    it('should build request', () => {
+    it('should build request with correct args', (done) => {
       wrapper.instance().visualize();
-      expect(ajaxSpy.callCount).toBe(1);
 
-      const spyCall = ajaxSpy.getCall(0);
-      expect(spyCall.args[0].type).toBe('POST');
-      expect(spyCall.args[0].url).toBe('/superset/sqllab_viz/');
-      expect(spyCall.args[0].data.data).toBe(JSON.stringify(mockOptions));
+      setTimeout(() => {
+        const calls = fetchMock.calls(visualizeEndpoint);
+        expect(calls).toHaveLength(1);
+        const formData = calls[0][1].body;
+
+        Object.keys(mockOptions).forEach((key) => {
+          // eslint-disable-next-line no-unused-expressions
+          expect(formData.get(key)).toBeDefined();
+        });
+
+        done();
+      });
     });
-    it('should open new window', () => {
+
+    it('should export chart and add an info toast', (done) => {
       const infoToastSpy = sinon.spy();
+      const datasourceSpy = sinon.stub();
 
-      datasourceSpy.callsFake(() => {
-        const d = $.Deferred();
-        d.resolve('done');
-        return d.promise();
-      });
+      datasourceSpy.callsFake(() => Promise.resolve(visualizationPayload));
 
       wrapper.setProps({
         actions: {
-          createDatasource: datasourceSpy,
           addInfoToast: infoToastSpy,
+          createDatasource: datasourceSpy,
         },
       });
 
       wrapper.instance().visualize();
-      expect(exploreUtils.exportChart.callCount).toBe(1);
-      expect(exploreUtils.exportChart.getCall(0).args[0].datasource).toBe('107__table');
-      expect(infoToastSpy.callCount).toBe(1);
-    });
-    it('should add error toast', () => {
-      const dangerToastSpy = sinon.spy();
 
-      datasourceSpy.callsFake(() => {
-        const d = $.Deferred();
-        d.reject('error message');
-        return d.promise();
+      setTimeout(() => {
+        expect(datasourceSpy.callCount).toBe(1);
+        expect(exploreUtils.exportChart.callCount).toBe(1);
+        expect(exploreUtils.exportChart.getCall(0).args[0].datasource).toBe('107__table');
+        expect(infoToastSpy.callCount).toBe(1);
+        done();
       });
+    });
 
+    it('should add error toast', (done) => {
+      const dangerToastSpy = sinon.stub(actions, 'addDangerToast');
+      const datasourceSpy = sinon.stub();
+
+      datasourceSpy.callsFake(() => Promise.reject({ error: 'error' }));
 
       wrapper.setProps({
         actions: {
@@ -218,8 +225,14 @@ describe('ExploreResultsButton', () => {
       });
 
       wrapper.instance().visualize();
-      expect(exploreUtils.exportChart.callCount).toBe(0);
-      expect(dangerToastSpy.callCount).toBe(1);
+
+      setTimeout(() => {
+        expect(datasourceSpy.callCount).toBe(1);
+        expect(exploreUtils.exportChart.callCount).toBe(0);
+        expect(dangerToastSpy.callCount).toBe(1);
+        dangerToastSpy.restore();
+        done();
+      });
     });
   });
 });