Skip to content

fix: report hdr source option in internal analytics#1053

Merged
tsi merged 3 commits into
masterfrom
fix/hdr-analytics-reporting
Jun 4, 2026
Merged

fix: report hdr source option in internal analytics#1053
tsi merged 3 commits into
masterfrom
fix/hdr-analytics-reporting

Conversation

@tsi

@tsi tsi commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

The hdr source option was intended to be reported to internal analytics but never actually was, due to a type bug. This fixes the reporting so the HDR setting is correctly captured.

Changes

  • In get-analytics-player-options.js, report hdr as a plain boolean instead of running it through hasConfig(). hasConfig() uses lodash isEmpty(), which returns true for any boolean, so hdr always collapsed to null and was then stripped out by filterDefaultsAndNulls. As a result hdr was never present in the analytics payload, regardless of the configured value.

How to Test

  • Configure a source with hdr: true and trigger a source change.
  • Inspect the request to /video_player_source on the internal analytics endpoint and confirm the query string now includes hdr=true.
  • Configure a source without hdr (or hdr: false) and confirm hdr is omitted from the payload.

🤖 Generated with Claude Code

The hdr field used hasConfig() (lodash isEmpty) which always returns
null for booleans, so it was filtered out and never sent. Report the
boolean directly instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tsi tsi requested a review from a team as a code owner June 4, 2026 12:05
@netlify

netlify Bot commented Jun 4, 2026

Copy link
Copy Markdown

Deploy Preview for cld-video-player ready!

Name Link
🔨 Latest commit 41a370d
🔍 Latest deploy log https://app.netlify.com/projects/cld-video-player/deploys/6a216c50df6c730008c5b332
😎 Deploy Preview https://deploy-preview-1053--cld-video-player.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify

netlify Bot commented Jun 4, 2026

Copy link
Copy Markdown

Deploy Preview for cld-vp-esm-pages ready!

Name Link
🔨 Latest commit fa83460
🔍 Latest deploy log https://app.netlify.com/projects/cld-vp-esm-pages/deploys/6a216a204e760700083b42f6
😎 Deploy Preview https://deploy-preview-1053--cld-vp-esm-pages.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify

netlify Bot commented Jun 4, 2026

Copy link
Copy Markdown

Deploy Preview for cld-vp-esm-pages ready!

Name Link
🔨 Latest commit 41a370d
🔍 Latest deploy log https://app.netlify.com/projects/cld-vp-esm-pages/deploys/6a216c500426bb00080237db
😎 Deploy Preview https://deploy-preview-1053--cld-vp-esm-pages.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Same hasConfig() (lodash isEmpty) bug as hdr: both are booleans that
always collapsed to null and were filtered out, so they were never sent.
Report the boolean directly instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tsi tsi merged commit 4a5b5b1 into master Jun 4, 2026
9 of 11 checks passed
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