(fix) Remove unnecessary IPC request to Platform to get pennid (it's the id field of user)

Author: darrentweng

backend/gsr_booking/api_wrapper.py

@@ -18,7 +18,6 @@
 
 from gsr_booking.models import GSR, GroupMembership, GSRBooking, Reservation
 from gsr_booking.serializers import GSRBookingSerializer, GSRSerializer
-from portal.logic import get_user_info
 from utils.errors import APIError
 
 
@@ -231,29 +230,15 @@ def request(self, *args, **kwargs):
         except (ConnectTimeout, ReadTimeout, ConnectionError):
             raise APIError("AGH LibCal: Connection timeout")
 
-    def get_user_pennid(self, user):
-        """
-        Get user's pennid.
-        """
-        user_info = get_user_info(user)
-        pennid = user_info.get("pennid")
-        if not pennid:
-            raise APIError("PennGroups: User pennid not found in Platform API response")
-        return pennid
-
     def get_authorized_rooms(self, user):
         """
         Check which AGH rooms the user can access via PennGroups API.
-        Returns dict mapping room extensions to their group info, or None if API fails.
+        Returns dict mapping room extensions to their group info, or empty dict if none.
         """
-        try:
-            # Get pennid from Platform API (PennGroups requires numerical pennid, not username)
-            try:
-                pennid = self.get_user_pennid(user)
-            except APIError:
-                raise APIError("PennGroups: Failed to get user pennid")
 
-            url = f"{PENNGROUPS_URL}{pennid}/groups"
+        try:
+            # user.id is the pennid (set by LabsUserBackend at authentication time)
+            url = f"{PENNGROUPS_URL}{user.id}/groups"
             response = requests.get(
                 url,
                 auth=HTTPBasicAuth(settings.PENNGROUPS_USERNAME, settings.PENNGROUPS_PASSWORD),

backend/portal/logic.py

@@ -1,8 +1,6 @@
 import json
-import os
 from collections import defaultdict
 
-import requests
 from accounts.ipc import authenticated_request
 from django.contrib.auth import get_user_model
 from rest_framework.exceptions import PermissionDenied
@@ -14,38 +12,11 @@
 
 
 def get_user_info(user):
-    """
-    Returns Platform user information.
-
-    In production: Uses IPC (authenticated_request)
-    Locally (with PLATFORM_BEARER_TOKEN): Uses direct HTTP request with bearer token
-    """
-    # Try IPC first (production)
-    try:
-        response = authenticated_request(user, "GET", "https://platform.pennlabs.org/accounts/me/")
-        if response.status_code == 403:
-            raise PermissionDenied("IPC request failed")
-        return json.loads(response.content)
-    except Exception as ipc_error:
-        # Fall back to bearer token for local development/testing
-        bearer_token = os.getenv("PLATFORM_BEARER_TOKEN")
-        if bearer_token:
-            try:
-                response = requests.get(
-                    "https://platform.pennlabs.org/accounts/me/",
-                    headers={"Authorization": f"Bearer {bearer_token}"},
-                    timeout=5,
-                )
-                if response.status_code == 401:
-                    raise PermissionDenied("Invalid PLATFORM_BEARER_TOKEN")
-                if response.status_code != 200:
-                    raise PermissionDenied(f"Platform API error: HTTP {response.status_code}")
-                return response.json()
-            except requests.exceptions.RequestException as e:
-                raise PermissionDenied(f"Platform API request failed: {str(e)}")
-
-        # No bearer token available, re-raise the original IPC error
-        raise PermissionDenied(f"IPC request failed: {str(ipc_error)}")
+    """Returns Platform user information"""
+    response = authenticated_request(user, "GET", "https://platform.pennlabs.org/accounts/me/")
+    if response.status_code == 403:
+        raise PermissionDenied("IPC request failed")
+    return json.loads(response.content)
 
 
 def get_user_clubs(user):

backend/tests/gsr_booking/test_gsr_wrapper.py

@@ -153,11 +153,6 @@ def json(self):
     return MockResponse({})
 
 
-def mock_get_user_pennid(self, user):
-    """Mock pennid extraction"""
-    return "12345678"
-
-
 class TestBookingWrapper(TestCase):
     @classmethod
     def setUpTestData(cls):
@@ -378,9 +373,6 @@ def test_group_wharton_availability(self, mock_is_seas, mock_is_wharton):
     @mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False)
     @mock.patch("gsr_booking.api_wrapper.PennGroupsBookingWrapper.request", mock_agh_libcal_request)
     @mock.patch("requests.get", mock_penngroups_api_get)
-    @mock.patch(
-        "gsr_booking.api_wrapper.PennGroupsBookingWrapper.get_user_pennid", mock_get_user_pennid
-    )
     def test_is_seas(self, mock_is_wharton):
         """Test SEAS status checking via PennGroups API"""
         from gsr_booking.api_wrapper import PennGroupsGSRBooker
@@ -478,9 +470,6 @@ def test_groupmembership_auto_sets_is_seas_false(self, mock_model_is_seas, mock_
     @mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False)
     @mock.patch("requests.get", mock_penngroups_api_get)
     @mock.patch("gsr_booking.api_wrapper.PennGroupsBookingWrapper.request", mock_agh_libcal_request)
-    @mock.patch(
-        "gsr_booking.api_wrapper.PennGroupsBookingWrapper.get_user_pennid", mock_get_user_pennid
-    )
     def test_penngroups_availability(self, mock_is_seas, mock_is_wharton):
         """Test AGH room availability for SEAS students"""
         availability = GSRBooker.get_availability(
@@ -508,9 +497,6 @@ def test_penngroups_availability(self, mock_is_seas, mock_is_wharton):
     @mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False)
     @mock.patch("requests.get", mock_penngroups_api_get)
     @mock.patch("gsr_booking.api_wrapper.PennGroupsBookingWrapper.request", mock_agh_libcal_request)
-    @mock.patch(
-        "gsr_booking.api_wrapper.PennGroupsBookingWrapper.get_user_pennid", mock_get_user_pennid
-    )
     def test_book_penngroups(self, mock_is_seas, mock_is_wharton):
         """Test booking an AGH room"""
         initial_reservation_count = Reservation.objects.count()
@@ -563,9 +549,6 @@ def test_cancel_penngroups(self, mock_is_seas, mock_is_wharton):
     @mock.patch("requests.get", mock_penngroups_api_get)
     @mock.patch("gsr_booking.api_wrapper.PennGroupsBookingWrapper.request", mock_agh_libcal_request)
     @mock.patch("gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=True)
-    @mock.patch(
-        "gsr_booking.api_wrapper.PennGroupsBookingWrapper.get_user_pennid", mock_get_user_pennid
-    )
     def test_group_book_penngroups(self, mock_model_is_seas, mock_is_wharton):
         """Test group booking for AGH rooms"""
         # Make sure group_user is SEAS - when membership is created, is_seas will be auto-set
@@ -649,9 +632,6 @@ def test_non_seas_book_fails(self, mock_is_seas, mock_is_wharton):
     @mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False)
     @mock.patch("requests.get", mock_penngroups_api_get)
     @mock.patch("gsr_booking.api_wrapper.PennGroupsBookingWrapper.request")
-    @mock.patch(
-        "gsr_booking.api_wrapper.PennGroupsBookingWrapper.get_user_pennid", mock_get_user_pennid
-    )
     def test_unauthorized_room_booking_fails(self, mock_request, mock_is_seas, mock_is_wharton):
         """Test that users cannot book rooms they're not authorized for"""
 
@@ -714,9 +694,6 @@ def json(self):
     @mock.patch("requests.get", mock_penngroups_api_get)
     @mock.patch("gsr_booking.api_wrapper.PennGroupsBookingWrapper.request", mock_agh_libcal_request)
     @mock.patch("gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=True)
-    @mock.patch(
-        "gsr_booking.api_wrapper.PennGroupsBookingWrapper.get_user_pennid", mock_get_user_pennid
-    )
     def test_group_penngroups_availability(self, mock_model_is_seas, mock_is_wharton):
         """Test group availability for AGH rooms"""
         # Test that group availability works when group has SEAS members
@@ -808,9 +785,6 @@ def test_is_room_authorized(self):
 
     @mock.patch("gsr_booking.models.WhartonGSRBooker.is_wharton", return_value=False)
     @mock.patch("requests.get")
-    @mock.patch(
-        "gsr_booking.api_wrapper.PennGroupsBookingWrapper.get_user_pennid", mock_get_user_pennid
-    )
     def test_get_authorized_rooms_api_errors(self, mock_get, mock_is_wharton):
         """Test error handling in get_authorized_rooms"""
         from gsr_booking.api_wrapper import PennGroupsGSRBooker
@@ -879,9 +853,6 @@ def test_cancel_penngroups_marks_booking_cancelled(self, mock_is_seas, mock_is_w
     @mock.patch("requests.get", mock_penngroups_api_get)
     @mock.patch("gsr_booking.api_wrapper.PennGroupsBookingWrapper.request", mock_agh_libcal_request)
     @mock.patch("gsr_booking.models.PennGroupsGSRBooker.is_seas", return_value=True)
-    @mock.patch(
-        "gsr_booking.api_wrapper.PennGroupsBookingWrapper.get_user_pennid", mock_get_user_pennid
-    )
     def test_group_book_penngroups_credit_distribution(self, mock_model_is_seas, mock_is_wharton):
         """Test that group bookings properly distribute credits among members"""
         # Set up group with multiple SEAS members