Skip to content

Edit request refactor#1380

Draft
lydiaralphgov wants to merge 18 commits intomasterfrom
S28-4780/edit-request-schema
Draft

Edit request refactor#1380
lydiaralphgov wants to merge 18 commits intomasterfrom
S28-4780/edit-request-schema

Conversation

@lydiaralphgov
Copy link
Copy Markdown
Contributor

@lydiaralphgov lydiaralphgov commented Mar 5, 2026

Change description

Big refactor of edit request code.

  • Split EditRequestService into several helper service classes, under services.edit package
  • Store edit request cut instructions in a proper database table, instead of as raw JSON
  • Don't store ffmpeg instructions in database at all. Convert to LocalTime at point of presentation, and to ffmpeg instructions at point of edit.
  • Consolidate edit request email templates
  • Add new edit request statuses ORIGINAL (version 1) and OUTDATED (for interim edits)
  • Separate out deprecated edit requests from CSV files

Still to do

  • Database logic for updating edit cut instructions
  • Fix remaining unit tests
  • Update and fix integration tests
  • Update and fix functional tests
  • Figure out recording <--> edit request relationship
  • Check test coverage
  • Check pmd report
  • Check checkstyle linter

"type": "string"
},
"edit_instructions": {
"type": "array",
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.

I don't think this needs to be an Array? Object should be fine unless we're going to have multiple instructions (That aren't already being handled by the ffmpeg/requested arrays)

Without Array:
edit_request: { edit_instructions: { ffmpeg_instructions: [], requested_instructions:
[] } }

With Array:
edit_request: { edit_instructions: [ { ffmpeg_instructions: [], requested_instructions: [] } ] }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've been trying to work that out. I'm sure that we need an array, but I'm not sure at which level.

object of arrays: edit_request: { edit_instructions: { ffmpeg_instructions: [], requested_instructions: [] } }

or array of objects: edit_request: { edit_instructions: [ { ffmpeg_instructions: {}, requested_instructions: {} ] }

It depends on how ffmpeg works internally. I think I'll need to partially implement it and then run it against tests. It should be easy enough to change from one to the other if it doesn't work first time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've been thinking some more

I think we should scrap ffmpeg_instructions altogether

IMO the JSON should only have the human-readable versions of the edit instruction, i.e.

    "edit_instructions": [
      {
        "reason": "First edit",
        "start_of_cut": "00:01:00",
        "end_of_cut": "00:02:00"
      }
    ]

Then we should translate that in the backend to ffmpeg instructions, but the ffmpeg instructions don't ever need to surface in the JSON

I'll push up a change to illustrate

@lydiaralphgov lydiaralphgov changed the title Proposed JSON schema and samples for edit requests Edit request refactor Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants