mikebridge commented on code in PR #44206:
URL: https://github.com/apache/superset/pull/44206#discussion_r4200912249
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -44,14 +51,34 @@ type Result = {
error?: string;
};
+export function getSemanticReportState(
+ queriesResponse: readonly { query?: string }[] | null | undefined,
+): 'has-requests' | 'none-reported' | 'not-run' {
+ if (!queriesResponse?.length) {
+ return 'not-run';
+ }
+ return queriesResponse.some(entry => entry.query)
+ ? 'has-requests'
+ : 'none-reported';
+}
+
const ViewQueryModalContainer = styled.div`
height: 100%;
display: flex;
flex-direction: column;
- gap: ${({ theme }) => theme.sizeUnit * 4}px;
+ gap: ${({ theme }: { theme: SupersetTheme }) => theme.sizeUnit * 4}px;
`;
const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
+ const isSemanticView =
+ new DatasourceKey(latestQueryFormData.datasource).type ===
+ DatasourceType.SemanticView;
+ const queriesResponse = useSelector<
+ RootState,
+ ChartState['queriesResponse'] | undefined
+ >(
+ state => state.charts?.[latestQueryFormData.slice_id ??
0]?.queriesResponse,
Review Comment:
Fixed in 3e2d2de2e4. Explore and Dashboard pass the owning chart ID
explicitly, so View query no longer depends on `latestQueryFormData` carrying
`slice_id`. The regressions open both real menus with `slice_id` absent and a
different report under key zero; they show the owning chart's request instead.
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -76,22 +103,51 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData,
ownState }) => {
setError(null);
})
.catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setError(
- error ||
- message ||
- response.statusText ||
- t('Sorry, An error occurred'),
- );
- setIsLoading(false);
- });
+ getClientErrorObject(response).then(
+ ({ error, message }: ClientErrorObject) => {
+ setError(
+ error ||
+ message ||
+ response.statusText ||
+ t('Sorry, An error occurred'),
+ );
+ setIsLoading(false);
+ },
+ );
});
},
[latestQueryFormData, ownState],
);
useEffect(() => {
- loadChartData('query');
- }, [loadChartData]);
+ if (!isSemanticView) {
+ loadChartData('query');
+ }
+ }, [isSemanticView, loadChartData]);
+
+ if (isSemanticView) {
+ const reportState = getSemanticReportState(queriesResponse);
+ const noRequestMessage = t('No provider query is available for this run.');
+ return (
+ <ViewQueryModalContainer>
+ {reportState === 'not-run' ? (
+ <Alert
+ type="info"
+ message={t(
+ 'The provider query will be available after the chart runs.',
+ )}
+ />
+ ) : (
+ queriesResponse?.map((entry, index) =>
+ entry.query ? (
+ <SemanticRequestView key={index} requestText={entry.query} />
+ ) : (
+ <Alert key={index} type="info" message={noRequestMessage} />
+ ),
Review Comment:
Fixed in 3e2d2de2e4. Error-bearing entries render the actual error,
including alongside reported request text. A failed run without a response uses
the chart's existing error message. Reducer-driven tests cover provider and
network failures without displaying the previous successful request or the
generic no-query message.
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -76,22 +103,51 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData,
ownState }) => {
setError(null);
})
.catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setError(
- error ||
- message ||
- response.statusText ||
- t('Sorry, An error occurred'),
- );
- setIsLoading(false);
- });
+ getClientErrorObject(response).then(
+ ({ error, message }: ClientErrorObject) => {
+ setError(
+ error ||
+ message ||
+ response.statusText ||
+ t('Sorry, An error occurred'),
+ );
+ setIsLoading(false);
+ },
+ );
});
},
[latestQueryFormData, ownState],
);
useEffect(() => {
- loadChartData('query');
- }, [loadChartData]);
+ if (!isSemanticView) {
+ loadChartData('query');
+ }
+ }, [isSemanticView, loadChartData]);
+
+ if (isSemanticView) {
+ const reportState = getSemanticReportState(queriesResponse);
+ const noRequestMessage = t('No provider query is available for this run.');
+ return (
+ <ViewQueryModalContainer>
+ {reportState === 'not-run' ? (
+ <Alert
+ type="info"
+ message={t(
+ 'The provider query will be available after the chart runs.',
+ )}
+ />
+ ) : (
+ queriesResponse?.map((entry, index) =>
+ entry.query ? (
+ <SemanticRequestView key={index} requestText={entry.query} />
+ ) : (
+ <Alert key={index} type="info" message={noRequestMessage} />
+ ),
+ )
Review Comment:
Fixed in 3e2d2de2e4: semantic response errors render as error alerts. The
regressions dispatch `CHART_UPDATE_FAILED` through the chart reducer and verify
the actual error appears; mixed request/error responses preserve both in entry
order.
##########
superset/semantic_layers/models.py:
##########
@@ -378,7 +378,13 @@ def get_query_result(self, query_object: QueryObject) ->
QueryResult:
return result
def get_query_str(self, query_obj: QueryObjectDict) -> str:
- return "Not implemented for semantic layers"
+ """Reject previews because provider queries are returned with chart
data."""
+ raise QueryObjectValidationError(
+ _(
+ "A semantic view's provider query is produced when the chart
runs "
+ "and returned with its data."
+ )
+ )
Review Comment:
Leaving translation backfill unchanged in this round, as agreed with
rusackas; the existing messages remain marked for translation.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
