repair user merging tool (#498)

counter/models.py

@@ -22,6 +22,7 @@
 #
 #
 from __future__ import annotations
+from django.db.models import Sum, F
 
 from typing import Tuple
 
@@ -90,12 +91,9 @@ class Customer(models.Model):
         about the relation between a User (not a Customer,
         don't mix them) and a Product.
         """
-        return self.user.subscriptions.last() and (
+        subscription = self.user.subscriptions.order_by("subscription_end").last()
-            date.today()
+        time_diff = date.today() - subscription.subscription_end
-            - self.user.subscriptions.order_by("subscription_end")
+        return subscription is not None and time_diff < timedelta(days=90)
-            .last()
-            .subscription_end
-        ) < timedelta(days=90)
 
     @classmethod
     def get_or_create(cls, user: User) -> Tuple[Customer, bool]:
@@ -151,11 +149,15 @@ class Customer(models.Model):
         super(Customer, self).save(*args, **kwargs)
 
     def recompute_amount(self):
-        self.amount = 0
+        refillings = self.refillings.aggregate(sum=Sum(F("amount")))["sum"]
-        for r in self.refillings.all():
+        self.amount = refillings if refillings is not None else 0
-            self.amount += r.amount
+        purchases = (
-        for s in self.buyings.filter(payment_method="SITH_ACCOUNT"):
+            self.buyings.filter(payment_method="SITH_ACCOUNT")
-            self.amount -= s.quantity * s.unit_price
+            .annotate(amount=F("quantity") * F("unit_price"))
+            .aggregate(sum=Sum(F("amount")))
+        )["sum"]
+        if purchases is not None:
+            self.amount -= purchases
         self.save()
 
     def get_absolute_url(self):

rootplace/tests.py

@@ -21,7 +21,212 @@
 # Place - Suite 330, Boston, MA 02111-1307, USA.
 #
 #
+from datetime import date, timedelta
 
+from django.core.management import call_command
 from django.test import TestCase
+from django.urls import reverse
 
-# Create your tests here.
+from club.models import Club
+from core.models import User, RealGroup
+from counter.models import Customer, Product, Selling, Counter, Refilling
+from subscription.models import Subscription
+
+
+class MergeUserTest(TestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        call_command("populate")
+        cls.ae = Club.objects.get(unix_name="ae")
+        cls.eboutic = Counter.objects.get(name="Eboutic")
+        cls.barbar = Product.objects.get(code="BARB")
+        cls.barbar.selling_price = 2
+        cls.barbar.save()
+        cls.root = User.objects.get(username="root")
+
+    def setUp(self) -> None:
+        super().setUp()
+        self.to_keep = User(username="to_keep", password="plop", email="u.1@utbm.fr")
+        self.to_delete = User(username="to_del", password="plop", email="u.2@utbm.fr")
+        self.to_keep.save()
+        self.to_delete.save()
+        self.client.login(username="root", password="plop")
+
+    def test_simple(self):
+        self.to_delete.first_name = "Biggus"
+        self.to_keep.last_name = "Dickus"
+        self.to_keep.nick_name = "B'ian"
+        self.to_keep.address = "Jerusalem"
+        self.to_delete.parent_address = "Rome"
+        self.to_delete.address = "Rome"
+        subscribers = RealGroup.objects.get(name="Subscribers")
+        mde_admin = RealGroup.objects.get(name="MDE admin")
+        sas_admin = RealGroup.objects.get(name="SAS admin")
+        self.to_keep.groups.add(subscribers.id)
+        self.to_delete.groups.add(mde_admin.id)
+        self.to_keep.groups.add(sas_admin.id)
+        self.to_delete.groups.add(sas_admin.id)
+        self.to_delete.save()
+        self.to_keep.save()
+        data = {"user1": self.to_keep.id, "user2": self.to_delete.id}
+        res = self.client.post(reverse("rootplace:merge"), data)
+        self.assertRedirects(res, self.to_keep.get_absolute_url())
+        self.assertFalse(User.objects.filter(pk=self.to_delete.pk).exists())
+        self.to_keep = User.objects.get(pk=self.to_keep.pk)
+        # fields of to_delete should be assigned to to_keep
+        # if they were not set beforehand
+        self.assertEqual("Biggus", self.to_keep.first_name)
+        self.assertEqual("Dickus", self.to_keep.last_name)
+        self.assertEqual("B'ian", self.to_keep.nick_name)
+        self.assertEqual("Jerusalem", self.to_keep.address)
+        self.assertEqual("Rome", self.to_keep.parent_address)
+        self.assertEqual(3, self.to_keep.groups.count())
+        groups = list(self.to_keep.groups.all())
+        expected = [subscribers, mde_admin, sas_admin]
+        self.assertCountEqual(groups, expected)
+
+    def test_both_subscribers_and_with_account(self):
+        Customer(user=self.to_keep, account_id="11000l", amount=0).save()
+        Customer(user=self.to_delete, account_id="12000m", amount=0).save()
+        Refilling(
+            amount=10,
+            operator=self.root,
+            customer=self.to_keep.customer,
+            counter=self.eboutic,
+        ).save()
+        Refilling(
+            amount=20,
+            operator=self.root,
+            customer=self.to_delete.customer,
+            counter=self.eboutic,
+        ).save()
+        Selling(
+            label="barbar",
+            counter=self.eboutic,
+            club=self.ae,
+            product=self.barbar,
+            customer=self.to_keep.customer,
+            seller=self.root,
+            unit_price=2,
+            quantity=2,
+            payment_method="SITH_ACCOUNT",
+        ).save()
+        Selling(
+            label="barbar",
+            counter=self.eboutic,
+            club=self.ae,
+            product=self.barbar,
+            customer=self.to_delete.customer,
+            seller=self.root,
+            unit_price=2,
+            quantity=4,
+            payment_method="SITH_ACCOUNT",
+        ).save()
+        today = date.today()
+        # both subscriptions began last month and shall end in 5 months
+        Subscription(
+            member=self.to_keep,
+            subscription_type="un-semestre",
+            payment_method="EBOUTIC",
+            subscription_start=today - timedelta(30),
+            subscription_end=today + timedelta(5 * 30),
+        ).save()
+        Subscription(
+            member=self.to_delete,
+            subscription_type="un-semestre",
+            payment_method="EBOUTIC",
+            subscription_start=today - timedelta(30),
+            subscription_end=today + timedelta(5 * 30),
+        ).save()
+        data = {"user1": self.to_keep.id, "user2": self.to_delete.id}
+        res = self.client.post(reverse("rootplace:merge"), data)
+        self.to_keep = User.objects.get(pk=self.to_keep.id)
+        self.assertRedirects(res, self.to_keep.get_absolute_url())
+        # to_keep had 10€ at first and bought 2 barbar worth 2€ each
+        # to_delete had 20€ and bought 4 barbar
+        # total should be 10 - 4 + 20 - 8 = 18
+        self.assertAlmostEqual(18, self.to_keep.customer.amount, delta=0.0001)
+        self.assertEqual(2, self.to_keep.customer.buyings.count())
+        self.assertEqual(2, self.to_keep.customer.refillings.count())
+        self.assertTrue(self.to_keep.is_subscribed)
+        # to_keep had 5 months of subscription remaining and received
+        # 5 more months from to_delete, so he should be subscribed for 10 months
+        self.assertEqual(
+            today + timedelta(10 * 30),
+            self.to_keep.subscriptions.order_by("subscription_end")
+            .last()
+            .subscription_end,
+        )
+
+    def test_godfathers(self):
+        users = list(User.objects.all()[:4])
+        self.to_keep.godfathers.add(users[0])
+        self.to_keep.godchildren.add(users[1])
+        self.to_delete.godfathers.add(users[2])
+        self.to_delete.godfathers.add(self.to_keep)
+        self.to_delete.godchildren.add(users[3])
+        data = {"user1": self.to_keep.id, "user2": self.to_delete.id}
+        res = self.client.post(reverse("rootplace:merge"), data)
+        self.assertRedirects(res, self.to_keep.get_absolute_url())
+        self.to_keep = User.objects.get(pk=self.to_keep.id)
+        self.assertCountEqual(list(self.to_keep.godfathers.all()), [users[0], users[2]])
+        self.assertCountEqual(
+            list(self.to_keep.godchildren.all()), [users[1], users[3]]
+        )
+
+    def test_keep_has_no_account(self):
+        Customer(user=self.to_delete, account_id="12000m", amount=0).save()
+        Refilling(
+            amount=20,
+            operator=self.root,
+            customer=self.to_delete.customer,
+            counter=self.eboutic,
+        ).save()
+        Selling(
+            label="barbar",
+            counter=self.eboutic,
+            club=self.ae,
+            product=self.barbar,
+            customer=self.to_delete.customer,
+            seller=self.root,
+            unit_price=2,
+            quantity=4,
+            payment_method="SITH_ACCOUNT",
+        ).save()
+        data = {"user1": self.to_keep.id, "user2": self.to_delete.id}
+        res = self.client.post(reverse("rootplace:merge"), data)
+        self.to_keep = User.objects.get(pk=self.to_keep.id)
+        self.assertRedirects(res, self.to_keep.get_absolute_url())
+        # to_delete had 20€ and bought 4 barbar worth 2€ each
+        # total should be 20 - 8 = 12
+        self.assertTrue(hasattr(self.to_keep, "customer"))
+        self.assertAlmostEqual(12, self.to_keep.customer.amount, delta=0.0001)
+
+    def test_delete_has_no_account(self):
+        Customer(user=self.to_keep, account_id="12000m", amount=0).save()
+        Refilling(
+            amount=20,
+            operator=self.root,
+            customer=self.to_keep.customer,
+            counter=self.eboutic,
+        ).save()
+        Selling(
+            label="barbar",
+            counter=self.eboutic,
+            club=self.ae,
+            product=self.barbar,
+            customer=self.to_keep.customer,
+            seller=self.root,
+            unit_price=2,
+            quantity=4,
+            payment_method="SITH_ACCOUNT",
+        ).save()
+        data = {"user1": self.to_keep.id, "user2": self.to_delete.id}
+        res = self.client.post(reverse("rootplace:merge"), data)
+        self.to_keep = User.objects.get(pk=self.to_keep.id)
+        self.assertRedirects(res, self.to_keep.get_absolute_url())
+        # to_keep had 20€ and bought 4 barbar worth 2€ each
+        # total should be 20 - 8 = 12
+        self.assertTrue(hasattr(self.to_keep, "customer"))
+        self.assertAlmostEqual(12, self.to_keep.customer.amount, delta=0.0001)

rootplace/views.py

@@ -23,72 +23,114 @@
 #
 #
 
-from django.utils.translation import gettext as _
+from ajax_select.fields import AutoCompleteSelectField
-from django.views.generic.edit import FormView
-from django.views.generic import ListView
-from django.urls import reverse
 from django import forms
 from django.core.exceptions import PermissionDenied
+from django.urls import reverse
+from django.utils import timezone
+from django.utils.translation import gettext as _
+from django.views.generic import ListView
+from django.views.generic.edit import FormView
 
-from ajax_select.fields import AutoCompleteSelectField
+from core.models import User, OperationLog, SithFile
-
 from core.views import CanEditPropMixin
-from core.models import User, OperationLog
 from counter.models import Customer
-
 from forum.models import ForumMessageMeta
 
 
-def merge_users(u1, u2):
+def __merge_subscriptions(u1: User, u2: User):
-    u1.nick_name = u1.nick_name or u2.nick_name
+    """
-    u1.date_of_birth = u1.date_of_birth or u2.date_of_birth
+    Give all the subscriptions of the second user to first one
-    u1.home = u1.home or u2.home
+    If some subscriptions are still active, update their end date
-    u1.sex = u1.sex or u2.sex
+    to increase the overall subscription time of the first user.
-    u1.pronouns = u1.pronouns or u2.pronouns
+
-    u1.tshirt_size = u1.tshirt_size or u2.tshirt_size
+    Some examples :
-    u1.role = u1.role or u2.role
+    - if u1 is not subscribed, his subscription end date become the one of u2
-    u1.department = u1.department or u2.department
+    - if u1 is subscribed but not u2, nothing happen
-    u1.dpt_option = u1.dpt_option or u2.dpt_option
+    - if u1 is subscribed for, let's say, 2 remaining months and u2 is subscribed for 3 remaining months,
-    u1.semester = u1.semester or u2.semester
+    he shall then be subscribed for 5 months
-    u1.quote = u1.quote or u2.quote
+    """
-    u1.school = u1.school or u2.school
+    last_subscription = (
-    u1.promo = u1.promo or u2.promo
+        u1.subscriptions.filter(
-    u1.forum_signature = u1.forum_signature or u2.forum_signature
+            subscription_start__lte=timezone.now(), subscription_end__gte=timezone.now()
-    u1.second_email = u1.second_email or u2.second_email
+        )
-    u1.phone = u1.phone or u2.phone
+        .order_by("subscription_end")
-    u1.parent_phone = u1.parent_phone or u2.parent_phone
+        .last()
-    u1.address = u1.address or u2.address
+    )
-    u1.parent_address = u1.parent_address or u2.parent_address
+    if last_subscription is not None:
+        subscription_end = last_subscription.subscription_end
+        for subscription in u2.subscriptions.filter(
+            subscription_end__gte=timezone.now()
+        ):
+            subscription.subscription_start = subscription_end
+            if subscription.subscription_start > timezone.now().date():
+                remaining = subscription.subscription_end - timezone.now().date()
+            else:
+                remaining = (
+                    subscription.subscription_end - subscription.subscription_start
+                )
+            subscription_end += remaining
+            subscription.subscription_end = subscription_end
+            subscription.save()
+    u2.subscriptions.all().update(member=u1)
+
+
+def __merge_pictures(u1: User, u2: User) -> None:
+    SithFile.objects.filter(owner=u2).update(owner=u1)
+    if u1.profile_pict is None and u2.profile_pict is not None:
+        u1.profile_pict, u2.profile_pict = u2.profile_pict, None
+    if u1.scrub_pict is None and u2.scrub_pict is not None:
+        u1.scrub_pict, u2.scrub_pict = u2.scrub_pict, None
+    if u1.avatar_pict is None and u2.avatar_pict is not None:
+        u1.avatar_pict, u2.avatar_pict = u2.avatar_pict, None
+    u2.save()
     u1.save()
-    for u in u2.godfathers.all():
+
-        u1.godfathers.add(u)
+
+def merge_users(u1: User, u2: User) -> User:
+    """
+    Merge u2 into u1
+    This means that u1 shall receive everything that belonged to u2 :
+
+        - pictures
+        - refills of the sith account
+        - purchases of any item bought on the eboutic or the counters
+        - subscriptions
+        - godfathers
+        - godchildren
+
+    If u1 had no account id, he shall receive the one of u2.
+    If u1 and u2 were both in the middle of a subscription, the remaining
+    durations stack
+    If u1 had no profile picture, he shall receive the one of u2
+    """
+    for field in u1._meta.fields:
+        if not field.is_relation and not u1.__dict__[field.name]:
+            u1.__dict__[field.name] = u2.__dict__[field.name]
+    for group in u2.groups.all():
+        u1.groups.add(group.id)
+    for godfather in u2.godfathers.exclude(id=u1.id):
+        u1.godfathers.add(godfather)
+    for godchild in u2.godchildren.exclude(id=u1.id):
+        u1.godchildren.add(godchild)
+    __merge_subscriptions(u1, u2)
+    __merge_pictures(u1, u2)
+    u2.invoices.all().update(user=u1)
+    c_src = Customer.objects.filter(user=u2).first()
+    if c_src is not None:
+        c_dest, created = Customer.get_or_create(u1)
+        c_src.refillings.update(customer=c_dest)
+        c_src.buyings.update(customer=c_dest)
+        c_dest.recompute_amount()
+        if created:
+            # swap the account numbers, so that the user keep
+            # the id he is accustomed to
+            tmp_id = c_src.account_id
+            # delete beforehand in order not to have a unique constraint violation
+            c_src.delete()
+            c_dest.account_id = tmp_id
     u1.save()
-    for i in u2.invoices.all():
+    u2.delete()  # everything remaining in u2 gets deleted thanks to on_delete=CASCADE
-        for f in i._meta.local_fields:  # I have sadly not found anything better :/
-            if f.name == "date":
-                f.auto_now = False
-        u1.invoices.add(i)
-    u1.save()
-    s1 = User.objects.filter(id=u1.id).first()
-    s2 = User.objects.filter(id=u2.id).first()
-    for s in s2.subscriptions.all():
-        s1.subscriptions.add(s)
-    s1.save()
-    c1 = Customer.objects.filter(user__id=u1.id).first()
-    c2 = Customer.objects.filter(user__id=u2.id).first()
-    if c1 and c2:
-        for r in c2.refillings.all():
-            c1.refillings.add(r)
-        c1.save()
-        for s in c2.buyings.all():
-            c1.buyings.add(s)
-        c1.save()
-    elif c2 and not c1:
-        c2.user = u1
-        c1 = c2
-        c1.save()
-    c1.recompute_amount()
-    u2.delete()
     return u1
 
 
@@ -128,9 +170,8 @@ class MergeUsersView(FormView):
     form_class = MergeForm
 
     def dispatch(self, request, *arg, **kwargs):
-        res = super(MergeUsersView, self).dispatch(request, *arg, **kwargs)
         if request.user.is_root:
-            return res
+            return super().dispatch(request, *arg, **kwargs)
         raise PermissionDenied
 
     def form_valid(self, form):
@@ -140,7 +181,7 @@ class MergeUsersView(FormView):
         return super(MergeUsersView, self).form_valid(form)
 
     def get_success_url(self):
-        return reverse("core:user_profile", kwargs={"user_id": self.final_user.id})
+        return self.final_user.get_absolute_url()
 
 
 class DeleteAllForumUserMessagesView(FormView):

subscription/models.py

@@ -165,7 +165,4 @@ class Subscription(models.Model):
         return user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) or user.is_root
 
     def is_valid_now(self):
-        return (
+        return self.subscription_start <= date.today() <= self.subscription_end
-            self.subscription_start <= date.today()
-            and date.today() <= self.subscription_end
-        )