From 853a239e6ec6525cd360b39c49db6c0601dd11bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20P=C3=A9pin?= <martin.pepin@ens.fr>
Date: Wed, 5 Apr 2017 19:33:18 +0100
Subject: [PATCH] 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 <int>"
+            )
+        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