fix(AVO-4098): show subtitles after upload, align sub flow with original#35
Open
bertyhell wants to merge 2 commits into
Open
fix(AVO-4098): show subtitles after upload, align sub flow with original#35bertyhell wants to merge 2 commits into
bertyhell wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reworks the subtitle upload/playback flow to better match the pre-upgrade behavior, aiming to ensure uploaded subtitles are available as VTT via /item_subtitles/... and are loaded into Flowplayer tracks.
Changes:
- Switch subtitle upload from MediaHaven API upload logic to FTP-based upload via
FtpUploader. - Serve VTT subtitles with an explicit
text/vttcontent type and update Flowplayer setup to load available subtitle track types. - Add a Docker named volume for
subtitle_uploadsand adjust test skip reasons for VCR/live MediaHaven dependency.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_app.py |
Updates skip reasons for tests that require live MediaHaven/VCR recording. |
docker-compose.yml |
Adds a named volume for subtitle uploads (currently mounted to the wrong service; see comments). |
docker-compose.develop.yml |
Mounts the subtitle uploads named volume in development. |
app/templates/metadata/edit.html |
Builds Flowplayer subtitle tracks dynamically from available subtitle types. |
app/services/mediahaven_api.py |
Removes API-based subtitle upload/delete methods in favor of FTP flow. |
app/redactietool.py |
Sets VTT response content type; switches subtitle upload path to FtpUploader; removes /delete_subtitle route. |
.gitignore |
Adds app/subtitle_uploads/ (and also AGENTS.md, which likely isn’t intended). |
Comments suppressed due to low confidence (1)
app/redactietool.py:423
- The
/delete_subtitleendpoint was removed, but the UI still calls it when the user deletes an existing subtitle (seeapp/templates/metadata/sections/subtitle_section.html,fetch('/delete_subtitle', ...)). This will now 404 and leave the UI in a broken state. Either restore/delete_subtitle(and the underlying MediaHaven delete call) or update the frontend to remove/disable the delete action.
@app.route('/subtitle_files', methods=['GET'])
@login_required
def get_subtitle_files():
pid = request.args.get('pid')
department = request.args.get('department')
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://meemoo.atlassian.net/browse/AVO-4098
Rework some subtitle upload code to align more with how the flow was before the upgrades/speechmatics track, since subtitles worked there, but not in the latest version
some code in mediahaven_api.py has been removed, to use the original code in ftp_uploader.py
after uploading, ensure the route:
http://localhost:8080/item_subtitles/testbeeld/qszg6jq77n/closed
correctly serves a vtt version of the subtitles (content type text/vtt)
then load the subtitles into the flowplayer tracks