From ff0cd9b53f463edf5f51e100adae07288152f06e Mon Sep 17 00:00:00 2001
From: Jorran de Wit <jorrandewit@outlook.com>
Date: Sun, 4 Jun 2017 20:40:33 +0200
Subject: [PATCH] Improve manager/constant use

---
 submissions/constants.py                      | 15 +++++++-------
 submissions/forms.py                          |  3 +--
 submissions/managers.py                       |  7 +++++--
 .../migrations/0044_auto_20170602_1836.py     | 20 +++++++++++++++++++
 submissions/models.py                         | 19 +++++++++---------
 submissions/test_views.py                     | 13 +++++++-----
 submissions/utils.py                          | 10 ++++++----
 submissions/views.py                          | 14 ++++++-------
 8 files changed, 63 insertions(+), 38 deletions(-)
 create mode 100644 submissions/migrations/0044_auto_20170602_1836.py

diff --git a/submissions/constants.py b/submissions/constants.py
index 84638110f..706bc1333 100644
--- a/submissions/constants.py
+++ b/submissions/constants.py
@@ -163,14 +163,13 @@ REPORT_ACTION_CHOICES = (
     (REPORT_ACTION_REFUSE, 'refuse'),
 )
 
-STATUS_VETTED = 1
-STATUS_UNVETTED = 0
-STATUS_UNCLEAR = -1
-STATUS_INCORRECT = -2
-STATUS_NOT_USEFUL = -3
-STATUS_NOT_ACADEMIC = -4
-
-REPORT_REFUSAL_NONE = 0
+STATUS_VETTED = 'vetted'
+STATUS_UNVETTED = 'unvetted'
+STATUS_UNCLEAR = 'unclear'
+STATUS_INCORRECT = 'incorrect'
+STATUS_NOT_USEFUL = 'notuseful'
+STATUS_NOT_ACADEMIC = 'notacademic'
+
 REPORT_REFUSAL_CHOICES = (
     (STATUS_UNVETTED, '-'),
     (STATUS_UNCLEAR, 'insufficiently clear'),
diff --git a/submissions/forms.py b/submissions/forms.py
index 4b6fdcc84..2f79c675e 100644
--- a/submissions/forms.py
+++ b/submissions/forms.py
@@ -1,7 +1,6 @@
 from django import forms
 from django.contrib.auth.models import Group
-from django.core.validators import RegexValidator
-from django.db import models, transaction
+from django.db import transaction
 
 from guardian.shortcuts import assign_perm
 
diff --git a/submissions/managers.py b/submissions/managers.py
index f1ce5d8a9..d68abaa5b 100644
--- a/submissions/managers.py
+++ b/submissions/managers.py
@@ -4,7 +4,7 @@ from django.db.models import Q
 from .constants import SUBMISSION_STATUS_OUT_OF_POOL, SUBMISSION_STATUS_PUBLICLY_UNLISTED,\
                        SUBMISSION_STATUS_PUBLICLY_INVISIBLE, STATUS_UNVETTED, STATUS_VETTED,\
                        STATUS_UNCLEAR, STATUS_INCORRECT, STATUS_NOT_USEFUL, STATUS_NOT_ACADEMIC,\
-                       SUBMISSION_HTTP404_ON_EDITORIAL_PAGE
+                       SUBMISSION_HTTP404_ON_EDITORIAL_PAGE#, STATUS_DRAFT
 
 
 class SubmissionManager(models.Manager):
@@ -110,7 +110,7 @@ class EICRecommendationManager(models.Manager):
 
 class ReportManager(models.Manager):
     def accepted(self):
-        return self.filter(status__gte=STATUS_VETTED)
+        return self.filter(status=STATUS_VETTED)
 
     def awaiting_vetting(self):
         return self.filter(status=STATUS_UNVETTED)
@@ -118,3 +118,6 @@ class ReportManager(models.Manager):
     def rejected(self):
         return self.filter(status__in=[STATUS_UNCLEAR, STATUS_INCORRECT,
                                        STATUS_NOT_USEFUL, STATUS_NOT_ACADEMIC])
+
+    # def in_draft(self):
+    #     return self.filter(status=STATUS_DRAFT)
diff --git a/submissions/migrations/0044_auto_20170602_1836.py b/submissions/migrations/0044_auto_20170602_1836.py
new file mode 100644
index 000000000..472d624fc
--- /dev/null
+++ b/submissions/migrations/0044_auto_20170602_1836.py
@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.3 on 2017-06-02 16:36
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('submissions', '0043_auto_20170512_0836'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='report',
+            name='status',
+            field=models.CharField(choices=[('vetted', 'Vetted'), ('unvetted', 'Unvetted'), ('incorrect', 'Rejected (incorrect)'), ('unclear', 'Rejected (unclear)'), ('notuseful', 'Rejected (not useful)'), ('notacademic', 'Rejected (not academic in style)')], default='unvetted', max_length=16),
+        ),
+    ]
diff --git a/submissions/models.py b/submissions/models.py
index 6bf5ae9c6..a5272e5ea 100644
--- a/submissions/models.py
+++ b/submissions/models.py
@@ -1,16 +1,15 @@
 import datetime
 
 from django.utils import timezone
-from django.db import models, transaction
+from django.db import models
 from django.contrib.postgres.fields import JSONField
 from django.urls import reverse
 
 from .constants import ASSIGNMENT_REFUSAL_REASONS, ASSIGNMENT_NULLBOOL,\
                        SUBMISSION_TYPE, ED_COMM_CHOICES, REFEREE_QUALIFICATION, QUALITY_SPEC,\
                        RANKING_CHOICES, REPORT_REC, SUBMISSION_STATUS, STATUS_UNASSIGNED,\
-                       REPORT_STATUSES, STATUS_UNVETTED, STATUS_RESUBMISSION_INCOMING,\
-                       SUBMISSION_CYCLES, CYCLE_DEFAULT, CYCLE_SHORT, CYCLE_DIRECT_REC,\
-                       SUBMISSION_EIC_RECOMMENDATION_REQUIRED
+                       REPORT_STATUSES, STATUS_UNVETTED, SUBMISSION_EIC_RECOMMENDATION_REQUIRED,\
+                       SUBMISSION_CYCLES, CYCLE_DEFAULT, CYCLE_SHORT, CYCLE_DIRECT_REC
 from .managers import SubmissionManager, EditorialAssignmentManager, EICRecommendationManager,\
                       ReportManager
 from .utils import ShortSubmissionCycle, DirectRecommendationSubmissionCycle,\
@@ -148,19 +147,19 @@ class Submission(ArxivCallable, models.Model):
         return self.referee_invitations.filter(accepted=None).count()
 
     def count_invited_reports(self):
-        return self.reports.filter(status=1, invited=True).count()
+        return self.reports.accepted().filter(invited=True).count()
 
     def count_contrib_reports(self):
-        return self.reports.filter(status=1, invited=False).count()
+        return self.reports.accepted().filter(invited=False).count()
 
     def count_obtained_reports(self):
-        return self.reports.filter(status=1, invited__isnull=False).count()
+        return self.reports.accepted().filter(invited__isnull=False).count()
 
     def count_refused_reports(self):
-        return self.reports.filter(status__lte=-1).count()
+        return self.reports.rejected().count()
 
     def count_awaiting_vetting(self):
-        return self.reports.filter(status=0).count()
+        return self.reports.awaiting_vetting().count()
 
 
 ######################
@@ -236,7 +235,7 @@ class RefereeInvitation(models.Model):
 
 class Report(models.Model):
     """ Both types of reports, invited or contributed. """
-    status = models.SmallIntegerField(choices=REPORT_STATUSES, default=STATUS_UNVETTED)
+    status = models.CharField(max_length=16, choices=REPORT_STATUSES, default=STATUS_UNVETTED)
     submission = models.ForeignKey('submissions.Submission', related_name='reports',
                                    on_delete=models.CASCADE)
     vetted_by = models.ForeignKey('scipost.Contributor', related_name="report_vetted_by",
diff --git a/submissions/test_views.py b/submissions/test_views.py
index 957c01159..bf64009bd 100644
--- a/submissions/test_views.py
+++ b/submissions/test_views.py
@@ -78,6 +78,8 @@ class PrefillUsingIdentifierTest(BaseContributorTestCase):
 
         # Registered Contributor should get 200
         response = self.client.get(self.url)
+        self.assertIsInstance(response.context['form'], SubmissionIdentifierForm)
+        self.assertFalse(response.context['form'].is_valid())
         self.assertEqual(response.status_code, 200)
 
     def test_retrieving_existing_arxiv_paper(self):
@@ -87,12 +89,10 @@ class PrefillUsingIdentifierTest(BaseContributorTestCase):
                                         TEST_SUBMISSION['arxiv_identifier_w_vn_nr']})
         self.assertEqual(response.status_code, 200)
         self.assertIsInstance(response.context['form'], RequestSubmissionForm)
-        self.assertIsInstance(response.context['identifierform'], SubmissionIdentifierForm)
-        self.assertTrue(response.context['identifierform'].is_valid())
 
         # Explicitly compare fields instead of assertDictEqual as metadata field may be outdated
-        self.assertEqual(TEST_SUBMISSION['is_resubmission'],
-                         response.context['form'].initial['is_resubmission'])
+        # self.assertEqual(TEST_SUBMISSION['is_resubmission'],
+        #                  response.context['form'].initial['is_resubmission'])
         self.assertEqual(TEST_SUBMISSION['title'], response.context['form'].initial['title'])
         self.assertEqual(TEST_SUBMISSION['author_list'],
                          response.context['form'].initial['author_list'])
@@ -180,7 +180,10 @@ class SubmitManuscriptTest(BaseContributorTestCase):
 
         # Submit new Submission form
         response = client.post(reverse('submissions:submit_manuscript'), params)
-        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.status_code, 200)
+        self.assertIsInstance(response.context['form'], RequestSubmissionForm)
+        self.assertFalse(response.context['form'].is_valid())
+        print(response.context['form'].errors.as_data())
 
         # No real check is done here to see if submission submit is aborted.
         # To be implemented after Arxiv caller.
diff --git a/submissions/utils.py b/submissions/utils.py
index 29ed7e85b..302496959 100644
--- a/submissions/utils.py
+++ b/submissions/utils.py
@@ -4,7 +4,8 @@ from django.core.mail import EmailMessage, EmailMultiAlternatives
 from django.template import Context, Template
 from django.utils import timezone
 
-from .constants import NO_REQUIRED_ACTION_STATUSES,\
+from .constants import NO_REQUIRED_ACTION_STATUSES, STATUS_VETTED, STATUS_UNCLEAR,\
+                       STATUS_INCORRECT, STATUS_NOT_USEFUL, STATUS_NOT_ACADEMIC,\
                        STATUS_REVISION_REQUESTED, STATUS_EIC_ASSIGNED,\
                        STATUS_RESUBMISSION_INCOMING, STATUS_AWAITING_ED_REC
 from .exceptions import CycleUpdateDeadlineError
@@ -1023,7 +1024,7 @@ class SubmissionUtils(BaseMailUtil):
             '<p>Dear {{ ref_title }} {{ ref_last_name }},</p>'
             '<p>Many thanks for your Report on Submission</p>'
             '<p>{{ sub_title }}</p>\n<p>by {{ author_list }}.</p>')
-        if cls.report.status == 1:
+        if cls.report.status == STATUS_VETTED:
             email_text += ('\n\nYour Report has been vetted through and is viewable at '
                            'https://scipost.org/submissions/'
                            + cls.report.submission.arxiv_identifier_w_vn_nr + '.')
@@ -1047,7 +1048,7 @@ class SubmissionUtils(BaseMailUtil):
                        '\n\nThe SciPost Team.')
         email_text_html += ('<p>Many thanks for your collaboration,</p>'
                             '<p>The SciPost Team.</p>')
-        if cls.report.status != 1:
+        if cls.report.status != STATUS_VETTED:
             if cls.email_response is not None:
                 email_text += '\n\nAdditional info from the Editor-in-charge: \n'
                 email_text += cls.email_response
@@ -1078,7 +1079,8 @@ class SubmissionUtils(BaseMailUtil):
             'requested_changes': cls.report.requested_changes,
             'remarks_for_editors': cls.report.remarks_for_editors,
         })
-        if cls.report.status < 0:
+        if cls.report.status in [STATUS_UNCLEAR, STATUS_INCORRECT,
+                                 STATUS_NOT_USEFUL, STATUS_NOT_ACADEMIC]:
             email_context['refusal_reason'] = cls.report.get_status_display()
         email_text_html += '<br/>' + EMAIL_FOOTER
         html_template = Template(email_text_html)
diff --git a/submissions/views.py b/submissions/views.py
index a3764b96f..17f455338 100644
--- a/submissions/views.py
+++ b/submissions/views.py
@@ -520,8 +520,8 @@ def editorial_page(request, arxiv_identifier_w_vn_nr):
                       .filter(arxiv_identifier_wo_vn_nr=submission.arxiv_identifier_wo_vn_nr)
                       .exclude(pk=submission.id))
     ref_invitations = RefereeInvitation.objects.filter(submission=submission)
-    nr_reports_to_vet = (Report.objects
-                         .filter(status=0, submission=submission,
+    nr_reports_to_vet = (Report.objects.awaiting_vetting()
+                         .filter(submission=submission,
                                  submission__editor_in_charge=request.user.contributor)
                          .count())
     communications = (EditorialCommunication.objects
@@ -1050,8 +1050,8 @@ def submit_report(request, arxiv_identifier_w_vn_nr):
 @permission_required('scipost.can_take_charge_of_submissions', raise_exception=True)
 def vet_submitted_reports(request):
     contributor = Contributor.objects.get(user=request.user)
-    report_to_vet = Report.objects.filter(status=0,
-                                          submission__editor_in_charge=contributor).first()
+    report_to_vet = (Report.objects.awaiting_vetting()
+                     .filter(submission__editor_in_charge=contributor).first())
     form = VetReportForm()
     context = {'contributor': contributor, 'report_to_vet': report_to_vet, 'form': form}
     return(render(request, 'submissions/vet_submitted_reports.html', context))
@@ -1067,19 +1067,19 @@ def vet_submitted_report_ack(request, report_id):
         report.vetted_by = request.user.contributor
         if form.cleaned_data['action_option'] == '1':
             # accept the report as is
-            report.status = 1
+            report.status = STATUS_VETTED
             report.save()
             report.submission.latest_activity = timezone.now()
             report.submission.save()
         elif form.cleaned_data['action_option'] == '2':
             # the report is simply rejected
-            report.status = int(form.cleaned_data['refusal_reason'])
+            report.status = form.cleaned_data['refusal_reason']
             report.save()
         # email report author
         SubmissionUtils.load({'report': report,
                               'email_response': form.cleaned_data['email_response_field']})
         SubmissionUtils.acknowledge_report_email()  # email report author, bcc EIC
-        if report.status == 1:
+        if report.status == STATUS_VETTED:
             SubmissionUtils.send_author_report_received_email()
         messages.success(request, 'Submitted Report vetted.')
         return redirect(reverse('submissions:editorial_page',
-- 
GitLab