From 751c8a8bc66e7f8dd569b324634a5dcc9ec8c52e Mon Sep 17 00:00:00 2001 From: Thomas Girod Date: Wed, 23 Nov 2022 12:23:17 +0100 Subject: [PATCH] unify account_id creation --- counter/migrations/0020_auto_20221215_1709.py | 21 +++++++++ counter/models.py | 47 ++++++++++++++----- counter/tests.py | 25 +++++++--- counter/views.py | 6 +-- eboutic/models.py | 5 +- subscription/models.py | 5 +- 6 files changed, 81 insertions(+), 28 deletions(-) create mode 100644 counter/migrations/0020_auto_20221215_1709.py diff --git a/counter/migrations/0020_auto_20221215_1709.py b/counter/migrations/0020_auto_20221215_1709.py new file mode 100644 index 00000000..ca5d1176 --- /dev/null +++ b/counter/migrations/0020_auto_20221215_1709.py @@ -0,0 +1,21 @@ +# Generated by Django 3.2.16 on 2022-12-15 16:09 + +import accounting.models +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("counter", "0019_billinginfo"), + ] + + operations = [ + migrations.AlterField( + model_name="customer", + name="amount", + field=accounting.models.CurrencyField( + decimal_places=2, default=0, max_digits=12, verbose_name="amount" + ), + ), + ] diff --git a/counter/models.py b/counter/models.py index 564d6a3b..00e81b4f 100644 --- a/counter/models.py +++ b/counter/models.py @@ -21,6 +21,9 @@ # Place - Suite 330, Boston, MA 02111-1307, USA. # # +from __future__ import annotations + +from typing import Tuple from django.db import models from django.db.models.functions import Length @@ -57,7 +60,7 @@ class Customer(models.Model): user = models.OneToOneField(User, primary_key=True, on_delete=models.CASCADE) account_id = models.CharField(_("account id"), max_length=10, unique=True) - amount = CurrencyField(_("amount")) + amount = CurrencyField(_("amount"), default=0) recorded_products = models.IntegerField(_("recorded product"), default=0) class Meta: @@ -94,12 +97,27 @@ class Customer(models.Model): ) < timedelta(days=90) @classmethod - def new_for_user(cls, user: User): + def get_or_create(cls, user: User) -> Tuple[Customer, bool]: """ - Create a new Customer instance for the user given in parameter without saving it - The account if is automatically generated and the amount set at 0 + Work in pretty much the same way as the usual get_or_create method, + but with the default field replaced by some under the hood. + + If the user has an account, return it as is. + Else create a new account with no money on it and a new unique account id + + Example : :: + + user = User.objects.get(pk=1) + account, created = Customer.get_or_create(user) + if created: + print(f"created a new account with id {account.id}") + else: + print(f"user has already an account, with {account.id} € on it" """ - # account_id are number with a letter appended + if hasattr(user, "customer"): + return user.customer, False + + # account_id are always a number with a letter appended account_id = ( Customer.objects.order_by(Length("account_id"), "account_id") .values("account_id") @@ -107,14 +125,19 @@ class Customer(models.Model): ) if account_id is None: # legacy from the old site - return cls(user=user, account_id="1504a", amount=0) - account_id = account_id["account_id"] - num = int(account_id[:-1]) - while Customer.objects.filter(account_id=account_id).exists(): - num += 1 - account_id = str(num) + random.choice(string.ascii_lowercase) + account = cls.objects.create(user=user, account_id="1504a") + return account, True - return cls(user=user, account_id=account_id, amount=0) + account_id = account_id["account_id"] + account_num = int(account_id[:-1]) + while Customer.objects.filter(account_id=account_id).exists(): + # when entering the first iteration, we are using an already existing account id + # so the loop should always execute at least one time + account_num += 1 + account_id = f"{account_num}{random.choice(string.ascii_lowercase)}" + + account = cls.objects.create(user=user, account_id=account_id) + return account, True def save(self, allow_negative=False, is_selling=False, *args, **kwargs): """ diff --git a/counter/tests.py b/counter/tests.py index 0a25a9ff..dbdf3898 100644 --- a/counter/tests.py +++ b/counter/tests.py @@ -23,6 +23,7 @@ # import json import re +import string from django.test import TestCase from django.urls import reverse @@ -745,18 +746,30 @@ class StudentCardTest(TestCase): self.assertEqual(response.status_code, 403) -class AccountIdTest(TestCase): +class CustomerAccountIdTest(TestCase): def setUp(self): - user_a = User.objects.create(username="a", password="plop", email="a.a@a.fr") + self.user_a = User.objects.create( + username="a", password="plop", email="a.a@a.fr" + ) user_b = User.objects.create(username="b", password="plop", email="b.b@b.fr") user_c = User.objects.create(username="c", password="plop", email="c.c@c.fr") - Customer.objects.create(user=user_a, amount=0, account_id="1111a") + Customer.objects.create(user=self.user_a, amount=10, account_id="1111a") Customer.objects.create(user=user_b, amount=0, account_id="9999z") Customer.objects.create(user=user_c, amount=0, account_id="12345f") def test_create_customer(self): user_d = User.objects.create(username="d", password="plop") - customer_d = Customer.new_for_user(user_d) - customer_d.save() - number = customer_d.account_id[:-1] + customer, created = Customer.get_or_create(user_d) + account_id = customer.account_id + number = account_id[:-1] + self.assertTrue(created) self.assertEqual(number, "12346") + self.assertEqual(6, len(account_id)) + self.assertIn(account_id[-1], string.ascii_lowercase) + self.assertEqual(0, customer.amount) + + def test_get_existing_account(self): + account, created = Customer.get_or_create(self.user_a) + self.assertFalse(created) + self.assertEqual(account.account_id, "1111a") + self.assertEqual(10, account.amount) diff --git a/counter/views.py b/counter/views.py index ca012688..69f88b22 100644 --- a/counter/views.py +++ b/counter/views.py @@ -1780,11 +1780,7 @@ def create_billing_info(request, user_id): if user.id != user_id and not user.has_perm("counter:add_billinginfo"): raise PermissionDenied() user = get_object_or_404(User, pk=user_id) - if not hasattr(user, "customer"): - customer = Customer.new_for_user(user) - customer.save() - else: - customer = get_object_or_404(Customer, user_id=user_id) + customer, _ = Customer.get_or_create(user) BillingInfo.objects.create(customer=customer) return __manage_billing_info_req(request, user_id, True) diff --git a/eboutic/models.py b/eboutic/models.py index 99fe6410..ebfb878b 100644 --- a/eboutic/models.py +++ b/eboutic/models.py @@ -245,12 +245,13 @@ class Invoice(models.Model): def validate(self): if self.validated: raise DataError(_("Invoice already validated")) + customer, created = Customer.get_or_create(user=self.user) eboutic = Counter.objects.filter(type="EBOUTIC").first() for i in self.items.all(): if i.type_id == settings.SITH_COUNTER_PRODUCTTYPE_REFILLING: new = Refilling( counter=eboutic, - customer=self.user.customer, + customer=customer, operator=self.user, amount=i.product_unit_price * i.quantity, payment_method="CARD", @@ -266,7 +267,7 @@ class Invoice(models.Model): club=product.club, product=product, seller=self.user, - customer=self.user.customer, + customer=customer, unit_price=i.product_unit_price, quantity=i.quantity, payment_method="CARD", diff --git a/subscription/models.py b/subscription/models.py index b724b734..31f6a2de 100644 --- a/subscription/models.py +++ b/subscription/models.py @@ -24,7 +24,6 @@ from datetime import date, timedelta from django.db import models -from django.utils import timezone from django.utils.translation import gettext_lazy as _ from django.conf import settings from django.core.exceptions import ValidationError @@ -101,8 +100,8 @@ class Subscription(models.Model): super(Subscription, self).save() from counter.models import Customer - if not Customer.objects.filter(user=self.member).exists(): - Customer.new_for_user(self.member).save() + _, created = Customer.get_or_create(self.member) + if created: form = PasswordResetForm({"email": self.member.email}) if form.is_valid(): form.save(