Skip to content

iris/dashboard: star top-level jobs and filter to starred#5007

Merged
ravwojdyla merged 5 commits intomainfrom
worktree-rav-iris-dashboard-star
Apr 22, 2026
Merged

iris/dashboard: star top-level jobs and filter to starred#5007
ravwojdyla merged 5 commits intomainfrom
worktree-rav-iris-dashboard-star

Conversation

@ravwojdyla-agent
Copy link
Copy Markdown
Contributor

@ravwojdyla-agent ravwojdyla-agent commented Apr 21, 2026

  • add a star button to top-level jobs in the controller jobs tab
  • starred jobIds persisted in localStorage under iris.controller.starredJobs 1
  • new "Starred" toggle in the filter bar, URL-synced as ?starred=1
  • starred mode calls GetJobStatus per starred id (via Promise.allSettled) and sorts/filters client-side
    • sidesteps server pagination — ListJobs has no jobId IN (...) filter, so starred jobs outside the current page would otherwise disappear
    • pagination UI is hidden in starred mode since the set is already bounded
  • handleFilterClear / hasActiveFilter now include the starred toggle

Footnotes

  1. deliberately localStorage (persists across sessions), unlike the sessionStorage-backed expanded-rows state.

Adds a star button to top-level jobs in the JobsTab, persisted in
localStorage under iris.controller.starredJobs, plus a "Starred" toggle
in the filter bar. Starred mode fetches each starred job via
GetJobStatus (client-side sorted/filtered) so it is not affected by
server pagination. URL-synced as ?starred=1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ravwojdyla-agent ravwojdyla-agent added the agent-generated Created by automation/agent label Apr 21, 2026
@ravwojdyla
Copy link
Copy Markdown
Contributor

ravwojdyla commented Apr 21, 2026

Example:

image

@ravwojdyla ravwojdyla requested review from Helw150 and rjpower April 21, 2026 18:32
starredError.value = null
try {
const results = await Promise.allSettled(
ids.map(id => controllerRpcCall<GetJobStatusResponse>('GetJobStatus', { jobId: id })),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we cap number of starred jobs somehow? I can imagine the server being mildly unhappy if every refresh of the page fires off 100 job status requests or something similar.

Copy link
Copy Markdown
Contributor

@ravwojdyla ravwojdyla Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good - point - let's limit to some safe/small number for now - say 10? If people use this and complain it's not enough - we can decide what to do then.

Copy link
Copy Markdown
Member

@Helw150 Helw150 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frontend code so mostly not worried - minor Q about whether we should be worried about the server load this induces if you have lots of starred jobs.

ravwojdyla and others added 2 commits April 21, 2026 12:36
GetJobStatus now reports has_children so the dashboard can render the
expand toggle when a starred job is viewed via the starred-only filter
(ListJobs sets it, GetJobStatus didn't, so starred parents appeared
leaf-only). Starring is also capped at 10 in the JobsTab: adding past
the cap flashes a notice asking the user to unstar first.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The GetJobStatus endpoint only started reporting has_children in this
branch, so the dashboard would fail to show the expand toggle for
starred top-level jobs when the controller hasn't been restarted yet.
Show the toggle defensively at depth 0 in starred mode and auto-collapse
if the subsequent ListJobs returns no children.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rjpower
Copy link
Copy Markdown
Collaborator

rjpower commented Apr 21, 2026

@claude seems like we could likely group the various starred* refs together and it would be a bit less sloppy, yes? Can you try to do that?

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 21, 2026

Claude finished @rjpower's task in 8m 35s —— View job


Task: Group starred* refs together

  • Read current JobsTab.vue to locate all starred* refs
  • Design a grouping — extract useStarredJobs composable (matches existing useAutoRefresh/useRpc pattern)
  • Add src/composables/useStarredJobs.ts
  • Refactor JobsTab.vue to use the composable (starred.ids, starred.jobs, starred.toggle, …)
  • Run pre-commit and relevant tests
  • Commit and push (64bcebaa0)

Summary

Grouped the scattered starred* refs behind a single useStarredJobs composable (lib/iris/dashboard/src/composables/useStarredJobs.ts). Concretely collapsed:

before (free-standing in JobsTab.vue) after
starredJobIds, showStarredOnly, starredJobsData, starredLoading, starredError, starLimitNotice refs starred.ids, starred.showOnly, starred.jobs, starred.loading, starred.error, starred.limitNotice
loadStarredJobs(), saveStarredJobs(), toggleStar(), fetchStarredJobs() starred.toggle(), starred.fetch() (load/save are now private to the composable)
STARRED_JOBS_KEY, MAX_STARRED_JOBS top-level constants passed as storageKey/maxCount options
post-construction mutation (starred.showOnly = … hydrated from URL) initialShowOnly option

The composable returns a reactive() object so starred.ids, starred.showOnly, etc. auto-unwrap in both script and template — no .value sprinkled through the callsite. Watch sources use a getter (watch(() => starred.showOnly, …)).

Kept a thin toggleStar(job) wrapper in the component so starring while in "starred only" mode still triggers an immediate refetch — that's a cross-cutting concern between starred and fetchJobs, so it belongs at the callsite rather than inside the composable.

JobsTab.vue net: +42 / −100 lines.

How I tested

  • ./infra/pre-commit.py --all-files --fixOK (ruff, black, pyrefly, etc.).
  • uv run --group dev python -m pytest lib/iris/tests/cluster/controller/test_service.py51 passed in 4.69s.
  • Did not run vue-tsc / rsbuild buildnpm install is blocked in this sandbox, so I couldn't install dashboard node deps. Manually audited the file for leftover .value, stale identifier names, and reactive/watch semantics; a grep for the old names (starredJobIds|showStarredOnly|…) across lib/iris/dashboard returns no hits.
    · branch: worktree-rav-iris-dashboard-star

Group the starred-jobs state (ids, showOnly, jobs, loading, error,
limitNotice) and its helpers (toggle, fetch) behind a single reactive
object returned by useStarredJobs(). Replaces seven ad-hoc refs plus
four free-standing helpers in JobsTab.vue with `starred.*` accessors.

Returned as reactive() so refs auto-unwrap in both script and template;
watches use a getter (`() => starred.showOnly`).

Co-authored-by: Russell Power <rjpower@users.noreply.github.com>
@rjpower
Copy link
Copy Markdown
Collaborator

rjpower commented Apr 21, 2026

@claude that makes it harder to follow, let's undo the composable change, I just meant to group the refs in the JobsTab itself. The original code was better

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 21, 2026

Claude finished @rjpower's task in 3m 30s —— View job


Task: Undo the useStarredJobs composable change.

  • Inspect the current branch state
  • Revert 64bcebaa0 ("iris/dashboard: extract useStarredJobs composable") via git revert
  • Verify JobsTab.vue is byte-identical to the pre-composable commit c8242cf3c (no unintended drift) and useStarredJobs.ts is deleted
  • Run ./infra/pre-commit.py --all-files --fixOK
  • Run uv run --group dev python -m pytest lib/iris/tests/cluster/controller/test_service.py51 passed in 4.95s
  • Push (5d93a8eaa)

Summary

Reverted the composable extraction (commit 5d93a8eaa). lib/iris/dashboard/src/composables/useStarredJobs.ts is deleted and JobsTab.vue is restored exactly to the pre-composable state — git diff c8242cf3c HEAD -- <both files> is empty. Net change of this revert: +100 / −165, which exactly undoes the composable commit's +165 / −100.

Kept the earlier commits in the PR untouched (9743b8f2e star button, f463f4433 has_children + star cap, c8242cf3c expand-toggle fix).

How I tested

  • ./infra/pre-commit.py --all-files --fix → OK (ruff, black, pyrefly, etc.).
  • uv run --group dev python -m pytest lib/iris/tests/cluster/controller/test_service.py → 51 passed in 4.95s.
  • Did not run vue-tsc / rsbuild build (dashboard node_modules not installable in this sandbox), but the revert restores bytes identical to c8242cf3c, which you had already reviewed and merged through 64bcebaa0.

· branch: worktree-rav-iris-dashboard-star
·

@ravwojdyla
Copy link
Copy Markdown
Contributor

@rjpower this is good to go?

@ravwojdyla ravwojdyla merged commit 06f2a31 into main Apr 22, 2026
40 checks passed
@ravwojdyla ravwojdyla deleted the worktree-rav-iris-dashboard-star branch April 22, 2026 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-generated Created by automation/agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants