Merge pull request #2667 from bcgov/2525-contact-grid-bug

Put operator and operation in contacts grid

bc_obps/registration/api/v2/contacts.py

@@ -1,14 +1,16 @@
 from typing import List, Literal, Optional, Tuple
 from common.permissions import authorize
 from django.http import HttpRequest
+from registration.schema.v2.contact import ContactFilterSchemaV2, ContactListOutV2
 from registration.utils import CustomPagination
 from registration.constants import CONTACT_TAGS
 from ninja.pagination import paginate
 from common.api.utils import get_current_user_guid
 from registration.decorators import handle_http_errors
 from registration.models.contact import Contact
-from registration.schema.v1.contact import ContactFilterSchema, ContactIn, ContactListOut, ContactOut
+from registration.schema.v1.contact import ContactIn, ContactOut
 from service.contact_service import ContactService
+from service.contact_service_v2 import ContactServiceV2
 from ..router import router
 from service.error_service.custom_codes_4xx import custom_codes_4xx
 from ninja import Query
@@ -18,7 +20,7 @@
 
 @router.get(
     "/contacts",
-    response={200: List[ContactListOut], custom_codes_4xx: Message},
+    response={200: List[ContactListOutV2], custom_codes_4xx: Message},
     tags=CONTACT_TAGS,
     description="""Retrieves a paginated list of contacts based on the provided filters.
     The endpoint allows authorized users to view and sort contacts associated to an operator filtered by various criteria such as first name, last name and email.""",
@@ -28,13 +30,13 @@
 @paginate(CustomPagination)
 def list_contacts(
     request: HttpRequest,
-    filters: ContactFilterSchema = Query(...),
+    filters: ContactFilterSchemaV2 = Query(...),
     sort_field: Optional[str] = "created_at",
     sort_order: Optional[Literal["desc", "asc"]] = "desc",
     paginate_result: bool = Query(True, description="Whether to paginate the results"),
 ) -> QuerySet[Contact]:
     # NOTE: PageNumberPagination raises an error if we pass the response as a tuple (like 200, ...)
-    return ContactService.list_contacts(get_current_user_guid(request), sort_field, sort_order, filters)
+    return ContactServiceV2.list_contacts_v2(get_current_user_guid(request), sort_field, sort_order, filters)
 
 
 #### POST #####

bc_obps/registration/schema/v2/contact.py

@@ -0,0 +1,22 @@
+from typing import Optional
+from ninja import ModelSchema, Field
+from registration.models import Contact
+from ninja import FilterSchema
+
+
+class ContactListOutV2(ModelSchema):
+    operators__legal_name: Optional[str] = None
+
+    class Meta:
+        model = Contact
+        fields = ['id', 'first_name', 'last_name', 'email']
+
+
+class ContactFilterSchemaV2(FilterSchema):
+    # NOTE: we could simply use the `q` parameter to filter by related fields but,
+    # due to this issue: https://github.com/vitalik/django-ninja/issues/1037 mypy is unhappy so I'm using the `json_schema_extra` parameter
+    # If we want to achieve more by using the `q` parameter, we should use it and ignore the mypy error
+    first_name: Optional[str] = Field(None, json_schema_extra={'q': 'first_name__icontains'})
+    last_name: Optional[str] = Field(None, json_schema_extra={'q': 'last_name__icontains'})
+    email: Optional[str] = Field(None, json_schema_extra={'q': 'email__icontains'})
+    operators__legal_name: Optional[str] = Field(None, json_schema_extra={'q': 'operators__legal_name__icontains'})

bc_obps/registration/tests/utils/baker_recipes.py

@@ -39,6 +39,7 @@
 
 operator = Recipe(
     Operator,
+    legal_name=seq('Operator 0'),
     bc_corporate_registry_number=generate_random_bc_corporate_registry_number,
     business_structure=BusinessStructure.objects.first(),
     mailing_address=foreign_key(address),
@@ -126,7 +127,9 @@
     meets_reporting_and_regulated_obligations=False,
     meets_notification_to_director_on_criteria_change=False,
 )
-contact = Recipe(Contact, business_role=BusinessRole.objects.first(), address=foreign_key(address))
+contact = Recipe(
+    Contact, business_role=BusinessRole.objects.first(), address=foreign_key(address), first_name=seq('Firstname 0')
+)
 
 
 transfer_event = Recipe(

bc_obps/service/contact_service_v2.py

@@ -0,0 +1,33 @@
+from typing import Optional, cast
+from django.db.models import QuerySet
+from uuid import UUID
+
+
+from registration.models.contact import Contact
+from registration.schema.v2.contact import ContactFilterSchemaV2
+from service.data_access_service.contact_service import ContactDataAccessService
+from service.data_access_service.user_service import UserDataAccessService
+from ninja import Query
+
+
+class ContactServiceV2:
+    @classmethod
+    def list_contacts_v2(
+        cls,
+        user_guid: UUID,
+        sort_field: Optional[str],
+        sort_order: Optional[str],
+        filters: ContactFilterSchemaV2 = Query(...),
+    ) -> QuerySet[Contact]:
+        user = UserDataAccessService.get_by_guid(user_guid)
+        sort_direction = "-" if sort_order == "desc" else ""
+        sort_by = f"{sort_direction}{sort_field}"
+        base_qs = ContactDataAccessService.get_all_contacts_for_user(user)
+        # we have filter before .values or else we'll get duplicate rows from the m2m relationship between operations_contacts and operators
+        queryset = (
+            filters.filter(base_qs)
+            .order_by(sort_by)
+            .values('id', 'first_name', 'last_name', 'email', 'operators__legal_name')
+            .distinct()
+        )
+        return cast(QuerySet[Contact], queryset)

bc_obps/service/tests/data_access_service/test_data_access_contact_service.py

@@ -10,16 +10,17 @@
 )
 from registration.models.user_operator import UserOperator
 from service.data_access_service.contact_service import ContactDataAccessService
+from model_bakery import baker
 
 pytestmark = pytest.mark.django_db
 
 
 class TestDataAccessContactService:
     @staticmethod
     def test_list_contacts_for_irc_user():
-        contact_baker(_quantity=10)
-        cas_admin = user_baker({'app_role': AppRole.objects.get(role_name='cas_admin')})
-        assert ContactDataAccessService.get_all_contacts_for_user(cas_admin).count() == 10
+        baker.make_recipe('utils.contact', _quantity=10)
+        user = baker.make_recipe('utils.cas_admin')
+        assert ContactDataAccessService.get_all_contacts_for_user(user).count() == 10
 
     @staticmethod
     def test_list_contacts_for_industry_user():

bc_obps/service/tests/test_contact_service_v2.py

@@ -0,0 +1,33 @@
+from registration.schema.v1.contact import ContactFilterSchema
+import pytest
+from service.contact_service_v2 import ContactServiceV2
+from model_bakery import baker
+
+pytestmark = pytest.mark.django_db
+
+
+class TestListContactService:
+    @staticmethod
+    def test_list_contacts():
+
+        user = baker.make_recipe('utils.cas_admin')
+        contact1 = baker.make_recipe('utils.contact')
+        contact2 = baker.make_recipe('utils.contact')
+
+        operator1 = baker.make_recipe('utils.operator')
+
+        operator2a = baker.make_recipe('utils.operator')
+        operator2b = baker.make_recipe('utils.operator')
+
+        # contact 1 is associated with one operator, count = 1
+        operator1.contacts.set([contact1])
+
+        # contact 2 belongs to two operators count = 3
+        operator2a.contacts.set([contact2])
+        operator2b.contacts.set([contact2])
+        assert (
+            ContactServiceV2.list_contacts_v2(
+                user_guid=user.user_guid, sort_field="created_at", sort_order="desc", filters=ContactFilterSchema()
+            ).count()
+            == 3
+        )

bciers/apps/administration/app/components/contacts/ContactsDataGrid.tsx

@@ -46,13 +46,22 @@ const ContactsDataGrid = ({
     [SearchCell, isExternalUser],
   );
 
+  // the mui grid requires a unique id, and since we can multiple rows with the same contact for different operators, we can't use the contact's id alone
+  function getRowId(row: ContactRow) {
+    const operator = row?.operators__legal_name
+      ? row.operators__legal_name
+      : "";
+    return row.id + operator;
+  }
+
   return (
     <DataGrid
       columns={columns}
       columnGroupModel={columnGroup}
       fetchPageData={fetchContactsPageData}
       paginationMode="server"
       initialData={initialData}
+      getRowId={getRowId}
     />
   );
 };

bciers/apps/administration/app/components/contacts/types.ts

@@ -5,6 +5,7 @@ export interface ContactRow {
   first_name: string;
   last_name: string;
   email: string;
+  operators__legal_name?: string;
 }
 
 export interface ContactsSearchParams {
@@ -16,6 +17,7 @@ export interface ContactsSearchParams {
   sort_field?: string;
   sort_order?: string;
   operator_id?: UUID;
+  operators__legal_name?: string;
 }
 
 export interface ContactFormData {

bciers/apps/administration/app/components/datagrid/models/contacts/contactColumns.ts

@@ -9,22 +9,14 @@ const contactColumns = (
       field: "first_name",
       headerName: "First Name",
       // Set flex to 1 to make the column take up all the remaining width if user zooms out
-      width: 200,
+      flex: 1,
     },
-    { field: "last_name", headerName: "Last Name", width: 200 },
+    { field: "last_name", headerName: "Last Name", flex: 1 },
     { field: "email", headerName: "Business Email Address", flex: 1 },
-    // Two below fields don't exist in the data coming from the server(until we figure out how to get them)
     {
-      field: "operation_name",
-      headerName: "Operation Name",
-      sortable: false,
-      width: 200,
-    },
-    {
-      field: "operator_legal_name",
+      field: "operators__legal_name",
       headerName: "Operator Legal Name",
-      sortable: false,
-      width: 200,
+      flex: 1,
     },
     {
       field: "action",
@@ -37,7 +29,7 @@ const contactColumns = (
 
   if (isExternalUser) {
     // remove operator_legal_name and operation_name columns for external users
-    columns.splice(3, 2);
+    columns.splice(3, 1);
   }
 
   return columns;

bciers/apps/administration/app/components/datagrid/models/contacts/contactGroupColumns.ts

@@ -11,7 +11,7 @@ const contactGroupColumns = (
   const columnGroupModel: GridColumnGroupingModel = [
     {
       groupId: "first_name",
-      headerName: "Facility Name",
+      headerName: "First Name",
       renderHeaderGroup: SearchCell,
       children: [{ field: "first_name" }],
     },
@@ -28,16 +28,10 @@ const contactGroupColumns = (
       children: [{ field: "email" }],
     },
     {
-      groupId: "operator_legal_name",
+      groupId: "operators__legal_name",
       headerName: "Operator Legal Name",
-      renderHeaderGroup: EmptyGroupCell,
-      children: [{ field: "operator_legal_name" }],
-    },
-    {
-      groupId: "operation_name",
-      headerName: "Operation Name",
-      renderHeaderGroup: EmptyGroupCell,
-      children: [{ field: "operation_name" }],
+      renderHeaderGroup: SearchCell,
+      children: [{ field: "operators__legal_name" }],
     },
     {
       groupId: "action",

bciers/apps/administration/tests/components/contacts/ContactsDataGrid.test.tsx

@@ -17,7 +17,7 @@ useSearchParams.mockReturnValue({
   get: vi.fn(),
 } as QueryParams);
 
-const mockResponse = {
+const mockExternalResponse = {
   rows: [
     {
       id: 1,
@@ -35,13 +35,36 @@ const mockResponse = {
   row_count: 2,
 };
 
+const mockInternalResponse = {
+  rows: [
+    {
+      id: 1,
+      first_name: "John",
+      last_name: "Doe",
+      email: "john.doe@example.com",
+      operators__legal_name: "Legal name Doe",
+    },
+    {
+      id: 2,
+      first_name: "Jane",
+      last_name: "Smith",
+      email: "jane.smith@example.com",
+      operators__legal_name: "Legal name Smith",
+    },
+  ],
+  row_count: 2,
+};
+
 describe("ContactsDataGrid component", () => {
   beforeEach(async () => {
     vi.clearAllMocks();
   });
   it("renders the ContactsDataGrid grid for external users", async () => {
     render(
-      <ContactsDataGrid isExternalUser={true} initialData={mockResponse} />,
+      <ContactsDataGrid
+        isExternalUser={true}
+        initialData={mockExternalResponse}
+      />,
     );
 
     // correct headers
@@ -55,9 +78,6 @@ describe("ContactsDataGrid component", () => {
       screen.queryByRole("columnheader", { name: "Business Email Address" }),
     ).toBeVisible();
     // Internal users should only see two below columns
-    expect(
-      screen.queryByRole("columnheader", { name: "Operation Name" }),
-    ).not.toBeInTheDocument();
     expect(
       screen.queryByRole("columnheader", { name: "Operator Legal Name" }),
     ).not.toBeInTheDocument();
@@ -76,7 +96,10 @@ describe("ContactsDataGrid component", () => {
 
   it("renders the ContactsDataGrid grid for internal users", async () => {
     render(
-      <ContactsDataGrid isExternalUser={false} initialData={mockResponse} />,
+      <ContactsDataGrid
+        isExternalUser={false}
+        initialData={mockInternalResponse}
+      />,
     );
 
     // correct headers
@@ -89,20 +112,20 @@ describe("ContactsDataGrid component", () => {
     expect(
       screen.queryByRole("columnheader", { name: "Business Email Address" }),
     ).toBeVisible();
-    expect(
-      screen.queryByRole("columnheader", { name: "Operation Name" }),
-    ).toBeVisible();
     expect(
       screen.queryByRole("columnheader", { name: "Operator Legal Name" }),
     ).toBeVisible();
     expect(
       screen.queryByRole("columnheader", { name: "Actions" }),
     ).toBeVisible();
-    expect(screen.queryAllByPlaceholderText(/Search/i)).toHaveLength(3);
+    expect(screen.queryAllByPlaceholderText(/Search/i)).toHaveLength(4);
 
     // Check data displays
     expect(screen.getByText("john.doe@example.com")).toBeVisible();
+
+    expect(screen.getByText("Legal name Doe")).toBeVisible();
     expect(screen.getByText("jane.smith@example.com")).toBeVisible();
+    expect(screen.getByText("Legal name Smith")).toBeVisible();
     expect(screen.getAllByRole("link", { name: /View Details/i })).toHaveLength(
       2,
     );
@@ -113,7 +136,10 @@ describe("ContactsDataGrid component", () => {
 
   it("makes API call with correct params when sorting", async () => {
     render(
-      <ContactsDataGrid isExternalUser={true} initialData={mockResponse} />,
+      <ContactsDataGrid
+        isExternalUser={true}
+        initialData={mockExternalResponse}
+      />,
     );
 
     // click on the first column header
@@ -169,9 +195,13 @@ describe("ContactsDataGrid component", () => {
       extractParams(String(mockReplace.mock.calls[3][2]), "sort_order"),
     ).toBe("desc");
   });
+
   it("makes API call with correct params when filtering", async () => {
     render(
-      <ContactsDataGrid isExternalUser={true} initialData={mockResponse} />,
+      <ContactsDataGrid
+        isExternalUser={true}
+        initialData={mockInternalResponse}
+      />,
     );
 
     const searchInput = screen.getAllByPlaceholderText(/Search/i)[0]; // first name search input