[cueweb/docs] Add Shows page: stats table, Show Properties, subscriptions (CueCommander parity)#2406
Open
ramonfigueiredo wants to merge 4 commits into
Open
Conversation
Replicates the CueGUI CueCommander Shows window in CueWeb, on top of the existing Create Show modal.
- Stats table: the /shows page now uses the shared SimpleDataTable with a new isShowsTable variant and CueGUI's columns - Show Name, Cores Run (reserved_cores), Frames Run (running_frames), Frames Pending (pending_frames), Jobs (pending_jobs) - loaded from GetActiveShows and auto-refreshing every 30s. Numeric columns sort by their underlying value.
- Row context menu (ShowContextMenu): Show Properties and Create Subscription.
- Show Properties dialog (ShowDialog parity), four tabs: Settings (default max/min cores, comment notification email), Booking (enable booking / enable dispatch), Statistics (read-only show_stats), Raw Show Data. Save calls only the setters whose value changed, then refreshes the table.
- Create Subscription dialog (SubscriptionCreator parity): Show + Alloc dropdowns, Size (100), Burst (110).
- Create Show dialog gains the per-allocation Subscriptions section (checkbox + Size + Burst per allocation), creating the show then a subscription on each checked allocation.
- New proxy routes: show/getactiveshows, allocation/getall, and show/action/ {enablebooking,enabledispatching,setdefaultmaxcores,setdefaultmincores, setcommentemail,createsubscription}. All validate the body (400 on malformed JSON / missing fields) and propagate the gateway's real HTTP status. createsubscription maps Cuebot's duplicate-key error to a short, user-facing "show already has a subscription on that allocation" message.
- get_utils: widen the shared Show type (default cores, comment email, extended show_stats) and add an Allocation type plus getActiveShows / getAllocations; action_utils: enableShowBooking/Dispatching, setShowDefaultMax/MinCores, setShowCommentEmail, createShowSubscription, and the row dispatchers; show-action-events.ts holds the shared dialog event contract.
- Tests: Jest coverage for the show action helpers and the show/allocation get_utils helpers.
Contributor
📝 WalkthroughWalkthroughAdds show data shapes and client helpers, creates POST API routes to fetch active shows/allocations and perform show actions, implements ShowPropertiesDialog and CreateSubscriptionDialog with event-driven coordination, refactors shows page to client-side fetching with auto-refresh, updates table/column UI for shows, and adds unit tests for the new utilities. ChangesShows Management Feature
Sequence Diagram sequenceDiagram
participant ShowsClient
participant NextAPI as /api/show/getactiveshows
participant handleRoute
participant Cuebot
ShowsClient->>NextAPI: POST {} (getActiveShows)
NextAPI->>handleRoute: forward to /show.ShowInterface/GetActiveShows
handleRoute->>Cuebot: RPC request
Cuebot-->>handleRoute: RPC response
handleRoute-->>NextAPI: proxied JSON
NextAPI-->>ShowsClient: { data: shows }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
cueweb/app/api/show/getactiveshows/route.ts (2)
23-28: 💤 Low valueRedundant method validation in both POST handlers (consolidated).
Both route handlers include manual POST method validation inside their exported
POSTfunctions. Next.js Route Handlers automatically enforce method restrictions based on the exported function name, making these checks unreachable. Themethodvariable and validation block can be removed in both files, passing'POST'directly tohandleRoute.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cueweb/app/api/show/getactiveshows/route.ts` around lines 23 - 28, Remove the redundant manual method check inside the exported POST handler: delete the local method variable and the if (method !== 'POST') { ... } block in the exported function POST, and instead call handleRoute with the literal 'POST' as the method argument; update any references in this file (e.g., the exported POST function and its call to handleRoute) so the route relies on Next.js's exported-function enforcement and passes 'POST' directly to handleRoute.
30-36: 💤 Low valueRedundant nullish coalescing in both route handlers (consolidated).
Both files initialize
parsedto{}on line 30 and never assign itnullorundefined, making the nullish coalescing operator?? {}on line 36 redundant. Simplify toJSON.stringify(parsed)in both routes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cueweb/app/api/show/getactiveshows/route.ts` around lines 30 - 36, Remove the redundant nullish coalescing when building the request body: parsed is initialized to {} and never set to null/undefined, so change the construction of body to use JSON.stringify(parsed) instead of JSON.stringify(parsed ?? {}); update this in the route handler where parsed is defined (the parsed variable assigned from await request.json()) and the body constant that uses it.cueweb/app/utils/action_utils.ts (1)
23-23: ⚡ Quick winUse shared show-event constants instead of hard-coded event strings.
These literals duplicate the contract already centralized in
show-action-events.ts; importing constants prevents silent drift.Suggested refactor
import { getFrameLogDir, getJobForLayer, Host, JobComment, Show } from "./get_utils"; +import { + OPEN_CREATE_SUBSCRIPTION_EVENT, + OPEN_SHOW_PROPERTIES_EVENT, +} from "`@/components/ui/show-action-events`"; @@ export function showPropertiesGivenRow(row: Row<any>) { if (typeof window === "undefined") return; window.dispatchEvent( - new CustomEvent("cueweb:open-show-properties", { + new CustomEvent(OPEN_SHOW_PROPERTIES_EVENT, { detail: { show: row.original as Show }, }), ); } @@ export function createSubscriptionGivenRow(row: Row<any>) { if (typeof window === "undefined") return; window.dispatchEvent( - new CustomEvent("cueweb:open-create-subscription", { + new CustomEvent(OPEN_CREATE_SUBSCRIPTION_EVENT, { detail: { show: row.original as Show }, }), ); }Also applies to: 1057-1073
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cueweb/app/utils/action_utils.ts` at line 23, Replace hard-coded show-event string literals in this module with the shared constants from show-action-events.ts: add an import for the event constants alongside the existing import of getFrameLogDir, getJobForLayer, Host, JobComment, Show and then swap every literal event string used in action_utils.ts (including occurrences around the current import and the later block at 1057–1073) to the corresponding named constant from show-action-events.ts so the module uses the centralized contract instead of duplicate strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cueweb/app/api/show/action/enablebooking/route.ts`:
- Around line 35-37: The request-body validation currently only checks for
jsonBody and jsonBody.show but doesn't verify the presence and boolean type of
the enabled field; update the guard in the route handler (the code that inspects
jsonBody and returns NextResponse.json) to explicitly check that
jsonBody.enabled is present and is a boolean (e.g., typeof jsonBody.enabled ===
'boolean'), and return the existing 400 error response when that check fails so
malformed payloads are rejected before proceeding.
In `@cueweb/components/ui/create-show-dialog.tsx`:
- Around line 59-65: The subs state is being merged with previous dialog
sessions (setSubs merges prev), causing old selections to persist; change
initialization to build a fresh subs map from the current list (do not
spread/merge prev) when opening the dialog and also reset/clear subs on dialog
close/unmount. Locate the setSubs usage in create-show-dialog (the block
referencing list and setting next[a.id] = { enabled: false, size: "100", burst:
"100" }) and replace the merge logic with a new object populated only from list;
add a cleanup/reset call that sets subs = {} (or equivalent) in the dialog close
handler and ensure the same replacement is applied to the similar block at the
other occurrence (lines noted around the second setSubs usage).
- Line 67: The empty .catch(() => {}) in create-show-dialog.tsx is swallowing
allocation-load failures; replace it with an error handler that surfaces the
failure (log the error and update UI state). Specifically, in the Promise chain
where the allocation is loaded (the expression ending with .catch(() => {})),
replace the empty handler with a function like .catch((err) => {
console.error('allocation load failed', err); setLoading(false);
setError(err?.message || 'Failed to load allocation'); /* or call
showToast/showAlert to inform the user */ }); so the error is both logged and
shown to the user via setError or your existing toast/alert API.
- Around line 110-124: The loop that calls createShowSubscription for each
allocation currently ignores failures so toastSuccess/handleOpenChange/onSuccess
may run even if some subscriptions failed; change the implementation to await
and collect results (use try/catch per await or Promise.allSettled over
createShowSubscription calls for allocs/subs), detect any failures, and only
call toastSuccess, handleOpenChange(false) and onSuccess(…) if all subscriptions
succeeded; if any failed, call an error handler (e.g., toastError) with details
(include failing a.id values or error messages) and avoid closing the dialog or
calling onSuccess so the user can retry.
In `@cueweb/components/ui/create-subscription-dialog.tsx`:
- Around line 95-100: The submit currently coerces invalid size/burst to zero
via "Number.parseFloat(size) || 0" before calling createShowSubscription;
instead, validate the parsed values and block submission when they are NaN or
out-of-range: parse size and burst with Number.parseFloat, check
isNaN(parsedSize) or isNaN(parsedBurst) (and any min/max constraints),
set/return an error state or disable the submit button, and only call
createShowSubscription(show, allocId, parsedSize, parsedBurst) when both parsed
values are valid.
In `@cueweb/components/ui/show-properties-dialog.tsx`:
- Around line 103-126: Validate parsed core inputs (maxCores/minCores) before
building the tasks array: parse nextMax/nextMin from maxCores/minCores, and if
either is NaN or negative, stop the save flow and surface an explicit validation
error (e.g., set an error state or show a message) instead of silently omitting
updates; only when both parsed values are finite and >= 0 proceed to compare
with show.defaultMaxCores/show.defaultMinCores and call
setShowDefaultMaxCores/setShowDefaultMinCores as before, leaving the rest of the
existing checks for setShowCommentEmail, enableShowBooking, and
enableShowDispatching unchanged.
In `@cueweb/components/ui/simple-data-table.tsx`:
- Around line 514-515: The placeholder and aria-label conditional for the table
filter in simple-data-table.tsx currently check isHostsTable, isProcsTable,
isShowsTable, and isFramesTable and otherwise default to "layers"; add an
isFramesLogTable branch to both expressions so the toolbar text reads
appropriately for frame-log tables (e.g., use "Filter frame logs..." for
placeholder and "Filter frame logs" for aria-label) by extending the ternary
chain to include isFramesLogTable before the final fallback.
---
Nitpick comments:
In `@cueweb/app/api/show/getactiveshows/route.ts`:
- Around line 23-28: Remove the redundant manual method check inside the
exported POST handler: delete the local method variable and the if (method !==
'POST') { ... } block in the exported function POST, and instead call
handleRoute with the literal 'POST' as the method argument; update any
references in this file (e.g., the exported POST function and its call to
handleRoute) so the route relies on Next.js's exported-function enforcement and
passes 'POST' directly to handleRoute.
- Around line 30-36: Remove the redundant nullish coalescing when building the
request body: parsed is initialized to {} and never set to null/undefined, so
change the construction of body to use JSON.stringify(parsed) instead of
JSON.stringify(parsed ?? {}); update this in the route handler where parsed is
defined (the parsed variable assigned from await request.json()) and the body
constant that uses it.
In `@cueweb/app/utils/action_utils.ts`:
- Line 23: Replace hard-coded show-event string literals in this module with the
shared constants from show-action-events.ts: add an import for the event
constants alongside the existing import of getFrameLogDir, getJobForLayer, Host,
JobComment, Show and then swap every literal event string used in
action_utils.ts (including occurrences around the current import and the later
block at 1057–1073) to the corresponding named constant from
show-action-events.ts so the module uses the centralized contract instead of
duplicate strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38c1c41e-a487-465e-81b6-4a1e36523f13
📒 Files selected for processing (21)
cueweb/app/__tests__/api/utils/show_action_utils.test.tscueweb/app/__tests__/utils/show_get_utils.test.tscueweb/app/api/allocation/getall/route.tscueweb/app/api/show/action/createsubscription/route.tscueweb/app/api/show/action/enablebooking/route.tscueweb/app/api/show/action/enabledispatching/route.tscueweb/app/api/show/action/setcommentemail/route.tscueweb/app/api/show/action/setdefaultmaxcores/route.tscueweb/app/api/show/action/setdefaultmincores/route.tscueweb/app/api/show/getactiveshows/route.tscueweb/app/shows/page.tsxcueweb/app/shows/show-columns.tsxcueweb/app/shows/shows-client.tsxcueweb/app/utils/action_utils.tscueweb/app/utils/get_utils.tscueweb/components/ui/context_menus/action-context-menu.tsxcueweb/components/ui/create-show-dialog.tsxcueweb/components/ui/create-subscription-dialog.tsxcueweb/components/ui/show-action-events.tscueweb/components/ui/show-properties-dialog.tsxcueweb/components/ui/simple-data-table.tsx
Replicates the CueGUI CueCommander Shows window in CueWeb, on top of the existing Create Show modal.
- Stats table: the /shows page now uses the shared SimpleDataTable with a new isShowsTable variant and CueGUI's columns - Show Name, Cores Run (reserved_cores), Frames Run (running_frames), Frames Pending (pending_frames), Jobs (pending_jobs) - loaded from GetActiveShows and auto-refreshing every 30s. Numeric columns sort by their underlying value.
- Row context menu (ShowContextMenu): Show Properties and Create Subscription.
- Show Properties dialog (ShowDialog parity), four tabs: Settings (default max/min cores, comment notification email), Booking (enable booking / enable dispatch), Statistics (read-only show_stats), Raw Show Data. Save calls only the setters whose value changed, then refreshes the table.
- Create Subscription dialog (SubscriptionCreator parity): Show + Alloc dropdowns, Size (100), Burst (110).
- Create Show dialog gains the per-allocation Subscriptions section (checkbox + Size + Burst per allocation), creating the show then a subscription on each checked allocation.
- New proxy routes: show/getactiveshows, allocation/getall, and show/action/ {enablebooking,enabledispatching,setdefaultmaxcores,setdefaultmincores, setcommentemail,createsubscription}. All validate the body (400 on malformed JSON / missing fields) and propagate the gateway's real HTTP status. createsubscription maps Cuebot's duplicate-key error to a short, user-facing "show already has a subscription on that allocation" message.
- get_utils: widen the shared Show type (default cores, comment email, extended show_stats) and add an Allocation type plus getActiveShows / getAllocations; action_utils: enableShowBooking/Dispatching, setShowDefaultMax/MinCores, setShowCommentEmail, createShowSubscription, and the row dispatchers; show-action-events.ts holds the shared dialog event contract.
- Tests: Jest coverage for the show action helpers and the show/allocation get_utils helpers.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cueweb/app/api/show/action/setdefaultmaxcores/route.ts (1)
35-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject negative default core values at the route boundary.
Line [35] only checks numeric type, so negative
max_coresstill passes validation and can be forwarded as an invalid mutation payload.Suggested fix
- if (!jsonBody || typeof jsonBody !== 'object' || !jsonBody.show || typeof jsonBody.max_cores !== 'number') { + if ( + !jsonBody || + typeof jsonBody !== 'object' || + !jsonBody.show || + typeof jsonBody.max_cores !== 'number' || + !Number.isFinite(jsonBody.max_cores) || + jsonBody.max_cores < 0 + ) { return NextResponse.json({ error: 'Invalid request body' }, { status: 400 }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cueweb/app/api/show/action/setdefaultmaxcores/route.ts` at line 35, The current request validation only checks that jsonBody.max_cores is a number and therefore allows negative values; update the route input guard in setdefaultmaxcores/route.ts to reject negatives by ensuring jsonBody.max_cores is a non-negative integer (e.g., typeof jsonBody.max_cores === 'number' && Number.isInteger(jsonBody.max_cores) && jsonBody.max_cores >= 0) and return a 4xx error when that fails so negative default core values are blocked at the route boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@cueweb/app/api/show/action/setdefaultmaxcores/route.ts`:
- Line 35: The current request validation only checks that jsonBody.max_cores is
a number and therefore allows negative values; update the route input guard in
setdefaultmaxcores/route.ts to reject negatives by ensuring jsonBody.max_cores
is a non-negative integer (e.g., typeof jsonBody.max_cores === 'number' &&
Number.isInteger(jsonBody.max_cores) && jsonBody.max_cores >= 0) and return a
4xx error when that fails so negative default core values are blocked at the
route boundary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9fbf2827-93b3-4681-b883-01c85a4aa7d7
📒 Files selected for processing (8)
cueweb/app/api/show/action/enablebooking/route.tscueweb/app/api/show/action/enabledispatching/route.tscueweb/app/api/show/action/setdefaultmaxcores/route.tscueweb/app/api/show/action/setdefaultmincores/route.tscueweb/components/ui/create-show-dialog.tsxcueweb/components/ui/create-subscription-dialog.tsxcueweb/components/ui/show-properties-dialog.tsxcueweb/components/ui/simple-data-table.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- cueweb/app/api/show/action/setdefaultmincores/route.ts
- cueweb/app/api/show/action/enabledispatching/route.ts
- cueweb/components/ui/create-show-dialog.tsx
- cueweb/components/ui/simple-data-table.tsx
Documents CueWeb's CueCommander Shows window (stats table, Create Show, Show Properties, Create Subscription) across the CueWeb docs. - User guide (cueweb-user-guide.md): new Shows section with the stats table, a Show columns table, Create Show (with subscriptions), the row actions menu, the four Show Properties tabs (Settings / Booking / Statistics / Raw Show Data), and Create Subscription - each with a light screenshot. - Overview (other-guides/cueweb.md): add a Shows feature entry (stats table, Create Show with per-allocation subscriptions, Show Properties and Create Subscription row actions). - Reference (reference/cueweb.md): add a Shows behavior section plus Show Properties / Create Subscription subsections, list the new show RPCs and /api/show/... + /api/allocation/getall proxy routes (with body-validation and duplicate-error notes), and mark the Shows route implemented in the roadmap. - Developer guide (cueweb-development.md): note the SimpleDataTable isShowsTable flag / ShowContextMenu, and add a Shows window section (file layout, stats table, CustomEvent dance, the three dialogs and their validation, action helpers/routes). - Add Shows screenshots (light + dark) for the page, columns, search, menu, Create Show, Create Subscription, and the four Show Properties tabs.
Collaborator
Author
|
@DiegoTavares / @lithorus |
Collaborator
Author
|
Number("") and Number(" ") return 0, so a blank numeric field passed the finite/non-negative checks as a valid 0 and could silently persist an unintended zero default or subscription value. Reject empty/whitespace input explicitly before parsing in all three paths:
- Show Properties dialog: default max/min cores.
- Create Subscription dialog: Size / Burst.
- Create Show dialog: each checked allocation's Size / Burst.
Each blocks submit with the message it already used (toast or inline error); the existing NaN/negative checks still cover the other invalid cases.
ramonfigueiredo
changed the title
ramonfigueiredo
changed the title
ramonfigueiredo
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
Main issue:
Issues related to this PR:
Summarize your change.
[cueweb] Add Shows page: stats table, Show Properties, subscriptions (CueCommander parity)
Replicates the CueGUI CueCommander Shows window in CueWeb, on top of the existing Create Show modal.
[cueweb/docs] Document the Shows page
Documents CueWeb's CueCommander Shows window (stats table, Create Show, Show Properties, Create Subscription) across the CueWeb docs.
LLM usage disclosure
Parts of this solution's implementation were developed with assistance from Claude Opus.