[592674] Media gallery uploads backend

* Raise FixtureMissingError in test helpers, e.g. on creating media gallery image and wiki revision, instead of creating testuser
* Add ImageUploadForm and VideoUploadForm
* Simplify gallery app's urls.py
* Adds a bunch of gallery utils to create and upload media
* Adds two views up_media_async, del_media_async
* Rename upload_images to upload_imageattachments
* Define a more generic upload_media function to be used by the gallery app
* Document some of our coding conventions
* Add a migration for the video model
* Define a MAX_FILESIZE for video uploads (16 megabytes), overwritable in settings_local.py

apps/gallery/forms.py

@@ -0,0 +1,58 @@
+from django import forms
+
+from tower import ugettext_lazy as _lazy
+
+from gallery.models import Image, Video
+from sumo.form_fields import StrippedCharField
+
+# Error messages
+MSG_TITLE_REQUIRED = _lazy(u'Please provide a title.')
+MSG_TITLE_SHORT = _lazy(
+    'The title is too short (%(show_value)s characters). It must be at '
+    'least %(limit_value)s characters.')
+MSG_TITLE_LONG = _lazy(
+    'Please keep the length of your title to %(limit_value)s characters '
+    'or less. It is currently %(show_value)s characters.')
+MSG_DESCRIPTION_REQUIRED = _lazy(u'Please provide a description.')
+MSG_DESCRIPTION_LONG = _lazy(
+    'Please keep the length of your description to %(limit_value)s '
+    'characters or less. It is currently %(show_value)s characters.')
+MSG_IMAGE_REQUIRED = _lazy(u'You have not selected an image to upload.')
+
+
+class ImageUploadForm(forms.ModelForm):
+    """Image upload form."""
+    file = forms.ImageField(error_messages={'required': MSG_IMAGE_REQUIRED})
+    title = StrippedCharField(
+        min_length=5, max_length=255,
+        error_messages={'required': MSG_TITLE_REQUIRED,
+                        'min_length': MSG_TITLE_SHORT,
+                        'max_length': MSG_TITLE_LONG})
+    description = StrippedCharField(
+        max_length=10000, widget=forms.Textarea(),
+        error_messages={'required': MSG_DESCRIPTION_REQUIRED,
+                        'max_length': MSG_DESCRIPTION_LONG})
+
+    class Meta:
+        model = Image
+        fields = ('file', 'title', 'description')
+
+
+class VideoUploadForm(forms.ModelForm):
+    """Video upload form."""
+    webm = forms.FileField(required=False)
+    ogv = forms.FileField(required=False)
+    flv = forms.FileField(required=False)
+    title = StrippedCharField(
+        min_length=5, max_length=255,
+        error_messages={'required': MSG_TITLE_REQUIRED,
+                        'min_length': MSG_TITLE_SHORT,
+                        'max_length': MSG_TITLE_LONG})
+    description = StrippedCharField(
+        max_length=10000, widget=forms.Textarea(),
+        error_messages={'required': MSG_DESCRIPTION_REQUIRED,
+                        'max_length': MSG_DESCRIPTION_LONG})
+
+    class Meta:
+        model = Video
+        fields = ('webm', 'ogv', 'flv', 'title', 'description')

apps/gallery/models.py

@@ -1,9 +1,12 @@
 from datetime import datetime
 
+from django.core.exceptions import ValidationError
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.db import models
 
+from tower import ugettext_lazy as _lazy
+
 from sumo.models import ModelBase
 from sumo.urlresolvers import reverse
 
@@ -25,10 +28,6 @@ class Media(ModelBase):
     def __unicode__(self):
         return self.title + ': ' + self.file.name[30:]
 
-    def thumbnail_or_file(self):
-        """Returns self.thumbnail, if set, else self.file"""
-        return self.thumbnail if self.thumbnail else self.file
-
 
 class Image(Media):
     creator = models.ForeignKey(User, related_name='gallery_images')
@@ -39,12 +38,31 @@ class Image(Media):
     def get_absolute_url(self):
         return reverse('gallery.media', args=['image', self.id])
 
+    def thumbnail_url_if_set(self):
+        """Returns self.thumbnail, if set, else self.file"""
+        return self.thumbnail.url if self.thumbnail else self.file.url
+
 
 class Video(Media):
     creator = models.ForeignKey(User, related_name='gallery_videos')
-    file = models.FileField(upload_to=settings.GALLERY_VIDEO_PATH)
+    webm = models.FileField(upload_to=settings.GALLERY_VIDEO_PATH, null=True)
+    ogv = models.FileField(upload_to=settings.GALLERY_VIDEO_PATH, null=True)
+    flv = models.FileField(upload_to=settings.GALLERY_VIDEO_PATH, null=True)
     thumbnail = models.ImageField(
         upload_to=settings.GALLERY_VIDEO_THUMBNAIL_PATH, null=True)
 
     def get_absolute_url(self):
         return reverse('gallery.media', args=['video', self.id])
+
+    def thumbnail_url_if_set(self):
+        """Returns self.thumbnail.url, if set, else default thumbnail URL"""
+        return self.thumbnail.url if self.thumbnail \
+                                  else settings.THUMBNAIL_PROGRESS_URL
+
+    def clean(self):
+        """Ensure one of the supported file formats has been uploaded"""
+        if not (self.webm or self.ogv or self.flv):
+            raise ValidationError(
+                _lazy('The video has no files associated with it. You must '
+                      'upload one of the following extensions: webm, ogv, '
+                      'flv.'))

apps/gallery/templates/gallery/gallery.html

@@ -34,12 +34,12 @@
           {% if filter == 'images' %}
             <span>{{ _('Images') }}</span>
           {% else %}
-            <a href="{{ url('gallery.gallery_images') }}">{{ _('Images') }}</a>
+            <a href="{{ url('gallery.gallery_media', 'image') }}">{{ _('Images') }}</a>
           {% endif %}
           {% if filter == 'videos' %}
             <span>{{ _('Videos') }}</span>
           {% else %}
-            <a href="{{ url('gallery.gallery_videos') }}">{{ _('Videos') }}</a>
+            <a href="{{ url('gallery.gallery_media', 'video') }}">{{ _('Videos') }}</a>
           {% endif %}
         </div>
       </div>
@@ -49,7 +49,7 @@
         {% for m in media.object_list %}
           <li>
             <a href="{{ m.get_absolute_url() }}" title="{{ m.title }}">
-              <img src="{{ m.thumbnail_or_file().url }}" alt="{{ m.title }}" />
+              <img src="{{ m.thumbnail_url_if_set() }}" alt="{{ m.title }}" />
             </a>
           </li>
         {% endfor %}
@@ -61,4 +61,4 @@
       {{ media|paginator }}
     </footer>
   </section>
-{% endblock %}
+{% endblock %}

apps/gallery/tests/__init__.py

@@ -1,18 +1,18 @@
-from django.contrib.auth.models import User
 from django.core.files import File
 
-from gallery.models import Image
+from gallery.models import Image, Video
+from sumo.tests import get_user
 
 
 def image(file_and_save=True, **kwargs):
-    """Return a saved image"""
+    """Return a saved image.
+
+    Requires a users fixture if no creator is provided.
+
+    """
     u = None
     if 'creator' not in kwargs:
-        try:
-            u = User.objects.get(username='testuser')
-        except User.DoesNotExist:
-            u = User(username='testuser', email='me@nobody.test')
-            u.save()
+        u = get_user()
 
     defaults = {'title': 'Some title', 'description': 'Some description',
                 'creator': u}
@@ -28,3 +28,36 @@ def image(file_and_save=True, **kwargs):
             img.file.save(up_file.name, up_file, save=True)
 
     return img
+
+
+def video(file_and_save=True, **kwargs):
+    """Return a saved video.
+
+    Requires a users fixture if no creator is provided.
+
+    """
+    u = None
+    if 'creator' not in kwargs:
+        u = get_user()
+
+    defaults = {'title': 'Some title', 'description': 'Some description',
+                'creator': u}
+    defaults.update(kwargs)
+
+    vid = Video(**defaults)
+    if not file_and_save:
+        return vid
+
+    if 'file' not in kwargs:
+        with open('apps/gallery/tests/media/test.webm') as f:
+            up_file = File(f)
+            vid.webm.save(up_file.name, up_file, save=False)
+        with open('apps/gallery/tests/media/test.ogv') as f:
+            up_file = File(f)
+            vid.ogv.save(up_file.name, up_file, save=False)
+        with open('apps/gallery/tests/media/test.flv') as f:
+            up_file = File(f)
+            vid.flv.save(up_file.name, up_file, save=False)
+        vid.save()
+
+    return vid

apps/gallery/tests/test__utils.py

@@ -0,0 +1,73 @@
+from django.conf import settings
+from django.contrib.auth.models import User
+from django.core.files import File
+
+from gallery.models import Image, Video
+from gallery.utils import create_image, create_video
+from sumo.tests import TestCase
+from sumo.urlresolvers import reverse
+from upload.tests import check_file_info
+
+
+class CreateImageTestCase(TestCase):
+    fixtures = ['users.json']
+
+    def setUp(self):
+        super(CreateImageTestCase, self).setUp()
+        self.user = User.objects.all()[0]
+
+    def tearDown(self):
+        Image.objects.all().delete()
+        super(CreateImageTestCase, self).tearDown()
+
+    def test_create_image(self):
+        """
+        An image is created from an uploaded file.
+
+        Verifies all appropriate fields are correctly set.
+        """
+        with open('apps/upload/tests/media/test.jpg') as f:
+            up_file = File(f)
+            file_info = create_image(
+                {'image': up_file}, self.user, settings.IMAGE_MAX_FILESIZE,
+                'Title', 'Description', 'en-US')
+
+        image = Image.objects.all()[0]
+        delete_url = reverse('gallery.del_media_async',
+                             args=['image', image.id])
+        check_file_info(
+            file_info, name='apps/upload/tests/media/test.jpg',
+            width=90, height=120, delete_url=delete_url,
+            url=image.get_absolute_url(), thumbnail_url=image.thumbnail.url)
+
+
+class CreateVideoTestCase(TestCase):
+    fixtures = ['users.json']
+
+    def setUp(self):
+        super(CreateVideoTestCase, self).setUp()
+        self.user = User.objects.all()[0]
+
+    def tearDown(self):
+        Video.objects.all().delete()
+        super(CreateVideoTestCase, self).tearDown()
+
+    def test_create_video(self):
+        """
+        A video is created from an uploaded file.
+
+        Verifies all appropriate fields are correctly set.
+        """
+        with open('apps/gallery/tests/media/test.flv') as f:
+            up_file = File(f)
+            file_info = create_video({'flv': up_file}, self.user, 'Title',
+                                     'Description', 'en-US', 'flv')
+
+        vid = Video.objects.all()[0]
+        delete_url = reverse('gallery.del_media_async',
+                             args=['video', vid.id])
+        check_file_info(
+            file_info, name='apps/gallery/tests/media/test.flv',
+            width=120, height=120, delete_url=delete_url,
+            url=vid.get_absolute_url(),
+            thumbnail_url=settings.THUMBNAIL_PROGRESS_URL)

apps/gallery/tests/test_models.py

@@ -1,14 +1,16 @@
+from django.conf import settings
+
 from nose.tools import eq_
+from nose import SkipTest
 
 from sumo.tests import TestCase
-from gallery.models import Image
-from gallery.tests import image
+from gallery.models import Image, Video
+from gallery.tests import image, video
+from upload.tasks import generate_image_thumbnail
 
 
 class ImageTestCase(TestCase):
-
-    def setUp(self):
-        super(ImageTestCase, self).setUp()
+    fixtures = ['users.json']
 
     def tearDown(self):
         Image.objects.all().delete()
@@ -18,5 +20,36 @@ class ImageTestCase(TestCase):
         """New Image is created and saved"""
         img = image()
         eq_('Some title', img.title)
-        eq_(img.file, img.thumbnail_or_file())
         eq_(150, img.file.width)
+        eq_(200, img.file.height)
+
+    def test_thumbnail_url_if_set(self):
+        """thumbnail_url_if_set() returns self.thumbnail if set, or else
+        returns self.file"""
+        img = image()
+        eq_(img.file.url, img.thumbnail_url_if_set())
+
+        generate_image_thumbnail(img, img.file.name)
+        eq_(img.thumbnail.url, img.thumbnail_url_if_set())
+
+
+class VideoTestCase(TestCase):
+    fixtures = ['users.json']
+
+    def tearDown(self):
+        Video.objects.all().delete()
+        super(VideoTestCase, self).tearDown()
+
+    def test_new_video(self):
+        """New Video is created and saved"""
+        vid = video()
+        eq_('Some title', vid.title)
+        eq_(settings.GALLERY_VIDEO_PATH + 'test.webm', vid.webm.name)
+        eq_(settings.GALLERY_VIDEO_PATH + 'test.ogv', vid.ogv.name)
+        eq_(settings.GALLERY_VIDEO_PATH + 'test.flv', vid.flv.name)
+
+    def test_thumbnail_url_if_set(self):
+        """thumbnail_url_if_set() returns self.thumbnail if set, or else
+        returns URL to default thumbnail"""
+        # TODO: write this test when implementing video thumbnail generation
+        raise SkipTest

apps/gallery/tests/test_templates.py

@@ -1,14 +1,19 @@
 from nose.tools import eq_
 from pyquery import PyQuery as pq
 
-from sumo.tests import TestCase
-from sumo.tests import get
-from sumo.urlresolvers import reverse
 from sumo.helpers import urlparams
+from sumo.tests import TestCase, get
+from sumo.urlresolvers import reverse
+from gallery.models import Image
 from gallery.tests import image
 
 
 class GalleryPageCase(TestCase):
+    fixtures = ['users.json']
+
+    def tearDown(self):
+        Image.objects.all().delete()
+        super(GalleryPageCase, self).tearDown()
 
     def test_gallery_images(self):
         """Test that all images show up on images gallery page.
@@ -17,17 +22,18 @@ class GalleryPageCase(TestCase):
 
         """
         img = image()
-        response = get(self.client, 'gallery.gallery_images')
+        response = get(self.client, 'gallery.gallery_media',
+                       args=['image'])
         eq_(200, response.status_code)
         doc = pq(response.content)
         imgs = doc('section.gallery li img')
         eq_(1, len(imgs))
-        eq_(img.thumbnail_or_file().url, imgs[0].attrib['src'])
+        eq_(img.thumbnail_url_if_set(), imgs[0].attrib['src'])
 
     def test_gallery_locale(self):
         """Test that images only show for their set locale."""
         image(locale='es')
-        url = reverse('gallery.gallery_images')
+        url = reverse('gallery.gallery_media', args=['image'])
         response = self.client.get(url, follow=True)
         eq_(200, response.status_code)
         doc = pq(response.content)
@@ -43,6 +49,11 @@ class GalleryPageCase(TestCase):
 
 
 class MediaPageCase(TestCase):
+    fixtures = ['users.json']
+
+    def tearDown(self):
+        Image.objects.all().delete()
+        super(MediaPageCase, self).tearDown()
 
     def test_image_media_page(self):
         """Test the media page."""

apps/gallery/tests/test_views.py

@@ -0,0 +1,247 @@
+import json
+
+from django.conf import settings
+
+from nose.tools import eq_
+from nose import SkipTest
+
+from sumo.tests import post, LocalizingClient, TestCase
+from gallery.models import Image, Video
+
+
+TEST_IMG = 'apps/upload/tests/media/test.jpg'
+TEST_VID = {'webm': 'apps/gallery/tests/media/test.webm',
+            'ogv': 'apps/gallery/tests/media/test.ogv',
+            'flv': 'apps/gallery/tests/media/test.flv'}
+VIDEO_PATH = settings.MEDIA_URL + settings.GALLERY_VIDEO_PATH
+
+
+class UploadImageTestCase(TestCase):
+    fixtures = ['users.json']
+
+    def setUp(self):
+        super(UploadImageTestCase, self).setUp()
+        self.client = LocalizingClient()
+        self.client.login(username='pcraciunoiu', password='testpass')
+
+    def tearDown(self):
+        Image.objects.all().delete()
+        super(UploadImageTestCase, self).tearDown()
+
+    def test_empty_image(self):
+        """Specifying an invalid model returns 400."""
+        r = post(self.client, 'gallery.up_media_async', {'file': ''},
+                 args=['image'])
+
+        eq_(400, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('error', json_r['status'])
+        eq_('Could not upload your image.', json_r['message'])
+        eq_('You have not selected an image to upload.',
+            json_r['errors']['file'][0])
+
+    def test_empty_title(self):
+        """Title is required when uploading."""
+        with open(TEST_IMG) as f:
+            r = post(self.client, 'gallery.up_media_async', {'file': f},
+                     args=['image'])
+
+        eq_(400, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('error', json_r['status'])
+        eq_('Could not upload your image.', json_r['message'])
+        eq_('Please provide a title.', json_r['errors']['title'][0])
+
+    def test_empty_description(self):
+        """Description is required when uploading."""
+        with open(TEST_IMG) as f:
+            r = post(self.client, 'gallery.up_media_async',
+                     {'file': f, 'title': 'Title'}, args=['image'])
+
+        eq_(400, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('error', json_r['status'])
+        eq_('Could not upload your image.', json_r['message'])
+        eq_('Please provide a description.',
+            json_r['errors']['description'][0])
+
+    def test_upload_image(self):
+        """Uploading an image works."""
+        with open(TEST_IMG) as f:
+            r = post(self.client, 'gallery.up_media_async',
+                     {'file': f, 'title': 'Title', 'description': 'Test'},
+                     args=['image'])
+        image = Image.objects.all()[0]
+
+        eq_(1, Image.objects.count())
+        eq_(200, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('success', json_r['status'])
+        file = json_r['file']
+        eq_('test.jpg', file['name'])
+        eq_(90, file['width'])
+        eq_(120, file['height'])
+        assert file['url'].endswith(image.get_absolute_url())
+        eq_('pcraciunoiu', image.creator.username)
+        eq_(150, image.file.width)
+        eq_(200, image.file.height)
+        eq_('Title', image.title)
+        eq_('Test', image.description)
+        eq_('en-US', image.locale)
+
+    def test_delete_image(self):
+        """Deleting an uploaded image works."""
+        # Upload the image first
+        self.test_upload_image()
+        im = Image.objects.all()[0]
+        r = post(self.client, 'gallery.del_media_async', args=['image', im.id])
+
+        eq_(200, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('success', json_r['status'])
+        eq_(0, Image.objects.count())
+
+    def test_invalid_image(self):
+        """Make sure invalid files are not accepted as images."""
+        with open('apps/gallery/__init__.py', 'rb') as f:
+            r = post(self.client, 'gallery.up_media_async', {'file': f},
+                     args=['image'])
+
+        eq_(400, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('error', json_r['status'])
+        eq_('Could not upload your image.', json_r['message'])
+        eq_('Upload a valid image. The file you uploaded was either not an '
+            'image or a corrupted image.', json_r['errors']['file'][0])
+
+
+class UploadVideoTestCase(TestCase):
+    fixtures = ['users.json']
+
+    def setUp(self):
+        super(UploadVideoTestCase, self).setUp()
+        self.client = LocalizingClient()
+        self.client.login(username='pcraciunoiu', password='testpass')
+
+    def tearDown(self):
+        Video.objects.all().delete()
+        super(UploadVideoTestCase, self).tearDown()
+
+    def test_empty_title(self):
+        """Title is required when uploading."""
+        with open(TEST_VID['ogv']) as f:
+            r = post(self.client, 'gallery.up_media_async', {'ogv': f},
+                     args=['video'])
+
+        eq_(400, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('error', json_r['status'])
+        eq_('Could not upload your video.', json_r['message'])
+        eq_('Please provide a title.', json_r['errors']['title'][0])
+
+    def test_empty_description(self):
+        """Description is required when uploading."""
+        with open(TEST_VID['flv']) as f:
+            r = post(self.client, 'gallery.up_media_async',
+                     {'flv': f, 'title': 'Title'}, args=['video'])
+
+        eq_(400, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('error', json_r['status'])
+        eq_('Could not upload your video.', json_r['message'])
+        eq_('Please provide a description.',
+            json_r['errors']['description'][0])
+
+    def _upload_extension(self, ext):
+        with open(TEST_VID[ext]) as f:
+            r = post(self.client, 'gallery.up_media_async',
+                     {ext: f, 'title': 'Title', 'description': 'Test'},
+                     args=['video'])
+        return r
+
+    def test_upload_video(self):
+        """Uploading a video works."""
+        r = self._upload_extension('ogv')
+        vid = Video.objects.all()[0]
+
+        eq_(1, Video.objects.count())
+        eq_(200, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('success', json_r['status'])
+        file = json_r['file']
+        eq_('test.ogv', file['name'])
+        eq_(120, file['width'])
+        eq_(120, file['height'])
+        assert file['url'].endswith(vid.get_absolute_url())
+        eq_('pcraciunoiu', vid.creator.username)
+        eq_('Title', vid.title)
+        eq_('Test', vid.description)
+        eq_('en-US', vid.locale)
+        with open(TEST_VID['ogv']) as f:
+            eq_(f.read(), vid.ogv.read())
+
+    def test_delete_video_ogv(self):
+        """Deleting an uploaded video works."""
+        # Upload the video first
+        self._upload_extension('ogv')
+        vid = Video.objects.all()[0]
+        r = post(self.client, 'gallery.del_media_async',
+                 args=['video', vid.id])
+
+        eq_(200, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('success', json_r['status'])
+        eq_(0, Video.objects.count())
+
+    def test_upload_video_ogv_flv(self):
+        """Upload the same video, in ogv and flv formats"""
+        ogv = open(TEST_VID['ogv'])
+        flv = open(TEST_VID['flv'])
+        post(self.client, 'gallery.up_media_async',
+             {'ogv': ogv, 'flv': flv, 'title': 'Title', 'description': 'Test'},
+             args=['video'])
+        ogv.close()
+        flv.close()
+        vid = Video.objects.all()[0]
+        eq_(VIDEO_PATH + 'test.ogv', vid.ogv.url)
+        eq_(VIDEO_PATH + 'test.flv', vid.flv.url)
+
+    def test_upload_video_all(self):
+        """Upload the same video, in all formats"""
+        webm = open(TEST_VID['webm'])
+        ogv = open(TEST_VID['ogv'])
+        flv = open(TEST_VID['flv'])
+        post(self.client, 'gallery.up_media_async',
+             {'webm': webm, 'ogv': ogv, 'flv': flv,
+              'title': 'Title', 'description': 'Test'}, args=['video'])
+        webm.close()
+        ogv.close()
+        flv.close()
+        vid = Video.objects.all()[0]
+        eq_(VIDEO_PATH + 'test.webm', vid.webm.url)
+        eq_(VIDEO_PATH + 'test.ogv', vid.ogv.url)
+        eq_(VIDEO_PATH + 'test.flv', vid.flv.url)
+
+    def test_video_required(self):
+        """At least one video format is required to upload."""
+        r = post(self.client, 'gallery.up_media_async',
+                 {'title': 'Title', 'description': 'Test'}, args=['video'])
+        eq_(400, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('error', json_r['status'])
+        eq_('Could not upload your video.', json_r['message'])
+        eq_('The video has no files associated with it. You must upload one '
+            'of the following extensions: webm, ogv, flv.',
+            json_r['errors']['__all__'][0])
+
+    def test_invalid_video_webm(self):
+        """Make sure invalid webm videos are not accepted."""
+        raise SkipTest
+
+    def test_invalid_video_ogv(self):
+        """Make sure invalid ogv videos are not accepted."""
+        raise SkipTest
+
+    def test_invalid_video_flv(self):
+        """Make sure invalid flv videos are not accepted."""
+        raise SkipTest

apps/gallery/urls.py

@@ -1,12 +1,12 @@
 from django.conf.urls.defaults import patterns, url
 
-
 urlpatterns = patterns('gallery.views',
     url(r'^$', 'gallery', name='gallery.gallery'),
-    url(r'^/images$', 'gallery', {'filter': 'images'},
-        name='gallery.gallery_images'),
-    url(r'^/videos$', 'gallery', {'filter': 'videos'},
-        name='gallery.gallery_videos'),
-    url(r'^/media/(?P<media_type>\w+)/(?P<media_id>\d+)$',
+    url(r'^/(?P<media_type>\w+)s$', 'gallery', name='gallery.gallery_media'),
+    url(r'^/(?P<media_type>\w+)/upload_async$', 'up_media_async',
+        name='gallery.up_media_async'),
+    url(r'^/(?P<media_type>\w+)/(?P<media_id>\d+)/delete_async$',
+        'del_media_async', name='gallery.del_media_async'),
+    url(r'^/(?P<media_type>\w+)/(?P<media_id>\d+)$',
         'media', name='gallery.media'),
 )

apps/gallery/utils.py

@@ -0,0 +1,70 @@
+from django.conf import settings
+from django.core.exceptions import ValidationError
+from django.core.files import File
+
+from sumo.urlresolvers import reverse
+from .forms import ImageUploadForm, VideoUploadForm
+from .models import Image, Video
+from upload.utils import upload_media, check_file_size
+from upload.tasks import generate_image_thumbnail, _scale_dimensions
+
+
+def create_image(files, user, max_allowed_size, title, description, locale):
+    """Given an uploaded file, a user, and other data, it creates an Image"""
+    up_file = files.values()[0]
+    check_file_size(up_file, max_allowed_size)
+
+    image = Image(title=title, creator=user, locale=locale,
+                  description=description)
+    image.file.save(up_file.name, File(up_file), save=True)
+
+    # Generate thumbnail off thread
+    generate_image_thumbnail.delay(image, up_file.name)
+
+    (width, height) = _scale_dimensions(image.file.width, image.file.height)
+    delete_url = reverse('gallery.del_media_async', args=['image', image.id])
+    return {'name': up_file.name, 'url': image.get_absolute_url(),
+            'thumbnail_url': image.thumbnail_url_if_set(),
+            'width': width, 'height': height,
+            'delete_url': delete_url}
+
+
+def upload_image(request):
+    """Uploads an image from the request."""
+    title = request.POST.get('title')
+    description = request.POST.get('description')
+    return upload_media(
+        request, ImageUploadForm, create_image, settings.IMAGE_MAX_FILESIZE,
+        title=title, description=description, locale=request.locale)
+
+
+def create_video(files, user, max_allowed_size, title, description, locale):
+    """Given an uploaded file, a user, and other data, it creates a Video"""
+    vid = Video(title=title, creator=user, description=description,
+                locale=locale)
+    for name in files:
+        up_file = files[name]
+        check_file_size(up_file, max_allowed_size)
+        # name is in (webm, ogv, flv) sent from upload_video(), below
+        getattr(vid, name).save(up_file.name, up_file, save=False)
+
+    try:
+        vid.clean()
+    except ValidationError, e:
+        return {'validation': e.messages}
+    vid.save()
+    delete_url = reverse('gallery.del_media_async', args=['video', vid.id])
+    return {'name': up_file.name, 'url': vid.get_absolute_url(),
+            'thumbnail_url': vid.thumbnail_url_if_set(),
+            'width': settings.THUMBNAIL_SIZE,
+            'height': settings.THUMBNAIL_SIZE,
+            'delete_url': delete_url}
+
+
+def upload_video(request):
+    """Uploads a video from the request; accepts multiple submitted formats"""
+    title = request.POST.get('title')
+    description = request.POST.get('description')
+    return upload_media(
+        request, VideoUploadForm, create_video, settings.VIDEO_MAX_FILESIZE,
+        title=title, description=description, locale=request.locale)

apps/gallery/views.py

@@ -1,14 +1,28 @@
+import json
+
+from django.contrib.auth.decorators import login_required
+from django.contrib.contenttypes.models import ContentType
+from django.http import (HttpResponse, HttpResponseNotFound,
+                         HttpResponseBadRequest, Http404)
 from django.shortcuts import get_object_or_404
-from django.http import Http404
+from django.views.decorators.http import require_POST
 
 import jingo
+from tower import ugettext as _
 
-from sumo.utils import paginate
-from .models import Image, Video
 import gallery as constants
+from sumo.utils import paginate
+from upload.utils import FileTooLargeError
+from .models import Image, Video
+from .utils import upload_image, upload_video
+
+MSG_LIMIT_ONE = {'image': _('You may only upload one image at a time.'),
+                 'video': _('You may only upload one video at a time.')}
+MSG_FAIL_UPLOAD = {'image': _('Could not upload your image.'),
+                   'video': _('Could not upload your video.')}
 
 
-def gallery(request, filter='images'):
+def gallery(request, media_type='image'):
     """The media gallery.
 
     Filter can be set to 'images' or 'videos'.
@@ -16,7 +30,7 @@ def gallery(request, filter='images'):
     """
     locale = request.GET.get('locale', request.locale)
 
-    if filter == 'images':
+    if media_type == 'image':
         media_qs = Image.objects.filter(locale=locale)
     else:
         media_qs = Video.objects.filter(locale=locale)
@@ -28,7 +42,7 @@ def gallery(request, filter='images'):
                          'locale': locale})
 
 
-def media(request, media_id, media_type):
+def media(request, media_id, media_type='image'):
     """The media page."""
     if media_type == 'image':
         media = get_object_or_404(Image, pk=media_id)
@@ -40,3 +54,45 @@ def media(request, media_id, media_type):
     return jingo.render(request, 'gallery/media.html',
                         {'media': media,
                          'media_type': media_type})
+
+
+@login_required
+@require_POST
+def up_media_async(request, media_type='image'):
+    """Upload images or videos from request.FILES."""
+
+    try:
+        if media_type == 'image':
+            file_info = upload_image(request)
+        else:
+            file_info = upload_video(request)
+    except FileTooLargeError as e:
+        return HttpResponseBadRequest(
+            json.dumps({'status': 'error', 'message': e.args[0]}))
+
+    if isinstance(file_info, dict) and 'thumbnail_url' in file_info:
+        return HttpResponse(
+            json.dumps({'status': 'success', 'file': file_info}))
+
+    message = MSG_FAIL_UPLOAD[media_type]
+    return HttpResponseBadRequest(
+        json.dumps({'status': 'error', 'message': message,
+                    'errors': file_info}))
+
+
+@login_required
+@require_POST
+def del_media_async(request, media_id, media_type='image'):
+    """Delete a media object given its id."""
+    model_class = ContentType.objects.get(model=media_type).model_class()
+    try:
+        media = model_class.objects.get(pk=media_id)
+    except Image.DoesNotExist:
+        message = _('The requested media (%s) could not be found.') % media_id
+        return HttpResponseNotFound(
+            json.dumps({'status': 'error', 'message': message}))
+
+    # Extra care: clean up all the files individually
+    media.delete()
+
+    return HttpResponse(json.dumps({'status': 'success'}))

apps/questions/views.py

@@ -41,7 +41,7 @@ from .tasks import (cache_top_contributors, build_solution_notification,
 import questions as constants
 from .question_config import products
 from upload.models import ImageAttachment
-from upload.views import upload_images
+from upload.views import upload_imageattachment
 
 
 log = logging.getLogger('k.questions')
@@ -291,7 +291,7 @@ def reply(request, question_id):
 
     # NOJS: upload image
     if 'upload_image' in request.POST:
-        upload_images(request, question)
+        upload_imageattachment(request, question)
         return answers(request, question_id, form)
 
     if form.is_valid():
@@ -543,7 +543,7 @@ def edit_answer(request, question_id, answer_id):
         raise PermissionDenied
 
     # NOJS: upload images, if any
-    upload_images(request, answer)
+    upload_imageattachment(request, answer)
 
     if request.method == 'GET':
         form = AnswerForm({'content': answer.content})

apps/sumo/tests/__init__.py

@@ -1,4 +1,5 @@
 from django.conf import settings
+from django.contrib.auth.models import User
 from django.test.client import Client
 
 from test_utils import TestCase  # So others can import it from here
@@ -31,3 +32,17 @@ class LocalizingClient(Client):
     # If you use this, you might also find the force_locale=True argument to
     # sumo.urlresolvers.reverse() handy, in case you need to force locale
     # prepending in a one-off case or do it outside a mock request.
+
+
+class FixtureMissingError(Exception):
+    """Raise this if a fixture is missing"""
+
+
+def get_user(username='jsocol'):
+    """Return a django user or raise FixtureMissingError"""
+    try:
+        return User.objects.get(username=username)
+    except User.DoesNotExist:
+        raise FixtureMissingError(
+            'Username "%s" not found. You probably forgot to import a'
+            ' users fixture.' % username)

apps/sumo/tests/test_parser.py

@@ -31,6 +31,7 @@ def markup_helper(content, markup, title='Template:test'):
 
 class SimpleSyntaxTestCase(TestCase):
     """Simple syntax regexing, like {note}...{/note}, {key Ctrl+K}"""
+    fixtures = ['users.json']
 
     def test_note_simple(self):
         """Simple note syntax"""
@@ -140,6 +141,8 @@ class SimpleSyntaxTestCase(TestCase):
 
 
 class TestWikiTemplate(TestCase):
+    fixtures = ['users.json']
+
     def test_template(self):
         """Simple template markup."""
         doc = markup_helper('Test content', '[[Template:test]]')[0]
@@ -235,6 +238,8 @@ class TestWikiTemplate(TestCase):
 
 
 class TestWikiInclude(TestCase):
+    fixtures = ['users.json']
+
     def test_revision_include(self):
         """Simple include markup."""
         p = doc_rev_parser('Test content', 'Test title')[2]
@@ -249,6 +254,8 @@ class TestWikiInclude(TestCase):
 
 
 class TestWikiParser(TestCase):
+    fixtures = ['users.json']
+
     def setUp(self):
         self.d, self.r, self.p = doc_rev_parser(
             'Test content', 'Installing Firefox')
@@ -336,6 +343,8 @@ class TestWikiParser(TestCase):
 
 
 class TestWikiInternalLinks(TestCase):
+    fixtures = ['users.json']
+
     def setUp(self):
         self.d, self.r, self.p = doc_rev_parser(
             'Test content', 'Installing Firefox')
@@ -419,6 +428,8 @@ def pq_img(p, text, selector='div.img'):
 
 
 class TestWikiImageTags(TestCase):
+    fixtures = ['users.json']
+
     def setUp(self):
         self.d, self.r, self.p = doc_rev_parser(
             'Test content', 'Installing Firefox')

apps/upload/forms.py

@@ -1,6 +1,10 @@
 from django import forms
 
+from tower import ugettext_lazy as _lazy
 
-class ImageUploadForm(forms.Form):
+MSG_IMAGE_REQUIRED = _lazy(u'You have not selected an image to upload.')
+
+
+class ImageAttachmentUploadForm(forms.Form):
     """Image upload form."""
-    image = forms.ImageField()
+    image = forms.ImageField(error_messages={'required': MSG_IMAGE_REQUIRED})

apps/upload/models.py

@@ -4,6 +4,8 @@ from django.contrib.contenttypes.models import ContentType
 from django.contrib.contenttypes import generic
 from django.db import models
 
+from sumo.helpers import reverse
+
 
 class ImageAttachment(models.Model):
     """An image attached to an object using a generic foreign key"""
@@ -19,6 +21,14 @@ class ImageAttachment(models.Model):
     def __unicode__(self):
         return self.file.name
 
-    def thumbnail_or_file(self):
+    def get_absolute_url(self):
+        return self.file.url
+
+    def thumbnail_if_set(self):
         """Returns self.thumbnail, if set, else self.file"""
         return self.thumbnail if self.thumbnail else self.file
+
+    def get_delete_url(self):
+        """Returns the URL to delete this object. Assumes the object has an
+        id."""
+        return reverse('upload.del_image_async', args=[self.id])

apps/upload/tasks.py

@@ -11,14 +11,15 @@ log = logging.getLogger('k.task')
 
 
 @task(rate_limit='15/m')
-def generate_thumbnail(image, image_name):
+def generate_image_thumbnail(image, image_name):
     """Generate a thumbnail given an image and a name."""
-    log.info('Generating thumbnail for ImageAttachment %s.' % image.id)
-    thumb_content = _create_thumbnail(image.file.path)
+    log.info('Generating thumbnail for %(model_class)s %(id)s.' %
+             {'model_class': image.__class__.__name__, 'id': image.id})
+    thumb_content = _create_image_thumbnail(image.file.path)
     image.thumbnail.save(image_name, thumb_content, save=True)
 
 
-def _create_thumbnail(file_path, longest_side=settings.THUMBNAIL_SIZE):
+def _create_image_thumbnail(file_path, longest_side=settings.THUMBNAIL_SIZE):
     """
     Returns a thumbnail file with a set longest side.
     """

apps/upload/templates/upload/attachments.html

@@ -24,7 +24,7 @@
       {% endif %}
     {% endif %}
     <a class="image" href="{{ image.file.url }}">
-      <img src="{{ image.thumbnail_or_file().url }}"/>
+      <img src="{{ image.thumbnail_if_set().url }}"/>
     </a>
   </div>
 {%- endmacro %}

apps/upload/tests/__init__.py

@@ -1,16 +1,43 @@
 import json
 
+from django.conf import settings
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
 from django.core.files import File
-from django.test import TestCase
 
 from nose.tools import eq_
 
 from questions.models import Question
 from sumo.tests import post, LocalizingClient, TestCase
 from upload.models import ImageAttachment
-from upload.utils import create_image_attachment
+from upload.utils import (create_imageattachment, check_file_size,
+                          FileTooLargeError)
+
+
+def check_file_info(file_info, name, width, height, delete_url, url,
+                    thumbnail_url):
+    eq_(name, file_info['name'])
+    eq_(width, file_info['width'])
+    eq_(height, file_info['height'])
+    eq_(delete_url, file_info['delete_url'])
+    eq_(url, file_info['url'])
+    eq_(thumbnail_url, file_info['thumbnail_url'])
+
+
+class CheckFileSizeTestCase(TestCase):
+    """Tests for check_file_size"""
+    def test_check_file_size_under(self):
+        """No exception should be raised"""
+        with open('apps/upload/tests/media/test.jpg') as f:
+            up_file = File(f)
+            check_file_size(up_file, settings.IMAGE_MAX_FILESIZE)
+
+    def test_check_file_size_over(self):
+        """FileTooLargeError should be raised"""
+        with open('apps/upload/tests/media/test.jpg') as f:
+            up_file = File(f)
+            fn = lambda: check_file_size(up_file, 0)
+            self.assertRaises(FileTooLargeError, fn)
 
 
 class CreateImageAttachmentTestCase(TestCase):
@@ -26,7 +53,7 @@ class CreateImageAttachmentTestCase(TestCase):
         ImageAttachment.objects.all().delete()
         super(CreateImageAttachmentTestCase, self).tearDown()
 
-    def test_basic(self):
+    def test_create_imageattachment(self):
         """
         An image attachment is created from an uploaded file.
 
@@ -34,15 +61,23 @@ class CreateImageAttachmentTestCase(TestCase):
         """
         with open('apps/upload/tests/media/test.jpg') as f:
             up_file = File(f)
-            image = create_image_attachment(up_file, self.obj, self.user)
+            file_info = create_imageattachment(
+                {'image': up_file}, self.user, settings.IMAGE_MAX_FILESIZE,
+                self.obj)
 
-        message = 'File name "%s" does not contain "test"' % image.file.name
-        assert 'test' in image.file.name, message
-        eq_(150, image.file.width)
-        eq_(200, image.file.height)
-        eq_(self.obj.id, image.object_id)
-        eq_(self.ct.id, image.content_type.id)
-        eq_(self.user, image.creator)
+        image = ImageAttachment.objects.all()[0]
+        check_file_info(
+            file_info, name='apps/upload/tests/media/test.jpg',
+            width=90, height=120, delete_url=image.get_delete_url(),
+            url=image.get_absolute_url(), thumbnail_url=image.thumbnail.url)
+
+    def test_create_imageattachment_too_big(self):
+        """Raise exception if uploaded image is too big."""
+        with open('apps/upload/tests/media/test.jpg') as f:
+            up_file = File(f)
+            fn = lambda: create_imageattachment({'image': up_file}, self.user,
+                                                100, self.obj)
+        self.assertRaises(FileTooLargeError, fn)
 
 
 class UploadImageTestCase(TestCase):
@@ -86,8 +121,10 @@ class UploadImageTestCase(TestCase):
         json_r = json.loads(r.content)
         eq_('error', json_r['status'])
         eq_('Invalid or no image received.', json_r['message'])
+        eq_('You have not selected an image to upload.',
+            json_r['errors']['image'][0])
 
-    def test_basic(self):
+    def test_upload_image(self):
         """Uploading an image works."""
         with open('apps/upload/tests/media/test.jpg') as f:
             r = post(self.client, 'upload.up_image_async', {'image': f},
@@ -96,7 +133,7 @@ class UploadImageTestCase(TestCase):
         eq_(200, r.status_code)
         json_r = json.loads(r.content)
         eq_('success', json_r['status'])
-        file = json_r['files'][0]
+        file = json_r['file']
         eq_('test.jpg', file['name'])
         eq_(90, file['width'])
         eq_(120, file['height'])
@@ -111,6 +148,18 @@ class UploadImageTestCase(TestCase):
         eq_('question', image.content_type.model)
         eq_(1, image.object_id)
 
+    def test_delete_image(self):
+        """Deleting an uploaded image works."""
+        # Upload the image first
+        self.test_upload_image()
+        im = ImageAttachment.objects.all()[0]
+        r = post(self.client, 'upload.del_image_async', args=[im.id])
+
+        eq_(200, r.status_code)
+        json_r = json.loads(r.content)
+        eq_('success', json_r['status'])
+        eq_(0, ImageAttachment.objects.count())
+
     def test_invalid_image(self):
         """Make sure invalid files are not accepted as images."""
         with open('apps/upload/__init__.py', 'rb') as f:
@@ -121,3 +170,4 @@ class UploadImageTestCase(TestCase):
         json_r = json.loads(r.content)
         eq_('error', json_r['status'])
         eq_('Invalid or no image received.', json_r['message'])
+        eq_('The submitted file is empty.', json_r['errors']['image'][0])

apps/upload/tests/test_models.py

@@ -7,7 +7,7 @@ from nose.tools import eq_
 from questions.models import Question
 from sumo.tests import TestCase
 from upload.models import ImageAttachment
-from upload.tasks import generate_thumbnail
+from upload.tasks import generate_image_thumbnail
 
 
 class ImageAttachmentTestCase(TestCase):
@@ -19,16 +19,20 @@ class ImageAttachmentTestCase(TestCase):
         self.obj = Question.objects.all()[0]
         self.ct = ContentType.objects.get_for_model(self.obj)
 
-    def test_thumbnail_or_file(self):
-        """thumbnail_or_file() returns self.thumbnail if set, or else returns
+    def tearDown(self):
+        ImageAttachment.objects.all().delete()
+        super(ImageAttachmentTestCase, self).tearDown()
+
+    def test_thumbnail_if_set(self):
+        """thumbnail_if_set() returns self.thumbnail if set, or else returns
         self.file"""
         image = ImageAttachment(content_object=self.obj, creator=self.user)
         with open('apps/upload/tests/media/test.jpg') as f:
             up_file = File(f)
             image.file.save(up_file.name, up_file, save=True)
 
-        eq_(image.file, image.thumbnail_or_file())
+        eq_(image.file, image.thumbnail_if_set())
 
-        generate_thumbnail(image, up_file.name)
+        generate_image_thumbnail(image, up_file.name)
 
-        eq_(image.thumbnail, image.thumbnail_or_file())
+        eq_(image.thumbnail, image.thumbnail_if_set())

apps/upload/tests/test_tasks.py

@@ -2,15 +2,14 @@ from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.files import File
 from django.core.files.images import ImageFile
-from django.test import TestCase
 
 from nose.tools import eq_
 
 from questions.models import Question
 from sumo.tests import TestCase
 from upload.models import ImageAttachment
-from upload.tasks import (_scale_dimensions, _create_thumbnail,
-                          generate_thumbnail)
+from upload.tasks import (_scale_dimensions, _create_image_thumbnail,
+                          generate_image_thumbnail)
 
 
 class ScaleDimensionsTestCase(TestCase):
@@ -60,9 +59,10 @@ class ScaleDimensionsTestCase(TestCase):
 
 class CreateThumbnailTestCase(TestCase):
 
-    def test_create_thumbnail_default(self):
+    def test_create_image_thumbnail_default(self):
         """A thumbnail is created from an image file."""
-        thumb_content = _create_thumbnail('apps/upload/tests/media/test.jpg')
+        thumb_content = _create_image_thumbnail(
+            'apps/upload/tests/media/test.jpg')
         actual_thumb = ImageFile(thumb_content)
         with open('apps/upload/tests/media/test_thumb.jpg') as f:
             expected_thumb = ImageFile(f)
@@ -82,14 +82,14 @@ class GenerateThumbnail(TestCase):
     def tearDown(self):
         ImageAttachment.objects.all().delete()
 
-    def test_generate_thumbnail_default(self):
-        """generate_thumbnail creates a thumbnail."""
+    def test_generate_image_thumbnail_default(self):
+        """generate_image_thumbnail creates a thumbnail."""
         image = ImageAttachment(content_object=self.obj, creator=self.user)
         with open('apps/upload/tests/media/test.jpg') as f:
             up_file = File(f)
             image.file.save(up_file.name, up_file, save=True)
 
-        generate_thumbnail(image, up_file.name)
+        generate_image_thumbnail(image, up_file.name)
 
         eq_(90, image.thumbnail.width)
         eq_(120, image.thumbnail.height)

apps/upload/utils.py

@@ -1,61 +1,80 @@
 from django.conf import settings
 from django.core.files import File
 
-from tower import ugettext as _
+from tower import ugettext_lazy as _lazy
 
-from sumo.helpers import reverse
-from .forms import ImageUploadForm
+from .forms import ImageAttachmentUploadForm
 from .models import ImageAttachment
-from .tasks import generate_thumbnail, _scale_dimensions
+from .tasks import generate_image_thumbnail, _scale_dimensions
 
 
-def create_image_attachment(up_file, obj, user):
+def check_file_size(f, max_allowed_size):
+    """Check the file size of f is less than max_allowed_size
+
+    Raise FileTooLargeError if the check fails.
+
+    """
+    if f.size > max_allowed_size:
+        message = _lazy('"%s" is too large (%sKB), the limit is %sKB') % (
+            f.name, f.size >> 10, max_allowed_size >> 10)
+        raise FileTooLargeError(message)
+
+
+def create_imageattachment(files, user, max_allowed_size, obj):
     """
     Given an uploaded file, a user and an object, it creates an ImageAttachment
     owned by `user` and attached to `obj`.
     """
+    up_file = files.values()[0]
+    check_file_size(up_file, max_allowed_size)
+
     image = ImageAttachment(content_object=obj, creator=user)
-    file_ = File(up_file)
-    image.file.save(up_file.name, file_, save=False)
-    image.save()
+    image.file.save(up_file.name, File(up_file), save=True)
 
     # Generate thumbnail off thread
-    generate_thumbnail.delay(image, up_file.name)
+    generate_image_thumbnail.delay(image, up_file.name)
 
-    return image
+    (width, height) = _scale_dimensions(image.file.width, image.file.height)
+    return {'name': up_file.name, 'url': image.file.url,
+            'thumbnail_url': image.thumbnail_if_set().url,
+            'width': width, 'height': height,
+            'delete_url': image.get_delete_url()}
 
 
 class FileTooLargeError(Exception):
     pass
 
 
-def upload_images(request, obj):
-    """
-    Takes in a request object and returns a list with information about each
-    image: name, url, thumbnail_url, width, height.
+def upload_imageattachment(request, obj):
+    """Uploads image attachments. See upload_media.
+
+    Attaches images to the given object, using the create_imageattachment
+    callback.
 
-    Attaches images to the given object.
     """
-    form = ImageUploadForm(request.POST, request.FILES)
+    return upload_media(
+        request, ImageAttachmentUploadForm, create_imageattachment,
+        settings.IMAGE_MAX_FILESIZE, obj=obj)
+
+
+def upload_media(request, form_cls, up_file_callback, max_allowed_size,
+                 **kwargs):
+    """
+    Uploads media files and returns a list with information about each media:
+    name, url, thumbnail_url, width, height.
+
+    Args:
+    * request object
+    * form class, used to instantiate and validate form for upload
+    * callback to save the file given its content and creator
+    * max upload size per one file
+    * extra kwargs will all be passed to the callback
+
+    """
+    form = form_cls(request.POST, request.FILES)
     if request.method == 'POST' and form.is_valid():
-        files = []
-        for name in request.FILES:
-            up_file = request.FILES[name]
-            if up_file.size > settings.IMAGE_MAX_FILESIZE:
-                message = _('"%s" is too large (%sKB), the limit is %sKB') % (
-                    up_file.name, up_file.size >> 10,
-                    settings.IMAGE_MAX_FILESIZE >> 10)
-                raise FileTooLargeError(message)
-
-            image = create_image_attachment(up_file, obj, request.user)
-
-            delete_url = reverse('upload.del_image_async', args=[image.id])
-            im = image.file
-            (width, height) = _scale_dimensions(im.width, im.height)
-            files.append({'name': up_file.name, 'url': image.file.url,
-                          'thumbnail_url': image.thumbnail_or_file().url,
-                          'width': width,
-                          'height': height,
-                          'delete_url': delete_url})
-        return files
+        return up_file_callback(request.FILES, request.user, max_allowed_size,
+                                **kwargs)
+    elif not form.is_valid():
+        return form.errors
     return None

apps/upload/views.py

@@ -11,7 +11,7 @@ from tower import ugettext as _
 
 from access.decorators import has_perm_or_owns_or_403
 from .models import ImageAttachment
-from .utils import upload_images, FileTooLargeError
+from .utils import upload_imageattachment, FileTooLargeError
 
 
 @login_required
@@ -35,23 +35,24 @@ def up_image_async(request, model_name, object_pk):
             json.dumps({'status': 'error', 'message': message}))
 
     try:
-        files = upload_images(request, obj)
+        file_info = upload_imageattachment(request, obj)
     except FileTooLargeError as e:
         return HttpResponseBadRequest(
             json.dumps({'status': 'error', 'message': e.args[0]}))
 
-    if files is not None:
+    if isinstance(file_info, dict) and 'thumbnail_url' in file_info:
         return HttpResponse(
-            json.dumps({'status': 'success', 'files': files}))
+            json.dumps({'status': 'success', 'file': file_info}))
 
     message = _('Invalid or no image received.')
     return HttpResponseBadRequest(
-        json.dumps({'status': 'error', 'message': message}))
+        json.dumps({'status': 'error', 'message': message,
+                    'errors': file_info}))
 
 
 @login_required
 @require_POST
-@has_perm_or_owns_or_403('upload_imageattachment.image_upload', 'creator',
+@has_perm_or_owns_or_403('upload.image_upload', 'creator',
                          (ImageAttachment, 'id__iexact', 'image_id'),
                          (ImageAttachment, 'id__iexact', 'image_id'))
 def del_image_async(request, image_id):

apps/wiki/tests/__init__.py

@@ -1,10 +1,9 @@
 from django.template.defaultfilters import slugify
-from django.contrib.auth.models import User
 from django.core.cache import cache
 
 from datetime import datetime
 
-from sumo.tests import LocalizingClient, TestCase
+from sumo.tests import LocalizingClient, TestCase, get_user
 from wiki.models import Document, Revision, CATEGORIES, SIGNIFICANCES
 
 
@@ -31,14 +30,14 @@ def document(**kwargs):
 
 def revision(**kwargs):
     """Return an empty revision with enough stuff filled out that it can be
-    saved."""
+    saved.
+
+    Requires a users fixture if no creator is provided.
+
+    """
     u = None
     if 'creator' not in kwargs:
-        try:
-            u = User.objects.get(username='testuser')
-        except User.DoesNotExist:
-            u = User(username='testuser', email='me@nobody.test')
-            u.save()
+        u = get_user()
 
     d = None
     if 'document' not in kwargs:

apps/wiki/tests/test_models.py

@@ -101,6 +101,8 @@ class DocumentTests(TestCase):
 
 class RevisionTests(TestCase):
     """Tests for the Revision model"""
+    fixtures = ['users.json']
+
     def test_approved_revision_updates_html(self):
         """Creating an approved revision updates document.html"""
         d = document(html='This goes away')

docs/coding.rst

@@ -0,0 +1,21 @@
+========================================
+Coding conventions and other coding info
+========================================
+
+This document contains useful information about our coding conventions, and
+things to watch out for, etc.
+
+
+Naming conventions
+------------------
+
+
+Tests
+^^^^^
+
+* Avoid naming test files ``test_utils.py``, since we use a library with the
+  same name. Use ``test__utils.py`` instead.
+
+* If you're expecting ``reverse`` to return locales in the URL, use
+  ``LocalizingClient`` instead of the default client for the ``TestCase``
+  class.

docs/production.rst

@@ -98,15 +98,20 @@ Uploads
 Uploaded files must be served from anywhere under the media folder. The full
 path to this folder can be changed in the setting ``MEDIA_ROOT``.
 
-Here are the currently defined upload paths, all relative to ``MEDIA_ROOT``:
+Here are the currently defined upload settings. All paths are relative to
+``MEDIA_ROOT``::
 
 ``THUMBNAIL_UPLOAD_PATH``
   Thumbnails of images for questions. defaults to
-  ``uploads/images/thumbnails/``
+  ``uploads/images/thumbnails/``.
 ``IMAGE_UPLOAD_PATH``
-  Images for questions, defaults to ``uploads/images/``
-``GALLERY_THUMBNAIL_PATH``
+  Images for questions, defaults to ``uploads/images/``.
+``GALLERY_IMAGE_THUMBNAIL_PATH``, ``GALLERY_VIDEO_THUMBNAIL_PATH``
   Thumbnails of video/images for the media gallery, defaults to
-  ``uploads/gallery/thumbnails/``
-``GALLERY_PATH``
-  Video/images for the media gallery, defaults to ``uploads/gallery/``
+  ``uploads/gallery/thumbnails/``.
+``GALLERY_IMAGE_PATH``, ``GALLERY_VIDEO_PATH``
+  Video/images for the media gallery, defaults to ``uploads/gallery/``.
+``IMAGE_MAX_FILESIZE``, ``VIDEO_MAX_FILESIZE``
+  Maximum size for uploaded images (defaults to 1mb) or videos (16mb).
+``THUMBNAIL_PROGRESS_URL``
+  URL to an image, used to indicate thumbnail generation is in progress.

media/js/upload.js

@@ -74,7 +74,7 @@ $(document).ready(function () {
 
                 $options.progress.removeClass('show');
                 if (upStatus == 'success') {
-                    upFile = iframeJSON.files[0];
+                    upFile = iframeJSON.file;
                     $thumbnail = $('<img/>')
                         .attr({alt: upFile.name, title: upFile.name,
                                width: upFile.width, height: upFile.height,

migrations/40-gallery-video.sql

@@ -0,0 +1,6 @@
+ALTER TABLE `gallery_video`
+    ADD `webm` varchar(100) NULL,
+    ADD `ogv` varchar(100) NULL,
+    ADD `flv` varchar(100) NULL;
+
+ALTER TABLE `gallery_video` DROP `file`;

settings.py

@@ -416,3 +416,5 @@ GALLERY_IMAGE_PATH = 'uploads/gallery/images/'
 GALLERY_IMAGE_THUMBNAIL_PATH = 'uploads/gallery/images/thumbnails/'
 GALLERY_VIDEO_PATH = 'uploads/gallery/videos/'
 GALLERY_VIDEO_THUMBNAIL_PATH = 'uploads/gallery/videos/thumbnails/'
+THUMBNAIL_PROGRESS_URL = MEDIA_URL + 'img/wait-trans.gif'
+VIDEO_MAX_FILESIZE = 16777216  # 16 megabytes, in bytes