Merge branch 'registrations-issues' into 'master'

Adjust registrations module

Closes #903, #902, and #901

See merge request !1330

website/registrations/admin.py

@@ -30,10 +30,11 @@ def _show_message(admin, request, n, message, error):
 class RegistrationAdmin(admin.ModelAdmin):
     """Manage the registrations"""
 
-    list_display = ('name', 'email', 'status',
-                    'created_at', 'payment_status')
-    list_filter = ('status', 'programme', 'payment__type',
-                   'payment__amount')
+    list_display = ('name', 'email', 'status', 'membership_type',
+                    'created_at', 'payment_status', 'no_references',
+                    'reference_count')
+    list_filter = ('status', 'programme', 'membership_type', 'no_references',
+                   'payment__type', 'payment__amount')
     inlines = (ReferenceInline,)
     search_fields = ('first_name', 'last_name', 'email', 'phone_number',
                      'student_number',)
@@ -74,6 +75,10 @@ class RegistrationAdmin(admin.ModelAdmin):
     )
     actions = ['accept_selected', 'reject_selected']
 
+    def reference_count(self, obj):
+        return obj.reference_set.count()
+    reference_count.short_description = _('references')
+
     def formfield_for_dbfield(self, db_field, request, **kwargs):
         field = super().formfield_for_dbfield(db_field, request, **kwargs)
         if db_field.name == 'payment':
@@ -176,9 +181,11 @@ class RegistrationAdmin(admin.ModelAdmin):
 class RenewalAdmin(RegistrationAdmin):
     """Manage the renewals"""
 
-    list_display = ('name', 'email', 'status',
-                    'created_at', 'payment_status',)
-    list_filter = ('status', 'payment__type', 'payment__amount')
+    list_display = ('name', 'email', 'status', 'membership_type',
+                    'created_at', 'payment_status', 'no_references',
+                    'reference_count')
+    list_filter = ('status', 'membership_type', 'no_references',
+                   'payment__type', 'payment__amount')
     search_fields = ('member__first_name', 'member__last_name',
                      'member__email', 'member__profile__phone_number',
                      'member__profile__student_number',)
@@ -209,6 +216,7 @@ class RenewalAdmin(RegistrationAdmin):
     @staticmethod
     def name(obj):
         return obj.member.get_full_name()
+    name.short_description = _('name')
 
     @staticmethod
     def email(obj):

website/registrations/forms.py

 """The forms defined by the registrations package"""
 from django import forms
+from django.core.exceptions import NON_FIELD_ERRORS, ValidationError
 from django.forms import TypedChoiceField
 from django.urls import reverse_lazy
 from django.utils import timezone
@@ -7,8 +8,10 @@ from django.utils.safestring import mark_safe
 from django.utils.text import capfirst
 from django.utils.translation import ugettext_lazy as _
 
+from members.models import Membership
+from registrations import services
 from utils.snippets import datetime_to_lectureyear
-from .models import Registration, Renewal
+from .models import Registration, Renewal, Reference
 
 
 class BaseRegistrationForm(forms.ModelForm):
@@ -91,3 +94,29 @@ class RenewalForm(forms.ModelForm):
         fields = '__all__'
         exclude = ['created_at', 'updated_at', 'status',
                    'payment', 'membership']
+
+
+class ReferenceForm(forms.ModelForm):
+    def clean(self):
+        super().clean()
+        membership = self.cleaned_data['member'].current_membership
+        if membership and membership.type == Membership.BENEFACTOR:
+            raise ValidationError(_('Benefactors cannot give '
+                                    'references.'))
+
+        membership = self.cleaned_data['member'].latest_membership
+        if (membership and membership.until and
+                membership.until < services.calculate_membership_since()):
+            raise ValidationError(_("It's not possible to give references for "
+                                    "memberships that start after your own "
+                                    "membership's end."))
+
+    class Meta:
+        model = Reference
+        fields = '__all__'
+        error_messages = {
+            NON_FIELD_ERRORS: {
+                'unique_together':
+                    _("You've already given a reference for this person."),
+            }
+        }

website/registrations/locale/nl/LC_MESSAGES/django.po

@@ -7,8 +7,8 @@ msgid ""
 msgstr ""
 "Project-Id-Version: \n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2019-06-20 17:13+0200\n"
-"PO-Revision-Date: 2019-06-20 17:14+0200\n"
+"POT-Creation-Date: 2019-08-15 20:07+0200\n"
+"PO-Revision-Date: 2019-08-15 20:07+0200\n"
 "Last-Translator: Thom Wiggers <thom@thomwiggers.nl>\n"
 "Language-Team: \n"
 "Language: nl\n"
@@ -34,6 +34,10 @@ msgstr "Adres"
 msgid "University information"
 msgstr "Unversiteitsinformatie"
 
+#: admin.py
+msgid "references"
+msgstr "referenties"
+
 #: admin.py tests/test_admin.py
 msgid "Processed"
 msgstr "Verwerkt"
@@ -68,6 +72,10 @@ msgstr "De geselecteerde registratie(s) konden niet worden goedgekeurd."
 msgid "Accept selected registrations"
 msgstr "Keur geselecteerde registraties goed"
 
+#: admin.py
+msgid "name"
+msgstr "naam"
+
 #: apps.py
 msgid "Registrations"
 msgstr "Registraties"
@@ -112,6 +120,22 @@ msgstr "Ik accepteer het <a href=\"{}\">privacybeleid</a>."
 msgid "I am an employee of iCIS"
 msgstr "Ik ben een medewerker van iCIS"
 
+#: forms.py
+msgid "Benefactors cannot give references."
+msgstr "Begunstigers kunnen geen referenties geven."
+
+#: forms.py
+msgid ""
+"It's not possible to give references for memberships that start after your "
+"own membership's end."
+msgstr ""
+"Het is niet mogelijk een referentie te geven voor lidmaatschappen die "
+"starten na het einde van je eigen lidmaatschap."
+
+#: forms.py templates/registrations/reference_success.html
+msgid "You've already given a reference for this person."
+msgstr "Je hebt deze persoon al een referentie gegeven."
+
 #: models.py
 msgid "created at"
 msgstr "aangemaakt op"
@@ -789,17 +813,12 @@ msgstr ""
 "Thalia worden. Ben je geen oud-Thaliaan, iCIS medewerker of alumni, dan dien "
 "je twee referenties van leden van Thalia te verzamelen.<br /><br /> <strong>"
 "%(name)s</strong> wil graag begunstiger van Thalia worden en heeft jouw "
-"gevraagd om een deze te geven."
+"gevraagd om een referentie te geven."
 
 #: templates/registrations/reference.html
 msgid "Your reference has been saved."
 msgstr "Je referentie is opgeslagen."
 
-#: templates/registrations/reference.html
-#: templates/registrations/reference_success.html
-msgid "You've already given a reference for this person."
-msgstr "Je hebt deze persoon al een referentie gegeven."
-
 #: templates/registrations/register_benefactor.html
 #: templates/registrations/renewal.html
 msgid "Benefactor"

website/registrations/services.py

@@ -310,6 +310,20 @@ def _create_member_from_registration(registration: Registration) -> Member:
     return Member.objects.get(pk=user.pk)
 
 
+def calculate_membership_since() -> timezone.datetime:
+    """
+    Calculate the start date of a membership
+
+    If it's August we act as if it's the next
+    lecture year already and we start new memberships in September
+    :return:
+    """
+    since = timezone.now().date()
+    if timezone.now().month == 8:
+        since = since.replace(month=9, day=1)
+    return since
+
+
 def _create_membership_from_entry(
         entry: Entry, member: Member = None) -> Union[Membership, None]:
     """
@@ -321,13 +335,10 @@ def _create_membership_from_entry(
     :rtype: Membership
     """
     lecture_year = datetime_to_lectureyear(timezone.now())
-    # If it's August we act as if it's the next
-    # lecture year already and we start new memberships in September
-    since = timezone.now().date()
+    since = calculate_membership_since()
+    until = None
     if timezone.now().month == 8:
         lecture_year += 1
-        since = since.replace(month=9, day=1)
-    until = None
 
     if entry.length == Entry.MEMBERSHIP_YEAR:
         # If entry is Renewal set since to current membership until + 1 day

website/registrations/templates/registrations/reference.html

@@ -26,8 +26,9 @@
                 {% trans "Your reference has been saved." as alert_text %}
                 {% alert 'success' alert_text extra_classes="mt-3" %}
             {% elif form.errors %}
-                {% trans "You've already given a reference for this person." as alert_text %}
-                {% alert 'danger' alert_text extra_classes="mt-3" %}
+                {% for error in form.non_field_errors %}
+                    {% alert 'danger' error extra_classes="mt-3" %}
+                {% endfor %}
             {% else %}
             <form method="post" class="">
                 {% csrf_token %}

website/registrations/tests/test_admin.py

@@ -13,7 +13,7 @@ from members.models import Member
 from payments.models import Payment
 from payments.widgets import PaymentWidget
 from registrations import admin
-from registrations.models import Entry, Registration, Renewal
+from registrations.models import Entry, Registration, Renewal, Reference
 
 
 def _get_mock_request(perms=None):
@@ -279,6 +279,17 @@ class RegistrationAdminTest(TestCase):
         )
         self.assertEqual(self.admin.name(reg), reg.get_full_name())
 
+    def test_reference_count(self):
+        reg = Registration.objects.create(
+            first_name='John',
+            last_name='Doe',
+            birthday=timezone.now(),
+        )
+        self.assertEqual(self.admin.reference_count(reg), 0)
+        Reference.objects.create(entry=reg, member=Member.objects.get(pk=1))
+        Reference.objects.create(entry=reg, member=Member.objects.get(pk=2))
+        self.assertEqual(self.admin.reference_count(reg), 2)
+
     def test_payment_status(self):
         reg = Registration(
             username='johnnytest',

website/registrations/tests/test_forms.py

 from django.test import TestCase
 from django.utils import timezone
+from freezegun import freeze_time
 
 from members.models import Member, Membership
 from registrations import forms
-from registrations.models import Entry
+from registrations.models import Entry, Renewal, Reference
 
 
 class MemberRegistrationFormTest(TestCase):
@@ -86,6 +87,7 @@ class RenewalFormTest(TestCase):
 
     def setUp(self):
         self.member = Member.objects.filter(last_name="Wiggers").first()
+        self.member.membership_set.all().delete()
         self.data = {
             'member': self.member.pk,
             'length': Entry.MEMBERSHIP_STUDY,
@@ -94,7 +96,6 @@ class RenewalFormTest(TestCase):
         }
 
     def test_is_valid(self):
-        self.member.membership_set.all().delete()
         with self.subTest("Form is valid"):
             form = forms.RenewalForm(self.data)
             self.assertTrue(form.is_valid(), msg=dict(form.errors))
@@ -106,3 +107,66 @@ class RenewalFormTest(TestCase):
     def test_has_privacy_policy_field(self):
         form = forms.RenewalForm(self.data)
         self.assertTrue(form.fields['privacy_policy'] is not None)
+
+
+class ReferenceFormTest(TestCase):
+    fixtures = ['members.json']
+
+    def setUp(self):
+        self.member = Member.objects.filter(last_name="Wiggers").first()
+        self.member.membership_set.all().delete()
+        self.entry = Renewal.objects.create(member=self.member)
+        self.member.membership_set.all().delete()
+        self.data = {
+            'member': self.member.pk,
+            'entry': self.entry.pk
+        }
+
+    @freeze_time('2018-08-01')
+    def test_clean(self):
+        with self.subTest("Form is valid"):
+            form = forms.ReferenceForm(self.data)
+            self.assertTrue(form.is_valid())
+            form.clean()
+        with self.subTest("Form throws error about benefactor type"):
+            m = Membership.objects.create(
+                type=Membership.BENEFACTOR,
+                user=self.member,
+                since='2017-09-01',
+                until='2018-08-31'
+            )
+            form = forms.ReferenceForm(self.data)
+            self.assertFalse(form.is_valid())
+            self.assertEqual(form.errors, {
+                '__all__': ['Benefactors cannot give references.']
+            })
+            m.delete()
+        with self.subTest("Form throws error about membership end"):
+            m = Membership.objects.create(
+                type=Membership.MEMBER,
+                user=self.member,
+                since='2017-09-01',
+                until='2018-08-31'
+            )
+            form = forms.ReferenceForm(self.data)
+            self.assertFalse(form.is_valid())
+            self.assertEqual(form.errors, {
+                '__all__': [
+                    "It's not possible to give references for "
+                    "memberships that start after your own "
+                    "membership's end."
+                ]
+            })
+            m.delete()
+        with self.subTest('Form throws error about uniqueness'):
+            Reference.objects.create(
+                member=self.member,
+                entry=self.entry
+            )
+            form = forms.ReferenceForm(self.data)
+            self.assertFalse(form.is_valid())
+            self.assertEqual(form.errors, {
+                '__all__': [
+                    "You've already given a reference for this person."
+                ]
+            })

website/registrations/tests/test_views.py

@@ -954,10 +954,10 @@ class ReferenceCreateViewTest(TestCase):
             response.redirect_chain
         )
         self.assertEqual({
-            '__all__': ['Reference with this Member and Entry already exists.']
+            '__all__': ['You\'ve already given a reference for this person.']
         }, response.context['form'].errors)
         self.assertEqual(False, response.context['success'])
         self.assertContains(
             response,
-            'You\'ve already given a reference for this person.'
+            'You've already given a reference for this person.'
         )

website/registrations/views.py

@@ -257,7 +257,7 @@ class ReferenceCreateView(CreateView):
     View that renders a reference creation form
     """
     model = Reference
-    fields = '__all__'
+    form_class = forms.ReferenceForm
     template_name = 'registrations/reference.html'
     entry = None
     success = False