group...feedbackfeed: Bugfix to handle case with uploading file, and removing it to add empty comment.

Author: espenak

devilry/devilry_group/tests/test_feedbackfeed/student/test_feedbackfeed_student.py

@@ -9,6 +9,7 @@
 from django.utils import timezone
 from cradmin_legacy import cradmin_testhelpers
 from model_bakery import baker
+from cradmin_legacy.apps.cradmin_temporaryfileuploadstore.models import TemporaryFileCollection
 
 from devilry.apps.core import models as core_models
 from devilry.devilry_account.models import PeriodUserGuidelineAcceptance
@@ -1175,8 +1176,35 @@ def test_upload_single_file(self):
                 }
             })
         self.assertEqual(1, group_models.GroupComment.objects.count())
+        self.assertEqual(1, TemporaryFileCollection.objects.count())
         self.assertEqual(1, comment_models.CommentFile.objects.count())
 
+    def test_upload_has_temporary_file_collection_id_but_empty_collection(self):
+        # Test that a CommentFile is created on upload.
+        feedbackset = group_baker.feedbackset_first_attempt_unpublished(
+            group__parentnode__parentnode=baker.make_recipe('devilry.apps.core.period_active'))
+        candidate = baker.make('core.Candidate', assignment_group=feedbackset.group)
+        temporary_filecollection = baker.make(
+            'cradmin_temporaryfileuploadstore.TemporaryFileCollection',
+            user=candidate.relatedstudent.user)
+        mockresponse = self.mock_http200_postrequest_htmls(
+            cradmin_role=candidate.assignment_group,
+            requestuser=candidate.relatedstudent.user,
+            viewkwargs={'pk': feedbackset.group.id},
+            requestkwargs={
+                'data': {
+                    'text': '',
+                    'student_add_comment': 'unused value',
+                    'temporary_file_collection_id': temporary_filecollection.id
+                }
+            })
+        self.assertEqual(
+            'A comment must have either text or a file attached, or both. An empty comment is not allowed.',
+            mockresponse.selector.one('#error_1_id_text').alltext_normalized)
+        self.assertEqual(0, group_models.GroupComment.objects.count())
+        self.assertEqual(1, TemporaryFileCollection.objects.count())
+        self.assertEqual(1, group_models.FeedbackSet.objects.count())
+
     def test_upload_single_file_content(self):
         # Test the content of a CommentFile after upload.
         feedbackset = group_baker.feedbackset_first_attempt_unpublished(

devilry/devilry_group/views/cradmin_feedbackfeed_base.py

@@ -59,6 +59,7 @@ class Meta:
     def __init__(self, *args, **kwargs):
         self.group = kwargs.pop('group')
         self.feedback_set = kwargs.pop('feedback_set')
+        self.user = kwargs.pop('user')
         super(GroupCommentForm, self).__init__(*args, **kwargs)
         self.instance.feedback_set = self.feedback_set
 
@@ -75,7 +76,16 @@ class StandardGroupCommentForm(GroupCommentForm):
     """
     def clean(self):
         super(GroupCommentForm, self).clean()
-        if len(self.cleaned_data['text']) == 0 and self.cleaned_data['temporary_file_collection_id'] is None:
+        temporary_file_collection_id = self.cleaned_data['temporary_file_collection_id']
+        temporary_file_collection = None
+        if temporary_file_collection_id is not None:
+            temporary_file_collection = TemporaryFileCollection.objects \
+                .filter_for_user(self.user) \
+                .get(id=temporary_file_collection_id)
+            if temporary_file_collection.files.count() == 0:
+                temporary_file_collection = None
+
+        if len(self.cleaned_data['text']) == 0 and temporary_file_collection is None:
             raise ValidationError({
               'text': gettext_lazy('A comment must have either text or a file attached, or both.'
                                     ' An empty comment is not allowed.')
@@ -183,6 +193,7 @@ def get_form_kwargs(self):
         kwargs = super(FeedbackFeedBaseView, self).get_form_kwargs()
         group = self.assignment_group
         kwargs['group'] = group
+        kwargs['user'] = self.request.user
         kwargs['feedback_set'] = group.cached_data.last_feedbackset
         return kwargs