EnxDev commented on code in PR #43376:
URL: https://github.com/apache/superset/pull/43376#discussion_r3948164454
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -616,10 +616,14 @@ test('Should show "View query"', () => {
Admin: [['can_view_query', 'Dashboard']],
});
openMenu();
- expect(screen.getByText('View query')).toBeInTheDocument();
+ userEvent.click(screen.getByText('Query inspector'));
+ expect(screen.getByRole('tab', { name: 'Stats' })).toBeInTheDocument();
Review Comment:
Could we seed queriesResponse in this test fixture before asserting the
tabs? createProps() leaves it undefined, and ViewQueryModal intentionally falls
back to the legacy query-only body in that case, so the Stats tab is never
mounted. I reproduced this on the PR head: the suite finishes with 49 passed
and this one failure, which appears to match the failing Jest shard. Passing
queriesResponse: null or [] here would exercise the intended query-only
permission case.
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -119,6 +180,70 @@ 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)
Review Comment:
Could we avoid showing this duration when the latest refresh was stopped, or
otherwise pair these timestamps with the response they belong to?
CHART_UPDATE_STARTED and CHART_UPDATE_STOPPED retain the previous
queriesResponse while replacing the start/end times. After a cancelled refresh,
rows, cache hits, and response size therefore describe the previous successful
payload, while Duration describes the cancelled attempt, which can be
misleading during troubleshooting.
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -51,7 +58,64 @@ 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);
+ const returnedRows = responses.reduce((total, response) => {
+ const { data } = response as JsonObject;
+ return total + (Array.isArray(data) ? data.length : 0);
+ }, 0);
+ const cachedQueries = responses.filter(
+ response => (response as JsonObject).is_cached === true,
+ ).length;
+
+ return {
+ cachedQueries,
+ queryCount: responses.length,
+ responseBytes: new Blob([JSON.stringify(responses)]).size,
Review Comment:
Could we preserve the distinction between no response and an empty response
here? When queriesResponse is null, normalizing it to [] makes the Stats tab
report a 2-byte response even though the Response tab says that no response
data is available. Returning an unavailable size for null, and only computing
bytes for an actual array, would keep those two views consistent.
--
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]
