Bug Report: Rundowns are not correctly scoped under playlists

[justandras]

About me

This bug report is posted on behalf of CBC

Observed Behavior

Internal rundownIds are generated from the studioId and the rundownExternalId which doesn't allow you to create a rundown in another playlist with the same rundownExternalId. playlistExternalId is not considered.

This overrides the content of an already existing rundown with a different playlistExternalId.

export function getRundownId(studioId: StudioId, rundownExternalId: string): RundownId {
	if (!studioId) throw new Error('getRundownId: studio not set!')
	if (!rundownExternalId) throw new Error('getRundownId: rundownExternalId must be set!')
	return protectString<RundownId>(getHash(`${studioId}_${rundownExternalId}`))
}

On other content levels the Id's are based on the parent's Id. (eg.: allowing two rundowns to both have a segment with the segmentExternalId of 'Intro'):

export function getSegmentId(rundownId: RundownId, segmentExternalId: string): SegmentId {
	if (!rundownId) throw new Error('getSegmentId: rundownId must be set!')
	if (!segmentExternalId) throw new Error('getSegmentId: segmentExternalId must be set!')
	return protectString<SegmentId>(getHash(`${rundownId}_segment_${segmentExternalId}`))
}
export function getPartId(rundownId: RundownId, partExternalId: string): PartId {
	if (!rundownId) throw new Error('getPartId: rundownId must be set!')
	if (!partExternalId) throw new Error('getPartId: partExternalId must be set!')
	return protectString<PartId>(getHash(`${rundownId}_part_${partExternalId}`))
}

Expected Behavior

Internal Id's should be unique for each ingested rundown.

Version

26.3.0-2

Severity / Impact

• Temporary fixes can be made to block the creation of new rundowns with existing rundownExternalIds however that doesn't follow the pattern of other content levels.
• Patching just the function would break the current stable Id's in the system, especially since the Ids cascade.

Level of Involvement

• I am implementing a temporary fix for the Ingest API to block unintended updates
• We need to come up with a plan to roll out a proper fix and also explore ways to make Id's more stable.

[coderabbitai]

🔗 Related PRs

#1469 - feat: Ingest API [merged]
#1595 - feat: rework ExpectedPackages generation/management to share packages between ingest and playout [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[Julusian]

This was done intentionally to allow a rundown to be moved across playlists without getting new ids and being re-ingested.
And because for any rundowns sourced from a mos-gateway, mos doesn't have a concept of playlists, so we don't know the playlistId until we have loaded the rundown from the database

The sofie ui allows users to drag and drop rundowns between playlists, and during ingest a rundown chooses what its playlistExternalId is, which may or may not be respected (depending on whether it has been overriden by moving in the ui).

Also worth noting that getPartId doesn't reference the segmentId for the same reason, to allow parts to move between segments while preserving their id. rundownExternalId is currently used as the sole identifier of a rundown for calls into the job-worker too.

Because this ingest api always has both ids, maybe it should combine them to form the rundownExternalId that it reports to the rest of sofie?
That would give the uniqueness you need, and keep the changes scoped to just this api

[justandras]

You are right, rundowns exist independently of a specific playlist. I did over look that.

The root of the issue came from the Ingest API and is not with the Id generation, rather the REST endpoint treating playlists as the owners of the rundown.

We should not check if a rundown exists in a specific playlist when ingesting a rundown from the Ingest API.

I don't think combining the rundownExternalId and playlistExternalId is a good solution, since that would detach what Sofie think the rundownExternalId is from what the external system thinks.

I would rather like to add a second endpoint that would allow creation of rundowns without specifying a playlist, letting Core create the playlist through it's already existing default behavior.

Blueprints don't have to specify playlist for a rundown to be created, so for /playlists/{playlistId} to have any effect, the blueprints have to support playlist grouping which we cannot assume to always be true.

The second endpoint should call the same implementation just without passing a playlistExternalId.

This way we'd have:

POST /ingest/{studioId}/rundowns

and

POST /ingest/{studioId}/playlists/{playlistId}/rundowns

This keeps the RESTful pattern, while also supports rundown ingest for simple rundown.

Alternatively Core could override the playlist with the one specified in the URL, but that wouldn't follow current patterns.

cc: @imaretic @nytamin