You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/04/26 20:50:41 UTC

[GitHub] [superset] lyndsiWilliams opened a new pull request, #19855: feat(explore): Frontend implementation of dataset creation from infobox

lyndsiWilliams opened a new pull request, #19855:
URL: https://github.com/apache/superset/pull/19855

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   This is the front end implementation of dataset creation from an infobox in a chart.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   #### BEFORE:
   ![cdsfibBefore](https://user-images.githubusercontent.com/55605634/165389134-1ec551e2-50ab-4dba-95e7-cfcd7d1015ca.png)
   
   #### AFTER:
   ![cdsfibAfter](https://user-images.githubusercontent.com/55605634/165389165-34cdf971-c78c-411a-9c10-312a7fc8a21c.png)
   
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   - Open a chart and observe the new infobox on the left panel
   - Click "Create a dataset" and observe the modal
   - Close the modal and close the infobox
   - Refresh the page and observe that the infobox does not return
   - Close the browser tab containing this instance of Superset and reopen a new one
   - Observe that the infobox has returned on a new session
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] jinghua-qa commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1134996869

   Tested in Ephemera env, verified regression test in dataset menu in explore and save dataset modal in sql lab. LGTM.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] AAfghahi commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r877246876


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +301,22 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {
+    if (sessionStorage.getItem('showInfobox') === 'false') return false;
+    return true;
+  };
+
+  const datasourceTypeCheck = () => {

Review Comment:
   We could rewrite as: 
   `const datasourceTypeCheck = datasource.type === DatasourceType.Dataset. . . .,etc` 
   
   and on checking it would return true or false. That way you don't have to write the if statement at all. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r891521694


##########
superset-frontend/packages/superset-ui-chart-controls/test/utils/defineSavedMetrics.test.tsx:
##########
@@ -0,0 +1,60 @@
+/**
+ * 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 {
+  DatasourceType,
+  DEFAULT_METRICS,
+  QueryResponse,
+  testQuery,
+} from '@superset-ui/core';
+import { defineSavedMetrics } from '@superset-ui/chart-controls';
+
+describe('defineSavedMetrics', () => {
+  it('defines saved metrics if source is a Dataset', () => {
+    expect(
+      defineSavedMetrics({
+        id: 1,
+        metrics: [
+          {
+            metric_name: 'COUNT(*) non-default-dataset-metric',
+            expression: 'COUNT(*) non-default-dataset-metric',
+          },
+        ],
+        type: DatasourceType.Table,

Review Comment:
   wouldn't this be Query?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] jess-dillard commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
jess-dillard commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1111204197

   Looks good to me (assuming the width issue is on the ephemeral – your screenshot looks good).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110424664

   /testenv up


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r864291834


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   bring this into the component:
   ```
   type ExploreDatasource =
       Dataset |
       Query 
   ```
   Rename `DatasourceMeta` to Dataset



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889420801


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -99,8 +114,8 @@ export const dnd_adhoc_metrics: SharedControlConfig<'DndMetricSelect'> = {
   label: t('Metrics'),
   validators: [validateNonEmpty],
   mapStateToProps: ({ datasource }) => ({
-    columns: datasource ? datasource.columns : [],
-    savedMetrics: datasource ? datasource.metrics : [],
+    columns: datasource?.columns || [],
+    savedMetrics: savedMetricsTypeCheck(datasource),

Review Comment:
   nice



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889227952


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -36,16 +41,17 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
   ),
   mapStateToProps(state, { includeTime }) {
     const newState: ExtraControlProps = {};
-    if (state.datasource) {
-      const options = state.datasource.columns.filter(c => c.groupby);
+    const { datasource } = state;
+    if (datasource?.columns[0]?.hasOwnProperty('groupby')) {
+      const options = (datasource as Dataset).columns.filter(c => c.groupby);
       if (includeTime) {
         options.unshift(TIME_COLUMN_OPTION);
       }
       newState.options = Object.fromEntries(
         options.map(option => [option.column_name, option]),
       );
-      newState.savedMetrics = state.datasource.metrics || [];
-    }
+      newState.savedMetrics = (datasource as Dataset).metrics || [];
+    } else newState.options = datasource?.columns;

Review Comment:
   Done in [`this commit`](https://github.com/apache/superset/pull/19855/commits/ccd3e3e800b99db12bb2117793a77a243c60240d) 😁 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889423432


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx:
##########
@@ -131,13 +135,14 @@ const groupByControl: SharedControlConfig<'SelectControl', ColumnMeta> = {
   promptTextCreator: (label: unknown) => label,
   mapStateToProps(state, { includeTime }) {
     const newState: ExtraControlProps = {};
-    if (state.datasource) {
-      const options = state.datasource.columns.filter(c => c.groupby);
+    const { datasource } = state;
+    if (datasource?.columns[0]?.hasOwnProperty('groupby')) {
+      const options = (datasource as Dataset).columns.filter(c => c.groupby);
       if (includeTime) {
-        options.unshift(TIME_COLUMN_OPTION);
+        options.unshift(DATASET_TIME_COLUMN_OPTION);
       }
       newState.options = options;
-    }
+    } else newState.options = datasource?.columns;

Review Comment:
   we should still do lines 141 and 142 if this is a query. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r872886242


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,153 +17,362 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+  // DatasourceType,
+} from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import moment from 'moment';
+import rison from 'rison';
+import { createDatasource } from 'src/SqlLab/actions/sqlLab';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+  SqlLabExploreRootState,
+  getInitialState,
+} from 'src/SqlLab/types';
+// import { Dataset } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
+
+// type ExploreDatasource = Dataset | Query;
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource: Query;

Review Comment:
   this can be a query or dataset?



##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,153 +17,362 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+  // DatasourceType,
+} from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import moment from 'moment';
+import rison from 'rison';
+import { createDatasource } from 'src/SqlLab/actions/sqlLab';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+  SqlLabExploreRootState,
+  getInitialState,
+} from 'src/SqlLab/types';
+// import { Dataset } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
+
+// type ExploreDatasource = Dataset | Query;
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource: Query;
 }
 
 const Styles = styled.div`
-  .smd-body {
+  .sdm-body {
     margin: 0 8px;
   }
-  .smd-input {
+  .sdm-input {
     margin-left: 45px;
     width: 401px;
   }
-  .smd-autocomplete {
+  .sdm-autocomplete {
     margin-left: 8px;
     width: 401px;
   }
-  .smd-radio {
+  .sdm-radio {
     display: block;
     height: 30px;
     margin: 10px 0px;
     line-height: 30px;
   }
-  .smd-overwrite-msg {
+  .sdm-overwrite-msg {
     margin: 7px;
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
+// const getDatasourceState = (datasource: ExploreDatasource) => {
+//   if (datasource.type === DatasourceType.Dataset) return datasource as Dataset;
+//   return datasource as Query;
+// };
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,

Review Comment:
   for consistency, can we also call this dataset or does this also include a query?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] hughhhh commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r876399979


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -493,48 +221,26 @@ export default class ResultSet extends React.PureComponent<
       }
       const { columns } = this.props.query.results;
       // Added compute logic to stop user from being able to Save & Explore
-      const {
-        saveDatasetRadioBtnState,
-        newSaveDatasetName,
-        datasetToOverwrite,
-        saveModalAutocompleteValue,
-        shouldOverwriteDataSet,
-        userDatasetOptions,
-        showSaveDatasetModal,
-      } = this.state;
-      const disableSaveAndExploreBtn =
-        (saveDatasetRadioBtnState === DatasetRadioState.SAVE_NEW &&
-          newSaveDatasetName.length === 0) ||
-        (saveDatasetRadioBtnState === DatasetRadioState.OVERWRITE_DATASET &&
-          Object.keys(datasetToOverwrite).length === 0 &&
-          saveModalAutocompleteValue.length === 0);
+      const { showSaveDatasetModal } = this.state;
 
       return (
         <ResultSetControls>
           <SaveDatasetModal
             visible={showSaveDatasetModal}
-            onOk={this.handleSaveInDataset}
-            saveDatasetRadioBtnState={saveDatasetRadioBtnState}
-            shouldOverwriteDataset={shouldOverwriteDataSet}
-            defaultCreateDatasetValue={newSaveDatasetName}
-            userDatasetOptions={userDatasetOptions}
-            disableSaveAndExploreBtn={disableSaveAndExploreBtn}
-            onHide={this.handleHideSaveModal}
-            handleDatasetNameChange={this.handleDatasetNameChange}
-            handleSaveDatasetRadioBtnState={this.handleSaveDatasetRadioBtnState}
-            handleOverwriteCancel={this.handleOverwriteCancel}
-            handleOverwriteDataset={this.handleOverwriteDataset}
-            handleOverwriteDatasetOption={this.handleOverwriteDatasetOption}
-            handleSaveDatasetModalSearch={this.handleSaveDatasetModalSearch}
-            filterAutocompleteOption={this.handleFilterAutocompleteOption}
-            onChangeAutoComplete={this.handleOnChangeAutoComplete}
+            onHide={() => this.setState({ showSaveDatasetModal: false })}
+            buttonTextOnSave={t('Save & Explore')}
+            buttonTextOnOverwrite={t('Overwrite & Explore')}
+            modalDescription={t(
+              'Save this query as a virtual dataset to continue exploring',
+            )}
+            datasource={this.props.query}

Review Comment:
   nit: deconstruct `query` then pass it in



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110380709

   @lyndsiWilliams Ephemeral environment spinning up at http://50.112.74.36:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r876973672


##########
superset-frontend/src/SqlLab/types.ts:
##########
@@ -128,3 +130,70 @@ export type RootState = {
   messageToasts: toastState[];
   common: {};
 };
+
+export type ExploreRootState = {

Review Comment:
   It would! Moved the type in [`this commit`](https://github.com/apache/superset/pull/19855/commits/ab8510e54db95c1c3cd640d6c47496bfb88b57bb).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r877281068


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +301,22 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {
+    if (sessionStorage.getItem('showInfobox') === 'false') return false;
+    return true;
+  };
+
+  const datasourceTypeCheck = () => {

Review Comment:
   That's even better! Changed in [`this commit`](https://github.com/apache/superset/pull/19855/commits/9cc44abcf0e89874d2094ac1fe7b39a912b4ae2c).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] AAfghahi commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r878451475


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +301,17 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {

Review Comment:
   oh ok, that makes sense! 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r883162120


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -82,14 +83,18 @@ export const dnd_adhoc_filters: SharedControlConfig<'DndFilterSelect'> = {
   label: t('Filters'),
   default: [],
   description: '',
-  mapStateToProps: ({ datasource, form_data }) => ({
-    columns: datasource?.columns.filter(c => c.filterable) || [],
-    savedMetrics: datasource?.metrics || [],
-    // current active adhoc metrics
-    selectedMetrics:
-      form_data.metrics || (form_data.metric ? [form_data.metric] : []),
-    datasource,
-  }),
+  mapStateToProps: ({ datasource, form_data }) => {
+    const dataset = datasource as Dataset;
+
+    return {
+      columns: dataset?.columns.filter(c => c.filterable) || [],
+      savedMetrics: dataset?.metrics || [],

Review Comment:
   ```suggestion
         savedMetrics: datasource.hasOwnProperty("metrics") && datasource.metrics ? datasource.metrics || [],
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r883159520


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -82,14 +83,18 @@ export const dnd_adhoc_filters: SharedControlConfig<'DndFilterSelect'> = {
   label: t('Filters'),
   default: [],
   description: '',
-  mapStateToProps: ({ datasource, form_data }) => ({
-    columns: datasource?.columns.filter(c => c.filterable) || [],
-    savedMetrics: datasource?.metrics || [],
-    // current active adhoc metrics
-    selectedMetrics:
-      form_data.metrics || (form_data.metric ? [form_data.metric] : []),
-    datasource,
-  }),
+  mapStateToProps: ({ datasource, form_data }) => {
+    const dataset = datasource as Dataset;

Review Comment:
   ```suggestion
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r878387165


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +301,17 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {

Review Comment:
   I tried adjusting this and learned that it won't work, I need `sessionStorage.getItem('showInfobox')` to run each time this renders to check if the user has closed the infobox for this session.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r879957459


##########
superset-frontend/packages/superset-ui-core/src/query/types/Query.ts:
##########
@@ -165,4 +165,154 @@ export interface QueryContext {
   form_data?: QueryFormData;
 }
 
+export const ErrorTypeEnum = {
+  // Frontend errors
+  FRONTEND_CSRF_ERROR: 'FRONTEND_CSRF_ERROR',

Review Comment:
   From [superset-frontend/src/components/ErrorMessage/types.ts](https://github.com/apache/superset/blob/master/superset-frontend/src/components/ErrorMessage/types.ts#L21), I needed to bring it over to superset-ui for the `SupersetError` type, which was needed in the `Query` type. I did not like copying this much code, but it seemed like the best way to do this for now 😅 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889228633


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -83,8 +89,12 @@ export const dnd_adhoc_filters: SharedControlConfig<'DndFilterSelect'> = {
   default: [],
   description: '',
   mapStateToProps: ({ datasource, form_data }) => ({
-    columns: datasource?.columns.filter(c => c.filterable) || [],
-    savedMetrics: datasource?.metrics || [],
+    columns: datasource?.columns[0]?.hasOwnProperty('filterable')
+      ? (datasource as Dataset)?.columns.filter(c => c.filterable)
+      : datasource?.columns || [],
+    savedMetrics: datasource?.hasOwnProperty('metrics')

Review Comment:
   Done in [`this commit`](https://github.com/apache/superset/pull/19855/commits/a5855462a4ba47258f598734a94c9df385715ea1) 😁 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889421101


##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/savedMetricsTypeCheck.ts:
##########
@@ -0,0 +1,30 @@
+/* eslint-disable camelcase */
+/**
+ * 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 { QueryResponse } from '@superset-ui/core';
+import { Dataset } from '../types';
+import { DEFAULT_METRICS } from '..';
+
+export const savedMetricsTypeCheck = (

Review Comment:
   to be more specific, I would say that this function is either fetching/defining or creating metrics. The type checking is just a side effect, if you wanted to rename this to something that explains what it does a little more succinctly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110258961

   @lyndsiWilliams Ephemeral environment creation failed. Please check the Actions logs for details.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860309645


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -190,6 +217,35 @@ export default function DataSourcePanel({
     [_columns],
   );
 
+  const placeholderSlDataset = {
+    sl_table: [],
+    query: [],
+    saved_query: [],
+  };
+
+  // eslint-disable-next-line no-param-reassign
+  datasource.sl_dataset = placeholderSlDataset;
+
+  const getDefaultDatasetName = () =>
+    `${datasource?.sl_dataset?.query.tab} ${moment().format(

Review Comment:
   can you just reference the datasource directly? `datasource?.query.tab`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860309233


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -190,6 +217,35 @@ export default function DataSourcePanel({
     [_columns],
   );
 
+  const placeholderSlDataset = {
+    sl_table: [],
+    query: [],
+    saved_query: [],
+  };
+
+  // eslint-disable-next-line no-param-reassign
+  datasource.sl_dataset = placeholderSlDataset;

Review Comment:
   I think the value that you're looking for is `datasource.type` which will come from the api.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860310058


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -287,6 +344,175 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const handleOverwriteDataset = async () => {
+    const { sql, results, dbId } = datasource?.sl_dataset?.query;
+
+    await updateDataset(
+      dbId,
+      datasetToOverwrite.datasetId,
+      sql,
+      // TODO: lyndsiWilliams - Define d

Review Comment:
   what is this todo for?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110274710

   @lyndsiWilliams Ephemeral environment spinning up at http://54.68.5.95:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] hughhhh commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860140815


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   can we cast these to values we actually want them to be



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889422134


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +301,17 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {

Review Comment:
   if the truthy value is "true", then maybe: 
   `const somethingThatSoundsLIkeABoolean = sessionStorage.getItem('showInfobox') === 'true'



##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +301,17 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {

Review Comment:
   if the truthy value is "true", then maybe: 
   `const somethingThatSoundsLIkeABoolean = sessionStorage.getItem('showInfobox') === 'true'`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889566396


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -178,14 +193,30 @@ export const dnd_granularity_sqla: typeof dndGroupByControl = {
       : 'Drop temporal column here',
   ),
   mapStateToProps: ({ datasource }) => {
-    const temporalColumns = datasource?.columns.filter(c => c.is_dttm) ?? [];
+    if (datasource?.columns[0]?.hasOwnProperty('column_name')) {
+      const temporalColumns =
+        (datasource as Dataset)?.columns.filter(c => c.is_dttm) ?? [];
+      const options = Object.fromEntries(
+        temporalColumns.map(option => [option.column_name, option]),
+      );
+      return {
+        options,
+        default:
+          (datasource as Dataset)?.main_dttm_col ||
+          temporalColumns[0]?.column_name ||
+          null,
+        isTemporal: true,
+      };
+    }
+
+    const temporalColumns =
+      (datasource as QueryResponse)?.columns.filter(c => c.is_dttm) ?? [];

Review Comment:
   Ohhh yes that's right. I added a sort for this in [`this commit`](https://github.com/apache/superset/pull/19855/commits/ac671a4e7f65ce966a1d2c8d9f637d5bfb8cf64a).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860248315


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   So it will look like this?
   
   ```
   sl_datasource?: {
     sl_dataset: string;
     sl_table?: string;
     query?: string;
     saved_query?: string;
   };
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110386047

   Ephemeral environment shutdown and build artifacts deleted.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860310606


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -287,6 +344,175 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const handleOverwriteDataset = async () => {
+    const { sql, results, dbId } = datasource?.sl_dataset?.query;
+
+    await updateDataset(
+      dbId,
+      datasetToOverwrite.datasetId,
+      sql,
+      // TODO: lyndsiWilliams - Define d

Review Comment:
   I defined d as `any` because I'm not sure what it will be, I wanted to remind myself to define that once data is properly flowing through it and I can figure out what it is.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110378063

   /testenv up


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863277793


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -500,48 +228,28 @@ export default class ResultSet extends React.PureComponent<
       }
       const { columns } = this.props.query.results;
       // Added compute logic to stop user from being able to Save & Explore
-      const {
-        saveDatasetRadioBtnState,
-        newSaveDatasetName,
-        datasetToOverwrite,
-        saveModalAutocompleteValue,
-        shouldOverwriteDataSet,
-        userDatasetOptions,
-        showSaveDatasetModal,
-      } = this.state;
-      const disableSaveAndExploreBtn =
-        (saveDatasetRadioBtnState === DatasetRadioState.SAVE_NEW &&
-          newSaveDatasetName.length === 0) ||
-        (saveDatasetRadioBtnState === DatasetRadioState.OVERWRITE_DATASET &&
-          Object.keys(datasetToOverwrite).length === 0 &&
-          saveModalAutocompleteValue.length === 0);
+      const { showSaveDatasetModal } = this.state;
 
       return (
         <ResultSetControls>
           <SaveDatasetModal
             visible={showSaveDatasetModal}
-            onOk={this.handleSaveInDataset}
-            saveDatasetRadioBtnState={saveDatasetRadioBtnState}
-            shouldOverwriteDataset={shouldOverwriteDataSet}
-            defaultCreateDatasetValue={newSaveDatasetName}
-            userDatasetOptions={userDatasetOptions}
-            disableSaveAndExploreBtn={disableSaveAndExploreBtn}
-            onHide={this.handleHideSaveModal}
-            handleDatasetNameChange={this.handleDatasetNameChange}
-            handleSaveDatasetRadioBtnState={this.handleSaveDatasetRadioBtnState}
-            handleOverwriteCancel={this.handleOverwriteCancel}
-            handleOverwriteDataset={this.handleOverwriteDataset}
-            handleOverwriteDatasetOption={this.handleOverwriteDatasetOption}
-            handleSaveDatasetModalSearch={this.handleSaveDatasetModalSearch}
-            filterAutocompleteOption={this.handleFilterAutocompleteOption}
-            onChangeAutoComplete={this.handleOnChangeAutoComplete}
+            onHide={() => this.setState({ showSaveDatasetModal: false })}
+            buttonTextOnSave={t('Save & Explore')}
+            buttonTextOnOverwrite={t('Overwrite & Explore')}
+            modalDescription={t(
+              'Save this query as a virtual dataset to continue exploring',
+            )}
+            user={this.props.user}

Review Comment:
   can we get this from redux inside the savedatasetmodal?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] AAfghahi commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r875111745


##########
superset-frontend/src/dashboard/types.ts:
##########
@@ -84,7 +84,7 @@ export type DashboardInfo = {
 
 export type ChartsState = { [key: string]: Chart };
 
-export type Datasource = DatasourceMeta & {
+export type Datasource = Dataset & {

Review Comment:
   is this similar to extends?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r879954298


##########
superset-frontend/packages/superset-ui-core/src/query/types/Datasource.ts:
##########
@@ -22,6 +22,10 @@ import { Metric } from './Metric';
 export enum DatasourceType {
   Table = 'table',
   Druid = 'druid',
+  Query = 'query',
+  Dataset = 'sl_dataset',

Review Comment:
   we're calling this "dataset" on the backend. We may have to change this, but it's fine for now since we're not using it. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r876974039


##########
superset-frontend/src/dashboard/types.ts:
##########
@@ -84,7 +84,7 @@ export type DashboardInfo = {
 
 export type ChartsState = { [key: string]: Chart };
 
-export type Datasource = DatasourceMeta & {
+export type Datasource = Dataset & {

Review Comment:
   This is exactly the same extends 😁 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r883159770


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -82,14 +83,18 @@ export const dnd_adhoc_filters: SharedControlConfig<'DndFilterSelect'> = {
   label: t('Filters'),
   default: [],
   description: '',
-  mapStateToProps: ({ datasource, form_data }) => ({
-    columns: datasource?.columns.filter(c => c.filterable) || [],
-    savedMetrics: datasource?.metrics || [],
-    // current active adhoc metrics
-    selectedMetrics:
-      form_data.metrics || (form_data.metric ? [form_data.metric] : []),
-    datasource,
-  }),
+  mapStateToProps: ({ datasource, form_data }) => {
+    const dataset = datasource as Dataset;
+
+    return {
+      columns: dataset?.columns.filter(c => c.filterable) || [],

Review Comment:
   ```suggestion
         columns: datasource.columns.filter(c => c.filterable) || [],
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams closed pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams closed pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox
URL: https://github.com/apache/superset/pull/19855


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] jess-dillard commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
jess-dillard commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110287562

   Thanks @lyndsiWilliams ! Two things: 
   
   1. The infobox shouldn't fill the full width of the sidebar (it should have the same gutter width on either side as the metrics/columns below it).
   2. I think we should remove the line of copy in the save dataset modal if it's opened from Explore – it doesn't make as much sense outside of SQL Lab.
   
   <img width="627" alt="Screen Shot 2022-04-26 at 2 50 44 PM" src="https://user-images.githubusercontent.com/90715511/165398757-46b87865-a690-436b-858d-120daba8f09b.png">


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] hughhhh commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r868314045


##########
superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx:
##########
@@ -162,3 +165,45 @@ test('should render a warning', async () => {
     await screen.findByRole('img', { name: 'alert-solid' }),
   ).toBeInTheDocument();
 });
+
+test('should render a create dataset infobox', () => {
+  render(
+    setup({
+      ...props,
+      datasource: {
+        ...datasource,
+        type: DatasourceType.SavedQuery,

Review Comment:
   should just be `DatasourceType.Query` not `SavedQuery` they are different models, you'll have to do this for another ticket



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams merged pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams merged PR #19855:
URL: https://github.com/apache/superset/pull/19855


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r876974626


##########
superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx:
##########
@@ -174,8 +174,69 @@ test('should render a warning', async () => {
         },
       },
     }),
+    { useRedux: true },
   );
   expect(
     await screen.findByRole('img', { name: 'alert-solid' }),
   ).toBeInTheDocument();
 });
+
+test('should render a create dataset infobox', () => {
+  render(
+    setup({
+      ...props,
+      datasource: {
+        ...datasource,
+        type: DatasourceType.Query,
+      },
+    }),
+    { useRedux: true },
+  );
+
+  screen.logTestingPlaygroundURL();

Review Comment:
   Whoops! Good catch, I missed that one. Removed in [`this commit`](https://github.com/apache/superset/pull/19855/commits/5d7be1141a9c33ce1b5af9365c074cbe167fb4eb).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] AAfghahi commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r875112076


##########
superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx:
##########
@@ -174,8 +174,69 @@ test('should render a warning', async () => {
         },
       },
     }),
+    { useRedux: true },
   );
   expect(
     await screen.findByRole('img', { name: 'alert-solid' }),
   ).toBeInTheDocument();
 });
+
+test('should render a create dataset infobox', () => {
+  render(
+    setup({
+      ...props,
+      datasource: {
+        ...datasource,
+        type: DatasourceType.Query,
+      },
+    }),
+    { useRedux: true },
+  );
+
+  screen.logTestingPlaygroundURL();

Review Comment:
   delete?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r875098396


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,153 +17,362 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+  // DatasourceType,
+} from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import moment from 'moment';
+import rison from 'rison';
+import { createDatasource } from 'src/SqlLab/actions/sqlLab';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+  SqlLabExploreRootState,
+  getInitialState,
+} from 'src/SqlLab/types';
+// import { Dataset } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
+
+// type ExploreDatasource = Dataset | Query;
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource: Query;

Review Comment:
   This was changed to a union type in [`this commit`](https://github.com/apache/superset/pull/19855/commits/1a31c5e6a5eefa39b3e5b86447e4927ac85cc46b).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863276361


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   maybe we can just start with Query: superset-frontend/src/SqlLab/types.ts



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r879953367


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx:
##########
@@ -131,8 +132,9 @@ const groupByControl: SharedControlConfig<'SelectControl', ColumnMeta> = {
   promptTextCreator: (label: unknown) => label,
   mapStateToProps(state, { includeTime }) {
     const newState: ExtraControlProps = {};
+    const dataset = state.datasource as Dataset;

Review Comment:
   same here, for future PR, will need be able to be a Query too.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889422536


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -36,15 +39,25 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
   ),
   mapStateToProps(state, { includeTime }) {
     const newState: ExtraControlProps = {};
-    if (state.datasource) {
-      const options = state.datasource.columns.filter(c => c.groupby);
+    const { datasource } = state;
+    if (datasource?.columns[0]?.hasOwnProperty('groupby')) {
+      const options = (datasource as Dataset).columns.filter(c => c.groupby);
       if (includeTime) {
-        options.unshift(TIME_COLUMN_OPTION);
+        options.unshift(DATASET_TIME_COLUMN_OPTION);
       }
       newState.options = Object.fromEntries(
         options.map(option => [option.column_name, option]),
       );
-      newState.savedMetrics = state.datasource.metrics || [];
+      newState.savedMetrics = (datasource as Dataset).metrics || [];
+    } else {
+      const options = datasource?.columns;
+      if (includeTime) {
+        (options as QueryColumn[])?.unshift(QUERY_TIME_COLUMN_OPTION);
+      }
+      newState.options = Object.fromEntries(
+        (options as QueryColumn[])?.map(option => [option.name, option]),
+      );
+      newState.options = datasource?.columns;

Review Comment:
   actually, looks like we'll have to wait for the column name property to be the same first. 👍 
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889126906


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -36,16 +41,17 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
   ),
   mapStateToProps(state, { includeTime }) {
     const newState: ExtraControlProps = {};
-    if (state.datasource) {
-      const options = state.datasource.columns.filter(c => c.groupby);
+    const { datasource } = state;
+    if (datasource?.columns[0]?.hasOwnProperty('groupby')) {
+      const options = (datasource as Dataset).columns.filter(c => c.groupby);
       if (includeTime) {
         options.unshift(TIME_COLUMN_OPTION);
       }
       newState.options = Object.fromEntries(
         options.map(option => [option.column_name, option]),
       );
-      newState.savedMetrics = state.datasource.metrics || [];
-    }
+      newState.savedMetrics = (datasource as Dataset).metrics || [];
+    } else newState.options = datasource?.columns;

Review Comment:
   @lyndsiWilliams I think we're going to need lines 47-52 applied to query columns as well. Just the filter we can skip. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863276742


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   looks like there's an interface for savedquery here: superset-frontend/src/views/CRUD/welcome/SavedQueries.tsx but don't be confused.. it's named query :)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863964256


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -74,96 +79,290 @@ const Styles = styled.div`
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,
+  user,
+  query,
+  actions,
+}) => {
+  const getDefaultDatasetName = () =>
+    `${query?.tab || datasource?.sl_dataset?.query.tab} ${moment().format(
+      'MM/DD/YYYY HH:mm:ss',
+    )}`;
+
+  const [newSaveDatasetName, setNewSaveDatasetName] = useState(
+    getDefaultDatasetName(),
+  );
+  const [saveDatasetRadioBtnState, setSaveDatasetRadioBtnState] = useState(
+    DatasetRadioState.SAVE_NEW,
+  );
+  const [shouldOverwriteDataset, setShouldOverwriteDataset] = useState(false);
+  const [userDatasetOptions, setUserDatasetOptions] = useState<
+    DatasetOptionAutocomplete[]
+  >([]);
+  const [datasetToOverwrite, setDatasetToOverwrite] = useState<
+    Record<string, any>
+  >({});
+  const [saveModalAutocompleteValue] = useState('');
+
+  const handleOverwriteDataset = async () => {
+    await updateDataset(
+      query!.dbId,
+      datasetToOverwrite.datasetId,
+      query!.sql,
+      // TODO: lyndsiWilliams - Define d
+      query!.results.selected_columns.map((d: any) => ({
+        column_name: d.name,
+        type: d.type,
+        is_dttm: d.is_dttm,
+      })),
+      datasetToOverwrite.owners.map((o: DatasetOwner) => o.id),
+      true,
+    );
+
+    setShouldOverwriteDataset(false);
+    setDatasetToOverwrite({});
+    setNewSaveDatasetName(getDefaultDatasetName());
+
+    exploreChart({
+      ...EXPLORE_CHART_DEFAULT,
+      datasource: `${datasetToOverwrite.datasetId}__table`,
+      // TODO: lyndsiWilliams -  Define d
+      all_columns: query!.results.selected_columns.map((d: any) => d.name),
+    });
+  };
+
+  const getUserDatasets = async (searchText = '') => {
+    // Making sure that autocomplete input has a value before rendering the dropdown
+    // Transforming the userDatasetsOwned data for SaveModalComponent)
+    const { userId } = user;
+    if (userId) {
+      const queryParams = rison.encode({
+        filters: [
+          {
+            col: 'table_name',
+            opr: 'ct',
+            value: searchText,
+          },
+          {
+            col: 'owners',
+            opr: 'rel_m_m',
+            value: userId,
+          },
+        ],
+        order_column: 'changed_on_delta_humanized',
+        order_direction: 'desc',
+      });
+
+      const response = await makeApi({
+        method: 'GET',
+        endpoint: '/api/v1/dataset',
+      })(`q=${queryParams}`);
+
+      return response.result.map(
+        (r: { table_name: string; id: number; owners: [DatasetOwner] }) => ({
+          value: r.table_name,
+          datasetId: r.id,
+          owners: r.owners,
+        }),
+      );
+    }
+
+    return null;
+  };
+
+  const handleSaveInDataset = () => {
+    // if user wants to overwrite a dataset we need to prompt them
+    if (saveDatasetRadioBtnState === DatasetRadioState.OVERWRITE_DATASET) {
+      setShouldOverwriteDataset(true);
+      return;
+    }
+
+    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.
+    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);
+      }
+    }
+
+    // TODO: lyndsiWilliams - set up when the back end logic is implemented
+    actions
+      ?.createDatasource({
+        schema: query?.schema,
+        sql: query?.sql,
+        dbId: query?.dbId,
+        templateParams: query?.templateParams,
+        datasourceName: newSaveDatasetName,
+        columns: selectedColumns,
+      })
+      .then((data: { table_id: number }) => {
+        exploreChart({
+          datasource: `${data.table_id}__table`,
+          metrics: [],
+          groupby: [],
+          time_range: 'No filter',
+          viz_type: 'table',
+          all_columns: selectedColumns.map(c => c.name),
+          row_limit: 1000,
+        });
+      })
+      .catch(() => {
+        actions.addDangerToast(t('An error occurred saving dataset'));
+      });
+
+    setNewSaveDatasetName(getDefaultDatasetName());
+    onHide();
+  };
+
+  const handleSaveDatasetModalSearch = async (searchText: string) => {
+    const userDatasetsOwned = await getUserDatasets(searchText);
+    setUserDatasetOptions(userDatasetsOwned);
+  };
+
+  const handleOverwriteDatasetOption = (
+    _data: string,
+    option: Record<string, any>,
+  ) => setDatasetToOverwrite(option);
+
+  const handleDatasetNameChange = (e: React.FormEvent<HTMLInputElement>) => {
+    // @ts-expect-error
+    setNewSaveDatasetName(e.target.value);
+  };
+
+  const handleSaveDatasetRadioBtnState = (e: RadioChangeEvent) => {
+    setSaveDatasetRadioBtnState(Number(e.target.value));
+  };
+
+  const handleOverwriteCancel = () => {
+    setShouldOverwriteDataset(false);
+    setDatasetToOverwrite({});
+  };
+
+  const disableSaveAndExploreBtn =
+    (saveDatasetRadioBtnState === DatasetRadioState.SAVE_NEW &&
+      newSaveDatasetName.length === 0) ||
+    (saveDatasetRadioBtnState === DatasetRadioState.OVERWRITE_DATASET &&
+      Object.keys(datasetToOverwrite).length === 0 &&
+      saveModalAutocompleteValue.length === 0);
+
+  const filterAutocompleteOption = (
+    inputValue: string,
+    option: { value: string; datasetId: number },
+  ) => option.value.toLowerCase().includes(inputValue.toLowerCase());
+
+  return (
+    <StyledModal
+      show={visible}
+      title={t('Save or Overwrite Dataset')}
+      onHide={onHide}
+      footer={
+        <>
+          {!shouldOverwriteDataset && (
             <Button
-              className="md"
-              buttonStyle="primary"
-              onClick={handleOverwriteDataset}
               disabled={disableSaveAndExploreBtn}
+              buttonStyle="primary"
+              onClick={handleSaveInDataset}
             >
-              {t('Overwrite & Explore')}
+              {buttonTextOnSave}
             </Button>
-          </>
+          )}
+          {shouldOverwriteDataset && (
+            <>
+              <Button onClick={handleOverwriteCancel}>Back</Button>
+              <Button
+                className="md"
+                buttonStyle="primary"
+                onClick={handleOverwriteDataset}
+                disabled={disableSaveAndExploreBtn}
+              >
+                {buttonTextOnOverwrite}
+              </Button>
+            </>
+          )}
+        </>
+      }
+    >
+      <Styles>
+        {!shouldOverwriteDataset && (
+          <div className="smd-body">

Review Comment:
   @lyndsiWilliams do you mind cleaning up the old code here too while we're at it? Really small nit, but I think this is supposed to be `sdm` for save dataset modal? :)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863282371


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -74,96 +79,290 @@ const Styles = styled.div`
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,
+  user,
+  query,
+  actions,
+}) => {
+  const getDefaultDatasetName = () =>
+    `${query?.tab || datasource?.sl_dataset?.query.tab} ${moment().format(
+      'MM/DD/YYYY HH:mm:ss',
+    )}`;
+
+  const [newSaveDatasetName, setNewSaveDatasetName] = useState(
+    getDefaultDatasetName(),
+  );
+  const [saveDatasetRadioBtnState, setSaveDatasetRadioBtnState] = useState(

Review Comment:
   might be more explicit to name this as to what the radio button does, too.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863282009


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -74,96 +79,290 @@ const Styles = styled.div`
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,
+  user,
+  query,
+  actions,
+}) => {
+  const getDefaultDatasetName = () =>
+    `${query?.tab || datasource?.sl_dataset?.query.tab} ${moment().format(
+      'MM/DD/YYYY HH:mm:ss',
+    )}`;
+
+  const [newSaveDatasetName, setNewSaveDatasetName] = useState(

Review Comment:
   not completely sure of the context here, but having the word "new" in state feels a little weird. Could this just be datasetName?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r872885684


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,153 +17,362 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+  // DatasourceType,
+} from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import moment from 'moment';
+import rison from 'rison';
+import { createDatasource } from 'src/SqlLab/actions/sqlLab';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+  SqlLabExploreRootState,
+  getInitialState,
+} from 'src/SqlLab/types';
+// import { Dataset } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
+
+// type ExploreDatasource = Dataset | Query;
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource: Query;
 }
 
 const Styles = styled.div`
-  .smd-body {
+  .sdm-body {
     margin: 0 8px;
   }
-  .smd-input {
+  .sdm-input {
     margin-left: 45px;
     width: 401px;
   }
-  .smd-autocomplete {
+  .sdm-autocomplete {
     margin-left: 8px;
     width: 401px;
   }
-  .smd-radio {
+  .sdm-radio {
     display: block;
     height: 30px;
     margin: 10px 0px;
     line-height: 30px;
   }
-  .smd-overwrite-msg {
+  .sdm-overwrite-msg {
     margin: 7px;
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
+// const getDatasourceState = (datasource: ExploreDatasource) => {
+//   if (datasource.type === DatasourceType.Dataset) return datasource as Dataset;
+//   return datasource as Query;
+// };
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,
+}) => {
+  const getDefaultDatasetName = () =>
+    `${datasource.tab} ${moment().format('MM/DD/YYYY HH:mm:ss')}`;
+  const [datasetName, setDatasetName] = useState(getDefaultDatasetName());
+  const [newOrOverwrite, setNewOrOverwrite] = useState(
+    DatasetRadioState.SAVE_NEW,
+  );
+  const [shouldOverwriteDataset, setShouldOverwriteDataset] = useState(false);
+  const [userDatasetOptions, setUserDatasetOptions] = useState<
+    DatasetOptionAutocomplete[]
+  >([]);
+  const [datasetToOverwrite, setDatasetToOverwrite] = useState<
+    Record<string, any>
+  >({});
+  const [autocompleteValue, setAutocompleteValue] = useState('');
+
+  const user = useSelector<SqlLabExploreRootState, User>(state =>
+    getInitialState(state),
+  );
+
+  const handleOverwriteDataset = async () => {
+    await updateDataset(
+      datasource.dbId,
+      datasetToOverwrite.datasetId,
+      datasource.sql,
+      // TODO: lyndsiWilliams - Define d

Review Comment:
   todo



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r883171027


##########
superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx:
##########
@@ -129,10 +130,12 @@ const dnd_all_columns: typeof sharedControls.groupby = {
   mapStateToProps({ datasource, controls }, controlState) {
     const newState: ExtraControlProps = {};
     if (datasource) {
-      const options = (datasource as Dataset).columns;
-      newState.options = Object.fromEntries(
-        options.map(option => [option.column_name, option]),
-      );
+      if (datasource?.columns?.hasOwnProperty('filterable')) {

Review Comment:
   this block doesn't look like it uses filterable?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r880373030


##########
superset-frontend/packages/superset-ui-core/src/query/types/Query.ts:
##########
@@ -165,4 +165,154 @@ export interface QueryContext {
   form_data?: QueryFormData;
 }
 
+export const ErrorTypeEnum = {
+  // Frontend errors
+  FRONTEND_CSRF_ERROR: 'FRONTEND_CSRF_ERROR',

Review Comment:
   It gives a few errors:
   
   'superset' should be listed in the project's dependencies. Run 'npm i -S superset' to add it
   
   File '/Users/lyndsikaywilliams/Preset/incubator-superset/superset-frontend/src/SqlLab/types.ts' is not under 'rootDir' '/Users/lyndsikaywilliams/Preset/incubator-superset/superset-frontend/packages/superset-ui-core/src'. 'rootDir' is expected to contain all source files.
   
   File '/Users/lyndsikaywilliams/Preset/incubator-superset/superset-frontend/src/SqlLab/types.ts' is not listed within the file list of project '/Users/lyndsikaywilliams/Preset/incubator-superset/superset-frontend/packages/superset-ui-core/tsconfig.json'. Projects must list all files or use an 'include' pattern.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889420748


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -36,15 +39,25 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
   ),
   mapStateToProps(state, { includeTime }) {
     const newState: ExtraControlProps = {};
-    if (state.datasource) {
-      const options = state.datasource.columns.filter(c => c.groupby);
+    const { datasource } = state;
+    if (datasource?.columns[0]?.hasOwnProperty('groupby')) {
+      const options = (datasource as Dataset).columns.filter(c => c.groupby);
       if (includeTime) {
-        options.unshift(TIME_COLUMN_OPTION);
+        options.unshift(DATASET_TIME_COLUMN_OPTION);
       }
       newState.options = Object.fromEntries(
         options.map(option => [option.column_name, option]),
       );
-      newState.savedMetrics = state.datasource.metrics || [];
+      newState.savedMetrics = (datasource as Dataset).metrics || [];
+    } else {
+      const options = datasource?.columns;
+      if (includeTime) {
+        (options as QueryColumn[])?.unshift(QUERY_TIME_COLUMN_OPTION);
+      }
+      newState.options = Object.fromEntries(
+        (options as QueryColumn[])?.map(option => [option.name, option]),
+      );
+      newState.options = datasource?.columns;

Review Comment:
   @lyndsiWilliams do you think you can dry up lines 45-60 more?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889421712


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +302,17 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {
+    if (sessionStorage.getItem('showInfobox') === 'false') return false;
+    return true;
+  };
+
+  const datasourceTypeCheck =

Review Comment:
   I would maybe name this something that sounds like a boolean like `isValidDatasourceType`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1138955041

   > This looks good @lyndsiWilliams! There are a lot of type coercions `datasource as Dataset`. Since we know that Query is going to have a column property per @hughhhh's PR, do you think you can add a column property to the query type and see how many of these coercions we can then remove? It will also give us a sense of what's left to be added to the query model. thanks!
   
   Done in [`this commit`](https://github.com/apache/superset/pull/19855/commits/3a5f2da86d331d764d6efbacc971e7cb63875ea6)! (and touched up in the two following commits 😅)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] AAfghahi commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r878350853


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +301,17 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {

Review Comment:
   this can follow the same process as datasourceTypeCheck



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r876975903


##########
superset-frontend/packages/superset-ui-core/src/query/types/Query.ts:
##########
@@ -165,4 +165,154 @@ export interface QueryContext {
   form_data?: QueryFormData;
 }
 
+export const ErrorTypeEnum = {
+  // Frontend errors
+  FRONTEND_CSRF_ERROR: 'FRONTEND_CSRF_ERROR',
+  FRONTEND_NETWORK_ERROR: 'FRONTEND_NETWORK_ERROR',
+  FRONTEND_TIMEOUT_ERROR: 'FRONTEND_TIMEOUT_ERROR',
+
+  // DB Engine errors
+  GENERIC_DB_ENGINE_ERROR: 'GENERIC_DB_ENGINE_ERROR',
+  COLUMN_DOES_NOT_EXIST_ERROR: 'COLUMN_DOES_NOT_EXIST_ERROR',
+  TABLE_DOES_NOT_EXIST_ERROR: 'TABLE_DOES_NOT_EXIST_ERROR',
+  SCHEMA_DOES_NOT_EXIST_ERROR: 'SCHEMA_DOES_NOT_EXIST_ERROR',
+  CONNECTION_INVALID_USERNAME_ERROR: 'CONNECTION_INVALID_USERNAME_ERROR',
+  CONNECTION_INVALID_PASSWORD_ERROR: 'CONNECTION_INVALID_PASSWORD_ERROR',
+  CONNECTION_INVALID_HOSTNAME_ERROR: 'CONNECTION_INVALID_HOSTNAME_ERROR',
+  CONNECTION_PORT_CLOSED_ERROR: 'CONNECTION_PORT_CLOSED_ERROR',
+  CONNECTION_INVALID_PORT_ERROR: 'CONNECTION_INVALID_PORT_ERROR',
+  CONNECTION_HOST_DOWN_ERROR: 'CONNECTION_HOST_DOWN_ERROR',
+  CONNECTION_ACCESS_DENIED_ERROR: 'CONNECTION_ACCESS_DENIED_ERROR',
+  CONNECTION_UNKNOWN_DATABASE_ERROR: 'CONNECTION_UNKNOWN_DATABASE_ERROR',
+  CONNECTION_DATABASE_PERMISSIONS_ERROR:
+    'CONNECTION_DATABASE_PERMISSIONS_ERROR',
+  CONNECTION_MISSING_PARAMETERS_ERRORS: 'CONNECTION_MISSING_PARAMETERS_ERRORS',
+  OBJECT_DOES_NOT_EXIST_ERROR: 'OBJECT_DOES_NOT_EXIST_ERROR',
+  SYNTAX_ERROR: 'SYNTAX_ERROR',
+
+  // Viz errors
+  VIZ_GET_DF_ERROR: 'VIZ_GET_DF_ERROR',
+  UNKNOWN_DATASOURCE_TYPE_ERROR: 'UNKNOWN_DATASOURCE_TYPE_ERROR',
+  FAILED_FETCHING_DATASOURCE_INFO_ERROR:
+    'FAILED_FETCHING_DATASOURCE_INFO_ERROR',
+
+  // Security access errors
+  TABLE_SECURITY_ACCESS_ERROR: 'TABLE_SECURITY_ACCESS_ERROR',
+  DATASOURCE_SECURITY_ACCESS_ERROR: 'DATASOURCE_SECURITY_ACCESS_ERROR',
+  DATABASE_SECURITY_ACCESS_ERROR: 'DATABASE_SECURITY_ACCESS_ERROR',
+  QUERY_SECURITY_ACCESS_ERROR: 'QUERY_SECURITY_ACCESS_ERROR',
+  MISSING_OWNERSHIP_ERROR: 'MISSING_OWNERSHIP_ERROR',
+
+  // Other errors
+  BACKEND_TIMEOUT_ERROR: 'BACKEND_TIMEOUT_ERROR',
+  DATABASE_NOT_FOUND_ERROR: 'DATABASE_NOT_FOUND_ERROR',
+
+  // Sqllab error
+  MISSING_TEMPLATE_PARAMS_ERROR: 'MISSING_TEMPLATE_PARAMS_ERROR',
+  INVALID_TEMPLATE_PARAMS_ERROR: 'INVALID_TEMPLATE_PARAMS_ERROR',
+  RESULTS_BACKEND_NOT_CONFIGURED_ERROR: 'RESULTS_BACKEND_NOT_CONFIGURED_ERROR',
+  DML_NOT_ALLOWED_ERROR: 'DML_NOT_ALLOWED_ERROR',
+  INVALID_CTAS_QUERY_ERROR: 'INVALID_CTAS_QUERY_ERROR',
+  INVALID_CVAS_QUERY_ERROR: 'INVALID_CVAS_QUERY_ERROR',
+  SQLLAB_TIMEOUT_ERROR: 'SQLLAB_TIMEOUT_ERROR',
+  RESULTS_BACKEND_ERROR: 'RESULTS_BACKEND_ERROR',
+  ASYNC_WORKERS_ERROR: 'ASYNC_WORKERS_ERROR',
+
+  // Generic errors
+  GENERIC_COMMAND_ERROR: 'GENERIC_COMMAND_ERROR',
+  GENERIC_BACKEND_ERROR: 'GENERIC_BACKEND_ERROR',
+
+  // API errors
+  INVALID_PAYLOAD_FORMAT_ERROR: 'INVALID_PAYLOAD_FORMAT_ERROR',
+  INVALID_PAYLOAD_SCHEMA_ERROR: 'INVALID_PAYLOAD_SCHEMA_ERROR',
+} as const;
+
+type ValueOf<T> = T[keyof T];
+
+export type ErrorType = ValueOf<typeof ErrorTypeEnum>;
+
+// Keep in sync with superset/views/errors.py
+export type ErrorLevel = 'info' | 'warning' | 'error';
+
+export type ErrorSource = 'dashboard' | 'explore' | 'sqllab';
+
+export type SupersetError<ExtraType = Record<string, any> | null> = {
+  error_type: ErrorType;
+  extra: ExtraType;
+  level: ErrorLevel;
+  message: string;
+};
+
+export const CtasEnum = {
+  TABLE: 'TABLE',
+  VIEW: 'VIEW',
+};
+
+export type QueryColumn = {
+  name: string;
+  type: string | null;
+  is_dttm: boolean;
+};
+
+export type QueryState =
+  | 'stopped'
+  | 'failed'
+  | 'pending'
+  | 'running'
+  | 'scheduled'
+  | 'success'
+  | 'fetching'
+  | 'timed_out';
+
+export type Query = {

Review Comment:
   Can do! Done in [`this commit`](https://github.com/apache/superset/pull/19855/commits/5e697605d9e11b8ed086b56c3fe12c8db2cd09e0).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110273175

   /testenv up


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] hughhhh commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860139927


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -190,6 +217,35 @@ export default function DataSourcePanel({
     [_columns],
   );
 
+  const placeholderSlDataset = {
+    sl_table: [],

Review Comment:
   why are we using arrays as placeholders here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860193470


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -26,6 +26,19 @@ import {
 import { debounce } from 'lodash';
 import { matchSorter, rankings } from 'match-sorter';
 import Collapse from 'src/components/Collapse';
+import Alert from 'src/components/Alert';
+import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
+import { exploreChart } from 'src/explore/exploreUtils';
+import moment from 'moment';
+import rison from 'rison';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  updateDataset,
+} from 'src/SqlLab/components/ResultSet';

Review Comment:
   Can do! Done in [`this commit`](https://github.com/apache/superset/pull/19855/commits/9985ec71158a6d7088f1382b24bfa1454d4891ea)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r872885593


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,153 +17,362 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+  // DatasourceType,
+} from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import moment from 'moment';
+import rison from 'rison';
+import { createDatasource } from 'src/SqlLab/actions/sqlLab';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+  SqlLabExploreRootState,
+  getInitialState,
+} from 'src/SqlLab/types';
+// import { Dataset } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
+
+// type ExploreDatasource = Dataset | Query;
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource: Query;
 }
 
 const Styles = styled.div`
-  .smd-body {
+  .sdm-body {
     margin: 0 8px;
   }
-  .smd-input {
+  .sdm-input {
     margin-left: 45px;
     width: 401px;
   }
-  .smd-autocomplete {
+  .sdm-autocomplete {
     margin-left: 8px;
     width: 401px;
   }
-  .smd-radio {
+  .sdm-radio {
     display: block;
     height: 30px;
     margin: 10px 0px;
     line-height: 30px;
   }
-  .smd-overwrite-msg {
+  .sdm-overwrite-msg {
     margin: 7px;
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
+// const getDatasourceState = (datasource: ExploreDatasource) => {
+//   if (datasource.type === DatasourceType.Dataset) return datasource as Dataset;
+//   return datasource as Query;
+// };
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,
+}) => {
+  const getDefaultDatasetName = () =>
+    `${datasource.tab} ${moment().format('MM/DD/YYYY HH:mm:ss')}`;
+  const [datasetName, setDatasetName] = useState(getDefaultDatasetName());
+  const [newOrOverwrite, setNewOrOverwrite] = useState(
+    DatasetRadioState.SAVE_NEW,
+  );
+  const [shouldOverwriteDataset, setShouldOverwriteDataset] = useState(false);
+  const [userDatasetOptions, setUserDatasetOptions] = useState<
+    DatasetOptionAutocomplete[]
+  >([]);
+  const [datasetToOverwrite, setDatasetToOverwrite] = useState<
+    Record<string, any>
+  >({});
+  const [autocompleteValue, setAutocompleteValue] = useState('');
+
+  const user = useSelector<SqlLabExploreRootState, User>(state =>
+    getInitialState(state),

Review Comment:
   is this returning the user or the entire state?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r872885994


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,153 +17,362 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+  // DatasourceType,
+} from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import moment from 'moment';
+import rison from 'rison';
+import { createDatasource } from 'src/SqlLab/actions/sqlLab';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+  SqlLabExploreRootState,
+  getInitialState,
+} from 'src/SqlLab/types';
+// import { Dataset } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
+
+// type ExploreDatasource = Dataset | Query;
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource: Query;
 }
 
 const Styles = styled.div`
-  .smd-body {
+  .sdm-body {
     margin: 0 8px;
   }
-  .smd-input {
+  .sdm-input {
     margin-left: 45px;
     width: 401px;
   }
-  .smd-autocomplete {
+  .sdm-autocomplete {
     margin-left: 8px;
     width: 401px;
   }
-  .smd-radio {
+  .sdm-radio {
     display: block;
     height: 30px;
     margin: 10px 0px;
     line-height: 30px;
   }
-  .smd-overwrite-msg {
+  .sdm-overwrite-msg {
     margin: 7px;
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
+// const getDatasourceState = (datasource: ExploreDatasource) => {
+//   if (datasource.type === DatasourceType.Dataset) return datasource as Dataset;
+//   return datasource as Query;
+// };
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,

Review Comment:
   for consistency, can we also call this dataset?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r872885393


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,153 +17,362 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+  // DatasourceType,
+} from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import moment from 'moment';
+import rison from 'rison';
+import { createDatasource } from 'src/SqlLab/actions/sqlLab';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+  SqlLabExploreRootState,
+  getInitialState,
+} from 'src/SqlLab/types';
+// import { Dataset } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
+
+// type ExploreDatasource = Dataset | Query;
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource: Query;
 }
 
 const Styles = styled.div`
-  .smd-body {
+  .sdm-body {
     margin: 0 8px;
   }
-  .smd-input {
+  .sdm-input {
     margin-left: 45px;
     width: 401px;
   }
-  .smd-autocomplete {
+  .sdm-autocomplete {
     margin-left: 8px;
     width: 401px;
   }
-  .smd-radio {
+  .sdm-radio {
     display: block;
     height: 30px;
     margin: 10px 0px;
     line-height: 30px;
   }
-  .smd-overwrite-msg {
+  .sdm-overwrite-msg {
     margin: 7px;
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
+// const getDatasourceState = (datasource: ExploreDatasource) => {
+//   if (datasource.type === DatasourceType.Dataset) return datasource as Dataset;
+//   return datasource as Query;
+// };

Review Comment:
   can we remove this?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860306935


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   I think you can delete all that.. it's all on the type property here under `DatasourceType`
   ```
   export interface DatasourceMeta {
     id: number;
     type: DatasourceType;
     ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110385822

   Closing and reopening to reset ephemeral environment.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r879953838


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -88,7 +89,7 @@ export interface ActionDispatcher<
  * Mapping of action dispatchers
  */
 export interface ControlPanelActionDispatchers {
-  setDatasource: ActionDispatcher<[DatasourceMeta]>;
+  setDatasource: ActionDispatcher<[Dataset]>;

Review Comment:
   will need to be Query too in the future



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1134938635

   /testenv up


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863988958


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,38 +17,43 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+} from '@superset-ui/core';
+import moment from 'moment';
+import rison from 'rison';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+} from 'src/SqlLab/types';
+import { DatasourceMeta } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource?: DatasourceMeta;
+  user: {
+    userId: number;
+  };
+  query?: Query;

Review Comment:
   why an optional query when sql and dbid are required? Can we instead pass in sql and dbid?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863988958


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,38 +17,43 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+} from '@superset-ui/core';
+import moment from 'moment';
+import rison from 'rison';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+} from 'src/SqlLab/types';
+import { DatasourceMeta } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource?: DatasourceMeta;
+  user: {
+    userId: number;
+  };
+  query?: Query;

Review Comment:
   why an optional query when sql and dbid are required?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889422925


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -178,14 +193,30 @@ export const dnd_granularity_sqla: typeof dndGroupByControl = {
       : 'Drop temporal column here',
   ),
   mapStateToProps: ({ datasource }) => {
-    const temporalColumns = datasource?.columns.filter(c => c.is_dttm) ?? [];
+    if (datasource?.columns[0]?.hasOwnProperty('column_name')) {
+      const temporalColumns =
+        (datasource as Dataset)?.columns.filter(c => c.is_dttm) ?? [];
+      const options = Object.fromEntries(
+        temporalColumns.map(option => [option.column_name, option]),
+      );
+      return {
+        options,
+        default:
+          (datasource as Dataset)?.main_dttm_col ||
+          temporalColumns[0]?.column_name ||
+          null,
+        isTemporal: true,
+      };
+    }
+
+    const temporalColumns =
+      (datasource as QueryResponse)?.columns.filter(c => c.is_dttm) ?? [];

Review Comment:
   This is the one where let's show all the columns but put the is_dttm ones at the top, since users can't turn on/ off the is_dttm property. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r883167883


##########
superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/columns.tsx:
##########
@@ -64,10 +65,12 @@ const dndAllColumns: typeof sharedControls.groupby = {
   mapStateToProps({ datasource, controls }, controlState) {
     const newState: ExtraControlProps = {};
     if (datasource) {
-      const options = (datasource as Dataset).columns;
-      newState.options = Object.fromEntries(
-        options.map(option => [option.column_name, option]),
-      );
+      if (datasource?.columns?.hasOwnProperty('filterable')) {
+        const options = (datasource as Dataset).columns;
+        newState.options = Object.fromEntries(
+          options.map((option: ColumnMeta) => [option.column_name, option]),
+        );
+      } else newState.options = datasource.columns;

Review Comment:
   nice



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r883159267


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -36,15 +36,16 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
   ),
   mapStateToProps(state, { includeTime }) {
     const newState: ExtraControlProps = {};
+    const dataset = state.datasource as Dataset;

Review Comment:
   ```suggestion
       const { dataset } = state;
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r891476386


##########
superset-frontend/packages/superset-ui-core/src/query/types/Datasource.ts:
##########
@@ -22,6 +22,10 @@ import { Metric } from './Metric';
 export enum DatasourceType {
   Table = 'table',
   Druid = 'druid',
+  Query = 'query',
+  Dataset = 'sl_dataset',

Review Comment:
   ```suggestion
     Dataset = 'dataset',
   ```
   On second thought, let's just change it now so that we don't forget. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1149110667

   Ephemeral environment shutdown and build artifacts deleted.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] AAfghahi commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r877246876


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +301,22 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {
+    if (sessionStorage.getItem('showInfobox') === 'false') return false;
+    return true;
+  };
+
+  const datasourceTypeCheck = () => {

Review Comment:
   We could rewrite as: 
   `const datasourceTypeCheck = datasource.type == DatasourceType.Dataset. . . .,etc` 
   
   and on checking it would return true or false. That way you don't have to write the if statement at all. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r876975211


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -303,6 +321,32 @@ export default function DataSourcePanel({
           placeholder={t('Search Metrics & Columns')}
         />
         <div className="field-selections">
+          {datasource.type === DatasourceType.Dataset ||

Review Comment:
   Good idea! Much cleaner in [`this commit`](https://github.com/apache/superset/pull/19855/commits/0fec1c3bd373bd53ac181705939bb818df80b36d).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860309627


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   If I delete this, how would I access the data in cases like this one? I think `DatasourceType` only defines the types
   https://github.com/apache/superset/blob/9985ec71158a6d7088f1382b24bfa1454d4891ea/superset-frontend/src/explore/components/DatasourcePanel/index.tsx#L253



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860311030


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -190,6 +217,35 @@ export default function DataSourcePanel({
     [_columns],
   );
 
+  const placeholderSlDataset = {
+    sl_table: [],
+    query: [],
+    saved_query: [],
+  };
+
+  // eslint-disable-next-line no-param-reassign
+  datasource.sl_dataset = placeholderSlDataset;
+
+  const getDefaultDatasetName = () =>
+    `${datasource?.sl_dataset?.query.tab} ${moment().format(

Review Comment:
   There's currently no `query` on DatasourceMeta, should I add it?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860313254


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -287,6 +344,175 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const handleOverwriteDataset = async () => {

Review Comment:
   can most of this functionality re saving/overwriting the dataset be moved into the dataset modal instead? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860192694


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -190,6 +217,35 @@ export default function DataSourcePanel({
     [_columns],
   );
 
+  const placeholderSlDataset = {
+    sl_table: [],

Review Comment:
   I wasn't sure what these properties were going to be but thought I needed to pass in something as a placeholder until the back end is implemented. Turns out I don't need it, so I removed it in [`this commit`](https://github.com/apache/superset/pull/19855/commits/fd65bfa6f6241c466fa4eac90efafcc87f5c95cd). Thanks for this catch!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860196376


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   Do you know what the types for these will be? I was going to figure that out once the backend came through and change it then.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r861046636


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   I think we should make different interfaces for the different datasource types and then create a type that is a union of all of them. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r861048994


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   Is the schema for these types anywhere so I can describe them properly?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110257459

   /testenv up


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863278533


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,38 +17,43 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+} from '@superset-ui/core';
+import moment from 'moment';
+import rison from 'rison';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+} from 'src/SqlLab/types';
+import { DatasourceMeta } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource?: DatasourceMeta;
+  user: {

Review Comment:
   can we get this from redux?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1139160022

   looking good. I think there are a few more places where we don't need the type coercion or more specifically only when there's a property that doesn't exist on datasource. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r879952816


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -36,15 +36,16 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
   ),
   mapStateToProps(state, { includeTime }) {
     const newState: ExtraControlProps = {};
+    const dataset = state.datasource as Dataset;

Review Comment:
   This works for now. In a future PR, we'll need to make sure that a Query type can be used in this component. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889422027


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +301,17 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {

Review Comment:
   same comment as below.. I would name this something that sounds like a boolean, and you an also rewrite it to be a constant value rather than a function: 
   ```
   const somethingThatSoundsLIkeABoolean = !(sessionStorage.getItem('showInfobox') === 'false')
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889130014


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -83,8 +89,12 @@ export const dnd_adhoc_filters: SharedControlConfig<'DndFilterSelect'> = {
   default: [],
   description: '',
   mapStateToProps: ({ datasource, form_data }) => ({
-    columns: datasource?.columns.filter(c => c.filterable) || [],
-    savedMetrics: datasource?.metrics || [],
+    columns: datasource?.columns[0]?.hasOwnProperty('filterable')
+      ? (datasource as Dataset)?.columns.filter(c => c.filterable)
+      : datasource?.columns || [],
+    savedMetrics: datasource?.hasOwnProperty('metrics')

Review Comment:
   do you think we can dry up these three?
   ```
   datasource?.hasOwnProperty('metrics')
         ? (datasource as Dataset)?.metrics || []
         : DEFAULT_METRICS,
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889566205


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx:
##########
@@ -131,13 +135,14 @@ const groupByControl: SharedControlConfig<'SelectControl', ColumnMeta> = {
   promptTextCreator: (label: unknown) => label,
   mapStateToProps(state, { includeTime }) {
     const newState: ExtraControlProps = {};
-    if (state.datasource) {
-      const options = state.datasource.columns.filter(c => c.groupby);
+    const { datasource } = state;
+    if (datasource?.columns[0]?.hasOwnProperty('groupby')) {
+      const options = (datasource as Dataset).columns.filter(c => c.groupby);
       if (includeTime) {
-        options.unshift(TIME_COLUMN_OPTION);
+        options.unshift(DATASET_TIME_COLUMN_OPTION);
       }
       newState.options = options;
-    }
+    } else newState.options = datasource?.columns;

Review Comment:
   Oh good catch, thank you! Fixed in [`this commit`](https://github.com/apache/superset/pull/19855/commits/faac3911412d0a9371261ff1ed3f207cfc9d741b).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889566129


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -289,6 +302,17 @@ export default function DataSourcePanel({
     [lists.columns, showAllColumns],
   );
 
+  const showInfoboxCheck = () => {
+    if (sessionStorage.getItem('showInfobox') === 'false') return false;
+    return true;
+  };
+
+  const datasourceTypeCheck =

Review Comment:
   Also a good idea! I changed the naming in [`this commit`](https://github.com/apache/superset/pull/19855/commits/90c20b23a223352b9f61e472aa36f7ebf6a60c43).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110258945

   @lyndsiWilliams Container image not yet published for this PR. Please try again when build is complete.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110284486

   # [Codecov](https://codecov.io/gh/apache/superset/pull/19855?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#19855](https://codecov.io/gh/apache/superset/pull/19855?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f68e27d) into [master](https://codecov.io/gh/apache/superset/commit/7e92340c7085358940de5ff199b9cc919b35111f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7e92340) will **decrease** coverage by `0.06%`.
   > The diff coverage is `68.04%`.
   
   > :exclamation: Current head f68e27d differs from pull request most recent head 8a50f7a. Consider uploading reports for the commit 8a50f7a to get more accurate results
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #19855      +/-   ##
   ==========================================
   - Coverage   66.51%   66.44%   -0.07%     
   ==========================================
     Files        1690     1713      +23     
     Lines       64616    65012     +396     
     Branches     6656     6697      +41     
   ==========================================
   + Hits        42978    43197     +219     
   - Misses      19937    20109     +172     
   - Partials     1701     1706       +5     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `51.13% <44.48%> (-0.06%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/19855?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...-chart-controls/src/sections/advancedAnalytics.tsx](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NlY3Rpb25zL2FkdmFuY2VkQW5hbHl0aWNzLnRzeA==) | `33.33% <ø> (ø)` | |
   | [...rset-ui-chart-controls/src/sections/chartTitle.tsx](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NlY3Rpb25zL2NoYXJ0VGl0bGUudHN4) | `100.00% <ø> (ø)` | |
   | [...omponents/ColumnConfigControl/ColumnConfigItem.tsx](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9jb21wb25lbnRzL0NvbHVtbkNvbmZpZ0NvbnRyb2wvQ29sdW1uQ29uZmlnSXRlbS50c3g=) | `0.00% <ø> (ø)` | |
   | [.../shared-controls/components/RadioButtonControl.tsx](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9jb21wb25lbnRzL1JhZGlvQnV0dG9uQ29udHJvbC50c3g=) | `0.00% <ø> (ø)` | |
   | [...d/packages/superset-ui-chart-controls/src/types.ts](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3R5cGVzLnRz) | `100.00% <ø> (ø)` | |
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...legacy-plugin-chart-partition/src/controlPanel.tsx](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXBhcnRpdGlvbi9zcmMvY29udHJvbFBhbmVsLnRzeA==) | `33.33% <ø> (ø)` | |
   | [...gins/legacy-plugin-chart-rose/src/controlPanel.tsx](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXJvc2Uvc3JjL2NvbnRyb2xQYW5lbC50c3g=) | `100.00% <ø> (ø)` | |
   | [...gins/legacy-plugin-chart-world-map/src/WorldMap.js](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvV29ybGRNYXAuanM=) | `0.00% <0.00%> (ø)` | |
   | [.../legacy-plugin-chart-world-map/src/controlPanel.ts](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvY29udHJvbFBhbmVsLnRz) | `100.00% <ø> (ø)` | |
   | ... and [108 more](https://codecov.io/gh/apache/superset/pull/19855/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/19855?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/19855?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [7e92340...8a50f7a](https://codecov.io/gh/apache/superset/pull/19855?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] AAfghahi commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r875113091


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -303,6 +321,32 @@ export default function DataSourcePanel({
           placeholder={t('Search Metrics & Columns')}
         />
         <div className="field-selections">
+          {datasource.type === DatasourceType.Dataset ||

Review Comment:
   I think it would look cleaner if you made this check into its own function and then invoked it here. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r875101064


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,153 +17,362 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+  // DatasourceType,
+} from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import moment from 'moment';
+import rison from 'rison';
+import { createDatasource } from 'src/SqlLab/actions/sqlLab';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+  SqlLabExploreRootState,
+  getInitialState,
+} from 'src/SqlLab/types';
+// import { Dataset } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
+
+// type ExploreDatasource = Dataset | Query;
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource: Query;
 }
 
 const Styles = styled.div`
-  .smd-body {
+  .sdm-body {
     margin: 0 8px;
   }
-  .smd-input {
+  .sdm-input {
     margin-left: 45px;
     width: 401px;
   }
-  .smd-autocomplete {
+  .sdm-autocomplete {
     margin-left: 8px;
     width: 401px;
   }
-  .smd-radio {
+  .sdm-radio {
     display: block;
     height: 30px;
     margin: 10px 0px;
     line-height: 30px;
   }
-  .smd-overwrite-msg {
+  .sdm-overwrite-msg {
     margin: 7px;
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
+// const getDatasourceState = (datasource: ExploreDatasource) => {
+//   if (datasource.type === DatasourceType.Dataset) return datasource as Dataset;
+//   return datasource as Query;
+// };
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,
+}) => {
+  const getDefaultDatasetName = () =>
+    `${datasource.tab} ${moment().format('MM/DD/YYYY HH:mm:ss')}`;
+  const [datasetName, setDatasetName] = useState(getDefaultDatasetName());
+  const [newOrOverwrite, setNewOrOverwrite] = useState(
+    DatasetRadioState.SAVE_NEW,
+  );
+  const [shouldOverwriteDataset, setShouldOverwriteDataset] = useState(false);
+  const [userDatasetOptions, setUserDatasetOptions] = useState<
+    DatasetOptionAutocomplete[]
+  >([]);
+  const [datasetToOverwrite, setDatasetToOverwrite] = useState<
+    Record<string, any>
+  >({});
+  const [autocompleteValue, setAutocompleteValue] = useState('');
+
+  const user = useSelector<SqlLabExploreRootState, User>(state =>
+    getInitialState(state),
+  );
+
+  const handleOverwriteDataset = async () => {
+    await updateDataset(
+      datasource.dbId,
+      datasetToOverwrite.datasetId,
+      datasource.sql,
+      // TODO: lyndsiWilliams - Define d

Review Comment:
   Done in [`this commit`](https://github.com/apache/superset/pull/19855/commits/0a82d11c5c8d7210e36242f7b0096b83cd1c8b2a).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] hughhhh commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r876399316


##########
superset-frontend/packages/superset-ui-core/src/query/types/Query.ts:
##########
@@ -165,4 +165,154 @@ export interface QueryContext {
   form_data?: QueryFormData;
 }
 
+export const ErrorTypeEnum = {
+  // Frontend errors
+  FRONTEND_CSRF_ERROR: 'FRONTEND_CSRF_ERROR',
+  FRONTEND_NETWORK_ERROR: 'FRONTEND_NETWORK_ERROR',
+  FRONTEND_TIMEOUT_ERROR: 'FRONTEND_TIMEOUT_ERROR',
+
+  // DB Engine errors
+  GENERIC_DB_ENGINE_ERROR: 'GENERIC_DB_ENGINE_ERROR',
+  COLUMN_DOES_NOT_EXIST_ERROR: 'COLUMN_DOES_NOT_EXIST_ERROR',
+  TABLE_DOES_NOT_EXIST_ERROR: 'TABLE_DOES_NOT_EXIST_ERROR',
+  SCHEMA_DOES_NOT_EXIST_ERROR: 'SCHEMA_DOES_NOT_EXIST_ERROR',
+  CONNECTION_INVALID_USERNAME_ERROR: 'CONNECTION_INVALID_USERNAME_ERROR',
+  CONNECTION_INVALID_PASSWORD_ERROR: 'CONNECTION_INVALID_PASSWORD_ERROR',
+  CONNECTION_INVALID_HOSTNAME_ERROR: 'CONNECTION_INVALID_HOSTNAME_ERROR',
+  CONNECTION_PORT_CLOSED_ERROR: 'CONNECTION_PORT_CLOSED_ERROR',
+  CONNECTION_INVALID_PORT_ERROR: 'CONNECTION_INVALID_PORT_ERROR',
+  CONNECTION_HOST_DOWN_ERROR: 'CONNECTION_HOST_DOWN_ERROR',
+  CONNECTION_ACCESS_DENIED_ERROR: 'CONNECTION_ACCESS_DENIED_ERROR',
+  CONNECTION_UNKNOWN_DATABASE_ERROR: 'CONNECTION_UNKNOWN_DATABASE_ERROR',
+  CONNECTION_DATABASE_PERMISSIONS_ERROR:
+    'CONNECTION_DATABASE_PERMISSIONS_ERROR',
+  CONNECTION_MISSING_PARAMETERS_ERRORS: 'CONNECTION_MISSING_PARAMETERS_ERRORS',
+  OBJECT_DOES_NOT_EXIST_ERROR: 'OBJECT_DOES_NOT_EXIST_ERROR',
+  SYNTAX_ERROR: 'SYNTAX_ERROR',
+
+  // Viz errors
+  VIZ_GET_DF_ERROR: 'VIZ_GET_DF_ERROR',
+  UNKNOWN_DATASOURCE_TYPE_ERROR: 'UNKNOWN_DATASOURCE_TYPE_ERROR',
+  FAILED_FETCHING_DATASOURCE_INFO_ERROR:
+    'FAILED_FETCHING_DATASOURCE_INFO_ERROR',
+
+  // Security access errors
+  TABLE_SECURITY_ACCESS_ERROR: 'TABLE_SECURITY_ACCESS_ERROR',
+  DATASOURCE_SECURITY_ACCESS_ERROR: 'DATASOURCE_SECURITY_ACCESS_ERROR',
+  DATABASE_SECURITY_ACCESS_ERROR: 'DATABASE_SECURITY_ACCESS_ERROR',
+  QUERY_SECURITY_ACCESS_ERROR: 'QUERY_SECURITY_ACCESS_ERROR',
+  MISSING_OWNERSHIP_ERROR: 'MISSING_OWNERSHIP_ERROR',
+
+  // Other errors
+  BACKEND_TIMEOUT_ERROR: 'BACKEND_TIMEOUT_ERROR',
+  DATABASE_NOT_FOUND_ERROR: 'DATABASE_NOT_FOUND_ERROR',
+
+  // Sqllab error
+  MISSING_TEMPLATE_PARAMS_ERROR: 'MISSING_TEMPLATE_PARAMS_ERROR',
+  INVALID_TEMPLATE_PARAMS_ERROR: 'INVALID_TEMPLATE_PARAMS_ERROR',
+  RESULTS_BACKEND_NOT_CONFIGURED_ERROR: 'RESULTS_BACKEND_NOT_CONFIGURED_ERROR',
+  DML_NOT_ALLOWED_ERROR: 'DML_NOT_ALLOWED_ERROR',
+  INVALID_CTAS_QUERY_ERROR: 'INVALID_CTAS_QUERY_ERROR',
+  INVALID_CVAS_QUERY_ERROR: 'INVALID_CVAS_QUERY_ERROR',
+  SQLLAB_TIMEOUT_ERROR: 'SQLLAB_TIMEOUT_ERROR',
+  RESULTS_BACKEND_ERROR: 'RESULTS_BACKEND_ERROR',
+  ASYNC_WORKERS_ERROR: 'ASYNC_WORKERS_ERROR',
+
+  // Generic errors
+  GENERIC_COMMAND_ERROR: 'GENERIC_COMMAND_ERROR',
+  GENERIC_BACKEND_ERROR: 'GENERIC_BACKEND_ERROR',
+
+  // API errors
+  INVALID_PAYLOAD_FORMAT_ERROR: 'INVALID_PAYLOAD_FORMAT_ERROR',
+  INVALID_PAYLOAD_SCHEMA_ERROR: 'INVALID_PAYLOAD_SCHEMA_ERROR',
+} as const;
+
+type ValueOf<T> = T[keyof T];
+
+export type ErrorType = ValueOf<typeof ErrorTypeEnum>;
+
+// Keep in sync with superset/views/errors.py
+export type ErrorLevel = 'info' | 'warning' | 'error';
+
+export type ErrorSource = 'dashboard' | 'explore' | 'sqllab';
+
+export type SupersetError<ExtraType = Record<string, any> | null> = {
+  error_type: ErrorType;
+  extra: ExtraType;
+  level: ErrorLevel;
+  message: string;
+};
+
+export const CtasEnum = {
+  TABLE: 'TABLE',
+  VIEW: 'VIEW',
+};
+
+export type QueryColumn = {
+  name: string;
+  type: string | null;
+  is_dttm: boolean;
+};
+
+export type QueryState =
+  | 'stopped'
+  | 'failed'
+  | 'pending'
+  | 'running'
+  | 'scheduled'
+  | 'success'
+  | 'fetching'
+  | 'timed_out';
+
+export type Query = {

Review Comment:
   can we remove the other reference for `Query` inside sqllab and replace it with this one?
   
   https://github.com/apache/superset/blob/master/superset-frontend/src/SqlLab/types.ts#L41



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] hughhhh commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860197187


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   @eschutho shouldn't the model look like this since sl_dataset it's own entity
   ```
   sl_datasource?: {
     sl_dataset: any;
     sl_table?: any;
     query?: any;
     saved_query?: any;
   };
   ```
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860242462


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   I think these values would actually be string options of `type`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860242462


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   I think these values would actually be string options or enum of `type`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863283026


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -74,96 +79,290 @@ const Styles = styled.div`
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,
+  user,
+  query,
+  actions,
+}) => {
+  const getDefaultDatasetName = () =>
+    `${query?.tab || datasource?.sl_dataset?.query.tab} ${moment().format(
+      'MM/DD/YYYY HH:mm:ss',
+    )}`;
+
+  const [newSaveDatasetName, setNewSaveDatasetName] = useState(
+    getDefaultDatasetName(),
+  );
+  const [saveDatasetRadioBtnState, setSaveDatasetRadioBtnState] = useState(
+    DatasetRadioState.SAVE_NEW,
+  );
+  const [shouldOverwriteDataset, setShouldOverwriteDataset] = useState(false);
+  const [userDatasetOptions, setUserDatasetOptions] = useState<
+    DatasetOptionAutocomplete[]
+  >([]);
+  const [datasetToOverwrite, setDatasetToOverwrite] = useState<
+    Record<string, any>
+  >({});
+  const [saveModalAutocompleteValue] = useState('');

Review Comment:
   this one can't be set?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863280965


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -74,96 +79,290 @@ const Styles = styled.div`
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,
+  user,
+  query,
+  actions,
+}) => {
+  const getDefaultDatasetName = () =>
+    `${query?.tab || datasource?.sl_dataset?.query.tab} ${moment().format(
+      'MM/DD/YYYY HH:mm:ss',
+    )}`;
+
+  const [newSaveDatasetName, setNewSaveDatasetName] = useState(
+    getDefaultDatasetName(),
+  );
+  const [saveDatasetRadioBtnState, setSaveDatasetRadioBtnState] = useState(

Review Comment:
   nit, but you don't need the word `State` in the naming here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863278914


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,38 +17,43 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+} from '@superset-ui/core';
+import moment from 'moment';
+import rison from 'rison';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+} from 'src/SqlLab/types';
+import { DatasourceMeta } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource?: DatasourceMeta;
+  user: {
+    userId: number;
+  };
+  query?: Query;
+  actions?: Record<string, any>;

Review Comment:
   import them directly instead of props?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863277894


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -500,48 +228,28 @@ export default class ResultSet extends React.PureComponent<
       }
       const { columns } = this.props.query.results;
       // Added compute logic to stop user from being able to Save & Explore
-      const {
-        saveDatasetRadioBtnState,
-        newSaveDatasetName,
-        datasetToOverwrite,
-        saveModalAutocompleteValue,
-        shouldOverwriteDataSet,
-        userDatasetOptions,
-        showSaveDatasetModal,
-      } = this.state;
-      const disableSaveAndExploreBtn =
-        (saveDatasetRadioBtnState === DatasetRadioState.SAVE_NEW &&
-          newSaveDatasetName.length === 0) ||
-        (saveDatasetRadioBtnState === DatasetRadioState.OVERWRITE_DATASET &&
-          Object.keys(datasetToOverwrite).length === 0 &&
-          saveModalAutocompleteValue.length === 0);
+      const { showSaveDatasetModal } = this.state;
 
       return (
         <ResultSetControls>
           <SaveDatasetModal
             visible={showSaveDatasetModal}
-            onOk={this.handleSaveInDataset}
-            saveDatasetRadioBtnState={saveDatasetRadioBtnState}
-            shouldOverwriteDataset={shouldOverwriteDataSet}
-            defaultCreateDatasetValue={newSaveDatasetName}
-            userDatasetOptions={userDatasetOptions}
-            disableSaveAndExploreBtn={disableSaveAndExploreBtn}
-            onHide={this.handleHideSaveModal}
-            handleDatasetNameChange={this.handleDatasetNameChange}
-            handleSaveDatasetRadioBtnState={this.handleSaveDatasetRadioBtnState}
-            handleOverwriteCancel={this.handleOverwriteCancel}
-            handleOverwriteDataset={this.handleOverwriteDataset}
-            handleOverwriteDatasetOption={this.handleOverwriteDatasetOption}
-            handleSaveDatasetModalSearch={this.handleSaveDatasetModalSearch}
-            filterAutocompleteOption={this.handleFilterAutocompleteOption}
-            onChangeAutoComplete={this.handleOnChangeAutoComplete}
+            onHide={() => this.setState({ showSaveDatasetModal: false })}
+            buttonTextOnSave={t('Save & Explore')}
+            buttonTextOnOverwrite={t('Overwrite & Explore')}
+            modalDescription={t(
+              'Save this query as a virtual dataset to continue exploring',
+            )}
+            user={this.props.user}
+            query={this.props.query}
+            actions={this.props.actions}

Review Comment:
   can we import the actions we need directly into the savedatasetmodal?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r863994620


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -74,96 +79,290 @@ const Styles = styled.div`
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,
+  user,
+  query,
+  actions,
+}) => {
+  const getDefaultDatasetName = () =>
+    `${query?.tab || datasource?.sl_dataset?.query.tab} ${moment().format(

Review Comment:
   sl_dataset won't be on the datasource object.. let's talk about this one.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] AAfghahi commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r875110207


##########
superset-frontend/src/SqlLab/types.ts:
##########
@@ -128,3 +130,70 @@ export type RootState = {
   messageToasts: toastState[];
   common: {};
 };
+
+export type ExploreRootState = {

Review Comment:
   Would it make sense to move this into where the rest of the explore types are and then import it in?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r875099754


##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,153 +17,362 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+  // DatasourceType,
+} from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import moment from 'moment';
+import rison from 'rison';
+import { createDatasource } from 'src/SqlLab/actions/sqlLab';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+  SqlLabExploreRootState,
+  getInitialState,
+} from 'src/SqlLab/types';
+// import { Dataset } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
+
+// type ExploreDatasource = Dataset | Query;
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource: Query;
 }
 
 const Styles = styled.div`
-  .smd-body {
+  .sdm-body {
     margin: 0 8px;
   }
-  .smd-input {
+  .sdm-input {
     margin-left: 45px;
     width: 401px;
   }
-  .smd-autocomplete {
+  .sdm-autocomplete {
     margin-left: 8px;
     width: 401px;
   }
-  .smd-radio {
+  .sdm-radio {
     display: block;
     height: 30px;
     margin: 10px 0px;
     line-height: 30px;
   }
-  .smd-overwrite-msg {
+  .sdm-overwrite-msg {
     margin: 7px;
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
+// const getDatasourceState = (datasource: ExploreDatasource) => {
+//   if (datasource.type === DatasourceType.Dataset) return datasource as Dataset;
+//   return datasource as Query;
+// };
+
 // eslint-disable-next-line no-empty-pattern
 export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
   visible,
-  onOk,
   onHide,
-  handleDatasetNameChange,
-  handleSaveDatasetRadioBtnState,
-  saveDatasetRadioBtnState,
-  shouldOverwriteDataset,
-  handleOverwriteCancel,
-  handleOverwriteDataset,
-  handleOverwriteDatasetOption,
-  defaultCreateDatasetValue,
-  disableSaveAndExploreBtn,
-  handleSaveDatasetModalSearch,
-  filterAutocompleteOption,
-  userDatasetOptions,
-  onChangeAutoComplete,
-}) => (
-  <StyledModal
-    show={visible}
-    title="Save or Overwrite Dataset"
-    onHide={onHide}
-    footer={
-      <>
-        {!shouldOverwriteDataset && (
-          <Button
-            disabled={disableSaveAndExploreBtn}
-            buttonStyle="primary"
-            onClick={onOk}
-          >
-            {t('Save & Explore')}
-          </Button>
-        )}
-        {shouldOverwriteDataset && (
-          <>
-            <Button onClick={handleOverwriteCancel}>Back</Button>
+  buttonTextOnSave,
+  buttonTextOnOverwrite,
+  modalDescription,
+  datasource,
+}) => {
+  const getDefaultDatasetName = () =>
+    `${datasource.tab} ${moment().format('MM/DD/YYYY HH:mm:ss')}`;
+  const [datasetName, setDatasetName] = useState(getDefaultDatasetName());
+  const [newOrOverwrite, setNewOrOverwrite] = useState(
+    DatasetRadioState.SAVE_NEW,
+  );
+  const [shouldOverwriteDataset, setShouldOverwriteDataset] = useState(false);
+  const [userDatasetOptions, setUserDatasetOptions] = useState<
+    DatasetOptionAutocomplete[]
+  >([]);
+  const [datasetToOverwrite, setDatasetToOverwrite] = useState<
+    Record<string, any>
+  >({});
+  const [autocompleteValue, setAutocompleteValue] = useState('');
+
+  const user = useSelector<SqlLabExploreRootState, User>(state =>
+    getInitialState(state),

Review Comment:
   Good call, I clarified the parameter name in [`this commit`](https://github.com/apache/superset/pull/19855/commits/419fbb7b6de02e6a5cc0efeb16d840861b165fad).



##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -17,153 +17,362 @@
  * under the License.
  */
 
-import React, { FunctionComponent } from 'react';
-import { AutoCompleteProps } from 'antd/lib/auto-complete';
+import React, { FunctionComponent, useState } from 'react';
 import { Radio } from 'src/components/Radio';
 import { AutoComplete, RadioChangeEvent } from 'src/components';
 import { Input } from 'src/components/Input';
 import StyledModal from 'src/components/Modal';
 import Button from 'src/components/Button';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  makeApi,
+  JsonResponse,
+  // DatasourceType,
+} from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import moment from 'moment';
+import rison from 'rison';
+import { createDatasource } from 'src/SqlLab/actions/sqlLab';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  Query,
+  SqlLabExploreRootState,
+  getInitialState,
+} from 'src/SqlLab/types';
+// import { Dataset } from '@superset-ui/chart-controls';
+import { exploreChart } from 'src/explore/exploreUtils';
+
+// type ExploreDatasource = Dataset | Query;
 
 interface SaveDatasetModalProps {
   visible: boolean;
-  onOk: () => void;
   onHide: () => void;
-  handleDatasetNameChange: (e: React.FormEvent<HTMLInputElement>) => void;
-  handleSaveDatasetModalSearch: (searchText: string) => Promise<void>;
-  filterAutocompleteOption: (
-    inputValue: string,
-    option: { value: string; datasetId: number },
-  ) => boolean;
-  handleSaveDatasetRadioBtnState: (e: RadioChangeEvent) => void;
-  handleOverwriteCancel: () => void;
-  handleOverwriteDataset: () => void;
-  handleOverwriteDatasetOption: (
-    data: string,
-    option: Record<string, any>,
-  ) => void;
-  onChangeAutoComplete: () => void;
-  defaultCreateDatasetValue: string;
-  disableSaveAndExploreBtn: boolean;
-  saveDatasetRadioBtnState: number;
-  shouldOverwriteDataset: boolean;
-  userDatasetOptions: AutoCompleteProps['options'];
+  buttonTextOnSave: string;
+  buttonTextOnOverwrite: string;
+  modalDescription?: string;
+  datasource: Query;
 }
 
 const Styles = styled.div`
-  .smd-body {
+  .sdm-body {
     margin: 0 8px;
   }
-  .smd-input {
+  .sdm-input {
     margin-left: 45px;
     width: 401px;
   }
-  .smd-autocomplete {
+  .sdm-autocomplete {
     margin-left: 8px;
     width: 401px;
   }
-  .smd-radio {
+  .sdm-radio {
     display: block;
     height: 30px;
     margin: 10px 0px;
     line-height: 30px;
   }
-  .smd-overwrite-msg {
+  .sdm-overwrite-msg {
     margin: 7px;
   }
 `;
 
+const updateDataset = async (
+  dbId: number,
+  datasetId: number,
+  sql: string,
+  columns: Array<Record<string, any>>,
+  owners: [number],
+  overrideColumns: boolean,
+) => {
+  const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
+  const headers = { 'Content-Type': 'application/json' };
+  const body = JSON.stringify({
+    sql,
+    columns,
+    owners,
+    database_id: dbId,
+  });
+
+  const data: JsonResponse = await SupersetClient.put({
+    endpoint,
+    headers,
+    body,
+  });
+  return data.json.result;
+};
+
+// const getDatasourceState = (datasource: ExploreDatasource) => {
+//   if (datasource.type === DatasourceType.Dataset) return datasource as Dataset;
+//   return datasource as Query;
+// };

Review Comment:
   Removed! 😁 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r861047782


##########
superset-frontend/packages/superset-ui-chart-controls/src/types.ts:
##########
@@ -65,6 +65,11 @@ export interface DatasourceMeta {
   granularity_sqla?: string;
   datasource_name: string | null;
   description: string | null;
+  sl_dataset?: {
+    sl_table?: any;

Review Comment:
   On a query, for example, the data structure will look like `datasource.query` but there won't be a query on a table, so we should define them separately.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] yousoph commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
yousoph commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110299959

   3. When opening the save/overwrite modal from Explore, the button text should be "Save" rather than "Save & Explore" 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110425638

   @lyndsiWilliams Ephemeral environment spinning up at http://52.88.44.27:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1110431064

   @jess-dillard @yousoph Thanks for the feedback! I implemented the changes in [`this commit`](https://github.com/apache/superset/pull/19855/commits/abdb6cbe45ec03b228111df4e7ae17cc31e4370d). Please let me know how it looks and if any other changes are needed 😁 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860313396


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -190,6 +217,35 @@ export default function DataSourcePanel({
     [_columns],
   );
 
+  const placeholderSlDataset = {
+    sl_table: [],
+    query: [],
+    saved_query: [],
+  };
+
+  // eslint-disable-next-line no-param-reassign
+  datasource.sl_dataset = placeholderSlDataset;

Review Comment:
   This was the hard coded placeholder that I discussed with you the other day in Slack, but I ended up removing it in [`this commit`](https://github.com/apache/superset/pull/19855/commits/fd65bfa6f6241c466fa4eac90efafcc87f5c95cd) because the example still works. Should I keep it as is or hard code a placeholder again?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] hughhhh commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r860138295


##########
superset-frontend/src/explore/components/DatasourcePanel/index.tsx:
##########
@@ -26,6 +26,19 @@ import {
 import { debounce } from 'lodash';
 import { matchSorter, rankings } from 'match-sorter';
 import Collapse from 'src/components/Collapse';
+import Alert from 'src/components/Alert';
+import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
+import { exploreChart } from 'src/explore/exploreUtils';
+import moment from 'moment';
+import rison from 'rison';
+import {
+  DatasetRadioState,
+  EXPLORE_CHART_DEFAULT,
+  DatasetOwner,
+  DatasetOptionAutocomplete,
+  updateDataset,
+} from 'src/SqlLab/components/ResultSet';

Review Comment:
   we should centralize this in a types file now instead of pulling it from `ResultSet`
   
   Can you move this to `superset/superset-frontend/src/SqlLab/types.ts` and import from there



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r889566062


##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/savedMetricsTypeCheck.ts:
##########
@@ -0,0 +1,30 @@
+/* eslint-disable camelcase */
+/**
+ * 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 { QueryResponse } from '@superset-ui/core';
+import { Dataset } from '../types';
+import { DEFAULT_METRICS } from '..';
+
+export const savedMetricsTypeCheck = (

Review Comment:
   Good idea! I changed it to `defineSavedMetrics` in [`this commit`](https://github.com/apache/superset/pull/19855/commits/cea65ce3bf31f7565bf0ddb75cebffbcb89ac1e2).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r876976553


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -493,48 +221,26 @@ export default class ResultSet extends React.PureComponent<
       }
       const { columns } = this.props.query.results;
       // Added compute logic to stop user from being able to Save & Explore
-      const {
-        saveDatasetRadioBtnState,
-        newSaveDatasetName,
-        datasetToOverwrite,
-        saveModalAutocompleteValue,
-        shouldOverwriteDataSet,
-        userDatasetOptions,
-        showSaveDatasetModal,
-      } = this.state;
-      const disableSaveAndExploreBtn =
-        (saveDatasetRadioBtnState === DatasetRadioState.SAVE_NEW &&
-          newSaveDatasetName.length === 0) ||
-        (saveDatasetRadioBtnState === DatasetRadioState.OVERWRITE_DATASET &&
-          Object.keys(datasetToOverwrite).length === 0 &&
-          saveModalAutocompleteValue.length === 0);
+      const { showSaveDatasetModal } = this.state;
 
       return (
         <ResultSetControls>
           <SaveDatasetModal
             visible={showSaveDatasetModal}
-            onOk={this.handleSaveInDataset}
-            saveDatasetRadioBtnState={saveDatasetRadioBtnState}
-            shouldOverwriteDataset={shouldOverwriteDataSet}
-            defaultCreateDatasetValue={newSaveDatasetName}
-            userDatasetOptions={userDatasetOptions}
-            disableSaveAndExploreBtn={disableSaveAndExploreBtn}
-            onHide={this.handleHideSaveModal}
-            handleDatasetNameChange={this.handleDatasetNameChange}
-            handleSaveDatasetRadioBtnState={this.handleSaveDatasetRadioBtnState}
-            handleOverwriteCancel={this.handleOverwriteCancel}
-            handleOverwriteDataset={this.handleOverwriteDataset}
-            handleOverwriteDatasetOption={this.handleOverwriteDatasetOption}
-            handleSaveDatasetModalSearch={this.handleSaveDatasetModalSearch}
-            filterAutocompleteOption={this.handleFilterAutocompleteOption}
-            onChangeAutoComplete={this.handleOnChangeAutoComplete}
+            onHide={() => this.setState({ showSaveDatasetModal: false })}
+            buttonTextOnSave={t('Save & Explore')}
+            buttonTextOnOverwrite={t('Overwrite & Explore')}
+            modalDescription={t(
+              'Save this query as a virtual dataset to continue exploring',
+            )}
+            datasource={this.props.query}

Review Comment:
   Done in [`this commit`](https://github.com/apache/superset/pull/19855/commits/a586d93a8c16741c4d120e63639fa94a4bb4fe66).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19855:
URL: https://github.com/apache/superset/pull/19855#issuecomment-1134942346

   @lyndsiWilliams Ephemeral environment spinning up at http://18.236.255.116:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r879954005


##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/columnChoices.ts:
##########
@@ -16,13 +16,13 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { DatasourceMeta } from '../types';
+import { Dataset } from '../types';
 
 /**
  * Convert Datasource columns to column choices
  */
 export default function columnChoices(
-  datasource?: DatasourceMeta | null,
+  datasource?: Dataset | null,

Review Comment:
   could be Query in the future



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r879955406


##########
superset-frontend/packages/superset-ui-core/src/query/types/Query.ts:
##########
@@ -165,4 +165,154 @@ export interface QueryContext {
   form_data?: QueryFormData;
 }
 
+export const ErrorTypeEnum = {
+  // Frontend errors
+  FRONTEND_CSRF_ERROR: 'FRONTEND_CSRF_ERROR',

Review Comment:
   where did these come from?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r879960394


##########
superset-frontend/packages/superset-ui-core/src/query/types/Query.ts:
##########
@@ -165,4 +165,154 @@ export interface QueryContext {
   form_data?: QueryFormData;
 }
 
+export const ErrorTypeEnum = {
+  // Frontend errors
+  FRONTEND_CSRF_ERROR: 'FRONTEND_CSRF_ERROR',

Review Comment:
   oh I see.. what happened when you tried importing the Query type from src/SqlLab/types?
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r883172351


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -128,12 +137,16 @@ export const dnd_sort_by: SharedControlConfig<'DndMetricSelect'> = {
     'Metric used to define how the top series are sorted if a series or row limit is present. ' +
       'If undefined reverts to the first metric (where appropriate).',
   ),
-  mapStateToProps: ({ datasource }) => ({
-    columns: datasource?.columns || [],
-    savedMetrics: datasource?.metrics || [],
-    datasource,
-    datasourceType: datasource?.type,
-  }),
+  mapStateToProps: ({ datasource }) => {
+    const dataset = datasource as Dataset;
+
+    return {
+      columns: dataset?.columns || [],

Review Comment:
   this can all be on datasource



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #19855: feat(explore): Frontend implementation of dataset creation from infobox

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19855:
URL: https://github.com/apache/superset/pull/19855#discussion_r883172008


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -36,15 +36,16 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
   ),
   mapStateToProps(state, { includeTime }) {
     const newState: ExtraControlProps = {};
+    const dataset = state.datasource as Dataset;
     if (state.datasource) {
-      const options = state.datasource.columns.filter(c => c.groupby);
+      const options = dataset.columns.filter(c => c.groupby);

Review Comment:
   you'll want to switch the type here if columns has a groupby property



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org