Skip to content

Commit

Permalink
feat(sqllab): Popup notification when download data can exceed row co…
Browse files Browse the repository at this point in the history
…unt (apache#31187)

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
  • Loading branch information
justinpark and michael-s-molina authored Dec 2, 2024
1 parent d66ac9f commit 339d491
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import {
render,
screen,
waitFor,
fireEvent,
within,
} from 'spec/helpers/testing-library';
import configureStore from 'redux-mock-store';
import { Store } from 'redux';
import thunk from 'redux-thunk';
Expand Down Expand Up @@ -492,6 +498,38 @@ describe('ResultSet', () => {
expect(queryByTestId('export-csv-button')).toBeInTheDocument();
});

test('should display a popup message when the CSV content is limited to the dropdown limit', async () => {
const queryLimit = 2;
const { getByTestId, findByRole } = setup(
mockedProps,
mockStore({
...initialState,
user: {
...user,
roles: {
sql_lab: [['can_export_csv', 'SQLLab']],
},
},
sqlLab: {
...initialState.sqlLab,
queries: {
[queries[0].id]: {
...queries[0],
limitingFactor: 'DROPDOWN',
queryLimit,
},
},
},
}),
);
const downloadButton = getByTestId('export-csv-button');
fireEvent.click(downloadButton);
const warningModal = await findByRole('dialog');
expect(
within(warningModal).getByText(`Download is on the way`),
).toBeInTheDocument();
});

test('should not allow download as CSV when user does not have permission to export data', async () => {
const { queryByTestId } = setup(
mockedProps,
Expand Down
20 changes: 19 additions & 1 deletion superset-frontend/src/SqlLab/components/ResultSet/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import CopyToClipboard from 'src/components/CopyToClipboard';
import { addDangerToast } from 'src/components/MessageToasts/actions';
import { prepareCopyToClipboardTabularData } from 'src/utils/common';
import { getItem, LocalStorageKeys } from 'src/utils/localStorageHelpers';
import Modal from 'src/components/Modal';
import {
addQueryEditor,
clearQueryResults,
Expand Down Expand Up @@ -296,6 +297,9 @@ const ResultSet = ({

const renderControls = () => {
if (search || visualize || csv) {
const { results, queryLimit, limitingFactor, rows } = query;
const limit = queryLimit || results.query.limit;
const rowsCount = Math.min(rows || 0, results?.data?.length || 0);
let { data } = query.results;
if (cache && query.cached) {
data = cachedData;
Expand Down Expand Up @@ -342,7 +346,21 @@ const ResultSet = ({
buttonSize="small"
href={getExportCsvUrl(query.id)}
data-test="export-csv-button"
onClick={() => logAction(LOG_ACTIONS_SQLLAB_DOWNLOAD_CSV, {})}
onClick={() => {
logAction(LOG_ACTIONS_SQLLAB_DOWNLOAD_CSV, {});
if (
limitingFactor === LimitingFactor.Dropdown &&
limit === rowsCount
) {
Modal.warning({
title: t('Download is on the way'),
content: t(
'Downloading %(rows)s rows based on the LIMIT configuration. If you want the entire result set, you need to adjust the LIMIT.',
{ rows: rowsCount.toLocaleString() },
),
});
}
}}
>
<i className="fa fa-file-text-o" /> {t('Download to CSV')}
</Button>
Expand Down

0 comments on commit 339d491

Please sign in to comment.