[GitHub] [superset] AAfghahi commented on a change in pull request #18593: refactor: converted ResultSet to functional component

[GitBox <gi...@apache.org>]

AAfghahi commented on a change in pull request #18593:
URL: https://github.com/apache/superset/pull/18593#discussion_r800782272



##########
File path: superset-frontend/src/SqlLab/components/ResultSet/index.tsx
##########
@@ -100,20 +100,6 @@ interface ResultSetProps {
   defaultQueryLimit: number;
 }
 
-interface ResultSetState {

Review comment:
       Hey, why was this interface deleted?

##########
File path: superset-frontend/src/SqlLab/components/ResultSet/index.tsx
##########
@@ -428,76 +390,53 @@ export default class ResultSet extends React.PureComponent<
     return null;
   };
 
-  handleSaveDatasetModalSearch = async (searchText: string) => {
-    const userDatasetsOwned = await this.getUserDatasets(searchText);
-    this.setState({ userDatasetOptions: userDatasetsOwned });
+  const handleSaveDatasetModalSearch = async (searchText: string) => {
+    const userDatasetsOwned = await getUserDatasets(searchText);
+    setUserDatasetOptions(userDatasetsOwned);
   };
 
-  handleFilterAutocompleteOption = (
+  const handleSaveDatasetModalSearchWithDebounce = debounce(
+    handleSaveDatasetModalSearch,
+    1000,
+  );
+
+  const handleFilterAutocompleteOption = (
     inputValue: string,
     option: { value: string; datasetId: number },
   ) => option.value.toLowerCase().includes(inputValue.toLowerCase());
 
-  clearQueryResults(query: Query) {
-    this.props.actions.clearQueryResults(query);
-  }
-
-  popSelectStar(tempSchema: string | null, tempTable: string) {
+  const popSelectStar = (tempSchema: string | null, tempTable: string) => {
     const qe = {
       id: shortid.generate(),
       title: tempTable,
       autorun: false,
-      dbId: this.props.query.dbId,
+      dbId: query.dbId,
       sql: `SELECT * FROM ${tempSchema ? `${tempSchema}.` : ''}${tempTable}`,
     };
-    this.props.actions.addQueryEditor(qe);
-  }
-
-  toggleExploreResultsButton() {
-    this.setState(prevState => ({
-      showExploreResultsButton: !prevState.showExploreResultsButton,
-    }));
-  }
-
-  changeSearch(event: React.ChangeEvent<HTMLInputElement>) {
-    this.setState({ searchText: event.target.value });
-  }
+    actions.addQueryEditor(qe);
+  };
 
-  fetchResults(query: Query) {
-    this.props.actions.fetchQueryResults(query, this.props.displayLimit);
-  }
+  const changeSearch = (event: React.ChangeEvent<HTMLInputElement>) => {
+    setSearchText(event.target.value);
+  };
 
-  reFetchQueryResults(query: Query) {
-    this.props.actions.reFetchQueryResults(query);
-  }
+  const fetchResults = (query: Query) => {
+    actions.fetchQueryResults(query, displayLimit);
+  };
 
-  reRunQueryIfSessionTimeoutErrorOnMount() {
-    const { query } = this.props;
-    if (
-      query.errorMessage &&
-      query.errorMessage.indexOf('session timed out') > 0
-    ) {
-      this.props.actions.reRunQuery(query);
-    }
-  }
+  const reFetchQueryResults = (query: Query) => {
+    actions.reFetchQueryResults(query);
+  };
 
-  renderControls() {
-    if (this.props.search || this.props.visualize || this.props.csv) {
-      let { data } = this.props.query.results;
-      if (this.props.cache && this.props.query.cached) {
-        ({ data } = this.state);
+  const renderControls = () => {
+    const saveModalAutocompleteValue = '';

Review comment:
       It looks like this value should be pulled from state, so you might have to create a useState in the beginning of the function for it. 

##########
File path: superset-frontend/src/SqlLab/components/ResultSet/index.tsx
##########
@@ -671,180 +607,172 @@ export default class ResultSet extends React.PureComponent<
         )}
       </ReturnedRows>
     );
+  };
+
+  let sql;

Review comment:
       hmm i would have to think about this more, but I wonder if this being in a function is better practice. Either you can put it in the return. Which feels clunky, or maybe create another function for it. I'd have to think through what the differences are. 




-- 
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