Merge pull request #1063 from ae-utbm/fixes

Fixes
This commit is contained in:
thomas girod 2025-04-06 21:28:45 +02:00 committed by GitHub
commit f254490790
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 228 additions and 143 deletions

View File

@ -19,7 +19,7 @@ import pytest
from django.conf import settings
from django.contrib.sites.models import Site
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import TestCase
from django.test import Client, TestCase
from django.urls import reverse
from django.utils import html
from django.utils.timezone import localtime, now
@ -323,7 +323,7 @@ class TestNewsCreation(TestCase):
@pytest.mark.django_db
def test_feed(client):
def test_feed(client: Client):
"""Smoke test that checks that the atom feed is working"""
Site.objects.clear_cache()
with assertNumQueries(2):
@ -332,3 +332,22 @@ def test_feed(client):
resp = client.get(reverse("com:news_feed"))
assert resp.status_code == 200
assert resp.headers["Content-Type"] == "application/rss+xml; charset=utf-8"
@pytest.mark.django_db
@pytest.mark.parametrize(
"url",
[
reverse("com:poster_list"),
reverse("com:poster_create"),
reverse("com:poster_moderate_list"),
],
)
def test_poster_management_views_crash_test(client: Client, url: str):
"""Test that poster management views work"""
user = baker.make(
User, groups=[Group.objects.get(pk=settings.SITH_GROUP_COM_ADMIN_ID)]
)
client.force_login(user)
res = client.get(url)
assert res.status_code == 200

View File

@ -61,8 +61,7 @@ sith = Sith.objects.first
class ComTabsMixin(TabedViewMixin):
def get_tabs_title(self):
return _("Communication administration")
tabs_title = _("Communication administration")
def get_list_of_tabs(self):
return [
@ -559,7 +558,11 @@ class MailingModerateView(View):
raise PermissionDenied
class PosterListBaseView(ListView):
class PosterAdminViewMixin(IsComAdminMixin, ComTabsMixin):
current_tab = "posters"
class PosterListBaseView(PosterAdminViewMixin, ListView):
"""List communication posters."""
current_tab = "posters"
@ -586,7 +589,7 @@ class PosterListBaseView(ListView):
return kwargs
class PosterCreateBaseView(CreateView):
class PosterCreateBaseView(PosterAdminViewMixin, CreateView):
"""Create communication poster."""
current_tab = "posters"
@ -618,7 +621,7 @@ class PosterCreateBaseView(CreateView):
return super().form_valid(form)
class PosterEditBaseView(UpdateView):
class PosterEditBaseView(PosterAdminViewMixin, UpdateView):
"""Edit communication poster."""
pk_url_kwarg = "poster_id"
@ -664,7 +667,7 @@ class PosterEditBaseView(UpdateView):
return super().form_valid(form)
class PosterDeleteBaseView(DeleteView):
class PosterDeleteBaseView(PosterAdminViewMixin, DeleteView):
"""Edit communication poster."""
pk_url_kwarg = "poster_id"
@ -681,7 +684,7 @@ class PosterDeleteBaseView(DeleteView):
return super().dispatch(request, *args, **kwargs)
class PosterListView(IsComAdminMixin, ComTabsMixin, PosterListBaseView):
class PosterListView(PosterListBaseView):
"""List communication posters."""
def get_context_data(self, **kwargs):
@ -690,7 +693,7 @@ class PosterListView(IsComAdminMixin, ComTabsMixin, PosterListBaseView):
return kwargs
class PosterCreateView(IsComAdminMixin, ComTabsMixin, PosterCreateBaseView):
class PosterCreateView(PosterCreateBaseView):
"""Create communication poster."""
success_url = reverse_lazy("com:poster_list")
@ -701,7 +704,7 @@ class PosterCreateView(IsComAdminMixin, ComTabsMixin, PosterCreateBaseView):
return kwargs
class PosterEditView(IsComAdminMixin, ComTabsMixin, PosterEditBaseView):
class PosterEditView(PosterEditBaseView):
"""Edit communication poster."""
success_url = reverse_lazy("com:poster_list")
@ -712,13 +715,13 @@ class PosterEditView(IsComAdminMixin, ComTabsMixin, PosterEditBaseView):
return kwargs
class PosterDeleteView(IsComAdminMixin, ComTabsMixin, PosterDeleteBaseView):
class PosterDeleteView(PosterDeleteBaseView):
"""Delete communication poster."""
success_url = reverse_lazy("com:poster_list")
class PosterModerateListView(IsComAdminMixin, ComTabsMixin, ListView):
class PosterModerateListView(PosterAdminViewMixin, ListView):
"""Moderate list communication poster."""
current_tab = "posters"
@ -732,7 +735,7 @@ class PosterModerateListView(IsComAdminMixin, ComTabsMixin, ListView):
return kwargs
class PosterModerateView(IsComAdminMixin, ComTabsMixin, View):
class PosterModerateView(PosterAdminViewMixin, View):
"""Moderate communication poster."""
def get(self, request, *args, **kwargs):

View File

@ -4,9 +4,10 @@ import pytest
from django.conf import settings
from django.contrib import auth
from django.core.management import call_command
from django.test import Client, TestCase
from django.test import Client, RequestFactory, TestCase
from django.urls import reverse
from django.utils.timezone import now
from django.views.generic import DetailView
from model_bakery import baker, seq
from model_bakery.recipe import Recipe, foreign_key
from pytest_django.asserts import assertRedirects
@ -18,6 +19,7 @@ from core.baker_recipes import (
very_old_subscriber_user,
)
from core.models import Group, User
from core.views import UserTabsMixin
from counter.models import Counter, Refilling, Selling
from eboutic.models import Invoice, InvoiceItem
@ -229,3 +231,88 @@ def test_logout(client: Client):
res = client.post(reverse("core:logout"))
assertRedirects(res, reverse("core:login"))
assert auth.get_user(client).is_anonymous
class UserTabTestView(UserTabsMixin, DetailView): ...
@pytest.mark.django_db
@pytest.mark.parametrize(
["user_factory", "expected_tabs"],
[
(
subscriber_user.make,
[
"infos",
"godfathers",
"pictures",
"tools",
"edit",
"prefs",
"clubs",
"stats",
"account",
],
),
(
# this user is superuser, but still won't see a few tabs,
# because he is not subscribed
lambda: baker.make(User, is_superuser=True),
[
"infos",
"godfathers",
"pictures",
"tools",
"edit",
"prefs",
"clubs",
"groups",
],
),
],
)
def test_displayed_user_self_tabs(user_factory, expected_tabs: list[str]):
"""Test that a user can view the appropriate tabs in its own profile"""
user = user_factory()
request = RequestFactory().get("")
request.user = user
view = UserTabTestView()
view.setup(request)
view.object = user
tabs = [tab["slug"] for tab in view.get_list_of_tabs()]
assert tabs == expected_tabs
@pytest.mark.django_db
@pytest.mark.parametrize(
["user_factory", "expected_tabs"],
[
(subscriber_user.make, ["infos", "godfathers", "pictures", "clubs"]),
(
# this user is superuser, but still won't see a few tabs,
# because he is not subscribed
lambda: baker.make(User, is_superuser=True),
[
"infos",
"godfathers",
"pictures",
"edit",
"prefs",
"clubs",
"groups",
"stats",
"account",
],
),
],
)
def test_displayed_other_user_tabs(user_factory, expected_tabs: list[str]):
"""Test that a user can view the appropriate tabs in another user's profile."""
request_user = user_factory()
request = RequestFactory().get("")
request.user = request_user
view = UserTabTestView()
view.setup(request)
view.object = subscriber_user.make() # user whose page is being seen
tabs = [tab["slug"] for tab in view.get_list_of_tabs()]
assert tabs == expected_tabs

View File

@ -242,7 +242,10 @@ class UserTabsMixin(TabedViewMixin):
if (
hasattr(user, "customer")
and user.customer
and (user == self.request.user or user.has_perm("counter.view_customer"))
and (
user == self.request.user
or self.request.user.has_perm("counter.view_customer")
)
):
tab_list.append(
{

View File

@ -0,0 +1,19 @@
# Generated by Django 4.2.20 on 2025-04-06 11:29
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
("counter", "0030_returnableproduct_returnableproductbalance_and_more")
]
operations = [
migrations.AlterModelOptions(
name="counter",
options={
"permissions": [("view_counter_stats", "Can view counter stats")],
"verbose_name": "counter",
},
),
]

View File

@ -526,6 +526,7 @@ class Counter(models.Model):
class Meta:
verbose_name = _("counter")
permissions = [("view_counter_stats", "Can view counter stats")]
def __str__(self):
return self.name
@ -598,13 +599,12 @@ class Counter(models.Model):
- the promo of the barman
- the total number of office hours the barman did attend
"""
name_expr = Concat(F("user__first_name"), Value(" "), F("user__last_name"))
return (
self.permanencies.exclude(end=None)
.annotate(
name=Concat(F("user__first_name"), Value(" "), F("user__last_name"))
name=name_expr, nickname=F("user__nick_name"), promo=F("user__promo")
)
.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)
@ -628,18 +628,17 @@ class Counter(models.Model):
since = get_start_of_semester()
if isinstance(since, date):
since = datetime(since.year, since.month, since.day, tzinfo=tz.utc)
name_expr = Concat(
F("customer__user__first_name"), Value(" "), F("customer__user__last_name")
)
return (
self.sellings.filter(date__gte=since)
.annotate(
name=Concat(
F("customer__user__first_name"),
Value(" "),
F("customer__user__last_name"),
)
name=name_expr,
nickname=F("customer__user__nick_name"),
promo=F("customer__user__promo"),
user=F("customer__user"),
)
.annotate(nickname=F("customer__user__nick_name"))
.annotate(promo=F("customer__user__promo"))
.annotate(user=F("customer__user"))
.values("user", "promo", "name", "nickname")
.annotate(
selling_sum=Sum(

View File

@ -18,7 +18,7 @@ from decimal import Decimal
import pytest
from django.conf import settings
from django.contrib.auth.models import make_password
from django.contrib.auth.models import Permission, make_password
from django.core.cache import cache
from django.http import HttpResponse
from django.shortcuts import resolve_url
@ -28,9 +28,10 @@ from django.utils import timezone
from django.utils.timezone import localdate, now
from freezegun import freeze_time
from model_bakery import baker
from model_bakery.recipe import Recipe
from pytest_django.asserts import assertRedirects
from club.models import Club, Membership
from club.models import Membership
from core.baker_recipes import board_user, subscriber_user, very_old_subscriber_user
from core.models import BanGroup, User
from counter.baker_recipes import product_recipe, sale_recipe
@ -572,121 +573,86 @@ class TestCounterClick(TestFullClickBase):
class TestCounterStats(TestCase):
@classmethod
def setUpTestData(cls):
cls.counter = Counter.objects.get(id=2)
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.root, cls.skia, cls.krophil)
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,
cls.users = subscriber_user.make(_quantity=4)
product = product_recipe.make(selling_price=1)
cls.counter = baker.make(
Counter, type=["BAR"], sellers=cls.users[:4], products=[product]
)
# 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,
)
_now = timezone.now()
permanence_recipe = Recipe(Permanency, counter=cls.counter)
perms = [
*[ # total of user 0 : 5 hours
permanence_recipe.prepare(user=cls.users[0], start=start, end=end)
for start, end in [
(_now, _now + timedelta(hours=1)),
(_now + timedelta(hours=4), _now + timedelta(hours=6)),
(_now + timedelta(hours=7), _now + timedelta(hours=9)),
]
],
*[ # total of user 1 : 16 days, 2 hours, 35 minutes and 54 seconds
permanence_recipe.prepare(user=cls.users[1], start=start, end=end)
for start, end in [
(_now, _now + timedelta(hours=1)),
(
_now + timedelta(days=4, hours=1),
_now + timedelta(days=20, hours=2, minutes=35, seconds=54),
),
]
],
*[ # total of user 2 : 2 hour + 20 hours (but the 20 hours were on last year)
permanence_recipe.prepare(user=cls.users[2], start=start, end=end)
for start, end in [
(_now + timedelta(days=5), _now + timedelta(days=5, hours=1)),
(_now - timedelta(days=300, hours=20), _now - timedelta(days=300)),
]
],
]
# user 3 has 0 hours of permanence
Permanency.objects.bulk_create(perms)
# 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=baker.make(Club),
_sale_recipe = Recipe(
Selling,
club=cls.counter.club,
counter=cls.counter,
product=product,
unit_price=2,
seller=cls.skia,
)
sales = [
*_sale_recipe.prepare(
quantity=100, customer=cls.users[0].customer, _quantity=10
), # 2000 €
*_sale_recipe.prepare(
quantity=100, customer=cls.users[1].customer, _quantity=5
), # 1000 €
_sale_recipe.prepare(quantity=1, customer=cls.users[2].customer), # 2€
_sale_recipe.prepare(quantity=50, customer=cls.users[3].customer), # 100€
]
Selling.objects.bulk_create(sales)
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]))
assert response.status_code == 403
def test_not_authenticated_access_fail(self):
url = reverse("counter:stats", args=[self.counter.id])
response = self.client.get(url)
assertRedirects(response, reverse("core:login") + f"?next={url}")
def test_unauthorized_user_fails(self):
self.client.force_login(User.objects.get(username="public"))
self.client.force_login(baker.make(User))
response = self.client.get(reverse("counter:stats", args=[self.counter.id]))
assert response.status_code == 403
def test_authorized_user_ok(self):
perm = Permission.objects.get(codename="view_counter_stats")
self.client.force_login(baker.make(User, user_permissions=[perm]))
response = self.client.get(reverse("counter:stats", args=[self.counter.id]))
assert response.status_code == 200
def test_get_total_sales(self):
"""Test the result of the Counter.get_total_sales() method."""
assert self.counter.get_total_sales() == 3102
def test_top_barmen(self):
"""Test the result of Counter.get_top_barmen() is correct."""
users = [self.skia, self.root, self.sli]
users = [self.users[1], self.users[2], self.users[0]]
perm_times = [
timedelta(days=16, hours=2, minutes=35, seconds=54),
timedelta(hours=21),
@ -700,12 +666,12 @@ class TestCounterStats(TestCase):
"nickname": user.nick_name,
"perm_sum": perm_time,
}
for user, perm_time in zip(users, perm_times, strict=False)
for user, perm_time in zip(users, perm_times, strict=True)
]
def test_top_customer(self):
"""Test the result of Counter.get_top_customers() is correct."""
users = [self.sli, self.skia, self.krophil, self.root]
users = [self.users[0], self.users[1], self.users[3], self.users[2]]
sale_amounts = [2000, 1000, 100, 2]
assert list(self.counter.get_top_customers()) == [
{
@ -715,7 +681,7 @@ class TestCounterStats(TestCase):
"nickname": user.nick_name,
"selling_sum": sale_amount,
}
for user, sale_amount in zip(users, sale_amounts, strict=False)
for user, sale_amount in zip(users, sale_amounts, strict=True)
]

View File

@ -27,7 +27,7 @@ from django.utils.translation import gettext as _
from django.views.generic import DetailView, ListView, TemplateView
from django.views.generic.edit import CreateView, DeleteView, FormView, UpdateView
from core.auth.mixins import CanEditMixin, CanViewMixin
from core.auth.mixins import CanViewMixin
from core.utils import get_semester_code, get_start_of_semester
from counter.forms import (
CloseCustomerAccountForm,
@ -274,12 +274,13 @@ class SellingDeleteView(DeleteView):
raise PermissionDenied
class CounterStatView(DetailView, CounterAdminMixin):
class CounterStatView(PermissionRequiredMixin, DetailView):
"""Show the bar stats."""
model = Counter
pk_url_kwarg = "counter_id"
template_name = "counter/stats.jinja"
permission_required = "counter.view_counter_stats"
def get_context_data(self, **kwargs):
"""Add stats to the context."""
@ -301,18 +302,6 @@ class CounterStatView(DetailView, CounterAdminMixin):
)
return kwargs
def dispatch(self, request, *args, **kwargs):
try:
return super().dispatch(request, *args, **kwargs)
except PermissionDenied:
if (
request.user.is_root
or request.user.is_board_member
or self.get_object().is_owned_by(request.user)
):
return super(CanEditMixin, self).dispatch(request, *args, **kwargs)
raise PermissionDenied
class CounterRefillingListView(CounterAdminTabsMixin, CounterAdminMixin, ListView):
"""List of refillings on a counter."""