From a26298317054ec6c33c2d0b470e6be135d1b1d13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Tue, 4 Apr 2017 23:51:22 +0100 Subject: [PATCH 1/5] PEP8 compliance in bda.views --- bda/views.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/bda/views.py b/bda/views.py index 41e5d08b..4301249f 100644 --- a/bda/views.py +++ b/bda/views.py @@ -664,7 +664,7 @@ def catalogue(request, request_type): tirage_id = request.GET.get('id', '') try: tirage = Tirage.objects.get(id=tirage_id) - except ObjectDoesNotExist: + except ObjectDoesNotExist: return HttpResponseBadRequest( "Aucun tirage correspondant à l'id " + tirage_id) @@ -703,17 +703,18 @@ def catalogue(request, request_type): location__id__in=location_id) except ObjectDoesNotExist: return HttpResponseBadRequest( - "Impossible de trouver des résultats correspondant " - "à ces caractéristiques : " - + "id = " + tirage_id - + ", catégories = " + categories - + ", salles = " + locations) + "Impossible de trouver des résultats correspondant à ces " + "caractéristiques : id = {}, catégories = {}, salles = {}" + .format(tirage_id, categories, locations) + ) except ValueError: # Contient JSONDecodeError return HttpResponseBadRequest( - "Impossible de parser les paramètres donnés : " - + "id = " + request.GET.get('id', '') - + ", catégories = " + request.GET.get('category', '[0]') - + ", salles = " + request.GET.get('location', '[0]')) + "Impossible de parser les paramètres donnés : " + "id = {}, catégories = {}, salles = {}" + .format(request.GET.get('id', ''), + request.GET.get('category', '[0]'), + request.GET.get('location', '[0]')) + ) # On convertit les descriptions à envoyer en une liste facilement # JSONifiable (il devrait y avoir un moyen plus efficace en From 853a239e6ec6525cd360b39c49db6c0601dd11bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Wed, 5 Apr 2017 19:33:18 +0100 Subject: [PATCH 2/5] Rewrite bda.views.catalogue - No string concatenations - Use `get_object_or_404` instead of performing a `.get` and catching the eventual exception. - More accurate error messages when a bad request is detected. - More accurate error handling --- bda/views.py | 83 ++++++++++++++++++++++++++++------------------------ 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/bda/views.py b/bda/views.py index 4301249f..f7515ed2 100644 --- a/bda/views.py +++ b/bda/views.py @@ -22,7 +22,6 @@ from django.core.urlresolvers import reverse from django.conf import settings from django.utils import timezone, formats from django.views.generic.list import ListView -from django.core.exceptions import ObjectDoesNotExist from gestioncof.decorators import cof_required, buro_required from bda.models import ( Spectacle, Participant, ChoixSpectacle, Attribution, Tirage, @@ -657,29 +656,34 @@ def catalogue(request, request_type): if request_type == "list": # Dans ce cas on retourne la liste des tirages et de leur id en JSON data_return = list( - Tirage.objects.filter(appear_catalogue=True).values('id', 'title')) + Tirage.objects.filter(appear_catalogue=True).values('id', 'title') + ) return JsonResponse(data_return, safe=False) if request_type == "details": # Dans ce cas on retourne une liste des catégories et des salles - tirage_id = request.GET.get('id', '') - try: - tirage = Tirage.objects.get(id=tirage_id) - except ObjectDoesNotExist: + tirage_id = request.GET.get('id', None) + if tirage_id is None: return HttpResponseBadRequest( - "Aucun tirage correspondant à l'id " - + tirage_id) + "Missing GET parameter: id " + ) + try: + tirage = get_object_or_404(Tirage, id=int(tirage_id)) except ValueError: return HttpResponseBadRequest( - "Mauvais format d'identifiant : " - + tirage_id) + "Bad format: int expected for `id`" + ) categories = list( - CategorieSpectacle.objects.filter( - spectacle__in=tirage.spectacle_set.all()) - .distinct().values('id', 'name')) + CategorieSpectacle.objects + .filter(spectacle__in=tirage.spectacle_set.all()) + .distinct() + .values('id', 'name') + ) locations = list( - Salle.objects.filter( - spectacle__in=tirage.spectacle_set.all()) - .distinct().values('id', 'name')) + Salle.objects + .filter(spectacle__in=tirage.spectacle_set.all()) + .distinct() + .values('id', 'name') + ) data_return = {'categories': categories, 'locations': locations} return JsonResponse(data_return, safe=False) if request_type == "descriptions": @@ -687,34 +691,35 @@ def catalogue(request, request_type): # à la salle spécifiées tirage_id = request.GET.get('id', '') - categories = request.GET.get('category', '[0]') - locations = request.GET.get('location', '[0]') + categories = request.GET.get('category', '[]') + locations = request.GET.get('location', '[]') try: - category_id = json.loads(categories) - location_id = json.loads(locations) - tirage = Tirage.objects.get(id=tirage_id) - - shows_qs = tirage.spectacle_set - if not(0 in category_id): - shows_qs = shows_qs.filter( - category__id__in=category_id) - if not(0 in location_id): - shows_qs = shows_qs.filter( - location__id__in=location_id) - except ObjectDoesNotExist: - return HttpResponseBadRequest( - "Impossible de trouver des résultats correspondant à ces " - "caractéristiques : id = {}, catégories = {}, salles = {}" - .format(tirage_id, categories, locations) - ) + tirage_id = int(tirage_id) + categories_id = json.loads(categories) + locations_id = json.loads(locations) + # Integers expected + if not all(isinstance(id, int) for id in categories_id): + raise ValueError + if not all(isinstance(id, int) for id in locations_id): + raise ValueError except ValueError: # Contient JSONDecodeError return HttpResponseBadRequest( - "Impossible de parser les paramètres donnés : " - "id = {}, catégories = {}, salles = {}" + "Parse error, please ensure the GET parameters have the " + "following types:\n" + "id: int, category: [int], location: [int]\n" + "Data received:\n" + "id = {}, category = {}, locations = {}" .format(request.GET.get('id', ''), - request.GET.get('category', '[0]'), - request.GET.get('location', '[0]')) + request.GET.get('category', '[]'), + request.GET.get('location', '[]')) ) + tirage = get_object_or_404(Tirage, id=tirage_id) + + shows_qs = tirage.spectacle_set + if categories_id: + shows_qs = shows_qs.filter(category__id__in=categories_id) + if locations_id: + shows_qs = shows_qs.filter(location__id__in=locations_id) # On convertit les descriptions à envoyer en une liste facilement # JSONifiable (il devrait y avoir un moyen plus efficace en From ff9cee5ffc008ffca134f10f357137c866426707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Wed, 5 Apr 2017 20:42:50 +0100 Subject: [PATCH 3/5] line too long in bda.models --- bda/models.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bda/models.py b/bda/models.py index 15acf686..0228b4c0 100644 --- a/bda/models.py +++ b/bda/models.py @@ -18,7 +18,10 @@ class Tirage(models.Model): fermeture = models.DateTimeField("Date et heure de fermerture du tirage") tokens = models.TextField("Graine(s) du tirage", blank=True) active = models.BooleanField("Tirage actif", default=False) - appear_catalogue = models.BooleanField("Tirage à afficher dans le catalogue", default=False) + appear_catalogue = models.BooleanField( + "Tirage à afficher dans le catalogue", + default=False + ) enable_do_tirage = models.BooleanField("Le tirage peut être lancé", default=False) From 9ec151e040808a4a603c46e5f5d2ec2095fb6b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Wed, 5 Apr 2017 20:48:18 +0100 Subject: [PATCH 4/5] A query optimization in bda.views.catalogue We only need to fetch shows identifiers here: FOO.filter(BAR__in=shows) which can be done using the `values_list` method --- bda/views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bda/views.py b/bda/views.py index f7515ed2..8fda604d 100644 --- a/bda/views.py +++ b/bda/views.py @@ -672,15 +672,16 @@ def catalogue(request, request_type): return HttpResponseBadRequest( "Bad format: int expected for `id`" ) + shows = tirage.spectacle_set.values_list("id", flat=True) categories = list( CategorieSpectacle.objects - .filter(spectacle__in=tirage.spectacle_set.all()) + .filter(spectacle__in=shows) .distinct() .values('id', 'name') ) locations = list( Salle.objects - .filter(spectacle__in=tirage.spectacle_set.all()) + .filter(spectacle__in=shows) .distinct() .values('id', 'name') ) From 981759f5ce294b8176fe29c1ae6f33a06807a173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Wed, 5 Apr 2017 21:02:00 +0100 Subject: [PATCH 5/5] Adds tests covering the bda-catalogue JSON API --- bda/tests.py | 89 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 73 insertions(+), 16 deletions(-) diff --git a/bda/tests.py b/bda/tests.py index 22efc5a2..2741084f 100644 --- a/bda/tests.py +++ b/bda/tests.py @@ -1,22 +1,79 @@ -# -*- coding: utf-8 -*- -""" -This file demonstrates writing tests using the unittest module. These will pass -when you run "manage.py test". +import json -Replace this with more appropriate tests for your application. -""" +from django.test import TestCase, Client +from django.utils import timezone -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals +from .models import Tirage, Spectacle, Salle, CategorieSpectacle -from django.test import TestCase +class TestBdAViews(TestCase): + def setUp(self): + self.tirage = Tirage.objects.create( + title="Test tirage", + appear_catalogue=True, + ouverture=timezone.now(), + fermeture=timezone.now(), + ) + self.category = CategorieSpectacle.objects.create(name="Category") + self.location = Salle.objects.create(name="here") + Spectacle.objects.bulk_create([ + Spectacle( + title="foo", date=timezone.now(), location=self.location, + price=0, slots=42, tirage=self.tirage, listing=False, + category=self.category + ), + Spectacle( + title="bar", date=timezone.now(), location=self.location, + price=1, slots=142, tirage=self.tirage, listing=False, + category=self.category + ), + Spectacle( + title="baz", date=timezone.now(), location=self.location, + price=2, slots=242, tirage=self.tirage, listing=False, + category=self.category + ), + ]) + def test_catalogue(self): + """Test the catalogue JSON API""" + client = Client() -class SimpleTest(TestCase): - def test_basic_addition(self): - """ - Tests that 1 + 1 always equals 2. - """ - self.assertEqual(1 + 1, 2) + # The `list` hooh + resp = client.get("/bda/catalogue/list") + self.assertJSONEqual( + resp.content.decode("utf-8"), + [{"id": self.tirage.id, "title": self.tirage.title}] + ) + + # The `details` hook + resp = client.get( + "/bda/catalogue/details?id={}".format(self.tirage.id) + ) + self.assertJSONEqual( + resp.content.decode("utf-8"), + { + "categories": [{ + "id": self.category.id, + "name": self.category.name + }], + "locations": [{ + "id": self.location.id, + "name": self.location.name + }], + } + ) + + # The `descriptions` hook + resp = client.get( + "/bda/catalogue/descriptions?id={}".format(self.tirage.id) + ) + raw = resp.content.decode("utf-8") + try: + results = json.loads(raw) + except ValueError: + self.fail("Not valid JSON: {}".format(raw)) + self.assertEqual(len(results), 3) + self.assertEqual( + {(s["title"], s["price"], s["slots"]) for s in results}, + {("foo", 0, 42), ("bar", 1, 142), ("baz", 2, 242)} + )