Skip to content


Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP


2163: don't create a new subtitle version when tasks are approved #140

merged 1 commit into from

3 participants


This change replaces the old code that always created a new version with new code that simple changes the visibility of the current version to public.

One difference is that before we would be able to set the author of the version to the reviewer/approver, rather than the original owner. However, if the reviewer/approver makes any changes then we create a new version anyways. So this only matters in the case where no changes are made. Still I'm not 100% sure that this is correct.


This seems okay aside from the issue you mentioned. Let's hope we don't get a ticket down the road that says "show who approved this version".


Oh , just remember, this will be a problem. T uses the reviewer's name, we can't break that, see

In our dev branch, that was removed (but it's definitely wrong)


We still call set_approved_by() and set_reviewed_by(), we simply call it on the existing SubtitleVersion object rather than a copy of it. Seems like we're not losing any data in this case.


I checked via the django shell and get_approved_by() and get_moderated_by() return the user who approved/moderated the subtitles, so I think this is good.

Arthur suggested to look at /api2/partners/videos/[video-id]/languages/[language-code]/?format=json, but it doesn't seem to me that that outputs the approved by data. Maybe it was removed in the dev branch? In any case, I think we can merge.

@bendk bendk merged commit ddaf3f6 into from
@bendk bendk deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 13 additions and 22 deletions.
  1. +9 −0 apps/subtitles/
  2. +4 −22 apps/teams/
9 apps/subtitles/
@@ -1377,6 +1377,15 @@ def has_subtitles(self):
def is_synced(self):
return self.get_subtitles().fully_synced
+ def publish(self):
+ """Make this version publicly viewable."""
+ team_video =
+ assert team_video, \
+ "Cannot unpublish for a video not moderated by a team."
+ self.visibility = 'public'
def unpublish(self, delete=False):
"""Unpublish this version.
26 apps/teams/
@@ -67,24 +67,6 @@
ALL_LANGUAGES = [(val, _(name))for val, name in settings.ALL_LANGUAGES]
VALID_LANGUAGE_CODES = [unicode(x[0]) for x in ALL_LANGUAGES]
-def publicize_version(subtitle_version, author):
- """Create a new SubtitleVersion that's a public copy of the given version.
- author should be the person making this happen, *not* the author of the
- original version.
- """
- pipeline.add_subtitles(
- language_code=subtitle_version.language_code,
- subtitles=subtitle_version.get_subtitles(),
- title=subtitle_version.title,
- description=subtitle_version.description,
- author=author,
- visibility='public',
- )
# Teams
class TeamManager(models.Manager):
def get_query_set(self):
@@ -1868,7 +1850,7 @@ def _complete_subtitle(self):
# Subtitle task is done, and there is no approval or review
# required, so we mark the version as approved.
- publicize_version(sv, self.assignee)
+ sv.publish()
# We need to make sure this is updated correctly here.
from apps.videos import metadata_manager
@@ -1905,7 +1887,7 @@ def _complete_translate(self):
- publicize_version(sv, self.assignee)
+ sv.publish()
# We need to make sure this is updated correctly here.
from apps.videos import metadata_manager
@@ -1947,7 +1929,7 @@ def _complete_review(self):
# determines whether the subtitles go public.
if approval:
# Make these subtitles public!
- publicize_version(self.new_subtitle_version, self.assignee)
+ self.new_subtitle_version.publish()
# If the subtitles are okay, go ahead and autocreate translation
# tasks if necessary.
@@ -1977,7 +1959,7 @@ def _complete_approve(self):
if approval:
# The subtitles are acceptable, so make them public!
- publicize_version(self.new_subtitle_version, self.assignee)
+ self.new_subtitle_version.publish()
# Create translation tasks if necessary.
if self.workflow.autocreate_translate:
Something went wrong with that request. Please try again.