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,12 +72,4 @@ 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,9 +36,6 @@ 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.
@@ -63,6 +60,13 @@ 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
@@ -222,9 +226,6 @@ 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,19 +345,13 @@ class GroupedOrderExportView(UserIsOrgaMixin, generic.DetailView):
         context["items"] = items
         context["orders_dict"] = orders_dict
 
-        # Query the DB only once for this grouped order's places
-        places = [x for x in grouped_order.places.all()]
-        if len(places) > 0:
+        if grouped_order.has_places:
             # Initialize empty list of orders for every place enabled for this GroupedOrder
-            places_orders = {x.code: {} for x in places}
+            places = {x.code: {} for x in grouped_order.places.all()}
 
             for order, order_items in orders_dict.items():
-                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
+                places[order.place.code][order] = order_items
+            context["places"] = places
 
         return context
 
@@ -419,7 +413,7 @@ class ExportGroupedOrderToCSVView(GroupedOrderExportView):
         row.append("Note")
         row.append("Date")
         row.append("Heure")
-        if "places" in context:
+        if grouped_order.has_places:
             row.append("Lieu")
         writer.writerow(row)
 
@@ -431,7 +425,8 @@ class ExportGroupedOrderToCSVView(GroupedOrderExportView):
         row = ["Nom", "Prénom"]
         writer.writerow(row)
 
-        def format_csv_order_row(order, ordered_items, place=None):
+        # write ordered values rows
+        for order, ordered_items in context["orders_dict"].items():
             row = [order.author.last_name]
             row.append(order.author.first_name)
             for ordered_nb in ordered_items:
@@ -442,22 +437,8 @@ 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 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)
+            if grouped_order.has_places:
+                row.append(order.place.name)
             writer.writerow(row)
 
         # write total row

la_chariotte/order/views/order.py

@@ -48,27 +48,31 @@ def place_order(request, code):
     email = request.POST["email"]
     note = request.POST["note"]
 
-    # 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"] == "":
+    # 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
             return http.HttpResponse(
                 "Aucun lieu n'est spécifié alors que la commande groupée en exige un",
                 status=400,
             )
 
-        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
+        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:
             return http.HttpResponseNotFound(
-                "Le lieu demandé n'est pas valide pour cette commande: %s" % place
+                "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"]
             )
     else:
         place = None

la_chariotte/tests/test_order_models.py

@@ -158,6 +158,25 @@ 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,11 +1549,9 @@ 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_orders"]) == 2
-        assert len(response.context["places_orders"]["kropotkine"]) == 0
-        assert len(response.context["places_orders"]["luxemburg"]) == 0
+        assert len(response.context["places"]["kropotkine"]) == 0
+        assert len(response.context["places"]["luxemburg"]) == 0
 
         # we order some items in the grouped order
         order = order_items_in_grouped_order(grouped_order, place=place2)
@@ -1566,11 +1564,11 @@ class TestGroupedOrderSheetView:
         assert "places" in response.context.keys()
 
         # test that orders are split by distribution place
-        places_orders = response.context["places_orders"]
-        assert len(places_orders["kropotkine"]) == 0
-        assert len(places_orders["luxemburg"]) == 3
+        places = response.context["places"]
+        assert len(places["kropotkine"]) == 0
+        assert len(places["luxemburg"]) == 3
 
-        orders = list(places_orders["luxemburg"])
+        orders = list(places["luxemburg"])
         assert orders[0].author.last_name == "alescargot"
         assert orders[0].author.first_name == "bob"
         assert orders[1].author.last_name == "alescargot"
@@ -1706,7 +1704,6 @@ 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
         )