Skip to content

Strip empty strings from optional media player entity fields#1474

Merged
Jack Keene (Jack-Keene) merged 4 commits into
release/4.8.2from
fix/disallow-empty-string-media
May 22, 2026
Merged

Strip empty strings from optional media player entity fields#1474
Jack Keene (Jack-Keene) merged 4 commits into
release/4.8.2from
fix/disallow-empty-string-media

Conversation

@Jack-Keene
Copy link
Copy Markdown
Contributor

@Jack-Keene Jack Keene (Jack-Keene) commented May 19, 2026

The media_player entity (iglu:com.snowplowanalytics.snowplow/media_player/jsonschema/2-0-0) can produce inconsistent values for optional string fields (label, playerType, quality). The removeEmptyProperties function in browser-plugin-media/src/core.ts strips null and undefined but passes through empty strings "". This means callers supplying "" where undefined is intended produce events where the field is present but empty, rather than absent.

Downstream, the dbt media player package treats "" and NULL as distinct values. When the same media content generates events with both representations, it produces different surrogate keys for media_identifier, causing merge conflicts and duplicate rows in aggregated models.

Fix

Added a stripEmptyStrings option to removeEmptyProperties, enabled only for buildMediaPlayerEntity. This keeps the change scoped to the media player entity — other entity builders (ad, ad break, session, event) are unaffected.
When stripEmptyStrings is true, empty string values are filtered out alongside null/undefined, so optional string fields that are "" are omitted from the payload entirely (equivalent to NULL in the warehouse).

@snowplowcla Snowplow CLA bot (snowplowcla) added the cla:yes [Auto generated] Snowplow Contributor License Agreement has been signed. label May 19, 2026
@matus-tomlein Matus Tomlein (matus-tomlein) changed the base branch from master to release/4.8.2 May 20, 2026 09:40
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but please update the PR title and description to give more context on exactly what changed and why.

Comment thread plugins/browser-plugin-media/src/core.ts Outdated
@Jack-Keene Jack Keene (Jack-Keene) changed the title Add option to strip empty strings Strip empty strings from optional media player entity fields May 20, 2026
@Jack-Keene Jack Keene (Jack-Keene) merged commit c043b3d into release/4.8.2 May 22, 2026
7 of 8 checks passed
@Jack-Keene Jack Keene (Jack-Keene) deleted the fix/disallow-empty-string-media branch May 22, 2026 11:01
Matus Tomlein (matus-tomlein) pushed a commit that referenced this pull request May 22, 2026
* explicitly remove optional properties from media entity
@github-actions github-actions Bot mentioned this pull request May 22, 2026
Matus Tomlein (matus-tomlein) pushed a commit that referenced this pull request May 22, 2026
* explicitly remove optional properties from media entity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla:yes [Auto generated] Snowplow Contributor License Agreement has been signed.

Development

Successfully merging this pull request may close these issues.

3 participants