Skip to content

fix(api): make hlsUrl optional, and finish what v21 starts - #2004

Open
johan-bell wants to merge 1 commit into
1878-api-cms-hls-media-data-modelfrom
1878-media-blockers
Open

fix(api): make hlsUrl optional, and finish what v21 starts#2004
johan-bell wants to merge 1 commit into
1878-api-cms-hls-media-data-modelfrom
1878-media-blockers

Conversation

@johan-bell

Copy link
Copy Markdown
Collaborator

The two review blockers, split out of #1998 so each is reviewable on its own. API only.

1. media optional on the parent, hlsUrl optional within it

A document can name a bucket before an encode has produced anything, so hlsUrl is @IsOptional() again — as it is on main. Making it required also meant a legacy audio-only post (media with no HLS collection) failed validation outright and could be neither saved nor deleted.

The one thing the required URL had bought — refusing a key with nothing to decrypt — moves into processMedia, which drops the key with a warning rather than writing a sidecar no player will ever ask for. That is a better place for the rule anyway: it can say why.

2. v21 stranded migrated posts, two ways

v21 moves legacy Content.video onto parent.media.hlsUrl. It did so:

  • without naming a bucket — so a URL sitting under a configured bucket's publicUrl failed its next ordinary save with "Bucket is not specified for media processing", and MediaBucketSelect hides the selector when only one bucket exists, leaving no way out short of re-encoding. It now sets mediaBucketId when the URL matches a bucket; external URLs (YouTube, another CDN) stay bucket-less.
  • without stamping parentMedia — it deleted child.video but left the mirror the app actually reads (parentMedia.hlsUrl || video) untouched, and only a parent save refreshes that. So a migrated post had neither field to play from until someone happened to re-save it. Every translation of a touched parent now receives parentMedia and parentMediaBucketId — not only those that carried a video.

Also

Drops the stray root package-lock.json, sibling of the package.json #1991 removed.

Tests

v21.spec: bucket named for an in-bucket URL, left unset for a YouTube link, media stamped onto every translation. processMediaDto.spec: a key with no URL is dropped and warned, writing no sidecar. validateChangeRequest.spec: media with no playlist URL validates.

986 api tests pass, tsc clean — verified with this branch alone on the epic.

Relationship to #1998

They overlap on one line of v21.ts (parent.media = { fileCollections: [] } vs {}); whichever merges second needs a one-line resolution. The DTO drift is only fully closed when both are in: this PR makes hlsUrl optional, #1998 removes fileCollections from shared so the API and the client agree again.

Two defects from the epic review, split out from the audio removal so
each is reviewable on its own.

Media is optional on a content parent, and a URL is optional within it: a
document can name a bucket before an encode has produced anything, so
hlsUrl is @IsOptional() again. The one thing the required URL had bought
— refusing a key with nothing to decrypt — moves into processMedia,
which drops the key with a warning rather than writing a sidecar no
player will ever ask for.

v21 copies legacy Content.video onto parent.media.hlsUrl but named no
bucket. A URL inside a configured bucket then failed its next ordinary
save with "Bucket is not specified", and the CMS hides the selector when
a single bucket exists, so there was no way out. It now sets
mediaBucketId when the URL lies under a bucket's publicUrl.

v21 also deleted `video` from the children without stamping the parent's
media onto them. The app plays from the child's parentMedia mirror and
only a parent save refreshes it, so after the upgrade a migrated post had
neither field to play from. Every translation of a touched parent now
receives parentMedia and parentMediaBucketId.

Also drops the stray root package-lock.json left beside the package.json
#1991 removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant