From 94d524c1b6bbfc48461f4e2079d91bf575a6b60e Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Wed, 13 Jul 2016 15:23:45 +0200 Subject: [PATCH 1/2] If chair value changes, create new membership See #11 --- website/committees/models.py | 48 +++++++++++++++++++++++------- website/committees/tests.py | 57 +++++++++++++++++++++++------------- 2 files changed, 75 insertions(+), 30 deletions(-) diff --git a/website/committees/models.py b/website/committees/models.py index 39fd8e69..19259a9c 100644 --- a/website/committees/models.py +++ b/website/committees/models.py @@ -1,13 +1,17 @@ -from django.utils import timezone +import logging from django.core.exceptions import ValidationError, NON_FIELD_ERRORS from django.contrib.auth.models import Permission from django.db import models +from django.utils import timezone from django.utils.translation import ugettext_lazy as _ from members.models import Member +logger = logging.getLogger(__name__) + + class Committee(models.Model): """A committee""" @@ -48,7 +52,7 @@ class ActiveMembershipManager(models.Manager): """Get only active memberships""" def get_queryset(self): """Get the currently active committee memberships""" - return super().get_queryset().exclude(until__lt=timezone.now()) + return super().get_queryset().exclude(until__lt=timezone.now().date()) class CommitteeMembership(models.Model): @@ -87,14 +91,22 @@ class CommitteeMembership(models.Model): default=False, ) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + if self.pk is None: + self._was_chair = bool(self.chair) + else: + self._was_chair = False + @property def is_active(self): """Is this membership currently active""" - return self.until is None or self.until > timezone.now() + return self.until is None or self.until > timezone.now().date() def clean(self): """Validation""" - if self.until and self.until > timezone.now(): + if self.until and self.until > timezone.now().date(): raise ValidationError({ 'until': _("Membership expiration date can't be in the future:" " '{}'").format(self.until) @@ -110,6 +122,7 @@ class CommitteeMembership(models.Model): # Check if a committee has more than one chair chairs = (CommitteeMembership.active_memberships .filter(committee=self.committee) + .exclude(member=self.member) .filter(chair=True) .count()) if chairs >= 1 and self.chair: @@ -118,12 +131,27 @@ class CommitteeMembership(models.Model): _('This committee already has a chair')}) # check if this member is already in the committee - members = (self.committee.members - .filter(pk=self.member.pk) - .count()) - if members >= 1: - raise ValidationError({ - 'member': _('This member is already in the committee')}) + if self.pk is None: + members = (self.committee.members + .filter(pk=self.member.pk) + .count()) + if members >= 1: + raise ValidationError({ + 'member': _('This member is already in the committee')}) + + def save(self, *args, **kwargs): + """Save the instance""" + # If the chair changed and we're still active, we create a new instance + # Inactive instances should be handled manually + if (self.pk is not None and self._was_chair != self.chair and + not self.until): + logger.info("Creating new membership instance") + self.until = timezone.now().date() + super().save(*args, **kwargs) + self.pk = None # forces INSERT + self.since = self.until # Set since date to older expiration + self.until = None + super().save(*args, **kwargs) def __str__(self): return "{} membership of {} since {}".format(self.member, diff --git a/website/committees/tests.py b/website/committees/tests.py index b0d8469e..94e2e6ca 100644 --- a/website/committees/tests.py +++ b/website/committees/tests.py @@ -43,56 +43,73 @@ class CommitteeMembersTest(TestCase): def test_until_date(self): m = CommitteeMembership(committee=self.testcie, member=self.testuser, - until=timezone.now().replace(year=2000), + until=timezone.now().date().replace(year=2000), chair=False) with self.assertRaises(ValidationError): m.clean() - m.since = timezone.now().replace(year=1900) + m.since = timezone.now().date().replace(year=1900) m.clean() def test_inactive(self): self.assertTrue(self.m.is_active) - self.m.until = timezone.now().replace(year=1900) + self.m.until = timezone.now().date().replace(year=1900) self.assertFalse(self.m.is_active) class CommitteeMembersChairTest(TestCase): fixtures = ['members.json', 'committees.json'] - @classmethod - def setUpTestData(cls): - testcie = Committee.objects.get(name='testcie1') - testuser = Member.objects.get(pk=1) - cls.m1 = CommitteeMembership(committee=testcie, - member=testuser, - chair=True) - cls.m1.full_clean() - cls.m1.save() - def setUp(self): self.testcie = Committee.objects.get(name='testcie1') self.testuser = Member.objects.get(pk=1) + self.testuser2 = Member.objects.get(pk=2) + self.m1 = CommitteeMembership(committee=self.testcie, + member=self.testuser, + chair=True) + self.m1.full_clean() + self.m1.save() def test_second_chair_fails(self): - testuser2 = Member.objects.get(pk=2) - m = CommitteeMembership(committee=self.testcie, - member=testuser2, + member=self.testuser2, chair=True) with self.assertRaises(ValidationError): m.full_clean() def test_inactive_chair(self): - testuser2 = Member.objects.get(pk=2) - self.m1.until = timezone.now().replace(year=1900) + self.m1.until = timezone.now().date().replace(year=1900) self.m1.save() + m = CommitteeMembership(committee=self.testcie, - member=testuser2, + member=self.testuser2, chair=True) m.full_clean() + def test_clean_self_chair(self): + self.m1.chair = True + self.m1.full_clean() + + def test_change_chair(self): + pk = self.m1.pk + original_chair = self.m1.chair + self.m1.save() + self.assertEqual(self.m1.pk, pk, "new object created") + self.m1.chair = not original_chair + self.m1.save() + self.assertNotEqual(self.m1.pk, pk, "No new object created") + + def test_change_chair_inactive(self): + pk = self.m1.pk + original_chair = self.m1.chair + self.m1.until = timezone.now().date() + self.m1.save() + self.assertEqual(self.m1.pk, pk, "new object created") + self.m1.chair = not original_chair + self.m1.save() + self.assertEqual(self.m1.pk, pk, "No new object created") + -class BackendTest(TestCase): +class PermissionsBackendTest(TestCase): fixtures = ['members.json', 'committees.json'] @classmethod -- GitLab From 9c7f490bd75d365888c0897b35508fbbd4baf9d6 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Wed, 13 Jul 2016 15:32:31 +0200 Subject: [PATCH 2/2] Make delete() first deactivate committee memberships See #11 --- website/committees/models.py | 8 ++++++++ website/committees/tests.py | 24 +++++++++++++++--------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/website/committees/models.py b/website/committees/models.py index 19259a9c..e4ea84e8 100644 --- a/website/committees/models.py +++ b/website/committees/models.py @@ -153,6 +153,14 @@ class CommitteeMembership(models.Model): self.until = None super().save(*args, **kwargs) + def delete(self, *args, **kwargs): + """Deactivates active memberships, deletes inactive ones""" + if self.is_active: + self.until = timezone.now().date() + self.save() + else: + super().delete(*args, **kwargs) + def __str__(self): return "{} membership of {} since {}".format(self.member, self.committee, diff --git a/website/committees/tests.py b/website/committees/tests.py index 94e2e6ca..9c645544 100644 --- a/website/committees/tests.py +++ b/website/committees/tests.py @@ -11,15 +11,14 @@ from members.models import Member class CommitteeMembersTest(TestCase): fixtures = ['members.json', 'committees.json'] - @classmethod - def setUpTestData(cls): - cls.testcie = Committee.objects.get(name='testcie1') - cls.testuser = Member.objects.get(pk=1) - - cls.m = CommitteeMembership(committee=cls.testcie, - member=cls.testuser, - chair=False) - cls.m.save() + def setUp(self): + # Don't use setUpTestData because delete() will cause problems + self.testcie = Committee.objects.get(name='testcie1') + self.testuser = Member.objects.get(pk=1) + self.m = CommitteeMembership(committee=self.testcie, + member=self.testuser, + chair=False) + self.m.save() def test_unique(self): with self.assertRaises(IntegrityError): @@ -55,6 +54,13 @@ class CommitteeMembersTest(TestCase): self.m.until = timezone.now().date().replace(year=1900) self.assertFalse(self.m.is_active) + def test_delete(self): + self.m.delete() + self.assertIsNotNone(self.m.until) + self.assertIsNotNone(self.m.pk) + self.m.delete() + self.assertIsNone(self.m.pk) + class CommitteeMembersChairTest(TestCase): fixtures = ['members.json', 'committees.json'] -- GitLab