From ca7ad0574661e076b2cd6060a7bc70558ec2d3ea Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Sun, 4 Oct 2020 20:26:43 +0200 Subject: [PATCH 1/5] Use a signal to prevent a user that the note balance is negative --- apps/note/apps.py | 11 ++++++++++- apps/note/models/notes.py | 28 ---------------------------- apps/note/signals.py | 12 ++++++++++++ 3 files changed, 22 insertions(+), 29 deletions(-) diff --git a/apps/note/apps.py b/apps/note/apps.py index b3dc5a0f..af1dcc12 100644 --- a/apps/note/apps.py +++ b/apps/note/apps.py @@ -3,7 +3,7 @@ from django.apps import AppConfig from django.conf import settings -from django.db.models.signals import post_save, pre_delete +from django.db.models.signals import pre_delete, pre_save, post_save from django.utils.translation import gettext_lazy as _ from . import signals @@ -17,6 +17,15 @@ class NoteConfig(AppConfig): """ Define app internal signals to interact with other apps """ + pre_save.connect( + signals.pre_save_note, + sender="note.noteuser", + ) + pre_save.connect( + signals.pre_save_note, + sender="note.noteclub", + ) + post_save.connect( signals.save_user_note, sender=settings.AUTH_USER_MODEL, diff --git a/apps/note/models/notes.py b/apps/note/models/notes.py index 49b9fd58..c649dbc9 100644 --- a/apps/note/models/notes.py +++ b/apps/note/models/notes.py @@ -159,20 +159,6 @@ class NoteUser(Note): def pretty(self): return _("%(user)s's note") % {'user': str(self.user)} - @transaction.atomic - def save(self, *args, **kwargs): - if self.pk and self.balance < 0: - old_note = NoteUser.objects.get(pk=self.pk) - super().save(*args, **kwargs) - if old_note.balance >= 0: - # Passage en négatif - self.last_negative = timezone.now() - self._force_save = True - self.save(*args, **kwargs) - self.send_mail_negative_balance() - else: - super().save(*args, **kwargs) - def send_mail_negative_balance(self): plain_text = render_to_string("note/mails/negative_balance.txt", dict(note=self)) html = render_to_string("note/mails/negative_balance.html", dict(note=self)) @@ -201,20 +187,6 @@ class NoteClub(Note): def pretty(self): return _("Note of %(club)s club") % {'club': str(self.club)} - @transaction.atomic - def save(self, *args, **kwargs): - if self.pk and self.balance < 0: - old_note = NoteClub.objects.get(pk=self.pk) - super().save(*args, **kwargs) - if old_note.balance >= 0: - # Passage en négatif - self.last_negative = timezone.now() - self._force_save = True - self.save(*args, **kwargs) - self.send_mail_negative_balance() - else: - super().save(*args, **kwargs) - def send_mail_negative_balance(self): plain_text = render_to_string("note/mails/negative_balance.txt", dict(note=self)) html = render_to_string("note/mails/negative_balance.html", dict(note=self)) diff --git a/apps/note/signals.py b/apps/note/signals.py index c1545ec2..8c02b3a5 100644 --- a/apps/note/signals.py +++ b/apps/note/signals.py @@ -1,6 +1,8 @@ # Copyright (C) 2018-2020 by BDE ENS Paris-Saclay # SPDX-License-Identifier: GPL-3.0-or-later +from django.utils import timezone + def save_user_note(instance, raw, **_kwargs): """ @@ -25,6 +27,16 @@ def save_club_note(instance, raw, **_kwargs): instance.note.save() +def pre_save_note(instance, raw, **_kwargs): + if not raw and instance.pk and not hasattr(instance, "_no_signal") and instance.balance < 0: + from note.models import Note + old_note = Note.objects.get(pk=instance.pk) + if old_note.balance >= 0: + # Passage en négatif + instance.last_negative = timezone.now() + instance.send_mail_negative_balance() + + def delete_transaction(instance, **_kwargs): """ Whenever we want to delete a transaction (caution with this), we ensure the transaction is invalid first. From f22e92132c12a8ecb86675b4d956212e688d5c3e Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Sun, 4 Oct 2020 20:47:15 +0200 Subject: [PATCH 2/5] Select for update transaction notes, and not only the transaction --- apps/note/models/transactions.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/note/models/transactions.py b/apps/note/models/transactions.py index bfe39a42..e0aa30f4 100644 --- a/apps/note/models/transactions.py +++ b/apps/note/models/transactions.py @@ -3,6 +3,7 @@ from django.core.exceptions import ValidationError from django.db import models, transaction +from django.db.models import F from django.urls import reverse from django.utils import timezone from django.utils.translation import gettext_lazy as _ @@ -217,6 +218,9 @@ class Transaction(PolymorphicModel): # When source == destination, no money is transferred and no transaction is created return + self.source = Note.objects.select_for_update().get(pk=self.source_id) + self.destination = Note.objects.select_for_update().get(pk=self.destination_id) + # Check that the amounts stay between big integer bounds diff_source, diff_dest = self.validate() From d6661790377ae441c798bbe88d99026ddf569f56 Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Sun, 4 Oct 2020 20:50:10 +0200 Subject: [PATCH 3/5] Display Renew membership button 15 days more --- apps/member/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/member/views.py b/apps/member/views.py index 5cce34f5..90dba96b 100644 --- a/apps/member/views.py +++ b/apps/member/views.py @@ -157,7 +157,7 @@ class UserDetailView(ProtectQuerysetMixin, LoginRequiredMixin, DetailView): history_table.paginate(per_page=20, page=self.request.GET.get("transaction-page", 1)) context['history_list'] = history_table - club_list = Membership.objects.filter(user=user, date_end__gte=date.today())\ + club_list = Membership.objects.filter(user=user, date_end__gte=date.today() - timedelta(days=15))\ .filter(PermissionBackend.filter_queryset(self.request.user, Membership, "view")) membership_table = MembershipTable(data=club_list, prefix='membership-') membership_table.paginate(per_page=10, page=self.request.GET.get("membership-page", 1)) @@ -409,7 +409,7 @@ class ClubDetailView(ProtectQuerysetMixin, LoginRequiredMixin, DetailView): # member list club_member = Membership.objects.filter( club=club, - date_end__gte=date.today(), + date_end__gte=date.today() - timedelta(days=15), ).filter(PermissionBackend.filter_queryset(self.request.user, Membership, "view")) membership_table = MembershipTable(data=club_member, prefix="membership-") From e172b4f4bb4df4e0da5700a3b4a994df53a2b89b Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Sun, 4 Oct 2020 20:54:03 +0200 Subject: [PATCH 4/5] When a membership is renewed, set the same roles as the previous membership --- apps/member/views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/member/views.py b/apps/member/views.py index 90dba96b..ab4a5bc7 100644 --- a/apps/member/views.py +++ b/apps/member/views.py @@ -680,6 +680,7 @@ class ClubAddMemberView(ProtectQuerysetMixin, ProtectedCreateView): club = Club.objects.filter(PermissionBackend.filter_queryset(self.request.user, Club, "view")) \ .get(pk=self.kwargs["club_pk"]) user = form.instance.user + old_membership = None else: # get from url for renewal old_membership = self.get_queryset().get(pk=self.kwargs["pk"]) club = old_membership.club @@ -754,6 +755,9 @@ class ClubAddMemberView(ProtectQuerysetMixin, ProtectedCreateView): member_role = Role.objects.filter(Q(name="Adhérent BDE") | Q(name="Membre de club")).all() \ if club.name == "BDE" else Role.objects.filter(Q(name="Adhérent Kfet") | Q(name="Membre de club")).all() \ if club.name == "Kfet"else Role.objects.filter(name="Membre de club").all() + # Set the same roles as before + if old_membership: + member_role = member_role.union(old_membership.roles.all()) form.instance.roles.set(member_role) form.instance._force_save = True form.instance.save() @@ -791,7 +795,7 @@ class ClubAddMemberView(ProtectQuerysetMixin, ProtectedCreateView): return ret def get_success_url(self): - return reverse_lazy('member:club_detail', kwargs={'pk': self.object.club.id}) + return reverse_lazy('member:user_detail', kwargs={'pk': self.object.user.id}) class ClubManageRolesView(ProtectQuerysetMixin, LoginRequiredMixin, UpdateView): From 541ed59f40deccf1a8f9b86ace2ff7a8c1e6ae81 Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Sun, 4 Oct 2020 21:08:35 +0200 Subject: [PATCH 5/5] When a membership is created, redirect to the user profile page rather than club detail --- apps/member/tests/test_memberships.py | 8 ++++---- apps/note/models/transactions.py | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/apps/member/tests/test_memberships.py b/apps/member/tests/test_memberships.py index 90b1f382..80c214f0 100644 --- a/apps/member/tests/test_memberships.py +++ b/apps/member/tests/test_memberships.py @@ -205,7 +205,7 @@ class TestMemberships(TestCase): first_name="Toto", bank="Le matelas", )) - self.assertRedirects(response, club.get_absolute_url(), 302, 200) + self.assertRedirects(response, user.profile.get_absolute_url(), 302, 200) self.assertTrue(Membership.objects.filter(user=user, club=club).exists()) @@ -244,9 +244,9 @@ class TestMemberships(TestCase): first_name="Toto", bank="Bank", )) - self.assertRedirects(response, club.get_absolute_url(), 302, 200) + self.assertRedirects(response, user.profile.get_absolute_url(), 302, 200) - response = self.client.get(user.profile.get_absolute_url()) + response = self.client.get(club.get_absolute_url()) self.assertEqual(response.status_code, 200) def test_auto_join_kfet_when_join_bde_with_soge(self): @@ -273,7 +273,7 @@ class TestMemberships(TestCase): first_name="Toto", bank="Société générale", )) - self.assertRedirects(response, bde.get_absolute_url(), 302, 200) + self.assertRedirects(response, user.profile.get_absolute_url(), 302, 200) self.assertTrue(Membership.objects.filter(user=user, club=bde).exists()) self.assertTrue(Membership.objects.filter(user=user, club=kfet).exists()) diff --git a/apps/note/models/transactions.py b/apps/note/models/transactions.py index e0aa30f4..b204e623 100644 --- a/apps/note/models/transactions.py +++ b/apps/note/models/transactions.py @@ -3,7 +3,6 @@ from django.core.exceptions import ValidationError from django.db import models, transaction -from django.db.models import F from django.urls import reverse from django.utils import timezone from django.utils.translation import gettext_lazy as _