chore: update 3play sync for multi language#3029
chore: update 3play sync for multi language#3029umar8hassan wants to merge 1 commit intoumar/10983-ingest-drive-files-for-multi-languagefrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for multi-language video captions and transcripts by transitioning metadata from single URL strings to arrays of objects. Key updates include modifying the site configuration to use multi-select relation widgets, adding filename parsing for language detection, and providing data migrations for existing records. Feedback was provided regarding code duplication in the task logic for building file data, with a suggestion to extract a shared helper function to improve maintainability.
| captions_file_data = [] | ||
| for r in captions_list: | ||
| if not (getattr(r, "file", None) and r.file.name): | ||
| continue | ||
| lang, locale = parse_caption_language_locale(r.filename or "") | ||
| entry: dict = { | ||
| "file": urljoin( | ||
| "/", | ||
| r.file.name.replace(website.s3_path, website.url_path), | ||
| ), | ||
| "language": lang, | ||
| } | ||
| if locale: | ||
| entry["locale"] = locale | ||
| captions_file_data.append(entry) |
There was a problem hiding this comment.
This logic for building file data from a list of resources is duplicated for captions, transcripts, and in update_transcripts_for_video. To improve maintainability and reduce code duplication, please extract this into a helper function in videos/utils.py.
umar8hassan
force-pushed
the
umar/10984-update-3play-sync-for-multi-language
branch
from
d53e07c to
061f4c1
Compare
| ("lecture1_captions_en_us_vtt", ("en", "US")), | ||
| ("lecture1_captions_fr_en_vtt", ("fr", "EN")), | ||
| ("lecture1_transcript_en_us_pdf", ("en", "US")), | ||
| ("lecture1_transcript_fr_gb_pdf", ("fr", "GB")), |
There was a problem hiding this comment.
It would be better to pick a realistic locale for the test, e.g., FR, CH, etc.
umar8hassan
changed the base branch from
master
to
umar/10983-ingest-drive-files-for-multi-language
- _attach_transcript_if_missing and _attach_captions_if_missing now guard
with .get('video_transcript_resource', {}).get('content') — falsy for
empty-string or empty-list content (site-config relation-widget default).
Previously the guard checked the old _file field, so new videos whose
_resource was initialised to {"content": ""} would be silently skipped.
- _create_new_content now returns (s3_path, text_id) tuple instead of just
the path string.
- On successful 3Play fetch, writes:
video_transcript_resource: {"content": [text_id], "website": name}
video_transcript_file: [{"file": path, "language": "en"}]
(and the captions pair equivalently).
- Tests:
- test_sync_video_captions_and_transcripts_sets_references: asserts
_resource.content is a single-item list.
- test_sync_video_captions_and_transcripts_skips_if_resource_exists: non-empty
content list prevents re-fetch.
- test_sync_video_captions_and_transcripts_treats_empty_content_as_unset:
empty-string content triggers fetch (guard is falsy-based).
umar8hassan
force-pushed
the
umar/10984-update-3play-sync-for-multi-language
branch
from
061f4c1 to
c8fae1e
Compare
2 participants
What are the relevant tickets?
https://github.com/mitodl/hq/issues/10984
Description (What does it do?)
Fixes the 3Play sync path to write captions and transcripts using the post-10982 array format and to guard correctly against existing content.
How can this be tested?
E8uZtq_vOYMand it'd pull the related transcript and captions.video_captions_resourcenow shows a single English caption resource andvideo_transcript_resourceshows a transcript.