codeant-ai-for-open-source[bot] commented on code in PR #36191: URL: https://github.com/apache/superset/pull/36191#discussion_r2755073245
########## superset-frontend/src/pages/FileHandler/index.tsx: ########## @@ -0,0 +1,138 @@ +/** + * 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 { useEffect, useState } from 'react'; +import { useHistory } from 'react-router-dom'; +import { t } from '@apache-superset/core'; +import { Loading } from '@superset-ui/core/components'; +import UploadDataModal from 'src/features/databases/UploadDataModel'; +import withToasts from 'src/components/MessageToasts/withToasts'; + +interface FileLaunchParams { + readonly files?: readonly FileSystemFileHandle[]; +} + +interface LaunchQueue { + setConsumer: (consumer: (params: FileLaunchParams) => void) => void; +} + +interface WindowWithLaunchQueue extends Window { + launchQueue?: LaunchQueue; +} + +interface FileHandlerProps { + addDangerToast: (msg: string) => void; + addSuccessToast: (msg: string) => void; +} + +const FileHandler = ({ addDangerToast, addSuccessToast }: FileHandlerProps) => { + const history = useHistory(); + const [uploadFile, setUploadFile] = useState<File | null>(null); + const [uploadType, setUploadType] = useState< + 'csv' | 'excel' | 'columnar' | null + >(null); + const [showModal, setShowModal] = useState(false); + const [allowedExtensions, setAllowedExtensions] = useState<string[]>([]); + + useEffect(() => { + const handleFileLaunch = async () => { + const { launchQueue } = window as WindowWithLaunchQueue; + + if (!launchQueue) { + addDangerToast( + t( + 'File handling is not supported in this browser. Please use a modern browser like Chrome or Edge.', + ), + ); + history.push('/superset/welcome/'); + return; + } + + launchQueue.setConsumer(async (launchParams: FileLaunchParams) => { + if (!launchParams.files || launchParams.files.length === 0) { + history.push('/superset/welcome/'); + return; + } + + try { + const fileHandle = launchParams.files[0]; + const file = await fileHandle.getFile(); + const fileName = file.name.toLowerCase(); + + let type: 'csv' | 'excel' | 'columnar' | null = null; + let extensions: string[] = []; + + if (fileName.endsWith('.csv')) { + type = 'csv'; + extensions = ['csv']; + } else if (fileName.endsWith('.xls') || fileName.endsWith('.xlsx')) { + type = 'excel'; + extensions = ['xls', 'xlsx']; + } else if (fileName.endsWith('.parquet')) { + type = 'columnar'; + extensions = ['parquet']; Review Comment: **Suggestion:** The file type detection logic only recognizes `.csv`, `.xls`, `.xlsx`, and `.parquet` extensions, ignoring other formats that the existing upload modal already supports (notably `.tsv` for delimited text and `.zip` for columnar uploads). This causes a valid TSV or zipped columnar file that can be uploaded via the standard UI to be rejected as "Unsupported file type" when opened via the PWA file handler, leading to inconsistent behavior between entry points. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ PWA file-launch rejects .tsv desktop uploads. - ❌ PWA file-launch rejects zipped columnar (.zip) uploads. - ⚠️ Inconsistent behavior vs UploadDataModal (upload UI). - ⚠️ User-facing redirect to welcome page on open. ``` </details> ```suggestion const extension = fileName.split('.').pop() || ''; let type: 'csv' | 'excel' | 'columnar' | null = null; let extensions: string[] = []; if (extension === 'csv' || extension === 'tsv') { type = 'csv'; // Match the CSV behavior in UploadDataModal (supports csv and tsv) extensions = ['csv', 'tsv']; } else if (extension === 'xls' || extension === 'xlsx') { type = 'excel'; extensions = ['xls', 'xlsx']; } else if (extension === 'parquet' || extension === 'zip') { // Columnar uploads also support zipped files type = 'columnar'; extensions = ['parquet', 'zip']; ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Navigate to the PWA file-handler route (the component is mounted at /superset/file-handler and defined in superset-frontend/src/pages/FileHandler/index.tsx). The launchQueue consumer is registered in useEffect at lines 52-112, specifically launchQueue.setConsumer(...) at lines 66-108. 2. From the desktop, open a supported-but-unchecked file type (example: a tab-separated file test.tsv or a zipped columnar file test.zip). The OS will invoke the PWA and the LaunchQueue consumer receives the file handle; the consumer callback receives launchParams and executes the code starting at line 73 (const fileHandle = launchParams.files[0];). 3. FileHandler reads the File object via fileHandle.getFile() (line 74) and computes fileName at line 75. The current logic checks fileName.endsWith('.csv'/.xls/.xlsx/.parquet) (lines 80-88) and will not match '.tsv' or '.zip'. 4. Because '.tsv' and '.zip' are not recognized, the else branch at lines 89-97 runs: a danger toast is added with the message 'Unsupported file type...' and the user is redirected to '/superset/welcome/' (lines 90-96). The upload modal is never opened. 5. Contrast with the existing UploadDataModal behavior: allowedExtensionsToAccept includes '.tsv' for csv and '.zip' for columnar at src/features/databases/UploadDataModel/index.tsx lines 166-170, and validateUploadFileExtension (lines 178-192) accepts those extensions. This shows the FileHandler rejection is inconsistent with the modal's supported extensions. 6. Reproduced in tests: the component tests in superset-frontend/src/pages/FileHandler/index.test.tsx simulate launchQueue file handles and expect the modal to open for various extensions (see tests for csv/xls/xlsx/parquet). Adding a test case for 'test.tsv' or 'test.zip' will fail with the current FileHandler logic (it will trigger the unsupported-file branch). ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/pages/FileHandler/index.tsx **Line:** 76:88 **Comment:** *Logic Error: The file type detection logic only recognizes `.csv`, `.xls`, `.xlsx`, and `.parquet` extensions, ignoring other formats that the existing upload modal already supports (notably `.tsv` for delimited text and `.zip` for columnar uploads). This causes a valid TSV or zipped columnar file that can be uploaded via the standard UI to be rejected as "Unsupported file type" when opened via the PWA file handler, leading to inconsistent behavior between entry points. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
