Merge pull request #720 from ae-utbm/counter-refactor

Refactor on counters

core/templates/core/base.jinja

@@ -74,7 +74,7 @@
                                                 {% endif %}
                                                 {% if bar.is_inactive() %}
                                                   <i class="fa fa-question" style="color: #f39c12"></i>
-                                                {% elif bar.is_open(): %}
+                                                {% elif bar.is_open %}
                                                   <i class="fa fa-check" style="color: #2ecc71"></i>
                                                 {% else %}
                                                   <i class="fa fa-times" style="color: #eb2f06"></i>

counter/models.py

@@ -383,19 +383,19 @@ class Counter(models.Model):
     def __str__(self):
         return self.name
 
-    def get_absolute_url(self):
+    def get_absolute_url(self) -> str:
         if self.type == "EBOUTIC":
             return reverse("eboutic:main")
         return reverse("counter:details", kwargs={"counter_id": self.id})
 
-    def __getattribute__(self, name):
+    def __getattribute__(self, name: str):
         if name == "edit_groups":
             return Group.objects.filter(
                 name=self.club.unix_name + settings.SITH_BOARD_SUFFIX
             ).all()
         return object.__getattribute__(self, name)
 
-    def is_owned_by(self, user):
+    def is_owned_by(self, user: User) -> bool:
         if user.is_anonymous:
             return False
         mem = self.club.get_membership_for(user)
@@ -403,90 +403,68 @@ class Counter(models.Model):
             return True
         return user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID)
 
-    def can_be_viewed_by(self, user):
+    def can_be_viewed_by(self, user: User) -> bool:
         if self.type == "BAR":
             return True
         return user.is_board_member or user in self.sellers.all()
 
-    def gen_token(self):
+    def gen_token(self) -> None:
         """Generate a new token for this counter."""
         self.token = "".join(
-            random.choice(string.ascii_letters + string.digits) for x in range(30)
+            random.choice(string.ascii_letters + string.digits) for _ in range(30)
         )
         self.save()
 
-    def add_barman(self, user):
-        """Logs a barman in to the given counter.
-
-        A user is stored as a tuple with its login time.
-        """
-        Permanency(user=user, counter=self, start=timezone.now(), end=None).save()
-
-    def del_barman(self, user):
-        """Logs a barman out and store its permanency."""
-        perm = Permanency.objects.filter(counter=self, user=user, end=None).all()
-        for p in perm:
-            p.end = p.activity
-            p.save()
-
     @cached_property
-    def barmen_list(self):
+    def barmen_list(self) -> list[User]:
         return self.get_barmen_list()
 
-    def get_barmen_list(self):
+    def get_barmen_list(self) -> list[User]:
         """Returns the barman list as list of User.
 
         Also handle the timeout of the barmen
         """
-        pl = Permanency.objects.filter(counter=self, end=None).all()
+        perms = self.permanencies.filter(end=None)
-        bl = []
-        for p in pl:
-            if timezone.now() - p.activity < timedelta(
-                minutes=settings.SITH_BARMAN_TIMEOUT
-            ):
-                bl.append(p.user)
-            else:
-                p.end = p.activity
-                p.save()
-        return bl
 
-    def get_random_barman(self):
+        # disconnect barmen who are inactive
+        timeout = timezone.now() - timedelta(minutes=settings.SITH_BARMAN_TIMEOUT)
+        perms.filter(activity__lte=timeout).update(end=F("activity"))
+
+        return [p.user for p in perms.select_related("user")]
+
+    def get_random_barman(self) -> User:
         """Return a random user being currently a barman."""
-        bl = self.get_barmen_list()
+        return random.choice(self.barmen_list)
-        return bl[random.randrange(0, len(bl))]
 
-    def update_activity(self):
+    def update_activity(self) -> None:
         """Update the barman activity to prevent timeout."""
-        for p in Permanency.objects.filter(counter=self, end=None).all():
+        self.permanencies.filter(end=None).update(activity=timezone.now())
-            p.save()  # Update activity
 
-    def is_open(self):
+    @property
+    def is_open(self) -> bool:
         return len(self.barmen_list) > 0
 
-    def is_inactive(self):
+    def is_inactive(self) -> bool:
         """Returns True if the counter self is inactive from SITH_COUNTER_MINUTE_INACTIVE's value minutes, else False."""
-        return self.is_open() and (
+        return self.is_open and (
             (timezone.now() - self.permanencies.order_by("-activity").first().activity)
             > timedelta(minutes=settings.SITH_COUNTER_MINUTE_INACTIVE)
         )
 
-    def barman_list(self):
+    def barman_list(self) -> list[int]:
         """Returns the barman id list."""
-        return [b.id for b in self.get_barmen_list()]
+        return [b.id for b in self.barmen_list]
 
-    def can_refill(self):
+    def can_refill(self) -> bool:
         """Show if the counter authorize the refilling with physic money."""
         if self.type != "BAR":
             return False
         if self.id in SITH_COUNTER_OFFICES:
             # If the counter is either 'AE' or 'BdF', refills are authorized
             return True
-        is_ae_member = False
+        # at least one of the barmen is in the AE board
         ae = Club.objects.get(unix_name=SITH_MAIN_CLUB["unix_name"])
-        for barman in self.get_barmen_list():
+        return any(ae.get_membership_for(barman) for barman in self.barmen_list)
-            if ae.get_membership_for(barman):
-                is_ae_member = True
-        return is_ae_member
 
     def get_top_barmen(self) -> QuerySet:
         """Return a QuerySet querying the office hours stats of all the barmen of all time
@@ -565,10 +543,13 @@ class Counter(models.Model):
             since = get_start_of_semester()
         if isinstance(since, date):
             since = datetime(since.year, since.month, since.day, tzinfo=tz.utc)
-        total = self.sellings.filter(date__gte=since).aggregate(
+        return self.sellings.filter(date__gte=since).aggregate(
-            total=Sum(F("quantity") * F("unit_price"), output_field=CurrencyField())
+            total=Sum(
+                F("quantity") * F("unit_price"),
+                default=0,
+                output_field=CurrencyField(),
+            )
         )["total"]
-        return total if total is not None else CurrencyField(0)
 
 
 class Refilling(models.Model):

counter/signals.py

@@ -41,7 +41,7 @@ def write_log(instance, operation_type):
         session_token = session.get("counter_token", None)
         if session_token:
             counter = Counter.objects.filter(token=session_token).first()
-            if counter and len(counter.get_barmen_list()) > 0:
+            if counter and len(counter.barmen_list) > 0:
                 return counter.get_random_barman()
 
         # Get the current logged user if not from a counter

counter/templates/counter/activity.jinja

@@ -14,9 +14,8 @@
         {% if counter.type == 'BAR' %}
             <h4>{% trans %}Barmen list{% endtrans %}</h4>
             <ul>
-                {% set barmans_list = counter.get_barmen_list() %}
+                {% if counter.barmen_list | length > 0 %}
-                {% if barmans_list | length > 0 %}
+                    {% for b in counter.barmen_list %}
-                    {% for b in barmans_list %}
                         <li>{{ user_profile_link(b) }}</li>
                     {% endfor %}
                 {% else %}

counter/templates/counter/counter_click.jinja

@@ -41,9 +41,10 @@
                 <input type="submit" value="{% trans %}Go{% endtrans %}"/>
             </form>
             <h6>{% trans %}Registered cards{% endtrans %}</h6>
-            {% if customer.student_cards.exists() %}
+            {% if student_cards %}
+                <p>{{ student_cards }}</p>
                 <ul>
-                    {% for card in customer.student_cards.all() %}
+                    {% for card in student_cards %}
                         <li>{{ card.uid }}</li>
                     {% endfor %}
                 </ul>
@@ -55,7 +56,7 @@
         <div id="click_form">
             <h5>{% trans %}Selling{% endtrans %}</h5>
             <div>
-                {% set counter_click_url = url('counter:click', counter_id=counter.id, user_id=customer.user.id) %}
+                {% set counter_click_url = url('counter:click', counter_id=counter.id, user_id=customer.user_id) %}
 
                 {# Formulaire pour rechercher un produit en tapant son code dans une barre de recherche #}
                 <form method="post" action=""
@@ -166,7 +167,7 @@
             {%- endfor %}
         </div>
     </div>
-{% endblock %}
+{% endblock content %}
 
 {% block script %}
     {{ super() }}
@@ -193,4 +194,4 @@
             {%- endfor %}
         ];
     </script>
-{% endblock %}
+{% endblock script %}

counter/urls.py

@@ -40,8 +40,8 @@ urlpatterns = [
         name="activity",
     ),
     path("<int:counter_id>/stats/", CounterStatView.as_view(), name="stats"),
-    path("<int:counter_id>/login/", CounterLogin.as_view(), name="login"),
+    path("<int:counter_id>/login/", counter_login, name="login"),
-    path("<int:counter_id>/logout/", CounterLogout.as_view(), name="logout"),
+    path("<int:counter_id>/logout/", counter_logout, name="logout"),
     path(
         "eticket/<int:selling_id>/pdf/",
         EticketPDFView.as_view(),

counter/views.py

@@ -27,13 +27,19 @@ from django.db import DataError, transaction
 from django.db.models import F
 from django.forms import CheckboxSelectMultiple
 from django.forms.models import modelform_factory
-from django.http import Http404, HttpResponse, HttpResponseRedirect, JsonResponse
+from django.http import (
-from django.shortcuts import get_object_or_404
+    Http404,
+    HttpRequest,
+    HttpResponse,
+    HttpResponseRedirect,
+    JsonResponse,
+)
+from django.shortcuts import get_object_or_404, redirect
 from django.urls import reverse, reverse_lazy
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 from django.views.decorators.http import require_POST
-from django.views.generic import DetailView, ListView, RedirectView, TemplateView
+from django.views.generic import DetailView, ListView, TemplateView
 from django.views.generic.base import View
 from django.views.generic.edit import (
     CreateView,
@@ -66,6 +72,7 @@ from counter.models import (
     Counter,
     Customer,
     Eticket,
+    Permanency,
     Product,
     ProductType,
     Refilling,
@@ -254,7 +261,7 @@ class CounterMain(
                 None, _("Bad location, someone is already logged in somewhere else")
             )
         if self.object.type == "BAR":
-            kwargs["barmen"] = self.object.get_barmen_list()
+            kwargs["barmen"] = self.object.barmen_list
         elif self.request.user.is_authenticated:
             kwargs["barmen"] = [self.request.user]
         if "last_basket" in self.request.session.keys():
@@ -316,23 +323,17 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView):
 
     def dispatch(self, request, *args, **kwargs):
         self.customer = get_object_or_404(Customer, user__id=self.kwargs["user_id"])
-        obj = self.get_object()
+        obj: Counter = self.get_object()
         if not self.customer.can_buy:
             raise Http404
-        if obj.type == "BAR":
+        if obj.type != "BAR" and not request.user.is_authenticated:
-            if (
+            raise PermissionDenied
-                not (
+        if (
-                    "counter_token" in request.session.keys()
+            "counter_token" not in request.session
-                    and request.session["counter_token"] == obj.token
+            or request.session["counter_token"] != obj.token
-                )
+            or len(obj.barmen_list) == 0
-                or len(obj.get_barmen_list()) < 1
+        ):
-            ):
+            return redirect(obj)
-                return HttpResponseRedirect(
-                    reverse_lazy("counter:details", kwargs={"counter_id": obj.id})
-                )
-        else:
-            if not request.user.is_authenticated:
-                raise PermissionDenied
         return super().dispatch(request, *args, **kwargs)
 
     def get(self, request, *args, **kwargs):
@@ -347,7 +348,7 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView):
         self.refill_form = None
         ret = super().get(request, *args, **kwargs)
         if (self.object.type != "BAR" and not request.user.is_authenticated) or (
-            self.object.type == "BAR" and len(self.object.get_barmen_list()) < 1
+            self.object.type == "BAR" and len(self.object.barmen_list) == 0
         ):  # Check that at least one barman is logged in
             ret = self.cancel(request)  # Otherwise, go to main view
         return ret
@@ -357,7 +358,7 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView):
         self.object = self.get_object()
         self.refill_form = None
         if (self.object.type != "BAR" and not request.user.is_authenticated) or (
-            self.object.type == "BAR" and len(self.object.get_barmen_list()) < 1
+            self.object.type == "BAR" and len(self.object.barmen_list) < 1
         ):  # Check that at least one barman is logged in
             return self.cancel(request)
         if self.object.type == "BAR" and not (
@@ -532,20 +533,18 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView):
 
     def add_student_card(self, request):
         """Add a new student card on the customer account."""
-        uid = request.POST["student_card_uid"]
+        uid = str(request.POST["student_card_uid"])
-        uid = str(uid)
         if not StudentCard.is_valid(uid):
             request.session["not_valid_student_card_uid"] = True
             return False
 
         if not (
             self.object.type == "BAR"
-            and "counter_token" in request.session.keys()
+            and "counter_token" in request.session
             and request.session["counter_token"] == self.object.token
-            and len(self.object.get_barmen_list()) > 0
+            and self.object.is_open
         ):
             raise PermissionDenied
-
         StudentCard(customer=self.customer, uid=uid).save()
         return True
 
@@ -679,6 +678,7 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView):
                     product
                 )
         kwargs["customer"] = self.customer
+        kwargs["student_cards"] = self.customer.student_cards.all()
         kwargs["basket_total"] = self.sum_basket(self.request)
         kwargs["refill_form"] = self.refill_form or RefillForm()
         kwargs["student_card_max_uid_size"] = StudentCard.UID_SIZE
@@ -686,56 +686,34 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView):
         return kwargs
 
 
-class CounterLogin(RedirectView):
+@require_POST
-    """Handle the login of a barman.
+def counter_login(request: HttpRequest, counter_id: int) -> HttpResponseRedirect:
+    """Log a user in a counter.
 
-    Logged barmen are stored in the Permanency model
+    A successful login will result in the beginning of a counter duty
+    for the user.
     """
-
+    counter = get_object_or_404(Counter, pk=counter_id)
-    permanent = False
+    form = LoginForm(request, data=request.POST)
-
+    if not form.is_valid():
-    def post(self, request, *args, **kwargs):
+        return redirect(counter.get_absolute_url() + "?credentials")
-        """Register the logged user as barman for this counter."""
+    user = form.get_user()
-        self.counter_id = kwargs["counter_id"]
+    if not counter.sellers.contains(user) or user in counter.barmen_list:
-        self.counter = Counter.objects.filter(id=kwargs["counter_id"]).first()
+        return redirect(counter.get_absolute_url() + "?sellers")
-        form = LoginForm(request, data=request.POST)
+    if len(counter.barmen_list) == 0:
-        self.errors = []
+        counter.gen_token()
-        if form.is_valid():
+    request.session["counter_token"] = counter.token
-            user = User.objects.filter(username=form.cleaned_data["username"]).first()
+    counter.permanencies.create(user=user, start=timezone.now())
-            if (
+    return redirect(counter)
-                user in self.counter.sellers.all()
-                and not user in self.counter.get_barmen_list()
-            ):
-                if len(self.counter.get_barmen_list()) <= 0:
-                    self.counter.gen_token()
-                request.session["counter_token"] = self.counter.token
-                self.counter.add_barman(user)
-            else:
-                self.errors += ["sellers"]
-        else:
-            self.errors += ["credentials"]
-        return super().post(request, *args, **kwargs)
-
-    def get_redirect_url(self, *args, **kwargs):
-        return (
-            reverse_lazy("counter:details", args=args, kwargs=kwargs)
-            + "?"
-            + "&".join(self.errors)
-        )
 
 
-class CounterLogout(RedirectView):
+@require_POST
-    permanent = False
+def counter_logout(request: HttpRequest, counter_id: int) -> HttpResponseRedirect:
-
+    """End the permanency of a user in this counter."""
-    def post(self, request, *args, **kwargs):
+    Permanency.objects.filter(counter=counter_id, user=request.POST["user_id"]).update(
-        """Unregister the user from the barman."""
+        end=F("activity")
-        self.counter = Counter.objects.filter(id=kwargs["counter_id"]).first()
+    )
-        user = User.objects.filter(id=request.POST["user_id"]).first()
+    return redirect("counter:details", counter_id=counter_id)
-        self.counter.del_barman(user)
-        return super().post(request, *args, **kwargs)
-
-    def get_redirect_url(self, *args, **kwargs):
-        return reverse_lazy("counter:details", args=args, kwargs=kwargs)
 
 
 # Counter admin views
@@ -1196,7 +1174,7 @@ class CounterLastOperationsView(CounterTabsMixin, CanViewMixin, DetailView):
         """We have here again a very particular right handling."""
         self.object = self.get_object()
         if (
-            self.object.get_barmen_list()
+            self.object.barmen_list
             and "counter_token" in request.session.keys()
             and request.session["counter_token"]
             and Counter.objects.filter(  # check if not null for counters that have no token set
@@ -1236,7 +1214,7 @@ class CounterCashSummaryView(CounterTabsMixin, CanViewMixin, DetailView):
         """We have here again a very particular right handling."""
         self.object = self.get_object()
         if (
-            self.object.get_barmen_list()
+            self.object.barmen_list
             and "counter_token" in request.session.keys()
             and request.session["counter_token"]
             and Counter.objects.filter(  # check if not null for counters that have no token set