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/08/24 22:53:44 UTC
[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #21186: chore: refactor ResultSet to functional component
lyndsiWilliams commented on code in PR #21186:
URL: https://github.com/apache/superset/pull/21186#discussion_r954349282
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -70,14 +70,6 @@ interface ResultSetProps {
defaultQueryLimit: number;
}
-interface ResultSetState {
- searchText: string;
- showExploreResultsButton: boolean;
- data: Record<string, any>[];
- showSaveDatasetModal: boolean;
- alertIsOpen: boolean;
-}
-
const ResultlessStyles = styled.div`
position: relative;
min-height: 100px;
Review Comment:
```suggestion
min-height: ${({ theme }) => theme.gridUnit * 25}px;
```
Unit values should stay within the Superset theme
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -121,115 +113,96 @@ const LimitMessage = styled.span`
margin-left: ${({ theme }) => theme.gridUnit * 2}px;
`;
-export default class ResultSet extends React.PureComponent<
- ResultSetProps,
- ResultSetState
-> {
- static defaultProps = {
- cache: false,
- csv: true,
- database: {},
- search: true,
- showSql: false,
- visualize: true,
- };
-
- constructor(props: ResultSetProps) {
- super(props);
- this.state = {
- searchText: '',
- showExploreResultsButton: false,
- data: [],
- showSaveDatasetModal: false,
- 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);
- }
-
- async componentDidMount() {
+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 [alertIsOpen, setAlertIsOpen] = useState(false);
+
+ useEffect(() => {
// only do this the first time the component is rendered/mounted
- this.reRunQueryIfSessionTimeoutErrorOnMount();
- }
+ reRunQueryIfSessionTimeoutErrorOnMount();
+ }, []);
- UNSAFE_componentWillReceiveProps(nextProps: ResultSetProps) {
- // when new results comes in, save them locally and clear in store
+ 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 &&
+ query.cached &&
+ query?.results?.data &&
+ query.results.data.length > 0
Review Comment:
```suggestion
query?.results?.data?.length > 0
```
These can be combined
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -121,115 +113,96 @@ const LimitMessage = styled.span`
margin-left: ${({ theme }) => theme.gridUnit * 2}px;
`;
-export default class ResultSet extends React.PureComponent<
- ResultSetProps,
- ResultSetState
-> {
- static defaultProps = {
- cache: false,
- csv: true,
- database: {},
- search: true,
- showSql: false,
- visualize: true,
- };
-
- constructor(props: ResultSetProps) {
- super(props);
- this.state = {
- searchText: '',
- showExploreResultsButton: false,
- data: [],
- showSaveDatasetModal: false,
- 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);
- }
-
- async componentDidMount() {
+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 [alertIsOpen, setAlertIsOpen] = useState(false);
+
+ useEffect(() => {
// only do this the first time the component is rendered/mounted
- this.reRunQueryIfSessionTimeoutErrorOnMount();
- }
+ reRunQueryIfSessionTimeoutErrorOnMount();
+ }, []);
- UNSAFE_componentWillReceiveProps(nextProps: ResultSetProps) {
- // when new results comes in, save them locally and clear in store
+ 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 &&
+ query.cached &&
+ query?.results?.data &&
+ query.results.data.length > 0
) {
- this.setState({ data: nextProps.query.results.data }, () =>
- this.clearQueryResults(nextProps.query),
- );
+ setCachedData(query.results.data);
+ clearQueryResults(query);
}
if (
- nextProps.query.resultsKey &&
- nextProps.query.resultsKey !== this.props.query.resultsKey
+ query.resultsKey &&
+ prevQuery?.resultsKey &&
+ query.resultsKey !== prevQuery.resultsKey
) {
- this.fetchResults(nextProps.query);
+ fetchResults(query);
}
- }
+ }, [query, cache]);
- calculateAlertRefHeight = (alertElement: HTMLElement | null) => {
+ const calculateAlertRefHeight = (alertElement: HTMLElement | null) => {
if (alertElement) {
- this.setState({ alertIsOpen: true });
+ setAlertIsOpen(true);
} else {
- this.setState({ alertIsOpen: false });
+ setAlertIsOpen(false);
}
};
- clearQueryResults(query: QueryResponse) {
- this.props.actions.clearQueryResults(query);
- }
+ const clearQueryResults = (query: QueryResponse) => {
+ actions.clearQueryResults(query);
+ };
- popSelectStar(tempSchema: string | null, tempTable: string) {
+ const popSelectStar = (tempSchema: string | null, tempTable: string) => {
const qe = {
id: shortid.generate(),
name: 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,
- }));
- }
+ actions.addQueryEditor(qe);
+ };
- changeSearch(event: React.ChangeEvent<HTMLInputElement>) {
- this.setState({ searchText: event.target.value });
- }
+ const changeSearch = (event: React.ChangeEvent<HTMLInputElement>) => {
+ setSearchText(event.target.value);
+ };
- fetchResults(query: QueryResponse) {
- this.props.actions.fetchQueryResults(query, this.props.displayLimit);
- }
+ const fetchResults = (query: QueryResponse) => {
+ actions.fetchQueryResults(query, displayLimit);
+ };
- reFetchQueryResults(query: QueryResponse) {
- this.props.actions.reFetchQueryResults(query);
- }
+ const reFetchQueryResults = (query: QueryResponse) => {
+ actions.reFetchQueryResults(query);
+ };
- reRunQueryIfSessionTimeoutErrorOnMount() {
- const { query } = this.props;
+ const reRunQueryIfSessionTimeoutErrorOnMount = () => {
if (
query.errorMessage &&
query.errorMessage.indexOf('session timed out') > 0
Review Comment:
```suggestion
query?.errorMessage?.indexOf('session timed out') > 0
```
These can be combined with optional chaining
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -414,193 +384,191 @@ export default class ResultSet extends React.PureComponent<
)}
</ReturnedRows>
);
+ };
+
+ const limitReached = query?.results?.displayLimitReached;
+ let sql;
+ let exploreDBId = query.dbId;
+ if (database?.explore_database_id) {
+ exploreDBId = database.explore_database_id;
}
- render() {
- const { query } = this.props;
- const limitReached = query?.results?.displayLimitReached;
- let sql;
- let exploreDBId = query.dbId;
- if (this.props.database && this.props.database.explore_database_id) {
- exploreDBId = this.props.database.explore_database_id;
- }
- let trackingUrl;
- if (
- query.trackingUrl &&
- query.state !== 'success' &&
- query.state !== 'fetching'
- ) {
- trackingUrl = (
- <Button
- className="sql-result-track-job"
- buttonSize="small"
- href={query.trackingUrl}
- target="_blank"
- >
- {query.state === 'running' ? t('Track job') : t('See query details')}
- </Button>
- );
- }
+ let trackingUrl;
+ if (
+ query.trackingUrl &&
+ query.state !== 'success' &&
+ query.state !== 'fetching'
+ ) {
+ trackingUrl = (
+ <Button
+ className="sql-result-track-job"
+ buttonSize="small"
+ href={query.trackingUrl}
+ target="_blank"
+ >
+ {query.state === 'running' ? t('Track job') : t('See query details')}
+ </Button>
+ );
+ }
- if (this.props.showSql) sql = <HighlightedSql sql={query.sql} />;
+ if (showSql) {
+ sql = <HighlightedSql sql={query.sql} />;
+ }
+
+ if (query.state === 'stopped') {
+ return <Alert type="warning" message={t('Query was stopped')} />;
+ }
- if (query.state === 'stopped') {
- return <Alert type="warning" message={t('Query was stopped')} />;
+ if (query.state === 'failed') {
+ return (
+ <ResultlessStyles>
+ <ErrorMessageWithStackTrace
+ title={t('Database error')}
+ error={query?.errors?.[0]}
+ subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
+ copyText={query.errorMessage || undefined}
+ link={query.link}
+ source="sqllab"
+ />
+ {trackingUrl}
+ </ResultlessStyles>
+ );
+ }
+
+ if (query.state === 'success' && query.ctas) {
+ const { tempSchema, tempTable } = query;
+ let object = 'Table';
+ if (query.ctas_method === CtasEnum.VIEW) {
+ object = 'View';
}
- if (query.state === 'failed') {
- return (
- <ResultlessStyles>
- <ErrorMessageWithStackTrace
- title={t('Database error')}
- error={query?.errors?.[0]}
- subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
- copyText={query.errorMessage || undefined}
- link={query.link}
- source="sqllab"
- />
- {trackingUrl}
- </ResultlessStyles>
- );
+ return (
+ <div>
+ <Alert
+ type="info"
+ message={
+ <>
+ {t(object)} [
+ <strong>
+ {tempSchema ? `${tempSchema}.` : ''}
+ {tempTable}
+ </strong>
+ ] {t('was created')}
+ <ButtonGroup>
+ <Button
+ buttonSize="small"
+ className="m-r-5"
+ onClick={() => popSelectStar(tempSchema, tempTable)}
+ >
+ {t('Query in a new tab')}
+ </Button>
+ <ExploreCtasResultsButton
+ // @ts-ignore Redux types are difficult to work with, ignoring for now
Review Comment:
Will this be addressed in a followup ticket soon after this is merged?
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -414,193 +384,191 @@ export default class ResultSet extends React.PureComponent<
)}
</ReturnedRows>
);
+ };
+
+ const limitReached = query?.results?.displayLimitReached;
+ let sql;
+ let exploreDBId = query.dbId;
+ if (database?.explore_database_id) {
+ exploreDBId = database.explore_database_id;
}
- render() {
- const { query } = this.props;
- const limitReached = query?.results?.displayLimitReached;
- let sql;
- let exploreDBId = query.dbId;
- if (this.props.database && this.props.database.explore_database_id) {
- exploreDBId = this.props.database.explore_database_id;
- }
- let trackingUrl;
- if (
- query.trackingUrl &&
- query.state !== 'success' &&
- query.state !== 'fetching'
- ) {
- trackingUrl = (
- <Button
- className="sql-result-track-job"
- buttonSize="small"
- href={query.trackingUrl}
- target="_blank"
- >
- {query.state === 'running' ? t('Track job') : t('See query details')}
- </Button>
- );
- }
+ let trackingUrl;
+ if (
+ query.trackingUrl &&
+ query.state !== 'success' &&
+ query.state !== 'fetching'
+ ) {
+ trackingUrl = (
+ <Button
+ className="sql-result-track-job"
+ buttonSize="small"
+ href={query.trackingUrl}
+ target="_blank"
+ >
+ {query.state === 'running' ? t('Track job') : t('See query details')}
+ </Button>
+ );
+ }
- if (this.props.showSql) sql = <HighlightedSql sql={query.sql} />;
+ if (showSql) {
+ sql = <HighlightedSql sql={query.sql} />;
+ }
+
+ if (query.state === 'stopped') {
+ return <Alert type="warning" message={t('Query was stopped')} />;
+ }
- if (query.state === 'stopped') {
- return <Alert type="warning" message={t('Query was stopped')} />;
+ if (query.state === 'failed') {
+ return (
+ <ResultlessStyles>
+ <ErrorMessageWithStackTrace
+ title={t('Database error')}
+ error={query?.errors?.[0]}
+ subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
+ copyText={query.errorMessage || undefined}
+ link={query.link}
+ source="sqllab"
+ />
+ {trackingUrl}
+ </ResultlessStyles>
+ );
+ }
+
+ if (query.state === 'success' && query.ctas) {
+ const { tempSchema, tempTable } = query;
+ let object = 'Table';
+ if (query.ctas_method === CtasEnum.VIEW) {
+ object = 'View';
}
- if (query.state === 'failed') {
- return (
- <ResultlessStyles>
- <ErrorMessageWithStackTrace
- title={t('Database error')}
- error={query?.errors?.[0]}
- subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
- copyText={query.errorMessage || undefined}
- link={query.link}
- source="sqllab"
- />
- {trackingUrl}
- </ResultlessStyles>
- );
+ return (
+ <div>
+ <Alert
+ type="info"
+ message={
+ <>
+ {t(object)} [
+ <strong>
+ {tempSchema ? `${tempSchema}.` : ''}
+ {tempTable}
+ </strong>
+ ] {t('was created')}
+ <ButtonGroup>
+ <Button
+ buttonSize="small"
+ className="m-r-5"
+ onClick={() => popSelectStar(tempSchema, tempTable)}
+ >
+ {t('Query in a new tab')}
+ </Button>
+ <ExploreCtasResultsButton
+ // @ts-ignore Redux types are difficult to work with, ignoring for now
+ actions={actions}
+ table={tempTable}
+ schema={tempSchema}
+ dbId={exploreDBId}
+ />
+ </ButtonGroup>
+ </>
+ }
+ />
+ </div>
+ );
+ }
+
+ if (query.state === 'success' && query.results) {
+ const { results } = query;
+ // Accounts for offset needed for height of ResultSetRowsReturned component if !limitReached
+ const rowMessageHeight = !limitReached ? 32 : 0;
+ // Accounts for offset needed for height of Alert if this.state.alertIsOpen
+ const alertContainerHeight = 70;
+ // We need to calculate the height of this.renderRowsReturned()
+ // if we want results panel to be proper height because the
+ // FilterTable component needs an explicit height to render
+ // react-virtualized Table component
+ const rowsHeight = alertIsOpen
+ ? height - alertContainerHeight
+ : height - rowMessageHeight;
+ let data;
+ if (cache && query.cached) {
+ data = cachedData;
+ } else if (results && results.data) {
+ ({ data } = results);
}
- if (query.state === 'success' && query.ctas) {
- const { tempSchema, tempTable } = query;
- let object = 'Table';
- if (query.ctas_method === CtasEnum.VIEW) {
- object = 'View';
- }
+ if (data && data.length > 0) {
+ const expandedColumns = results.expanded_columns
+ ? results.expanded_columns.map(col => col.name)
+ : [];
return (
- <div>
- <Alert
- type="info"
- message={
- <>
- {t(object)} [
- <strong>
- {tempSchema ? `${tempSchema}.` : ''}
- {tempTable}
- </strong>
- ] {t('was created')}
- <ButtonGroup>
- <Button
- buttonSize="small"
- className="m-r-5"
- onClick={() => this.popSelectStar(tempSchema, tempTable)}
- >
- {t('Query in a new tab')}
- </Button>
- <ExploreCtasResultsButton
- // @ts-ignore Redux types are difficult to work with, ignoring for now
- actions={this.props.actions}
- table={tempTable}
- schema={tempSchema}
- dbId={exploreDBId}
- />
- </ButtonGroup>
- </>
- }
+ <>
+ {renderControls()}
+ {renderRowsReturned()}
+ {sql}
+ <FilterableTable
+ data={data}
+ orderedColumnKeys={results.columns.map(col => col.name)}
+ height={rowsHeight}
+ filterText={searchText}
+ expandedColumns={expandedColumns}
/>
- </div>
+ </>
);
}
- if (query.state === 'success' && query.results) {
- const { results } = query;
- // Accounts for offset needed for height of ResultSetRowsReturned component if !limitReached
- const rowMessageHeight = !limitReached ? 32 : 0;
- // Accounts for offset needed for height of Alert if this.state.alertIsOpen
- const alertContainerHeight = 70;
- // We need to calculate the height of this.renderRowsReturned()
- // if we want results panel to be propper height because the
- // FilterTable component nedds an explcit height to render
- // react-virtualized Table component
- const height = this.state.alertIsOpen
- ? this.props.height - alertContainerHeight
- : this.props.height - rowMessageHeight;
- let data;
- if (this.props.cache && query.cached) {
- ({ data } = this.state);
- } else if (results && results.data) {
- ({ data } = results);
- }
- if (data && data.length > 0) {
- const expandedColumns = results.expanded_columns
- ? results.expanded_columns.map(col => col.name)
- : [];
- return (
- <>
- {this.renderControls()}
- {this.renderRowsReturned()}
- {sql}
- <FilterableTable
- data={data}
- orderedColumnKeys={results.columns.map(col => col.name)}
- height={height}
- filterText={this.state.searchText}
- expandedColumns={expandedColumns}
- />
- </>
- );
- }
- if (data && data.length === 0) {
- return (
- <Alert type="warning" message={t('The query returned no data')} />
- );
- }
+ if (data && data.length === 0) {
Review Comment:
```suggestion
if (data?.length === 0) {
```
This can be shortened with optional chaining
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -70,14 +70,6 @@ interface ResultSetProps {
defaultQueryLimit: number;
}
-interface ResultSetState {
- searchText: string;
- showExploreResultsButton: boolean;
- data: Record<string, any>[];
- showSaveDatasetModal: boolean;
- alertIsOpen: boolean;
-}
-
const ResultlessStyles = styled.div`
position: relative;
min-height: 100px;
Review Comment:
I can't target this spot for another "suggested change" comment, but the values for `font-size` and `line-height` in `ReturnedRows` on [these lines](https://github.com/apache/superset/blob/7d21ad4a1124c81d076906fbc76538995372e008/superset-frontend/src/SqlLab/components/ResultSet/index.tsx#L95-L96) also need to be converted to use the Superset theme
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -414,193 +384,191 @@ export default class ResultSet extends React.PureComponent<
)}
</ReturnedRows>
);
+ };
+
+ const limitReached = query?.results?.displayLimitReached;
+ let sql;
+ let exploreDBId = query.dbId;
+ if (database?.explore_database_id) {
+ exploreDBId = database.explore_database_id;
}
- render() {
- const { query } = this.props;
- const limitReached = query?.results?.displayLimitReached;
- let sql;
- let exploreDBId = query.dbId;
- if (this.props.database && this.props.database.explore_database_id) {
- exploreDBId = this.props.database.explore_database_id;
- }
- let trackingUrl;
- if (
- query.trackingUrl &&
- query.state !== 'success' &&
- query.state !== 'fetching'
- ) {
- trackingUrl = (
- <Button
- className="sql-result-track-job"
- buttonSize="small"
- href={query.trackingUrl}
- target="_blank"
- >
- {query.state === 'running' ? t('Track job') : t('See query details')}
- </Button>
- );
- }
+ let trackingUrl;
+ if (
+ query.trackingUrl &&
+ query.state !== 'success' &&
+ query.state !== 'fetching'
+ ) {
+ trackingUrl = (
+ <Button
+ className="sql-result-track-job"
+ buttonSize="small"
+ href={query.trackingUrl}
+ target="_blank"
+ >
+ {query.state === 'running' ? t('Track job') : t('See query details')}
+ </Button>
+ );
+ }
- if (this.props.showSql) sql = <HighlightedSql sql={query.sql} />;
+ if (showSql) {
+ sql = <HighlightedSql sql={query.sql} />;
+ }
+
+ if (query.state === 'stopped') {
+ return <Alert type="warning" message={t('Query was stopped')} />;
+ }
- if (query.state === 'stopped') {
- return <Alert type="warning" message={t('Query was stopped')} />;
+ if (query.state === 'failed') {
+ return (
+ <ResultlessStyles>
+ <ErrorMessageWithStackTrace
+ title={t('Database error')}
+ error={query?.errors?.[0]}
+ subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
+ copyText={query.errorMessage || undefined}
+ link={query.link}
+ source="sqllab"
+ />
+ {trackingUrl}
+ </ResultlessStyles>
+ );
+ }
+
+ if (query.state === 'success' && query.ctas) {
+ const { tempSchema, tempTable } = query;
+ let object = 'Table';
+ if (query.ctas_method === CtasEnum.VIEW) {
+ object = 'View';
}
- if (query.state === 'failed') {
- return (
- <ResultlessStyles>
- <ErrorMessageWithStackTrace
- title={t('Database error')}
- error={query?.errors?.[0]}
- subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
- copyText={query.errorMessage || undefined}
- link={query.link}
- source="sqllab"
- />
- {trackingUrl}
- </ResultlessStyles>
- );
+ return (
+ <div>
+ <Alert
+ type="info"
+ message={
+ <>
+ {t(object)} [
+ <strong>
+ {tempSchema ? `${tempSchema}.` : ''}
+ {tempTable}
+ </strong>
+ ] {t('was created')}
+ <ButtonGroup>
+ <Button
+ buttonSize="small"
+ className="m-r-5"
+ onClick={() => popSelectStar(tempSchema, tempTable)}
+ >
+ {t('Query in a new tab')}
+ </Button>
+ <ExploreCtasResultsButton
+ // @ts-ignore Redux types are difficult to work with, ignoring for now
+ actions={actions}
+ table={tempTable}
+ schema={tempSchema}
+ dbId={exploreDBId}
+ />
+ </ButtonGroup>
+ </>
+ }
+ />
+ </div>
+ );
+ }
+
+ if (query.state === 'success' && query.results) {
+ const { results } = query;
+ // Accounts for offset needed for height of ResultSetRowsReturned component if !limitReached
+ const rowMessageHeight = !limitReached ? 32 : 0;
+ // Accounts for offset needed for height of Alert if this.state.alertIsOpen
+ const alertContainerHeight = 70;
+ // We need to calculate the height of this.renderRowsReturned()
+ // if we want results panel to be proper height because the
+ // FilterTable component needs an explicit height to render
+ // react-virtualized Table component
+ const rowsHeight = alertIsOpen
+ ? height - alertContainerHeight
+ : height - rowMessageHeight;
+ let data;
+ if (cache && query.cached) {
+ data = cachedData;
+ } else if (results && results.data) {
Review Comment:
```suggestion
} else if (results?.data) {
```
This can be shortened with optional chaining
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -414,193 +384,191 @@ export default class ResultSet extends React.PureComponent<
)}
</ReturnedRows>
);
+ };
+
+ const limitReached = query?.results?.displayLimitReached;
+ let sql;
+ let exploreDBId = query.dbId;
+ if (database?.explore_database_id) {
+ exploreDBId = database.explore_database_id;
}
- render() {
- const { query } = this.props;
- const limitReached = query?.results?.displayLimitReached;
- let sql;
- let exploreDBId = query.dbId;
- if (this.props.database && this.props.database.explore_database_id) {
- exploreDBId = this.props.database.explore_database_id;
- }
- let trackingUrl;
- if (
- query.trackingUrl &&
- query.state !== 'success' &&
- query.state !== 'fetching'
- ) {
- trackingUrl = (
- <Button
- className="sql-result-track-job"
- buttonSize="small"
- href={query.trackingUrl}
- target="_blank"
- >
- {query.state === 'running' ? t('Track job') : t('See query details')}
- </Button>
- );
- }
+ let trackingUrl;
+ if (
+ query.trackingUrl &&
+ query.state !== 'success' &&
+ query.state !== 'fetching'
+ ) {
+ trackingUrl = (
+ <Button
+ className="sql-result-track-job"
+ buttonSize="small"
+ href={query.trackingUrl}
+ target="_blank"
+ >
+ {query.state === 'running' ? t('Track job') : t('See query details')}
+ </Button>
+ );
+ }
- if (this.props.showSql) sql = <HighlightedSql sql={query.sql} />;
+ if (showSql) {
+ sql = <HighlightedSql sql={query.sql} />;
+ }
+
+ if (query.state === 'stopped') {
+ return <Alert type="warning" message={t('Query was stopped')} />;
+ }
- if (query.state === 'stopped') {
- return <Alert type="warning" message={t('Query was stopped')} />;
+ if (query.state === 'failed') {
+ return (
+ <ResultlessStyles>
+ <ErrorMessageWithStackTrace
+ title={t('Database error')}
+ error={query?.errors?.[0]}
+ subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
+ copyText={query.errorMessage || undefined}
+ link={query.link}
+ source="sqllab"
+ />
+ {trackingUrl}
+ </ResultlessStyles>
+ );
+ }
+
+ if (query.state === 'success' && query.ctas) {
+ const { tempSchema, tempTable } = query;
+ let object = 'Table';
+ if (query.ctas_method === CtasEnum.VIEW) {
+ object = 'View';
}
- if (query.state === 'failed') {
- return (
- <ResultlessStyles>
- <ErrorMessageWithStackTrace
- title={t('Database error')}
- error={query?.errors?.[0]}
- subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
- copyText={query.errorMessage || undefined}
- link={query.link}
- source="sqllab"
- />
- {trackingUrl}
- </ResultlessStyles>
- );
+ return (
+ <div>
+ <Alert
+ type="info"
+ message={
+ <>
+ {t(object)} [
+ <strong>
+ {tempSchema ? `${tempSchema}.` : ''}
+ {tempTable}
+ </strong>
+ ] {t('was created')}
+ <ButtonGroup>
+ <Button
+ buttonSize="small"
+ className="m-r-5"
+ onClick={() => popSelectStar(tempSchema, tempTable)}
+ >
+ {t('Query in a new tab')}
+ </Button>
+ <ExploreCtasResultsButton
+ // @ts-ignore Redux types are difficult to work with, ignoring for now
+ actions={actions}
+ table={tempTable}
+ schema={tempSchema}
+ dbId={exploreDBId}
+ />
+ </ButtonGroup>
+ </>
+ }
+ />
+ </div>
+ );
+ }
+
+ if (query.state === 'success' && query.results) {
+ const { results } = query;
+ // Accounts for offset needed for height of ResultSetRowsReturned component if !limitReached
+ const rowMessageHeight = !limitReached ? 32 : 0;
+ // Accounts for offset needed for height of Alert if this.state.alertIsOpen
+ const alertContainerHeight = 70;
+ // We need to calculate the height of this.renderRowsReturned()
+ // if we want results panel to be proper height because the
+ // FilterTable component needs an explicit height to render
+ // react-virtualized Table component
+ const rowsHeight = alertIsOpen
+ ? height - alertContainerHeight
+ : height - rowMessageHeight;
+ let data;
+ if (cache && query.cached) {
+ data = cachedData;
+ } else if (results && results.data) {
+ ({ data } = results);
}
- if (query.state === 'success' && query.ctas) {
- const { tempSchema, tempTable } = query;
- let object = 'Table';
- if (query.ctas_method === CtasEnum.VIEW) {
- object = 'View';
- }
+ if (data && data.length > 0) {
Review Comment:
```suggestion
if (data?.length > 0) {
```
This can be shortened with optional chaining
--
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