lyndsiWilliams commented on a change in pull request #18593:
URL: https://github.com/apache/superset/pull/18593#discussion_r806459799
##########
File path: superset-frontend/src/SqlLab/components/ResultSet/index.tsx
##########
@@ -173,106 +160,93 @@ const updateDataset = async (
return data.json.result;
};
-export default class ResultSet extends React.PureComponent<
- ResultSetProps,
- ResultSetState
-> {
- static defaultProps = {
- cache: false,
- csv: true,
- database: {},
- search: true,
- showSql: false,
- visualize: true,
+const ResultSet = ({
+ actions,
+ cache = false,
+ csv = true,
+ database = {},
+ displayLimit,
+ height,
+ query,
+ search = true,
+ showSql = false,
+ visualize = true,
+ user,
+ defaultQueryLimit,
+}: ResultSetProps) => {
+ const [searchText, setSearchText] = useState('');
+ const [cachedData, setCachedData] = useState<Record<string, unknown>[]>([]);
+ const [showSaveDatasetModal, setShowSaveDatasetModal] = useState(false);
+ const [saveDatasetRadioBtnState, setSaveDatasetRadioBtnState] = useState(
+ DatasetRadioState.SAVE_NEW,
+ );
+ const [shouldOverwriteDataSet, setShouldOverwriteDataSet] = useState(false);
+ const [datasetToOverwrite, setDatasetToOverwrite] = useState<
+ Record<string, any>
+ >({});
+ const [userDatasetOptions, setUserDatasetOptions] = useState<
+ DatasetOptionAutocomplete[]
+ >([]);
+ const [alertIsOpen, setAlertIsOpen] = useState(false);
+
+ const reRunQueryIfSessionTimeoutErrorOnMount = () => {
+ if (
+ query.errorMessage &&
+ query.errorMessage.indexOf('session timed out') > 0
+ ) {
+ actions.reRunQuery(query);
+ }
};
- constructor(props: ResultSetProps) {
- super(props);
- this.state = {
- searchText: '',
- showExploreResultsButton: false,
- data: [],
- showSaveDatasetModal: false,
- newSaveDatasetName: this.getDefaultDatasetName(),
- saveDatasetRadioBtnState: DatasetRadioState.SAVE_NEW,
- shouldOverwriteDataSet: false,
- datasetToOverwrite: {},
- saveModalAutocompleteValue: '',
- userDatasetOptions: [],
- alertIsOpen: false,
- };
- this.changeSearch = this.changeSearch.bind(this);
- this.fetchResults = this.fetchResults.bind(this);
- this.popSelectStar = this.popSelectStar.bind(this);
- this.reFetchQueryResults = this.reFetchQueryResults.bind(this);
- this.toggleExploreResultsButton =
- this.toggleExploreResultsButton.bind(this);
- this.handleSaveInDataset = this.handleSaveInDataset.bind(this);
- this.handleHideSaveModal = this.handleHideSaveModal.bind(this);
- this.handleDatasetNameChange = this.handleDatasetNameChange.bind(this);
- this.handleSaveDatasetRadioBtnState =
- this.handleSaveDatasetRadioBtnState.bind(this);
- this.handleOverwriteCancel = this.handleOverwriteCancel.bind(this);
- this.handleOverwriteDataset = this.handleOverwriteDataset.bind(this);
- this.handleOverwriteDatasetOption =
- this.handleOverwriteDatasetOption.bind(this);
- this.handleSaveDatasetModalSearch = debounce(
- this.handleSaveDatasetModalSearch.bind(this),
- 1000,
- );
- this.handleFilterAutocompleteOption =
- this.handleFilterAutocompleteOption.bind(this);
- this.handleOnChangeAutoComplete =
- this.handleOnChangeAutoComplete.bind(this);
- this.handleExploreBtnClick = this.handleExploreBtnClick.bind(this);
- }
-
- async componentDidMount() {
+ useEffect(() => {
// only do this the first time the component is rendered/mounted
- this.reRunQueryIfSessionTimeoutErrorOnMount();
- const userDatasetsOwned = await this.getUserDatasets();
- this.setState({ userDatasetOptions: userDatasetsOwned });
- }
-
- UNSAFE_componentWillReceiveProps(nextProps: ResultSetProps) {
- // when new results comes in, save them locally and clear in store
+ reRunQueryIfSessionTimeoutErrorOnMount();
+ (async () => {
+ const userDatasetsOwned = await getUserDatasets();
+ setUserDatasetOptions(userDatasetsOwned);
+ })();
+ }, []);
+
+ const prevQuery = usePrevious(query);
+ useEffect(() => {
if (
- this.props.cache &&
- !nextProps.query.cached &&
- nextProps.query.results &&
- nextProps.query.results.data &&
- nextProps.query.results.data.length > 0
+ cache &&
+ !prevQuery?.cached &&
+ prevQuery?.results?.data &&
+ prevQuery.results.data.length > 0
) {
- this.setState({ data: nextProps.query.results.data }, () =>
- this.clearQueryResults(nextProps.query),
- );
+ // this.setState({ data: prevQuery.results.data }, () =>
+ // clearQueryResults(prevQuery),
+ // );
+ setCachedData(prevQuery.results.data);
+ actions.clearQueryResults(prevQuery);
Review comment:
Are you referring to setting the state on lines 218-220? You might be
able to use the
[useCallback](https://reactjs.org/docs/hooks-reference.html#usecallback) hook
for this.
##########
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:
What error was it giving you?
##########
File path: superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.jsx
##########
@@ -105,51 +105,51 @@ test('renders a Table', () => {
expect(wrapper.find(FilterableTable)).toExist();
});
-describe('componentDidMount', () => {
Review comment:
You can test the functionality that was done within these tests, they
were converted to functional but still live on these lines:
https://github.com/apache/superset/blob/4ec10c23014cb9e5fe2cedf6407bb711cf05dbb8/superset-frontend/src/SqlLab/components/ResultSet/index.tsx#L192-L227
##########
File path: superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.jsx
##########
@@ -177,7 +179,7 @@ test('should render empty results', () => {
test('should render cached query', () => {
const wrapper = shallow(<ResultSet {...cachedQueryProps} />);
const cachedData = [{ col1: 'a', col2: 'b' }];
- wrapper.setState({ data: cachedData });
+ wrapper.setState({ data: cachedData }); // How to set state to Functional
Component?
Review comment:
I believe this is setting the `query.results.data` as seen in `newProps`
here:
https://github.com/apache/superset/blob/f62aa7b1b8cddd6f2655dbb657b730b5e27778df/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.jsx#L88-L96
so you'll need to change the render's props in RTL to reflect that.
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
