From 105cf4a0441fd37712bb5169b1c92c3f6a35f79b Mon Sep 17 00:00:00 2001
From: Midgard <midgard@zeus.ugent.be>
Date: Wed, 25 May 2022 13:55:31 +0200
Subject: [PATCH] Revert "Don't crash on orders that don't have a slug"

This reverts commit ff0ea068def1abd2e25cb39db9720c2d8359a2a2.

The next commit (Create a slug for old orders in the migration) will
change our strategy of writing code to handle the legacy slugless orders:
we will generate a slug for old orders.
---
 app/models/order.py             | 16 ----------------
 app/notification.py             |  4 ++--
 app/templates/order.html        | 22 ++++++++++-----------
 app/templates/order_prices.html |  6 +++---
 app/templates/utils.html        |  2 +-
 app/views/order.py              | 34 ++++++++++++++++-----------------
 6 files changed, 33 insertions(+), 51 deletions(-)

diff --git a/app/models/order.py b/app/models/order.py
index b7fe624..6e5391c 100644
--- a/app/models/order.py
+++ b/app/models/order.py
@@ -132,19 +132,3 @@ class Order(db.Model):
     def can_modify_payment(self, user_id: int) -> bool:
         user = User.query.filter_by(id=user_id).first()
         return user and (user.is_admin() or user == self.courier)
-
-    @staticmethod
-    def get_by_slug(slug: str) -> "typing.Optional[Order]":
-        """
-        Find an order by slug. Also matches orders by ID if they don't have a slug
-        """
-        order_id = None
-        try:
-            order_id = int(slug)
-        except:
-            pass
-
-        return Order.query.filter(
-            (Order.slug == slug) |
-            ((Order.slug == None) & (Order.id == order_id))
-        ).first()
diff --git a/app/notification.py b/app/notification.py
index d6cf1cb..5569f2f 100644
--- a/app/notification.py
+++ b/app/notification.py
@@ -18,7 +18,7 @@ def webhook_text(order: Order) -> typing.Optional[str]:
     if order.courier is not None:
         # pylint: disable=C0301, C0209
         return "<!channel|@channel> {3} is going to {1}, order <{0}|here>! Deadline in {2} minutes!".format(
-            url_for("order_bp.order_from_slug", order_slug=order.slug or order.id, _external=True),
+            url_for("order_bp.order_from_slug", order_slug=order.slug, _external=True),
             order.location_name,
             remaining_minutes(order.stoptime),
             order.courier.username.title(),
@@ -28,7 +28,7 @@ def webhook_text(order: Order) -> typing.Optional[str]:
     return "<!channel|@channel> New order for {}. Deadline in {} minutes. <{}|Open here.>".format(
         order.location_name,
         remaining_minutes(order.stoptime),
-        url_for("order_bp.order_from_slug", order_slug=order.slug or order.id, _external=True),
+        url_for("order_bp.order_from_slug", order_slug=order.slug, _external=True),
     )
 
 
diff --git a/app/templates/order.html b/app/templates/order.html
index 5d9965a..0ad5611 100644
--- a/app/templates/order.html
+++ b/app/templates/order.html
@@ -23,7 +23,7 @@
 		<div id="qrcode"></div>
 		<script type="text/javascript">
 			var qrcode = new QRCode(document.getElementById("qrcode"), {
-					text: "{{ url_for("order_bp.order_from_slug", order_slug=order.slug or order.id, _external=True) }}",
+					text: "{{ url_for("order_bp.order_from_slug", order_slug=order.slug, _external=True) }}",
 					width: 128,
 					height: 128,
 					colorDark : "#000000",
@@ -43,7 +43,7 @@
 			{% endif %}
 		</div>
 		<div>
-			Unique order link: <code>{{ url_for("order_bp.order_from_slug", order_slug=order.slug or order.id, _external=True) }}</code>
+			Unique order link: <code>{{ url_for("order_bp.order_from_slug", order_slug=order.slug, _external=True) }}</code>
 		</div>
 	</div>
 </header>
@@ -57,7 +57,7 @@
 					{% for item in my_items %}
 					<li class="spacecake">
 						{% if item.can_delete(order.id, current_user.id, session.get('anon_name', '')) -%}
-							<form action="{{ url_for('order_bp.delete_item', order_slug=order.slug or order.id, item_id=item.id) }}" method="post" style="display:inline">
+							<form action="{{ url_for('order_bp.delete_item', order_slug=order.slug, item_id=item.id) }}" method="post" style="display:inline">
 								<button class="btn btn-link btn-sm" type="submit" style="padding: 0 0.5em;"><span class="glyphicon glyphicon-remove"></span></button>
 							</form>
 						{%- endif %}
@@ -86,7 +86,7 @@
 			<h3>Add item to order</h3>
 
 			{% for dish in order.location.dishes %}
-				<form method="post" action="{{ url_for('order_bp.order_item_create', order_slug=order.slug or order.id) }}" id="dish_{{ dish.id }}">
+				<form method="post" action="{{ url_for('order_bp.order_item_create', order_slug=order.slug) }}" id="dish_{{ dish.id }}">
 					{{ form.csrf_token }}
 					<input type="hidden" name="dish_id" value="{{ dish.id }}" />
 
@@ -188,7 +188,7 @@
 					<dd>
 						{% if order.courier == None %}
 							{% if not current_user.is_anonymous() %}
-							<form action="{{ url_for('order_bp.volunteer', order_slug=order.slug or order.id) }}" method="post" style="display:inline">
+							<form action="{{ url_for('order_bp.volunteer', order_slug=order.slug) }}" method="post" style="display:inline">
 								<input type="submit" class="btn btn-primary btn-sm" value="Volunteer"></input>
 							</form>
 							{% else %}No-one yet{% endif %}
@@ -201,12 +201,12 @@
 
 			<div>
 				{% if order.can_close(current_user.id) -%}
-					<form action="{{ url_for('order_bp.close_order', order_slug=order.slug or order.id) }}" method="post" style="display:inline">
+					<form action="{{ url_for('order_bp.close_order', order_slug=order.slug) }}" method="post" style="display:inline">
 						<input type="submit" class="btn btn-danger" value="Close"></input>
 					</form>
 				{% endif %}
 				{% if courier_or_admin %}
-					<a class="btn" href="{{ url_for('order_bp.order_edit', order_slug=order.slug or order.id) }}">Edit</a>
+					<a class="btn" href="{{ url_for('order_bp.order_edit', order_slug=order.slug) }}">Edit</a>
 				{%- endif %}
 			</div>
 		</div>
@@ -279,7 +279,7 @@
 			<div class="footer">
 				Total {{ order.items.count() }} items — {{ total_price|euro }}
 				&nbsp;
-				<a class="btn btn-sm" href="{{ url_for('order_bp.items_shop_view', order_slug=order.slug or order.id) }}">Shop view</a>
+				<a class="btn btn-sm" href="{{ url_for('order_bp.items_shop_view', order_slug=order.slug) }}">Shop view</a>
 			</div>
 		</div>
 	</div>
@@ -288,7 +288,7 @@
 <section class="single_column">
 	<div class="box" id="per_person">
 		<h3>Items per person</h3>
-		<form action="{{ url_for('order_bp.modify_items', order_slug=order.slug or order.id) }}" method="post">
+		<form action="{{ url_for('order_bp.modify_items', order_slug=order.slug) }}" method="post">
 		<table class="table table-condensed">
 			<thead>
 				<tr><th>Total</th><th>Name</th><th>Items</th></tr>
@@ -314,7 +314,7 @@
 							<li class="{{ 'paid' if item.paid }}">
 								<div class="actions">
 								{% if item.can_delete(order.id, current_user.id, session.get('anon_name', '')) -%}
-									<form action="{{ url_for('order_bp.delete_item', order_slug=order.slug or order.id, item_id=item.id) }}" method="post" style="display:inline">
+									<form action="{{ url_for('order_bp.delete_item', order_slug=order.slug, item_id=item.id) }}" method="post" style="display:inline">
 										<button class="btn btn-link btn-sm" type="submit" style="padding: 0 0.5em;"><span class="glyphicon glyphicon-remove"></span></button>
 									</form>
 								{% else %}
@@ -348,7 +348,7 @@
 
 			{% if order.can_modify_prices(current_user.id) %}
 				&nbsp; <span style="border-left: 1px solid var(--gray0); display: inline-block;">&nbsp;</span>&nbsp;
-				<a href="{{ url_for('order_bp.prices', order_slug=order.slug or order.id) }}" class="btn btn-sm">
+				<a href="{{ url_for('order_bp.prices', order_slug=order.slug) }}" class="btn btn-sm">
 					<span class="glyphicon glyphicon-pencil"></span>  Edit prices
 				</a>
 			{% endif %}
diff --git a/app/templates/order_prices.html b/app/templates/order_prices.html
index 9c5c0be..49ac068 100644
--- a/app/templates/order_prices.html
+++ b/app/templates/order_prices.html
@@ -11,10 +11,10 @@
 {% block container %}
 <header>
 	<h2 id="order-title">Edit prices</h2>
-	<div>Only applied to <a href="{{ url_for('order_bp.order_from_slug', order_slug=order.slug or order.id) }}">order {{ order.id }}</a>. To permanently change prices for {{ order.location_name }}, edit the <a href="https://git.zeus.gent/haldis/menus/-/blob/master/{{order.location_id}}.hlds">HLDS location definition</a>.</div>
+	<div>Only applied to <a href="{{ url_for('order_bp.order_from_slug', order_slug=order.slug) }}">order {{ order.id }}</a>. To permanently change prices for {{ order.location_name }}, edit the <a href="https://git.zeus.gent/haldis/menus/-/blob/master/{{order.location_id}}.hlds">HLDS location definition</a>.</div>
 </header>
 
-<form action="{{ url_for('order_bp.prices', order_slug=order.slug or order.id) }}" method="post">
+<form action="{{ url_for('order_bp.prices', order_slug=order.slug) }}" method="post">
 	<div class="col-md-6" id="per_dish">
 		<h3>Per dish</h3>
 		<div class="noscript">This functionality requires JavaScript.</div>
@@ -86,7 +86,7 @@
 	</div>
 
 	<div>
-		<a href="{{ url_for('order_bp.order_from_slug', order_slug=order.slug or order.id) }}" class="btn btn-sm">Cancel</a>
+		<a href="{{ url_for('order_bp.order_from_slug', order_slug=order.slug) }}" class="btn btn-sm">Cancel</a>
 		<button class="btn btn-sm btn-primary">Apply</button>
 	</div>
 </form>
diff --git a/app/templates/utils.html b/app/templates/utils.html
index c18f909..0af7b82 100644
--- a/app/templates/utils.html
+++ b/app/templates/utils.html
@@ -9,7 +9,7 @@
 		{% else %}open{% endif %}<br/>
 	</div>
 	<div class="col-md-4 col-lg-3 expand_button_wrapper">
-		<a class="btn btn-primary btn-block align-bottom expand_button" href="{{ url_for('order_bp.order_from_slug', order_slug=(order.slug or order.id)) }}">Expand</a>
+		<a class="btn btn-primary btn-block align-bottom expand_button" href="{{ url_for('order_bp.order_from_slug', order_slug=order.slug) }}">Expand</a>
 	</div>
 </div>
 {%- endmacro %}
diff --git a/app/views/order.py b/app/views/order.py
index a19ea8f..ddce63d 100644
--- a/app/views/order.py
+++ b/app/views/order.py
@@ -43,14 +43,14 @@ def order_create() -> typing.Union[str, Response]:
         db.session.add(order)
         db.session.commit()
         post_order_to_webhook(order)
-        return redirect(url_for("order_bp.order_from_slug", order_slug=order.slug or order.id))
+        return redirect(url_for("order_bp.order_from_slug", order_slug=order.slug))
     return orders(form=orderForm)
 
 
 @order_bp.route("/<order_slug>")
 def order_from_slug(order_slug: str, form: OrderForm = None, dish_id=None) -> str:
     """Generate order view from id"""
-    order = Order.get_by_slug(order_slug)
+    order = Order.query.filter(Order.slug == order_slug).first()
     if order is None:
         abort(404)
     if current_user.is_anonymous() and not order.public:
@@ -80,7 +80,7 @@ def order_from_slug(order_slug: str, form: OrderForm = None, dish_id=None) -> st
 @order_bp.route("/<order_slug>/items")
 def items_shop_view(order_slug: int) -> str:
     """Generate order items view from id"""
-    order = Order.get_by_slug(order_slug)
+    order = Order.query.filter(Order.slug == order_slug).first()
     if order is None:
         abort(404)
     if current_user.is_anonymous() and not order.public:
@@ -94,7 +94,7 @@ def items_shop_view(order_slug: int) -> str:
 @login_required
 def order_edit(order_slug: str) -> typing.Union[str, Response]:
     """Generate order edit view from id"""
-    order = Order.get_by_slug(order_slug)
+    order = Order.query.filter(Order.slug == order_slug).first()
     if current_user.id is not order.courier_id and not current_user.is_admin():
         abort(401)
     if order is None:
@@ -105,8 +105,8 @@ def order_edit(order_slug: str) -> typing.Union[str, Response]:
         order_form.populate_obj(order)
         order.update_from_hlds()
         db.session.commit()
-        return redirect(url_for("order_bp.order_from_slug", order_slug=order.slug or order.id))
-    return render_template("order_edit.html", form=order_form, order_slug=order.slug or order.id)
+        return redirect(url_for("order_bp.order_from_slug", order_slug=order.slug))
+    return render_template("order_edit.html", form=order_form, order_slug=order.slug)
 
 
 @order_bp.route("/<order_slug>/create", methods=["GET", "POST"])
@@ -114,7 +114,7 @@ def order_item_create(order_slug: str) -> typing.Any:
     # type is 'typing.Union[str, Response]', but this errors due to
     #   https://github.com/python/mypy/issues/7187
     """Add item to order from slug"""
-    current_order = Order.get_by_slug(order_slug)
+    current_order = Order.query.filter(Order.slug == order_slug).first()
     if current_order is None:
         abort(404)
     if current_order.is_closed():
@@ -171,7 +171,7 @@ def order_item_create(order_slug: str) -> typing.Any:
             return redirect(
                 url_for(
                     "order_bp.order_item_create",
-                    order_slug=current_order.slug or current_order.id,
+                    order_slug=current_order.slug,
                     dish=form.dish_id.data,
                     user_name=user_name,
                     comment=comment,
@@ -241,7 +241,7 @@ def modify_items(order_slug: str) -> typing.Optional[Response]:
         return None
 
 def set_items_paid(order_slug: str, user_names: typing.Iterable[str], paid: bool):
-    order = Order.get_by_slug(order_slug)
+    order = Order.query.filter(Order.slug == order_slug).first()
     total_paid_items = 0
     total_failed_items = 0
     for user_name in user_names:
@@ -279,9 +279,7 @@ def delete_item(order_slug: str, item_id: int) -> typing.Any:
     #   https://github.com/python/mypy/issues/7187
     """Delete an item from an order"""
     item: OrderItem = OrderItem.query.filter(OrderItem.id == item_id).first()
-    order = Order.get_by_slug(order_slug)
-    if order is None:
-        abort(404)
+    order: Order = Order.query.filter(Order.slug == order_slug).first()
     user_id = None
     if not current_user.is_anonymous():
         user_id = current_user.id
@@ -298,7 +296,7 @@ def delete_item(order_slug: str, item_id: int) -> typing.Any:
 @login_required
 def volunteer(order_slug: str) -> Response:
     """Add a volunteer to an order"""
-    order = Order.get_by_slug(order_slug)
+    order = Order.query.filter(Order.slug == order_slug).first()
     if order is None:
         abort(404)
     if order.courier_id is None or order.courier_id == 0:
@@ -307,14 +305,14 @@ def volunteer(order_slug: str) -> Response:
         flash("Thank you for volunteering!")
     else:
         flash("Volunteering not possible!")
-    return redirect(url_for("order_bp.order_from_slug", order_slug=order.slug or order.id))
+    return redirect(url_for("order_bp.order_from_slug", order_slug=order.slug))
 
 
 @order_bp.route("/<order_slug>/close", methods=["POST"])
 @login_required
 def close_order(order_slug: str) -> typing.Optional[Response]:
     """Close an order"""
-    order = Order.get_by_slug(order_slug)
+    order = Order.query.filter(Order.slug == order_slug).first()
     if order is None:
         abort(404)
     if (
@@ -333,12 +331,12 @@ def close_order(order_slug: str) -> typing.Optional[Response]:
 @order_bp.route("/<order_slug>/prices", methods=["GET", "POST"])
 @login_required
 def prices(order_slug: str) -> typing.Optional[Response]:
-    order = Order.get_by_slug(order_slug)
+    order = Order.query.filter(Order.slug == order_slug).first()
     if order is None:
         abort(404)
     if not order.can_modify_prices(current_user.id):
         flash("You cannot modify the prices at this time.", "error")
-        return redirect(url_for("order_bp.order_from_slug", order_slug=order.slug or order.id))
+        return redirect(url_for("order_bp.order_from_slug", order_slug=order.slug))
 
     if request.method == "GET":
         return render_template(
@@ -368,7 +366,7 @@ def prices(order_slug: str) -> typing.Optional[Response]:
                 item.price_modified = datetime.now()
         db.session.commit()
 
-    return redirect(url_for("order_bp.order_from_slug", order_slug=order.slug or order.id))
+    return redirect(url_for("order_bp.order_from_slug", order_slug=order.slug))