aglinxinyuan commented on code in PR #7463:
URL: https://github.com/apache/texera/pull/7463#discussion_r3885561067
##########
frontend/src/app/dashboard/component/user/search-bar/search-bar.component.ts:
##########
@@ -143,8 +142,6 @@ export class SearchBarComponent {
convertToName(resultItem: SearchResultItem): string {
if (resultItem.workflow) {
return new DashboardEntry(resultItem.workflow).name;
Review Comment:
Fixed as suggested — guarded in `SearchService.search`, the funnel both
consumers call, dropping items with no client-modelled payload
(`workflow`/`file`/`dataset`) so a stale row is ignored rather than fatal. You
were right that this refutes the description's "the /project endpoints simply
stop being called" — the break was on /dashboard/search, not a /project
endpoint.
Added the regression test in `search.service.spec.ts` you asked for: it
flushes a project-shaped row alongside a workflow and asserts only the workflow
survives (and that `more` is untouched). I checked it actually catches the bug
rather than passing vacuously — neutralising the predicate to `() => true`
makes it fail, restoring it makes it pass.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts:
##########
@@ -162,14 +155,13 @@ export class UserWorkflowComponent implements
AfterViewInit, OnDestroy {
constructor(
private userService: UserService,
private workflowPersistService: WorkflowPersistService,
- private userProjectService: UserProjectService,
private notificationService: NotificationService,
- private modalService: NzModalService,
private router: Router,
private downloadService: DownloadService,
private searchService: SearchService,
private config: GuiConfigService,
- private notebookMigrationService: NotebookMigrationService
+ private notebookMigrationService: NotebookMigrationService,
+ private modalService: NzModalService
Review Comment:
Restored `private modalService: NzModalService` to its original fifth
position (after `notificationService`), matching main. It had drifted to last
when the dependency was removed and then re-added during a later merge; the
diff is now the deletion only.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
frontend/src/app/dashboard/component/admin/settings/admin-settings.component.spec.ts:
##########
@@ -542,7 +542,7 @@ describe("AdminSettingsComponent", () => {
});
/**
* The settings form is four near-identical Save/Reset cards, three
near-identical upload blocks and
- * twelve switches whose keys include two confusable singular/plural pairs
(workflow_enabled vs
+ * eleven switches whose keys include two confusable singular/plural pairs
(workflow_enabled vs
Review Comment:
Applied your suggestion — "twelve switches", which I confirmed against the
array (`SWITCH_KEYS` has 12 entries). Your read of the arithmetic was right:
the merge base already carried thirteen keys because main added
`models_enabled`, so the change is 13 -> 12, not 12 -> 11. Corrected the PR
description to "thirteen switches → twelve" as well.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts:
##########
@@ -71,7 +67,7 @@ import { FormsModule } from "@angular/forms";
/**
* Saved-workflow-section component contains information and functionality
- * of the saved workflows section and is re-used in the user projects section
when a project is clicked
+ * of the saved workflows section
Review Comment:
Applied your suggestion verbatim — the docblock line now names what the
component is for rather than restating the class name.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts:
##########
@@ -283,29 +261,22 @@ describe("SavedWorkflowSectionComponent", () => {
});
it("searchByManyParameters", async () => {
- // Apply the project, ID, owner, and operator filter all at once.
+ // Apply the ID, owner, and operator filter all at once.
Review Comment:
Applied — pluralised to "filters".
_🤖 Addressed by [Claude Code](https://claude.com/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]
