You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "aglinxinyuan (via GitHub)" <gi...@apache.org> on 2026/08/20 05:41:38 UTC
[PR] fix(frontend): sort cache size ascending, drop the broken preset updater [texera]
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(frontend): sort cache size ascending, drop the broken preset updater [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7806:
URL: https://github.com/apache/texera/pull/7806#issuecomment-5351853741
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@Yicong-Huang`, `@PG1204`, `@Ma77Ball`
You can notify them by mentioning `@Yicong-Huang`, `@PG1204`, `@Ma77Ball` in a comment.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(frontend): sort cache size ascending, drop the broken preset updater [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7806:
URL: https://github.com/apache/texera/pull/7806#issuecomment-5361021022
Backport PR opened: draft #7808 (https://github.com/apache/texera/pull/7808) to `release/v1.2`, assigned to @aglinxinyuan — needs manual work because the cherry-pick conflicts.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(frontend): sort cache size ascending, drop the broken preset updater [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7806:
URL: https://github.com/apache/texera/pull/7806#issuecomment-5351943637
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/7806?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 91.53%. Comparing base ([`c8b9495`](https://app.codecov.io/gh/apache/texera/commit/c8b9495d96e90b93873993bce2ba8898b3280941?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2d8d300`](https://app.codecov.io/gh/apache/texera/commit/2d8d3001b75af241e8886b5df3d8f30a2dc7373f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #7806 +/- ##
============================================
+ Coverage 91.34% 91.53% +0.19%
Complexity 4479 4479
============================================
Files 1171 1171
Lines 47199 47190 -9
Branches 5303 5298 -5
============================================
+ Hits 43114 43196 +82
+ Misses 2428 2340 -88
+ Partials 1657 1654 -3
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (ø)` | | Carriedforward from [c8b9495](https://app.codecov.io/gh/apache/texera/commit/c8b9495d96e90b93873993bce2ba8898b3280941?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.62% <ø> (ø)` | | Carriedforward from [c8b9495](https://app.codecov.io/gh/apache/texera/commit/c8b9495d96e90b93873993bce2ba8898b3280941?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `88.10% <ø> (ø)` | | Carriedforward from [c8b9495](https://app.codecov.io/gh/apache/texera/commit/c8b9495d96e90b93873993bce2ba8898b3280941?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.67% <ø> (ø)` | | Carriedforward from [c8b9495](https://app.codecov.io/gh/apache/texera/commit/c8b9495d96e90b93873993bce2ba8898b3280941?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.73% <ø> (ø)` | | Carriedforward from [c8b9495](https://app.codecov.io/gh/apache/texera/commit/c8b9495d96e90b93873993bce2ba8898b3280941?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.90% <ø> (ø)` | | Carriedforward from [c8b9495](https://app.codecov.io/gh/apache/texera/commit/c8b9495d96e90b93873993bce2ba8898b3280941?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `93.28% <100.00%> (+0.43%)` | :arrow_up: | |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.13% <ø> (ø)` | | Carriedforward from [c8b9495](https://app.codecov.io/gh/apache/texera/commit/c8b9495d96e90b93873993bce2ba8898b3280941?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.57% <ø> (ø)` | | Carriedforward from [c8b9495](https://app.codecov.io/gh/apache/texera/commit/c8b9495d96e90b93873993bce2ba8898b3280941?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/7806/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.19% <ø> (ø)` | | Carriedforward from [c8b9495](https://app.codecov.io/gh/apache/texera/commit/c8b9495d96e90b93873993bce2ba8898b3280941?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/7806?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(frontend): sort cache size ascending, drop the broken preset updater [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #7806:
URL: https://github.com/apache/texera/pull/7806
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(frontend): sort cache size ascending, drop the broken preset updater [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7806:
URL: https://github.com/apache/texera/pull/7806#issuecomment-5351853335
<!-- backport-auto-label-report -->
### Backport auto-label report
This `fix:` PR was checked against each actively-supported release branch. `release/*` labels drive the post-merge backport, so add or remove one to change where this fix lands.
| Release branch | Analysis |
| --- | --- |
| ✅ `release/v1.2` | Change detected on this branch — label added; this fix is queued to backport here. Requested review from @xuang7. |
_[Auto-label run](https://github.com/apache/texera/actions/runs/32336566922)._
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org