feat: ingest API: add the ability to create rundowns without specifying a playlist#1691
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
WalkthroughMakes Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant REST as REST API (ingest route)
participant API as IngestServerAPI.postRundown
participant DB as Playlists DB
participant Worker as Ingest Worker
Client->>REST: POST /ingest/:studioId/rundowns (rundown payload)
REST->>API: postRundown(connection, event, studioId, undefined, rawIngestRundown)
API-->>DB: (optional) lookup playlist by playlistId/externalId
DB-->>API: playlist (or null)
API->>Worker: runIngestOperation(UpdateRundown, ingestRundown, resyncUrl)
Worker-->>API: job accepted / queued
API-->>REST: 202 Accepted
REST-->>Client: 202 Accepted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
meteor/server/lib/rest/v1/ingest.ts (1)
59-65: Keep the REST rundown type aligned with the new studio-scoped API.
postRundowncan now be invoked without a pathplaylistId, butRestApiIngestRundownstill omitsplaylistExternalId. That means the only caller-supplied playlist hint for the new route is currently untyped and unvalidated on the server side.🧩 Suggested type/validation sync
-export type RestApiIngestRundown = Omit<IngestRundown, 'playlistExternalId'> & { +export type RestApiIngestRundown = IngestRundown & { resyncUrl: string }private validateRundown(ingestRundown: RestApiIngestRundown) { check(ingestRundown, Object) check(ingestRundown.externalId, String) check(ingestRundown.name, String) check(ingestRundown.type, String) check(ingestRundown.segments, Array) check(ingestRundown.resyncUrl, String) + if (ingestRundown.playlistExternalId !== undefined) check(ingestRundown.playlistExternalId, String) ingestRundown.segments.forEach((ingestSegment) => this.validateSegment(ingestSegment)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@meteor/server/lib/rest/v1/ingest.ts` around lines 59 - 65, The REST handler postRundown accepts routes without a path playlistId but RestApiIngestRundown still omits playlistExternalId, so add an optional playlistExternalId field to the RestApiIngestRundown type and validate it in postRundown: when playlistId is undefined, use/resolve ingestRundown.playlistExternalId (ensure it is present and well-formed) and fallback behavior is explicit; update any validation/error messages and places that derive a playlist from the request to prefer the path playlistId then ingestRundown.playlistExternalId so server-side typing and runtime checks stay in sync.packages/openapi/src/__tests__/ingest.spec.ts (1)
146-150: Add a case that omitsplaylistExternalId.This new test still passes a playlist hint, so it only verifies the alias route. It won't catch regressions in the actual “create rundown without specifying a playlist” path that motivated this change.
🧪 Suggested extra coverage
test('Can create rundown (studio-scoped)', async () => { const result = await ingestApi.postRundownInStudio({ studioId, rundown: { ...rundown, externalId: 'newRundownInStudio', playlistExternalId: playlistIds[0] }, }) expect(result).toBe(undefined) }) + +test('Can create rundown in studio without playlistExternalId', async () => { + const result = await ingestApi.postRundownInStudio({ + studioId, + rundown: { ...rundown, externalId: 'newRundownInStudioWithoutPlaylist' }, + }) + expect(result).toBe(undefined) +})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/openapi/src/__tests__/ingest.spec.ts` around lines 146 - 150, Add a new test that exercises the "create rundown without specifying a playlist" path by calling ingestApi.postRundownInStudio without the playlistExternalId field; duplicate the existing Can create rundown (studio-scoped) test scenario but remove playlistExternalId from the rundown payload and assert the expected success behavior (same assertions as the existing test). Locate the existing test named "Can create rundown (studio-scoped)" and create a sibling test (e.g., "Can create rundown in studio without playlistExternalId") that calls postRundownInStudio({ studioId, rundown: { ...rundown, externalId: 'newRundownNoPlaylist' } }) to ensure the route works when no playlist hint is provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@meteor/server/api/rest/v1/ingest.ts`:
- Around line 489-492: The code is blindly assigning the raw playlistId path
segment to ingestRundown.playlistExternalId when calling runIngestOperation
(IngestJobs.UpdateRundown), which causes internal playlist _id values to be
stored as external IDs; resolve/normalize playlistId the same way
putRundowns/putRundown do before passing it through: if playlistId refers to an
existing internal playlist _id, load that Playlist (by _id) and use its
externalId, otherwise accept the provided value as a new externalId, then pass
ingestRundown with playlistExternalId set to the resolved external ID.
In `@packages/openapi/api/definitions/ingest.yaml`:
- Around line 228-229: Update the requestBody description in the ingest.yaml
route to stop claiming that playlistExternalId is required: modify the
requestBody.description text so it no longer says "Must include
playlistExternalId" (or rephrase to indicate it's optional or recommended) so
the documented contract matches the shared rundown schema and server behavior;
locate the requestBody description string in
packages/openapi/api/definitions/ingest.yaml and update it accordingly.
---
Nitpick comments:
In `@meteor/server/lib/rest/v1/ingest.ts`:
- Around line 59-65: The REST handler postRundown accepts routes without a path
playlistId but RestApiIngestRundown still omits playlistExternalId, so add an
optional playlistExternalId field to the RestApiIngestRundown type and validate
it in postRundown: when playlistId is undefined, use/resolve
ingestRundown.playlistExternalId (ensure it is present and well-formed) and
fallback behavior is explicit; update any validation/error messages and places
that derive a playlist from the request to prefer the path playlistId then
ingestRundown.playlistExternalId so server-side typing and runtime checks stay
in sync.
In `@packages/openapi/src/__tests__/ingest.spec.ts`:
- Around line 146-150: Add a new test that exercises the "create rundown without
specifying a playlist" path by calling ingestApi.postRundownInStudio without the
playlistExternalId field; duplicate the existing Can create rundown
(studio-scoped) test scenario but remove playlistExternalId from the rundown
payload and assert the expected success behavior (same assertions as the
existing test). Locate the existing test named "Can create rundown
(studio-scoped)" and create a sibling test (e.g., "Can create rundown in studio
without playlistExternalId") that calls postRundownInStudio({ studioId, rundown:
{ ...rundown, externalId: 'newRundownNoPlaylist' } }) to ensure the route works
when no playlist hint is provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2319768-2aec-4622-9608-76005476a0d8
📒 Files selected for processing (5)
meteor/server/api/rest/v1/ingest.tsmeteor/server/lib/rest/v1/ingest.tspackages/openapi/api/actions.yamlpackages/openapi/api/definitions/ingest.yamlpackages/openapi/src/__tests__/ingest.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/openapi/api/definitions/ingest.yaml`:
- Around line 1132-1134: The TypeScript type RestApiIngestRundown currently uses
Omit<IngestRundown, 'playlistExternalId'> but the handler reads and processes
playlistExternalId at runtime; update the type so it includes playlistExternalId
(e.g., change RestApiIngestRundown to equal IngestRundown or otherwise include
playlistExternalId as an optional property) to match the OpenAPI schema and the
handler code that accesses playlistExternalId.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7c9da57-39e3-4887-99b0-66f6e58ccf9e
📒 Files selected for processing (2)
meteor/server/api/rest/v1/ingest.tspackages/openapi/api/definitions/ingest.yaml
✅ Files skipped from review due to trivial changes (1)
- meteor/server/api/rest/v1/ingest.ts
About the Contributor
This pull request is posted on behalf of the CBC.
Type of Contribution
This is a: Feature / Code improvement
Current Behavior
Previously, the Ingest REST API only supported creating a rundown via the endpoint:
This forced external systems to provide a
playlistIdin the URL, treating the playlist as the "owner" of the rundown.As discussed in #1686, this created limitations for workflows where the external system might not know the playlist mapping beforehand or where rundowns need to be ingested more flexibly.
New Behavior
This PR introduces a new studio-scoped endpoint:
(while keeping the already existing endpoint)
Benefits:
playlistIdin the URL path.playlistExternalIdis provided within the rundown's data body, it is respected; otherwise, the system falls back to default behavior (allowing blueprints or core logic to handle playlist grouping/creation).postRundownmethod in the Ingest API now treatsplaylistIdas optional (string | undefined).undefinedas theplaylistId.Testing
Affected areas
IngestServerAPIimplementation.actions.yamlanddefinitions/ingest.yamlto reflect the new endpoint.Time Frame
Not urgent, but we would like to get this merged into the in-development release.
Status