jsk1004ha commented on code in PR #43376:
URL: https://github.com/apache/superset/pull/43376#discussion_r4084115325
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -750,9 +788,6 @@ test('Dataset drill info API call is made for an
explore-only user', async () =>
(global as any).featureFlags = {
[FeatureFlag.DrillToDetail]: false,
};
- // "View as table" is offered to `canExplore || canViewTable`, so the fetch
that
- // feeds its column headers has to cover the same set -- an explore user with
- // neither `can_samples` nor `can_view_chart_as_table` opens the same modal.
Review Comment:
Addressed in `97a8e62da`: restored the drill-info explanation and the other
rationale comments in this test. Thanks for catching that!
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -51,7 +58,67 @@ const ViewQueryModalContainer = styled.div`
gap: ${({ theme }) => theme.sizeUnit * 4}px;
`;
-const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
+const InspectorContainer = styled.div`
+ height: 100%;
+
+ .ant-tabs,
+ .ant-tabs-content,
+ .ant-tabs-tabpane {
+ height: 100%;
+ }
+
+ .ant-tabs-tabpane {
+ overflow: auto;
+ }
+`;
Review Comment:
Addressed in `97a8e62da`: switched this to the shared `<Tabs>` component and
dropped the Ant Design overrides. Thanks!
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -51,7 +58,67 @@ const ViewQueryModalContainer = styled.div`
gap: ${({ theme }) => theme.sizeUnit * 4}px;
`;
-const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
+const InspectorContainer = styled.div`
+ height: 100%;
+
+ .ant-tabs,
+ .ant-tabs-content,
+ .ant-tabs-tabpane {
+ height: 100%;
+ }
+
+ .ant-tabs-tabpane {
+ overflow: auto;
+ }
+`;
+
+const StatsGrid = styled.dl`
+ display: grid;
+ grid-template-columns: max-content 1fr;
+ gap: ${({ theme }) => theme.sizeUnit * 3}px
+ ${({ theme }) => theme.sizeUnit * 6}px;
+ margin: 0;
+
+ dt {
+ color: ${({ theme }) => theme.colorTextSecondary};
+ }
+
+ dd {
+ margin: 0;
+ }
+`;
+
+const getResponseStats = (queriesResponse: QueryData[] | null) => {
+ const responses = queriesResponse ?? [];
+ const serializedResponse = JSON.stringify(responses, null, 2);
Review Comment:
Addressed in `97a8e62da`: memoized the stats, reused the compact JSON for
the byte count, and only format the response after opening the Response tab.
Thanks!
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -119,6 +183,74 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData,
ownState }) => {
))}
</ViewQueryModalContainer>
);
+
+ if (queriesResponse === undefined) {
+ return queryContent;
+ }
+
+ const {
+ cachedQueries,
+ queryCount,
+ responseBytes,
+ returnedRows,
+ serializedResponse,
+ } = getResponseStats(queriesResponse);
+ const duration =
+ chartUpdateStartTime != null && chartUpdateEndTime != null
+ ? Math.max(0, chartUpdateEndTime - chartUpdateStartTime)
+ : null;
+ const items = [
+ {
+ key: 'query',
+ label: t('Query'),
+ children: queryContent,
+ },
+ ...(showResponse
+ ? [
+ {
+ key: 'response',
+ label: t('Response'),
+ children: queriesResponse?.length ? (
+ <CodeSyntaxHighlighter language="json" showLineNumbers>
Review Comment:
Addressed in `97a8e62da`: added a plain-text fallback for large responses,
with the full payload available to copy, plus a regression test. Thanks!
--
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]
