Raja-Hamid opened a new pull request, #7841:
URL: https://github.com/apache/texera/pull/7841
Dropping a folder onto the dataset file uploader reported "1 file failed to
be selected." even though nothing failed and nothing was meant to be uploaded.
fileDropped settles one promise per dropped entry, and there are three
outcomes: a valid file resolves to an item, a directory deliberately resolves
to null, and an oversized or unreadable file rejects. The failure count was
derived from the difference between the total settled results and the non-null
successes, so a directory -- fulfilled, but filtered out as null -- was
indistinguishable from a genuine rejection.
Count the rejected results directly instead. Directories are ignored
silently, while files that really did fail are still reported, with the
existing singular/plural wording.
Closes #7457
### What changes were proposed in this PR?
Dropping a folder onto the dataset file uploader showed a red banner reading
*"1 file failed to be selected."* — even though nothing failed and nothing was
meant to be uploaded. Valid files dropped alongside it were still selected
correctly; only the banner was wrong.
`fileDropped` settles one promise per dropped entry
(`files-uploader.component.ts:283-312`), with three possible outcomes:
| Outcome | How it settles | In `successfulUploads`? | In `results`? |
|---|---|---|---|
| valid file | `resolve({...})` | yes | yes |
| **directory** | `resolve(null)` (`:310`) | no — filtered out as `null` |
yes |
| oversized / unreadable | `reject(...)` (`:295`, `:306`) | no | yes |
`successfulUploads` keeps only fulfilled **non-null** values (`:319-322`),
but the failure count was derived from the raw length:
const failedCount = results.length - successfulUploads.length;
That difference equals *rejections + directories*. A directory is
fulfilled-with-`null`, never a rejection, so it was indistinguishable from a
genuine failure and inflated the count.
The fix counts rejections directly, as the issue proposes:
const failedCount = results.filter(result => result.status ===
"rejected").length;
`resolve(null)` occurs only in the non-file branch, so fulfilled-`null`
corresponds exactly to a directory, and both `reject` sites are real failures.
Directories are now ignored silently; files that genuinely failed are still
reported, with the singular/plural wording untouched.
The behaviour change is limited to that banner — selection, conflict
resolution, and emission are unaffected.
### Any related issues, documentation, discussions?
Closes #7457
### How was this PR tested?
Added five tests under `describe("dropped folders are not failures")` in
`files-uploader.component.spec.ts`, plus a `droppedDirectory()` helper building
an entry with `isFile: false` (the existing `droppedFile()` helper always sets
`isFile: true`):
- a folder on its own produces **no banner at all** — asserts
`fileUploadingFinished === false` and an empty message
- a folder alongside a valid file yields the success banner only, and emits
just that file
- an oversized file on its own still reports `1 file failed to be selected.`
— green before and after, guarding the real-failure path
- a folder plus an oversized file counts exactly **1**, not 2
- a folder plus two oversized files keeps the plural `2 files failed to be
selected.`
Command:
corepack yarn ng test --watch=false \
--include="**/files-uploader.component.spec.ts"
- Against **unmodified** source: **4 failed, 31 passed (35)**. The plural
case reported `3 files failed to be selected.` where `2 files failed to be
selected.` was expected — the dropped folder inflating the count.
- After the fix: **35/35 passing**.
Regression sweep including `dataset-detail.component.spec.ts`, the only
component embedding the uploader:
corepack yarn ng test --watch=false \
--include="**/files-uploader.component.spec.ts" \
--include="**/dataset-detail.component.spec.ts"
Result: **2 files, 230/230 passing**. `yarn prettier-eslint` reports both
touched files unchanged.
Before / after: dropping a folder previously raised a red *"1 file failed to
be selected."* banner; it now completes silently.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Yes, Alongside Claude Code
--
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]
