chore: apply suggestions from code review

bc_obps/registration/api/v1/transfer_events.py

@@ -1,4 +1,5 @@
-from typing import List, Literal, Optional, Dict, Any
+from typing import List, Literal, Optional
+from registration.models.event.transfer_event import TransferEvent
 from registration.schema.v1.transfer_event import TransferEventFilterSchema, TransferEventListOut
 from service.transfer_event_service import TransferEventService
 from common.permissions import authorize
@@ -11,6 +12,7 @@
 from service.error_service.custom_codes_4xx import custom_codes_4xx
 from ninja import Query
 from registration.schema.generic import Message
+from django.db.models import QuerySet
 
 
 @router.get(
@@ -29,6 +31,6 @@ def list_transfer_events(
     sort_field: Optional[str] = "status",
     sort_order: Optional[Literal["desc", "asc"]] = "desc",
     paginate_result: bool = Query(True, description="Whether to paginate the results"),
-) -> List[Dict[str, Any]]:
+) -> QuerySet[TransferEvent]:
     # NOTE: PageNumberPagination raises an error if we pass the response as a tuple (like 200, ...)
     return TransferEventService.list_transfer_events(sort_field, sort_order, filters)

bc_obps/registration/schema/v1/transfer_event.py

@@ -1,20 +1,13 @@
 from typing import Optional
 from uuid import UUID
 
-from registration.models.facility import Facility
 from registration.models.event.transfer_event import TransferEvent
 from ninja import ModelSchema, Field, FilterSchema
 from django.db.models import Q
 import re
 from typing import Dict, Any
 
 
-class FacilityForTransferEventGrid(ModelSchema):
-    class Meta:
-        model = Facility
-        fields = ['name']
-
-
 class TransferEventListOut(ModelSchema):
     operation__id: Optional[UUID] = None
     operation__name: Optional[str] = Field(None, alias="operation__name")
@@ -46,7 +39,8 @@ class TransferEventFilterSchema(FilterSchema):
     facilities__name: Optional[str] = Field(None, json_schema_extra={'q': 'facilities__name__icontains'})
     status: Optional[str] = Field(None, json_schema_extra={'q': 'status__icontains'})
 
-    def filtering_including_not_applicable(self, field: str, value: str) -> Q:
+    @staticmethod
+    def filtering_including_not_applicable(field: str, value: str) -> Q:
         if value and re.search(value, 'n/a', re.IGNORECASE):
             return Q(**{f"{field}__icontains": value}) | Q(**{f"{field}__isnull": True})
         return Q(**{f"{field}__icontains": value}) if value else Q()

bc_obps/service/transfer_event_service.py

@@ -1,7 +1,7 @@
+from typing import cast
+from django.db.models import QuerySet
 from registration.models.event.transfer_event import TransferEvent
-from typing import Optional, Dict, Any, List
-
-
+from typing import Optional
 from registration.schema.v1.transfer_event import TransferEventFilterSchema
 from ninja import Query
 
@@ -13,7 +13,7 @@ def list_transfer_events(
         sort_field: Optional[str],
         sort_order: Optional[str],
         filters: TransferEventFilterSchema = Query(...),
-    ) -> List[Dict[str, Any]]:
+    ) -> QuerySet[TransferEvent]:
         sort_direction = "-" if sort_order == "desc" else ""
         sort_by = f"{sort_direction}{sort_field}"
         queryset = (
@@ -29,4 +29,4 @@ def list_transfer_events(
             )
             .distinct()
         )
-        return list(queryset)
+        return cast(QuerySet[TransferEvent], queryset)

bciers/apps/administration/app/components/operations/OperationDataGridPage.tsx

@@ -20,9 +20,8 @@ export default async function OperationDataGridPage({
     rows: OperationRow[];
     row_count: number;
   } = await fetchOperationsPageData(searchParams);
-  if (!operations) {
-    return <div>No operations data in database.</div>;
-  }
+  if (!operations || "error" in operations)
+    throw new Error("Failed to retrieve operations");
 
   const isAuthorizedAdminUser = [
     FrontEndRoles.CAS_ADMIN,

bciers/apps/administration/app/components/operators/OperatorDataGridPage.tsx

@@ -15,9 +15,8 @@ export default async function Operators({
     rows: OperatorRow[];
     row_count: number;
   } = await fetchOperatorsPageData(searchParams);
-  if (!operators) {
-    return <div>No operator data in database.</div>;
-  }
+  if (!operators || "error" in operators)
+    throw new Error("Failed to retrieve operators");
 
   // Render the DataGrid component
   return (

bciers/apps/administration/tests/components/operations/OperationDataGridPage.test.tsx

@@ -51,11 +51,12 @@ describe("Operations component", () => {
     vi.clearAllMocks();
   });
 
-  it("renders a message when there are no operations in the database", async () => {
+  it("throws an error when there's a problem fetching data", async () => {
     fetchOperationsPageData.mockReturnValueOnce(undefined);
-    render(await Operations({ searchParams: {} }));
+    await expect(async () => {
+      render(await Operations({ searchParams: {} }));
+    }).rejects.toThrow("Failed to retrieve operations");
     expect(screen.queryByRole("grid")).not.toBeInTheDocument();
-    expect(screen.getByText(/No operations data in database./i)).toBeVisible();
   });
 
   it("renders the OperationDataGrid component when there are operations in the database", async () => {

bciers/apps/administration/tests/components/operators/OperatorDataGridPage.test.tsx

@@ -54,11 +54,13 @@ describe("OperatorDataGridPage component", () => {
     vi.clearAllMocks();
   });
 
-  it("renders a message when there are no operators in the database", async () => {
+  it("throws an error when there's a problem fetching data", async () => {
     fetchOperatorsPageData.mockReturnValueOnce(undefined);
-    render(await Operators({ searchParams: {} }));
+    await expect(async () => {
+      render(await Operators({ searchParams: {} }));
+    }).rejects.toThrow("Failed to retrieve operators");
+
     expect(screen.queryByRole("grid")).not.toBeInTheDocument();
-    expect(screen.getByText(/No operator data in database./i)).toBeVisible();
   });
 
   it("renders the OperatorDataGrid component when there are operators in the database", async () => {

bciers/apps/registration/app/components/datagrid/models/transfers/transferColumns.ts

@@ -7,7 +7,6 @@ const transferColumns = (
     {
       field: "created_at",
       headerName: "Submission Date",
-      // Set flex to 1 to make the column take up all the remaining width if user zooms out
       width: 200,
     },
     { field: "operation__name", headerName: "Operation", width: 200 },

bciers/apps/registration/app/components/transfers/TransfersDataGrid.tsx

@@ -25,7 +25,6 @@ const TransfersDataGrid = ({
     row_count: number;
   };
 }) => {
-  console.log("initial", initialData);
   const [lastFocusedField, setLastFocusedField] = useState<string | null>(null);
 
   const SearchCell = useMemo(

bciers/apps/registration/app/components/transfers/TransfersDataGridPage.tsx

@@ -15,9 +15,8 @@ export default async function TransfersDataGridPage({
     rows: TransferRow[];
     row_count: number;
   } = await fetchTransferEventsPageData(searchParams);
-  if (!transfers) {
-    return <div>No transfers data in database.</div>;
-  }
+  if (!transfers || "error" in transfers)
+    throw new Error("Failed to retrieve transfers");
 
   // Render the DataGrid component
   return (

bciers/apps/registration/tests/components/transfers/TransfersDataGridPage.test.tsx

@@ -73,11 +73,12 @@ describe("Transfers component", () => {
     vi.clearAllMocks();
   });
 
-  it("renders a message when there are no transfers in the database", async () => {
+  it("throws an error when there's a problem fetching data", async () => {
     fetchTransferEventsPageData.mockReturnValueOnce(undefined);
-    render(await TransfersDataGridPage({ searchParams: {} }));
+    await expect(async () => {
+      render(await TransfersDataGridPage({ searchParams: {} }));
+    }).rejects.toThrow("Failed to retrieve transfers");
     expect(screen.queryByRole("grid")).not.toBeInTheDocument();
-    expect(screen.getByText(/No transfers data in database./i)).toBeVisible();
   });
 
   it("renders the TransfersDataGrid component when there are transfers in the database", async () => {