aglinxinyuan opened a new pull request, #7806:
URL: https://github.com/apache/texera/pull/7806
### What changes were proposed in this PR?
Three frontend defects. **5 files, +14 / −60.**
**1. `sortBySize` now matches the `NzTableSortFn` contract.** It returned `b
- a`, but ng-zorro uses the comparator's result as-is for `ascend` and negates
it for `descend` (`ng-zorro-antd-table.mjs:822`), with `nzSortDirections`
defaulting to `['ascend','descend',null]` (`:1063`) — which this header does
not override. So the first click **lit the up-caret while rendering
largest-first**. `admin-user.component.ts:268` already uses the
contract-correct form, so this was a one-off, not a convention.
**2. `PresetService.updatePreset` removed — dead *and* broken.** No caller
repo-wide outside its own spec, and it could not have worked: `indexOf(presets,
originalPreset)` is lodash **reference** equality against a freshly
`JSON.parse`d array. Reproduced by execution:
```
contains(presets, originalPreset) = true (the guard passes)
indexOf(presets, originalPreset) = -1
splice branch -> [{"p":"a"},{"p":"b"}] // deleted the LAST
preset, not the target
assign branch -> [{"p":"a"},{"p":"b"},{"p":"c"}] // the replacement was
dropped entirely
```
Its sibling `updateOrCreatePreset` carries the explanatory comment and uses
`findIndex(isEqual)` — the fix was applied there and not here. Removed the
method, its doc block, the now-unused `indexOf` import, and the one spec test
that existed solely for it.
**3. An unreachable template fallback simplified.** `{{ displayFileName ||
"Selected image" }}` → `{{ displayFileName }}`. Guard dominance confirmed: the
enclosing div is `*ngIf="previewSrc"`, `previewSrc` returns `hasImage ?
formControl.value : ""`, so a truthy `previewSrc` implies `hasImage`, and
`displayFileName` then returns a non-empty filename or `"Uploaded image"` —
never empty. No subclass, single template consumer. The spec already asserted
the span reads `"Uploaded image"` in the no-filename case, which is direct
evidence the fallback was dead.
### The sort fix is pinned in both directions
The spec already held a **characterization** test recording the broken
caret/order pairing, with a comment saying a fix "is meant to flip both
halves". Both halves are now flipped to assert correct behaviour, and verified
with the production file reverted and restored:
| | production reverted | with fix |
|---|---|---|
| `user-quota.component.spec.ts` | **2 failed, 23 passed** | **25 passed** |
The failure that matters:
```
AssertionError: expected [ '30', '10', '20' ] to deeply equal [ '20', '10',
'30' ]
```
And the detail that makes this a real user-facing bug rather than a
comparator nit: in the before-state the **caret assertions passed**
(`caretActive("up") === true`) while the rows came out largest-first. That is
exactly what a user sees. Asserting row order alone would still pass against a
comparator inverted the other way, so the test pins the caret and the order
together, in both directions.
### Verification
- `user-quota.component.spec.ts` + `preset.service.spec.ts` +
`hugging-face-image-upload.component.spec.ts`: **124 passed (124)**.
- `npx ng build`: **exit 0**, zero errors and zero NG####/TS diagnostics.
Worth running because `ng test` and `tsc --noEmit` both miss Angular template
errors — and the bundle was grepped to confirm the template change was
genuinely AOT-compiled: `"Uploaded image"` appears in 2 chunks, `"Selected
image"` in **0**.
- `yarn format:ci`: exit 0.
### A follow-up worth filing separately
The same defect class appears six more times in `admin-user.component.ts`:
`sortByName`, `sortByEmail`, `sortByComment`, `sortByRole`, `sortByAffiliation`
and `sortByJoiningReason` all use `b.localeCompare(a)`, and their headers set
`nzSortDirections="['ascend','descend']"`, so they also start at `ascend` and
render reversed. Not touched here — different component, and it deserves its
own change with its own tests.
### Any related issues, documentation, discussions?
Closes #7805
### How was this PR tested?
```
npx ng test --watch=false --include="**/user-quota.component.spec.ts"
--include="**/preset.service.spec.ts"
--include="**/hugging-face-image-upload.component.spec.ts"
```
```
Test Files 3 passed (3)
Tests 124 passed (124)
```
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
--
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]
