[ui-importer] Add configured filesystems in importer #4131

nidhibhatg · 2025-04-30T09:41:50Z

What changes were proposed in this pull request?

Adds configured filesystems in importer and displays all the filesystems
Modifies file chooser to support upload and create folder
Adds functionality to click files for importer use case
Style improvements

How was this patch tested?

unit tests and manually

Please review Hue Contributing Guide before opening a pull request.

github-actions · 2025-04-30T09:42:33Z

✅ Test files were modified. Ensure that the tests cover all relevant changes. ✅

github-actions · 2025-04-30T10:19:23Z

Python Coverage Report •

File	Stmts	Miss	Cover	Missing
TOTAL	53734	27013	49%

report-only-changed-files is enabled. No files were changed during this commit :)

Pytest Report

Tests	Skipped	Failures	Errors	Time
1090	106 💤	0 ❌	0 🔥	6m 2s ⏱️

ramprasadagarwal

Nice work.
Please check the comments below.

desktop/core/src/desktop/js/apps/newimporter/types.ts

desktop/core/src/desktop/js/apps/storageBrowser/FileChooserModal/FileChooserModal.tsx

ramprasadagarwal · 2025-04-30T14:48:23Z

desktop/core/src/desktop/js/apps/storageBrowser/FileChooserModal/FileChooserModal.tsx

+          }}
+        />
+      )}
+      <InputModal


Can we refactor the **/StorageDirectoryActions/CreateAndUpload/CreateAndUploadAction.tsx component to take the create file and folder action to separate component and reuse it here?

will be added in a separate PR

desktop/core/src/desktop/js/apps/storageBrowser/FileChooserModal/FileChooserModal.tsx

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx

desktop/core/src/desktop/js/apps/storageBrowser/FileChooserModal/FileChooserModal.tsx

bjornalm · 2025-05-05T08:50:09Z

Lots of good feedback from @ramprasadagarwal. I'll hold while you guys sort it out, but I did notice that the screenshot of the drag and drop looks weird where the "drop area" appears behind the action buttons of the modal.

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx

.../core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.test.tsx

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx

...p/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/LocalFileUploadOption.test.tsx

desktop/core/src/desktop/js/apps/storageBrowser/FileChooserModal/FileChooserModal.tsx

desktop/core/src/desktop/js/reactComponents/BottomSlidePanel/BottomSlidePanel.tsx

github-actions · 2025-06-03T07:12:16Z

UI Coverage Report

Lines	Statements	Branches	Functions
	39.15% (30527/77959)	31.01% (14247/45936)	23.89% (2130/8915)

github-actions · 2025-06-03T07:24:33Z

Python Coverage Report •

File	Stmts	Miss	Cover	Missing
TOTAL	54185	27078	50%

report-only-changed-files is enabled. No files were changed during this commit :)

Pytest Report

Tests	Skipped	Failures	Errors	Time
1186	106 💤	0 ❌	0 🔥	6m 2s ⏱️

bjornalm

Nice work. I reviewed the first half of the files. Will continue with the rest later, but you can start addressing the issues.

bjornalm · 2025-06-04T09:53:59Z

.../core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.test.tsx

+    expect(screen.getByText('Select a source to import from')).toBeInTheDocument();
+    expect(screen.queryByText('LocalUpload')).toBeInTheDocument();
+    expect(screen.getByText('Amazon S3')).toBeInTheDocument();
+    expect(screen.getByText('HDFS')).toBeInTheDocument();


I think we should cover all filesystems defined in ImporterSourceSelector.tsx

bjornalm · 2025-06-04T10:06:38Z

.../core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.test.tsx

+
+    render(<ImporterSourceSelector setFileMetaData={mockSetFileMetaData} />);
+
+    const s3Button = screen.getAllByRole('button')[1]; // second button (first is local upload)


When we are forced to use an index to access a button it is often a tell that we need to improve the accessibility of that button.

bjornalm · 2025-06-04T10:11:23Z

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx

+          )}
+          {fileSystemsData?.map(filesystem => (
+            <div className="hue-importer__source-selector-option" key={filesystem.name}>
+              <Button


Either move the span title inside the button component or add aria-label={t(fileSystems[filesystem.name].title)} to the button

bjornalm · 2025-06-04T10:20:00Z

.../core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.test.tsx

+
+    render(<ImporterSourceSelector setFileMetaData={mockSetFileMetaData} />);
+
+    expect(screen.getByText('Select a source to import from')).toBeInTheDocument();


How is this testing the loading state? The 'Select a source to import from' is shown after loading.

bjornalm · 2025-06-04T10:39:22Z

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx

+  const errorConfig = [
+    {
+      enabled: !!error,
+      message: t('An error occurred while fetching the filesystem'),


Can we improve this message? Something about "one or more available filesystems". Can you check if we have any useful information from the API about why it failed?

bjornalm · 2025-06-04T11:56:33Z

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/LocalFileUploadOption.tsx

+    const fileSize = file.size;
+    if (fileSize === 0) {
+      setUploadError(t('This file is empty, please select another file.'));
+    } else if (fileSize > 150 * 1024 * 1024) {


A couple of questions:

The error msg states max is 200 KB, I assume it should be 150 MB and the error msg is wrong or? In that case this should be 150 * 1000 * 100 for MB (1024 is MiB)

Where is this limit coming from? Is it available as a config value?

Please at least define this value in a const and reuse it in the error msg. There is a util function available to translate sizes into human readable form in a old vue file, I think we should extract that into a ts file, modify it to use Decimal base and start using it.

export const humanSize = (value?: number): string => { // Use SI decimal units for 1000-based calculations const UNITS = ['B', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB']; // Stays the same if (value === 0) { return '0 B'; } if (!value) { return ''; // Or '0 B' depending on desired behavior for undefined/null } if (value < 1000) { // Changed here! return `${value} B`; } // Note! Changed base for calculations from 1024 to 1000 const unitIndex = Math.min(Math.floor(Math.log(value) / Math.log(1000)), UNITS.length - 1); const unitValue = Math.round((value / Math.pow(1000, unitIndex)) * 10) / 10; return `${unitValue} ${UNITS[unitIndex]}`; };

cc @ramprasadagarwal

bjornalm · 2025-06-04T12:08:58Z

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/LocalFileUploadOption.tsx

+    } else if (fileSize > 150 * 1024 * 1024) {
+      setUploadError(
+        t(
+          'File size exceeds the supported size (200 KB). Please use the S3, ABFS or HDFS browser to upload files.'


Why are we sending them off to a third party tool? Can't the user upload a larger file using the new storage browser in Hue? If so, lets create an actionable error msg that can take the user there.

bjornalm · 2025-06-04T12:23:28Z

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/LocalFileUploadOption.tsx

+  const { t } = i18nReact.useTranslation();
+  const uploadRef = useRef<HTMLInputElement>(null);
+
+  const { save: upload } = useSaveData<LocalFileUploadResponse>(UPLOAD_LOCAL_FILE_API_URL);


To where are we actually uploading this file? The Hue web server? HDFS?
cc @ramprasadagarwal

bjornalm · 2025-06-04T12:27:52Z

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/LocalFileUploadOption.tsx

+        data-testid="hue-file-input"
+        className="hue-importer__source-selector-option-upload"
+        onChange={handleFileUpload}
+        accept=".csv, .xlsx, .xls"


did we really agree that these are the only supported file formats?

bjornalm · 2025-06-04T12:28:59Z

...p/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/LocalFileUploadOption.test.tsx

+    );
+  });
+
+  it('should show error for file > 150mb', () => {


bjornalm · 2025-06-05T14:34:13Z

@nidhibhatg I'm also trying out some deep PR review AI prompting using the the copilot chat functionality. Please have a look, I think they make sense. Unfortunately I can't get them to appear as comments in the actual code.

Accessibility: Table Row Interactivity
File: FileChooserModal.tsx
Location: Table row rendering and click handling

Comment:
For accessibility, consider adding role="button" and tabIndex={0} to clickable table rows, and handle keyboard interaction (Enter/Space) to allow keyboard users to select files or folders. This will make file/folder selection more accessible to all users.

Accessibility: Error Message Announcements
Files:

ImporterSourceSelector.tsx
FileChooserModal.tsx
LocalFileUploadOption.tsx
Comment:
To improve accessibility, error messages shown to users should be wrapped in a container with aria-live="polite" or aria-live="assertive" so that screen readers announce them as soon as they appear.

Accessibility: Modal Focus Management
File: FileChooserModal.tsx
Location: Modal open/close logic

Comment:
Consider implementing focus management for modals: focus should be trapped within the modal while open, and returned to the triggering element when closed. This will improve keyboard navigation and accessibility for screen reader users.

Error Handling: Backend Error Messages
File: FileChooserModal.tsx
Location: Folder creation error handling

Comment:
Currently, API errors (e.g., when creating a folder) are surfaced directly to users. Please consider mapping or sanitizing backend error messages into more user-friendly text to avoid exposing technical details or confusing messages to end users.

Testing: Prefer user-event Over fireEvent
Files:

LocalFileUploadOption.test.tsx
ImporterSourceSelector.test.tsx
BottomSlidePanel.test.tsx
Comment:
For future test maintenance, please prefer using @testing-library/user-event rather than fireEvent. user-event simulates real user interactions and helps catch more realistic issues.

Minor: onPressEnter Handler Bug
File: FileChooserModal.tsx
Location: BottomSlidePanel for create folder

Comment:
The onPressEnter prop is currently set as onPressEnter={() => handleCreate}. This creates a new function that returns handleCreate instead of calling it. It should be onPressEnter={handleCreate} to ensure the handler is called when Enter is pressed.

Copilot

Pull Request Overview

This PR adds support for configured filesystems in the importer and enhances the file chooser with upload and folder-creation capabilities.

Introduces a reusable BottomSlidePanel component for folder creation UI
Extends FileChooserModal with drag-and-drop, polling, upload button, and error/retry handling
Updates ImporterSourceSelector to fetch and render configured filesystems dynamically

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

File	Description
reactComponents/BottomSlidePanel/BottomSlidePanel.tsx	New bottom slide panel component
apps/storageBrowser/FileChooserModal/FileChooserModal.tsx	File chooser extended to support upload & folder creation
apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx	Fetch and display configured filesystems
apps/newimporter/ImporterSourceSelector/LocalFileUploadOption.tsx	Local file upload option with size validation

Comments suppressed due to low confidence (4)

desktop/core/src/desktop/js/reactComponents/BottomSlidePanel/BottomSlidePanel.tsx:24

The onPrimaryClick prop is typed as (unknown) => void, which is ambiguous. Consider changing it to a clear signature like () => void or specify the exact argument type.

onPrimaryClick?: (unknown) => void;

desktop/core/src/desktop/js/reactComponents/BottomSlidePanel/BottomSlidePanel.tsx:66

The mask div has role="button" but no accessible name. Add aria-label or aria-labelledby (e.g., aria-label="Close panel") so screen readers announce its purpose.

<div className={`hue-bottom-slide-mask ${isOpen ? 'fade-in' : 'fade-out'}`}

desktop/core/src/desktop/js/reactComponents/BottomSlidePanel/BottomSlidePanel.scss:62

Missing semicolon after the padding: 16px 24px property in .hue-bottom-slide-panel__footer. Add a semicolon to avoid CSS parsing errors.

    padding: 16px 24px

desktop/core/src/desktop/js/apps/storageBrowser/FileChooserModal/FileChooserModal.tsx:207

The variable name is undefined here. You probably meant to use createFolderValue or the new folder name returned from the API.

          setDestPath(prev => `${prev}/${name}`);

Copilot · 2025-06-05T14:47:56Z

desktop/core/src/desktop/js/apps/storageBrowser/FileChooserModal/FileChooserModal.tsx

+            <Input
+              defaultValue={createFolderValue}
+              disabled={createFolderLoading}
+              onPressEnter={() => handleCreate}


The handler () => handleCreate returns the function instead of invoking it. Change this to onPressEnter={handleCreate} or onPressEnter={() => handleCreate()}.

Suggested change

onPressEnter={() => handleCreate}

onPressEnter={() => handleCreate()}

Copilot · 2025-06-05T14:47:56Z

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/LocalFileUploadOption.tsx

+    } else if (fileSize > 150 * 1024 * 1024) {
+      setUploadError(
+        t(
+          'File size exceeds the supported size (200 KB). Please use the S3, ABFS or HDFS browser to upload files.'


The error message references "200 KB" but the code checks fileSize > 150 * 1024 * 1024 (~150 MB). Either update the threshold or correct the message to match.

Suggested change

'File size exceeds the supported size (200 KB). Please use the S3, ABFS or HDFS browser to upload files.'

'File size exceeds the supported size (150 MB). Please use the S3, ABFS or HDFS browser to upload files.'

Copilot · 2025-06-05T14:47:57Z

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx

+          {fileSystemsData?.map(filesystem => (
+            <div className="hue-importer__source-selector-option" key={filesystem.name}>
+              <Button
+                className="hue-importer__source-selector-option-button"
+                size="large"
+                icon={fileSystems[filesystem.name].icon}
+                onClick={() => {
+                  setSelectedUserHomeDirectory(filesystem.userHomeDirectory);
+                }}
+              ></Button>
+              <span className="hue-importer__source-selector-option-btn-title">
+                {t(fileSystems[filesystem.name].title)}
+              </span>
+            </div>
+          ))}


Indexing fileSystems by filesystem.name without checking if the key exists may throw at runtime. Consider filtering unsupported names or providing a fallback icon.

nidhibhatg requested review from tabraiz12, bjornalm and ramprasadagarwal April 30, 2025 09:41

nidhibhatg force-pushed the nidhi_importer_filesystems branch from 465c3d9 to 42dccf4 Compare April 30, 2025 09:55

ramprasadagarwal requested changes May 2, 2025

View reviewed changes

bjornalm reviewed May 5, 2025

View reviewed changes

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx Outdated Show resolved Hide resolved

ramprasadagarwal reviewed May 8, 2025

View reviewed changes

desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx Outdated Show resolved Hide resolved

nidhibhatg force-pushed the nidhi_importer_filesystems branch from 811ebb9 to 3700a3c Compare May 20, 2025 09:54

ramprasadagarwal requested changes May 27, 2025

View reviewed changes

nidhibhatg added 5 commits June 3, 2025 12:24

[ui-importer] Add configured filesystems in importer

b671c5f

[ui-importer] Update file source and add key

7c15459

[ui-importer] Removed drag and drop modal

c243b61

[ui-importer] Added Bottom slide panel,tests and improvements

a909817

[ui-importer]Updated tests and improvements

1e68db6

nidhibhatg force-pushed the nidhi_importer_filesystems branch from 3700a3c to 1e68db6 Compare June 3, 2025 07:07

nidhibhatg requested a review from ramprasadagarwal June 3, 2025 07:08

bjornalm requested changes Jun 4, 2025

View reviewed changes

bjornalm requested a review from Copilot June 5, 2025 14:45

Copilot AI reviewed Jun 5, 2025

View reviewed changes


		render(<ImporterSourceSelector setFileMetaData={mockSetFileMetaData} />);

		const s3Button = screen.getAllByRole('button')[1]; // second button (first is local upload)


		render(<ImporterSourceSelector setFileMetaData={mockSetFileMetaData} />);

		expect(screen.getByText('Select a source to import from')).toBeInTheDocument();

	onPressEnter={() => handleCreate}
	onPressEnter={() => handleCreate()}

	'File size exceeds the supported size (200 KB). Please use the S3, ABFS or HDFS browser to upload files.'
	'File size exceeds the supported size (150 MB). Please use the S3, ABFS or HDFS browser to upload files.'

[ui-importer] Add configured filesystems in importer #4131

Are you sure you want to change the base?

[ui-importer] Add configured filesystems in importer #4131

Uh oh!

Conversation

nidhibhatg commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

github-actions bot commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pytest Report

Uh oh!

ramprasadagarwal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bjornalm commented May 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jun 3, 2025

UI Coverage Report

Uh oh!

github-actions bot commented Jun 3, 2025

Pytest Report

Uh oh!

bjornalm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjornalm commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

nidhibhatg commented Apr 30, 2025 •

edited

Loading

github-actions bot commented Apr 30, 2025 •

edited

Loading

github-actions bot commented Apr 30, 2025 •

edited

Loading

bjornalm commented Jun 5, 2025 •

edited

Loading