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: move blueprint ab logic into sofie SOFIE-2403 #946

Merged
merged 18 commits into from Jun 13, 2023

Conversation

Julusian
Copy link
Member

@Julusian Julusian commented Jun 8, 2023

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

feature

  • What is the current behavior? (You can also link to an open issue here)

AB playback logic is 99% handled by blueprints in onTimelineGenerate. There are a couple of helper methods provided to deal with ids, the rest is left to blueprints to figure out themselves.

  • What is the new behavior (if this is a feature change)?

All of this AB logic that blueprints used to have to implement, is now moved into the core of Sofie.
Blueprints can enable this AB logic by implementing the getAbResolverConfiguration method in the ShowStyle blueprint, which is where the pools are defined along with any rules on how to apply the player assignments to the timeline.

  • Other information:

Q) Should abSession be defined on Piece or PieceGeneric? There are complications risen due to needing to handle AB from adlib pieces, so perhaps we could take a stance of not supporting it?

Status

  • Code documentation for the relevant parts in the code have been added/updated by the PR author
  • The functionality has been tested by the PR author
  • The functionality has been tested by NRK

@codecov-commenter
Copy link

codecov-commenter commented Jun 8, 2023

Codecov Report

Patch coverage: 83.81% and project coverage change: -24.53 ⚠️

Comparison is base (617b57a) 80.86% compared to head (15f0f61) 56.33%.

Additional details and impacted files
@@              Coverage Diff               @@
##           release50     #946       +/-   ##
==============================================
- Coverage      80.86%   56.33%   -24.53%     
==============================================
  Files            135      492      +357     
  Lines          26523    78225    +51702     
  Branches        2116     3874     +1758     
==============================================
+ Hits           21448    44070    +22622     
- Misses          5050    34105    +29055     
- Partials          25       50       +25     
Impacted Files Coverage Δ
...ackages/job-worker/src/playout/abPlayback/index.ts 25.23% <25.23%> (ø)
...rc/blueprints/context/OnTimelineGenerateContext.ts 46.39% <37.50%> (-51.81%) ⬇️
...-worker/src/playout/abPlayback/applyAssignments.ts 73.03% <73.03%> (ø)
...ckages/job-worker/src/playout/timeline/generate.ts 92.55% <86.00%> (+0.17%) ⬆️
...orker/src/playout/abPlayback/abPlaybackSessions.ts 88.88% <88.88%> (ø)
...b-worker/src/playout/abPlayback/abSessionHelper.ts 98.93% <98.93%> (ø)
...orker/src/playout/abPlayback/abPlaybackResolver.ts 99.23% <99.23%> (ø)
packages/blueprints-integration/src/context.ts 100.00% <100.00%> (ø)
packages/corelib/src/dataModel/RundownPlaylist.ts 100.00% <100.00%> (ø)
packages/job-worker/src/blueprints/context/lib.ts 82.45% <100.00%> (+0.07%) ⬆️
... and 3 more

... and 352 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Julusian Julusian marked this pull request as ready for review June 8, 2023 16:10
@Julusian Julusian changed the title feat: move blueprint ab logic into sofie feat: move blueprint ab logic into sofie SOFIE-2403 Jun 8, 2023
@Julusian Julusian requested a review from a team June 8, 2023 16:10
Copy link
Contributor

@nytamin nytamin left a comment

Choose a reason for hiding this comment

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

  • Code quality: Skimmed it, looks good
  • Unit tests exist
  • Documentation has been updated

@Julusian Julusian merged commit cc37b75 into release50 Jun 13, 2023
64 of 66 checks passed
@Julusian Julusian deleted the feat/move-ab-logic-to-core branch June 13, 2023 12:46
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.

None yet

4 participants