Merge branch 'atomicity' into 'beta'

Atomicité

See merge request bde/nk20!122

apps/activity/models.py

@@ -7,7 +7,7 @@ from threading import Thread
 
 from django.conf import settings
 from django.contrib.auth.models import User
-from django.db import models
+from django.db import models, transaction
 from django.db.models import Q
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
@@ -123,6 +123,7 @@ class Activity(models.Model):
         verbose_name=_('open'),
     )
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         """
         Update the activity wiki page each time the activity is updated (validation, change description, ...)
@@ -194,8 +195,8 @@ class Entry(models.Model):
             else _("Entry for {note} to the activity {activity}").format(
             guest=str(self.guest), note=str(self.note), activity=str(self.activity))
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
-
         qs = Entry.objects.filter(~Q(pk=self.pk), activity=self.activity, note=self.note, guest=self.guest)
         if qs.exists():
             raise ValidationError(_("Already entered on ") + _("{:%Y-%m-%d %H:%M:%S}").format(qs.get().time, ))
@@ -260,6 +261,7 @@ class Guest(models.Model):
         except AttributeError:
             return False
 
+    @transaction.atomic
     def save(self, force_insert=False, force_update=False, using=None, update_fields=None):
         one_year = timedelta(days=365)
 

apps/activity/views.py

@@ -7,6 +7,7 @@ from django.conf import settings
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import PermissionDenied
+from django.db import transaction
 from django.db.models import F, Q
 from django.http import HttpResponse
 from django.urls import reverse_lazy
@@ -44,6 +45,7 @@ class ActivityCreateView(ProtectQuerysetMixin, ProtectedCreateView):
             date_end=timezone.now(),
         )
 
+    @transaction.atomic
     def form_valid(self, form):
         form.instance.creater = self.request.user
         return super().form_valid(form)
@@ -145,6 +147,7 @@ class ActivityInviteView(ProtectQuerysetMixin, ProtectedCreateView):
         form.fields["inviter"].initial = self.request.user.note
         return form
 
+    @transaction.atomic
     def form_valid(self, form):
         form.instance.activity = Activity.objects\
             .filter(PermissionBackend.filter_queryset(self.request.user, Activity, "view")).get(pk=self.kwargs["pk"])

apps/member/forms.py

@@ -8,6 +8,7 @@ from django import forms
 from django.conf import settings
 from django.contrib.auth.forms import AuthenticationForm
 from django.contrib.auth.models import User
+from django.db import transaction
 from django.forms import CheckboxSelectMultiple
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
@@ -57,6 +58,7 @@ class ProfileForm(forms.ModelForm):
         self.fields['address'].widget.attrs.update({"placeholder": "4 avenue des Sciences, 91190 GIF-SUR-YVETTE"})
         self.fields['promotion'].widget.attrs.update({"max": timezone.now().year})
 
+    @transaction.atomic
     def save(self, commit=True):
         if not self.instance.section or (("department" in self.changed_data
                                          or "promotion" in self.changed_data) and "section" not in self.changed_data):

apps/member/models.py

@@ -7,7 +7,7 @@ import os
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
-from django.db import models
+from django.db import models, transaction
 from django.db.models import Q
 from django.template import loader
 from django.urls import reverse, reverse_lazy
@@ -271,6 +271,7 @@ class Club(models.Model):
             self._force_save = True
             self.save(force_update=True)
 
+    @transaction.atomic
     def save(self, force_insert=False, force_update=False, using=None,
              update_fields=None):
         if not self.require_memberships:
@@ -406,6 +407,7 @@ class Membership(models.Model):
                 parent_membership.roles.set(Role.objects.filter(name="Membre de club").all())
             parent_membership.save()
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         """
         Calculate fee and end date before saving the membership and creating the transaction if needed.

apps/member/views.py

@@ -38,6 +38,7 @@ class CustomLoginView(LoginView):
     """
     form_class = CustomAuthenticationForm
 
+    @transaction.atomic
     def form_valid(self, form):
         logout(self.request)
         _set_current_user_and_ip(form.get_user(), self.request.session, None)
@@ -76,6 +77,7 @@ class UserUpdateView(ProtectQuerysetMixin, LoginRequiredMixin, UpdateView):
 
         return context
 
+    @transaction.atomic
     def form_valid(self, form):
         """
         Check if ProfileForm is correct
@@ -269,6 +271,7 @@ class PictureUpdateView(ProtectQuerysetMixin, LoginRequiredMixin, FormMixin, Det
         self.object = self.get_object()
         return self.form_valid(form) if form.is_valid() else self.form_invalid(form)
 
+    @transaction.atomic
     def form_valid(self, form):
         """Save image to note"""
         image = form.cleaned_data['image']
@@ -650,6 +653,7 @@ class ClubAddMemberView(ProtectQuerysetMixin, ProtectedCreateView):
 
         return not error
 
+    @transaction.atomic
     def form_valid(self, form):
         """
         Create membership, check that all is good, make transactions

apps/note/models/notes.py

@@ -8,7 +8,7 @@ from django.conf.global_settings import DEFAULT_FROM_EMAIL
 from django.core.exceptions import ValidationError
 from django.core.mail import send_mail
 from django.core.validators import RegexValidator
-from django.db import models
+from django.db import models, transaction
 from django.template.loader import render_to_string
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
@@ -93,6 +93,7 @@ class Note(PolymorphicModel):
         delta = timezone.now() - self.last_negative
         return "{:d} jours".format(delta.days)
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         """
         Save note with it's alias (called in polymorphic children)
@@ -154,6 +155,7 @@ 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)
@@ -195,6 +197,7 @@ 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)
@@ -310,6 +313,7 @@ class Alias(models.Model):
             pass
         self.normalized_name = normalized_name
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         self.clean()
         super().save(*args, **kwargs)

apps/note/models/transactions.py

@@ -170,15 +170,17 @@ class Transaction(PolymorphicModel):
         previous_source_balance = self.source.balance
         previous_dest_balance = self.destination.balance
 
-        source_balance = self.source.balance
-        dest_balance = self.destination.balance
+        source_balance = previous_source_balance
+        dest_balance = previous_dest_balance
 
         created = self.pk is None
-        to_transfer = self.amount * self.quantity
-        if not created and not self.valid and not hasattr(self, "_force_save"):
+        to_transfer = self.total
+        if not created:
             # Revert old transaction
-            old_transaction = Transaction.objects.get(pk=self.pk)
+            # We make a select for update to avoid concurrency issues
+            old_transaction = Transaction.objects.select_for_update().get(pk=self.pk)
             # Check that nothing important changed
+            if not hasattr(self, "_force_save"):
                 for field_name in ["source_id", "destination_id", "quantity", "amount"]:
                     if getattr(self, field_name) != getattr(old_transaction, field_name):
                         raise ValidationError(_("You can't update the {field} on a Transaction. "
@@ -215,10 +217,6 @@ class Transaction(PolymorphicModel):
             # When source == destination, no money is transferred and no transaction is created
             return
 
-        # We refresh the notes with the "select for update" tag to avoid concurrency issues
-        self.source = Note.objects.filter(pk=self.source_id).select_for_update().get()
-        self.destination = Note.objects.filter(pk=self.destination_id).select_for_update().get()
-
         # Check that the amounts stay between big integer bounds
         diff_source, diff_dest = self.validate()
 
@@ -237,9 +235,11 @@ class Transaction(PolymorphicModel):
         super().save(*args, **kwargs)
 
         # Save notes
+        self.source.refresh_from_db()
         self.source.balance += diff_source
         self.source._force_save = True
         self.source.save()
+        self.destination.refresh_from_db()
         self.destination.balance += diff_dest
         self.destination._force_save = True
         self.destination.save()
@@ -273,6 +273,7 @@ class RecurrentTransaction(Transaction):
                 _("The destination of this transaction must equal to the destination of the template."))
         return super().clean()
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         self.clean()
         return super().save(*args, **kwargs)
@@ -323,6 +324,7 @@ class SpecialTransaction(Transaction):
             raise(ValidationError(_("A special transaction is only possible between a"
                                     " Note associated to a payment method and a User or a Club")))
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         self.clean()
         super().save(*args, **kwargs)

apps/permission/decorators.py

@@ -4,7 +4,6 @@
 from functools import lru_cache
 from time import time
 
-from django.conf import settings
 from django.contrib.sessions.models import Session
 from note_kfet.middlewares import get_current_session
 
@@ -33,9 +32,9 @@ def memoize(f):
         sess_funs = new_sess_funs
 
     def func(*args, **kwargs):
-        if settings.DEBUG:
-            # Don't memoize in DEBUG mode
-            return f(*args, **kwargs)
+        # if settings.DEBUG:
+        #     # Don't memoize in DEBUG mode
+        #     return f(*args, **kwargs)
 
         nonlocal last_collect
 

apps/permission/models.py

@@ -199,6 +199,7 @@ class Permission(models.Model):
         if self.field and self.type not in {'view', 'change'}:
             raise ValidationError(_("Specifying field applies only to view and change permission types."))
 
+    @transaction.atomic
     def save(self, **kwargs):
         self.full_clean()
         super().save()

apps/permission/views.py

@@ -6,6 +6,7 @@ from datetime import date
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.contrib.auth.models import User
 from django.core.exceptions import PermissionDenied
+from django.db import transaction
 from django.db.models import Q
 from django.forms import HiddenInput
 from django.http import Http404
@@ -56,6 +57,7 @@ class ProtectQuerysetMixin:
 
         return form
 
+    @transaction.atomic
     def form_valid(self, form):
         """
         Submit the form, if the page is a FormView.

apps/registration/views.py

@@ -5,6 +5,7 @@ from django.conf import settings
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
+from django.db import transaction
 from django.db.models import Q
 from django.shortcuts import resolve_url, redirect
 from django.urls import reverse_lazy
@@ -47,6 +48,7 @@ class UserCreateView(CreateView):
 
         return context
 
+    @transaction.atomic
     def form_valid(self, form):
         """
         If the form is valid, then the user is created with is_active set to False
@@ -234,6 +236,7 @@ class FutureUserDetailView(ProtectQuerysetMixin, LoginRequiredMixin, FormMixin,
         form.fields["first_name"].initial = user.first_name
         return form
 
+    @transaction.atomic
     def form_valid(self, form):
         user = self.get_object()
 

apps/treasury/forms.py

@@ -4,6 +4,7 @@
 from crispy_forms.helper import FormHelper
 from crispy_forms.layout import Submit
 from django import forms
+from django.db import transaction
 from django.utils.translation import gettext_lazy as _
 from note_kfet.inputs import AmountInput
 
@@ -149,6 +150,7 @@ class LinkTransactionToRemittanceForm(forms.ModelForm):
         self.instance.transaction.bank = cleaned_data["bank"]
         return cleaned_data
 
+    @transaction.atomic
     def save(self, commit=True):
         """
         Save the transaction and the remittance.

apps/treasury/models.py

@@ -5,7 +5,7 @@ from datetime import date
 
 from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
-from django.db import models
+from django.db import models, transaction
 from django.db.models import Q
 from django.template.loader import render_to_string
 from django.utils import timezone
@@ -76,6 +76,7 @@ class Invoice(models.Model):
         verbose_name=_("tex source"),
     )
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         """
         When an invoice is generated, we store the tex source.
@@ -228,6 +229,7 @@ class Remittance(models.Model):
         """
         return sum(transaction.total for transaction in self.transactions.all())
 
+    @transaction.atomic
     def save(self, force_insert=False, force_update=False, using=None, update_fields=None):
         # Check if all transactions have the right type.
         if self.transactions.exists() and self.transactions.filter(~Q(source=self.remittance_type.note)).exists():
@@ -306,10 +308,10 @@ class SogeCredit(models.Model):
         if self.valid:
             self.credit_transaction.valid = False
             self.credit_transaction.save()
-        for transaction in self.transactions.all():
-            transaction.valid = False
-            transaction._force_save = True
-            transaction.save()
+        for tr in self.transactions.all():
+            tr.valid = False
+            tr._force_save = True
+            tr.save()
 
     def validate(self, force=False):
         if self.valid and not force:
@@ -324,12 +326,13 @@ class SogeCredit(models.Model):
         self.credit_transaction.save()
         self.save()
 
-        for transaction in self.transactions.all():
-            transaction.valid = True
-            transaction._force_save = True
-            transaction.created_at = timezone.now()
-            transaction.save()
+        for tr in self.transactions.all():
+            tr.valid = True
+            tr._force_save = True
+            tr.created_at = timezone.now()
+            tr.save()
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         if not self.credit_transaction:
             self.credit_transaction = SpecialTransaction.objects.create(
@@ -362,11 +365,11 @@ class SogeCredit(models.Model):
                                     "Please ask her/him to credit the note before invalidating this credit."))
 
         self.invalidate()
-        for transaction in self.transactions.all():
-            transaction._force_save = True
-            transaction.valid = True
-            transaction.created_at = timezone.now()
-            transaction.save()
+        for tr in self.transactions.all():
+            tr._force_save = True
+            tr.valid = True
+            tr.created_at = timezone.now()
+            tr.save()
         self.credit_transaction.valid = False
         self.credit_transaction.reason += " (invalide)"
         self.credit_transaction.save()

apps/treasury/views.py

@@ -9,6 +9,7 @@ from tempfile import mkdtemp
 from crispy_forms.helper import FormHelper
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.core.exceptions import ValidationError, PermissionDenied
+from django.db import transaction
 from django.db.models import Q
 from django.forms import Form
 from django.http import HttpResponse
@@ -65,6 +66,7 @@ class InvoiceCreateView(ProtectQuerysetMixin, ProtectedCreateView):
         del form.fields["locked"]
         return form
 
+    @transaction.atomic
     def form_valid(self, form):
         ret = super().form_valid(form)
 
@@ -144,6 +146,7 @@ class InvoiceUpdateView(ProtectQuerysetMixin, LoginRequiredMixin, UpdateView):
         del form.fields["id"]
         return form
 
+    @transaction.atomic
     def form_valid(self, form):
         ret = super().form_valid(form)
 
@@ -439,6 +442,7 @@ class SogeCreditManageView(LoginRequiredMixin, ProtectQuerysetMixin, BaseFormVie
     form_class = Form
     extra_context = {"title": _("Manage credits from the Société générale")}
 
+    @transaction.atomic
     def form_valid(self, form):
         if "validate" in form.data:
             self.get_object().validate(True)

apps/wei/forms/surveys/wei2020.py

@@ -4,6 +4,7 @@
 from random import choice
 
 from django import forms
+from django.db import transaction
 from django.utils.translation import gettext_lazy as _
 
 from .base import WEISurvey, WEISurveyInformation, WEISurveyAlgorithm, WEIBusInformation
@@ -88,6 +89,7 @@ class WEISurvey2020(WEISurvey):
         """
         form.set_registration(self.registration)
 
+    @transaction.atomic
     def form_valid(self, form):
         word = form.cleaned_data["word"]
         self.information.step += 1

apps/wei/views.py

@@ -10,6 +10,7 @@ from tempfile import mkdtemp
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.contrib.auth.models import User
 from django.core.exceptions import PermissionDenied
+from django.db import transaction
 from django.db.models import Q, Count
 from django.db.models.functions.text import Lower
 from django.forms import HiddenInput
@@ -84,6 +85,7 @@ class WEICreateView(ProtectQuerysetMixin, ProtectedCreateView):
             date_end=date.today(),
         )
 
+    @transaction.atomic
     def form_valid(self, form):
         form.instance.requires_membership = True
         form.instance.parent_club = Club.objects.get(name="Kfet")
@@ -517,6 +519,7 @@ class WEIRegister1AView(ProtectQuerysetMixin, ProtectedCreateView):
         del form.fields["information_json"]
         return form
 
+    @transaction.atomic
     def form_valid(self, form):
         form.instance.wei = WEIClub.objects.get(pk=self.kwargs["wei_pk"])
         form.instance.first_year = True
@@ -597,6 +600,7 @@ class WEIRegister2AView(ProtectQuerysetMixin, ProtectedCreateView):
 
         return form
 
+    @transaction.atomic
     def form_valid(self, form):
         form.instance.wei = WEIClub.objects.get(pk=self.kwargs["wei_pk"])
         form.instance.first_year = False
@@ -688,6 +692,7 @@ class WEIUpdateRegistrationView(ProtectQuerysetMixin, LoginRequiredMixin, Update
             del form.fields["information_json"]
         return form
 
+    @transaction.atomic
     def form_valid(self, form):
         # If the membership is already validated, then we update the bus and the team (and the roles)
         if form.instance.is_validated:
@@ -866,6 +871,7 @@ class WEIValidateRegistrationView(ProtectQuerysetMixin, ProtectedCreateView):
                 ).all()
         return form
 
+    @transaction.atomic
     def form_valid(self, form):
         """
         Create membership, check that all is good, make transactions
@@ -1016,6 +1022,7 @@ class WEISurveyView(LoginRequiredMixin, BaseFormView, DetailView):
         context["club"] = self.object.wei
         return context
 
+    @transaction.atomic
     def form_valid(self, form):
         """
         Update the survey with the data of the form.