Skip to content

feat: return 200 when a transcript upload replaces an existing language #39038

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.

Sign up for GitHub

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

Open
abhalsod-sonata wants to merge 1 commit into
base: master
Choose a base branch
from
+29 −1
Open

feat: return 200 when a transcript upload replaces an existing language #39038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion cms/djangoapps/contentstore/transcript_storage_handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ def upload_transcript(request):
new_language_code = request.POST['new_language_code']
transcript_file = request.FILES['file']
try:
# Determine whether this upload replaces an existing transcript
# (return 200) or creates a new one (return 201).
is_replace = new_language_code in get_available_transcript_languages(video_id=edx_video_id)
# Convert SRT transcript into an SJSON format
# and upload it to S3.
sjson_subs = Transcript.convert(
Expand All @@ -201,7 +204,7 @@ def upload_transcript(request):
},
file_data=ContentFile(sjson_subs),
)
response = JsonResponse(status=201)
response = JsonResponse(status=200 if is_replace else 201)
except (TranscriptsGenerationException, UnicodeDecodeError):
LOGGER.error("Unable to update transcript on edX video %s for language %s", edx_video_id, new_language_code)
response = JsonResponse(
Expand Down
25 changes: 25 additions & 0 deletions cms/djangoapps/contentstore/views/tests/test_transcript_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,31 @@ def test_transcript_upload_handler(self, mock_create_or_update_video_transcript)
file_data=ANY,
)

@patch('cms.djangoapps.contentstore.transcript_storage_handlers.create_or_update_video_transcript')
@patch(
'cms.djangoapps.contentstore.transcript_storage_handlers.get_available_transcript_languages',
Mock(return_value=['en']),
)
def test_transcript_upload_handler_returns_200_on_replace(self, mock_create_or_update_video_transcript):

@robrap robrap Sep 1, 2026 •
edited
Loading

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 would have expected to see these minor changes on our fork, but I don't see this function name. Is there a reason why the upstream contribution has a more complicated diff, or simply because we didn't first get our fork where we ultimately want it before contributing a change upstream?

Note: If this method must be different from our fork (other than the fact that we haven't removed the toggle from our fork yet), feel free to explain. Thank you.

@abhalsod-sonata abhalsod-sonata Sep 2, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The handler change is identical to the fork's (edx/edx-platform#267) - two things differ. The toggle and its serializer/tests are left out per the product decision to ship without a flag. This test is new because #267 only tested the toggle (in test_course_waffle_flags.py), so with the toggle dropped the 200-vs-201 change had no coverage, and upstream needs a test for a response-code change. I'll add this same test to the fork along with the toggle removal so the two stay in sync.

"""
Verify that uploading a transcript for a language that already has a
transcript returns 200 (replace) instead of 201 (create).
"""
transcript_file_stream = StringIO('0\n00:00:00,010 --> 00:00:00,100\nHello, edX greets you.\n\n')
response = self.client.post(
self.view_url,
{
'edx_video_id': '123',
'language_code': 'en',
'new_language_code': 'en',
'file': transcript_file_stream,
},
format='multipart'
)

self.assertEqual(response.status_code, 200) # noqa: PT009
mock_create_or_update_video_transcript.assert_called_once()

@ddt.data(
(
{
Expand Down
Loading