Fix AGH issue by maintaining old unauthenticated locations endpoint, adding new authenticated locations_user endpoint. Clean up GSR booking tests and cache clearing.

Author: darrentweng

backend/gsr_booking/urls.py

@@ -14,6 +14,7 @@
     MyMembershipViewSet,
     RecentGSRs,
     ReservationsView,
+    UserLocations,
 )
 
 
@@ -27,6 +28,7 @@
 urlpatterns = [
     path("", include(router.urls)),
     path("locations/", Locations.as_view(), name="locations"),
+    path("user-locations/", UserLocations.as_view(), name="user-locations"),
     path("recent/", RecentGSRs.as_view(), name="recent-gsrs"),
     path("wharton/", CheckWharton.as_view(), name="is-wharton"),
     path("seas/", CheckSEAS.as_view(), name="is-seas"),

backend/gsr_booking/views.py

@@ -145,6 +145,13 @@ def get_queryset(self):
 class Locations(generics.ListAPIView):
     """Lists all available locations to book from"""
 
+    serializer_class = GSRSerializer
+    queryset = GSR.objects.all()
+
+
+class UserLocations(generics.ListAPIView):
+    """Lists all available locations to book based on user permission level"""
+
     serializer_class = GSRSerializer
     permission_classes = [IsAuthenticated]
 

backend/tests/gsr_booking/test_gsr_booking.py

@@ -1,3 +1,5 @@
+from unittest import mock
+
 from django.contrib.auth import get_user_model
 from django.test import TestCase
 from rest_framework.test import APIClient
@@ -17,13 +19,16 @@ def setUp(self):
             username="user2", password="password", first_name="user", last_name="two"
         )
 
-        Group.objects.create(
-            owner=self.user1, name="g1", color="blue"
-        )  # creating group also adds user
-        group2 = Group.objects.create(owner=self.user2, name="g2", color="blue")
-        GroupMembership.objects.create(user=self.user1, group=group2, accepted=True)
-        group3 = Group.objects.create(owner=self.user2, name="g3", color="blue")
-        GroupMembership.objects.create(user=self.user1, group=group3)
+        with mock.patch(
+            "gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False
+        ), mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False):
+            Group.objects.create(
+                owner=self.user1, name="g1", color="blue"
+            )  # creating group also adds user
+            group2 = Group.objects.create(owner=self.user2, name="g2", color="blue")
+            GroupMembership.objects.create(user=self.user1, group=group2, accepted=True)
+            group3 = Group.objects.create(owner=self.user2, name="g3", color="blue")
+            GroupMembership.objects.create(user=self.user1, group=group3)
         self.client = APIClient()
         self.client.login(username="user1", password="password")
 
@@ -42,11 +47,23 @@ class MembershipViewTestCase(TestCase):
     def setUp(self):
         self.user1 = User.objects.create_user(username="user1", password="password")
         self.user2 = User.objects.create_user(username="user2", password="password")
+        self._patcher_seas = mock.patch(
+            "gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False
+        )
+        self._patcher_wharton = mock.patch(
+            "gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False
+        )
+        self._patcher_seas.start()
+        self._patcher_wharton.start()
         self.group = Group.objects.create(owner=self.user1, name="g1", color="blue")
         self.group2 = Group.objects.create(owner=self.user2, name="g2", color="white")
         self.client = APIClient()
         self.client.login(username="user1", password="password")
 
+    def tearDown(self):
+        self._patcher_seas.stop()
+        self._patcher_wharton.stop()
+
     def test_invite_single(self):
         self.client.login(username="user2", password="password")
         response = self.client.post(
@@ -148,11 +165,23 @@ class GroupTestCase(TestCase):
     def setUp(self):
         self.user1 = User.objects.create_user(username="user1", password="password")
         self.user2 = User.objects.create_user(username="user2", password="password")
+        self._patcher_seas = mock.patch(
+            "gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False
+        )
+        self._patcher_wharton = mock.patch(
+            "gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False
+        )
+        self._patcher_seas.start()
+        self._patcher_wharton.start()
         self.group = Group.objects.create(owner=self.user1, name="g1", color="blue")
         self.group2 = Group.objects.create(owner=self.user2, name="g2", color="white")
         self.client = APIClient()
         self.client.login(username="user1", password="password")
 
+    def tearDown(self):
+        self._patcher_seas.stop()
+        self._patcher_wharton.stop()
+
     def test_get_groups(self):
         response = self.client.get("/gsr/groups/")
         self.assertEqual(200, response.status_code)

backend/tests/gsr_booking/test_gsr_views.py

@@ -86,7 +86,10 @@ class TestGSRs(TestCase):
     def setUpTestData(cls):
         create_test_gsrs(cls)
         test_user = User.objects.create_user("user1", "user")
-        Group.objects.create(owner=test_user, name="Penn Labs", color="blue")
+        with mock.patch(
+            "gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False
+        ), mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False):
+            Group.objects.create(owner=test_user, name="Penn Labs", color="blue")
 
     def setUp(self):
         # Clear cache to avoid stale data from previous tests
@@ -100,9 +103,22 @@ def setUp(self):
     @mock.patch("gsr_booking.views.WhartonGSRBooker.is_wharton", return_value=False)
     @mock.patch("gsr_booking.views.PennGroupsGSRBooker.is_seas", return_value=False)
     def test_get_location(self, mock_is_seas, mock_is_wharton):
-        """Test that regular users do not see Wharton or PennGroups GSRs"""
         response = self.client.get(reverse("locations"))
         res_json = json.loads(response.content)
+        for entry in res_json:
+            if entry["id"] == 1:
+                self.assertEquals(entry["name"], "Huntsman")
+            if entry["id"] == 2:
+                self.assertEquals(entry["name"], "Amy Gutmann Hall")
+            if entry["id"] == 3:
+                self.assertEquals(entry["name"], "Weigle")
+
+    @mock.patch("gsr_booking.views.WhartonGSRBooker.is_wharton", return_value=False)
+    @mock.patch("gsr_booking.views.PennGroupsGSRBooker.is_seas", return_value=False)
+    def test_get_user_location(self, mock_is_seas, mock_is_wharton):
+        """Test that regular users do not see Wharton or PennGroups GSRs"""
+        response = self.client.get(reverse("user-locations"))
+        res_json = json.loads(response.content)
 
         gsr_ids = [entry["id"] for entry in res_json]
 
@@ -115,9 +131,9 @@ def test_get_location(self, mock_is_seas, mock_is_wharton):
 
     @mock.patch("gsr_booking.views.WhartonGSRBooker.is_wharton", return_value=False)
     @mock.patch("gsr_booking.views.PennGroupsGSRBooker.is_seas", return_value=False)
-    def test_get_location_regular_user(self, mock_is_seas, mock_is_wharton):
+    def test_get_user_location_regular_user(self, mock_is_seas, mock_is_wharton):
         """Test that regular users only see LibCal GSRs"""
-        response = self.client.get(reverse("locations"))
+        response = self.client.get(reverse("user-locations"))
         res_json = json.loads(response.content)
 
         # Regular users should only see LibCal GSRs
@@ -128,9 +144,9 @@ def test_get_location_regular_user(self, mock_is_seas, mock_is_wharton):
 
     @mock.patch("gsr_booking.views.WhartonGSRBooker.is_wharton", return_value=True)
     @mock.patch("gsr_booking.views.PennGroupsGSRBooker.is_seas", return_value=False)
-    def test_get_location_wharton_user(self, mock_is_seas, mock_is_wharton):
+    def test_get_user_location_wharton_user(self, mock_is_seas, mock_is_wharton):
         """Test that Wharton users see LibCal and Wharton GSRs"""
-        response = self.client.get(reverse("locations"))
+        response = self.client.get(reverse("user-locations"))
         res_json = json.loads(response.content)
 
         # Wharton users should see LibCal and Wharton GSRs
@@ -145,9 +161,9 @@ def test_get_location_wharton_user(self, mock_is_seas, mock_is_wharton):
 
     @mock.patch("gsr_booking.views.WhartonGSRBooker.is_wharton", return_value=False)
     @mock.patch("gsr_booking.views.PennGroupsGSRBooker.is_seas", return_value=True)
-    def test_get_location_seas_user(self, mock_is_seas, mock_is_wharton):
+    def test_get_user_location_seas_user(self, mock_is_seas, mock_is_wharton):
         """Test that SEAS users see LibCal and PennGroups GSRs"""
-        response = self.client.get(reverse("locations"))
+        response = self.client.get(reverse("user-locations"))
         res_json = json.loads(response.content)
 
         # SEAS users should see LibCal and PennGroups GSRs
@@ -162,9 +178,9 @@ def test_get_location_seas_user(self, mock_is_seas, mock_is_wharton):
 
     @mock.patch("gsr_booking.views.WhartonGSRBooker.is_wharton", return_value=True)
     @mock.patch("gsr_booking.views.PennGroupsGSRBooker.is_seas", return_value=True)
-    def test_get_location_wharton_seas_user(self, mock_is_seas, mock_is_wharton):
+    def test_get_user_location_wharton_seas_user(self, mock_is_seas, mock_is_wharton):
         """Test that users with both Wharton and SEAS access see all non-Penn Labs GSRs"""
-        response = self.client.get(reverse("locations"))
+        response = self.client.get(reverse("user-locations"))
         res_json = json.loads(response.content)
 
         # Users with both should see all kinds
@@ -177,15 +193,17 @@ def test_get_location_wharton_seas_user(self, mock_is_seas, mock_is_wharton):
         self.assertIn(GSR.KIND_WHARTON, kinds_seen)
         self.assertIn(GSR.KIND_PENNGROUPS, kinds_seen)
 
-    def test_get_location_penn_labs_member(self):
+    @mock.patch("gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False)
+    @mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False)
+    def test_get_user_location_penn_labs_member(self, mock_is_wharton, mock_is_seas):
         """Test that Penn Labs members see all GSRs regardless of their individual status"""
         # Add user to Penn Labs group
         penn_labs_group = Group.objects.get(name="Penn Labs")
         GroupMembership.objects.create(
             user=self.user, group=penn_labs_group, accepted=True, type=GroupMembership.MEMBER
         )
 
-        response = self.client.get(reverse("locations"))
+        response = self.client.get(reverse("user-locations"))
         res_json = json.loads(response.content)
 
         # Penn Labs members should see all GSRs
@@ -212,9 +230,9 @@ def test_get_location_penn_labs_member(self):
 
     @mock.patch("gsr_booking.views.WhartonGSRBooker.is_wharton", return_value=False)
     @mock.patch("gsr_booking.views.PennGroupsGSRBooker.is_seas", return_value=False)
-    def test_get_location_api_error_handling(self, mock_is_seas, mock_is_wharton):
+    def test_get_user_location_api_error_handling(self, mock_is_seas, mock_is_wharton):
         """Test that when permission checks return False, users only see LibCal GSRs"""
-        response = self.client.get(reverse("locations"))
+        response = self.client.get(reverse("user-locations"))
         res_json = json.loads(response.content)
 
         # When permission checks return False, users should only see LibCal GSRs
@@ -228,7 +246,10 @@ class TestGSRFunctions(TestCase):
     def setUpTestData(cls):
         create_test_gsrs(cls)
         test_user = User.objects.create_user("user1", "user")
-        Group.objects.create(owner=test_user, name="Penn Labs", color="blue")
+        with mock.patch(
+            "gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False
+        ), mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False):
+            Group.objects.create(owner=test_user, name="Penn Labs", color="blue")
 
     def setUp(self):
         self.user = User.objects.create_user("user", "user@seas.upenn.edu", "user")
@@ -368,7 +389,10 @@ class TestSEASViews(TestCase):
     def setUpTestData(cls):
         create_test_gsrs(cls)
         test_user = User.objects.create_user("user1", "user")
-        Group.objects.create(owner=test_user, name="Penn Labs", color="blue")
+        with mock.patch(
+            "gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False
+        ), mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False):
+            Group.objects.create(owner=test_user, name="Penn Labs", color="blue")
 
     def setUp(self):
         self.user = User.objects.create_user("user", "user@seas.upenn.edu", "user")
@@ -430,7 +454,10 @@ class TestPennGroupsViews(TestCase):
     def setUpTestData(cls):
         create_test_gsrs(cls)
         test_user = User.objects.create_user("user1", "user")
-        Group.objects.create(owner=test_user, name="Penn Labs", color="blue")
+        with mock.patch(
+            "gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False
+        ), mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False):
+            Group.objects.create(owner=test_user, name="Penn Labs", color="blue")
 
     def setUp(self):
         self.user = User.objects.create_user("user", "user@seas.upenn.edu", "user")

backend/tests/gsr_booking/test_gsr_wrapper.py

@@ -159,15 +159,21 @@ def mock_get_user_pennid(self, user):
 
 
 class TestBookingWrapper(TestCase):
-    def setUp(self):
+    @classmethod
+    def setUpTestData(cls):
         call_command("load_gsrs")
+
+    def setUp(self):
         self.user = User.objects.create_user("user", "user@seas.upenn.edu", "user")
         self.group_user = User.objects.create_user(
             "grou_user", "group_user@seas.upenn.edu", "group_user"
         )
         self.client = APIClient()
         self.client.force_authenticate(user=self.user)
-        self.group = Group.objects.create(owner=self.group_user, name="Penn Labs", color="blue")
+        with mock.patch(
+            "gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False
+        ), mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False):
+            self.group = Group.objects.create(owner=self.group_user, name="Penn Labs", color="blue")
 
     @mock.patch("gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False)
     @mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False)

backend/tests/gsr_booking/test_share_codes.py

@@ -1,5 +1,6 @@
 import json
 from datetime import datetime, timedelta
+from unittest import mock
 
 from django.contrib.auth import get_user_model
 from django.core.management import call_command
@@ -43,13 +44,19 @@ def load_sample_booking(owner):
 
 
 class ShareCodeViewTests(TestCase):
-    def setUp(self):
+    @classmethod
+    def setUpTestData(cls):
         call_command("load_gsrs")
+
+    def setUp(self):
         self.client = APIClient()
         self.owner = User.objects.create_user("owner", password="one")
         self.other = User.objects.create_user("other", password="two")
-        Group.objects.create(owner=self.other, name="Penn Labs", color="blue")
-        self.group = Group.objects.create(owner=self.owner, name="group", color="blue")
+        with mock.patch(
+            "gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=False
+        ), mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False):
+            Group.objects.create(owner=self.other, name="Penn Labs", color="blue")
+            self.group = Group.objects.create(owner=self.owner, name="group", color="blue")
 
         self.booking = load_sample_booking(self.owner)
 
@@ -258,8 +265,11 @@ def test_create_share_code_replaces_expired(self):
 
 
 class ShareCodeModelSerializerTests(TestCase):
-    def setUp(self):
+    @classmethod
+    def setUpTestData(cls):
         call_command("load_gsrs")
+
+    def setUp(self):
         self.user = User.objects.create_user("owner", password="one")
         self.booking = load_sample_booking(self.user)