From cde864fdc7d7dcb9fe480a60e525aea0ff86c448 Mon Sep 17 00:00:00 2001 From: Sli Date: Sun, 15 Dec 2024 21:33:43 +0100 Subject: [PATCH] Apply review comments --- counter/api.py | 11 +- counter/models.py | 9 ++ counter/schemas.py | 4 +- .../bundled/counter/counter-click-index.ts | 149 ++++++++++++------ counter/tests/test_api.py | 105 ++++++++++++ counter/views/click.py | 18 +-- 6 files changed, 226 insertions(+), 70 deletions(-) create mode 100644 counter/tests/test_api.py diff --git a/counter/api.py b/counter/api.py index d58f0154..e51aea26 100644 --- a/counter/api.py +++ b/counter/api.py @@ -26,7 +26,7 @@ from counter.models import Counter, Customer, Product from counter.schemas import ( CounterFilterSchema, CounterSchema, - CustomerBalance, + CustomerSchema, ProductSchema, SimplifiedCounterSchema, ) @@ -63,8 +63,13 @@ class CounterController(ControllerBase): @api_controller("/customer") class CustomerController(ControllerBase): - @route.get("/balance", response=CustomerBalance, permissions=[IsLoggedInCounter]) - def get_balance(self, customer_id: int): + @route.get( + "{customer_id}", + response=CustomerSchema, + permissions=[IsLoggedInCounter], + url_name="get_customer", + ) + def get_customer(self, customer_id: int): return self.get_object_or_exception(Customer, pk=customer_id) diff --git a/counter/models.py b/counter/models.py index 292cd59f..da2f33f4 100644 --- a/counter/models.py +++ b/counter/models.py @@ -650,6 +650,15 @@ class Counter(models.Model): ) )["total"] + def customer_is_barman(self, customer: Customer | User) -> bool: + """Check if current counter is a `bar` and that the customer is on the barmen_list + + This is useful to compute special prices""" + if isinstance(customer, Customer): + customer: User = customer.user + + return self.type == "BAR" and customer in self.barmen_list + class RefillingQuerySet(models.QuerySet): def annotate_total(self) -> Self: diff --git a/counter/schemas.py b/counter/schemas.py index 4fbbc712..7fbe1a71 100644 --- a/counter/schemas.py +++ b/counter/schemas.py @@ -16,10 +16,10 @@ class CounterSchema(ModelSchema): fields = ["id", "name", "type", "club", "products"] -class CustomerBalance(ModelSchema): +class CustomerSchema(ModelSchema): class Meta: model = Customer - fields = ["amount"] + fields = ["user", "account_id", "amount", "recorded_products"] class CounterFilterSchema(FilterSchema): diff --git a/counter/static/bundled/counter/counter-click-index.ts b/counter/static/bundled/counter/counter-click-index.ts index b0ddb42c..01d7e8f9 100644 --- a/counter/static/bundled/counter/counter-click-index.ts +++ b/counter/static/bundled/counter/counter-click-index.ts @@ -1,74 +1,117 @@ -document.addEventListener("alpine:init", () => { - Alpine.data("counter", () => ({ - // biome-ignore lint/correctness/noUndeclaredVariables: defined in counter_click.jinja - basket: sessionBasket, - errors: [], +import { exportToHtml } from "#core:utils/globals"; +import { customerGetCustomer } from "#openapi"; - sumBasket() { - if (!this.basket || Object.keys(this.basket).length === 0) { - return 0; - } - const total = Object.values(this.basket).reduce( - (acc, cur) => acc + cur.qty * cur.price, - 0, - ); - return total / 100; - }, +interface CounterConfig { + csrfToken: string; + clickApiUrl: string; + sessionBasket: Record; + customerBalance: number; + customerId: number; +} +interface BasketItem { + // biome-ignore lint/style/useNamingConvention: talking with python + bonus_qty: number; + price: number; + qty: number; +} - async handleCode(event) { - const code = $(event.target).find("#code_field").val().toUpperCase(); - if (["FIN", "ANN"].includes(code)) { - $(event.target).submit(); - } else { - await this.handleAction(event); - } - }, +exportToHtml("loadCounter", (config: CounterConfig) => { + document.addEventListener("alpine:init", () => { + Alpine.data("counter", () => ({ + basket: config.sessionBasket, + errors: [], + customerBalance: config.customerBalance, - async handleAction(event) { - const payload = $(event.target).serialize(); - // biome-ignore lint/correctness/noUndeclaredVariables: defined in counter_click.jinja - const request = new Request(clickApiUrl, { - method: "POST", - body: payload, - headers: { - // biome-ignore lint/style/useNamingConvention: this goes into http headers - Accept: "application/json", - // biome-ignore lint/correctness/noUndeclaredVariables: defined in counter_click.jinja - "X-CSRFToken": csrfToken, - }, - }); - const response = await fetch(request); - const json = await response.json(); - this.basket = json.basket; - this.errors = json.errors; - $("form.code_form #code_field").val("").focus(); - }, - })); + sumBasket() { + if (!this.basket || Object.keys(this.basket).length === 0) { + return 0; + } + const total = Object.values(this.basket).reduce( + (acc: number, cur: BasketItem) => acc + cur.qty * cur.price, + 0, + ) as number; + return total / 100; + }, + + async updateBalance() { + this.customerBalance = ( + await customerGetCustomer({ + path: { + // biome-ignore lint/style/useNamingConvention: api is in snake_case + customer_id: config.customerId, + }, + }) + ).data.amount; + }, + + async handleCode(event: SubmitEvent) { + const code = ( + $(event.target).find("#code_field").val() as string + ).toUpperCase(); + if (["FIN", "ANN"].includes(code)) { + $(event.target).submit(); + } else { + await this.handleAction(event); + } + }, + + async handleAction(event: SubmitEvent) { + const payload = $(event.target).serialize(); + const request = new Request(config.clickApiUrl, { + method: "POST", + body: payload, + headers: { + // biome-ignore lint/style/useNamingConvention: this goes into http headers + Accept: "application/json", + "X-CSRFToken": config.csrfToken, + }, + }); + const response = await fetch(request); + const json = await response.json(); + this.basket = json.basket; + this.errors = json.errors; + $("form.code_form #code_field").val("").focus(); + }, + })); + }); }); +interface Product { + value: string; + label: string; + tags: string; +} +declare global { + const productsAutocomplete: Product[]; +} + $(() => { /* Autocompletion in the code field */ - const codeField = $("#code_field"); + // biome-ignore lint/suspicious/noExplicitAny: dealing with legacy jquery + const codeField: any = $("#code_field"); let quantity = ""; codeField.autocomplete({ - select: (event, ui) => { + // biome-ignore lint/suspicious/noExplicitAny: dealing with legacy jquery + select: (event: any, ui: any) => { event.preventDefault(); codeField.val(quantity + ui.item.value); }, - focus: (event, ui) => { + // biome-ignore lint/suspicious/noExplicitAny: dealing with legacy jquery + focus: (event: any, ui: any) => { event.preventDefault(); codeField.val(quantity + ui.item.value); }, - source: (request, response) => { + // biome-ignore lint/suspicious/noExplicitAny: dealing with legacy jquery + source: (request: any, response: any) => { // biome-ignore lint/performance/useTopLevelRegex: performance impact is minimal const res = /^(\d+x)?(.*)/i.exec(request.term); quantity = res[1] || ""; const search = res[2]; - const matcher = new RegExp($.ui.autocomplete.escapeRegex(search), "i"); + // biome-ignore lint/suspicious/noExplicitAny: dealing with legacy jquery + const matcher = new RegExp(($ as any).ui.autocomplete.escapeRegex(search), "i"); response( - // biome-ignore lint/correctness/noUndeclaredVariables: defined in counter_click.jinja - $.grep(productsAutocomplete, (value) => { + $.grep(productsAutocomplete, (value: Product) => { return matcher.test(value.tags); }), ); @@ -76,11 +119,13 @@ $(() => { }); /* Accordion UI between basket and refills */ - $("#click_form").accordion({ + // biome-ignore lint/suspicious/noExplicitAny: dealing with legacy jquery + ($("#click_form") as any).accordion({ heightStyle: "content", activate: () => $(".focus").focus(), }); - $("#products").tabs(); + // biome-ignore lint/suspicious/noExplicitAny: dealing with legacy jquery + ($("#products") as any).tabs(); codeField.focus(); }); diff --git a/counter/tests/test_api.py b/counter/tests/test_api.py new file mode 100644 index 00000000..8a5efdc1 --- /dev/null +++ b/counter/tests/test_api.py @@ -0,0 +1,105 @@ +import pytest +from django.contrib.auth.models import make_password +from django.test.client import Client +from django.urls import reverse +from model_bakery import baker + +from core.baker_recipes import board_user, subscriber_user +from core.models import User +from counter.models import Counter + + +@pytest.fixture +def customer_user() -> User: + return subscriber_user.make() + + +@pytest.fixture +def counter_bar() -> Counter: + return baker.make(Counter, type="BAR") + + +@pytest.fixture +def barmen(counter_bar: Counter) -> User: + user = subscriber_user.make(password=make_password("plop")) + counter_bar.sellers.add(user) + return user + + +@pytest.fixture +def board_member() -> User: + return board_user.make() + + +@pytest.fixture +def root_user() -> User: + return baker.make(User, is_superuser=True) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + ("connected_user"), + [ + None, # Anonymous user + "barmen", + "customer_user", + "board_member", + "root_user", + ], +) +def test_get_customer_fail( + client: Client, + customer_user: User, + request: pytest.FixtureRequest, + connected_user: str | None, +): + if connected_user is not None: + client.force_login(request.getfixturevalue(connected_user)) + assert ( + client.get( + reverse("api:get_customer", kwargs={"customer_id": customer_user.id}) + ).status_code + == 403 + ) + + +@pytest.mark.django_db +def test_get_customer_from_bar_fail_wrong_referrer( + client: Client, customer_user: User, barmen: User, counter_bar: Counter +): + client.post( + reverse("counter:login", args=[counter_bar.pk]), + {"username": barmen.username, "password": "plop"}, + ) + + assert ( + client.get( + reverse("api:get_customer", kwargs={"customer_id": customer_user.id}) + ).status_code + == 403 + ) + + +@pytest.mark.django_db +def test_get_customer_from_bar_success( + client: Client, customer_user: User, barmen: User, counter_bar: Counter +): + client.post( + reverse("counter:login", args=[counter_bar.pk]), + {"username": barmen.username, "password": "plop"}, + ) + + response = client.get( + reverse("api:get_customer", kwargs={"customer_id": customer_user.id}), + HTTP_REFERER=reverse( + "counter:click", + kwargs={"counter_id": counter_bar.id, "user_id": customer_user.id}, + ), + ) + assert response.status_code == 200 + assert response.json() == { + "user": customer_user.id, + "account_id": customer_user.customer.account_id, + "amount": f"{customer_user.customer.amount:.2f}", + "recorded_products": customer_user.customer.recorded_products, + } diff --git a/counter/views/click.py b/counter/views/click.py index 1d87ac8b..6a379564 100644 --- a/counter/views/click.py +++ b/counter/views/click.py @@ -137,7 +137,7 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): request.session["no_age"] = False if self.object.type != "BAR": self.operator = request.user - elif self.customer_is_barman(): + elif self.object.customer_is_barman(self.customer): self.operator = self.customer.user else: self.operator = self.object.get_random_barman() @@ -157,16 +157,12 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): context = self.get_context_data(object=self.object) return self.render_to_response(context) - def customer_is_barman(self) -> bool: - barmen = self.object.barmen_list - return self.object.type == "BAR" and self.customer.user in barmen - def get_product(self, pid): return Product.objects.filter(pk=int(pid)).first() def get_price(self, pid): p = self.get_product(pid) - if self.customer_is_barman(): + if self.object.customer_is_barman(self.customer): price = p.special_selling_price else: price = p.selling_price @@ -331,7 +327,7 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): for pid, infos in request.session["basket"].items(): # This duplicates code for DB optimization (prevent to load many times the same object) p = Product.objects.filter(pk=pid).first() - if self.customer_is_barman(): + if self.object.customer_is_barman(self.customer): uprice = p.special_selling_price else: uprice = p.selling_price @@ -385,7 +381,7 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): """Add customer to the context.""" kwargs = super().get_context_data(**kwargs) products = self.object.products.select_related("product_type") - if self.customer_is_barman(): + if self.object.customer_is_barman(self.customer): products = products.annotate(price=F("special_selling_price")) else: products = products.annotate(price=F("selling_price")) @@ -444,17 +440,13 @@ class RefillingCreateView(FormView): if not self.counter.can_refill(): raise PermissionDenied - if self.customer_is_barman(): + if self.counter.customer_is_barman(self.customer): self.operator = self.customer.user else: self.operator = self.counter.get_random_barman() return super().dispatch(request, *args, **kwargs) - def customer_is_barman(self) -> bool: - barmen = self.counter.barmen_list - return self.counter.type == "BAR" and self.customer.user in barmen - def form_valid(self, form): res = super().form_valid(form) form.clean()