Skip to content

Commit

Permalink
do not publish "unready" JWP videos
Browse files Browse the repository at this point in the history
As noted in uisautomation#294, there needs to be some better tracking of state for
videos. After re-reading uisautomation#294, I don't think we need to track state in
the MediaItem model *just yet* since we have it available on the JWP
CachedResource object.

This commit fixes an immediate pain point in that videos will be
published even if they have not yet been transcoded. We update the
publication check to *NOT* publish if:

1. The publication date is in the future.
2. There is a JWP video associated with the item and the video's status
   is not "ready".

We no longer need the NULL published_at check because or database
disallows NULL publication dates (introduced in commit 809931c).
  • Loading branch information
rjw57 committed Sep 18, 2018
1 parent 4820667 commit 2c4964e
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 5 deletions.
7 changes: 7 additions & 0 deletions api/tests/fixtures/mediaitems.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
- ccc
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: pa
Expand All @@ -72,6 +73,7 @@
updated_at: 2010-09-15 14:40:45
data:
keu: vid2key
status: ready

- model: mediaplatform_jwp.Video
pk: jwpvid2
Expand All @@ -86,6 +88,7 @@
channel: channel1
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: pemptyv
Expand All @@ -100,6 +103,7 @@
deleted_at: 2011-09-15 12:00:00
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: pdeleted
Expand All @@ -124,6 +128,7 @@
- tag2
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: populatedv
Expand All @@ -138,6 +143,7 @@
updated_at: 2010-09-15 14:40:45
data:
keu: vid1key
status: ready

- model: mediaplatform_jwp.Video
pk: jwpvid1
Expand Down Expand Up @@ -165,6 +171,7 @@
channel: channel1
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: puseronly
Expand Down
2 changes: 2 additions & 0 deletions legacysms/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,13 @@ def test_no_media(self):
'custom': {'sms_media_id': 'media:34:', 'sms_acl': 'acl:WORLD:'},
'mediatype': 'audio',
'key': 'myaudiokey',
'status': 'ready',
},
{
'custom': {'sms_media_id': 'media:34:', 'sms_acl': 'acl:WORLD:'},
'mediatype': 'video',
'key': 'myvideokey',
'status': 'ready',
},
]

Expand Down
22 changes: 17 additions & 5 deletions mediaplatform/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,19 @@ def _permission_condition(self, fieldname, user):

class MediaItemQuerySet(PermissionQuerySetMixin, models.QuerySet):

def _published_condition(self):
# An item is *NOT* published if any of the following are true:
#
# 1. It has a publication time is in the future.
# 2. It has an associated JWP video and that video's status is not "ready".
#
# We negate the OR of these checks to determined if a video is published.
return ~(
models.Q(published_at__gt=timezone.now()) |
(models.Q(jwp__isnull=False) & ~models.Q(jwp__resource__data__status='ready'))
)
return models.Q(published_at__isnull=True) | models.Q(published_at__lt=timezone.now())

def _viewable_condition(self, user):
# The item can be viewed if any of the following are satisfied:
#
Expand All @@ -92,12 +105,11 @@ def _viewable_condition(self, user):
if user is not None and user.has_perm('mediaplatform.view_mediaitem'):
return models.Q(id=models.F('id'))

# An item is "published" if it either has no publication time or the publication time is in
# the past.
published = models.Q(published_at__isnull=True) | models.Q(published_at__lt=timezone.now())

return (
(self._permission_condition('view_permission', user) & published) |
(
self._permission_condition('view_permission', user) &
self._published_condition()
) |
self._editable_condition(user)
)

Expand Down
7 changes: 7 additions & 0 deletions mediaplatform/tests/fixtures/test_data.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
channel: channel1
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: pemptyv
Expand All @@ -52,6 +53,7 @@
deleted_at: 2011-09-15 12:00:00
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: pdeletedv
Expand All @@ -67,6 +69,7 @@
deleted_at: 2011-09-15 12:00:00
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

# A media item with "is signed in" permission
- model: mediaplatform.MediaItem
Expand All @@ -75,6 +78,7 @@
channel: channel1
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: psignedinv
Expand All @@ -89,6 +93,7 @@
channel: channel1
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: ppublicv
Expand All @@ -103,6 +108,7 @@
updated_at: 2010-09-15 14:40:45
data:
keu: publicvid
status: ready

- model: mediaplatform_jwp.Video
pk: jwpvidpublic
Expand All @@ -118,6 +124,7 @@
channel: channel1
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: pemptypermv
Expand Down
51 changes: 51 additions & 0 deletions mediaplatform/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.core.cache import cache
from django.db import IntegrityError
from django.test import TestCase, override_settings
from django.utils import timezone

from legacysms import models as legacymodels
from mediaplatform_jwp.models import CachedResource
Expand Down Expand Up @@ -350,6 +351,56 @@ def test_super_downloaded(self):
# check that the user can now view the item
self.assert_user_can_download(new_user, item)

def test_published_at_in_future_not_published(self):
"""An item with publication date in the future is not published."""
item = models.MediaItem.objects.get(id='public')
self.assert_user_can_view(self.user, item)
item.published_at = timezone.now() + datetime.timedelta(days=1)
item.save()
self.assert_user_cannot_view(self.user, item)

def test_no_jwp_video_required_for_publication(self):
"""An item does not need a JWP video to be published."""
item = models.MediaItem.objects.get(id='public')
self.assert_user_can_view(self.user, item)
item.jwp.delete()
self.assert_user_can_view(self.user, item)

def test_publications_requires_ready_jwp_video(self):
"""An item with a JWP video must have that video be "ready" to be published."""
item = models.MediaItem.objects.get(id='public')
self.assert_user_can_view(self.user, item)
item.jwp.resource.data['status'] = 'error'
item.jwp.resource.save()
self.assert_user_cannot_view(self.user, item)

def test_future_published_items_visible_to_editor(self):
"""An unpublished item can still be seen by someone who can edit it."""
item = models.MediaItem.objects.get(id='public')
item.channel.edit_permission.reset()
item.channel.edit_permission.crsids.append(self.user.username)
item.channel.edit_permission.save()
self.assert_user_can_view(None, item)
self.assert_user_can_view(self.user, item)
item.published_at = timezone.now() + datetime.timedelta(days=1)
item.save()
self.assert_user_cannot_view(None, item)
self.assert_user_can_view(self.user, item)

def test_items_with_errors_visible_to_editor(self):
"""An item with JWP error can still be seen by someone who can edit it."""
item = models.MediaItem.objects.get(id='public')
item.channel.edit_permission.reset()
item.channel.edit_permission.crsids.append(self.user.username)
item.channel.edit_permission.save()
self.assert_user_can_view(None, item)
self.assert_user_can_view(self.user, item)
item.jwp.resource.data['status'] = 'error'
item.jwp.resource.save()
item.save()
self.assert_user_cannot_view(None, item)
self.assert_user_can_view(self.user, item)

def assert_user_cannot_view(self, user, item_or_id):
if isinstance(item_or_id, str):
item_or_id = models.MediaItem.objects_including_deleted.get(id=item_or_id)
Expand Down
3 changes: 3 additions & 0 deletions mediaplatform_jwp/tests/fixtures/mediaitems.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
fields:
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: emptyv
Expand All @@ -16,6 +17,7 @@
fields:
created_at: 2010-09-15 14:40:45
updated_at: 2010-09-15 14:40:45
published_at: 2010-09-15 14:40:45

- model: mediaplatform.Permission
pk: existingv
Expand All @@ -30,6 +32,7 @@
updated_at: 2010-09-15 14:40:45
data:
keu: video1key
status: ready

- model: mediaplatform_jwp.Video
pk: video1
Expand Down

0 comments on commit 2c4964e

Please sign in to comment.