Merge branch 'feat-distribution-place' into 'develop'

Draft: feature: Add placekey relationship for distribution spots

See merge request la-chariotte/la-chariotte!129

la_chariotte/order/migrations/0030_add_place.py

@@ -72,4 +72,12 @@ class Migration(migrations.Migration):
             name="place",
             field=models.ForeignKey(blank=True, null=True, on_delete=models.deletion.CASCADE, to='order.place'),
         ),
+        migrations.AddIndex(
+            model_name="groupedorder",
+            index=models.Index(fields=["code"], name="order_group_code_50902d_idx"),
+        ),
+        migrations.AddIndex(
+            model_name="place",
+            index=models.Index(fields=["code"], name="order_place_code_4e2b27_idx"),
+        ),
     ]

la_chariotte/order/models.py

@@ -36,6 +36,9 @@ class GroupedOrder(models.Model):
         default=False, verbose_name="Numéro de téléphone obligatoire"
     )
 
+    class Meta:
+        indexes = [models.Index(fields=["code"])]
+
     def create_code_from_pk(self):
         """When a grouped order is created, a unique code is generated, to be used to
         build the order URL.
@@ -60,13 +63,6 @@ class GroupedOrder(models.Model):
         )
         self.code = f"{base_36_pk}{random_string}"[:code_length]
 
-    @property
-    def has_places(self):
-        # Check whether this GroupedOrder has any distribution Place enabled
-        if len(self.places.all()) > 0:
-            return True
-        return False
-
     @property
     def total_price(self):
         price = 0
@@ -226,6 +222,9 @@ class Place(models.Model):
     )
     description = models.TextField("Description", null=True, blank=True)
 
+    class Meta:
+        indexes = [models.Index(fields=["code"])]
+
     def __str__(self):  # pragma: no cover
         return self.name
 

la_chariotte/order/views/grouped_order.py

@@ -345,13 +345,19 @@ class GroupedOrderExportView(UserIsOrgaMixin, generic.DetailView):
         context["items"] = items
         context["orders_dict"] = orders_dict
 
-        if grouped_order.has_places:
+        # Query the DB only once for this grouped order's places
+        places = [x for x in grouped_order.places.all()]
+        if len(places) > 0:
             # Initialize empty list of orders for every place enabled for this GroupedOrder
-            places = {x.code: {} for x in grouped_order.places.all()}
+            places_orders = {x.code: {} for x in places}
 
             for order, order_items in orders_dict.items():
-                places[order.place.code][order] = order_items
-            context["places"] = places
+                places_orders[order.place.code][order] = order_items
+
+            # places contains the full places objects
+            # places_orders contains the order filtered by place.code
+            context["places"] = {x.code: x for x in places}
+            context["places_orders"] = places_orders
 
         return context
 
@@ -413,7 +419,7 @@ class ExportGroupedOrderToCSVView(GroupedOrderExportView):
         row.append("Note")
         row.append("Date")
         row.append("Heure")
-        if grouped_order.has_places:
+        if "places" in context:
             row.append("Lieu")
         writer.writerow(row)
 
@@ -425,8 +431,7 @@ class ExportGroupedOrderToCSVView(GroupedOrderExportView):
         row = ["Nom", "Prénom"]
         writer.writerow(row)
 
-        # write ordered values rows
-        for order, ordered_items in context["orders_dict"].items():
+        def format_csv_order_row(order, ordered_items, place=None):
             row = [order.author.last_name]
             row.append(order.author.first_name)
             for ordered_nb in ordered_items:
@@ -437,8 +442,22 @@ class ExportGroupedOrderToCSVView(GroupedOrderExportView):
             row.append(order.note)
             row.append(order.created_date.strftime("%d/%m/%Y"))
             row.append(order.created_date.strftime("%H:%M"))
-            if grouped_order.has_places:
-                row.append(order.place.name)
+            if place:
+                row.append(place.name)
+            return row
+
+        # Write ordered values rows.
+        # Avoid extra queries by reusing GroupedOrderExportView context
+        if "places" in context:
+            for place_code, place_orders in context["places_orders"].items():
+                for order, ordered_items in place_orders.items():
+                    row = format_csv_order_row(
+                        order, ordered_items, place=context["places"][place_code]
+                    )
+                    writer.writerow(row)
+        else:
+            for order, ordered_items in context["orders_dict"].items():
+                row = format_csv_order_row(order, ordered_items)
                 writer.writerow(row)
 
         # write total row

la_chariotte/order/views/order.py

@@ -48,31 +48,27 @@ def place_order(request, code):
     email = request.POST["email"]
     note = request.POST["note"]
 
-    # Make sure requested place is valid in this group order (and exists at all)
-    # If no places are enabled for this group order, chosen place is always None
-    if grouped_order.has_places:
-        # If places are enabled for the GroupedOrder
-        if "place" not in request.POST:
-            # HTTP error code 400
+    # Make sure requested place is valid in this group order
+    # Query grouped order places only once to avoid many DB roundtrips
+    grouped_order_places = grouped_order.places.all()
+    if len(grouped_order_places) > 0:
+        # Places are enabled for this grouped order, make sure one was supplied
+        if "place" not in request.POST or request.POST["place"] == "":
             return http.HttpResponse(
                 "Aucun lieu n'est spécifié alors que la commande groupée en exige un",
                 status=400,
             )
 
-        places = grouped_order.places.all()
-        # Return 404 if the requested place does not exist at all
-        place = Place.objects.all().filter(code=request.POST["place"]).first()
-        if not place:
+        place = request.POST["place"]
+        try:
+            # QuerySet is already queried from DB so this is a cheap operation.
+            # Could fail if no entry is found, or multiple entries are found
+            place = grouped_order_places.get(code=place)
+        except Exception as e:
+            # Return 404 because the requested place, whether it exists or not,
+            # is not enabled for this GroupedOrder
             return http.HttpResponseNotFound(
-                "Le lieu spécifié n'a pas été trouvé dans la base de données: %s"
-                % request.POST["place"]
-            )
-        if place not in places:
-            # Return 404 is the requested place exists but is not enabled for this
-            # GroupedOrder instance
-            return http.HttpResponseNotFound(
-                "Le lieu demandé n'est pas valide pour cette commande: %s"
-                % request.POST["place"]
+                "Le lieu demandé n'est pas valide pour cette commande: %s" % place
             )
     else:
         place = None

la_chariotte/tests/test_order_models.py

@@ -158,25 +158,6 @@ class TestPlaceModel:
         assert response.status_code == 302
         assert Place.objects.count() == 1
 
-    def test_order_has_places(self, client_log):
-        now = timezone.now()
-
-        place = Place.objects.create(
-            name="Centre social Kropotkine",
-            orga=auth.get_user(client_log),
-            code="kropotkine",
-        )
-
-        grouped_order_active = GroupedOrder.objects.create(
-            name="test",
-            orga=auth.get_user(client_log),
-            delivery_date=now.date(),
-            deadline=now + datetime.timedelta(days=30),
-        )
-        assert grouped_order_active.has_places == False
-        grouped_order_active.places.add(place.id)
-        assert grouped_order_active.has_places == True
-
     def test_place_active_orders(self, client_log):
         now = timezone.now()
 

la_chariotte/tests/test_order_views_grouped_order.py

@@ -1549,9 +1549,11 @@ class TestGroupedOrderSheetView:
         assert len(response.context["items"]) == 0
         assert len(response.context["orders_dict"]) == 0
         assert "places" in response.context.keys()
+        assert "places_orders" in response.context.keys()
         assert len(response.context["places"]) == 2
-        assert len(response.context["places"]["kropotkine"]) == 0
-        assert len(response.context["places"]["luxemburg"]) == 0
+        assert len(response.context["places_orders"]) == 2
+        assert len(response.context["places_orders"]["kropotkine"]) == 0
+        assert len(response.context["places_orders"]["luxemburg"]) == 0
 
         # we order some items in the grouped order
         order = order_items_in_grouped_order(grouped_order, place=place2)
@@ -1564,11 +1566,11 @@ class TestGroupedOrderSheetView:
         assert "places" in response.context.keys()
 
         # test that orders are split by distribution place
-        places = response.context["places"]
-        assert len(places["kropotkine"]) == 0
-        assert len(places["luxemburg"]) == 3
+        places_orders = response.context["places_orders"]
+        assert len(places_orders["kropotkine"]) == 0
+        assert len(places_orders["luxemburg"]) == 3
 
-        orders = list(places["luxemburg"])
+        orders = list(places_orders["luxemburg"])
         assert orders[0].author.last_name == "alescargot"
         assert orders[0].author.first_name == "bob"
         assert orders[1].author.last_name == "alescargot"
@@ -1704,6 +1706,7 @@ class TestExportGroupOrderEmailAdressesToDownloadView:
             orga_user=auth.get_user(client_log),
         )
         grouped_order.places.add(place.id)
+        grouped_order.save()
         item = models.Item.objects.create(
             name="test item 1", grouped_order=grouped_order, price=2
         )