Merge pull request #218 from preset-io/ericbriscoe/sc-41493/query-pre…

…view-modal-for-chart-source-query

feat: Ericbriscoe/sc 41493/query preview modal for chart source query

superset-frontend/packages/superset-ui-core/src/query/types/Datasource.ts

@@ -54,4 +54,10 @@ export const DEFAULT_METRICS = [
   },
 ];
 
+export const isValidDatasourceType = (datasource: DatasourceType) =>
+  datasource === DatasourceType.Dataset ||
+  datasource === DatasourceType.SlTable ||
+  datasource === DatasourceType.SavedQuery ||
+  datasource === DatasourceType.Query;
+
 export default {};

superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx

@@ -211,19 +211,20 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
       return;
     }
 
-    const selectedColumns = query.results.selected_columns || [];
+    const selectedColumns = query?.results?.selected_columns ?? [];
 
     // The filters param is only used to test jinja templates.
     // Remove the special filters entry from the templateParams
     // before saving the dataset.
+    let templateParams;
     if (query.templateParams) {
       const p = JSON.parse(query.templateParams);
       /* eslint-disable-next-line no-underscore-dangle */
       if (p._filters) {
         /* eslint-disable-next-line no-underscore-dangle */
         delete p._filters;
         // eslint-disable-next-line no-param-reassign
-        query.templateParams = JSON.stringify(p);
+        templateParams = JSON.stringify(p);
       }
     }
 
@@ -232,7 +233,7 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
         schema: query.schema,
         sql: query.sql,
         dbId: query.dbId,
-        templateParams: query.templateParams,
+        templateParams,
         datasourceName: datasetName,
         columns: selectedColumns,
       }),

superset-frontend/src/components/Modal/Modal.tsx

@@ -246,7 +246,13 @@ const CustomModal = ({
   const draggableRef = useRef<HTMLDivElement>(null);
   const [bounds, setBounds] = useState<DraggableBounds>();
   const [dragDisabled, setDragDisabled] = useState<boolean>(true);
-  const modalFooter = isNil(footer)
+  let FooterComponent;
+  if (React.isValidElement(footer)) {
+    // If a footer component is provided inject a closeModal function
+    // so the footer can provide a "close" button if desired
+    FooterComponent = React.cloneElement(footer, { closeModal: onHide });
+  }
+  const modalFooter = isNil(FooterComponent)
     ? [
         <Button key="back" onClick={onHide} cta data-test="modal-cancel-button">
           {t('Cancel')}
@@ -263,7 +269,7 @@ const CustomModal = ({
           {primaryButtonName}
         </Button>,
       ]
-    : footer;
+    : FooterComponent;
 
   const modalWidth = width || (responsive ? '100vw' : '600px');
   const shouldShowMask = !(resizable || draggable);

superset-frontend/src/components/ModalTrigger/ModalTrigger.stories.tsx

@@ -20,11 +20,11 @@ import React from 'react';
 import ModalTrigger from '.';
 
 interface IModalTriggerProps {
-  triggerNode: React.ReactNode;
+  triggerNode: JSX.Element;
   dialogClassName?: string;
-  modalTitle?: React.ReactNode;
-  modalBody?: React.ReactNode;
-  modalFooter?: React.ReactNode;
+  modalTitle?: string;
+  modalBody?: JSX.Element;
+  modalFooter?: JSX.Element;
   beforeOpen?: () => void;
   onExit?: () => void;
   isButton?: boolean;

superset-frontend/src/components/ModalTrigger/index.tsx

@@ -0,0 +1,112 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { useState, useEffect } from 'react';
+import Modal from 'src/components/Modal';
+import Button from 'src/components/Button';
+
+interface ModalTriggerProps {
+  dialogClassName?: string;
+  triggerNode: JSX.Element;
+  modalTitle?: string;
+  modalBody?: JSX.Element; // not required because it can be generated by beforeOpen
+  modalFooter?: JSX.Element;
+  beforeOpen?: Function;
+  isButton?: boolean;
+  className?: string;
+  tooltip?: string;
+  width?: string;
+  maxWidth?: string;
+  responsive?: boolean;
+  resizable?: boolean;
+  resizableConfig?: any;
+  draggable?: boolean;
+  draggableConfig?: any;
+  destroyOnClose?: boolean;
+}
+
+function ModalTrigger({
+  beforeOpen = () => {},
+  isButton = false,
+  resizable = false,
+  draggable = false,
+  className = '',
+  tooltip,
+  modalFooter,
+  triggerNode,
+  destroyOnClose = true,
+  modalBody,
+  draggableConfig = {},
+  resizableConfig = {},
+  modalTitle,
+  responsive,
+  width,
+  maxWidth,
+}: ModalTriggerProps) {
+  const [showModal, setShowModal] = useState(false);
+
+  const close = () => {
+    setShowModal(false);
+  };
+
+  const open = (e: React.MouseEvent) => {
+    e.preventDefault();
+    beforeOpen?.();
+    setShowModal(true);
+  };
+  /* eslint-disable jsx-a11y/interactive-supports-focus */
+  return (
+    <>
+      {isButton && (
+        <Button
+          className="modal-trigger"
+          data-test="btn-modal-trigger"
+          tooltip={tooltip}
+          onClick={open}
+        >
+          {triggerNode}
+        </Button>
+      )}
+      {!isButton && (
+        <span data-test="span-modal-trigger" onClick={open} role="button">
+          {triggerNode}
+        </span>
+      )}
+      <Modal
+        className={className}
+        show={showModal}
+        onHide={close}
+        title={modalTitle}
+        footer={modalFooter}
+        hideFooter={!modalFooter}
+        width={width}
+        maxWidth={maxWidth}
+        responsive={responsive}
+        resizable={resizable}
+        resizableConfig={resizableConfig}
+        draggable={draggable}
+        draggableConfig={draggableConfig}
+        destroyOnClose={destroyOnClose}
+      >
+        {modalBody}
+      </Modal>
+    </>
+  );
+}
+
+export default ModalTrigger;

superset-frontend/src/explore/components/DatasourcePanel/index.tsx

@@ -307,6 +307,8 @@ export default function DataSourcePanel({
     return true;
   };
 
+  const dataSourceIsQuery = datasource?.type === DatasourceType.Query;
+
   const mainBody = useMemo(
     () => (
       <>
@@ -321,7 +323,7 @@ export default function DataSourcePanel({
           placeholder={t('Search Metrics & Columns')}
         />
         <div className="field-selections">
-          {datasource.type === DatasourceType.Query && showInfoboxCheck() && (
+          {dataSourceIsQuery && showInfoboxCheck() && (
             <StyledInfoboxWrapper>
               <Alert
                 closable
@@ -434,19 +436,22 @@ export default function DataSourcePanel({
       search,
       showAllColumns,
       showAllMetrics,
+      dataSourceIsQuery,
       shouldForceUpdate,
     ],
   );
 
   return (
     <DatasourceContainer>
-      <SaveDatasetModal
-        visible={showSaveDatasetModal}
-        onHide={() => setShowSaveDatasetModal(false)}
-        buttonTextOnSave={t('Save')}
-        buttonTextOnOverwrite={t('Overwrite')}
-        datasource={datasource}
-      />
+      {dataSourceIsQuery && (
+        <SaveDatasetModal
+          visible={showSaveDatasetModal}
+          onHide={() => setShowSaveDatasetModal(false)}
+          buttonTextOnSave={t('Save')}
+          buttonTextOnOverwrite={t('Overwrite')}
+          datasource={datasource}
+        />
+      )}
       <Control {...datasourceControl} name="datasource" actions={actions} />
       {datasource.id != null && mainBody}
     </DatasourceContainer>