Skip to content

Add externalId in TheoLiveDistribution#853

Open
OlegRyz wants to merge 2 commits into
developfrom
feature/OPTI-2140-theolive-external-id
Open

Add externalId in TheoLiveDistribution#853
OlegRyz wants to merge 2 commits into
developfrom
feature/OPTI-2140-theolive-external-id

Conversation

@OlegRyz

@OlegRyz OlegRyz commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Caution

Needs native Player SDK v11.6.0 for Android and iOS.

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Thanks for this contribution, @OlegRyz! Great to see the THEOlive distribution API getting richer. 🎉


A distribution arrives, bearing its name,
But lacking a link to the world's wider frame.
Now externalId bridges the gap with grace,
Connecting each channel to its rightful place.
On Android, iOS, and Web — all three,
The bridge now carries what it's meant to be.


Review Summary

Code Analysis

The change adds an optional externalId property to TheoLiveDistribution and bridges it from native Android and iOS DistributionAPI events. Here's my analysis:

TypeScript API (TheoLiveDistribution.ts):

  • ✅ Property correctly typed as externalId?: string (optional) — this is the right approach since older SDK versions won't provide it.
  • 💡 Suggestion: Consider adding a JSDoc comment for the new field, consistent with typical API documentation in the codebase.

Android (THEOliveEventAdapter.kt):

  • ✅ Uses safe null-handling via ?.let { } — correct Kotlin idiom for optional properties.
  • ✅ Follows the existing pattern of the other fields (id, name).

iOS/tvOS (THEOplayerRCTTHEOliveEventAdapter.swift):

  • ✅ Uses if let unwrapping — correct Swift idiom for optionals.
  • ✅ Follows existing pattern of the other distribution fields.

Web platform:

  • ✅ No changes needed — the WebEventForwarder passes through event.distribution directly from the Web SDK, and the TypeScript TheoLiveDistribution interface defines the shape. If the Web SDK returns externalId, it will flow through automatically.

Cross-platform consistency: All three platforms (Android, iOS, Web) are covered. The pattern is consistent across each.


⚠️ CI Build Failures — Native SDK Compatibility Issue

All 3 build failures are caused by this PR (they pass on the develop branch):

Check Status Error
Build for API Level 36 THEOliveEventAdapter.kt:72:49 Cannot infer type for this parameterexternalId does not exist on DistributionAPI in Android SDK [11.5.0, 12.0.0)
Build for iOS (Xcode 26.2.0) value of type 'any DistributionAPI' has no member 'externalId'externalId does not exist on DistributionAPI in iOS SDK ~> 11.4
Build for tvOS (Xcode 26.2.0) Same iOS SDK error as above
lint
build (TS)

Note: The tvOS check has a preexisting e2e flake on develop (exit code 42), but on this PR the tvOS failure is a new build error (no member 'externalId').

Root cause: The current THEOplayer native SDKs (Android [11.5.0, 12.0.0), iOS ~> 11.4) do not yet expose externalId on the DistributionAPI type. This PR will only compile once a native SDK version including this property is released.

⚠️ Caution: This PR requires a newer THEOplayer native player SDK version (both Android and iOS) that includes the externalId property on DistributionAPI. The current SDK version constraints in build.gradle and the .podspec do not resolve to a version that has this API. This PR should be merged only after the native SDKs are updated, or the version constraints should be bumped to the minimum version that includes this property.


📋 Missing CHANGELOG Entry

No CHANGELOG entry was added. Here's a suggested entry that can be committed directly:

## [Unreleased]

### Added

- Added `externalId` to `TheoLiveDistribution`, bridged from native Android and iOS SDKs.

Summary

The code itself is clean and follows existing patterns well. The only blocker is native SDK compatibility — once the THEOplayer Android and iOS SDKs ship with externalId on DistributionAPI, this PR is ready to go (after adding the CHANGELOG entry and bumping SDK version constraints if needed).

Co-Authored-By: tom.vanlaerhoven <tom.vanlaerhoven@dolby.com>
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.

2 participants