Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(project): OWA-56 add DRM check for CDN analytics #469

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

olga-jwp
Copy link
Collaborator

@olga-jwp olga-jwp commented Mar 21, 2024

Description

OWA-52

Steps completed:

According to our definition of done, I have completed the following steps:

  • Acceptance criteria met
  • Unit tests added
  • Docs updated (including config and env variables)
  • Translations added
  • UX tested
  • Browsers / platforms tested
  • Rebased & ready to merge without conflicts
  • Reviewed own code

Copy link

Visit the preview URL for this PR (updated for commit 3186c28):

https://ottwebapp--pr469-owa-56-web-app-user-pq44p614.web.app

(expires Sat, 20 Apr 2024 14:42:28 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c198f8a3a199ba8747819f7f1e45cf602b777529

Comment on lines +27 to +28
const cdnUrlVOD = `https://cdn.jwplayer.com/manifests/${mediaId}.m3u8`;
const cdnUrlBCL = `https://content.jwplatform.com/live/broadcast/${mediaId}.m3u8`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think we should remove the extension check? The sources could also contain MPEG-DASH or MP4 files that won't have the analytic parameters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hey Christiaan, that is a good catch. I was referring to Marco's previous notes while working on this task.
Even though it is still blocked by Media Core team I would suggest discussing this in our monthly sync.
CC: @dbudzins @AntonLantukh

@@ -5,10 +5,12 @@ import type { PlaylistItem, Source } from '../../types/playlist';

export const attachAnalyticsParams = (item: PlaylistItem) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not related to your change, but this function mutates the given item, should we create a new object instead?

export const attachAnalyticsParams = (item: PlaylistItem) => {
  const updatedSources = item.sources.map(source => {
    // logic
    return { ...source, file: url.toString() };
  });

  return { ...item, sources: updatedSources };
};

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