remove-useless-queries-counter-stats (#519)

counter/models.py

@@ -22,13 +22,12 @@
 #
 #
 from __future__ import annotations
-from django.db.models import Sum, F
 
 from typing import Tuple
 
 from django.db import models
-from django.db.models import OuterRef, Exists
-from django.db.models.functions import Length
+from django.db.models import F, Value, Sum, QuerySet, OuterRef, Exists
+from django.db.models.functions import Concat, Length
 from django.utils.translation import gettext_lazy as _
 from django.utils import timezone
 from django.conf import settings
@@ -37,14 +36,14 @@ from django.core.validators import MinLengthValidator
 from django.forms import ValidationError
 from django.utils.functional import cached_property
 
-from datetime import timedelta, date
+from datetime import timedelta, date, datetime
 import random
 import string
 import os
 import base64
-import datetime
 from dict2xml import dict2xml
 
+from core.utils import get_start_of_semester
 from sith.settings import SITH_COUNTER_OFFICES, SITH_MAIN_CLUB
 from club.models import Club, Membership
 from accounting.models import CurrencyField
@@ -92,8 +91,9 @@ class Customer(models.Model):
         don't mix them) and a Product.
         """
         subscription = self.user.subscriptions.order_by("subscription_end").last()
-        time_diff = date.today() - subscription.subscription_end
-        return subscription is not None and time_diff < timedelta(days=90)
+        if subscription is None:
+            return False
+        return (date.today() - subscription.subscription_end) < timedelta(days=90)
 
     @classmethod
     def get_or_create(cls, user: User) -> Tuple[Customer, bool]:
@@ -491,7 +491,7 @@ class Counter(models.Model):
         """
         return self.is_open() and (
             (timezone.now() - self.permanencies.order_by("-activity").first().activity)
-            > datetime.timedelta(minutes=settings.SITH_COUNTER_MINUTE_INACTIVE)
+            > timedelta(minutes=settings.SITH_COUNTER_MINUTE_INACTIVE)
         )
 
     def barman_list(self):
@@ -517,6 +517,77 @@ class Counter(models.Model):
                 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
+        of this counter, ordered by descending number of hours.
+
+        Each element of the QuerySet corresponds to a barman and has the following data :
+            - the full name (first name + last name) of the barman
+            - the nickname of the barman
+            - the promo of the barman
+            - the total number of office hours the barman did attend
+        """
+        return (
+            self.permanencies.exclude(end=None)
+            .annotate(
+                name=Concat(F("user__first_name"), Value(" "), F("user__last_name"))
+            )
+            .annotate(nickname=F("user__nick_name"))
+            .annotate(promo=F("user__promo"))
+            .values("user", "name", "nickname", "promo")
+            .annotate(perm_sum=Sum(F("end") - F("start")))
+            .exclude(perm_sum=None)
+            .order_by("-perm_sum")
+        )
+
+    def get_top_customers(self, since=get_start_of_semester()) -> QuerySet:
+        """
+        Return a QuerySet querying the money spent by customers of this counter
+        since the specified date, ordered by descending amount of money spent.
+
+        Each element of the QuerySet corresponds to a customer and has the following data :
+            - the full name (first name + last name) of the customer
+            - the nickname of the customer
+            - the amount of money spent by the customer
+        """
+        return (
+            self.sellings.filter(date__gte=since)
+            .annotate(
+                name=Concat(
+                    F("customer__user__first_name"),
+                    Value(" "),
+                    F("customer__user__last_name"),
+                )
+            )
+            .annotate(nickname=F("customer__user__nick_name"))
+            .annotate(promo=F("customer__user__promo"))
+            .values("customer__user", "name", "nickname")
+            .annotate(
+                selling_sum=Sum(
+                    F("unit_price") * F("quantity"), output_field=CurrencyField()
+                )
+            )
+            .filter(selling_sum__gt=0)
+            .order_by("-selling_sum")
+        )
+
+    def get_total_sales(self, since=get_start_of_semester()) -> CurrencyField:
+        """
+        Compute and return the total turnover of this counter
+        since the date specified in parameter (by default, since the start of the current
+        semester)
+        :param since: timestamp from which to perform the calculation
+        :type since: datetime | date
+        :return: Total revenue earned at this counter
+        """
+        if isinstance(since, date):
+            since = datetime.combine(since, datetime.min.time())
+        total = self.sellings.filter(date__gte=since).aggregate(
+            total=Sum(F("quantity") * F("unit_price"), output_field=CurrencyField())
+        )["total"]
+        return total if total is not None else CurrencyField(0)
+
 
 class Refilling(models.Model):
     """

counter/templates/counter/stats.jinja

@@ -2,43 +2,34 @@
 {% from 'core/macros.jinja' import user_profile_link %}
 
 {% block title %}
-{% trans counter_name=counter %}{{ counter_name }} stats{% endtrans %}
+    {% trans counter_name=counter %}{{ counter_name }} stats{% endtrans %}
+{% endblock %}
+
+{% block jquery_css %}
+    {# Remove jquery_css #}
 {% endblock %}
 
 {% block content %}
-<h3>{% trans counter_name=counter %}{{ counter_name }} stats{% endtrans %}</h3>
+    <h3>{% trans counter_name=counter %}{{ counter_name }} stats{% endtrans %}</h3>
     <h4>{% trans counter_name=counter.name %}Top 100 {{ counter_name }}{% endtrans %}</h4>
     <table>
         <thead>
-            <tr>
-                <td>{% trans %}Nb{% endtrans %}</td>
-                <td>{% trans %}User{% endtrans %}</td>
-                <td>{% trans %}Promo{% endtrans %}</td>
-                <td>{% trans %}Clubs{% endtrans %}</td>
-                <td>{% trans %}Total{% endtrans %}</td>
-                <td>{% trans %}Percentage{% endtrans %}</td>
-            </tr>
+        <tr>
+            <td>N°</td>
+            <td>{% trans %}User{% endtrans %}</td>
+            <td>{% trans %}Promo{% endtrans %}</td>
+            <td>{% trans %}Total{% endtrans %}</td>
+            <td>{% trans %}Percentage{% endtrans %}</td>
+        </tr>
         </thead>
         <tbody>
-        {% for r in top %}
-            {% set customer=Customer.objects.filter(user__id=r.customer__user).first() %}
-            {% if customer.user == user %}
-            <tr class="highlight">
-            {% else %}
-            <tr>
-            {% endif %}
+        {% for customer in top_customers %}
+            <tr class="{% if customer.user == request.user.id %}highlight{% endif %}">
                 <td>{{ loop.index }}</td>
-                <td>{{ customer.user.get_display_name() }}</td>
-                <td>{{ customer.user.promo or '' }}</td>
-                <td>
-                {% for m in customer.user.memberships.filter(club__parent=None, end_date=None,
-                        role__gt=settings.SITH_MAXIMUM_FREE_ROLE).all() -%}
-                    {%- if loop.index>1 -%}, {% endif -%}
-                    {{ m.club.name }}
-                {%- endfor %}
-                </td>
-                <td>{{ r.selling_sum }} €</td>
-                <td>{{ '%.2f'|format(100 * r.selling_sum / total_sellings) }}</td>
+                <td>{{ customer.name }} {% if customer.nickname %} ({{ customer.nickname }}) {% endif %}</td>
+                <td>{{ customer.promo or '' }}</td>
+                <td>{{ "%.2f"|format(customer.selling_sum) }} €</td>
+                <td>{{ '%.2f'|format(100 * customer.selling_sum / total_sellings) }}%</td>
             </tr>
         {% endfor %}
         </tbody>
@@ -47,23 +38,20 @@
     <h4>{% trans counter_name=counter.name %}Top 100 barman {{ counter_name }}{% endtrans %}</h4>
     <table>
         <thead>
-            <tr>
-                <td>{% trans %}Nb{% endtrans %}</td>
-                <td>{% trans %}User{% endtrans %}</td>
-                <td>{% trans %}Time{% endtrans %}</td>
-            </tr>
+        <tr>
+            <td>N°</td>
+            <td>{% trans %}User{% endtrans %}</td>
+            <td>{% trans %}Promo{% endtrans %}</td>
+            <td>{% trans %}Time{% endtrans %}</td>
+        </tr>
         </thead>
         <tbody>
-        {% for r in top_barman_semester %}
-            {% set u=User.objects.filter(id=r.user).first() %}
-            {% if u == user %}
-            <tr class="highlight">
-            {% else %}
-            <tr>
-            {% endif %}
+        {% for barman in top_barman_semester %}
+            <tr {% if barman.user == request.user.id %}class="highlight"{% endif %}>
                 <td>{{ loop.index }}</td>
-                <td>{{ u.get_display_name() }}</td>
-                <td>{{ r.perm_sum }}</td>
+                <td>{{ barman.name }} {% if barman.nickname %}({{ barman.nickname }}){% endif %}</td>
+                <td>{{ barman.promo or '' }}</td>
+                <td>{{ barman.perm_sum|format_timedelta|truncate_time("millis") }}</td>
             </tr>
         {% endfor %}
         </tbody>
@@ -72,23 +60,20 @@
     <h4>{% trans counter_name=counter.name %}Top 100 barman {{ counter_name }} (all semesters){% endtrans %}</h4>
     <table>
         <thead>
-            <tr>
-                <td>{% trans %}Nb{% endtrans %}</td>
-                <td>{% trans %}User{% endtrans %}</td>
-                <td>{% trans %}Time{% endtrans %}</td>
-            </tr>
+        <tr>
+            <td>N°</td>
+            <td>{% trans %}User{% endtrans %}</td>
+            <td>{% trans %}Promo{% endtrans %}</td>
+            <td>{% trans %}Time{% endtrans %}</td>
+        </tr>
         </thead>
         <tbody>
-        {% for r in top_barman %}
-            {% set u=User.objects.filter(id=r.user).first() %}
-            {% if u == user %}
-            <tr class="highlight">
-            {% else %}
-            <tr>
-            {% endif %}
+        {% for barman in top_barman %}
+            <tr {% if barman.user == request.user.id %}class="highlight"{% endif %}>
                 <td>{{ loop.index }}</td>
-                <td>{{ u.get_display_name() }}</td>
-                <td>{{ r.perm_sum }}</td>
+                <td>{{ barman.name }} {% if barman.nickname %}({{ barman.nickname }}){% endif %}</td>
+                <td>{{ barman.promo or '' }}</td>
+                <td>{{ barman.perm_sum|format_timedelta|truncate_time("millis") }}</td>
             </tr>
         {% endfor %}
         </tbody>

counter/tests.py

@@ -28,9 +28,13 @@ import string
 from django.test import TestCase
 from django.urls import reverse
 from django.core.management import call_command
+from django.utils import timezone
+from django.utils.timezone import timedelta
 
+from club.models import Club
 from core.models import User
-from counter.models import Counter, Customer, BillingInfo
+from counter.models import Counter, Customer, BillingInfo, Permanency, Selling, Product
+from sith.settings import SITH_MAIN_CLUB
 
 
 class CounterTest(TestCase):
@@ -164,15 +168,211 @@ class CounterTest(TestCase):
 
 
 class CounterStatsTest(TestCase):
-    def setUp(self):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
         call_command("populate")
-        self.counter = Counter.objects.filter(id=2).first()
+        cls.counter = Counter.objects.filter(id=2).first()
+        cls.krophil = User.objects.get(username="krophil")
+        cls.skia = User.objects.get(username="skia")
+        cls.sli = User.objects.get(username="sli")
+        cls.root = User.objects.get(username="root")
+        cls.subscriber = User.objects.get(username="subscriber")
+        cls.old_subscriber = User.objects.get(username="old_subscriber")
+        cls.counter.sellers.add(cls.sli)
+        cls.counter.sellers.add(cls.root)
+        cls.counter.sellers.add(cls.skia)
+        cls.counter.sellers.add(cls.krophil)
 
-    def test_unauthorised_user_fail(self):
+        barbar = Product.objects.get(code="BARB")
+
+        # remove everything to make sure the fixtures bring no side effect
+        Permanency.objects.all().delete()
+        Selling.objects.all().delete()
+
+        now = timezone.now()
+        # total of sli : 5 hours
+        Permanency.objects.create(
+            user=cls.sli, start=now, end=now + timedelta(hours=1), counter=cls.counter
+        )
+        Permanency.objects.create(
+            user=cls.sli,
+            start=now + timedelta(hours=4),
+            end=now + timedelta(hours=6),
+            counter=cls.counter,
+        )
+        Permanency.objects.create(
+            user=cls.sli,
+            start=now + timedelta(hours=7),
+            end=now + timedelta(hours=9),
+            counter=cls.counter,
+        )
+
+        # total of skia : 16 days, 2 hours, 35 minutes and 54 seconds
+        Permanency.objects.create(
+            user=cls.skia, start=now, end=now + timedelta(hours=1), counter=cls.counter
+        )
+        Permanency.objects.create(
+            user=cls.skia,
+            start=now + timedelta(days=4, hours=1),
+            end=now + timedelta(days=20, hours=2, minutes=35, seconds=54),
+            counter=cls.counter,
+        )
+
+        # total of root : 1 hour + 20 hours (but the 20 hours were on last year)
+        Permanency.objects.create(
+            user=cls.root,
+            start=now + timedelta(days=5),
+            end=now + timedelta(days=5, hours=1),
+            counter=cls.counter,
+        )
+        Permanency.objects.create(
+            user=cls.root,
+            start=now - timedelta(days=300, hours=20),
+            end=now - timedelta(days=300),
+            counter=cls.counter,
+        )
+
+        # total of krophil : 0 hour
+        s = Selling(
+            label=barbar.name,
+            product=barbar,
+            club=Club.objects.get(name=SITH_MAIN_CLUB["name"]),
+            counter=cls.counter,
+            unit_price=2,
+            seller=cls.skia,
+        )
+
+        krophil_customer = Customer.get_or_create(cls.krophil)[0]
+        sli_customer = Customer.get_or_create(cls.sli)[0]
+        skia_customer = Customer.get_or_create(cls.skia)[0]
+        root_customer = Customer.get_or_create(cls.root)[0]
+
+        # moderate drinker. Total : 100 €
+        s.quantity = 50
+        s.customer = krophil_customer
+        s.save(allow_negative=True)
+
+        # Sli is a drunkard. Total : 2000 €
+        s.quantity = 100
+        s.customer = sli_customer
+        for _ in range(10):
+            # little trick to make sure the instance is duplicated in db
+            s.pk = None
+            s.save(allow_negative=True)  # save ten different sales
+
+        # Skia is a heavy drinker too. Total : 1000 €
+        s.customer = skia_customer
+        for _ in range(5):
+            s.pk = None
+            s.save(allow_negative=True)
+
+        # Root is quite an abstemious one. Total : 2 €
+        s.pk = None
+        s.quantity = 1
+        s.customer = root_customer
+        s.save(allow_negative=True)
+
+    def test_not_authenticated_user_fail(self):
         # Test with not login user
         response = self.client.get(reverse("counter:stats", args=[self.counter.id]))
         self.assertTrue(response.status_code == 403)
 
+    def test_unauthorized_user_fails(self):
+        user = User.objects.get(username="public")
+        self.client.login(username=user.username, password="plop")
+        response = self.client.get(reverse("counter:stats", args=[self.counter.id]))
+        self.assertTrue(response.status_code == 403)
+
+    def test_get_total_sales(self):
+        """
+        Test the result of the Counter.get_total_sales() method
+        """
+        total = self.counter.get_total_sales()
+        self.assertEqual(total, 3102)
+
+    def test_top_barmen(self):
+        """
+        Test the result of Counter.get_top_barmen() is correct
+        """
+        top = iter(self.counter.get_top_barmen())
+        self.assertEqual(
+            next(top),
+            {
+                "user": self.skia.id,
+                "name": f"{self.skia.first_name} {self.skia.last_name}",
+                "promo": self.skia.promo,
+                "nickname": self.skia.nick_name,
+                "perm_sum": timedelta(days=16, hours=2, minutes=35, seconds=54),
+            },
+        )
+        self.assertEqual(
+            next(top),
+            {
+                "user": self.root.id,
+                "name": f"{self.root.first_name} {self.root.last_name}",
+                "promo": self.root.promo,
+                "nickname": self.root.nick_name,
+                "perm_sum": timedelta(hours=21),
+            },
+        )
+        self.assertEqual(
+            next(top),
+            {
+                "user": self.sli.id,
+                "name": f"{self.sli.first_name} {self.sli.last_name}",
+                "promo": self.sli.promo,
+                "nickname": self.sli.nick_name,
+                "perm_sum": timedelta(hours=5),
+            },
+        )
+        self.assertIsNone(
+            next(top, None), msg="barmen with no office hours should not be in the top"
+        )
+
+    def test_top_customer(self):
+        """
+        Test the result of Counter.get_top_customers() is correct
+        """
+        top = iter(self.counter.get_top_customers())
+        self.assertEqual(
+            next(top),
+            {
+                "customer__user": self.sli.id,
+                "name": f"{self.sli.first_name} {self.sli.last_name}",
+                "nickname": self.sli.nick_name,
+                "selling_sum": 2000,
+            },
+        )
+        self.assertEqual(
+            next(top),
+            {
+                "customer__user": self.skia.id,
+                "name": f"{self.skia.first_name} {self.skia.last_name}",
+                "nickname": self.skia.nick_name,
+                "selling_sum": 1000,
+            },
+        )
+        self.assertEqual(
+            next(top),
+            {
+                "customer__user": self.krophil.id,
+                "name": f"{self.krophil.first_name} {self.krophil.last_name}",
+                "nickname": self.krophil.nick_name,
+                "selling_sum": 100,
+            },
+        )
+        self.assertEqual(
+            next(top),
+            {
+                "customer__user": self.root.id,
+                "name": f"{self.root.first_name} {self.root.last_name}",
+                "nickname": self.root.nick_name,
+                "selling_sum": 2,
+            },
+        )
+        self.assertIsNone(next(top, None))
+
 
 class BillingInfoTest(TestCase):
     @classmethod

counter/views.py

@@ -48,13 +48,15 @@ from django.utils import timezone
 from django import forms
 from django.utils.translation import gettext_lazy as _
 from django.conf import settings
-from django.db import DataError, transaction, models
+from django.db import DataError, transaction
 
+import json
 import re
 import pytz
-from datetime import date, timedelta, datetime
+from datetime import timedelta, datetime
 from http import HTTPStatus
 
+from core.utils import get_start_of_semester
 from core.views import CanViewMixin, TabedViewMixin, CanEditMixin
 from core.views.forms import LoginForm
 from core.models import User
@@ -68,7 +70,6 @@ from counter.forms import (
     CashSummaryFormBase,
     EticketForm,
 )
-from subscription.models import Subscription
 from counter.models import (
     Counter,
     Customer,
@@ -80,7 +81,6 @@ from counter.models import (
     CashRegisterSummary,
     CashRegisterSummaryItem,
     Eticket,
-    Permanency,
     BillingInfo,
 )
 from accounting.models import CurrencyField
@@ -1365,80 +1365,21 @@ class CounterStatView(DetailView, CounterAdminMixin):
 
     def get_context_data(self, **kwargs):
         """Add stats to the context"""
-        from django.db.models import Sum, Case, When, F
-
+        counter = self.object
+        semester_start = get_start_of_semester()
+        office_hours = counter.get_top_barmen()
         kwargs = super(CounterStatView, self).get_context_data(**kwargs)
-        kwargs["Customer"] = Customer
-        kwargs["User"] = User
-        semester_start = Subscription.compute_start(d=date.today(), duration=3)
-        kwargs["total_sellings"] = Selling.objects.filter(
-            date__gte=semester_start, counter=self.object
-        ).aggregate(
-            total_sellings=Sum(
-                F("quantity") * F("unit_price"), output_field=CurrencyField()
-            )
-        )[
-            "total_sellings"
-        ]
-        kwargs["top"] = (
-            Selling.objects.values("customer__user")
-            .annotate(
-                selling_sum=Sum(
-                    Case(
-                        When(
-                            counter=self.object,
-                            date__gte=semester_start,
-                            unit_price__gt=0,
-                            then=F("unit_price") * F("quantity"),
-                        ),
-                        output_field=CurrencyField(),
-                    )
-                )
-            )
-            .exclude(selling_sum=None)
-            .order_by("-selling_sum")
-            .all()[:100]
+        kwargs.update(
+            {
+                "counter": counter,
+                "total_sellings": counter.get_total_sales(since=semester_start),
+                "top_customers": counter.get_top_customers(since=semester_start)[:100],
+                "top_barman": office_hours[:100],
+                "top_barman_semester": (
+                    office_hours.filter(start__gt=semester_start)[:100]
+                ),
+            }
         )
-        kwargs["top_barman"] = (
-            Permanency.objects.values("user")
-            .annotate(
-                perm_sum=Sum(
-                    Case(
-                        When(
-                            counter=self.object,
-                            end__gt=datetime(year=1999, month=1, day=1),
-                            then=F("end") - F("start"),
-                        ),
-                        output_field=models.DateTimeField(),
-                    )
-                )
-            )
-            .exclude(perm_sum=None)
-            .order_by("-perm_sum")
-            .all()[:100]
-        )
-        kwargs["top_barman_semester"] = (
-            Permanency.objects.values("user")
-            .annotate(
-                perm_sum=Sum(
-                    Case(
-                        When(
-                            counter=self.object,
-                            start__gt=semester_start,
-                            end__gt=datetime(year=1999, month=1, day=1),
-                            then=F("end") - F("start"),
-                        ),
-                        output_field=models.DateTimeField(),
-                    )
-                )
-            )
-            .exclude(perm_sum=None)
-            .order_by("-perm_sum")
-            .all()[:100]
-        )
-
-        kwargs["sith_date"] = settings.SITH_START_DATE[0]
-        kwargs["semester_start"] = semester_start
         return kwargs
 
     def dispatch(self, request, *args, **kwargs):