Merge pull request #1147 from normanjaeckel/EditOwnFiles

Enable update and delete view for uploader refering to his own files.

CHANGELOG

@@ -10,6 +10,8 @@ Version 1.6 (unreleased)
 
 Participants:
 - Disable widgets by default.
+Files:
+- Enabled update and delete view for uploader refering to his own files.
 Other:
 - Changed widget api. Used new metaclass.
 

openslides/mediafile/forms.py

@@ -7,30 +7,34 @@ from openslides.utils.forms import CssClassMixin
 from .models import Mediafile
 
 
-class MediafileNormalUserCreateForm(CssClassMixin, ModelForm):
+class MediafileFormMixin(object):
     """
-    Form to create a media file. This form is only used by normal users,
+    Mixin for mediafile forms. It is used to delete old files.
-    not by managers.
     """
-    class Meta:
-        model = Mediafile
-        exclude = ('uploader',)
-
-
-class MediafileUpdateForm(CssClassMixin, ModelForm):
-    """
-    Form to edit mediafile entries. This form is only for managers to update
-    the mediafile entry.
-    """
-    class Meta:
-        model = Mediafile
-
     def save(self, *args, **kwargs):
         """
-        Method to save the form. Here the overwrite is to delete old files.
+        Method to save the form. Here the override is to delete old files.
         """
         if not self.instance.pk is None:
             old_file = Mediafile.objects.get(pk=self.instance.pk).mediafile
             if not old_file == self.instance.mediafile:
                 old_file.delete()
-        return super(MediafileUpdateForm, self).save(*args, **kwargs)
+        return super(MediafileFormMixin, self).save(*args, **kwargs)
+
+
+class MediafileNormalUserForm(MediafileFormMixin, CssClassMixin, ModelForm):
+    """
+    This form is only used by normal users, not by managers.
+    """
+    class Meta:
+        model = Mediafile
+        fields = ('mediafile', 'title', 'is_presentable')
+
+
+class MediafileManagerForm(MediafileFormMixin, CssClassMixin, ModelForm):
+    """
+    This form is only used be managers, not by normal users.
+    """
+    class Meta:
+        model = Mediafile
+        fields = ('mediafile', 'title', 'uploader', 'is_presentable')

openslides/mediafile/templates/mediafile/mediafile_list.html

@@ -20,7 +20,7 @@
             <th>{% trans 'Size' %}</th>
             <th>{% trans 'Upload time' %}</th>
             <th>{% trans 'Uploaded by' %}</th>
-            {% if perms.mediafile.can_manage %}
+            {% if perms.mediafile.can_manage or perms.mediafile.can_upload %}
                 <th class="mini_width">{% trans "Actions" %}</th>
             {% endif %}
         </tr>
@@ -31,18 +31,22 @@
             <td>{{ mediafile.get_filesize }}</td>
             <td>{{ mediafile.timestamp }}</td>
             <td><a href="{{ mediafile.uploader|absolute_url }}">{{ mediafile.uploader }}</a></td>
-            {% if perms.mediafile.can_manage %}
+            {% if perms.mediafile.can_manage or perms.mediafile.can_upload %}
             <td>
+                {% if mediafile.with_action_buttons %}
                 <span style="width: 1px; white-space: nowrap;">
                     <a href="{{ mediafile|absolute_url:'update' }}" rel="tooltip" data-original-title="{% trans 'Edit' %}" class="btn btn-mini"><i class="icon-pencil"></i></a>
                     <a href="{{ mediafile|absolute_url:'delete' }}" rel="tooltip" data-original-title="{% trans 'Delete' %}" class="btn btn-mini"><i class="icon-remove"></i></a>
-                    {% if mediafile.is_presentable %}{% if mediafile.filetype in mediafile.PRESENTABLE_FILE_TYPES %}
+                    {% if perms.mediafile.can_manage and mediafile.is_presentable %}{% if mediafile.filetype in mediafile.PRESENTABLE_FILE_TYPES %}
                     <a href="{{ mediafile|absolute_url:'projector' }}" class="activate_link choose-pdf btn {% if mediafile.is_active_slide %}btn-primary{% endif %} btn-mini" title="{% trans 'Show' %}">
                         <i class="icon-facetime-video {% if mediafile.is_active_slide %}icon-white{% endif %}">
                         </i>
                     </a>
                     {% endif %}{% endif %}
                 </span>
+                {% else %}
+                    &nbsp;
+                {% endif %}
             </td>
             {% endif %}
         </tr>

openslides/mediafile/views.py

@@ -11,7 +11,7 @@ from openslides.utils.tornado_webserver import ProjectorSocketHandler
 from openslides.utils.views import (AjaxView, CreateView, DeleteView, RedirectView, ListView,
                                     UpdateView)
 
-from .forms import MediafileNormalUserCreateForm, MediafileUpdateForm
+from .forms import MediafileManagerForm, MediafileNormalUserForm
 from .models import Mediafile
 
 
@@ -26,27 +26,34 @@ class MediafileListView(ListView):
                 request.user.has_perm('mediafile.can_upload') or
                 request.user.has_perm('mediafile.can_manage'))
 
+    def get_context_data(self, *args, **kwargs):
+        context = super(MediafileListView, self).get_context_data(*args, **kwargs)
+        for mediafile in context['mediafile_list']:
+            if self.request.user.has_perm('mediafile.can_manage'):
+                mediafile.with_action_buttons = True
+            elif self.request.user.has_perm('mediafile.can_upload') and self.request.user == mediafile.uploader:
+                mediafile.with_action_buttons = True
+            else:
+                mediafile.with_action_buttons = False
+        return context
 
-class MediafileCreateView(CreateView):
+
+class MediafileViewMixin(object):
     """
-    View to upload a new file. A manager can also set the uploader, else
+    Mixin for create and update views for mediafiles.
-    the request user is set as uploader.
+
+    A manager can set the uploader manually, else the request user is set as uploader.
     """
     model = Mediafile
-    permission_required = 'mediafile.can_upload'
     success_url_name = 'mediafile_list'
     url_name_args = []
 
     def get_form(self, form_class):
         form_kwargs = self.get_form_kwargs()
-        if self.request.method == 'GET':
-            form_kwargs['initial'].update({'uploader': self.request.user.person_id})
         if not self.request.user.has_perm('mediafile.can_manage'):
-            # Returns our own ModelForm from .forms
+            return MediafileNormalUserForm(**form_kwargs)
-            return MediafileNormalUserCreateForm(**form_kwargs)
         else:
-            # Returns a ModelForm created by Django.
+            return MediafileManagerForm(**form_kwargs)
-            return form_class(**form_kwargs)
 
     def manipulate_object(self, *args, **kwargs):
         """
@@ -56,18 +63,29 @@ class MediafileCreateView(CreateView):
         """
         if not self.request.user.has_perm('mediafile.can_manage'):
             self.object.uploader = self.request.user
-        return super(MediafileCreateView, self).manipulate_object(*args, **kwargs)
+        return super(MediafileViewMixin, self).manipulate_object(*args, **kwargs)
 
 
-class MediafileUpdateView(UpdateView):
+class MediafileCreateView(MediafileViewMixin, CreateView):
+    """
+    View to upload a new file.
+    """
+    permission_required = 'mediafile.can_upload'
+
+    def get_form_kwargs(self, *args, **kwargs):
+        form_kwargs = super(MediafileCreateView, self).get_form_kwargs(*args, **kwargs)
+        if self.request.method == 'GET':
+            form_kwargs['initial'].update({'uploader': self.request.user.person_id})
+        return form_kwargs
+
+
+class MediafileUpdateView(MediafileViewMixin, UpdateView):
     """
     View to edit the entry of an uploaded file.
     """
-    model = Mediafile
+    def has_permission(self, request, *args, **kwargs):
-    permission_required = 'mediafile.can_manage'
+        return (request.user.has_perm('mediafile.can_manage') or
-    form_class = MediafileUpdateForm
+                (request.user.has_perm('mediafile.can_upload') and self.get_object().uploader == self.request.user))
-    success_url_name = 'mediafile_list'
-    url_name_args = []
 
     def get_form_kwargs(self, *args, **kwargs):
         form_kwargs = super(MediafileUpdateView, self).get_form_kwargs(*args, **kwargs)
@@ -80,9 +98,12 @@ class MediafileDeleteView(DeleteView):
     View to delete the entry of an uploaded file and the file itself.
     """
     model = Mediafile
-    permission_required = 'mediafile.can_manage'
     success_url_name = 'mediafile_list'
 
+    def has_permission(self, request, *args, **kwargs):
+        return (request.user.has_perm('mediafile.can_manage') or
+                (request.user.has_perm('mediafile.can_upload') and self.get_object().uploader == self.request.user))
+
     def on_clicked_yes(self, *args, **kwargs):
         """Deletes the file in the filesystem, if user clicks "Yes"."""
         self.object.mediafile.delete()

tests/mediafile/tests.py

@@ -35,7 +35,7 @@ class MediafileTest(TestCase):
         # Setup a mediafile object
         self.tmp_dir = settings.MEDIA_ROOT
         tmpfile_no, mediafile_path = tempfile.mkstemp(prefix='tmp_openslides_test_', dir=self.tmp_dir)
-        self.object = Mediafile.objects.create(title='Title File 1', mediafile=mediafile_path, uploader=self.vip_user)
+        self.object = Mediafile.objects.create(title='Title File 1', mediafile=mediafile_path, uploader=self.normal_user)
         os.close(tmpfile_no)
 
     def tearDown(self):
@@ -132,7 +132,7 @@ class MediafileTest(TestCase):
         clients = self.login_clients()
         response = clients['client_manager'].get('/mediafile/1/edit/')
         self.assertContains(response, '---------', status_code=200)
-        self.assertContains(response, '<option value="user:2" selected="selected">mediafile_test_vip_user</option>', status_code=200)
+        self.assertContains(response, '<option value="user:3" selected="selected">mediafile_test_normal_user</option>', status_code=200)
         self.assertTemplateUsed(response, 'mediafile/mediafile_form.html')
         response = clients['client_vip_user'].get('/mediafile/1/edit/')
         self.assertEqual(response.status_code, 403)
@@ -142,6 +142,15 @@ class MediafileTest(TestCase):
         response = bad_client.get('/mediafile/1/edit/')
         self.assertRedirects(response, expected_url='/login/?next=/mediafile/1/edit/', status_code=302, target_status_code=200)
 
+    def test_edit_mediafile_get_request_own_file(self):
+        clients = self.login_clients()
+        self.object.uploader = self.vip_user
+        self.object.save()
+        response = clients['client_vip_user'].get('/mediafile/1/edit/')
+        self.assertNotContains(response, '---------', status_code=200)
+        self.assertNotContains(response, '<option value="user:2" selected="selected">mediafile_test_vip_user</option>', status_code=200)
+        self.assertTemplateUsed(response, 'mediafile/mediafile_form.html')
+
     def test_edit_mediafile_post_request(self):
         # Test only one user
         tmpfile_no, mediafile_2_path = tempfile.mkstemp(prefix='tmp_openslides_test_', dir=self.tmp_dir)
@@ -161,6 +170,31 @@ class MediafileTest(TestCase):
         object_2.mediafile.delete()
         self.assertFalse(os.path.exists(path_2))
 
+    def test_edit_mediafile_post_request_own_file(self):
+        tmpfile_no, mediafile_2_path = tempfile.mkstemp(prefix='tmp_openslides_test_', dir=self.tmp_dir)
+        os.close(tmpfile_no)
+        object_2 = Mediafile.objects.create(title='Title File 2b', mediafile=mediafile_2_path, uploader=self.vip_user)
+        client = self.login_clients()['client_vip_user']
+        new_file_1 = SimpleUploadedFile(name='new_test_file.txt', content='test content hello vip user')
+        response_1 = client.post('/mediafile/2/edit/',
+                                 {'title': 'new_test_file_title_2b',
+                                  'mediafile': new_file_1})
+        self.assertEqual(response_1.status_code, 302)
+        object_2 = Mediafile.objects.get(pk=2)
+        self.assertEqual(object_2.mediafile.url, '/media/file/new_test_file.txt')
+        self.assertEqual(object_2.uploader, self.vip_user)
+        path_2 = object_2.mediafile.path
+        object_2.mediafile.delete()
+        self.assertFalse(os.path.exists(path_2))
+
+    def test_edit_mediafile_post_request_another_file(self):
+        client = self.login_clients()['client_vip_user']
+        new_file_1 = SimpleUploadedFile(name='new_test_file.txt', content='test content hello vip user')
+        response = client.post('/mediafile/1/edit/',
+                               {'title': 'new_test_file_title_2c',
+                                'mediafile': new_file_1})
+        self.assertEqual(response.status_code, 403)
+
     def test_delete_mediafile_get_request(self):
         clients = self.login_clients()
         response = clients['client_manager'].get('/mediafile/1/del/')
@@ -173,6 +207,12 @@ class MediafileTest(TestCase):
         response = bad_client.get('/mediafile/2/del/')
         self.assertRedirects(response, expected_url='/login/?next=/mediafile/2/del/', status_code=302, target_status_code=200)
 
+    def test_delete_mediafile_get_request_own_file(self):
+        self.object.uploader = self.vip_user
+        self.object.save()
+        response = self.login_clients()['client_vip_user'].get('/mediafile/1/del/')
+        self.assertRedirects(response, expected_url='/mediafile/1/edit/', status_code=302, target_status_code=200)
+
     def test_delete_mediafile_post_request(self):
         tmpfile_no, mediafile_3_path = tempfile.mkstemp(prefix='tmp_openslides_test_', dir=self.tmp_dir)
         os.close(tmpfile_no)
@@ -182,6 +222,27 @@ class MediafileTest(TestCase):
         self.assertRedirects(response_1, expected_url='/mediafile/', status_code=302, target_status_code=200)
         self.assertFalse(os.path.exists(object_3.mediafile.path))
 
+    def test_delete_mediafile_post_request_own_file(self):
+        tmpfile_no, mediafile_3_path = tempfile.mkstemp(prefix='tmp_openslides_test_', dir=self.tmp_dir)
+        os.close(tmpfile_no)
+        object_3 = Mediafile.objects.create(title='Title File 3b', mediafile=mediafile_3_path, uploader=self.vip_user)
+        client_1 = self.login_clients()['client_vip_user']
+        response_1 = client_1.post('/mediafile/2/del/', {'yes': 'foo'})
+        self.assertRedirects(response_1, expected_url='/mediafile/', status_code=302, target_status_code=200)
+        self.assertFalse(os.path.exists(object_3.mediafile.path))
+
+    def test_delete_mediafile_post_request_another_file(self):
+        tmpfile_no, mediafile_3_path = tempfile.mkstemp(prefix='tmp_openslides_test_', dir=self.tmp_dir)
+        os.close(tmpfile_no)
+        object_3 = Mediafile.objects.create(title='Title File 3c', mediafile=mediafile_3_path, uploader=self.normal_user)
+        client_1 = self.login_clients()['client_vip_user']
+        response = client_1.post('/mediafile/2/del/', {'yes': 'foo'})
+        self.assertEqual(response.status_code, 403)
+        path_3 = object_3.mediafile.path
+        self.assertTrue(os.path.exists(path_3))
+        object_3.mediafile.delete()
+        self.assertFalse(os.path.exists(path_3))
+
     def test_filesize(self):
         tmpfile_no, mediafile_4_path = tempfile.mkstemp(prefix='tmp_openslides_test_', dir=self.tmp_dir)
         os.close(tmpfile_no)