feat: emit ingest 'user edit' operations from adlib actions#1671
feat: emit ingest 'user edit' operations from adlib actions#1671Julusian wants to merge 4 commits intoSofie-Automation:mainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds playout-triggered ingest operations: new PlayoutOperationChange type, emitIngestOperation API across blueprint contexts, job-worker plumbing to queue and handle playout ingest jobs, and core worker types for the new job. Changes
Sequence DiagramsequenceDiagram
participant Blueprint as Blueprint Code
participant Context as OnTake/OnSetAsNext Context
participant Lib as emitIngestOperation Lib
participant JobQueue as Ingest Job Queue
participant Handler as PlayoutExecuteChangeOperation Handler
participant DB as Rundown Data
Blueprint->>Context: emitIngestOperation(operation)
Context->>Lib: emitIngestOperation(context, playoutModel, operation)
Lib->>Lib: select ref PartInstance (current/next)
Lib->>DB: resolve rundown (playoutModel.getRundown)
Lib->>JobQueue: queue PlayoutExecuteChangeOperation (rundownExternalId, segmentId, partId, operation)
JobQueue->>Handler: execute queued job
Handler->>DB: create UpdateIngestRundownChange with PlayoutOperationChange
Handler->>DB: persist ingest change
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/job-worker/src/blueprints/context/lib.ts (2)
734-740: Redundant optional chaining after null check.
refPartInstanceis guaranteed to be non-null after the check on line 729, so the optional chaining on lines 737-738 is unnecessary.Suggested fix
await context .queueIngestJob(IngestJobs.PlayoutExecuteChangeOperation, { rundownExternalId: rundown.rundown.externalId, - segmentId: refPartInstance?.partInstance.segmentId ?? null, - partId: refPartInstance?.partInstance.part._id ?? null, + segmentId: refPartInstance.partInstance.segmentId, + partId: refPartInstance.partInstance.part._id, operation, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/job-worker/src/blueprints/context/lib.ts` around lines 734 - 740, Remove the redundant optional chaining for refPartInstance when calling context.queueIngestJob with IngestJobs.PlayoutExecuteChangeOperation: since refPartInstance is checked to be non-null earlier, replace refPartInstance?.partInstance.segmentId and refPartInstance?.partInstance.part._id with refPartInstance.partInstance.segmentId and refPartInstance.partInstance.part._id so the call to context.queueIngestJob uses the guaranteed non-null properties.
741-745: Original error details lost in rethrown exception.The original error is logged but the rethrown error only contains a generic message, making debugging harder for callers who catch this exception. Consider including the original error or using error chaining.
Suggested improvement
.catch((e) => { logger.warn(`Failed to queue ingest operation: ${stringifyError(e)}`) - throw new Error('Internal error while queueing ingest operation') + throw new Error(`Internal error while queueing ingest operation: ${stringifyError(e)}`) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/job-worker/src/blueprints/context/lib.ts` around lines 741 - 745, The catch block that currently logs the error with logger.warn(`Failed to queue ingest operation: ${stringifyError(e)}`) then throws new Error('Internal error while queueing ingest operation') loses the original error details; change the rethrow to preserve the original error (either rethrow the original error, throw a new Error with the original message included, or use error chaining via new Error('Internal error while queueing ingest operation', { cause: e })) so callers can access the original exception; update the code around the catch where stringifyError, logger.warn, and the throw occur to include the original error (e) as the cause or include its message/stack in the thrown Error.packages/blueprints-integration/src/ingest.ts (1)
196-207: Consider generic typing forPlayoutOperationChange.operation.
operation: unknownforces runtime casting/guards everywhere it’s consumed. A generic operation type (even with a default) would improve blueprint-side type safety without changing runtime behavior.♻️ Suggested typing upgrade
-export interface PlayoutOperationChange { +export interface PlayoutOperationChange<TOperation = unknown> { /** Indicate that this change is from playout operations */ source: IngestChangeType.Playout @@ /** The blueprint defined payload for the operation */ - operation: unknown + operation: TOperation }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blueprints-integration/src/ingest.ts` around lines 196 - 207, The PlayoutOperationChange interface should be made generic so callers can supply a concrete operation type instead of always using unknown; change PlayoutOperationChange to a generic like PlayoutOperationChange<T = unknown> and replace the operation: unknown field with operation: T, then update any places that reference PlayoutOperationChange (e.g., unions, factory functions, handlers) to either provide a concrete type argument or rely on the default; ensure exported types that compose with PlayoutOperationChange (such as any IngestChange union/type aliases and functions that construct or consume PlayoutOperationChange) are updated to accept the generic parameter or keep using the default to preserve runtime behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/blueprints-integration/src/ingest.ts`:
- Around line 196-207: The PlayoutOperationChange interface should be made
generic so callers can supply a concrete operation type instead of always using
unknown; change PlayoutOperationChange to a generic like
PlayoutOperationChange<T = unknown> and replace the operation: unknown field
with operation: T, then update any places that reference PlayoutOperationChange
(e.g., unions, factory functions, handlers) to either provide a concrete type
argument or rely on the default; ensure exported types that compose with
PlayoutOperationChange (such as any IngestChange union/type aliases and
functions that construct or consume PlayoutOperationChange) are updated to
accept the generic parameter or keep using the default to preserve runtime
behavior.
In `@packages/job-worker/src/blueprints/context/lib.ts`:
- Around line 734-740: Remove the redundant optional chaining for
refPartInstance when calling context.queueIngestJob with
IngestJobs.PlayoutExecuteChangeOperation: since refPartInstance is checked to be
non-null earlier, replace refPartInstance?.partInstance.segmentId and
refPartInstance?.partInstance.part._id with
refPartInstance.partInstance.segmentId and refPartInstance.partInstance.part._id
so the call to context.queueIngestJob uses the guaranteed non-null properties.
- Around line 741-745: The catch block that currently logs the error with
logger.warn(`Failed to queue ingest operation: ${stringifyError(e)}`) then
throws new Error('Internal error while queueing ingest operation') loses the
original error details; change the rethrow to preserve the original error
(either rethrow the original error, throw a new Error with the original message
included, or use error chaining via new Error('Internal error while queueing
ingest operation', { cause: e })) so callers can access the original exception;
update the code around the catch where stringifyError, logger.warn, and the
throw occur to include the original error (e) as the cause or include its
message/stack in the thrown Error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb3ce0c2-8407-4e0e-974d-e58feab14f88
📒 Files selected for processing (14)
packages/blueprints-integration/src/api/studio.tspackages/blueprints-integration/src/context/adlibActionContext.tspackages/blueprints-integration/src/context/executeTsrActionContext.tspackages/blueprints-integration/src/context/onSetAsNextContext.tspackages/blueprints-integration/src/context/onTakeContext.tspackages/blueprints-integration/src/ingest.tspackages/corelib/src/worker/ingest.tspackages/job-worker/src/blueprints/context/OnSetAsNextContext.tspackages/job-worker/src/blueprints/context/OnTakeContext.tspackages/job-worker/src/blueprints/context/adlibActions.tspackages/job-worker/src/blueprints/context/lib.tspackages/job-worker/src/ingest/runOperation.tspackages/job-worker/src/ingest/userOperation.tspackages/job-worker/src/workers/ingest/jobs.ts
About the Contributor
This pull request is posted on behalf of the BBC
Type of Contribution
This is a: Feature
New Behavior
In certain circumstances, it can be desirable to modify some portions of the planned rundown during playout. This is largely an escape hatch for which we have a proposal for a proper solution, but this can still be useful in certain circumstances.
This leverages the existing user-edits flow, to add a playout event source which certain blueprint playout methods are able to emit.
Testing
Affected areas
Time Frame
Other Information
Status