-
Notifications
You must be signed in to change notification settings - Fork 0
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: 'overrides' SUPERFLY-2 #5
base: upstream-release51
Are you sure you want to change the base?
Conversation
…eRundownFromIngestData
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am very unsure about what the intended flow is for these userEditStates
properties which are everywhere.
This is likely because it looks like the ui isn't complete in this, so I don't have a good grasp on how they are intended to be used
Should it be more strongly typed? I see that the mute segment feature has the ui looking for a specific key. Or will this be used with some 'looser' matching that will allow any string to be used?
Perhaps we should talk through this to help me understand.
A bunch of places are using CoreUserEditingDefinitionAction
instead of CoreUserEditingDefinition
, which has resulted in needing a lot of casting to make the types 'match', even though those CoreUserEditingDefinitionAction
could also be CoreUserEditingDefinitionForm
which now isnt reflected in the types.
@@ -385,6 +385,8 @@ export class PartAndPieceInstanceActionService { | |||
invalidReason: undefined, | |||
floated: false, | |||
expectedDurationWithTransition: undefined, // Filled in later | |||
userEditStates: {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be removing whatever the blueprints tried to set when queuing this part?
I suspect this might be intentional, if so a comment to justify it would be good so that someone doesnt change it thinking this is a bug later on.
packages/job-worker/src/blueprints/ingest/MutableIngestPartImpl.ts
Outdated
Show resolved
Hide resolved
packages/job-worker/src/blueprints/ingest/MutableIngestPartImpl.ts
Outdated
Show resolved
Hide resolved
packages/job-worker/src/ingest/model/implementation/IngestModelImpl.ts
Outdated
Show resolved
Hide resolved
…down activation (SOFIE-3478)
* chore(ci): Add build step for openapi for test and publishing * chore: fix ci * chore(ci): openapi: re-add gendocs and genserver to CI
Bug fix: currentPart timeline duration should respect postroll in autonext
…-processIngestData
About the Contributor
Type of Contribution
This is a Feature
New Behavior
This lays the groundwork for 'overrides'.
As far as I am aware, the backend portion of this is implemented, and works.
Large portions of the ingest flow have been rewritten/refactored to achieve this, which has resulted in this being a very large change.
Currently, this exposes a user action for the user facing operations, but no ui to trigger it
Note: naming of the feature and methods is not final, and might still be changed.
The core of this feature is that blueprints have a new method that they can optionally implement called
processIngestData
.This method serves a few purposes:
IngestRundown
. It can split/merge/invent Parts or Segments, or copy data between them without worrying about impacting ingest performance. Changes are tracked to minimise what needs to be run throughgetSegment
.As part of this, it is now the responsibility of this method to ensure that ingest changes are pulled into the
IngestRundown
correctly. There is a methodcontext.defaultApplyIngestChanges
which can do this for you, or you can customise the behaviour by doing something different.IngestRundown
. How this gets done and persisted is entirely up to the blueprint. It needs to figure that out, and figure out persistence when other ingest changes are made (if persistence beyond that is even wanted).As a side effect of this, the grouping of MOS stories into segments has been rewritten to be another method that can be used inside of
processIngestData
. If implementing the method, the mos grouping will have to be applied manually when desired.If
processIngestData
is not implemented, core uses the default implementation of, which will match previous behaviour:The user operations are triggered by a new userAction
executeUserChangeOperation
. Where this is used and the exact payload is not very well defined yet, that will be done as part of figuring out the ui design.Testing Instructions
Time Frame
Other Information
This should be squashed when merging.
When contributing upstream, it will need to be broken up into multiple smaller PRs so that each chunk can be reviewed and merged independently. I have a rough plan on how it can be broken up for that
There is an example crude simple ui prototyped at feat/blueprint-processIngestData...bbc:sofie-core:wip/user-overrides-ui
This adds some context menu options for parts and segments:
And a form driven one:
Status