From f36bc217ec7a4ce6d472aa4e7d3b168d62bcd0dd Mon Sep 17 00:00:00 2001 From: Bert Verhelst Date: Fri, 26 Jun 2026 16:36:05 +0200 Subject: [PATCH 1/2] fix(AVO-4098): show subtitles after upload, align sub flow with original https://meemoo.atlassian.net/browse/AVO-4098 --- .gitignore | 4 +- app/redactietool.py | 91 ++++++++------------------------ app/services/mediahaven_api.py | 50 ------------------ app/templates/metadata/edit.html | 27 +++++----- docker-compose.develop.yml | 2 + docker-compose.yml | 2 + tests/test_app.py | 8 +-- 7 files changed, 45 insertions(+), 139 deletions(-) diff --git a/.gitignore b/.gitignore index cae7d0fe..80559e2f 100644 --- a/.gitignore +++ b/.gitignore @@ -12,5 +12,5 @@ htmlcov /frontend/redactietool_widgets/node_modules /.idea /.github - -app/subtitle_uploads/ +app/subtitle_uploads/ +AGENTS.md diff --git a/app/redactietool.py b/app/redactietool.py index 9568616b..7a7eecb5 100644 --- a/app/redactietool.py +++ b/app/redactietool.py @@ -34,6 +34,7 @@ from app.debug_login import legacy_login, legacy_login_submit from app.saml import saml_login from app.services.elastic_api import ElasticApi +from app.services.ftp_uploader import FtpUploader from app.services.mediahaven_api import MediahavenApi from app.services.meta_mapping import MetaMapping from app.services.subtitle_files import (delete_files, get_vtt_subtitles, @@ -210,7 +211,7 @@ def get_subtitle_by_type(department, pid, subtype): srt_url = f"{object_store_url}/{org_name}/{object_id}/{object_id}.srt" print("SRT LINK:", srt_url) - response = Response(get_vtt_subtitles(srt_url)) + response = Response(get_vtt_subtitles(srt_url), content_type='text/vtt; charset=utf-8') response.cache_control.max_age = 0 response.headers.add('Last-Modified', datetime.datetime.now()) response.headers.add( @@ -295,22 +296,6 @@ def edit_metadata(): ) -def _subtitle_upload_error(errors): - """Return a human-readable subtitle upload error message. - - Detects EDUPLICATE responses and surfaces the conflicting record ID. - """ - error_str = errors[0] if errors else '' - if 'EDUPLICATE' in error_str: - try: - error_data = json.loads(error_str) - record_ids = error_data.get('ExistingRecordIds', []) - record_id = record_ids[0] if record_ids else '(onbekend)' - return f'Dit ondertitel bestand is reeds in gebruik voor Record: {record_id}' - except Exception: - pass - return 'Fout bij uploaden ondertitel: ' + error_str - @app.route('/edit_metadata', methods=['POST']) @login_required @@ -345,7 +330,6 @@ def save_item_metadata(): template_vars['mh_errors'] = response['errors'] # Phase 2 — Subtitle handling - replace_subtitle = request.form.get('replace_subtitle') == 'confirm' logger.info( 'subtitle upload check', data={ @@ -353,12 +337,10 @@ def save_item_metadata(): 'mh_synced': template_vars.get('mh_synced'), 'has_subtitle_file': has_subtitle_file, 'subtitle_validation_error': subtitle_validation_error, - 'replace_subtitle': replace_subtitle, } ) if template_vars['mh_synced']: if has_subtitle_file and not subtitle_validation_error: - # Upload subtitle directly via MediaHaven API try: tp = { 'pid': pid, @@ -366,44 +348,26 @@ def save_item_metadata(): 'subtitle_type': subtitle_type, } - # If replacing, delete existing subtitle record from MH first - if replace_subtitle: - existing_sub = mh_api.get_subtitle(department, pid, subtitle_type) - if existing_sub: - sub_fragment_id = existing_sub.get( - 'Internal', {}).get('FragmentId') - if sub_fragment_id: - del_response = mh_api.delete_subtitle( - sub_fragment_id, - reason=f"Replacing subtitle for {pid}" - ) - if not del_response['status']: - template_vars['subtitle_error'] = ( - 'Fout bij verwijderen bestaand ondertitelbestand: ' - + ', '.join(del_response['errors']) - ) - - if not template_vars.get('subtitle_error'): - tp['srt_file'], tp['vtt_file'] = save_subtitles( - upload_folder(), pid, uploaded_file) - if tp['srt_file']: - tp['srt_file'] = move_subtitle(upload_folder(), tp) - tp['xml_file'], xml_sidecar_data = save_sidecar_xml( - upload_folder(), mam_data, tp) - - srt_path = os.path.join(upload_folder(), tp['srt_file']) - api_response = mh_api.upload_subtitle( - srt_path, tp['srt_file'], xml_sidecar_data) - - delete_files(upload_folder(), tp) - if not api_response['status']: - template_vars['subtitle_error'] = _subtitle_upload_error( - api_response['errors']) - else: - template_vars['subtitle_synced'] = True - template_vars['subtitle_synced_filename'] = f"{pid}_{subtitle_type}.srt" + tp['srt_file'], tp['vtt_file'] = save_subtitles( + upload_folder(), pid, uploaded_file) + if tp['srt_file']: + tp['srt_file'] = move_subtitle(upload_folder(), tp) + tp['xml_file'], tp['xml_sidecar'] = save_sidecar_xml( + upload_folder(), mam_data, tp) + + ftp_uploader = FtpUploader() + ftp_response = ftp_uploader.upload_subtitles( + upload_folder(), mam_data, tp) + + delete_files(upload_folder(), tp) + if 'ftp_error' in ftp_response: + template_vars['subtitle_error'] = ( + 'FTP upload fout: ' + ftp_response['ftp_error']) else: - template_vars['subtitle_error'] = 'Ondertitels moeten in SRT formaat' + template_vars['subtitle_synced'] = True + template_vars['subtitle_synced_filename'] = tp['srt_file'] + else: + template_vars['subtitle_error'] = 'Ondertitels moeten in SRT formaat' except Exception as e: logger.exception('subtitle upload failed', data={'pid': pid, 'error': str(e)}) template_vars['subtitle_error'] = str(e) @@ -451,19 +415,6 @@ def save_item_metadata(): return redirect(url_for('edit_metadata', pid=pid, department=department)) -@app.route('/delete_subtitle', methods=['POST']) -@login_required -def delete_subtitle(): - data = request.get_json() - fragment_id = data.get('fragment_id') - pid = data.get('pid') - - mh_api = MediahavenApi() - result = mh_api.delete_subtitle(fragment_id, reason=f"Deleted by editor for {pid}") - - return jsonify(result) - - @app.route('/subtitle_files', methods=['GET']) @login_required def get_subtitle_files(): diff --git a/app/services/mediahaven_api.py b/app/services/mediahaven_api.py index f7d1b160..e6f819b4 100644 --- a/app/services/mediahaven_api.py +++ b/app/services/mediahaven_api.py @@ -116,56 +116,6 @@ def get_subtitle(self, department, pid, subtype): else: return False - def delete_subtitle(self, record_id, reason=None): - """Delete a subtitle record from MediaHaven.""" - try: - logger.info( - 'deleting existing subtitle record', - data={'record_id': record_id, 'reason': reason} - ) - self.client.records.delete(record_id, reason=reason) - return {'status': True, 'errors': []} - except MediaHavenException as me: - logger.error( - 'failed to delete subtitle record', - data={'record_id': record_id, 'error': str(me)} - ) - return {'status': False, 'errors': [str(me)]} - - def upload_subtitle(self, srt_path, srt_filename, xml_sidecar): - """Upload a subtitle file directly to MediaHaven via the API. - - Posts the SRT file with the XML sidecar metadata to create - a new record in MediaHaven. - """ - try: - logger.info( - 'uploading subtitle via API', - data={'srt_filename': srt_filename} - ) - with open(srt_path, 'rb') as srt_file: - files = { - "file": (srt_filename, srt_file, "application/x-subrip"), - "metadata": ("metadata", xml_sidecar, "application/xml"), - } - response = self.client.records.mh_client._post( - self.client.records._construct_path(), - files=files, - autoPublish="true", - ignoreDuplicates="true", - ) - logger.info( - 'subtitle API upload response', - data={'srt_filename': srt_filename, 'response': str(response)} - ) - return {'status': True, 'response': response, 'errors': []} - except MediaHavenException as me: - logger.error( - 'subtitle API upload failed', - data={'srt_filename': srt_filename, 'error': str(me)} - ) - return {'status': False, 'errors': [str(me)]} - def update_metadata(self, department, fragment_id, external_id, xml_sidecar): try: logger.info("syncing metadata to mediahaven...") diff --git a/app/templates/metadata/edit.html b/app/templates/metadata/edit.html index c75a1f20..b237cfd4 100644 --- a/app/templates/metadata/edit.html +++ b/app/templates/metadata/edit.html @@ -57,22 +57,23 @@

PID: {{pid}}

diff --git a/docker-compose.develop.yml b/docker-compose.develop.yml index 0265f414..a8f5a50a 100644 --- a/docker-compose.develop.yml +++ b/docker-compose.develop.yml @@ -14,6 +14,7 @@ services: - ./app:/app/app - ./wsgi.py:/app/wsgi.py - ./debug.py:/app/debug.py + - subtitle_uploads:/app/app/subtitle_uploads environment: - FLASK_ENV=DEVELOPMENT entrypoint: ["python", "debug.py"] @@ -41,6 +42,7 @@ services: - app-network volumes: + subtitle_uploads: postgres-data: networks: diff --git a/docker-compose.yml b/docker-compose.yml index 615cab3b..8eff6f8a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -20,6 +20,7 @@ services: volumes: - postgres-data:/var/lib/postgresql/data - ./db/init:/docker-entrypoint-initdb.d + - subtitle_uploads:/app/app/subtitle_uploads healthcheck: test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER} -d ${POSTGRES_DB}"] interval: 5s @@ -29,6 +30,7 @@ services: - app-network volumes: + subtitle_uploads: postgres-data: networks: diff --git a/tests/test_app.py b/tests/test_app.py index 6fa77ce1..9f90dd01 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -448,10 +448,10 @@ def test_random_404(client, setup): # =================== Combined metadata + subtitle tests ===================== -@pytest.mark.skip(reason="requires mediahaven OAuth2 token") +@pytest.mark.skip(reason="requires VCR cassette recording against a live MediaHaven server") @pytest.mark.vcr def test_edit_metadata_with_subtitle_published(auth_client, mocker): - """POST with a valid SRT file and publicatiestatus_checked=true should FTP-upload.""" + """POST with a valid SRT file should save metadata then FTP-upload the subtitle.""" with open('./tests/fixture_data/edit_mam_data.json', "r") as f: mam_data = json.loads(f.read()) @@ -504,7 +504,7 @@ def mock_ftp_client(self, server): assert ftp_mock.login.called -@pytest.mark.skip(reason="requires mediahaven OAuth2 token") +@pytest.mark.skip(reason="requires VCR cassette recording against a live MediaHaven server") @pytest.mark.vcr def test_edit_metadata_without_subtitle(auth_client, mocker): """POST without subtitle file should only save metadata.""" @@ -544,7 +544,7 @@ def test_edit_metadata_without_subtitle(auth_client, mocker): assert 'automatisch opgeladen' not in body -@pytest.mark.skip(reason="requires mediahaven OAuth2 token") +@pytest.mark.skip(reason="requires VCR cassette recording against a live MediaHaven server") @pytest.mark.vcr def test_edit_metadata_with_invalid_srt(auth_client, mocker): """POST with a non-SRT file should show a subtitle validation error.""" From 2e579c5792cd2447bfadddb4533708f20bf30c80 Mon Sep 17 00:00:00 2001 From: Bert Verhelst Date: Fri, 26 Jun 2026 16:53:32 +0200 Subject: [PATCH 2/2] fix(AVO-4098): move subtitles volume to app container https://meemoo.atlassian.net/browse/AVO-4098 --- docker-compose.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 8eff6f8a..105da212 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -8,6 +8,8 @@ services: depends_on: db: condition: service_healthy + volumes: + - subtitle_uploads:/app/app/subtitle_uploads networks: - app-network @@ -20,7 +22,6 @@ services: volumes: - postgres-data:/var/lib/postgresql/data - ./db/init:/docker-entrypoint-initdb.d - - subtitle_uploads:/app/app/subtitle_uploads healthcheck: test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER} -d ${POSTGRES_DB}"] interval: 5s