Migrate Query pages to React: fix tests

client/app/components/Parameters.jsx

@@ -51,11 +51,13 @@ export class Parameters extends React.Component {
 
   componentDidUpdate = prevProps => {
     const { parameters, disableUrlUpdate } = this.props;
-    if (prevProps.parameters !== parameters) {
+    const parametersChanged = prevProps.parameters !== parameters;
+    const disableUrlUpdateChanged = prevProps.disableUrlUpdate !== disableUrlUpdate;
+    if (parametersChanged) {
       this.setState({ parameters });
-      if (!disableUrlUpdate) {
-        updateUrl(parameters);
-      }
+    }
+    if ((parametersChanged || disableUrlUpdateChanged) && !disableUrlUpdate) {
+      updateUrl(parameters);
     }
   };
 

client/app/pages/queries/QuerySource.jsx

@@ -1,5 +1,5 @@
 import { isEmpty, find, map, extend, includes } from "lodash";
-import React, { useState, useRef, useEffect, useMemo, useCallback } from "react";
+import React, { useState, useRef, useEffect, useCallback } from "react";
 import PropTypes from "prop-types";
 import { react2angular } from "react2angular";
 import { useDebouncedCallback } from "use-debounce";
@@ -149,24 +149,18 @@ function QuerySource(props) {
     }
   }, []);
 
-  const canExecuteQuery = useMemo(() => queryFlags.canExecute && !isQueryExecuting && !areParametersDirty, [
-    isQueryExecuting,
-    areParametersDirty,
-    queryFlags.canExecute,
-  ]);
-
   const [selectedText, setSelectedText] = useState(null);
 
   const doExecuteQuery = useCallback(() => {
-    if (!canExecuteQuery) {
+    if (!queryFlags.canExecute || isQueryExecuting) {
       return;
     }
     if (isDirty || !isEmpty(selectedText)) {
       executeAdhocQuery(selectedText);
     } else {
       executeQuery();
     }
-  }, [canExecuteQuery, isDirty, selectedText, executeQuery, executeAdhocQuery]);
+  }, [queryFlags.canExecute, isQueryExecuting, isDirty, selectedText, executeQuery, executeAdhocQuery]);
 
   return (
     <div className="query-page-wrapper">
@@ -183,24 +177,31 @@ function QuerySource(props) {
       <main className="query-fullscreen">
         <Resizable direction="horizontal" sizeAttribute="flex-basis" toggleShortcut="Alt+Shift+D, Alt+D">
           <nav>
-            <div className="editor__left__data-source">
-              <Select
-                className="w-100"
-                placeholder="Choose data source..."
-                value={dataSource ? dataSource.id : undefined}
-                disabled={!queryFlags.canEdit || !dataSourcesLoaded || dataSources.length === 0}
-                loading={!dataSourcesLoaded}
-                optionFilterProp="data-name"
-                showSearch
-                onChange={handleDataSourceChange}>
-                {map(dataSources, ds => (
-                  <Select.Option key={`ds-${ds.id}`} value={ds.id} data-name={ds.name}>
-                    <img src={`/static/images/db-logos/${ds.type}.png`} width="20" alt={ds.name} />
-                    <span>{ds.name}</span>
-                  </Select.Option>
-                ))}
-              </Select>
-            </div>
+            {dataSourcesLoaded && (
+              <div className="editor__left__data-source">
+                <Select
+                  className="w-100"
+                  data-test="SelectDataSource"
+                  placeholder="Choose data source..."
+                  value={dataSource ? dataSource.id : undefined}
+                  disabled={!queryFlags.canEdit || !dataSourcesLoaded || dataSources.length === 0}
+                  loading={!dataSourcesLoaded}
+                  optionFilterProp="data-name"
+                  showSearch
+                  onChange={handleDataSourceChange}>
+                  {map(dataSources, ds => (
+                    <Select.Option
+                      key={`ds-${ds.id}`}
+                      value={ds.id}
+                      data-name={ds.name}
+                      data-test={`SelectDataSource${ds.id}`}>
+                      <img src={`/static/images/db-logos/${ds.type}.png`} width="20" alt={ds.name} />
+                      <span>{ds.name}</span>
+                    </Select.Option>
+                  ))}
+                </Select>
+              </div>
+            )}
             <div className="editor__left__schema">
               <SchemaBrowser
                 schema={schema}
@@ -270,7 +271,7 @@ function QuerySource(props) {
                         }
                       }
                       executeButtonProps={{
-                        disabled: !canExecuteQuery,
+                        disabled: !queryFlags.canExecute || isQueryExecuting || areParametersDirty,
                         shortcut: "mod+enter, alt+enter",
                         onClick: doExecuteQuery,
                         text: (

client/app/pages/queries/components/QueryViewExecuteButton.jsx

@@ -37,7 +37,12 @@ export default function QueryViewExecuteButton({ shortcut, disabled, children, o
   return (
     <Tooltip placement="top" title={humanReadableShortcut(shortcut, 1)} visible={tooltipVisible}>
       <span {...eventHandlers}>
-        <Button type="primary" disabled={disabled} onClick={onClick} style={disabled ? { pointerEvents: "none" } : {}}>
+        <Button
+          data-test="ExecuteButton"
+          type="primary"
+          disabled={disabled}
+          onClick={onClick}
+          style={disabled ? { pointerEvents: "none" } : {}}>
           {children}
         </Button>
       </span>

client/app/pages/queries/components/QueryVisualizationTabs.jsx

@@ -11,7 +11,7 @@ import "./QueryVisualizationTabs.less";
 
 const { TabPane } = Tabs;
 
-function TabWithDeleteButton({ visualizationName, canDelete, onDelete }) {
+function TabWithDeleteButton({ visualizationName, canDelete, onDelete, ...props }) {
   const handleDelete = useCallback(
     e => {
       e.stopPropagation();
@@ -29,14 +29,14 @@ function TabWithDeleteButton({ visualizationName, canDelete, onDelete }) {
   );
 
   return (
-    <>
+    <span {...props}>
       {visualizationName}
       {canDelete && (
         <a className="delete-visualization-button" onClick={handleDelete}>
           <i className="zmdi zmdi-close" />
         </a>
       )}
-    </>
+    </span>
   );
 }
 
@@ -78,7 +78,7 @@ export default function QueryVisualizationTabs({
 
   if (showNewVisualizationButton) {
     tabsProps.tabBarExtraContent = (
-      <Button onClick={() => onAddVisualization()}>
+      <Button data-test="NewVisualization" onClick={() => onAddVisualization()}>
         <i className="fa fa-plus" />
         <span className="m-l-5 hidden-xs">New Visualization</span>
       </Button>
@@ -93,15 +93,18 @@ export default function QueryVisualizationTabs({
     <Tabs
       {...tabsProps}
       className="query-visualization-tabs"
+      data-test="QueryPageVisualizationTabs"
       animated={false}
       tabBarGutter={0}
       onChange={activeKey => onChangeTab(+activeKey)}
       destroyInactiveTabPane>
       {orderedVisualizations.map(visualization => (
         <TabPane
           key={`${visualization.id}`}
+          data-test={`QueryPageVisualization${selectedTab}`}
           tab={
             <TabWithDeleteButton
+              data-test={`QueryPageVisualizationTab${visualization.id}`}
               canDelete={!isMobile && canDeleteVisualizations && !isFirstVisualization(visualization.id)}
               visualizationName={visualization.name}
               onDelete={() => onDeleteVisualization(visualization.id)}

client/app/pages/queries/hooks/useQuery.js

@@ -7,12 +7,13 @@ export default function useQuery(originalQuery) {
   const [originalQuerySource, setOriginalQuerySource] = useState(originalQuery.query);
 
   const updateQuery = useUpdateQuery(query, updatedQuery => {
-    setQuery(updatedQuery);
-    setOriginalQuerySource(updatedQuery.query);
+    // It's important to update URL first, and only then update state
     if (updatedQuery.id !== query.id) {
       // Don't reload page when saving new query
       navigateTo(updatedQuery.getSourceLink(), true, false);
     }
+    setQuery(updatedQuery);
+    setOriginalQuerySource(updatedQuery.query);
   });
 
   return useMemo(

client/cypress/integration/embed/share_embed_spec.js

@@ -32,6 +32,8 @@ describe("Embedded Queries", () => {
     cy.getByTestId("QueryEditor")
       .get(".ace_text-input")
       .type("SELECT name, slug FROM organizations WHERE id='{{}{{}id}}'{esc}", { force: true });
+    // QueryEditor::onChange is debounced, so this wait is needed
+    cy.wait(500); // eslint-disable-line cypress/no-unnecessary-waiting
 
     cy.getByTestId("TextParamInput").type("1");
     cy.getByTestId("ParameterApplyButton").click();
@@ -66,6 +68,8 @@ describe("Embedded Queries", () => {
     cy.getByTestId("QueryEditor")
       .get(".ace_text-input")
       .type("SELECT name, slug FROM organizations WHERE name='{{}{{}name}}'{esc}", { force: true });
+    // QueryEditor::onChange is debounced, so this wait is needed
+    cy.wait(500); // eslint-disable-line cypress/no-unnecessary-waiting
 
     cy.getByTestId("TextParamInput").type("Redash");
     cy.getByTestId("ParameterApplyButton").click();

client/cypress/integration/query/create_query_spec.js

@@ -5,15 +5,18 @@ describe('Create Query', () => {
   });
 
   it('executes and saves a query', () => {
-    cy.getByTestId('SelectDataSource')
-      .click()
-      .contains('Test PostgreSQL').click();
+    cy.clickThrough(`
+      SelectDataSource
+      SelectDataSource1
+    `);
 
     cy.getByTestId('QueryEditor')
       .get('.ace_text-input')
       .type('SELECT id, name FROM organizations{esc}', { force: true });
+    // QueryEditor::onChange is debounced, so this wait is needed
+    cy.wait(500); // eslint-disable-line cypress/no-unnecessary-waiting
 
-    cy.getByTestId('ExecuteButton').click();
+    cy.getByTestId('ExecuteButton').should('be.enabled').click();
 
     cy.getByTestId('TableVisualization').should('exist');
     cy.percySnapshot('Edit Query');

client/cypress/integration/query/parameter_spec.js

@@ -566,7 +566,7 @@ describe('Parameter', () => {
       cy.getByTestId('ExecuteButton').should('be.disabled');
 
       cy.get('@Input').clear();
-      cy.getByTestId('ExecuteButton').should('not.be.disabled');
+      cy.getByTestId('ExecuteButton').should('be.enabled');
     });
   });
 
@@ -592,6 +592,8 @@ describe('Parameter', () => {
         .first()
         .invoke('width')
         .as('paramWidth');
+
+      cy.get('body').type('{alt}D'); // hide schema browser
     });
 
     const dragParam = (paramName, offsetLeft, offsetTop) => {

client/cypress/integration/visualizations/edit_visualization_dialog_spec.js

@@ -39,6 +39,6 @@ describe('Edit visualization dialog', () => {
     cy.getByTestId('VisualizationName').clear().type(visualizationName);
 
     cy.getByTestId('EditVisualizationDialog').contains('button', 'Save').click();
-    cy.getByTestId('QueryPageVisualizationTabs').contains('li', visualizationName).should('exist');
+    cy.getByTestId('QueryPageVisualizationTabs').contains('span', visualizationName).should('exist');
   });
 });

client/cypress/integration/visualizations/pivot_spec.js

@@ -50,7 +50,7 @@ describe('Pivot', () => {
     const visualizationName = 'Pivot';
     createPivotThroughUI(visualizationName);
 
-    cy.getByTestId('QueryPageVisualizationTabs').contains('li', visualizationName).should('exist');
+    cy.getByTestId('QueryPageVisualizationTabs').contains('span', visualizationName).should('exist');
   });
 
   it('creates Pivot without controls', function () {
@@ -98,6 +98,9 @@ describe('Pivot', () => {
           .first()
           .focus()
           .type(" UNION ALL {enter}SELECT 'c' AS stage1, 'c5' AS stage2, 55 AS value");
+        // QueryEditor::onChange is debounced, so this wait is needed
+        cy.wait(500); // eslint-disable-line cypress/no-unnecessary-waiting
+
         cy.getByTestId('SaveButton').click();
         cy.getByTestId('ExecuteButton').click();
 

client/cypress/integration/visualizations/sankey_sunburst_spec.js

@@ -41,7 +41,7 @@ describe('Sankey and Sunburst', () => {
     cy.percySnapshot('Visualizations - Sunburst', { widths: [viewportWidth] });
 
     cy.getByTestId('EditVisualizationDialog').contains('button', 'Save').click();
-    cy.getByTestId('QueryPageVisualizationTabs').contains('li', visualizationName).should('exist');
+    cy.getByTestId('QueryPageVisualizationTabs').contains('span', visualizationName).should('exist');
   });
 
   it('creates Sankey', () => {
@@ -58,6 +58,6 @@ describe('Sankey and Sunburst', () => {
     cy.percySnapshot('Visualizations - Sankey', { widths: [viewportWidth] });
 
     cy.getByTestId('EditVisualizationDialog').contains('button', 'Save').click();
-    cy.getByTestId('QueryPageVisualizationTabs').contains('li', visualizationName).should('exist');
+    cy.getByTestId('QueryPageVisualizationTabs').contains('span', visualizationName).should('exist');
   });
 });

client/cypress/integration/visualizations/table/table_spec.js

@@ -22,8 +22,7 @@ function prepareVisualization(query, type, name, options) {
       cy.get('body').type('{alt}D');
 
       // do some pre-checks here to ensure that visualization was created and is visible
-      cy.getByTestId('QueryPageVisualizationTabs').find(`li[tab-id=${visualizationId}]`)
-        .should('exist').click();
+      cy.getByTestId(`QueryPageVisualizationTab${visualizationId}`).should('exist').click();
       cy.getByTestId(`QueryPageVisualization${visualizationId}`).should('exist')
         .find('table').should('exist');