fix(sqllab): misplaced limit warning alert (apache#25306)

preset-io · Sep 27, 2023 · 463962a · 463962a
1 parent 1716b9f
commit 463962a
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 78 deletions.
diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
@@ -301,7 +301,7 @@ const ResultSet = ({
     return <div />;
   };
 
-  const renderRowsReturned = () => {
+  const renderRowsReturned = (alertMessage: boolean) => {
     const { results, rows, queryLimit, limitingFactor } = query;
     let limitMessage = '';
     const limitReached = results?.displayLimitReached;
@@ -353,59 +353,70 @@ const ResultSet = ({
 
     const tooltipText = `${rowsReturnedMessage}. ${limitMessage}`;
 
+    if (alertMessage) {
+      return (
+        <>
+          {!limitReached && shouldUseDefaultDropdownAlert && (
+            <div ref={calculateAlertRefHeight}>
+              <Alert
+                type="warning"
+                message={t('%(rows)d rows returned', { rows })}
+                onClose={() => setAlertIsOpen(false)}
+                description={t(
+                  'The number of rows displayed is limited to %(rows)d by the dropdown.',
+                  { rows },
+                )}
+              />
+            </div>
+          )}
+          {limitReached && (
+            <div ref={calculateAlertRefHeight}>
+              <Alert
+                type="warning"
+                onClose={() => setAlertIsOpen(false)}
+                message={t('%(rows)d rows returned', { rows: rowsCount })}
+                description={
+                  isAdmin
+                    ? displayMaxRowsReachedMessage.withAdmin
+                    : displayMaxRowsReachedMessage.withoutAdmin
+                }
+              />
+            </div>
+          )}
+        </>
+      );
+    }
+    const showRowsReturned =
+      showSqlInline || (!limitReached && !shouldUseDefaultDropdownAlert);
+
     return (
-      <ReturnedRows>
-        {!limitReached && !shouldUseDefaultDropdownAlert && (
-          <Tooltip
-            id="sqllab-rowcount-tooltip"
-            title={tooltipText}
-            placement="left"
-          >
-            <Label
-              css={css`
-                line-height: ${theme.typography.sizes.l}px;
-              `}
+      <>
+        {showRowsReturned && (
+          <ReturnedRows>
+            <Tooltip
+              id="sqllab-rowcount-tooltip"
+              title={tooltipText}
+              placement="left"
             >
-              {limitMessage && (
-                <Icons.ExclamationCircleOutlined
-                  css={css`
-                    font-size: ${theme.typography.sizes.m}px;
-                    margin-right: ${theme.gridUnit}px;
-                  `}
-                />
-              )}
-              {tn('%s row', '%s rows', rows, formattedRowCount)}
-            </Label>
-          </Tooltip>
-        )}
-        {!limitReached && shouldUseDefaultDropdownAlert && (
-          <div ref={calculateAlertRefHeight}>
-            <Alert
-              type="warning"
-              message={t('%(rows)d rows returned', { rows })}
-              onClose={() => setAlertIsOpen(false)}
-              description={t(
-                'The number of rows displayed is limited to %(rows)d by the dropdown.',
-                { rows },
-              )}
-            />
-          </div>
-        )}
-        {limitReached && (
-          <div ref={calculateAlertRefHeight}>
-            <Alert
-              type="warning"
-              onClose={() => setAlertIsOpen(false)}
-              message={t('%(rows)d rows returned', { rows: rowsCount })}
-              description={
-                isAdmin
-                  ? displayMaxRowsReachedMessage.withAdmin
-                  : displayMaxRowsReachedMessage.withoutAdmin
-              }
-            />
-          </div>
+              <Label
+                css={css`
+                  line-height: ${theme.typography.sizes.l}px;
+                `}
+              >
+                {limitMessage && (
+                  <Icons.ExclamationCircleOutlined
+                    css={css`
+                      font-size: ${theme.typography.sizes.m}px;
+                      margin-right: ${theme.gridUnit}px;
+                    `}
+                  />
+                )}
+                {tn('%s row', '%s rows', rows, formattedRowCount)}
+              </Label>
+            </Tooltip>
+          </ReturnedRows>
         )}
-      </ReturnedRows>
+      </>
     );
   };
 
@@ -531,35 +542,39 @@ const ResultSet = ({
         <ResultContainer>
           {renderControls()}
           {showSql && showSqlInline ? (
-            <div
-              css={css`
-                display: flex;
-                justify-content: space-between;
-                gap: ${GAP}px;
-              `}
-            >
-              <Card
-                css={[
-                  css`
-                    height: 28px;
-                    width: calc(100% - ${ROWS_CHIP_WIDTH + GAP}px);
-                    code {
-                      width: 100%;
-                      overflow: hidden;
-                      white-space: nowrap !important;
-                      text-overflow: ellipsis;
-                      display: block;
-                    }
-                  `,
-                ]}
+            <>
+              <div
+                css={css`
+                  display: flex;
+                  justify-content: space-between;
+                  gap: ${GAP}px;
+                `}
               >
-                {sql}
-              </Card>
-              {renderRowsReturned()}
-            </div>
+                <Card
+                  css={[
+                    css`
+                      height: 28px;
+                      width: calc(100% - ${ROWS_CHIP_WIDTH + GAP}px);
+                      code {
+                        width: 100%;
+                        overflow: hidden;
+                        white-space: nowrap !important;
+                        text-overflow: ellipsis;
+                        display: block;
+                      }
+                    `,
+                  ]}
+                >
+                  {sql}
+                </Card>
+                {renderRowsReturned(false)}
+              </div>
+              {renderRowsReturned(true)}
+            </>
           ) : (
             <>
-              {renderRowsReturned()}
+              {renderRowsReturned(false)}
+              {renderRowsReturned(true)}
               {sql}
             </>
           )}

diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js
@@ -637,7 +637,11 @@ export default function sqlLabReducer(state = {}, action) {
             // when it started fetching or finished rendering results
             state:
               currentState === QueryState.SUCCESS &&
-              [QueryState.FETCHING, QueryState.SUCCESS].includes(prevState)
+              [
+                QueryState.FETCHING,
+                QueryState.SUCCESS,
+                QueryState.RUNNING,
+              ].includes(prevState)
                 ? prevState
                 : currentState,
           };