Allow admin for shift without joining it (#580)

* Allow admin for shift without joining it

* Change templates to make clear that only managing is also possible

---------

Co-authored-by: Vince van Diermen <[email protected]>

website/announcements/tests/test_services.py

@@ -6,22 +6,22 @@
 
 class OrderServicesTests(TestCase):
     def test_sanitize_closed_announcements_none(self):
-        self.assertEquals([], sanitize_closed_announcements(None))
+        self.assertEqual([], sanitize_closed_announcements(None))
 
     def test_sanitize_closed_announcements_non_string(self):
-        self.assertEquals([], sanitize_closed_announcements(5))
+        self.assertEqual([], sanitize_closed_announcements(5))
 
     def test_sanitize_closed_announcements_non_json(self):
-        self.assertEquals([], sanitize_closed_announcements("this,is,not,json"))
+        self.assertEqual([], sanitize_closed_announcements("this,is,not,json"))
 
     def test_sanitize_closed_announcements_non_list(self):
-        self.assertEquals([], sanitize_closed_announcements("{}"))
+        self.assertEqual([], sanitize_closed_announcements("{}"))
 
     def test_sanitize_closed_announcements_list_of_ints(self):
-        self.assertEquals([1, 8, 4], sanitize_closed_announcements("[1, 8, 4]"))
+        self.assertEqual([1, 8, 4], sanitize_closed_announcements("[1, 8, 4]"))
 
     def test_sanitize_closed_announcements_list_of_different_types(self):
-        self.assertEquals(
+        self.assertEqual(
             [1, 8, 4], sanitize_closed_announcements('[1, "bla", 8, 4, 123.4, {"a": "b"}, ["This is also a list"]]')
         )
 

website/orders/api/v1/views.py

@@ -21,7 +21,6 @@
 from orders.models import Order, Shift, Product, OrderVenue
 from orders.services import (
     user_can_manage_shifts_in_venue,
-    user_can_manage_shift,
     add_scanned_order,
     user_gets_prioritized_orders,
 )
@@ -62,7 +61,7 @@ def get_serializer_context(self):
     def perform_create(self, serializer):
         """Create an order, either as ordering users or as managers."""
         shift = self.kwargs.get("shift")
-        if user_can_manage_shift(self.request.user, shift):
+        if user_can_manage_shifts_in_venue(self.request.user, shift.venue):
             # Save the order as it was passed to the API as the user has permission to save orders for all users in
             # the shift.
             order = serializer.save(shift=shift, user=self.request.user)
@@ -117,7 +116,7 @@ class OrderRetrieveUpdateDestroyAPIView(LoggedRetrieveUpdateDestroyAPIView):
     def put(self, request, *args, **kwargs):
         """PUT is only available for shift managers."""
         shift = self.kwargs.get("shift")
-        if user_can_manage_shift(request.user, shift):
+        if user_can_manage_shifts_in_venue(request.user, shift.venue):
             return super().put(request, *args, **kwargs)
         else:
             self.permission_denied(request)
@@ -126,7 +125,7 @@ def update(self, request, *args, **kwargs):
         """Update an order."""
         instance = self.get_object()
         shift = self.kwargs.get("shift")
-        if user_can_manage_shift(self.request.user, shift):
+        if user_can_manage_shifts_in_venue(request.user, shift.venue):
             # All changeable order fields can be changed by the user.
             return super().update(request, *args, **kwargs)
         else:
@@ -155,7 +154,7 @@ def update(self, request, *args, **kwargs):
     def destroy(self, request, *args, **kwargs):
         """Destroy an order."""
         shift = kwargs.get("shift")
-        if not user_can_manage_shift(request.user, shift):
+        if not user_can_manage_shifts_in_venue(request.user, shift.venue):
             self.permission_denied(request)
         return super().destroy(request, *args, **kwargs)
 
@@ -230,7 +229,7 @@ class ShiftRetrieveUpdateAPIView(LoggedRetrieveUpdateAPIView):
     def update(self, request, *args, **kwargs):
         """Update a shift."""
         shift = get_object_or_404(Shift, pk=kwargs.get("pk"))
-        if not user_can_manage_shift(request.user, shift):
+        if not user_can_manage_shifts_in_venue(request.user, shift.venue):
             self.permission_denied(request)
         return super().update(request, *args, **kwargs)
 
@@ -282,7 +281,7 @@ def post(self, request, **kwargs):
         except Product.DoesNotExist:
             return Response(status=status.HTTP_404_NOT_FOUND)
 
-        if not user_can_manage_shift(request.user, shift):
+        if not user_can_manage_shifts_in_venue(request.user, shift.venue):
             raise PermissionDenied
 
         order = add_scanned_order(product, shift)

website/orders/services.py

@@ -13,11 +13,6 @@
 logger = logging.getLogger(__name__)
 
 
-def user_can_manage_shift(user, shift):
-    """Return if the user can manage this shift."""
-    return user_can_manage_shifts_in_venue(user, shift.venue) and user in shift.assignees.all()
-
-
 def user_can_manage_shifts_in_venue(user, venue):
     """Return if the user can manage this shift."""
     return user.has_perm("orders.can_manage_shift_in_venue", venue)

website/orders/templates/orders/join_shift.html

@@ -9,12 +9,12 @@
     <div class="container my-5">
         <div class="row justify-content-center">
             <form method="post" class="col-8 col-sm-offset-2">
-                <h1>Join {{ shift.venue.venue.name }} shift #{{ shift.pk }}</h1>
+                <h1>Join/Manage {{ shift.venue.venue.name }} shift #{{ shift.pk }}</h1>
                 {% bootstrap_messages %}
                 <p>Do you want to join this shift as a baker (add yourself to the baker list)?</p>
                 {% csrf_token %}
-                {% bootstrap_button button_type="submit" name="confirm" value="Yes" content="Yes" button_class="btn-success" extra_classes="mt-4 w-100" %}
-                {% bootstrap_button button_type="submit" name="confirm" value="No" content="No" button_class="btn-danger" extra_classes="mt-4 w-100" %}
+                {% bootstrap_button button_type="submit" name="confirm" value="Yes" content="Yes, add me to the baker list" button_class="btn-success" extra_classes="mt-4 w-100" %}
+                {% bootstrap_button button_type="submit" name="confirm" value="No" content="No, I only want to manage" button_class="btn-danger" extra_classes="mt-4 w-100" %}
             </form>
         </div>
     </div>

website/orders/templates/orders/shift_overview.html

@@ -9,7 +9,7 @@
 
 {% block footer %}
     {% if can_manage_shift and user not in shift.assignees.all %}
-        <footer class="navbar navbar-expand-md mt-auto footer" style="z-index: unset;">
+        <footer class="page-footer navbar navbar-expand-md mt-auto footer" style="z-index: unset;">
             <div class="container text-center">
                 <div class="row flex-grow-1">
                     <div class="col">

website/orders/tests/test_views.py

@@ -136,8 +136,13 @@ def test_join_shift_view_post_no(self):
         self.assertTrue(self.normal_user.has_perm("orders.can_manage_shift_in_venue", self.order_venue_1))
         self.assertFalse(self.normal_user in self.shift.assignees.all())
         response = self.client.post(reverse("orders:shift_join", kwargs={"shift": self.shift}), data={"confirm": "No"})
-        self.assertRedirects(response, reverse("index"))
+        self.assertRedirects(response, reverse("orders:shift_admin", kwargs={"shift": self.shift}))
         self.assertFalse(self.normal_user in self.shift.assignees.all())
+        self.assertTrue(
+            str(response.cookies.get(f"TOSTI_ASKED_JOIN_SHIFT_{self.shift.id}", None)).startswith(
+                f"Set-Cookie: TOSTI_ASKED_JOIN_SHIFT_{self.shift.id}=true;"
+            )
+        )
 
     def test_join_shift_view_post_no_data(self):
         self.assertTrue(self.client.login(username=self.normal_user.username, password="temporary"))
@@ -167,4 +172,4 @@ def test_shift_admin_view_no_permissions(self):
         self.assertTrue(self.client.login(username=self.normal_user.username, password="temporary"))
         self.assertFalse(self.normal_user.has_perm("orders.can_manage_shift_in_venue", self.order_venue_1))
         response = self.client.get(reverse("orders:shift_admin", kwargs={"shift": self.shift}))
-        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.status_code, 403)

website/orders/views.py

@@ -17,7 +17,6 @@
 from .forms import CreateShiftForm
 from .services import (
     user_is_blacklisted,
-    user_can_manage_shift,
     user_can_manage_shifts_in_venue,
     generate_order_statistics,
     generate_orders_per_venue_statistics,
@@ -38,7 +37,7 @@ def get_context_data(self, **kwargs):
             {
                 "shift": shift,
                 "can_manage_shift": self.request.user.is_authenticated
-                and user_can_manage_shift(self.request.user, shift),
+                and user_can_manage_shifts_in_venue(self.request.user, shift.venue),
                 "has_order_permissions": self.request.user.is_authenticated
                 and not user_is_blacklisted(self.request.user),
                 "user_gets_priority": user_gets_prioritized_orders(self.request.user, shift),
@@ -55,16 +54,28 @@ class ShiftManagementView(LoginRequiredMixin, TemplateView):
     def dispatch(self, request, *args, **kwargs):
         """Redirect users that haven't joined the shift to the join page."""
         shift = kwargs.get("shift")
-        if not user_can_manage_shift(request.user, shift):
+
+        if not user_can_manage_shifts_in_venue(request.user, shift.venue):
+            raise PermissionDenied
+
+        if request.user in shift.assignees.all():
+            return super(ShiftManagementView, self).dispatch(request, *args, **kwargs)
+
+        asked_join_shift = request.COOKIES.get(f"TOSTI_ASKED_JOIN_SHIFT_{shift.id}", None)
+
+        if asked_join_shift is not None:
+            # We have already asked the user whether they wanted to join the shift, and they denied.
+            return super(ShiftManagementView, self).dispatch(request, *args, **kwargs)
+        else:
+            # We will ask the user whether they want to join the shift.
             return redirect("orders:shift_join", shift=shift)
-        return super(ShiftManagementView, self).dispatch(request, *args, **kwargs)
 
     def get_context_data(self, **kwargs):
         """Get context data."""
         context = super(ShiftManagementView, self).get_context_data(**kwargs)
         shift = kwargs.get("shift")
         context.update(
-            {"shift": shift, "has_change_order_permissions": user_can_manage_shift(self.request.user, shift)},
+            {"shift": shift},
         )
         return context
 
@@ -140,7 +151,10 @@ def post(self, request, **kwargs):
             log_action(request.user, shift, CHANGE, "Joined shift via website.")
             return redirect("orders:shift_admin", shift=shift)
         elif confirm == "No":
-            return redirect("index")
+            response = redirect("orders:shift_admin", shift=shift)
+            # Set a cookie that will prevent this page from displaying for 10 minutes.
+            response.set_cookie(f"TOSTI_ASKED_JOIN_SHIFT_{shift.id}", "true", max_age=600)
+            return response
         else:
             return render(request, self.template_name, {"shift": shift})
 

website/tosti/templates/tosti/venue_card.html

@@ -28,7 +28,7 @@ <h5>No music player</h5>
                         <a href="{% url 'orders:shift_join' shift=venue_shift %}">
                             <div class="btn-ml btn-on my-2">
                                 <h4 class="my-0">
-                                    Join shift at {{ venue }}
+                                    Join/Manage shift at {{ venue }}
                                 </h4>
                             </div>
                         </a>

website/tosti/templatetags/venue_cards.py

@@ -1,7 +1,7 @@
 from django import template
 from django.utils import timezone
 
-from orders.services import user_can_manage_shifts_in_venue, user_can_manage_shift
+from orders.services import user_can_manage_shifts_in_venue
 from thaliedje.models import Player
 from orders.models import OrderVenue
 
@@ -23,7 +23,7 @@ def render_venue_card(context, shift=None, venue=None, show_player=True, show_ve
         or None,
         "request": context.get("request"),
         "admin": (venue and user_can_manage_shifts_in_venue(context["request"].user, venue))
-        or (shift and user_can_manage_shift(context["request"].user, shift)),
+        or (shift and user_can_manage_shifts_in_venue(context["request"].user, shift.venue)),
         "show_player": show_player and Player.get_player(venue.venue) is not None,
     }