From e465876ed418f669e449648578ba9cfe73de4d9d Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Fri, 19 Apr 2024 14:19:42 +0100 Subject: [PATCH] feat: new Excel upload form and API (#28105) --- RESOURCES/STANDARD_ROLES.md | 3 +- superset-frontend/package-lock.json | 156 ++++++ superset-frontend/package.json | 1 + .../ColumnsPreview.tsx | 2 +- .../StyledFormItemWithTip.tsx | 0 .../UploadDataModal.test.tsx} | 280 ++++++++++- .../index.tsx | 473 +++++++++++++----- .../styles.ts | 0 .../src/features/home/RightMenu.test.tsx | 12 +- .../src/features/home/RightMenu.tsx | 6 +- .../src/pages/DatabaseList/index.tsx | 44 +- superset-frontend/src/views/CRUD/utils.tsx | 2 +- superset/commands/database/csv_import.py | 2 +- superset/commands/database/excel_import.py | 183 +++++++ superset/databases/api.py | 63 +++ superset/databases/schemas.py | 122 +++-- superset/initialization/__init__.py | 7 +- ..._d60591c5515f_mig_new_excel_upload_perm.py | 87 ++++ superset/security/manager.py | 1 + .../superset/form_view/csv_macros.html | 75 --- .../superset/form_view/csv_scripts.html | 39 -- .../form_view/csv_to_database_view/edit.html | 137 ----- .../excel_to_database_view/edit.html | 25 - superset/views/database/forms.py | 150 +----- superset/views/database/views.py | 139 +---- tests/integration_tests/csv_upload_tests.py | 101 ---- .../integration_tests/dashboards/api_tests.py | 2 +- .../integration_tests/databases/api_tests.py | 1 + .../databases/commands/excel_upload_test.py | 257 ++++++++++ tests/unit_tests/databases/api_test.py | 222 +++++++- tests/unit_tests/fixtures/common.py | 11 + 31 files changed, 1714 insertions(+), 889 deletions(-) rename superset-frontend/src/features/databases/{CSVUploadModal => UploadDataModel}/ColumnsPreview.tsx (95%) rename superset-frontend/src/features/databases/{CSVUploadModal => UploadDataModel}/StyledFormItemWithTip.tsx (100%) rename superset-frontend/src/features/databases/{CSVUploadModal/CSVUploadModal.test.tsx => UploadDataModel/UploadDataModal.test.tsx} (55%) rename superset-frontend/src/features/databases/{CSVUploadModal => UploadDataModel}/index.tsx (65%) rename superset-frontend/src/features/databases/{CSVUploadModal => UploadDataModel}/styles.ts (100%) create mode 100644 superset/commands/database/excel_import.py create mode 100644 superset/migrations/versions/2024-04-17_14-04_d60591c5515f_mig_new_excel_upload_perm.py delete mode 100644 superset/templates/superset/form_view/csv_macros.html delete mode 100644 superset/templates/superset/form_view/csv_scripts.html delete mode 100644 superset/templates/superset/form_view/csv_to_database_view/edit.html delete mode 100644 superset/templates/superset/form_view/excel_to_database_view/edit.html create mode 100644 tests/integration_tests/databases/commands/excel_upload_test.py diff --git a/RESOURCES/STANDARD_ROLES.md b/RESOURCES/STANDARD_ROLES.md index 06238de37d079..94aee296d5ef7 100644 --- a/RESOURCES/STANDARD_ROLES.md +++ b/RESOURCES/STANDARD_ROLES.md @@ -67,11 +67,10 @@ under the License. | can invalidate on CacheRestApi |:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| | can function names on Database |:heavy_check_mark:|O|O|O| | can csv upload on Database |:heavy_check_mark:|O|O|O| +| can excel upload on Database |:heavy_check_mark:|O|O|O| | can query form data on Api |:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| | can query on Api |:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| | can time range on Api |:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| -| can this form get on ExcelToDatabaseView |:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| -| can this form post on ExcelToDatabaseView |:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| | can external metadata on Datasource |:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| | can save on Datasource |:heavy_check_mark:|:heavy_check_mark:|O|O| | can get on Datasource |:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index d192c379f4fcf..e56a4cef726b6 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -138,6 +138,7 @@ "use-event-callback": "^0.1.0", "use-immer": "^0.9.0", "use-query-params": "^1.1.9", + "xlsx": "^0.18.5", "yargs": "^17.7.2" }, "devDependencies": { @@ -25254,6 +25255,14 @@ "node": ">= 0.12.0" } }, + "node_modules/adler-32": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/adler-32/-/adler-32-1.3.1.tgz", + "integrity": "sha512-ynZ4w/nUUv5rrsR8UUGoe1VC9hZj6V5hU9Qw1HlMDJGEJw5S7TfTErWTjMys6M7vr0YWcPqs3qAr4ss0nDfP+A==", + "engines": { + "node": ">=0.8" + } + }, "node_modules/agent-base": { "version": "6.0.2", "resolved": "https://registry.npmjs.org/agent-base/-/agent-base-6.0.2.tgz", @@ -28004,6 +28013,18 @@ "url": "https://github.com/sponsors/wooorm" } }, + "node_modules/cfb": { + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/cfb/-/cfb-1.2.2.tgz", + "integrity": "sha512-KfdUZsSOw19/ObEWasvBP/Ac4reZvAGauZhs6S/gqNhXhI7cKwvlH7ulj+dOEYnca4bm4SGo8C1bTAQvnTjgQA==", + "dependencies": { + "adler-32": "~1.3.0", + "crc-32": "~1.2.0" + }, + "engines": { + "node": ">=0.8" + } + }, "node_modules/chainsaw": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/chainsaw/-/chainsaw-0.1.0.tgz", @@ -28802,6 +28823,14 @@ "node": ">=0.10.0" } }, + "node_modules/codepage": { + "version": "1.15.0", + "resolved": "https://registry.npmjs.org/codepage/-/codepage-1.15.0.tgz", + "integrity": "sha512-3g6NUTPd/YtuuGrhMnOMRjFc+LJw/bnMp3+0r/Wcz3IXUuCosKRJvMphm5+Q+bvTVGcJJuRvVLuYba+WojaFaA==", + "engines": { + "node": ">=0.8" + } + }, "node_modules/collect-v8-coverage": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/collect-v8-coverage/-/collect-v8-coverage-1.0.1.tgz", @@ -29773,6 +29802,17 @@ "node": ">=8" } }, + "node_modules/crc-32": { + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/crc-32/-/crc-32-1.2.2.tgz", + "integrity": "sha512-ROmzCKrTnOwybPcJApAA6WBWij23HVfGVNKqqrZpuyZOHqK2CwHSvpGuyt/UNNvaIjEd8X5IFGp4Mh+Ie1IHJQ==", + "bin": { + "crc32": "bin/crc32.njs" + }, + "engines": { + "node": ">=0.8" + } + }, "node_modules/create-emotion": { "version": "10.0.27", "resolved": "https://registry.npmjs.org/create-emotion/-/create-emotion-10.0.27.tgz", @@ -37052,6 +37092,14 @@ "node": ">=12.20.0" } }, + "node_modules/frac": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/frac/-/frac-1.1.2.tgz", + "integrity": "sha512-w/XBfkibaTl3YDqASwfDUqkna4Z2p9cFSr1aHDt0WoMTECnRfBOv2WArlZILlqgWlmdIlALXGpM2AOhEk5W3IA==", + "engines": { + "node": ">=0.8" + } + }, "node_modules/fragment-cache": { "version": "0.2.1", "resolved": "https://registry.npmjs.org/fragment-cache/-/fragment-cache-0.2.1.tgz", @@ -61886,6 +61934,17 @@ "resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz", "integrity": "sha1-BOaSb2YolTVPPdAVIDYzuFcpfiw=" }, + "node_modules/ssf": { + "version": "0.11.2", + "resolved": "https://registry.npmjs.org/ssf/-/ssf-0.11.2.tgz", + "integrity": "sha512-+idbmIXoYET47hH+d7dfm2epdOMUDjqcB4648sTZ+t2JwoyBFL/insLfB/racrDmsKB3diwsDA696pZMieAC5g==", + "dependencies": { + "frac": "~1.1.2" + }, + "engines": { + "node": ">=0.8" + } + }, "node_modules/sshpk": { "version": "1.15.2", "resolved": "https://registry.npmjs.org/sshpk/-/sshpk-1.15.2.tgz", @@ -66055,6 +66114,22 @@ "integrity": "sha512-JcKqAHLPxcdb9KM49dufGXn2x3ssnfjbcaQdLlfZsL9rH9wgDQjUtDxbo8NE0F6SFvydeu1VhZe7hZuHsB2/pw==", "dev": true }, + "node_modules/wmf": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wmf/-/wmf-1.0.2.tgz", + "integrity": "sha512-/p9K7bEh0Dj6WbXg4JG0xvLQmIadrner1bi45VMJTfnbVHsc7yIajZyoSoK60/dtVBs12Fm6WkUI5/3WAVsNMw==", + "engines": { + "node": ">=0.8" + } + }, + "node_modules/word": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/word/-/word-0.3.0.tgz", + "integrity": "sha512-OELeY0Q61OXpdUfTp+oweA/vtLVg5VDOXh+3he3PNzLGG/y0oylSOC1xRVj0+l4vQ3tj/bB1HVHv1ocXkQceFA==", + "engines": { + "node": ">=0.8" + } + }, "node_modules/wordwrap": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/wordwrap/-/wordwrap-1.0.0.tgz", @@ -66369,6 +66444,26 @@ "url": "https://opencollective.com/node-fetch" } }, + "node_modules/xlsx": { + "version": "0.18.5", + "resolved": "https://registry.npmjs.org/xlsx/-/xlsx-0.18.5.tgz", + "integrity": "sha512-dmg3LCjBPHZnQp5/F/+nnTa+miPJxUXB6vtk42YjBBKayDNagxGEeIdWApkYPOf3Z3pm3k62Knjzp7lMeTEtFQ==", + "dependencies": { + "adler-32": "~1.3.0", + "cfb": "~1.2.1", + "codepage": "~1.15.0", + "crc-32": "~1.2.1", + "ssf": "~0.11.2", + "wmf": "~1.0.1", + "word": "~0.3.0" + }, + "bin": { + "xlsx": "bin/xlsx.njs" + }, + "engines": { + "node": ">=0.8" + } + }, "node_modules/xml-name-validator": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/xml-name-validator/-/xml-name-validator-3.0.0.tgz", @@ -91599,6 +91694,11 @@ "integrity": "sha512-aT6camzM4xEA54YVJYSqxz1kv4IHnQZRtThJJHhUMRExaU5spC7jX5ugSwTaTgJliIgs4VhZOk7htClvQ/LmRA==", "dev": true }, + "adler-32": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/adler-32/-/adler-32-1.3.1.tgz", + "integrity": "sha512-ynZ4w/nUUv5rrsR8UUGoe1VC9hZj6V5hU9Qw1HlMDJGEJw5S7TfTErWTjMys6M7vr0YWcPqs3qAr4ss0nDfP+A==" + }, "agent-base": { "version": "6.0.2", "resolved": "https://registry.npmjs.org/agent-base/-/agent-base-6.0.2.tgz", @@ -93705,6 +93805,15 @@ "resolved": "https://registry.npmjs.org/ccount/-/ccount-2.0.1.tgz", "integrity": "sha512-eyrF0jiFpY+3drT6383f1qhkbGsLSifNAjA61IUjZjmLCWjItY6LB9ft9YhoDgwfmclB2zhu51Lc7+95b8NRAg==" }, + "cfb": { + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/cfb/-/cfb-1.2.2.tgz", + "integrity": "sha512-KfdUZsSOw19/ObEWasvBP/Ac4reZvAGauZhs6S/gqNhXhI7cKwvlH7ulj+dOEYnca4bm4SGo8C1bTAQvnTjgQA==", + "requires": { + "adler-32": "~1.3.0", + "crc-32": "~1.2.0" + } + }, "chainsaw": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/chainsaw/-/chainsaw-0.1.0.tgz", @@ -94316,6 +94425,11 @@ "resolved": "https://registry.npmjs.org/code-point-at/-/code-point-at-1.1.0.tgz", "integrity": "sha1-DQcLTQQ6W+ozovGkDi7bPZpMz3c=" }, + "codepage": { + "version": "1.15.0", + "resolved": "https://registry.npmjs.org/codepage/-/codepage-1.15.0.tgz", + "integrity": "sha512-3g6NUTPd/YtuuGrhMnOMRjFc+LJw/bnMp3+0r/Wcz3IXUuCosKRJvMphm5+Q+bvTVGcJJuRvVLuYba+WojaFaA==" + }, "collect-v8-coverage": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/collect-v8-coverage/-/collect-v8-coverage-1.0.1.tgz", @@ -95083,6 +95197,11 @@ } } }, + "crc-32": { + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/crc-32/-/crc-32-1.2.2.tgz", + "integrity": "sha512-ROmzCKrTnOwybPcJApAA6WBWij23HVfGVNKqqrZpuyZOHqK2CwHSvpGuyt/UNNvaIjEd8X5IFGp4Mh+Ie1IHJQ==" + }, "create-emotion": { "version": "10.0.27", "resolved": "https://registry.npmjs.org/create-emotion/-/create-emotion-10.0.27.tgz", @@ -100756,6 +100875,11 @@ "fetch-blob": "^3.1.2" } }, + "frac": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/frac/-/frac-1.1.2.tgz", + "integrity": "sha512-w/XBfkibaTl3YDqASwfDUqkna4Z2p9cFSr1aHDt0WoMTECnRfBOv2WArlZILlqgWlmdIlALXGpM2AOhEk5W3IA==" + }, "fragment-cache": { "version": "0.2.1", "resolved": "https://registry.npmjs.org/fragment-cache/-/fragment-cache-0.2.1.tgz", @@ -119636,6 +119760,14 @@ "resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz", "integrity": "sha1-BOaSb2YolTVPPdAVIDYzuFcpfiw=" }, + "ssf": { + "version": "0.11.2", + "resolved": "https://registry.npmjs.org/ssf/-/ssf-0.11.2.tgz", + "integrity": "sha512-+idbmIXoYET47hH+d7dfm2epdOMUDjqcB4648sTZ+t2JwoyBFL/insLfB/racrDmsKB3diwsDA696pZMieAC5g==", + "requires": { + "frac": "~1.1.2" + } + }, "sshpk": { "version": "1.15.2", "resolved": "https://registry.npmjs.org/sshpk/-/sshpk-1.15.2.tgz", @@ -122785,6 +122917,16 @@ "integrity": "sha512-JcKqAHLPxcdb9KM49dufGXn2x3ssnfjbcaQdLlfZsL9rH9wgDQjUtDxbo8NE0F6SFvydeu1VhZe7hZuHsB2/pw==", "dev": true }, + "wmf": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wmf/-/wmf-1.0.2.tgz", + "integrity": "sha512-/p9K7bEh0Dj6WbXg4JG0xvLQmIadrner1bi45VMJTfnbVHsc7yIajZyoSoK60/dtVBs12Fm6WkUI5/3WAVsNMw==" + }, + "word": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/word/-/word-0.3.0.tgz", + "integrity": "sha512-OELeY0Q61OXpdUfTp+oweA/vtLVg5VDOXh+3he3PNzLGG/y0oylSOC1xRVj0+l4vQ3tj/bB1HVHv1ocXkQceFA==" + }, "wordwrap": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/wordwrap/-/wordwrap-1.0.0.tgz", @@ -123024,6 +123166,20 @@ } } }, + "xlsx": { + "version": "0.18.5", + "resolved": "https://registry.npmjs.org/xlsx/-/xlsx-0.18.5.tgz", + "integrity": "sha512-dmg3LCjBPHZnQp5/F/+nnTa+miPJxUXB6vtk42YjBBKayDNagxGEeIdWApkYPOf3Z3pm3k62Knjzp7lMeTEtFQ==", + "requires": { + "adler-32": "~1.3.0", + "cfb": "~1.2.1", + "codepage": "~1.15.0", + "crc-32": "~1.2.1", + "ssf": "~0.11.2", + "wmf": "~1.0.1", + "word": "~0.3.0" + } + }, "xml-name-validator": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/xml-name-validator/-/xml-name-validator-3.0.0.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index d513517d33538..2152041d58ece 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -204,6 +204,7 @@ "use-event-callback": "^0.1.0", "use-immer": "^0.9.0", "use-query-params": "^1.1.9", + "xlsx": "^0.18.5", "yargs": "^17.7.2" }, "devDependencies": { diff --git a/superset-frontend/src/features/databases/CSVUploadModal/ColumnsPreview.tsx b/superset-frontend/src/features/databases/UploadDataModel/ColumnsPreview.tsx similarity index 95% rename from superset-frontend/src/features/databases/CSVUploadModal/ColumnsPreview.tsx rename to superset-frontend/src/features/databases/UploadDataModel/ColumnsPreview.tsx index f175eb80513e4..dd0d5d85c741a 100644 --- a/superset-frontend/src/features/databases/CSVUploadModal/ColumnsPreview.tsx +++ b/superset-frontend/src/features/databases/UploadDataModel/ColumnsPreview.tsx @@ -43,7 +43,7 @@ const ColumnsPreview: React.FC = ({ Columns: {columns.length === 0 ? ( -

{t('Upload CSV file to preview columns')}

+

{t('Upload file to preview columns')}

) : ( )} diff --git a/superset-frontend/src/features/databases/CSVUploadModal/StyledFormItemWithTip.tsx b/superset-frontend/src/features/databases/UploadDataModel/StyledFormItemWithTip.tsx similarity index 100% rename from superset-frontend/src/features/databases/CSVUploadModal/StyledFormItemWithTip.tsx rename to superset-frontend/src/features/databases/UploadDataModel/StyledFormItemWithTip.tsx diff --git a/superset-frontend/src/features/databases/CSVUploadModal/CSVUploadModal.test.tsx b/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx similarity index 55% rename from superset-frontend/src/features/databases/CSVUploadModal/CSVUploadModal.test.tsx rename to superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx index 300f7eeda6704..7adb325b6d81e 100644 --- a/superset-frontend/src/features/databases/CSVUploadModal/CSVUploadModal.test.tsx +++ b/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx @@ -18,9 +18,9 @@ */ import React from 'react'; import fetchMock from 'fetch-mock'; -import CSVUploadModal, { +import UploadDataModal, { validateUploadFileExtension, -} from 'src/features/databases/CSVUploadModal'; +} from 'src/features/databases/UploadDataModel'; import { render, screen } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import { waitFor } from '@testing-library/react'; @@ -28,6 +28,7 @@ import { UploadFile } from 'antd/lib/upload/interface'; import { forEach } from 'lodash'; fetchMock.post('glob:*api/v1/database/1/csv_upload/', {}); +fetchMock.post('glob:*api/v1/database/1/excel_upload/', {}); fetchMock.get( 'glob:*api/v1/database/?q=(filters:!((col:allow_file_upload,opr:eq,value:!t)),page:0,page_size:100)', @@ -57,10 +58,18 @@ const csvProps = { show: true, onHide: () => {}, allowedExtensions: ['csv', 'tsv'], + type: 'csv', }; -test('renders the general information elements correctly', () => { - render(, { +const excelProps = { + show: true, + onHide: () => {}, + allowedExtensions: ['xls', 'xlsx'], + type: 'excel', +}; + +test('CSV, renders the general information elements correctly', () => { + render(, { useRedux: true, }); @@ -95,7 +104,6 @@ test('renders the general information elements correctly', () => { const selectDelimiter = screen.getByRole('combobox', { name: /choose a delimiter/i, }); - const inputTableName = screen.getByRole('textbox', { name: /table name/i, }); @@ -122,8 +130,78 @@ test('renders the general information elements correctly', () => { }); }); -test('renders the file settings elements correctly', () => { - render(, { +test('Excel, renders the general information elements correctly', () => { + render(, { + useRedux: true, + }); + + const cancelButton = screen.getByRole('button', { + name: 'Cancel', + }); + const uploadButton = screen.getByRole('button', { + name: 'Upload', + }); + const selectButton = screen.getByRole('button', { + name: 'Select', + }); + + const title = screen.getByRole('heading', { + name: /excel upload/i, + }); + const missingTitle = screen.queryByRole('heading', { + name: /csv upload/i, + }); + expect(missingTitle).not.toBeInTheDocument(); + const panel1 = screen.getByRole('heading', { + name: /General information/i, + }); + const panel2 = screen.getByRole('heading', { + name: /file settings/i, + }); + const panel3 = screen.getByRole('heading', { + name: /columns/i, + }); + const panel4 = screen.getByRole('heading', { + name: /rows/i, + }); + const selectDatabase = screen.getByRole('combobox', { + name: /select a database/i, + }); + const selectDelimiter = screen.queryByRole('combobox', { + name: /choose a delimiter/i, + }); + expect(selectDelimiter).not.toBeInTheDocument(); + const selectSheetName = screen.getByRole('combobox', { + name: /choose sheet name/i, + }); + const inputTableName = screen.getByRole('textbox', { + name: /table name/i, + }); + const inputSchema = screen.getByRole('combobox', { + name: /schema/i, + }); + + const visibleComponents = [ + cancelButton, + uploadButton, + selectButton, + title, + panel1, + panel2, + panel3, + panel4, + selectDatabase, + selectSheetName, + inputTableName, + inputSchema, + ]; + visibleComponents.forEach(component => { + expect(component).toBeVisible(); + }); +}); + +test('CSV, renders the file settings elements correctly', () => { + render(, { useRedux: true, }); @@ -162,8 +240,50 @@ test('renders the file settings elements correctly', () => { }); }); -test('renders the columns elements correctly', () => { - render(, { +test('Excel, renders the file settings elements correctly', () => { + render(, { + useRedux: true, + }); + + expect(screen.queryByText('If Table Already Exists')).not.toBeInTheDocument(); + const panelHeader = screen.getByRole('heading', { + name: /file settings/i, + }); + userEvent.click(panelHeader); + const selectTableAlreadyExists = screen.getByRole('combobox', { + name: /choose already exists/i, + }); + const inputDecimalCharacter = screen.getByRole('textbox', { + name: /decimal character/i, + }); + const selectColumnsDates = screen.getByRole('combobox', { + name: /choose columns to be parsed as dates/i, + }); + const selectNullValues = screen.getByRole('combobox', { + name: /null values/i, + }); + userEvent.click(selectColumnsDates); + userEvent.click(selectNullValues); + + const switchSkipInitialSpace = screen.queryByText('skipInitialSpace'); + expect(switchSkipInitialSpace).not.toBeInTheDocument(); + const switchSkipBlankLines = screen.queryByText('skipBlankLines'); + expect(switchSkipBlankLines).not.toBeInTheDocument(); + const switchDayFirst = screen.queryByText('dayFirst'); + expect(switchDayFirst).not.toBeInTheDocument(); + + const visibleComponents = [ + selectTableAlreadyExists, + inputDecimalCharacter, + selectNullValues, + ]; + visibleComponents.forEach(component => { + expect(component).toBeVisible(); + }); +}); + +test('CSV, renders the columns elements correctly', () => { + render(, { useRedux: true, }); @@ -200,8 +320,46 @@ test('renders the columns elements correctly', () => { }); }); +test('Excel, renders the columns elements correctly', () => { + render(, { + useRedux: true, + }); + + const panelHeader = screen.getByRole('heading', { + name: /columns/i, + }); + userEvent.click(panelHeader); + const selectIndexColumn = screen.getByRole('combobox', { + name: /Choose index column/i, + }); + const switchDataFrameIndex = screen.getByTestId('dataFrameIndex'); + const inputColumnLabels = screen.getByRole('textbox', { + name: /Column labels/i, + }); + const selectColumnsToRead = screen.getByRole('combobox', { + name: /Choose columns to read/i, + }); + userEvent.click(selectColumnsToRead); + + const columnDataTypes = screen.queryByRole('textbox', { + name: /Column data types/i, + }); + + expect(columnDataTypes).not.toBeInTheDocument(); + + const visibleComponents = [ + selectIndexColumn, + switchDataFrameIndex, + inputColumnLabels, + selectColumnsToRead, + ]; + visibleComponents.forEach(component => { + expect(component).toBeVisible(); + }); +}); + test('renders the rows elements correctly', () => { - render(, { + render(, { useRedux: true, }); @@ -226,7 +384,7 @@ test('renders the rows elements correctly', () => { }); test('database and schema are correctly populated', async () => { - render(, { + render(, { useRedux: true, }); @@ -256,7 +414,7 @@ test('database and schema are correctly populated', async () => { }); test('form without required fields', async () => { - render(, { + render(, { useRedux: true, }); @@ -271,8 +429,9 @@ test('form without required fields', async () => { await waitFor(() => screen.getByText('Selecting a database is required')); await waitFor(() => screen.getByText('Table name is required')); }); -test('form post', async () => { - render(, { + +test('CSV, form post', async () => { + render(, { useRedux: true, }); @@ -328,8 +487,69 @@ test('form post', async () => { expect(fileData.name).toBe('test.csv'); }); -test('validate file extension returns false', () => { - const invalidFileNames = ['out', 'out.exe', 'out.csv.exe', '.csv']; +test('Excel, form post', async () => { + render(, { + useRedux: true, + }); + + const selectButton = screen.getByRole('button', { + name: 'Select', + }); + userEvent.click(selectButton); + + // Select a file from the file dialog + const file = new File(['test'], 'test.xls', { type: 'text' }); + const inputElement = document.querySelector('input[type="file"]'); + + if (inputElement) { + userEvent.upload(inputElement, file); + } + + const selectDatabase = screen.getByRole('combobox', { + name: /select a database/i, + }); + userEvent.click(selectDatabase); + await waitFor(() => screen.getByText('database1')); + await waitFor(() => screen.getByText('database2')); + + screen.getByText('database1').click(); + const selectSchema = screen.getByRole('combobox', { + name: /schema/i, + }); + userEvent.click(selectSchema); + await waitFor(() => screen.getAllByText('public')); + screen.getAllByText('public')[1].click(); + + // Fill out form fields + const inputTableName = screen.getByRole('textbox', { + name: /table name/i, + }); + userEvent.type(inputTableName, 'table1'); + const uploadButton = screen.getByRole('button', { + name: 'Upload', + }); + + userEvent.click(uploadButton); + await waitFor(() => + fetchMock.called('glob:*api/v1/database/1/excel_upload/'), + ); + + // Get the matching fetch calls made + const matchingCalls = fetchMock.calls( + 'glob:*api/v1/database/1/excel_upload/', + ); + expect(matchingCalls).toHaveLength(1); + const [_, options] = matchingCalls[0]; + const formData = options?.body as FormData; + expect(formData.get('table_name')).toBe('table1'); + expect(formData.get('schema')).toBe('public'); + expect(formData.get('table_name')).toBe('table1'); + const fileData = formData.get('file') as File; + expect(fileData.name).toBe('test.xls'); +}); + +test('CSV, validate file extension returns false', () => { + const invalidFileNames = ['out', 'out.exe', 'out.csv.exe', '.csv', 'out.xls']; forEach(invalidFileNames, fileName => { const file: UploadFile = { name: fileName, @@ -341,7 +561,20 @@ test('validate file extension returns false', () => { }); }); -test('validate file extension returns true', () => { +test('Excel, validate file extension returns false', () => { + const invalidFileNames = ['out', 'out.exe', 'out.xls.exe', '.csv', 'out.csv']; + forEach(invalidFileNames, fileName => { + const file: UploadFile = { + name: fileName, + uid: 'xp', + size: 100, + type: 'text/csv', + }; + expect(validateUploadFileExtension(file, ['xls', 'xlsx'])).toBe(false); + }); +}); + +test('CSV, validate file extension returns true', () => { const invalidFileNames = ['out.csv', 'out.tsv', 'out.exe.csv', 'out a.csv']; forEach(invalidFileNames, fileName => { const file: UploadFile = { @@ -353,3 +586,16 @@ test('validate file extension returns true', () => { expect(validateUploadFileExtension(file, ['csv', 'tsv'])).toBe(true); }); }); + +test('Excel, validate file extension returns true', () => { + const invalidFileNames = ['out.xls', 'out.xlsx', 'out.exe.xls', 'out a.xls']; + forEach(invalidFileNames, fileName => { + const file: UploadFile = { + name: fileName, + uid: 'xp', + size: 100, + type: 'text/csv', + }; + expect(validateUploadFileExtension(file, ['xls', 'xlsx'])).toBe(true); + }); +}); diff --git a/superset-frontend/src/features/databases/CSVUploadModal/index.tsx b/superset-frontend/src/features/databases/UploadDataModel/index.tsx similarity index 65% rename from superset-frontend/src/features/databases/CSVUploadModal/index.tsx rename to superset-frontend/src/features/databases/UploadDataModel/index.tsx index 9f19bfd5e1ee4..2fcb267aed83d 100644 --- a/superset-frontend/src/features/databases/CSVUploadModal/index.tsx +++ b/superset-frontend/src/features/databases/UploadDataModel/index.tsx @@ -25,25 +25,26 @@ import { } from '@superset-ui/core'; import Modal from 'src/components/Modal'; import Button from 'src/components/Button'; -import { Switch } from 'src/components/Switch'; +import { Switch, SwitchProps } from 'src/components/Switch'; import Collapse from 'src/components/Collapse'; import { - Upload, AntdForm, + AsyncSelect, Col, Row, - AsyncSelect, Select, + Upload, } from 'src/components'; import { UploadOutlined } from '@ant-design/icons'; import { Input, InputNumber } from 'src/components/Input'; import rison from 'rison'; import { UploadChangeParam, UploadFile } from 'antd/lib/upload/interface'; import withToasts from 'src/components/MessageToasts/withToasts'; +import * as XLSX from 'xlsx'; import { - antDModalStyles, - antDModalNoPaddingStyles, antdCollapseStyles, + antDModalNoPaddingStyles, + antDModalStyles, formStyles, StyledFormItem, StyledSwitchContainer, @@ -51,18 +52,48 @@ import { import ColumnsPreview from './ColumnsPreview'; import StyledFormItemWithTip from './StyledFormItemWithTip'; -interface CSVUploadModalProps { +type UploadType = 'csv' | 'excel' | 'columnar'; + +interface UploadDataModalProps { addDangerToast: (msg: string) => void; addSuccessToast: (msg: string) => void; onHide: () => void; show: boolean; allowedExtensions: string[]; + type: UploadType; } +const CSVSpecificFields = [ + 'delimiter', + 'skip_initial_space', + 'skip_blank_lines', + 'day_first', + 'overwrite_duplicates', + 'column_data_types', +]; + +const ExcelSpecificFields = ['sheet_name']; + +const ColumnarSpecificFields: string[] = []; + +const NonNullFields = ['rows_to_read', 'index_column']; + +const AllSpecificFields = [ + ...CSVSpecificFields, + ...ExcelSpecificFields, + ...ColumnarSpecificFields, +]; + +const UploadTypeToSpecificFields: Record = { + csv: CSVSpecificFields, + excel: ExcelSpecificFields, + columnar: ColumnarSpecificFields, +}; + interface UploadInfo { - database_id: number; table_name: string; schema: string; + sheet_name?: string; delimiter: string; already_exists: string; skip_initial_space: boolean; @@ -83,9 +114,9 @@ interface UploadInfo { } const defaultUploadInfo: UploadInfo = { - database_id: 0, table_name: '', schema: '', + sheet_name: undefined, delimiter: ',', already_exists: 'fail', skip_initial_space: false, @@ -108,7 +139,6 @@ const defaultUploadInfo: UploadInfo = { // Allowed extensions to accept for file upload, users can always override this // by selecting all file extensions on the OS file picker. Also ".txt" will // allow all files to be selected. -const allowedExtensionsToAccept = '.csv, .tsv'; const READ_HEADER_SIZE = 10000; export const validateUploadFileExtension = ( @@ -124,33 +154,60 @@ export const validateUploadFileExtension = ( return allowedExtensions.includes(fileType); }; -const SwitchContainer: React.FC<{ label: string; dataTest: string }> = ({ +interface StyledSwitchContainerProps extends SwitchProps { + label: string; + dataTest: string; + children?: React.ReactNode; +} + +const SwitchContainer = ({ label, dataTest, children, -}) => ( + ...switchProps +}: StyledSwitchContainerProps) => ( - +
{label}
{children} ); -const CSVUploadModal: FunctionComponent = ({ +const UploadDataModal: FunctionComponent = ({ addDangerToast, addSuccessToast, onHide, show, allowedExtensions, + type = 'csv', }) => { const [form] = AntdForm.useForm(); - // Declare states here const [currentDatabaseId, setCurrentDatabaseId] = useState(0); const [fileList, setFileList] = useState([]); const [columns, setColumns] = React.useState([]); + const [sheetNames, setSheetNames] = React.useState([]); + const [currentSheetName, setCurrentSheetName] = React.useState< + string | undefined + >(); const [delimiter, setDelimiter] = useState(','); const [isLoading, setIsLoading] = useState(false); const [currentSchema, setCurrentSchema] = useState(); + const [previewUploadedFile, setPreviewUploadedFile] = useState(true); + const [fileLoading, setFileLoading] = useState(false); + + const allowedExtensionsToAccept = { + csv: '.csv, .tsv', + excel: '.xls, .xlsx', + columnar: '.parquet, .orc', + }; + + const createTypeToEndpointMap = ( + databaseId: number, + ): { [key: string]: string } => ({ + csv: `/api/v1/database/${databaseId}/csv_upload/`, + excel: `/api/v1/database/${databaseId}/excel_upload/`, + columnar: `/api/v1/database/${databaseId}/columnar_upload/`, + }); const nullValuesOptions = [ { @@ -209,6 +266,10 @@ const CSVUploadModal: FunctionComponent = ({ }, ]; + const onChangePreviewUploadedFile = (value: boolean) => { + setPreviewUploadedFile(value); + }; + const onChangeDatabase = (database: { value: number; label: string }) => { setCurrentDatabaseId(database?.value); setCurrentSchema(undefined); @@ -228,7 +289,12 @@ const CSVUploadModal: FunctionComponent = ({ setColumns([]); setCurrentSchema(''); setCurrentDatabaseId(0); + setCurrentSheetName(undefined); + setSheetNames([]); setIsLoading(false); + setDelimiter(','); + setPreviewUploadedFile(true); + setFileLoading(false); form.resetFields(); }; @@ -280,6 +346,22 @@ const CSVUploadModal: FunctionComponent = ({ [currentDatabaseId], ); + const getAllFieldsNotInType = (): string[] => { + const specificFields = UploadTypeToSpecificFields[type] || []; + return [...AllSpecificFields].filter( + field => !specificFields.includes(field), + ); + }; + + const appendFormData = (formData: FormData, data: Record) => { + const allFieldsNotInType = getAllFieldsNotInType(); + Object.entries(data).forEach(([key, value]) => { + if (!(allFieldsNotInType.includes(key) || NonNullFields.includes(key))) { + formData.append(key, value); + } + }); + }; + const onClose = () => { clearModal(); onHide(); @@ -287,7 +369,7 @@ const CSVUploadModal: FunctionComponent = ({ const onFinish = () => { const fields = form.getFieldsValue(); - fields.database_id = currentDatabaseId; + delete fields.database; fields.schema = currentSchema; const mergedValues = { ...defaultUploadInfo, ...fields }; const formData = new FormData(); @@ -295,37 +377,16 @@ const CSVUploadModal: FunctionComponent = ({ if (file) { formData.append('file', file); } - formData.append('delimiter', mergedValues.delimiter); - formData.append('table_name', mergedValues.table_name); - formData.append('schema', mergedValues.schema); - formData.append('already_exists', mergedValues.already_exists); - formData.append('skip_initial_space', mergedValues.skip_initial_space); - formData.append('skip_blank_lines', mergedValues.skip_blank_lines); - formData.append('day_first', mergedValues.day_first); - formData.append('decimal_character', mergedValues.decimal_character); - formData.append('null_values', mergedValues.null_values); - formData.append('header_row', mergedValues.header_row); - if (mergedValues.rows_to_read != null) { - formData.append('rows_to_read', mergedValues.rows_to_read); - } - formData.append('skip_rows', mergedValues.skip_rows); - formData.append('column_dates', mergedValues.column_dates); - if (mergedValues.index_column != null) { - formData.append('index_column', mergedValues.index_column); - } - formData.append('dataframe_index', mergedValues.dataframe_index); - formData.append('column_labels', mergedValues.column_labels); - formData.append('columns_read', mergedValues.columns_read); - formData.append('overwrite_duplicates', mergedValues.overwrite_duplicates); - formData.append('column_data_types', mergedValues.column_data_types); + appendFormData(formData, mergedValues); setIsLoading(true); + const endpoint = createTypeToEndpointMap(currentDatabaseId)[type]; return SupersetClient.post({ - endpoint: `/api/v1/database/${currentDatabaseId}/csv_upload/`, + endpoint, body: formData, headers: { Accept: 'application/json' }, }) .then(() => { - addSuccessToast(t('CSV Imported')); + addSuccessToast(t('Data Imported')); setIsLoading(false); onClose(); }) @@ -342,15 +403,28 @@ const CSVUploadModal: FunctionComponent = ({ const onRemoveFile = (removedFile: UploadFile) => { setFileList(fileList.filter(file => file.uid !== removedFile.uid)); setColumns([]); + setSheetNames([]); + setCurrentSheetName(undefined); + form.setFieldsValue({ sheet_name: undefined }); return false; }; + const onSheetNameChange = (value: string) => { + setCurrentSheetName(value); + }; + const columnsToOptions = () => columns.map(column => ({ value: column, label: column, })); + const sheetNamesToOptions = () => + sheetNames.map(sheetName => ({ + value: sheetName, + label: sheetName, + })); + const readFileContent = (file: File) => new Promise((resolve, reject) => { const reader = new FileReader(); @@ -368,19 +442,80 @@ const CSVUploadModal: FunctionComponent = ({ reader.readAsText(file.slice(0, READ_HEADER_SIZE)); }); - const processFileContent = async (file: File) => { + const processCSVFile = async (file: File) => { try { + setFileLoading(true); const text = await readFileContent(file); const firstLine = text.split('\n')[0].trim(); const firstRow = firstLine .split(delimiter) .map(column => column.replace(/^"(.*)"$/, '$1')); setColumns(firstRow); + setFileLoading(false); } catch (error) { addDangerToast('Failed to process file content'); + setFileLoading(false); } }; + const processExcelColumns = (workbook: XLSX.WorkBook, sn: string[]) => { + if (!workbook) { + return; + } + let cSheetName = currentSheetName; + if (!currentSheetName) { + setCurrentSheetName(sn[0]); + cSheetName = sn[0]; + } + cSheetName = cSheetName || sn[0]; + form.setFieldsValue({ sheet_name: cSheetName }); + const worksheet = workbook.Sheets[cSheetName]; + + const worksheetRef: string = worksheet['!ref'] ? worksheet['!ref'] : ''; + const range = XLSX.utils.decode_range(worksheetRef); + const columnNames = Array.from({ length: range.e.c + 1 }, (_, i) => { + const cellAddress = XLSX.utils.encode_cell({ r: 0, c: i }); + return worksheet[cellAddress]?.v; + }); + setColumns(columnNames); + }; + + const processExcelFile = async (file: File) => + new Promise((resolve, reject) => { + setFileLoading(true); + const reader = new FileReader(); + reader.readAsBinaryString(file); + + reader.onload = event => { + if (!event.target && event.target == null) { + reader.onerror = () => { + reject(new Error('Failed to read file content')); + }; + return; + } + // Read workbook + const workbook = XLSX.read(event.target.result, { type: 'binary' }); + if (workbook == null) { + reject(new Error('Failed to process file content')); + addDangerToast('Failed to process file content'); + setFileLoading(false); + return; + } + // Extract sheet names + const tmpSheetNames = workbook.SheetNames; + if (tmpSheetNames.length < 1) { + reject(new Error('Failed to read file content')); + addDangerToast('Failed to process file content'); + setFileLoading(false); + return; + } + processExcelColumns(workbook, tmpSheetNames); + setSheetNames(workbook.SheetNames); + setFileLoading(false); + resolve('success'); + }; + }); + const onChangeFile = async (info: UploadChangeParam) => { setFileList([ { @@ -388,7 +523,17 @@ const CSVUploadModal: FunctionComponent = ({ status: 'done', }, ]); - await processFileContent(info.file.originFileObj); + if (!previewUploadedFile) { + return; + } + if (type === 'csv') { + await processCSVFile(info.file.originFileObj); + } + if (type === 'excel') { + setSheetNames([]); + setCurrentSheetName(undefined); + await processExcelFile(info.file.originFileObj); + } }; useEffect(() => { @@ -397,10 +542,28 @@ const CSVUploadModal: FunctionComponent = ({ fileList[0].originFileObj && fileList[0].originFileObj instanceof File ) { - processFileContent(fileList[0].originFileObj).then(r => r); + if (!previewUploadedFile) { + return; + } + processCSVFile(fileList[0].originFileObj).then(r => r); } }, [delimiter]); + useEffect(() => { + (async () => { + if ( + columns.length > 0 && + fileList[0].originFileObj && + fileList[0].originFileObj instanceof File + ) { + if (!previewUploadedFile) { + return; + } + await processExcelFile(fileList[0].originFileObj); + } + })(); + }, [currentSheetName]); + const validateUpload = (_: any, value: string) => { if (fileList.length === 0) { return Promise.reject(t('Uploading a file is required')); @@ -423,6 +586,17 @@ const CSVUploadModal: FunctionComponent = ({ return Promise.resolve(); }; + const uploadTitles = { + csv: t('CSV Upload'), + excel: t('Excel Upload'), + columnar: t('Columnar Upload'), + }; + + const UploadTitle: React.FC = () => { + const title = uploadTitles[type] || t('Upload'); + return

{title}

; + }; + return ( [ @@ -432,14 +606,14 @@ const CSVUploadModal: FunctionComponent = ({ ]} primaryButtonLoading={isLoading} name="database" - data-test="csvupload-modal" + data-test="upload-modal" onHandledPrimaryAction={form.submit} onHide={onClose} width="500px" primaryButtonName="Upload" centered show={show} - title={

{t('CSV Upload')}

} + title={} > = ({ header={

{t('General information')}

-

- {t('Upload a CSV file to a database.')} -

+

{t('Upload a file to a database.')}

} key="general" > - + @@ -477,31 +649,47 @@ const CSVUploadModal: FunctionComponent = ({ name="modelFile" id="modelFile" data-test="model-file-input" - accept={allowedExtensionsToAccept} + accept={allowedExtensionsToAccept[type]} fileList={fileList} onChange={onChangeFile} onRemove={onRemoveFile} // upload is handled by hook customRequest={() => {}} > - - - - - + + + + + {previewUploadedFile && ( + + + + + + )} = ({ name="table_name" data-test="properties-modal-name-input" type="text" - placeholder={t('Name of table to be created with CSV file')} + placeholder={t('Name of table to be created')} /> - - + + )} + {type === 'excel' && ( + + - - - + {type === 'csv' && ( + + + + + + + + )} @@ -766,18 +975,20 @@ const CSVUploadModal: FunctionComponent = ({ - - - - - - - + {type === 'csv' && ( + + + + + + + + )} = ({ ); }; -export default withToasts(CSVUploadModal); +export default withToasts(UploadDataModal); diff --git a/superset-frontend/src/features/databases/CSVUploadModal/styles.ts b/superset-frontend/src/features/databases/UploadDataModel/styles.ts similarity index 100% rename from superset-frontend/src/features/databases/CSVUploadModal/styles.ts rename to superset-frontend/src/features/databases/UploadDataModel/styles.ts diff --git a/superset-frontend/src/features/home/RightMenu.test.tsx b/superset-frontend/src/features/home/RightMenu.test.tsx index 36715ffe875ed..30706459430f7 100644 --- a/superset-frontend/src/features/home/RightMenu.test.tsx +++ b/superset-frontend/src/features/home/RightMenu.test.tsx @@ -63,12 +63,6 @@ const dropdownItems = [ url: '/columnartodatabaseview/form', perm: true, }, - { - label: 'Upload Excel file to database', - name: 'Upload Excel', - url: '/exceltodatabaseview/form', - perm: true, - }, ], }, { @@ -176,6 +170,7 @@ const resetUseSelectorMock = () => { roles: { Admin: [ ['can_csv_upload', 'Database'], // So we can upload CSV + ['can_excel_upload', 'Database'], // So we can upload CSV ['can_write', 'Database'], // So we can write DBs ['can_write', 'Dataset'], // So we can write Datasets ['can_write', 'Chart'], // So we can write Datasets @@ -316,7 +311,10 @@ test('If there is a DB with allow_file_upload set as True the option should be e userEvent.hover(dataMenu); expect( (await screen.findByText('Upload CSV to database')).closest('a'), - ).toHaveAttribute('href', '/csvtodatabaseview/form'); + ).toHaveAttribute('href', '#'); + expect( + (await screen.findByText('Upload Excel to database')).closest('a'), + ).toHaveAttribute('href', '#'); }); test('If there is NOT a DB with allow_file_upload set as True the option should be disabled', async () => { diff --git a/superset-frontend/src/features/home/RightMenu.tsx b/superset-frontend/src/features/home/RightMenu.tsx index 438ca14197ad2..ff4a6a8095469 100644 --- a/superset-frontend/src/features/home/RightMenu.tsx +++ b/superset-frontend/src/features/home/RightMenu.tsx @@ -189,7 +189,7 @@ const RightMenu = ({ { label: t('Upload CSV to database'), name: 'Upload a CSV', - url: '/csvtodatabaseview/form', + url: '#', perm: canUploadCSV && showUploads, disable: isAdmin && !allowUploads, }, @@ -201,9 +201,9 @@ const RightMenu = ({ disable: isAdmin && !allowUploads, }, { - label: t('Upload Excel file to database'), + label: t('Upload Excel to database'), name: 'Upload Excel', - url: '/exceltodatabaseview/form', + url: '#', perm: canUploadExcel && showUploads, disable: isAdmin && !allowUploads, }, diff --git a/superset-frontend/src/pages/DatabaseList/index.tsx b/superset-frontend/src/pages/DatabaseList/index.tsx index c8d838c1d54d1..8b8779de00143 100644 --- a/superset-frontend/src/pages/DatabaseList/index.tsx +++ b/superset-frontend/src/pages/DatabaseList/index.tsx @@ -49,7 +49,7 @@ import { ExtensionConfigs } from 'src/features/home/types'; import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; import type { MenuObjectProps } from 'src/types/bootstrapTypes'; import DatabaseModal from 'src/features/databases/DatabaseModal'; -import CSVUploadModal from 'src/features/databases/CSVUploadModal'; +import UploadDataModal from 'src/features/databases/UploadDataModel'; import { DatabaseObject } from 'src/features/databases/types'; import { ModifiedInfo } from 'src/components/AuditInfo'; import { QueryObjectColumns } from 'src/views/CRUD/types'; @@ -136,7 +136,10 @@ function DatabaseList({ const [currentDatabase, setCurrentDatabase] = useState( null, ); - const [csvUploadModalOpen, setCsvUploadModalOpen] = useState(false); + const [csvUploadDataModalOpen, setCsvUploadDataModalOpen] = + useState(false); + const [excelUploadDataModalOpen, setExcelUploadDataModalOpen] = + useState(false); const [allowUploads, setAllowUploads] = useState(false); const isAdmin = isUserAdmin(fullUser); @@ -238,11 +241,21 @@ function DatabaseList({ name: 'Upload CSV file', url: '#', onClick: () => { - setCsvUploadModalOpen(true); + setCsvUploadDataModalOpen(true); }, perm: canUploadCSV && showUploads, disable: isDisabled, }, + { + label: t('Upload Excel'), + name: 'Upload Excel file', + url: '#', + onClick: () => { + setExcelUploadDataModalOpen(true); + }, + perm: canUploadExcel && showUploads, + disable: isDisabled, + }, { label: t('Upload columnar file'), name: 'Upload columnar file', @@ -250,13 +263,6 @@ function DatabaseList({ perm: canUploadColumnar && showUploads, disable: isDisabled, }, - { - label: t('Upload Excel file'), - name: 'Upload Excel file', - url: '/exceltodatabaseview/form', - perm: canUploadExcel && showUploads, - disable: isDisabled, - }, ], }, ]; @@ -379,7 +385,7 @@ function DatabaseList({ }, { accessor: 'allow_file_upload', - Header: t('CSV upload'), + Header: t('File upload'), Cell: ({ row: { original: { allow_file_upload: allowFileUpload }, @@ -563,15 +569,25 @@ function DatabaseList({ refreshData(); }} /> - { - setCsvUploadModalOpen(false); + setCsvUploadDataModalOpen(false); }} - show={csvUploadModalOpen} + show={csvUploadDataModalOpen} allowedExtensions={CSV_EXTENSIONS} /> + { + setExcelUploadDataModalOpen(false); + }} + show={excelUploadDataModalOpen} + allowedExtensions={EXCEL_EXTENSIONS} + type="excel" + /> {databaseCurrentlyDeleting && ( None: + self._model_id = model_id + self._model: Optional[Database] = None + self._table_name = table_name + self._schema = options.get("schema") + self._file = file + self._options = options + + def _read_excel(self) -> pd.DataFrame: + """ + Read Excel file into a DataFrame + + :return: pandas DataFrame + :throws DatabaseUploadFailed: if there is an error reading the CSV file + """ + + kwargs = { + "header": self._options.get("header_row", 0), + "index_col": self._options.get("index_column"), + "io": self._file, + "keep_default_na": not self._options.get("null_values"), + "na_values": self._options.get("null_values") + if self._options.get("null_values") # None if an empty list + else None, + "parse_dates": self._options.get("column_dates"), + "skiprows": self._options.get("skip_rows", 0), + "sheet_name": self._options.get("sheet_name", 0), + "nrows": self._options.get("rows_to_read"), + } + if self._options.get("columns_read"): + kwargs["usecols"] = self._options.get("columns_read") + try: + return pd.read_excel(**kwargs) + except ( + pd.errors.ParserError, + pd.errors.EmptyDataError, + UnicodeDecodeError, + ValueError, + ) as ex: + raise DatabaseUploadFailed( + message=_("Parsing error: %(error)s", error=str(ex)) + ) from ex + except Exception as ex: + raise DatabaseUploadFailed(_("Error reading Excel file")) from ex + + def _dataframe_to_database(self, df: pd.DataFrame, database: Database) -> None: + """ + Upload DataFrame to database + + :param df: + :throws DatabaseUploadFailed: if there is an error uploading the DataFrame + """ + try: + data_table = Table(table=self._table_name, schema=self._schema) + database.db_engine_spec.df_to_sql( + database, + data_table, + df, + to_sql_kwargs={ + "chunksize": READ_EXCEL_CHUNK_SIZE, + "if_exists": self._options.get("already_exists", "fail"), + "index": self._options.get("index_column"), + "index_label": self._options.get("column_labels"), + }, + ) + except ValueError as ex: + raise DatabaseUploadFailed( + message=_( + "Table already exists. You can change your " + "'if table already exists' strategy to append or " + "replace or provide a different Table Name to use." + ) + ) from ex + except Exception as ex: + raise DatabaseUploadFailed(exception=ex) from ex + + def run(self) -> None: + self.validate() + if not self._model: + return + + df = self._read_excel() + self._dataframe_to_database(df, self._model) + + sqla_table = ( + db.session.query(SqlaTable) + .filter_by( + table_name=self._table_name, + schema=self._schema, + database_id=self._model_id, + ) + .one_or_none() + ) + if not sqla_table: + sqla_table = SqlaTable( + table_name=self._table_name, + database=self._model, + database_id=self._model_id, + owners=[get_user()], + schema=self._schema, + ) + db.session.add(sqla_table) + + sqla_table.fetch_metadata() + + try: + db.session.commit() + except SQLAlchemyError as ex: + db.session.rollback() + raise DatabaseUploadSaveMetadataFailed() from ex + + def validate(self) -> None: + self._model = DatabaseDAO.find_by_id(self._model_id) + if not self._model: + raise DatabaseNotFoundError() + if not schema_allows_file_upload(self._model, self._schema): + raise DatabaseSchemaUploadNotAllowed() diff --git a/superset/databases/api.py b/superset/databases/api.py index 4537fc8bc512f..b5552d1ab9b13 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -36,6 +36,7 @@ from superset.commands.database.create import CreateDatabaseCommand from superset.commands.database.csv_import import CSVImportCommand from superset.commands.database.delete import DeleteDatabaseCommand +from superset.commands.database.excel_import import ExcelImportCommand from superset.commands.database.exceptions import ( DatabaseConnectionFailedError, DatabaseCreateFailedError, @@ -82,6 +83,7 @@ DatabaseTablesResponse, DatabaseTestConnectionSchema, DatabaseValidateParametersSchema, + ExcelUploadPostSchema, get_export_ids_schema, OAuth2ProviderResponseSchema, openapi_spec_methods_override, @@ -147,6 +149,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): "schemas_access_for_file_upload", "get_connection", "csv_upload", + "excel_upload", "oauth2", } @@ -266,6 +269,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): DatabaseTablesResponse, DatabaseTestConnectionSchema, DatabaseValidateParametersSchema, + ExcelUploadPostSchema, TableExtraMetadataResponseSchema, TableMetadataResponseSchema, SelectStarResponseSchema, @@ -1497,6 +1501,65 @@ def csv_upload(self, pk: int) -> Response: return self.response_400(message=error.messages) return self.response(200, message="OK") + @expose("//excel_upload/", methods=("POST",)) + @protect() + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_", + log_to_statsd=False, + ) + @requires_form_data + def excel_upload(self, pk: int) -> Response: + """Upload an Excel file into a database. + --- + post: + summary: Upload an Excel file to a database table + parameters: + - in: path + schema: + type: integer + name: pk + requestBody: + required: true + content: + multipart/form-data: + schema: + $ref: '#/components/schemas/ExcelUploadPostSchema' + responses: + 200: + description: Excel upload response + content: + application/json: + schema: + type: object + properties: + message: + type: string + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + try: + request_form = request.form.to_dict() + request_form["file"] = request.files.get("file") + parameters = ExcelUploadPostSchema().load(request_form) + ExcelImportCommand( + pk, + parameters["table_name"], + parameters["file"], + parameters, + ).run() + except ValidationError as error: + return self.response_400(message=error.messages) + return self.response(200, message="OK") + @expose("//function_names/", methods=("GET",)) @protect() @safe diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index dccab980bcd18..5843473bc0ff7 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -1009,20 +1009,7 @@ def _deserialize( ) from exc -class CSVUploadPostSchema(Schema): - """ - Schema for CSV Upload - """ - - file = fields.Raw( - required=True, - metadata={ - "description": "The CSV file to upload", - "type": "string", - "format": "text/csv", - }, - ) - delimiter = fields.String(metadata={"description": "The delimiter of the CSV file"}) +class BaseUploadPostSchema(Schema): already_exists = fields.String( load_default="fail", validate=OneOf(choices=("fail", "replace", "append")), @@ -1031,14 +1018,6 @@ class CSVUploadPostSchema(Schema): "exists accepts: fail, replace, append" }, ) - column_data_types = fields.String( - metadata={ - "description": "A dictionary with column names and " - "their data types if you need to change " - "the defaults. Example: {'user_id':'int'}. " - "Check Python Pandas library for supported data types" - } - ) column_dates = DelimitedListField( fields.String(), metadata={ @@ -1063,11 +1042,6 @@ class CSVUploadPostSchema(Schema): "Leave empty if no index column" } ) - day_first = fields.Boolean( - metadata={ - "description": "DD/MM format dates, international and European format" - } - ) decimal_character = fields.String( metadata={ "description": "Character to recognize as decimal point. Default is '.'" @@ -1093,12 +1067,6 @@ class CSVUploadPostSchema(Schema): "Warning: Hive database supports only a single value" }, ) - overwrite_duplicates = fields.Boolean( - metadata={ - "description": "If duplicate columns are not overridden," - "they will be presented as 'X.1, X.2 ...X.x'." - } - ) rows_to_read = fields.Integer( metadata={ "description": "Number of rows to read from the file. " @@ -1108,16 +1076,7 @@ class CSVUploadPostSchema(Schema): validate=Range(min=1), ) schema = fields.String( - metadata={"description": "The schema to upload the CSV file to."} - ) - skip_blank_lines = fields.Boolean( - metadata={"description": "Skip blank lines in the CSV file."} - ) - skip_initial_space = fields.Boolean( - metadata={"description": "Skip spaces after delimiter."} - ) - skip_rows = fields.Integer( - metadata={"description": "Number of rows to skip at start of file."} + metadata={"description": "The schema to upload the data file to."} ) table_name = fields.String( required=True, @@ -1125,6 +1084,50 @@ class CSVUploadPostSchema(Schema): allow_none=False, metadata={"description": "The name of the table to be created/appended"}, ) + skip_rows = fields.Integer( + metadata={"description": "Number of rows to skip at start of file."} + ) + + +class CSVUploadPostSchema(BaseUploadPostSchema): + """ + Schema for CSV Upload + """ + + file = fields.Raw( + required=True, + metadata={ + "description": "The CSV file to upload", + "type": "string", + "format": "text/csv", + }, + ) + delimiter = fields.String(metadata={"description": "The delimiter of the CSV file"}) + column_data_types = fields.String( + metadata={ + "description": "A dictionary with column names and " + "their data types if you need to change " + "the defaults. Example: {'user_id':'int'}. " + "Check Python Pandas library for supported data types" + } + ) + day_first = fields.Boolean( + metadata={ + "description": "DD/MM format dates, international and European format" + } + ) + overwrite_duplicates = fields.Boolean( + metadata={ + "description": "If duplicate columns are not overridden," + "they will be presented as 'X.1, X.2 ...X.x'." + } + ) + skip_blank_lines = fields.Boolean( + metadata={"description": "Skip blank lines in the CSV file."} + ) + skip_initial_space = fields.Boolean( + metadata={"description": "Skip spaces after delimiter."} + ) @post_load def convert_column_data_types( @@ -1162,6 +1165,39 @@ def validate_file_extension(self, file: FileStorage) -> None: raise ValidationError([_("File extension is not allowed.")]) +class ExcelUploadPostSchema(BaseUploadPostSchema): + """ + Schema for Excel Upload + """ + + file = fields.Raw( + required=True, + metadata={ + "description": "The Excel file to upload", + "type": "string", + "format": "binary", + }, + ) + sheet_name = fields.String( + metadata={ + "description": "Strings used for sheet names " + "(default is the first sheet)." + } + ) + + @validates("file") + def validate_file_extension(self, file: FileStorage) -> None: + allowed_extensions = current_app.config["ALLOWED_EXTENSIONS"].intersection( + current_app.config["EXCEL_EXTENSIONS"] + ) + matches = re.match(r".+\.([^.]+)$", file.filename) + if not matches: + raise ValidationError([_("File extension is not allowed.")]) + extension = matches.group(1) + if extension not in allowed_extensions: + raise ValidationError([_("File extension is not allowed.")]) + + class OAuth2ProviderResponseSchema(Schema): """ Schema for the payload sent on OAuth2 redirect. diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index e9060d0076a58..069ed19483d9f 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -170,11 +170,7 @@ def init_views(self) -> None: DashboardModelView, DashboardModelViewAsync, ) - from superset.views.database.views import ( - ColumnarToDatabaseView, - DatabaseView, - ExcelToDatabaseView, - ) + from superset.views.database.views import ColumnarToDatabaseView, DatabaseView from superset.views.datasource.views import DatasetEditor, Datasource from superset.views.dynamic_plugins import DynamicPluginsView from superset.views.explore import ExplorePermalinkView, ExploreView @@ -295,7 +291,6 @@ def init_views(self) -> None: # appbuilder.add_view_no_menu(Api) appbuilder.add_view_no_menu(CssTemplateAsyncModelView) - appbuilder.add_view_no_menu(ExcelToDatabaseView) appbuilder.add_view_no_menu(ColumnarToDatabaseView) appbuilder.add_view_no_menu(Dashboard) appbuilder.add_view_no_menu(DashboardModelViewAsync) diff --git a/superset/migrations/versions/2024-04-17_14-04_d60591c5515f_mig_new_excel_upload_perm.py b/superset/migrations/versions/2024-04-17_14-04_d60591c5515f_mig_new_excel_upload_perm.py new file mode 100644 index 0000000000000..c400212d926fa --- /dev/null +++ b/superset/migrations/versions/2024-04-17_14-04_d60591c5515f_mig_new_excel_upload_perm.py @@ -0,0 +1,87 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""mig new excel upload perm + +Revision ID: d60591c5515f +Revises: 5ad7321c2169 +Create Date: 2024-04-17 14:04:36.041749 + +""" + +# revision identifiers, used by Alembic. +revision = "d60591c5515f" +down_revision = "5ad7321c2169" + +from alembic import op +from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.orm import Session + +from superset.migrations.shared.security_converge import ( + add_pvms, + get_reversed_new_pvms, + get_reversed_pvm_map, + migrate_roles, + Pvm, +) + +NEW_PVMS = {"Database": ("can_excel_upload",)} + +PVM_MAP = { + Pvm("ExcelToDatabaseView", "can_this_form_post"): ( + Pvm("Database", "can_excel_upload"), + ), + Pvm("ExcelToDatabaseView", "can_this_form_get"): ( + Pvm("Database", "can_excel_upload"), + ), +} + + +def do_upgrade(session: Session) -> None: + add_pvms(session, NEW_PVMS) + migrate_roles(session, PVM_MAP) + + +def do_downgrade(session: Session) -> None: + add_pvms(session, get_reversed_new_pvms(PVM_MAP)) + migrate_roles(session, get_reversed_pvm_map(PVM_MAP)) + + +def upgrade(): + bind = op.get_bind() + session = Session(bind=bind) + + do_upgrade(session) + + try: + session.commit() + except SQLAlchemyError as ex: + session.rollback() + raise Exception(f"An error occurred while upgrading permissions: {ex}") + + +def downgrade(): + bind = op.get_bind() + session = Session(bind=bind) + + do_downgrade(session) + + try: + session.commit() + except SQLAlchemyError as ex: + print(f"An error occurred while downgrading permissions: {ex}") + session.rollback() + pass diff --git a/superset/security/manager.py b/superset/security/manager.py index 52c81513aa6b9..5037e9bace98d 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -253,6 +253,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods ALPHA_ONLY_PMVS = { ("can_csv_upload", "Database"), + ("can_excel_upload", "Database"), } ADMIN_ONLY_PERMISSIONS = { diff --git a/superset/templates/superset/form_view/csv_macros.html b/superset/templates/superset/form_view/csv_macros.html deleted file mode 100644 index 40c7bf54a0b27..0000000000000 --- a/superset/templates/superset/form_view/csv_macros.html +++ /dev/null @@ -1,75 +0,0 @@ -{# -Licensed to the Apache Software Foundation (ASF) under one -or more contributor license agreements. See the NOTICE file -distributed with this work for additional information -regarding copyright ownership. The ASF licenses this file -to you under the Apache License, Version 2.0 (the -"License"); you may not use this file except in compliance -with the License. You may obtain a copy of the License at - -http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, -software distributed under the License is distributed on an -"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -KIND, either express or implied. See the License for the -specific language governing permissions and limitations -under the License. -#} -{% macro render_delimiter_field(field, begin_sep_label='', end_sep_label='', begin_sep_field='', end_sep_field='') %} - {% if field.id != 'csrf_token' %} - {% if field.type == 'HiddenField' %} - {{ field}} - {% else %} - {{begin_sep_label|safe}} - - {{end_sep_label|safe}} - {{begin_sep_field|safe}} - {{ field(**kwargs)|safe }} - - {{ field.description }} - {% endif %} - {% if field.errors %} -
- {% for error in field.errors %} - {{ _(error) }} - {% endfor %} -
- {% endif %} - {{end_sep_field|safe}} - {% endif %} -{% endmacro %} - -{% macro render_collapsable_form_group(id, section_title='') %} -
-
- - - - - - - - - - -
- - {{section_title}} -
-
- - - {{ caller() }} - -
-
-
-
-
-{% endmacro %} diff --git a/superset/templates/superset/form_view/csv_scripts.html b/superset/templates/superset/form_view/csv_scripts.html deleted file mode 100644 index b2d12b1aea3fe..0000000000000 --- a/superset/templates/superset/form_view/csv_scripts.html +++ /dev/null @@ -1,39 +0,0 @@ -{# -Licensed to the Apache Software Foundation (ASF) under one -or more contributor license agreements. See the NOTICE file -distributed with this work for additional information -regarding copyright ownership. The ASF licenses this file -to you under the Apache License, Version 2.0 (the -"License"); you may not use this file except in compliance -with the License. You may obtain a copy of the License at - -http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, -software distributed under the License is distributed on an -"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -KIND, either express or implied. See the License for the -specific language governing permissions and limitations -under the License. -#} -{% import "superset/macros.html" as macros %} - - diff --git a/superset/templates/superset/form_view/csv_to_database_view/edit.html b/superset/templates/superset/form_view/csv_to_database_view/edit.html deleted file mode 100644 index 7f50c2f978642..0000000000000 --- a/superset/templates/superset/form_view/csv_to_database_view/edit.html +++ /dev/null @@ -1,137 +0,0 @@ -{# Licensed to the Apache Software Foundation (ASF) under one or more -contributor license agreements. See the NOTICE file distributed with this work -for additional information regarding copyright ownership. The ASF licenses this -file to you under the Apache License, Version 2.0 (the "License"); you may not -use this file except in compliance with the License. You may obtain a copy of -the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by -applicable law or agreed to in writing, software distributed under the License -is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. #} {% extends -"appbuilder/base.html" %} {% import 'appbuilder/general/lib.html' as lib %} {% -import "superset/macros.html" as macros %} {% set begin_sep_label = ' - - ' %} {% set end_sep_label = ' - -' %} {% set begin_sep_field = ' -' %} {% set end_sep_field = ' -' %} {% import 'superset/form_view/database_schemas_selector.html' as -schemas_selector %} {% import 'superset/form_view/csv_scripts.html' as -csv_scripts %} {% import 'superset/form_view/csv_macros.html' as csv_macros %} -{% block content %} {{ lib.panel_begin(title, "edit") }} -
-
- {{form.hidden_tag()}} -
-
- - - - {{ lib.render_field(form.csv_file, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.table_name, begin_sep_label, - end_sep_label, begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.database, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.schema, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ csv_macros.render_delimiter_field(form.delimiter, - begin_sep_label, end_sep_label, begin_sep_field, end_sep_field) }} - - -
-
-
- {% call csv_macros.render_collapsable_form_group("accordion1", "File - Settings") %} - - {{ lib.render_field(form.if_exists, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.skip_initial_space, begin_sep_label, - end_sep_label, begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.skip_blank_lines, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.parse_dates, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.day_first, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.decimal, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.null_values, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - {% endcall %} {% call csv_macros.render_collapsable_form_group("accordion2", - "Columns") %} - - {{ lib.render_field(form.index_col, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.dataframe_index, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.index_label, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.use_cols, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.overwrite_duplicate, begin_sep_label, - end_sep_label, begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.dtype, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - {% endcall %} {% call csv_macros.render_collapsable_form_group("accordion3", - "Rows") %} - - {{ lib.render_field(form.header, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.nrows, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - - {{ lib.render_field(form.skiprows, begin_sep_label, end_sep_label, - begin_sep_field, end_sep_field) }} - - {% endcall %} -
-
- {{ lib.render_form_controls() }} -
-
-
-
-{% endblock %} {% block add_tail_js %} - -{% endblock %} {% block tail_js %} {{ super() }} {{ schemas_selector }} {{ -csv_scripts }} {% endblock %} diff --git a/superset/templates/superset/form_view/excel_to_database_view/edit.html b/superset/templates/superset/form_view/excel_to_database_view/edit.html deleted file mode 100644 index 2bec3aa12abb1..0000000000000 --- a/superset/templates/superset/form_view/excel_to_database_view/edit.html +++ /dev/null @@ -1,25 +0,0 @@ -{# - Licensed to the Apache Software Foundation (ASF) under one - or more contributor license agreements. See the NOTICE file - distributed with this work for additional information - regarding copyright ownership. The ASF licenses this file - to you under the Apache License, Version 2.0 (the - "License"); you may not use this file except in compliance - with the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, - software distributed under the License is distributed on an - "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - KIND, either express or implied. See the License for the - specific language governing permissions and limitations - under the License. -#} -{% import 'superset/form_view/database_schemas_selector.html' as schemas_selector %} -{% extends 'appbuilder/general/model/edit.html' %} - -{% block tail_js %} - {{ super() }} - {{ schemas_selector }} -{% endblock %} diff --git a/superset/views/database/forms.py b/superset/views/database/forms.py index 9091540ece37a..f1d35334463b6 100644 --- a/superset/views/database/forms.py +++ b/superset/views/database/forms.py @@ -20,22 +20,12 @@ from flask_appbuilder.fieldwidgets import BS3TextFieldWidget from flask_appbuilder.forms import DynamicForm from flask_babel import lazy_gettext as _ -from flask_wtf.file import FileAllowed, FileField, FileRequired -from wtforms import ( - BooleanField, - IntegerField, - MultipleFileField, - SelectField, - StringField, -) -from wtforms.validators import DataRequired, Length, NumberRange, Optional, Regexp +from flask_wtf.file import FileAllowed +from wtforms import BooleanField, MultipleFileField, SelectField, StringField +from wtforms.validators import DataRequired, Optional, Regexp from superset import app, db, security_manager -from superset.forms import ( - CommaSeparatedListField, - filter_not_empty_values, - JsonListField, -) +from superset.forms import JsonListField from superset.models.core import Database config = app.config @@ -103,138 +93,6 @@ def is_engine_allowed_to_file_upl(database: Database) -> bool: return False -class ExcelToDatabaseForm(UploadToDatabaseForm): - name = StringField( - _("Table Name"), - description=_("Name of table to be created from excel data."), - validators=[ - DataRequired(), - Regexp(r"^[^\.]+$", message=_("Table name cannot contain a schema")), - ], - widget=BS3TextFieldWidget(), - ) - excel_file = FileField( - _("Excel File"), - description=_("Select a Excel file to be uploaded to a database."), - validators=[ - FileRequired(), - FileAllowed( - config["ALLOWED_EXTENSIONS"].intersection(config["EXCEL_EXTENSIONS"]), - _( - "Only the following file extensions are allowed: " - "%(allowed_extensions)s", - allowed_extensions=", ".join( - config["ALLOWED_EXTENSIONS"].intersection( - config["EXCEL_EXTENSIONS"] - ) - ), - ), - ), - ], - ) - - sheet_name = StringField( - _("Sheet Name"), - description=_("Strings used for sheet names (default is the first sheet)."), - validators=[Optional()], - widget=BS3TextFieldWidget(), - ) - - database = QuerySelectField( - _("Database"), - query_func=UploadToDatabaseForm.file_allowed_dbs, - get_pk_func=lambda a: a.id, - get_label=lambda a: a.database_name, - ) - schema = StringField( - _("Schema"), - description=_("Specify a schema (if database flavor supports this)."), - validators=[Optional()], - widget=BS3TextFieldWidget(), - ) - if_exists = SelectField( - _("Table Exists"), - description=_( - "If table exists do one of the following: " - "Fail (do nothing), Replace (drop and recreate table) " - "or Append (insert data)." - ), - choices=[ - ("fail", _("Fail")), - ("replace", _("Replace")), - ("append", _("Append")), - ], - validators=[DataRequired()], - ) - header = IntegerField( - _("Header Row"), - description=_( - "Row containing the headers to use as " - "column names (0 is first line of data). " - "Leave empty if there is no header row." - ), - validators=[Optional(), NumberRange(min=0)], - widget=BS3TextFieldWidget(), - ) - index_col = IntegerField( - _("Index Column"), - description=_( - "Column to use as the row labels of the " - "dataframe. Leave empty if no index column." - ), - validators=[Optional(), NumberRange(min=0)], - widget=BS3TextFieldWidget(), - ) - skiprows = IntegerField( - _("Skip Rows"), - description=_("Number of rows to skip at start of file."), - validators=[Optional(), NumberRange(min=0)], - widget=BS3TextFieldWidget(), - ) - nrows = IntegerField( - _("Rows to Read"), - description=_("Number of rows of file to read."), - validators=[Optional(), NumberRange(min=0)], - widget=BS3TextFieldWidget(), - ) - parse_dates = CommaSeparatedListField( - _("Parse Dates"), - description=_( - "A comma separated list of columns that should be parsed as dates." - ), - filters=[filter_not_empty_values], - ) - decimal = StringField( - _("Decimal Character"), - default=".", - description=_("Character to interpret as decimal point."), - validators=[Optional(), Length(min=1, max=1)], - widget=BS3TextFieldWidget(), - ) - index = BooleanField( - _("Dataframe Index"), description=_("Write dataframe index as a column.") - ) - index_label = StringField( - _("Column Label(s)"), - description=_( - "Column label for index column(s). If None is given " - "and Dataframe Index is True, Index Names are used." - ), - validators=[Optional()], - widget=BS3TextFieldWidget(), - ) - null_values = JsonListField( - _("Null values"), - default=config["CSV_DEFAULT_NA_NAMES"], - description=_( - "Json list of the values that should be treated as null. " - 'Examples: [""], ["None", "N/A"], ["nan", "null"]. ' - "Warning: Hive database supports only single value. " - 'Use [""] for empty string.' - ), - ) - - class ColumnarToDatabaseForm(UploadToDatabaseForm): name = StringField( _("Table Name"), diff --git a/superset/views/database/views.py b/superset/views/database/views.py index b6e5cf97fa53a..876f6935329f4 100644 --- a/superset/views/database/views.py +++ b/superset/views/database/views.py @@ -15,8 +15,6 @@ # specific language governing permissions and limitations # under the License. import io -import os -import tempfile import zipfile from typing import Any, TYPE_CHECKING @@ -41,7 +39,7 @@ from superset.utils import core as utils from superset.views.base import DeleteMixin, SupersetModelView, YamlExportMixin -from .forms import ColumnarToDatabaseForm, ExcelToDatabaseForm +from .forms import ColumnarToDatabaseForm from .mixins import DatabaseMixin from .validators import schema_allows_file_upload, sqlalchemy_uri_validator @@ -154,141 +152,6 @@ def this_form_post(self) -> Any: ) -class ExcelToDatabaseView(SimpleFormView): - form = ExcelToDatabaseForm - form_template = "superset/form_view/excel_to_database_view/edit.html" - form_title = _("Excel to Database configuration") - add_columns = ["database", "schema", "table_name"] - - def form_get(self, form: ExcelToDatabaseForm) -> None: - form.header.data = 0 - form.decimal.data = "." - form.if_exists.data = "fail" - form.sheet_name.data = "" - - def form_post(self, form: ExcelToDatabaseForm) -> Response: - database = form.database.data - excel_table = Table(table=form.name.data, schema=form.schema.data) - - if not schema_allows_file_upload(database, excel_table.schema): - message = _( - 'Database "%(database_name)s" schema "%(schema_name)s" ' - "is not allowed for excel uploads. Please contact your Superset Admin.", - database_name=database.database_name, - schema_name=excel_table.schema, - ) - flash(message, "danger") - return redirect("/exceltodatabaseview/form") - - uploaded_tmp_file_path = ( - tempfile.NamedTemporaryFile( # pylint: disable=consider-using-with - dir=app.config["UPLOAD_FOLDER"], - suffix=os.path.splitext(form.excel_file.data.filename)[1].lower(), - delete=False, - ).name - ) - - try: - utils.ensure_path_exists(config["UPLOAD_FOLDER"]) - upload_stream_write(form.excel_file.data, uploaded_tmp_file_path) - - df = pd.read_excel( - header=form.header.data if form.header.data else 0, - index_col=form.index_col.data, - io=form.excel_file.data, - keep_default_na=not form.null_values.data, - na_values=form.null_values.data if form.null_values.data else [], - parse_dates=form.parse_dates.data, - skiprows=form.skiprows.data, - sheet_name=form.sheet_name.data if form.sheet_name.data else 0, - ) - - database = ( - db.session.query(models.Database) - .filter_by(id=form.data.get("database").data.get("id")) - .one() - ) - - database.db_engine_spec.df_to_sql( - database, - excel_table, - df, - to_sql_kwargs={ - "chunksize": 1000, - "if_exists": form.if_exists.data, - "index": form.index.data, - "index_label": form.index_label.data, - }, - ) - - # Connect table to the database that should be used for exploration. - # E.g. if hive was used to upload a excel, presto will be a better option - # to explore the table. - explore_database = database - explore_database_id = database.explore_database_id - if explore_database_id: - explore_database = ( - db.session.query(models.Database) - .filter_by(id=explore_database_id) - .one_or_none() - or database - ) - - sqla_table = ( - db.session.query(SqlaTable) - .filter_by( - table_name=excel_table.table, - schema=excel_table.schema, - database_id=explore_database.id, - ) - .one_or_none() - ) - - if sqla_table: - sqla_table.fetch_metadata() - if not sqla_table: - sqla_table = SqlaTable(table_name=excel_table.table) - sqla_table.database = explore_database - sqla_table.database_id = database.id - sqla_table.owners = [g.user] - sqla_table.schema = excel_table.schema - sqla_table.fetch_metadata() - db.session.add(sqla_table) - db.session.commit() - except Exception as ex: # pylint: disable=broad-except - db.session.rollback() - message = _( - 'Unable to upload Excel file "%(filename)s" to table ' - '"%(table_name)s" in database "%(db_name)s". ' - "Error message: %(error_msg)s", - filename=form.excel_file.data.filename, - table_name=form.name.data, - db_name=database.database_name, - error_msg=str(ex), - ) - - flash(message, "danger") - stats_logger.incr("failed_excel_upload") - return redirect("/exceltodatabaseview/form") - - # Go back to welcome page / splash screen - message = _( - 'Excel file "%(excel_filename)s" uploaded to table "%(table_name)s" in ' - 'database "%(db_name)s"', - excel_filename=form.excel_file.data.filename, - table_name=str(excel_table), - db_name=sqla_table.database.database_name, - ) - flash(message, "info") - event_logger.log_with_context( - action="successful_excel_upload", - database=form.database.data.name, - schema=form.schema.data, - table=form.name.data, - ) - return redirect("/tablemodelview/list/") - - class ColumnarToDatabaseView(SimpleFormView): form = ColumnarToDatabaseForm form_template = "superset/form_view/columnar_to_database_view/edit.html" diff --git a/tests/integration_tests/csv_upload_tests.py b/tests/integration_tests/csv_upload_tests.py index a51c63e4d6ead..299340b0c4bff 100644 --- a/tests/integration_tests/csv_upload_tests.py +++ b/tests/integration_tests/csv_upload_tests.py @@ -43,8 +43,6 @@ test_client = app.test_client() CSV_UPLOAD_DATABASE = "csv_explore_db" -CSV_FILENAME1 = "testCSV1.csv" -CSV_FILENAME2 = "testCSV2.csv" EXCEL_FILENAME = "testExcel.xlsx" PARQUET_FILENAME1 = "testZip/testParquet1.parquet" PARQUET_FILENAME2 = "testZip/testParquet2.parquet" @@ -93,27 +91,6 @@ def setup_csv_upload_with_context(): yield from _setup_csv_upload() -@pytest.fixture(scope="module") -def create_csv_files(): - with open(CSV_FILENAME1, "w+") as test_file: - for line in ["a,b", "john,1", "paul,2"]: - test_file.write(f"{line}\n") - - with open(CSV_FILENAME2, "w+") as test_file: - for line in ["b,c,d", "john,1,x", "paul,2,"]: - test_file.write(f"{line}\n") - yield - os.remove(CSV_FILENAME1) - os.remove(CSV_FILENAME2) - - -@pytest.fixture() -def create_excel_files(): - pd.DataFrame({"a": ["john", "paul"], "b": [1, 2]}).to_excel(EXCEL_FILENAME) - yield - os.remove(EXCEL_FILENAME) - - @pytest.fixture() def create_columnar_files(): os.mkdir(ZIP_DIRNAME) @@ -129,25 +106,6 @@ def get_upload_db(): return db.session.query(Database).filter_by(database_name=CSV_UPLOAD_DATABASE).one() -def upload_excel( - filename: str, table_name: str, extra: Optional[dict[str, str]] = None -): - excel_upload_db_id = get_upload_db().id - form_data = { - "excel_file": open(filename, "rb"), - "name": table_name, - "database": excel_upload_db_id, - "sheet_name": "Sheet1", - "if_exists": "fail", - "index_label": "test_label", - } - if schema := utils.get_example_default_schema(): - form_data["schema"] = schema - if extra: - form_data.update(extra) - return get_resp(test_client, "/exceltodatabaseview/form", data=form_data) - - def upload_columnar( filename: str, table_name: str, extra: Optional[dict[str, str]] = None ): @@ -199,65 +157,6 @@ def escaped_parquet(text): return escaped_double_quotes(f"['{text}']") -@pytest.mark.usefixtures("setup_csv_upload_with_context") -@pytest.mark.usefixtures("create_excel_files") -@mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3) -@mock.patch("superset.views.database.views.event_logger.log_with_context") -def test_import_excel(mock_event_logger): - if utils.backend() == "hive": - pytest.skip("Hive doesn't excel upload.") - - schema = utils.get_example_default_schema() - full_table_name = f"{schema}.{EXCEL_UPLOAD_TABLE}" if schema else EXCEL_UPLOAD_TABLE - test_db = get_upload_db() - - success_msg = f"Excel file {escaped_double_quotes(EXCEL_FILENAME)} uploaded to table {escaped_double_quotes(full_table_name)}" - - # initial upload with fail mode - resp = upload_excel(EXCEL_FILENAME, EXCEL_UPLOAD_TABLE) - assert success_msg in resp - mock_event_logger.assert_called_with( - action="successful_excel_upload", - database=test_db.name, - schema=schema, - table=EXCEL_UPLOAD_TABLE, - ) - - # ensure user is assigned as an owner - table = SupersetTestCase.get_table(name=EXCEL_UPLOAD_TABLE) - assert security_manager.find_user("admin") in table.owners - - # upload again with fail mode; should fail - fail_msg = f"Unable to upload Excel file {escaped_double_quotes(EXCEL_FILENAME)} to table {escaped_double_quotes(EXCEL_UPLOAD_TABLE)}" - resp = upload_excel(EXCEL_FILENAME, EXCEL_UPLOAD_TABLE) - assert fail_msg in resp - - if utils.backend() != "hive": - # upload again with append mode - resp = upload_excel( - EXCEL_FILENAME, EXCEL_UPLOAD_TABLE, extra={"if_exists": "append"} - ) - assert success_msg in resp - - # upload again with replace mode - resp = upload_excel( - EXCEL_FILENAME, EXCEL_UPLOAD_TABLE, extra={"if_exists": "replace"} - ) - assert success_msg in resp - mock_event_logger.assert_called_with( - action="successful_excel_upload", - database=test_db.name, - schema=schema, - table=EXCEL_UPLOAD_TABLE, - ) - - with test_db.get_sqla_engine() as engine: - data = engine.execute( - f"SELECT * from {EXCEL_UPLOAD_TABLE} ORDER BY b" - ).fetchall() - assert data == [(0, "john", 1), (1, "paul", 2)] - - @pytest.mark.usefixtures("setup_csv_upload_with_context") @pytest.mark.usefixtures("create_columnar_files") @mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3) diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 8f430279269a6..7532e121646b3 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -465,7 +465,7 @@ def test_info_dashboard(self): rv = self.get_assert_metric(uri, "info") self.assertEqual(rv.status_code, 200) - def test_info_security_database(self): + def test_info_security_dashboard(self): """ Dashboard API: Test info security """ diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py index 5734f1a91d106..8181db9b29516 100644 --- a/tests/integration_tests/databases/api_tests.py +++ b/tests/integration_tests/databases/api_tests.py @@ -1448,6 +1448,7 @@ def test_info_security_database(self): assert set(data["permissions"]) == { "can_read", "can_csv_upload", + "can_excel_upload", "can_write", "can_export", } diff --git a/tests/integration_tests/databases/commands/excel_upload_test.py b/tests/integration_tests/databases/commands/excel_upload_test.py new file mode 100644 index 0000000000000..51ac4e71f36da --- /dev/null +++ b/tests/integration_tests/databases/commands/excel_upload_test.py @@ -0,0 +1,257 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import json +from datetime import datetime + +import pytest + +from superset import db, security_manager +from superset.commands.database.excel_import import ExcelImportCommand +from superset.commands.database.exceptions import ( + DatabaseNotFoundError, + DatabaseSchemaUploadNotAllowed, + DatabaseUploadFailed, +) +from superset.models.core import Database +from superset.utils.core import override_user +from superset.utils.database import get_or_create_db +from tests.integration_tests.conftest import only_postgresql +from tests.integration_tests.test_app import app +from tests.unit_tests.fixtures.common import create_excel_file + +EXCEL_UPLOAD_DATABASE = "excel_explore_db" +EXCEL_UPLOAD_TABLE = "excel_upload" +EXCEL_UPLOAD_TABLE_W_SCHEMA = "excel_upload_w_schema" + + +EXCEL_FILE_1 = { + "Name": ["name1", "name2", "name3"], + "Age": [30, 29, 28], + "City": ["city1", "city2", "city3"], + "Birth": ["1-1-1980", "1-1-1981", "1-1-1982"], +} + +EXCEL_FILE_2 = { + "Name": ["name1", "name2", "name3"], + "Age": ["N/A", 29, 28], + "City": ["city1", "None", "city3"], + "Birth": ["1-1-1980", "1-1-1981", "1-1-1982"], +} + + +def _setup_excel_upload(allowed_schemas: list[str] | None = None): + upload_db = get_or_create_db( + EXCEL_UPLOAD_DATABASE, app.config["SQLALCHEMY_EXAMPLES_URI"] + ) + upload_db.allow_file_upload = True + extra = upload_db.get_extra() + allowed_schemas = allowed_schemas or [] + extra["schemas_allowed_for_file_upload"] = allowed_schemas + upload_db.extra = json.dumps(extra) + + db.session.commit() + + yield + + upload_db = get_upload_db() + with upload_db.get_sqla_engine_with_context() as engine: + engine.execute(f"DROP TABLE IF EXISTS {EXCEL_UPLOAD_TABLE}") + engine.execute(f"DROP TABLE IF EXISTS {EXCEL_UPLOAD_TABLE_W_SCHEMA}") + db.session.delete(upload_db) + db.session.commit() + + +def get_upload_db(): + return ( + db.session.query(Database).filter_by(database_name=EXCEL_UPLOAD_DATABASE).one() + ) + + +@pytest.fixture(scope="function") +def setup_excel_upload_with_context(): + with app.app_context(): + yield from _setup_excel_upload() + + +@pytest.fixture(scope="function") +def setup_excel_upload_with_context_schema(): + with app.app_context(): + yield from _setup_excel_upload(["public"]) + + +@only_postgresql +@pytest.mark.parametrize( + "excel_data,options, table_data", + [ + ( + EXCEL_FILE_1, + {}, + [ + ("name1", 30, "city1", "1-1-1980"), + ("name2", 29, "city2", "1-1-1981"), + ("name3", 28, "city3", "1-1-1982"), + ], + ), + ( + EXCEL_FILE_1, + {"columns_read": ["Name", "Age"]}, + [("name1", 30), ("name2", 29), ("name3", 28)], + ), + ( + EXCEL_FILE_1, + {"columns_read": []}, + [ + ("name1", 30, "city1", "1-1-1980"), + ("name2", 29, "city2", "1-1-1981"), + ("name3", 28, "city3", "1-1-1982"), + ], + ), + ( + EXCEL_FILE_1, + {"rows_to_read": 1}, + [ + ("name1", 30, "city1", "1-1-1980"), + ], + ), + ( + EXCEL_FILE_1, + {"rows_to_read": 1, "columns_read": ["Name", "Age"]}, + [ + ("name1", 30), + ], + ), + ( + EXCEL_FILE_1, + {"skip_rows": 1}, + [("name2", 29, "city2", "1-1-1981"), ("name3", 28, "city3", "1-1-1982")], + ), + ( + EXCEL_FILE_1, + {"rows_to_read": 2}, + [ + ("name1", 30, "city1", "1-1-1980"), + ("name2", 29, "city2", "1-1-1981"), + ], + ), + ( + EXCEL_FILE_1, + {"column_dates": ["Birth"]}, + [ + ("name1", 30, "city1", datetime(1980, 1, 1, 0, 0)), + ("name2", 29, "city2", datetime(1981, 1, 1, 0, 0)), + ("name3", 28, "city3", datetime(1982, 1, 1, 0, 0)), + ], + ), + ( + EXCEL_FILE_2, + {"null_values": ["N/A", "None"]}, + [ + ("name1", None, "city1", "1-1-1980"), + ("name2", 29, None, "1-1-1981"), + ("name3", 28, "city3", "1-1-1982"), + ], + ), + ( + EXCEL_FILE_2, + { + "null_values": ["N/A", "None"], + "column_dates": ["Birth"], + "columns_read": ["Name", "Age", "Birth"], + }, + [ + ("name1", None, datetime(1980, 1, 1, 0, 0)), + ("name2", 29, datetime(1981, 1, 1, 0, 0)), + ("name3", 28, datetime(1982, 1, 1, 0, 0)), + ], + ), + ], +) +@pytest.mark.usefixtures("setup_excel_upload_with_context") +def test_excel_upload_options(excel_data, options, table_data): + admin_user = security_manager.find_user(username="admin") + upload_database = get_upload_db() + + with override_user(admin_user): + ExcelImportCommand( + upload_database.id, + EXCEL_UPLOAD_TABLE, + create_excel_file(excel_data), + options=options, + ).run() + with upload_database.get_sqla_engine_with_context() as engine: + data = engine.execute(f"SELECT * from {EXCEL_UPLOAD_TABLE}").fetchall() + assert data == table_data + + +@only_postgresql +@pytest.mark.usefixtures("setup_excel_upload_with_context") +def test_excel_upload_database_not_found(): + admin_user = security_manager.find_user(username="admin") + + with override_user(admin_user): + with pytest.raises(DatabaseNotFoundError): + ExcelImportCommand( + 1000, + EXCEL_UPLOAD_TABLE, + create_excel_file(EXCEL_FILE_1), + options={}, + ).run() + + +@only_postgresql +@pytest.mark.usefixtures("setup_excel_upload_with_context_schema") +def test_excel_upload_schema_not_allowed(): + admin_user = security_manager.find_user(username="admin") + upload_db_id = get_upload_db().id + with override_user(admin_user): + with pytest.raises(DatabaseSchemaUploadNotAllowed): + ExcelImportCommand( + upload_db_id, + EXCEL_UPLOAD_TABLE, + create_excel_file(EXCEL_FILE_1), + options={}, + ).run() + + with pytest.raises(DatabaseSchemaUploadNotAllowed): + ExcelImportCommand( + upload_db_id, + EXCEL_UPLOAD_TABLE, + create_excel_file(EXCEL_FILE_1), + options={"schema": "schema1"}, + ).run() + + ExcelImportCommand( + upload_db_id, + EXCEL_UPLOAD_TABLE, + create_excel_file(EXCEL_FILE_1), + options={"schema": "public"}, + ).run() + + +@only_postgresql +@pytest.mark.usefixtures("setup_excel_upload_with_context") +def test_excel_upload_broken_file(): + admin_user = security_manager.find_user(username="admin") + + with override_user(admin_user): + with pytest.raises(DatabaseUploadFailed): + ExcelImportCommand( + get_upload_db().id, + EXCEL_UPLOAD_TABLE, + create_excel_file([""]), + options={"column_dates": ["Birth"]}, + ).run() diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py index 5e004992de28c..7f25f28f157a9 100644 --- a/tests/unit_tests/databases/api_test.py +++ b/tests/unit_tests/databases/api_test.py @@ -34,11 +34,12 @@ from superset import db from superset.commands.database.csv_import import CSVImportCommand +from superset.commands.database.excel_import import ExcelImportCommand from superset.db_engine_specs.sqlite import SqliteEngineSpec from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.exceptions import SupersetSecurityException from superset.sql_parse import Table -from tests.unit_tests.fixtures.common import create_csv_file +from tests.unit_tests.fixtures.common import create_csv_file, create_excel_file def test_filter_by_uuid( @@ -1175,6 +1176,225 @@ def test_csv_upload_file_extension_valid( assert response.status_code == 200 +@pytest.mark.parametrize( + "payload,cmd_called_with", + [ + ( + { + "file": (create_excel_file(), "out.xls"), + "table_name": "table1", + }, + ( + 1, + "table1", + ANY, + { + "already_exists": "fail", + "file": ANY, + "table_name": "table1", + }, + ), + ), + ( + { + "file": (create_excel_file(), "out.xls"), + "table_name": "table2", + "sheet_name": "Sheet1", + "already_exists": "replace", + "column_dates": "col1,col2", + }, + ( + 1, + "table2", + ANY, + { + "already_exists": "replace", + "column_dates": ["col1", "col2"], + "sheet_name": "Sheet1", + "file": ANY, + "table_name": "table2", + }, + ), + ), + ( + { + "file": (create_excel_file(), "out.xls"), + "table_name": "table2", + "sheet_name": "Sheet1", + "already_exists": "replace", + "columns_read": "col1,col2", + "rows_to_read": "1", + "skip_rows": "10", + "null_values": "None,N/A,''", + }, + ( + 1, + "table2", + ANY, + { + "already_exists": "replace", + "columns_read": ["col1", "col2"], + "null_values": ["None", "N/A", "''"], + "rows_to_read": 1, + "skip_rows": 10, + "sheet_name": "Sheet1", + "file": ANY, + "table_name": "table2", + }, + ), + ), + ], +) +def test_excel_upload( + payload: dict[str, Any], + cmd_called_with: tuple[int, str, Any, dict[str, Any]], + mocker: MockFixture, + client: Any, + full_api_access: None, +) -> None: + """ + Test Excel Upload success. + """ + init_mock = mocker.patch.object(ExcelImportCommand, "__init__") + init_mock.return_value = None + _ = mocker.patch.object(ExcelImportCommand, "run") + response = client.post( + f"/api/v1/database/1/excel_upload/", + data=payload, + content_type="multipart/form-data", + ) + assert response.status_code == 200 + assert response.json == {"message": "OK"} + init_mock.assert_called_with(*cmd_called_with) + + +@pytest.mark.parametrize( + "payload,expected_response", + [ + ( + { + "file": (create_excel_file(), "out.xls"), + "sheet_name": "Sheet1", + "already_exists": "fail", + }, + {"message": {"table_name": ["Missing data for required field."]}}, + ), + ( + { + "file": (create_excel_file(), "out.xls"), + "table_name": "", + "sheet_name": "Sheet1", + "already_exists": "fail", + }, + {"message": {"table_name": ["Length must be between 1 and 10000."]}}, + ), + ( + {"table_name": "table1", "already_exists": "fail"}, + {"message": {"file": ["Field may not be null."]}}, + ), + ( + { + "file": "xpto", + "table_name": "table1", + "already_exists": "fail", + }, + {"message": {"file": ["Field may not be null."]}}, + ), + ( + { + "file": (create_excel_file(), "out.xls"), + "table_name": "table1", + "already_exists": "xpto", + }, + {"message": {"already_exists": ["Must be one of: fail, replace, append."]}}, + ), + ( + { + "file": (create_excel_file(), "out.xls"), + "table_name": "table1", + "already_exists": "fail", + "header_row": "test1", + }, + {"message": {"header_row": ["Not a valid integer."]}}, + ), + ( + { + "file": (create_excel_file(), "out.xls"), + "table_name": "table1", + "already_exists": "fail", + "rows_to_read": 0, + }, + {"message": {"rows_to_read": ["Must be greater than or equal to 1."]}}, + ), + ( + { + "file": (create_excel_file(), "out.xls"), + "table_name": "table1", + "already_exists": "fail", + "skip_rows": "test1", + }, + {"message": {"skip_rows": ["Not a valid integer."]}}, + ), + ], +) +def test_excel_upload_validation( + payload: Any, + expected_response: dict[str, str], + mocker: MockFixture, + client: Any, + full_api_access: None, +) -> None: + """ + Test Excel Upload validation fails. + """ + _ = mocker.patch.object(ExcelImportCommand, "run") + + response = client.post( + f"/api/v1/database/1/excel_upload/", + data=payload, + content_type="multipart/form-data", + ) + assert response.status_code == 400 + assert response.json == expected_response + + +@pytest.mark.parametrize( + "filename", + [ + "out.xpto", + "out.exe", + "out", + "out xls", + "", + "out.slx.exe", + ".xls", + "out.", + ".", + "out xls a.exe", + ], +) +def test_excel_upload_file_extension_invalid( + filename: str, + mocker: MockFixture, + client: Any, + full_api_access: None, +) -> None: + """ + Test Excel Upload file extension fails. + """ + _ = mocker.patch.object(ExcelImportCommand, "run") + response = client.post( + f"/api/v1/database/1/excel_upload/", + data={ + "file": (create_excel_file(), filename), + "table_name": "table1", + }, + content_type="multipart/form-data", + ) + assert response.status_code == 400 + assert response.json == {"message": {"file": ["File extension is not allowed."]}} + + def test_table_extra_metadata_happy_path( mocker: MockFixture, client: Any, diff --git a/tests/unit_tests/fixtures/common.py b/tests/unit_tests/fixtures/common.py index 412c3d34d54d4..841566971b6b0 100644 --- a/tests/unit_tests/fixtures/common.py +++ b/tests/unit_tests/fixtures/common.py @@ -20,7 +20,9 @@ import csv from datetime import datetime from io import BytesIO, StringIO +from typing import Any +import pandas as pd import pytest @@ -46,3 +48,12 @@ def create_csv_file(data: list[list[str]] | None = None) -> BytesIO: output.seek(0) bytes_buffer = BytesIO(output.getvalue().encode("utf-8")) return bytes_buffer + + +def create_excel_file(data: dict[str, list[Any]] | None = None) -> BytesIO: + data = {"Name": ["John"], "Age": [30], "City": ["New York"]} if not data else data + excel_buffer = BytesIO() + df = pd.DataFrame(data) + df.to_excel(excel_buffer, index=False) + excel_buffer.seek(0) + return excel_buffer