From 403eda93a6c36f9d187ea1b0a1d6454e388ee90f Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 16 Jan 2025 13:12:05 -0800 Subject: [PATCH 01/13] chore: BE work on replacing operation rep --- .../api/v2/_contacts/contact_id.py | 5 +- .../api/v2/_operations/operation_id.py | 8 +++- bc_obps/registration/schema/v2/operation.py | 21 ++++++++- bc_obps/service/contact_service_v2.py | 46 +++++++++++++++++++ bc_obps/service/operation_service_v2.py | 37 ++++++++++----- 5 files changed, 101 insertions(+), 16 deletions(-) diff --git a/bc_obps/registration/api/v2/_contacts/contact_id.py b/bc_obps/registration/api/v2/_contacts/contact_id.py index b229c8e5f3..44a1fd138b 100644 --- a/bc_obps/registration/api/v2/_contacts/contact_id.py +++ b/bc_obps/registration/api/v2/_contacts/contact_id.py @@ -4,11 +4,12 @@ from registration.constants import CONTACT_TAGS from registration.models.contact import Contact from registration.schema.v1.contact import ContactIn, ContactOut -from service.contact_service import ContactService, ContactWithPlacesAssigned from common.api.utils import get_current_user_guid from registration.decorators import handle_http_errors from registration.api.router import router from registration.schema.generic import Message +from service.contact_service_v2 import ContactServiceV2, ContactWithPlacesAssigned +from service.contact_service import ContactService from service.error_service.custom_codes_4xx import custom_codes_4xx @@ -23,7 +24,7 @@ ) @handle_http_errors() def get_contact(request: HttpRequest, contact_id: int) -> Tuple[Literal[200], Optional[ContactWithPlacesAssigned]]: - return 200, ContactService.get_with_places_assigned(get_current_user_guid(request), contact_id) + return 200, ContactServiceV2.get_with_places_assigned_v2(get_current_user_guid(request), contact_id) @router.put( diff --git a/bc_obps/registration/api/v2/_operations/operation_id.py b/bc_obps/registration/api/v2/_operations/operation_id.py index 4329d45f53..964f97219f 100644 --- a/bc_obps/registration/api/v2/_operations/operation_id.py +++ b/bc_obps/registration/api/v2/_operations/operation_id.py @@ -1,6 +1,10 @@ from typing import Literal, Tuple from uuid import UUID -from registration.schema.v2.operation import OperationOutV2, OperationInformationIn, OperationOutWithDocuments +from registration.schema.v2.operation import ( + OperationInformationInUpdate, + OperationOutV2, + OperationOutWithDocuments, +) from common.permissions import authorize from django.http import HttpRequest from registration.constants import OPERATION_TAGS @@ -57,6 +61,6 @@ def get_operation_with_documents(request: HttpRequest, operation_id: UUID) -> Tu ) @handle_http_errors() def update_operation( - request: HttpRequest, operation_id: UUID, payload: OperationInformationIn + request: HttpRequest, operation_id: UUID, payload: OperationInformationInUpdate ) -> Tuple[Literal[200], Operation]: return 200, OperationServiceV2.update_operation(get_current_user_guid(request), payload, operation_id) diff --git a/bc_obps/registration/schema/v2/operation.py b/bc_obps/registration/schema/v2/operation.py index 92d07f420d..c5576e1cf4 100644 --- a/bc_obps/registration/schema/v2/operation.py +++ b/bc_obps/registration/schema/v2/operation.py @@ -112,6 +112,10 @@ class Meta: fields = ["name", 'type'] +class OperationInformationInUpdate(OperationInformationIn): + operation_representatives: List[int] + + class OptedInOperationDetailOut(ModelSchema): class Meta: model = OptedInOperationDetail @@ -153,6 +157,11 @@ class OperationOutV2(ModelSchema): date_of_first_shipment: Optional[str] = None new_entrant_application: Optional[str] = None bcghg_id: Optional[str] = Field(None, alias="bcghg_id.id") + operation_representatives: Optional[List[int]] = [] + + @staticmethod + def resolve_operation_representatives(obj: Operation) -> List[int]: + return list(obj.contacts.filter(business_role='Operation Representative').values_list('id', flat=True)) @staticmethod def resolve_multiple_operators_array(obj: Operation) -> List[MultipleOperator]: @@ -176,7 +185,17 @@ def resolve_operator(obj: Operation, context: DictStrAny) -> Optional[Operator]: class Meta: model = Operation - fields = ["id", 'name', 'type', 'opt_in', 'regulated_products', 'status', 'activities', 'opted_in_operation'] + fields = [ + "id", + 'name', + 'type', + 'opt_in', + 'regulated_products', + 'status', + 'activities', + 'opted_in_operation', + 'contacts', + ] from_attributes = True diff --git a/bc_obps/service/contact_service_v2.py b/bc_obps/service/contact_service_v2.py index 391efc2ac0..4cda2478f0 100644 --- a/bc_obps/service/contact_service_v2.py +++ b/bc_obps/service/contact_service_v2.py @@ -8,6 +8,25 @@ from service.data_access_service.contact_service import ContactDataAccessService from service.data_access_service.user_service import UserDataAccessService from ninja import Query +from uuid import UUID + + +from registration.constants import UNAUTHORIZED_MESSAGE +from registration.models.contact import Contact +from registration.schema.v1.contact import ContactOut +from service.data_access_service.contact_service import ContactDataAccessService +from service.data_access_service.user_service import UserDataAccessService +from ninja import Schema + + +class PlacesAssigned(Schema): + role_name: str + operation_name: str + operation_id: UUID + + +class ContactWithPlacesAssigned(ContactOut): + places_assigned: Optional[list[PlacesAssigned]] class ContactServiceV2: @@ -31,3 +50,30 @@ def list_contacts_v2( .distinct() ) return cast(QuerySet[Contact], queryset) + + def get_if_authorized_v2(cls, user_guid: UUID, contact_id: int) -> Optional[Contact]: + user = UserDataAccessService.get_by_guid(user_guid) + user_contacts = ContactDataAccessService.get_all_contacts_for_user(user) + contact = user_contacts.filter(id=contact_id).first() + if user.is_industry_user() and not contact: + raise Exception(UNAUTHORIZED_MESSAGE) + return contact + + @classmethod + def get_with_places_assigned_v2(cls, user_guid: UUID, contact_id: int) -> Optional[ContactWithPlacesAssigned]: + contact = cls.get_if_authorized_v2(user_guid, contact_id) + places_assigned = [] + if contact: + role_name = contact.business_role.role_name + for operation in contact.operations_contacts.all(): + place = PlacesAssigned( + role_name=role_name, + operation_name=operation.name, + operation_id=operation.id, + ) + places_assigned.append(place) + result = cast(ContactWithPlacesAssigned, contact) + if places_assigned: + result.places_assigned = places_assigned + return result + return None diff --git a/bc_obps/service/operation_service_v2.py b/bc_obps/service/operation_service_v2.py index 8a96f70d3d..468e6c2e65 100644 --- a/bc_obps/service/operation_service_v2.py +++ b/bc_obps/service/operation_service_v2.py @@ -1,4 +1,4 @@ -from typing import Optional, Tuple, Callable, Generator +from typing import Optional, Tuple, Callable, Generator, Union from django.db.models import QuerySet from registration.schema.v2.operation_timeline import OperationTimelineFilterSchema from service.data_access_service.operation_designated_operator_timeline_service import ( @@ -29,6 +29,7 @@ from service.operation_service import OperationService from registration.schema.v2.operation import ( OperationInformationIn, + OperationInformationInUpdate, OperationRepresentativeRemove, OptedInOperationDetailIn, OperationNewEntrantApplicationIn, @@ -194,7 +195,7 @@ def remove_opted_in_operation_detail(cls, user_guid: UUID, operation_id: UUID) - def create_or_update_operation_v2( cls, user_guid: UUID, - payload: OperationInformationIn, + payload: Union[OperationInformationIn, OperationInformationInUpdate], operation_id: UUID | None = None, ) -> Operation: user_operator: UserOperator = UserDataAccessService.get_user_operator_by_user(user_guid) @@ -224,9 +225,29 @@ def create_or_update_operation_v2( # set m2m relationships operation.activities.set(payload.activities) if payload.activities else operation.activities.clear() - operation.regulated_products.set( - payload.regulated_products - ) if payload.regulated_products else operation.regulated_products.clear() + + ( + operation.regulated_products.set(payload.regulated_products) + if payload.regulated_products + else operation.regulated_products.clear() + ) + + if isinstance(payload, OperationInformationInUpdate): + for contact_id in payload.operation_representatives: + contact = Contact.objects.get(id=contact_id) + address = contact.address + if ( + not address + or not address.street_address + or not address.municipality + or not address.province + or not address.postal_code + ): + raise Exception( + f'The contact {contact.first_name} {contact.last_name} is missing address information. Please return to Contacts and fill in their address information before assigning them as an Operation Representative here.' + ) + + operation.contacts.set(payload.operation_representatives) # create or replace documents operation_documents = [ @@ -369,12 +390,6 @@ def update_operation( payload, operation_id, ) - - if payload.regulated_products: - # We should add a conditional to check registration_purpose type here - # At the time of implementation there are some changes to registration_purpose coming from the business area - operation.regulated_products.set(payload.regulated_products) - operation.set_create_or_update(user_guid) return operation @classmethod From d2ae65eacc089ad3770830d54fc9a19b04c3bd19 Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 16 Jan 2025 13:13:06 -0800 Subject: [PATCH 02/13] test: pytests for removing operation rep --- .../endpoints/v2/_contacts/test_contact_id.py | 10 ++- .../v2/_operations/test_operation_id.py | 2 + .../registration/tests/utils/baker_recipes.py | 6 +- .../service/tests/test_contact_service_v2.py | 35 +++++++++ .../tests/test_operation_service_v2.py | 75 +++++++++++++++++++ 5 files changed, 125 insertions(+), 3 deletions(-) diff --git a/bc_obps/registration/tests/endpoints/v2/_contacts/test_contact_id.py b/bc_obps/registration/tests/endpoints/v2/_contacts/test_contact_id.py index ad2cc52c2f..5833b94a0c 100644 --- a/bc_obps/registration/tests/endpoints/v2/_contacts/test_contact_id.py +++ b/bc_obps/registration/tests/endpoints/v2/_contacts/test_contact_id.py @@ -36,7 +36,7 @@ def test_industry_users_can_get_contacts_associated_with_their_operator(self): response = TestUtils.mock_get_with_auth_role( self, - endpoint=custom_reverse_lazy("v1_get_contact", kwargs={"contact_id": contact.id}), + endpoint=custom_reverse_lazy("get_contact", kwargs={"contact_id": contact.id}), role_name="industry_user", ) assert response.status_code == 200 @@ -44,7 +44,13 @@ def test_industry_users_can_get_contacts_associated_with_their_operator(self): assert response_json.get('first_name') == contact.first_name assert response_json.get('last_name') == contact.last_name assert response_json.get('email') == contact.email - assert response_json.get('places_assigned') == [f"Operation Representative - {operation.name}"] + assert response_json.get('places_assigned') == [ + { + 'role_name': 'Operation Representative', + 'operation_name': operation.name, + 'operation_id': str(operation.id), + } + ] def test_industry_users_cannot_get_contacts_not_associated_with_their_operator(self): contact = contact_baker() diff --git a/bc_obps/registration/tests/endpoints/v2/_operations/test_operation_id.py b/bc_obps/registration/tests/endpoints/v2/_operations/test_operation_id.py index c1e835ee02..fe1315459d 100644 --- a/bc_obps/registration/tests/endpoints/v2/_operations/test_operation_id.py +++ b/bc_obps/registration/tests/endpoints/v2/_operations/test_operation_id.py @@ -107,6 +107,8 @@ def test_operations_with_documents_endpoint_get_success(self): def test_operations_endpoint_put_success(self): approved_user_operator = baker.make_recipe('utils.approved_user_operator', user=self.user) operation = baker.make_recipe('utils.operation', operator=approved_user_operator.operator) + contact = baker.make_recipe('utils.contact') + self.test_payload["operation_representatives"] = [contact.id] response = TestUtils.mock_put_with_auth_role( self, "industry_user", diff --git a/bc_obps/registration/tests/utils/baker_recipes.py b/bc_obps/registration/tests/utils/baker_recipes.py index 5776cbf4e3..3bbc6038ad 100644 --- a/bc_obps/registration/tests/utils/baker_recipes.py +++ b/bc_obps/registration/tests/utils/baker_recipes.py @@ -128,7 +128,11 @@ meets_notification_to_director_on_criteria_change=False, ) contact = Recipe( - Contact, business_role=BusinessRole.objects.first(), address=foreign_key(address), first_name=seq('Firstname 0') + Contact, + business_role=BusinessRole.objects.first(), + address=foreign_key(address), + first_name=seq('Firstname 0'), + last_name=seq('Lastname 0'), ) diff --git a/bc_obps/service/tests/test_contact_service_v2.py b/bc_obps/service/tests/test_contact_service_v2.py index bfd1bdf340..51af9ff1f3 100644 --- a/bc_obps/service/tests/test_contact_service_v2.py +++ b/bc_obps/service/tests/test_contact_service_v2.py @@ -2,6 +2,10 @@ import pytest from service.contact_service_v2 import ContactServiceV2 from model_bakery import baker +from registration.models.business_role import BusinessRole +import pytest +from model_bakery import baker +from service.contact_service_v2 import ContactServiceV2 pytestmark = pytest.mark.django_db @@ -31,3 +35,34 @@ def test_list_contacts(): ).count() == 3 ) + + +class TestContactService: + @staticmethod + def test_get_with_places_assigned_with_contacts(): + contact = baker.make_recipe( + 'utils.contact', business_role=BusinessRole.objects.get(role_name='Operation Representative') + ) + approved_user_operator = baker.make_recipe('utils.approved_user_operator') + # add contact to operator (they have to be associated with the operator or will throw unauthorized) + approved_user_operator.operator.contacts.set([contact]) + # add contact to operation + operation = baker.make_recipe('utils.operation', operator=approved_user_operator.operator) + operation.contacts.set([contact]) + + result = ContactServiceV2.get_with_places_assigned_v2(approved_user_operator.user.user_guid, contact.id) + assert result.places_assigned == [ + f"Operation Representative - {operation.name}", + ] + + @staticmethod + def test_get_with_places_assigned_with_no_contacts(): + contact = baker.make_recipe( + 'utils.contact', business_role=BusinessRole.objects.get(role_name='Operation Representative') + ) + approved_user_operator = baker.make_recipe('utils.approved_user_operator') + # add contact to operator (they have to be associated with the operator or will throw unauthorized) + approved_user_operator.operator.contacts.set([contact]) + + result = ContactServiceV2.get_with_places_assigned_v2(approved_user_operator.user.user_guid, contact.id) + assert not hasattr(result, 'places_assigned') diff --git a/bc_obps/service/tests/test_operation_service_v2.py b/bc_obps/service/tests/test_operation_service_v2.py index 6a972d014d..d16a4facae 100644 --- a/bc_obps/service/tests/test_operation_service_v2.py +++ b/bc_obps/service/tests/test_operation_service_v2.py @@ -16,6 +16,7 @@ from registration.constants import UNAUTHORIZED_MESSAGE from registration.models.address import Address from registration.schema.v2.operation import ( + OperationInformationInUpdate, OperationRepresentativeIn, OperationNewEntrantApplicationIn, OperationRepresentativeRemove, @@ -618,6 +619,80 @@ def test_update_operation_archive_multiple_operators(): assert operation.updated_by == approved_user_operator.user assert operation.updated_at is not None + @staticmethod + def test_update_operation_with_operation_representatives_with_address(): + approved_user_operator = baker.make_recipe('utils.approved_user_operator') + existing_operation = baker.make_recipe('utils.operation') + contacts = baker.make_recipe( + 'utils.contact', business_role=BusinessRole.objects.get(role_name='Operation Representative'), _quantity=3 + ) + + payload = OperationInformationInUpdate( + registration_purpose='Reporting Operation', + regulated_products=[1], + name="I am updated", + type="SFO", + naics_code_id=1, + secondary_naics_code_id=2, + tertiary_naics_code_id=3, + activities=[1], + process_flow_diagram=MOCK_DATA_URL, + boundary_map=MOCK_DATA_URL, + operation_representatives=[contact.id for contact in contacts], + ) + operation = OperationServiceV2.create_or_update_operation_v2( + approved_user_operator.user.user_guid, + payload, + existing_operation.id, + ) + operation.refresh_from_db() + assert Operation.objects.count() == 1 + assert operation.contacts.count() == 3 + assert operation.updated_by == approved_user_operator.user + assert operation.updated_at is not None + + @staticmethod + def test_update_operation_with_operation_representative_without_address(): + approved_user_operator = baker.make_recipe('utils.approved_user_operator') + existing_operation = baker.make_recipe('utils.operation') + # create contacts with incomplete address data + contacts = baker.make_recipe( + 'utils.contact', business_role=BusinessRole.objects.get(role_name='Operation Representative'), _quantity=5 + ) + contacts[0].address = None + contacts[0].save() + contacts[1].address.street_address = None + contacts[1].address.save() + contacts[2].address.municipality = None + contacts[2].address.save() + contacts[3].address.province = None + contacts[3].address.save() + contacts[4].address.postal_code = None + contacts[4].address.save() + for contact in contacts: + payload = OperationInformationInUpdate( + registration_purpose='Reporting Operation', + regulated_products=[1], + name="I am updated", + type="SFO", + naics_code_id=1, + secondary_naics_code_id=2, + tertiary_naics_code_id=3, + activities=[1], + process_flow_diagram=MOCK_DATA_URL, + boundary_map=MOCK_DATA_URL, + operation_representatives=[contact.id], + ) + with pytest.raises( + Exception, + match=f'The contact "{contact.first_name}" "{contact.last_name}"is missing address information. Please return to Contacts and fill in their address information before assigning them as an Operation Representative here.', + ): + OperationServiceV2.create_or_update_operation_v2( + approved_user_operator.user.user_guid, + payload, + existing_operation.id, + ) + class TestOperationServiceV2UpdateOperation: def test_update_operation(self): From 91caeaa980fc32db8c12ba99d18d240581f60704 Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 16 Jan 2025 13:13:54 -0800 Subject: [PATCH 03/13] chore: update rjsf schemas --- .../app/data/jsonSchema/contact.ts | 34 ++++++++++++++----- .../administrationOperationInformation.ts | 1 + .../administrationRegistrationInformation.ts | 27 +++++++++++++-- 3 files changed, 50 insertions(+), 12 deletions(-) diff --git a/bciers/apps/administration/app/data/jsonSchema/contact.ts b/bciers/apps/administration/app/data/jsonSchema/contact.ts index 61ea7ba5d1..7a1c659ff6 100644 --- a/bciers/apps/administration/app/data/jsonSchema/contact.ts +++ b/bciers/apps/administration/app/data/jsonSchema/contact.ts @@ -1,7 +1,7 @@ import { RJSFSchema } from "@rjsf/utils"; import provinceOptions from "@bciers/data/provinces.json"; import SectionFieldTemplate from "@bciers/components/form/fields/SectionFieldTemplate"; -import { ArrayFieldTemplate } from "@bciers/components/form/fields"; +import { PlacesAssignedFieldTemplate } from "@bciers/components/form/fields"; const section1: RJSFSchema = { type: "object", @@ -18,11 +18,15 @@ const section1: RJSFSchema = { }, places_assigned: { type: "array", - default: ["None"], title: "Places assigned", readOnly: true, items: { - type: "string", + type: "object", + properties: { + role_name: { type: "string" }, + operation_name: { type: "string" }, + operation_id: { type: "string" }, + }, }, }, }, @@ -104,15 +108,27 @@ export const contactsUiSchema = { "ui:FieldTemplate": SectionFieldTemplate, "ui:order": ["selected_user", "first_name", "last_name", "places_assigned"], places_assigned: { - "ui:ArrayFieldTemplate": ArrayFieldTemplate, - "ui:options": { - note: "You cannot delete this contact unless you replace them with other contact(s) in the place(s) above.", - addable: false, - removable: false, - }, + "ui:ArrayFieldTemplate": PlacesAssignedFieldTemplate, "ui:classNames": "[&>div:last-child]:w-2/3", items: { "ui:widget": "ReadOnlyWidget", + "ui:options": { + label: false, + inline: true, + }, + role_name: { + "ui:options": { + label: false, + }, + }, + operation_name: { + "ui:options": { + label: false, + }, + }, + operation_id: { + "ui:widget": "hidden", + }, }, }, }, diff --git a/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationOperationInformation.ts b/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationOperationInformation.ts index 626968cd85..a005f49513 100644 --- a/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationOperationInformation.ts +++ b/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationOperationInformation.ts @@ -45,6 +45,7 @@ export const administrationOperationInformationUiSchema: UiSchema = { section3: { ...registrationInformationUiSchema, "ui:order": [ + "operation_representatives", "registration_purpose", "regulated_operation_preface", "regulated_products", diff --git a/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts b/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts index 50a6c522ac..a97239f3a5 100644 --- a/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts +++ b/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts @@ -1,7 +1,7 @@ import SectionFieldTemplate from "@bciers/components/form/fields/SectionFieldTemplate"; import { TitleOnlyFieldTemplate } from "@bciers/components/form/fields"; import { RJSFSchema, UiSchema } from "@rjsf/utils"; -import { getRegulatedProducts } from "@bciers/actions/api"; +import { getRegulatedProducts, getContacts } from "@bciers/actions/api"; import { RegistrationPurposes } from "apps/registration/app/components/operations/registration/enums"; export const createAdministrationRegistrationInformationSchema = async ( @@ -10,7 +10,8 @@ export const createAdministrationRegistrationInformationSchema = async ( // fetch db values that are dropdown options const regulatedProducts: { id: number; name: string }[] = await getRegulatedProducts(); - + const contacts: { id: number; first_name: string; last_name: string }[] = + await getContacts(); const isRegulatedProducts = registrationPurposeValue === RegistrationPurposes.OBPS_REGULATED_OPERATION.valueOf(); @@ -23,7 +24,10 @@ export const createAdministrationRegistrationInformationSchema = async ( const registrationInformationSchema: RJSFSchema = { title: "Registration Information", type: "object", - required: isRegulatedProducts ? ["regulated_products"] : [], + required: [ + "operation_representatives", + ...(isRegulatedProducts ? ["regulated_products"] : []), + ], properties: { registration_purpose: { type: "string", @@ -47,6 +51,19 @@ export const createAdministrationRegistrationInformationSchema = async ( }, }, }), + operation_representatives: { + title: "Operation Representative(s)", + type: "array", + minItems: 1, + items: { + enum: contacts.items.map((contact) => contact.id), + // Ts-ignore until we refactor enumNames https://github.com/bcgov/cas-registration/issues/2176 + // @ts-ignore + enumNames: contacts.items.map( + (contact) => `${contact.first_name} ${contact.last_name}`, + ), + }, + }, ...(isOptIn && { opted_in_preface: { // Not an actual field, just used to display a message @@ -114,6 +131,10 @@ export const registrationInformationUiSchema: UiSchema = { "new_entrant_application", ], "ui:FieldTemplate": SectionFieldTemplate, + // brianna why doesn't error show, looks exactly the same as regulated_products + operation_representatives: { + "ui:widget": "MultiSelectWidget", + }, regulated_operation_preface: { "ui:classNames": "text-bc-bg-blue text-lg", "ui:FieldTemplate": TitleOnlyFieldTemplate, From 06bfca7bad54917aa16c64db5f455506ca470a29 Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 16 Jan 2025 13:14:33 -0800 Subject: [PATCH 04/13] chore: create PlacesAssignedFieldTemplate --- .../fields/PlacesAssignedFieldTemplate.tsx | 38 +++++++++++++++++++ .../libs/components/src/form/fields/index.ts | 1 + 2 files changed, 39 insertions(+) create mode 100644 bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.tsx diff --git a/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.tsx b/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.tsx new file mode 100644 index 0000000000..9519c40934 --- /dev/null +++ b/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.tsx @@ -0,0 +1,38 @@ +import { ArrayFieldTemplateProps } from "@rjsf/utils"; +import Link from "next/link"; + +const PlacesAssignedFieldTemplate = ({ items }: ArrayFieldTemplateProps) => { + if (items.length < 1) { + return
None
; + } + return ( +
+ {items?.map((item) => { + const formData = item.children.props.formData; + if ( + !formData.role_name || + !formData.operation_name || + !formData.operation_id + ) { + throw new Error(`Invalid places assigned data`); + } + return ( +
+ {formData.role_name} -{" "} + + {formData.operation_name} + +
+ ); + })} +
+ Note: You cannot delete this contact unless you replace them with + other contact(s) in the place(s) above. +
+
+ ); +}; + +export default PlacesAssignedFieldTemplate; diff --git a/bciers/libs/components/src/form/fields/index.ts b/bciers/libs/components/src/form/fields/index.ts index e7955882a6..2945b5e4ae 100644 --- a/bciers/libs/components/src/form/fields/index.ts +++ b/bciers/libs/components/src/form/fields/index.ts @@ -1,5 +1,6 @@ export { default as ArrayFieldTemplate } from "./ArrayFieldTemplate"; export { default as InlineArrayFieldTemplate } from "./InlineArrayFieldTemplate"; +export { default as PlacesAssignedFieldTemplate } from "./PlacesAssignedFieldTemplate"; export { default as FieldTemplate } from "./FieldTemplate"; export { default as FieldTemplateWithSubmitButton } from "./FieldTemplateWithSubmitButton"; From 23663695c38c44e3438f5aedad4d1a75292f7e6d Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 16 Jan 2025 13:14:56 -0800 Subject: [PATCH 05/13] feat: replace operation rep --- .../operations/OperationInformationForm.tsx | 58 +++++++++++-------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx b/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx index d8de303135..fc973a1ff7 100644 --- a/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx +++ b/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx @@ -1,7 +1,7 @@ "use client"; import { useState } from "react"; -import { useRouter } from "next/navigation"; +import { useRouter, useSearchParams } from "next/navigation"; import { UUID } from "crypto"; import SingleStepTaskListForm from "@bciers/components/form/SingleStepTaskListForm"; @@ -18,6 +18,7 @@ import { } from "apps/registration/app/components/operations/registration/enums"; import { FormMode, FrontEndRoles } from "@bciers/utils/src/enums"; import { useSessionRole } from "@bciers/utils/src/sessionUtils"; +import Note from "@bciers/components/layout/Note"; const OperationInformationForm = ({ formData, @@ -29,11 +30,11 @@ const OperationInformationForm = ({ schema: RJSFSchema; }) => { const [error, setError] = useState(undefined); - const router = useRouter(); - // To get the user's role from the session const role = useSessionRole(); + const searchParams = useSearchParams(); + const isRedirectedFromContacts = searchParams.get("from_contacts") as string; const handleSubmit = async (data: { formData?: OperationInformationFormData; @@ -67,29 +68,36 @@ const OperationInformationForm = ({ return { error: response2.error }; } }; - return ( - router.push("/operations")} - formContext={{ - operationId, - isRegulatedOperation: regulatedOperationPurposes.includes( - formData.registration_purpose as RegistrationPurposes, - ), - isCasDirector: role === FrontEndRoles.CAS_DIRECTOR, - isEio: formData.registration_purpose?.match( - RegistrationPurposes.ELECTRICITY_IMPORT_OPERATION.valueOf(), - ), - status: formData.status, - }} - /> + <> + {isRedirectedFromContacts && ( + + To remove the current operation representative, please select a new + contact to replace them. + + )} + router.push("/operations")} + formContext={{ + operationId, + isRegulatedOperation: regulatedOperationPurposes.includes( + formData.registration_purpose as RegistrationPurposes, + ), + isCasDirector: role === FrontEndRoles.CAS_DIRECTOR, + isEio: formData.registration_purpose?.match( + RegistrationPurposes.ELECTRICITY_IMPORT_OPERATION.valueOf(), + ), + status: formData.status, + }} + /> + ); }; From bc4cdb6ece1d6e06c3c6ba54a140423b5e312184 Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 16 Jan 2025 13:15:20 -0800 Subject: [PATCH 06/13] test: vitests for replacing operation rep --- .../components/contacts/ContactForm.test.tsx | 24 ++- .../OperationInformationForm.test.tsx | 182 +++++++++++++++++- .../OperationInformationPage.test.tsx | 5 +- .../tests/components/operations/mocks.ts | 5 + .../PlacesAssignedFieldTemplate.test.tsx | 101 ++++++++++ 5 files changed, 308 insertions(+), 9 deletions(-) create mode 100644 bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.test.tsx diff --git a/bciers/apps/administration/tests/components/contacts/ContactForm.test.tsx b/bciers/apps/administration/tests/components/contacts/ContactForm.test.tsx index bebecb1f9e..6674d140a2 100644 --- a/bciers/apps/administration/tests/components/contacts/ContactForm.test.tsx +++ b/bciers/apps/administration/tests/components/contacts/ContactForm.test.tsx @@ -26,7 +26,13 @@ const contactFormData = { municipality: "Cityville", province: "ON", postal_code: "A1B 2C3", - places_assigned: ["Operation Representative - Operation 1"], + places_assigned: [ + { + role_name: "Operation Representative", + operation_name: "Operation 1", + operation_id: "c0743c09-82fa-4186-91aa-4b5412e3415c", + }, + ], }; export const checkEmptyContactForm = () => { @@ -141,9 +147,11 @@ describe("ContactForm component", () => { ).toHaveTextContent("Doe"); expect(screen.getByText(/Places Assigned/i)).toBeVisible(); - expect( - screen.getByText(/Operation Representative - Operation 1/i), - ).toBeVisible(); + expect(screen.getByText(/Operation Representative/i)).toBeVisible(); + expect(screen.getByRole("link")).toHaveAttribute( + "href", + "/operations/c0743c09-82fa-4186-91aa-4b5412e3415c?operations_title=Operation 1&from_contacts=true", + ); expect( container.querySelector("#root_section2_position_title"), @@ -407,7 +415,13 @@ describe("ContactForm component", () => { body: JSON.stringify({ first_name: "John updated", last_name: "Doe updated", - places_assigned: ["Operation Representative - Operation 1"], + places_assigned: [ + { + role_name: "Operation Representative", + operation_name: "Operation 1", + operation_id: "c0743c09-82fa-4186-91aa-4b5412e3415c", + }, + ], position_title: "Senior Officer", email: "john.doe@example.com", phone_number: "+16044011234", diff --git a/bciers/apps/administration/tests/components/operations/OperationInformationForm.test.tsx b/bciers/apps/administration/tests/components/operations/OperationInformationForm.test.tsx index 112b4b17dc..f168fbad90 100644 --- a/bciers/apps/administration/tests/components/operations/OperationInformationForm.test.tsx +++ b/bciers/apps/administration/tests/components/operations/OperationInformationForm.test.tsx @@ -1,13 +1,18 @@ import { act, fireEvent, render, screen, within } from "@testing-library/react"; import { RJSFSchema } from "@rjsf/utils"; import OperationInformationForm from "@/administration/app/components/operations/OperationInformationForm"; -import { actionHandler, useSession } from "@bciers/testConfig/mocks"; +import { + actionHandler, + useSearchParams, + useSession, +} from "@bciers/testConfig/mocks"; import { getBusinessStructures, getNaicsCodes, getOperationWithDocuments, getRegulatedProducts, getReportingActivities, + getContacts, } from "./mocks"; import { createAdministrationOperationInformationSchema } from "apps/administration/app/data/jsonSchema/operationInformation/administrationOperationInformation"; import { FrontEndRoles, OperationStatus } from "@bciers/utils/src/enums"; @@ -22,6 +27,10 @@ useSession.mockReturnValue({ }, }, }); +useSearchParams.mockReturnValue({ + get: vi.fn(), +}); + const mockDataUri = "data:application/pdf;name=testpdf.pdf;base64,ZHVtbXk="; export const fetchFormEnums = () => { @@ -64,6 +73,25 @@ export const fetchFormEnums = () => { { id: 2, name: "Cement equivalent" }, ]); + // Contacts + getContacts.mockResolvedValue({ + items: [ + { + id: 1, + first_name: "Ivy", + last_name: "Jones", + email: "ivy.jones@example.com", + }, + { + id: 2, + first_name: "Jack", + last_name: "King", + email: "jack.king@example.com", + }, + ], + count: 2, + }); + // Registration purposes actionHandler.mockResolvedValue(["Potential Reporting Operation"]); }; @@ -162,7 +190,7 @@ const formData = { naics_code_id: 1, secondary_naics_code_id: 2, operation_has_multiple_operators: true, - activities: [1, 2, 3, 4, 5], + activities: [1, 2], multiple_operators_array: [ { mo_is_extraprovincial_company: false, @@ -178,7 +206,7 @@ const formData = { }, ], registration_purpose: "Reporting Operation", - regulated_products: [6], + regulated_products: [2], opt_in: false, }; @@ -801,4 +829,152 @@ describe("the OperationInformationForm component", () => { }, ); }); + + it("should not allow external users to remove their operation rep", async () => { + useSession.mockReturnValue({ + data: { + user: { + app_role: "industry_user_admin", + }, + }, + }); + + fetchFormEnums(); + const createdFormSchema = + await createAdministrationOperationInformationSchema( + formData.registration_purpose, + OperationStatus.REGISTERED, + ); + + render( + , + ); + await userEvent.click(screen.getByRole("button", { name: "Edit" })); + const cancelChipIcon = screen.getAllByTestId("CancelIcon"); + await userEvent.click(cancelChipIcon[2]); // 0-1 are activities + expect(screen.queryByText(/ivy/i)).not.toBeInTheDocument(); + + const submitButton = screen.getByRole("button", { + name: "Submit", + }); + await userEvent.click(submitButton); + expect(actionHandler).toHaveBeenCalledTimes(0); + expect(screen.getByText(/Must not have fewer than 1 items/i)).toBeVisible(); + }); + + it("should allow external users to replace their operation rep", async () => { + const testFormData = { + name: "Operation 3", + type: "Single Facility Operation", + naics_code_id: 1, + secondary_naics_code_id: 2, + operation_has_multiple_operators: false, + activities: [1, 2], + registration_purpose: "Reporting Operation", + regulated_products: [1], + opt_in: false, + operation_representatives: [1], + boundary_map: mockDataUri, + process_flow_diagram: mockDataUri, + }; + useSession.mockReturnValue({ + data: { + user: { + app_role: "industry_user_admin", + }, + }, + }); + + fetchFormEnums(); + const createdFormSchema = + await createAdministrationOperationInformationSchema( + testFormData.registration_purpose, + OperationStatus.REGISTERED, + ); + + render( + , + ); + await userEvent.click(screen.getByRole("button", { name: "Edit" })); + const cancelChipIcon = screen.getAllByTestId("CancelIcon"); + await userEvent.click(cancelChipIcon[2]); // 0-1 are activities + expect(screen.queryByText(/ivy/i)).not.toBeInTheDocument(); + const operationRepresentativesComboBoxInput = screen.getByRole("combobox", { + name: /Operation Representative(s)*/i, + }); + const openOperationReps = operationRepresentativesComboBoxInput + .parentElement?.children[1]?.children[0] as HTMLInputElement; + await userEvent.click(openOperationReps); + await userEvent.type( + operationRepresentativesComboBoxInput, + "Jack King{enter}", + ); + + const submitButton = screen.getByRole("button", { + name: "Submit", + }); + await userEvent.click(submitButton); + expect(actionHandler).toHaveBeenCalledTimes(1); + expect(actionHandler).toHaveBeenCalledWith( + `registration/operations/${operationId}`, + "PUT", + "", + { + body: JSON.stringify({ + name: "Operation 3", + type: "Single Facility Operation", + naics_code_id: 1, + secondary_naics_code_id: 2, + activities: [1, 2], + process_flow_diagram: mockDataUri, + boundary_map: mockDataUri, + operation_has_multiple_operators: false, + registration_purpose: "Reporting Operation", + operation_representatives: [2], + }), + }, + ); + }); + + it("should show a note if user navigated to operation from the contacts form", async () => { + useSession.mockReturnValue({ + data: { + user: { + app_role: "industry_user_admin", + }, + }, + }); + const mockGet = vi.fn(); + useSearchParams.mockReturnValue({ + get: mockGet, + }); + mockGet.mockReturnValue("true"); + fetchFormEnums(); + const createdFormSchema = + await createAdministrationOperationInformationSchema( + formData.registration_purpose, + OperationStatus.REGISTERED, + ); + + render( + , + ); + expect( + screen.getByText( + /To remove the current operation representative, please select a new contact to replace them./i, + ), + ).toBeVisible(); + }); }); diff --git a/bciers/apps/administration/tests/components/operations/OperationInformationPage.test.tsx b/bciers/apps/administration/tests/components/operations/OperationInformationPage.test.tsx index e02e046998..2796bc0de3 100644 --- a/bciers/apps/administration/tests/components/operations/OperationInformationPage.test.tsx +++ b/bciers/apps/administration/tests/components/operations/OperationInformationPage.test.tsx @@ -1,7 +1,7 @@ import { render, screen } from "@testing-library/react"; import OperationInformationPage from "@/administration/app/components/operations/OperationInformationPage"; import { getOperationWithDocuments } from "./mocks"; -import { useSession } from "@bciers/testConfig/mocks"; +import { useSearchParams, useSession } from "@bciers/testConfig/mocks"; import { fetchFormEnums } from "./OperationInformationForm.test"; import { beforeAll } from "vitest"; import { OperationStatus } from "@bciers/utils/src/enums"; @@ -46,6 +46,9 @@ describe("the OperationInformationPage component", () => { }, }, }); + useSearchParams.mockReturnValue({ + get: vi.fn(), + }); }); it("renders the OperationInformationPage component without Registration Information", async () => { fetchFormEnums(); diff --git a/bciers/apps/administration/tests/components/operations/mocks.ts b/bciers/apps/administration/tests/components/operations/mocks.ts index f6f1e45ab6..e3ec3ad6c7 100644 --- a/bciers/apps/administration/tests/components/operations/mocks.ts +++ b/bciers/apps/administration/tests/components/operations/mocks.ts @@ -6,6 +6,7 @@ const getRegulatedProducts = vi.fn(); const getRegistrationPurposes = vi.fn(); const getBusinessStructures = vi.fn(); const fetchOperationsPageData = vi.fn(); +const getContacts = vi.fn(); vi.mock("libs/actions/src/api/getOperation", () => ({ default: getOperation, @@ -34,6 +35,9 @@ vi.mock("libs/actions/src/api/getRegistrationPurposes", () => ({ vi.mock("libs/actions/src/api/getBusinessStructures", () => ({ default: getBusinessStructures, })); +vi.mock("libs/actions/src/api/getContacts", () => ({ + default: getContacts, +})); vi.mock("libs/actions/src/api/fetchOperationsPageData", () => ({ default: fetchOperationsPageData, @@ -48,4 +52,5 @@ export { getRegistrationPurposes, getBusinessStructures, fetchOperationsPageData, + getContacts, }; diff --git a/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.test.tsx b/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.test.tsx new file mode 100644 index 0000000000..eb8befe60d --- /dev/null +++ b/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.test.tsx @@ -0,0 +1,101 @@ +import { render, screen } from "@testing-library/react"; +import { RJSFSchema } from "@rjsf/utils"; +import FormBase from "@bciers/components/form/FormBase"; +import PlacesAssignedFieldTemplate from "./PlacesAssignedFieldTemplate"; + +const mockSchema: RJSFSchema = { + type: "object", + properties: { + places_assigned: { + type: "array", + title: "Places assigned", + readOnly: true, + items: { + type: "object", + properties: { + role_name: { type: "string" }, + operation_name: { type: "string" }, + operation_id: { type: "string" }, + }, + }, + }, + }, +}; + +const mockUiSchema = { + places_assigned: { + "ui:ArrayFieldTemplate": PlacesAssignedFieldTemplate, + "ui:classNames": "[&>div:last-child]:w-2/3", + items: { + "ui:widget": "ReadOnlyWidget", + "ui:options": { + label: false, + inline: true, + }, + role_name: { + "ui:options": { + label: false, + }, + }, + operation_name: { + "ui:options": { + label: false, + }, + }, + operation_id: { + "ui:widget": "hidden", + }, + }, + }, +}; + +const mockFormData = { + places_assigned: [ + { + role_name: "testrole", + operation_name: "testoperation", + operation_id: "uuid1", + }, + ], +}; + +describe("RJSF PlacesAssignedFieldTemplate", () => { + it("should render the field template when there are no places assigned", async () => { + render( + , + ); + expect(screen.getByText("None")).toBeVisible(); + }); + it("should render the field template when formData is provided", async () => { + render( + , + ); + expect(screen.getByText("testrole -")).toBeVisible(); + expect(screen.getByText("testoperation")).toBeVisible(); + expect(screen.getByRole("link")).toHaveAttribute( + "href", + "/operations/uuid1?operations_title=testoperation&from_contacts=true", + ); + expect( + screen.getByText( + "You cannot delete this contact unless you replace them with other contact(s) in the place(s) above.", + ), + ).toBeVisible(); + }); + + it("should throw an error if given bad formData", async () => { + expect(() => + render( + , + ), + ).toThrow("Invalid places assigned data"); + }); +}); From 7c4d12faf4772678bdcba19d44051d17c22e899e Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 16 Jan 2025 13:15:41 -0800 Subject: [PATCH 07/13] chore: update mock data --- bc_obps/registration/fixtures/mock/address.json | 8 ++++++++ bc_obps/registration/fixtures/mock/contact.json | 7 +++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/bc_obps/registration/fixtures/mock/address.json b/bc_obps/registration/fixtures/mock/address.json index df7dc4140b..e99c5227d1 100644 --- a/bc_obps/registration/fixtures/mock/address.json +++ b/bc_obps/registration/fixtures/mock/address.json @@ -208,5 +208,13 @@ "province": "ON", "postal_code": "N2L 3G1" } + }, + { + "model": "registration.address", + "pk": 21, + "fields": { + "street_address": "Incomplete address", + "municipality": "Grove" + } } ] diff --git a/bc_obps/registration/fixtures/mock/contact.json b/bc_obps/registration/fixtures/mock/contact.json index 01d2083fc1..32678942ce 100644 --- a/bc_obps/registration/fixtures/mock/contact.json +++ b/bc_obps/registration/fixtures/mock/contact.json @@ -135,9 +135,8 @@ "fields": { "business_role": "Operation Representative", "first_name": "Ivy", - "last_name": "Jones", + "last_name": "Jones - no address", "position_title": "Manager", - "address": 11, "email": "ivy.jones@example.com", "phone_number": "+16044011243" } @@ -148,9 +147,9 @@ "fields": { "business_role": "Operation Representative", "first_name": "Jack", - "last_name": "King", + "last_name": "King - incomplete address", "position_title": "Manager", - "address": 12, + "address": 21, "email": "jack.king@example.com", "phone_number": "+16044011244" } From 2616ff141322bf43c5bd30aabea680ca4a185392 Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 16 Jan 2025 14:54:12 -0800 Subject: [PATCH 08/13] chore: add link to contacts address error --- .../components/operations/OperationInformationForm.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx b/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx index fc973a1ff7..4d0a6e66cf 100644 --- a/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx +++ b/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx @@ -19,6 +19,7 @@ import { import { FormMode, FrontEndRoles } from "@bciers/utils/src/enums"; import { useSessionRole } from "@bciers/utils/src/sessionUtils"; import Note from "@bciers/components/layout/Note"; +import Link from "next/link"; const OperationInformationForm = ({ formData, @@ -49,6 +50,15 @@ const OperationInformationForm = ({ ); if (response?.error) { + if (response.error.includes("Please return to Contacts")) { + const splitError = response.error.split("Contacts"); + response.error = ( + <> + {splitError[0]} Contacts + {splitError[1]} + + ); + } setError(response.error); return { error: response.error }; } From 43d2894fe30a310571a17fde0cf6926f98db22a2 Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 16 Jan 2025 15:36:20 -0800 Subject: [PATCH 09/13] chore: users cannot update unregistered operations --- .../v2/_operations/test_operation_id.py | 5 ++- bc_obps/service/contact_service_v2.py | 5 +-- bc_obps/service/operation_service_v2.py | 12 +++++-- .../service/tests/test_contact_service_v2.py | 9 +++-- .../tests/test_operation_service_v2.py | 36 +++++++++++++++++-- 5 files changed, 51 insertions(+), 16 deletions(-) diff --git a/bc_obps/registration/tests/endpoints/v2/_operations/test_operation_id.py b/bc_obps/registration/tests/endpoints/v2/_operations/test_operation_id.py index fe1315459d..f278055fe2 100644 --- a/bc_obps/registration/tests/endpoints/v2/_operations/test_operation_id.py +++ b/bc_obps/registration/tests/endpoints/v2/_operations/test_operation_id.py @@ -2,6 +2,7 @@ from registration.models import ( UserOperator, ) +from registration.models.operation import Operation from registration.tests.constants import MOCK_DATA_URL from registration.tests.utils.helpers import CommonTestSetup, TestUtils from registration.tests.utils.bakers import operation_baker, operator_baker @@ -106,7 +107,9 @@ def test_operations_with_documents_endpoint_get_success(self): def test_operations_endpoint_put_success(self): approved_user_operator = baker.make_recipe('utils.approved_user_operator', user=self.user) - operation = baker.make_recipe('utils.operation', operator=approved_user_operator.operator) + operation = baker.make_recipe( + 'utils.operation', operator=approved_user_operator.operator, status=Operation.Statuses.REGISTERED + ) contact = baker.make_recipe('utils.contact') self.test_payload["operation_representatives"] = [contact.id] response = TestUtils.mock_put_with_auth_role( diff --git a/bc_obps/service/contact_service_v2.py b/bc_obps/service/contact_service_v2.py index 4cda2478f0..582f6bfb2c 100644 --- a/bc_obps/service/contact_service_v2.py +++ b/bc_obps/service/contact_service_v2.py @@ -8,14 +8,10 @@ from service.data_access_service.contact_service import ContactDataAccessService from service.data_access_service.user_service import UserDataAccessService from ninja import Query -from uuid import UUID from registration.constants import UNAUTHORIZED_MESSAGE -from registration.models.contact import Contact from registration.schema.v1.contact import ContactOut -from service.data_access_service.contact_service import ContactDataAccessService -from service.data_access_service.user_service import UserDataAccessService from ninja import Schema @@ -51,6 +47,7 @@ def list_contacts_v2( ) return cast(QuerySet[Contact], queryset) + @classmethod def get_if_authorized_v2(cls, user_guid: UUID, contact_id: int) -> Optional[Contact]: user = UserDataAccessService.get_by_guid(user_guid) user_contacts = ContactDataAccessService.get_all_contacts_for_user(user) diff --git a/bc_obps/service/operation_service_v2.py b/bc_obps/service/operation_service_v2.py index 468e6c2e65..de963a6123 100644 --- a/bc_obps/service/operation_service_v2.py +++ b/bc_obps/service/operation_service_v2.py @@ -383,14 +383,20 @@ def update_operation( payload: OperationInformationIn, operation_id: UUID, ) -> Operation: - OperationService.get_if_authorized(user_guid, operation_id) + """ + This service is used for updating an operation after it's been registered. During registration, we use the endpoints in cas-registration/bc_obps/registration/api/v2/_operations/_operation_id/_registration + """ + operation = OperationService.get_if_authorized(user_guid, operation_id) - operation: Operation = cls.create_or_update_operation_v2( + if not operation.status == Operation.Statuses.REGISTERED: + raise Exception('Operation must be registered') + + updated_operation: Operation = cls.create_or_update_operation_v2( user_guid, payload, operation_id, ) - return operation + return updated_operation @classmethod def is_operation_opt_in_information_complete(cls, operation: Operation) -> bool: diff --git a/bc_obps/service/tests/test_contact_service_v2.py b/bc_obps/service/tests/test_contact_service_v2.py index 51af9ff1f3..8ec94e8deb 100644 --- a/bc_obps/service/tests/test_contact_service_v2.py +++ b/bc_obps/service/tests/test_contact_service_v2.py @@ -1,11 +1,8 @@ from registration.schema.v1.contact import ContactFilterSchema import pytest -from service.contact_service_v2 import ContactServiceV2 +from service.contact_service_v2 import ContactServiceV2, PlacesAssigned from model_bakery import baker from registration.models.business_role import BusinessRole -import pytest -from model_bakery import baker -from service.contact_service_v2 import ContactServiceV2 pytestmark = pytest.mark.django_db @@ -52,7 +49,9 @@ def test_get_with_places_assigned_with_contacts(): result = ContactServiceV2.get_with_places_assigned_v2(approved_user_operator.user.user_guid, contact.id) assert result.places_assigned == [ - f"Operation Representative - {operation.name}", + PlacesAssigned( + role_name=contact.business_role.role_name, operation_name=operation.name, operation_id=operation.id + ) ] @staticmethod diff --git a/bc_obps/service/tests/test_operation_service_v2.py b/bc_obps/service/tests/test_operation_service_v2.py index d16a4facae..f9272c6951 100644 --- a/bc_obps/service/tests/test_operation_service_v2.py +++ b/bc_obps/service/tests/test_operation_service_v2.py @@ -685,7 +685,7 @@ def test_update_operation_with_operation_representative_without_address(): ) with pytest.raises( Exception, - match=f'The contact "{contact.first_name}" "{contact.last_name}"is missing address information. Please return to Contacts and fill in their address information before assigning them as an Operation Representative here.', + match=f'The contact {contact.first_name} {contact.last_name} is missing address information. Please return to Contacts and fill in their address information before assigning them as an Operation Representative here.', ): OperationServiceV2.create_or_update_operation_v2( approved_user_operator.user.user_guid, @@ -695,10 +695,36 @@ def test_update_operation_with_operation_representative_without_address(): class TestOperationServiceV2UpdateOperation: + def test_update_operation_fails_if_operation_not_registered(self): + approved_user_operator = baker.make_recipe('utils.approved_user_operator') + existing_operation = baker.make_recipe( + 'utils.operation', + operator=approved_user_operator.operator, + created_by=approved_user_operator.user, + status=Operation.Statuses.DRAFT, + ) + payload = OperationInformationIn( + registration_purpose='Potential Reporting Operation', + regulated_products=[1], + name="Test Update Operation Name", + type="SFO", + naics_code_id=1, + secondary_naics_code_id=1, + tertiary_naics_code_id=2, + activities=[2], + process_flow_diagram=MOCK_DATA_URL, + boundary_map=MOCK_DATA_URL, + ) + with pytest.raises(Exception, match='Operation must be registered'): + OperationServiceV2.update_operation(approved_user_operator.user.user_guid, payload, existing_operation.id) + def test_update_operation(self): approved_user_operator = baker.make_recipe('utils.approved_user_operator') existing_operation = baker.make_recipe( - 'utils.operation', operator=approved_user_operator.operator, created_by=approved_user_operator.user + 'utils.operation', + operator=approved_user_operator.operator, + created_by=approved_user_operator.user, + status=Operation.Statuses.REGISTERED, ) payload = OperationInformationIn( registration_purpose='Potential Reporting Operation', @@ -726,7 +752,10 @@ def test_update_operation(self): def test_update_operation_with_no_regulated_products(self): approved_user_operator = baker.make_recipe('utils.approved_user_operator') existing_operation = baker.make_recipe( - 'utils.operation', operator=approved_user_operator.operator, created_by=approved_user_operator.user + 'utils.operation', + operator=approved_user_operator.operator, + created_by=approved_user_operator.user, + status=Operation.Statuses.REGISTERED, ) payload = OperationInformationIn( registration_purpose='OBPS Regulated Operation', @@ -759,6 +788,7 @@ def test_update_operation_with_new_entrant_application_data(self): operator=approved_user_operator.operator, created_by=approved_user_operator.user, date_of_first_shipment=Operation.DateOfFirstShipmentChoices.ON_OR_AFTER_APRIL_1_2024, + status=Operation.Statuses.REGISTERED, ) payload = OperationInformationIn( registration_purpose='New Entrant Operation', From 0506322e81e9e2c6b799c208397425b93bcfab1c Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Fri, 17 Jan 2025 08:47:35 -0800 Subject: [PATCH 10/13] chore: cleanup --- bc_obps/service/operation_service.py | 8 +++++--- bc_obps/service/tests/test_operation_service_v2.py | 5 +++-- .../components/operations/OperationInformationForm.tsx | 1 + .../administrationRegistrationInformation.ts | 6 +++--- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/bc_obps/service/operation_service.py b/bc_obps/service/operation_service.py index b9fad77d18..bcb043583e 100644 --- a/bc_obps/service/operation_service.py +++ b/bc_obps/service/operation_service.py @@ -218,9 +218,11 @@ def list_operations( base_qs = OperationDataAccessService.get_all_operations_for_user(user) list_of_filters = [ Q(bcghg_id__id__icontains=bcghg_id) if bcghg_id else Q(), - Q(bc_obps_regulated_operation__id__icontains=bc_obps_regulated_operation) - if bc_obps_regulated_operation - else Q(), + ( + Q(bc_obps_regulated_operation__id__icontains=bc_obps_regulated_operation) + if bc_obps_regulated_operation + else Q() + ), Q(name__icontains=name) if name else Q(), Q(operator__legal_name__icontains=operator) if operator else Q(), Q(status__icontains=status) if status else Q(), diff --git a/bc_obps/service/tests/test_operation_service_v2.py b/bc_obps/service/tests/test_operation_service_v2.py index f9272c6951..83c291011e 100644 --- a/bc_obps/service/tests/test_operation_service_v2.py +++ b/bc_obps/service/tests/test_operation_service_v2.py @@ -622,7 +622,7 @@ def test_update_operation_archive_multiple_operators(): @staticmethod def test_update_operation_with_operation_representatives_with_address(): approved_user_operator = baker.make_recipe('utils.approved_user_operator') - existing_operation = baker.make_recipe('utils.operation') + existing_operation = baker.make_recipe('utils.operation', operator=approved_user_operator.operator) contacts = baker.make_recipe( 'utils.contact', business_role=BusinessRole.objects.get(role_name='Operation Representative'), _quantity=3 ) @@ -640,6 +640,7 @@ def test_update_operation_with_operation_representatives_with_address(): boundary_map=MOCK_DATA_URL, operation_representatives=[contact.id for contact in contacts], ) + operation = OperationServiceV2.create_or_update_operation_v2( approved_user_operator.user.user_guid, payload, @@ -654,7 +655,7 @@ def test_update_operation_with_operation_representatives_with_address(): @staticmethod def test_update_operation_with_operation_representative_without_address(): approved_user_operator = baker.make_recipe('utils.approved_user_operator') - existing_operation = baker.make_recipe('utils.operation') + existing_operation = baker.make_recipe('utils.operation', operator=approved_user_operator.operator) # create contacts with incomplete address data contacts = baker.make_recipe( 'utils.contact', business_role=BusinessRole.objects.get(role_name='Operation Representative'), _quantity=5 diff --git a/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx b/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx index 4d0a6e66cf..3aae947af6 100644 --- a/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx +++ b/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx @@ -40,6 +40,7 @@ const OperationInformationForm = ({ const handleSubmit = async (data: { formData?: OperationInformationFormData; }) => { + setError(undefined); const response = await actionHandler( `registration/operations/${operationId}`, "PUT", diff --git a/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts b/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts index a97239f3a5..d0229a3507 100644 --- a/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts +++ b/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts @@ -10,8 +10,9 @@ export const createAdministrationRegistrationInformationSchema = async ( // fetch db values that are dropdown options const regulatedProducts: { id: number; name: string }[] = await getRegulatedProducts(); - const contacts: { id: number; first_name: string; last_name: string }[] = - await getContacts(); + const contacts: { + items: [{ id: number; first_name: string; last_name: string }]; + } = await getContacts(); const isRegulatedProducts = registrationPurposeValue === RegistrationPurposes.OBPS_REGULATED_OPERATION.valueOf(); @@ -131,7 +132,6 @@ export const registrationInformationUiSchema: UiSchema = { "new_entrant_application", ], "ui:FieldTemplate": SectionFieldTemplate, - // brianna why doesn't error show, looks exactly the same as regulated_products operation_representatives: { "ui:widget": "MultiSelectWidget", }, From 871d31402a0a0a95508ccf95aff8fba540ad2b34 Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Fri, 17 Jan 2025 14:21:47 -0800 Subject: [PATCH 11/13] chore: update mock data --- .../registration/fixtures/mock/contact.json | 33 +++++------ .../registration/fixtures/mock/operation.json | 58 ++++++++++++------- .../registration/fixtures/mock/operator.json | 21 ++----- 3 files changed, 56 insertions(+), 56 deletions(-) diff --git a/bc_obps/registration/fixtures/mock/contact.json b/bc_obps/registration/fixtures/mock/contact.json index 32678942ce..37052f3a22 100644 --- a/bc_obps/registration/fixtures/mock/contact.json +++ b/bc_obps/registration/fixtures/mock/contact.json @@ -4,11 +4,11 @@ "pk": 1, "fields": { "business_role": "Operation Representative", - "first_name": "John", - "last_name": "Doe", + "first_name": "Alice", + "last_name": "Art", "position_title": "Manager", "address": 1, - "email": "john.doe@example.com", + "email": "alice.art@example.com", "phone_number": "+16044011234" } }, @@ -17,11 +17,11 @@ "pk": 2, "fields": { "business_role": "Operation Representative", - "first_name": "Jane", - "last_name": "Smith", + "first_name": "Althea", + "last_name": "Ark", "position_title": "Manager", "address": 2, - "email": "jane.smith@example.com", + "email": "althea.ark@example.com", "phone_number": "+16044011234" } }, @@ -30,11 +30,11 @@ "pk": 3, "fields": { "business_role": "Operation Representative", - "first_name": "Alice", - "last_name": "Johnson", + "first_name": "Bill", + "last_name": "Blue", "position_title": "Manager", "address": 3, - "email": "alice.johnson@example.com", + "email": "bill.blue@example.com", "phone_number": "+16044011235" } }, @@ -56,11 +56,10 @@ "pk": 5, "fields": { "business_role": "Operation Representative", - "first_name": "Carol", - "last_name": "Davis", + "first_name": "Blair", + "last_name": "Balloons - no address", "position_title": "Manager", - "address": 5, - "email": "carol.davis@example.com", + "email": "blair.balloons@example.com", "phone_number": "+16044011237" } }, @@ -69,11 +68,11 @@ "pk": 6, "fields": { "business_role": "Operation Representative", - "first_name": "Dave", - "last_name": "Evans", + "first_name": "Bart", + "last_name": "Banker - incomplete address", "position_title": "Manager", - "address": 6, - "email": "dave.evans@example.com", + "address": 21, + "email": "bart.banker@example.com", "phone_number": "+16044011238" } }, diff --git a/bc_obps/registration/fixtures/mock/operation.json b/bc_obps/registration/fixtures/mock/operation.json index bf010d645a..ccc9e5074e 100644 --- a/bc_obps/registration/fixtures/mock/operation.json +++ b/bc_obps/registration/fixtures/mock/operation.json @@ -14,7 +14,8 @@ "bc_obps_regulated_operation": "24-0014", "created_at": "2024-2-01T15:27:00.000Z", "activities": [1, 5], - "registration_purpose": "Reporting Operation" + "registration_purpose": "Reporting Operation", + "contacts": [1, 2] } }, { @@ -34,8 +35,8 @@ "bc_obps_regulated_operation": "24-0015", "created_at": "2024-2-02T15:27:00.000Z", "activities": [1, 5], - "contacts": [10], - "registration_purpose": "OBPS Regulated Operation" + "registration_purpose": "OBPS Regulated Operation", + "contacts": [3, 4] } }, { @@ -53,7 +54,6 @@ "status": "Draft", "created_at": "2024-2-02T15:27:00.000Z", "activities": [1, 5], - "contacts": [10], "registration_purpose": "OBPS Regulated Operation" } }, @@ -75,8 +75,8 @@ "bc_obps_regulated_operation": "23-0001", "created_at": "2024-1-31T15:27:00.000Z", "activities": [1, 3], - "contacts": [14, 15], - "registration_purpose": "OBPS Regulated Operation" + "registration_purpose": "OBPS Regulated Operation", + "contacts": [3, 4] } }, { @@ -97,8 +97,8 @@ "bc_obps_regulated_operation": "23-0002", "created_at": "2024-1-30T15:27:00.000Z", "activities": [1, 5], - "contacts": [10, 11, 12], - "registration_purpose": "OBPS Regulated Operation" + "registration_purpose": "OBPS Regulated Operation", + "contacts": [1] } }, { @@ -120,7 +120,8 @@ "bc_obps_regulated_operation": "24-0003", "created_at": "2024-1-29T15:27:00.000Z", "activities": [1, 5], - "operation_has_multiple_operators": true + "operation_has_multiple_operators": true, + "contacts": [3] } }, { @@ -141,7 +142,8 @@ "bc_obps_regulated_operation": "24-0004", "created_at": "2024-1-28T15:27:00.000Z", "activities": [1, 5], - "registration_purpose": "Opted-in Operation" + "registration_purpose": "Opted-in Operation", + "contacts": [1] } }, { @@ -161,7 +163,8 @@ "bc_obps_regulated_operation": "24-0005", "created_at": "2024-1-27T15:27:00.000Z", "activities": [1, 5], - "registration_purpose": "Potential Reporting Operation" + "registration_purpose": "Potential Reporting Operation", + "contacts": [2] } }, { @@ -181,7 +184,8 @@ "bc_obps_regulated_operation": "24-0006", "created_at": "2024-1-26T15:27:00.000Z", "activities": [1, 5], - "registration_purpose": "Electricity Import Operation" + "registration_purpose": "Electricity Import Operation", + "contacts": [1] } }, { @@ -202,7 +206,8 @@ "bc_obps_regulated_operation": "24-0007", "created_at": "2024-1-25T15:27:00.000Z", "activities": [1, 5], - "registration_purpose": "New Entrant Operation" + "registration_purpose": "New Entrant Operation", + "contacts": [1] } }, { @@ -221,7 +226,8 @@ "bc_obps_regulated_operation": "24-0008", "created_at": "2024-1-24T15:27:00.000Z", "activities": [1, 5], - "registration_purpose": "Reporting Operation" + "registration_purpose": "Reporting Operation", + "contacts": [2] } }, { @@ -242,7 +248,8 @@ "bc_obps_regulated_operation": "24-0009", "created_at": "2024-1-23T15:27:00.000Z", "activities": [1, 5], - "registration_purpose": "OBPS Regulated Operation" + "registration_purpose": "OBPS Regulated Operation", + "contacts": [1, 2] } }, { @@ -262,7 +269,8 @@ "bc_obps_regulated_operation": "24-0010", "created_at": "2024-1-22T15:27:00.000Z", "activities": [1, 5], - "registration_purpose": "Reporting Operation" + "registration_purpose": "Reporting Operation", + "contacts": [1, 2] } }, { @@ -282,7 +290,8 @@ "bc_obps_regulated_operation": "24-0011", "created_at": "2024-1-21T15:27:00.000Z", "activities": [1, 5], - "registration_purpose": "OBPS Regulated Operation" + "registration_purpose": "OBPS Regulated Operation", + "contacts": [1] } }, { @@ -360,7 +369,8 @@ "bc_obps_regulated_operation": "24-0012", "created_at": "2024-1-17T15:27:00.000Z", "activities": [1, 5], - "registration_purpose": "OBPS Regulated Operation" + "registration_purpose": "OBPS Regulated Operation", + "contacts": [3] } }, { @@ -399,7 +409,8 @@ "bc_obps_regulated_operation": "24-0013", "registration_purpose": "New Entrant Operation", "created_at": "2024-1-15T15:27:00.000Z", - "activities": [1, 3] + "activities": [1, 3], + "contacts": [4] } }, { @@ -419,7 +430,8 @@ "bc_obps_regulated_operation": "24-0016", "created_at": "2024-1-14T15:27:00.000Z", "registration_purpose": "Reporting Operation", - "activities": [1, 5] + "activities": [1, 5], + "contacts": [3] } }, { @@ -438,7 +450,8 @@ "bc_obps_regulated_operation": "24-0017", "created_at": "2024-1-13T15:27:00.000Z", "registration_purpose": "Reporting Operation", - "activities": [1, 5] + "activities": [1, 5], + "contacts": [3] } }, { @@ -457,7 +470,8 @@ "bc_obps_regulated_operation": "24-0018", "created_at": "2024-1-12T15:27:00.000Z", "registration_purpose": "Reporting Operation", - "activities": [1, 5] + "activities": [1, 5], + "contacts": [4] } }, { diff --git a/bc_obps/registration/fixtures/mock/operator.json b/bc_obps/registration/fixtures/mock/operator.json index eb90f3358b..5da7318310 100644 --- a/bc_obps/registration/fixtures/mock/operator.json +++ b/bc_obps/registration/fixtures/mock/operator.json @@ -8,7 +8,8 @@ "bc_corporate_registry_number": "abc1234567", "business_structure": "Sole Proprietorship", "status": "Approved", - "is_new": false + "is_new": false, + "contacts": [1, 2] } }, { @@ -20,9 +21,9 @@ "bc_corporate_registry_number": "def1234567", "business_structure": "General Partnership", "mailing_address": 3, - "contacts": [10, 11, 12, 13, 14, 15], "status": "Approved", - "is_new": false + "is_new": false, + "contacts": [3, 4, 5, 6] } }, { @@ -34,7 +35,6 @@ "bc_corporate_registry_number": "ghi1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -48,7 +48,6 @@ "bc_corporate_registry_number": "jkl1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1, 2], "status": "Approved", "is_new": false } @@ -62,7 +61,6 @@ "bc_corporate_registry_number": "mno1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -76,7 +74,6 @@ "bc_corporate_registry_number": "pqr1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -90,7 +87,6 @@ "bc_corporate_registry_number": "stu1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -104,7 +100,6 @@ "bc_corporate_registry_number": "vwx1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -118,7 +113,6 @@ "bc_corporate_registry_number": "yza1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -132,7 +126,6 @@ "bc_corporate_registry_number": "bcd1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -146,7 +139,6 @@ "bc_corporate_registry_number": "efg1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -160,7 +152,6 @@ "bc_corporate_registry_number": "hij1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -174,7 +165,6 @@ "bc_corporate_registry_number": "klm1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -188,7 +178,6 @@ "bc_corporate_registry_number": "nop1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -202,7 +191,6 @@ "bc_corporate_registry_number": "qrs1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } @@ -216,7 +204,6 @@ "bc_corporate_registry_number": "qrs1234567", "business_structure": "BC Corporation", "mailing_address": 4, - "contacts": [1], "status": "Approved", "is_new": false } From 882d9000c147762150f12e87ed609d18f0153bd9 Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 23 Jan 2025 13:56:18 -0800 Subject: [PATCH 12/13] chore: apply suggestions from code review --- .../api/v2/_contacts/contact_id.py | 4 +- bc_obps/registration/schema/v2/contact.py | 15 +++++ bc_obps/registration/schema/v2/operation.py | 2 +- bc_obps/service/contact_service_v2.py | 42 +++++-------- bc_obps/service/operation_service_v2.py | 14 +---- .../service/tests/test_contact_service_v2.py | 33 ++++++++++ .../tests/test_operation_service_v2.py | 62 ------------------- .../app/components/contacts/ContactForm.tsx | 7 ++- .../operations/OperationInformationForm.tsx | 3 +- .../administrationRegistrationInformation.ts | 5 ++ .../components/contacts/ContactForm.test.tsx | 26 ++++++-- .../components/contacts/ContactPage.test.tsx | 12 ++-- .../tests/components/contacts/mocks.ts | 7 ++- .../OperationInformationForm.test.tsx | 2 +- .../tests/components/operations/mocks.ts | 5 -- .../PlacesAssignedFieldTemplate.test.tsx | 28 ++++++++- .../fields/PlacesAssignedFieldTemplate.tsx | 31 +++++----- 17 files changed, 162 insertions(+), 136 deletions(-) diff --git a/bc_obps/registration/api/v2/_contacts/contact_id.py b/bc_obps/registration/api/v2/_contacts/contact_id.py index 44a1fd138b..d9cea7545d 100644 --- a/bc_obps/registration/api/v2/_contacts/contact_id.py +++ b/bc_obps/registration/api/v2/_contacts/contact_id.py @@ -15,7 +15,7 @@ @router.get( "/contacts/{contact_id}", - response={200: ContactOut, custom_codes_4xx: Message}, + response={200: ContactWithPlacesAssigned, custom_codes_4xx: Message}, tags=CONTACT_TAGS, description="""Retrieves the details of a specific contact by its ID. The endpoint checks if the current user is authorized to access the contact. Industry users can only access contacts that are associated with their own operator. If an unauthorized user attempts to access the contact, an error is raised.""", @@ -24,7 +24,7 @@ ) @handle_http_errors() def get_contact(request: HttpRequest, contact_id: int) -> Tuple[Literal[200], Optional[ContactWithPlacesAssigned]]: - return 200, ContactServiceV2.get_with_places_assigned_v2(get_current_user_guid(request), contact_id) + return 200, ContactServiceV2.get_with_places_assigned_v2(contact_id) @router.put( diff --git a/bc_obps/registration/schema/v2/contact.py b/bc_obps/registration/schema/v2/contact.py index 5fe1d761d2..f2529960cc 100644 --- a/bc_obps/registration/schema/v2/contact.py +++ b/bc_obps/registration/schema/v2/contact.py @@ -2,6 +2,21 @@ from ninja import ModelSchema, Field from registration.models import Contact from ninja import FilterSchema +from uuid import UUID + + +from registration.schema.v1.contact import ContactOut +from ninja import Schema + + +class PlacesAssigned(Schema): + role_name: str + operation_name: str + operation_id: UUID + + +class ContactWithPlacesAssigned(ContactOut): + places_assigned: Optional[list[PlacesAssigned]] class ContactListOutV2(ModelSchema): diff --git a/bc_obps/registration/schema/v2/operation.py b/bc_obps/registration/schema/v2/operation.py index c5576e1cf4..c3d50c2094 100644 --- a/bc_obps/registration/schema/v2/operation.py +++ b/bc_obps/registration/schema/v2/operation.py @@ -194,7 +194,7 @@ class Meta: 'status', 'activities', 'opted_in_operation', - 'contacts', + # 'contacts', ] from_attributes = True diff --git a/bc_obps/service/contact_service_v2.py b/bc_obps/service/contact_service_v2.py index 582f6bfb2c..b5960505bd 100644 --- a/bc_obps/service/contact_service_v2.py +++ b/bc_obps/service/contact_service_v2.py @@ -4,27 +4,12 @@ from registration.models.contact import Contact -from registration.schema.v2.contact import ContactFilterSchemaV2 +from registration.schema.v2.contact import ContactFilterSchemaV2, ContactWithPlacesAssigned, PlacesAssigned from service.data_access_service.contact_service import ContactDataAccessService from service.data_access_service.user_service import UserDataAccessService from ninja import Query -from registration.constants import UNAUTHORIZED_MESSAGE -from registration.schema.v1.contact import ContactOut -from ninja import Schema - - -class PlacesAssigned(Schema): - role_name: str - operation_name: str - operation_id: UUID - - -class ContactWithPlacesAssigned(ContactOut): - places_assigned: Optional[list[PlacesAssigned]] - - class ContactServiceV2: @classmethod def list_contacts_v2( @@ -48,17 +33,12 @@ def list_contacts_v2( return cast(QuerySet[Contact], queryset) @classmethod - def get_if_authorized_v2(cls, user_guid: UUID, contact_id: int) -> Optional[Contact]: - user = UserDataAccessService.get_by_guid(user_guid) - user_contacts = ContactDataAccessService.get_all_contacts_for_user(user) - contact = user_contacts.filter(id=contact_id).first() - if user.is_industry_user() and not contact: - raise Exception(UNAUTHORIZED_MESSAGE) - return contact + def get_by_id(cls, contact_id: int) -> Contact: + return Contact.objects.get(id=contact_id) @classmethod - def get_with_places_assigned_v2(cls, user_guid: UUID, contact_id: int) -> Optional[ContactWithPlacesAssigned]: - contact = cls.get_if_authorized_v2(user_guid, contact_id) + def get_with_places_assigned_v2(cls, contact_id: int) -> Optional[ContactWithPlacesAssigned]: + contact = cls.get_by_id(contact_id) places_assigned = [] if contact: role_name = contact.business_role.role_name @@ -74,3 +54,15 @@ def get_with_places_assigned_v2(cls, user_guid: UUID, contact_id: int) -> Option result.places_assigned = places_assigned return result return None + + @classmethod + def raise_exception_if_contact_missing_address_information(cls, contact_id: int) -> None: + """This function checks that a contact has a complete address record (contact.address exists and all fields in the address model have a value). In general in the app, address is not mandatory, but in certain cases (e.g., when a contact is assigned to an operation as the Operation Representative), the business area requires the contact to have an address.""" + contact = cls.get_by_id(contact_id) + address = contact.address + if not address or any( + not getattr(address, field, None) for field in ['street_address', 'municipality', 'province', 'postal_code'] + ): + raise Exception( + f'The contact {contact.first_name} {contact.last_name} is missing address information. Please return to Contacts and fill in their address information before assigning them as an Operation Representative here.' + ) diff --git a/bc_obps/service/operation_service_v2.py b/bc_obps/service/operation_service_v2.py index de963a6123..d5bdd921ac 100644 --- a/bc_obps/service/operation_service_v2.py +++ b/bc_obps/service/operation_service_v2.py @@ -1,6 +1,7 @@ from typing import Optional, Tuple, Callable, Generator, Union from django.db.models import QuerySet from registration.schema.v2.operation_timeline import OperationTimelineFilterSchema +from service.contact_service_v2 import ContactServiceV2 from service.data_access_service.operation_designated_operator_timeline_service import ( OperationDesignatedOperatorTimelineDataAccessService, ) @@ -234,18 +235,7 @@ def create_or_update_operation_v2( if isinstance(payload, OperationInformationInUpdate): for contact_id in payload.operation_representatives: - contact = Contact.objects.get(id=contact_id) - address = contact.address - if ( - not address - or not address.street_address - or not address.municipality - or not address.province - or not address.postal_code - ): - raise Exception( - f'The contact {contact.first_name} {contact.last_name} is missing address information. Please return to Contacts and fill in their address information before assigning them as an Operation Representative here.' - ) + ContactServiceV2.raise_exception_if_contact_missing_address_information(contact_id) operation.contacts.set(payload.operation_representatives) diff --git a/bc_obps/service/tests/test_contact_service_v2.py b/bc_obps/service/tests/test_contact_service_v2.py index 8ec94e8deb..cdbb31b9d1 100644 --- a/bc_obps/service/tests/test_contact_service_v2.py +++ b/bc_obps/service/tests/test_contact_service_v2.py @@ -65,3 +65,36 @@ def test_get_with_places_assigned_with_no_contacts(): result = ContactServiceV2.get_with_places_assigned_v2(approved_user_operator.user.user_guid, contact.id) assert not hasattr(result, 'places_assigned') + + @staticmethod + def test_raises_exception_if_contact_missing_address(): + contact = baker.make_recipe('utils.contact', address=None) + + with pytest.raises( + Exception, + match=f'The contact {contact.first_name} {contact.last_name} is missing address information. Please return to Contacts and fill in their address information before assigning them as an Operation Representative here.', + ): + ContactServiceV2.raise_exception_if_contact_missing_address_information(contact.id) + + @staticmethod + def test_raises_exception_if_operation_rep_missing_required_fields(): + contacts = baker.make_recipe( + 'utils.contact', business_role=BusinessRole.objects.get(role_name='Operation Representative'), _quantity=5 + ) + contacts[0].address = None + contacts[0].save() + contacts[1].address.street_address = None + contacts[1].address.save() + contacts[2].address.municipality = None + contacts[2].address.save() + contacts[3].address.province = None + contacts[3].address.save() + contacts[4].address.postal_code = None + contacts[4].address.save() + + for contact in contacts: + with pytest.raises( + Exception, + match=f'The contact {contact.first_name} {contact.last_name} is missing address information. Please return to Contacts and fill in their address information before assigning them as an Operation Representative here.', + ): + ContactServiceV2.raise_exception_if_contact_missing_address_information(contact.id) diff --git a/bc_obps/service/tests/test_operation_service_v2.py b/bc_obps/service/tests/test_operation_service_v2.py index 83c291011e..55f3c06811 100644 --- a/bc_obps/service/tests/test_operation_service_v2.py +++ b/bc_obps/service/tests/test_operation_service_v2.py @@ -652,48 +652,6 @@ def test_update_operation_with_operation_representatives_with_address(): assert operation.updated_by == approved_user_operator.user assert operation.updated_at is not None - @staticmethod - def test_update_operation_with_operation_representative_without_address(): - approved_user_operator = baker.make_recipe('utils.approved_user_operator') - existing_operation = baker.make_recipe('utils.operation', operator=approved_user_operator.operator) - # create contacts with incomplete address data - contacts = baker.make_recipe( - 'utils.contact', business_role=BusinessRole.objects.get(role_name='Operation Representative'), _quantity=5 - ) - contacts[0].address = None - contacts[0].save() - contacts[1].address.street_address = None - contacts[1].address.save() - contacts[2].address.municipality = None - contacts[2].address.save() - contacts[3].address.province = None - contacts[3].address.save() - contacts[4].address.postal_code = None - contacts[4].address.save() - for contact in contacts: - payload = OperationInformationInUpdate( - registration_purpose='Reporting Operation', - regulated_products=[1], - name="I am updated", - type="SFO", - naics_code_id=1, - secondary_naics_code_id=2, - tertiary_naics_code_id=3, - activities=[1], - process_flow_diagram=MOCK_DATA_URL, - boundary_map=MOCK_DATA_URL, - operation_representatives=[contact.id], - ) - with pytest.raises( - Exception, - match=f'The contact {contact.first_name} {contact.last_name} is missing address information. Please return to Contacts and fill in their address information before assigning them as an Operation Representative here.', - ): - OperationServiceV2.create_or_update_operation_v2( - approved_user_operator.user.user_guid, - payload, - existing_operation.id, - ) - class TestOperationServiceV2UpdateOperation: def test_update_operation_fails_if_operation_not_registered(self): @@ -893,26 +851,6 @@ def test_raises_exception_if_no_operation_rep(): with pytest.raises(Exception, match="Operation must have an operation representative with an address."): OperationServiceV2.raise_exception_if_operation_missing_registration_information(operation) - @staticmethod - def test_raises_exception_if_operation_rep_missing_address(): - operation = set_up_valid_mock_operation(Operation.Purposes.OPTED_IN_OPERATION) - op_rep = operation.contacts.first() - op_rep.address = None - op_rep.save() - - with pytest.raises(Exception, match="Operation must have an operation representative with an address."): - OperationServiceV2.raise_exception_if_operation_missing_registration_information(operation) - - @staticmethod - def test_raises_exception_if_operation_rep_missing_required_fields(): - operation = set_up_valid_mock_operation(Operation.Purposes.OPTED_IN_OPERATION) - op_rep_address = operation.contacts.first().address - op_rep_address.street_address = None - op_rep_address.save() - - with pytest.raises(Exception, match="Operation must have an operation representative with an address."): - OperationServiceV2.raise_exception_if_operation_missing_registration_information(operation) - @staticmethod def test_raises_exception_if_no_facilities(): operation = set_up_valid_mock_operation(Operation.Purposes.OPTED_IN_OPERATION) diff --git a/bciers/apps/administration/app/components/contacts/ContactForm.tsx b/bciers/apps/administration/app/components/contacts/ContactForm.tsx index 4b8aeb873a..d9a9b58603 100644 --- a/bciers/apps/administration/app/components/contacts/ContactForm.tsx +++ b/bciers/apps/administration/app/components/contacts/ContactForm.tsx @@ -7,6 +7,7 @@ import { actionHandler } from "@bciers/actions"; import { ContactFormData } from "./types"; import { FormMode } from "@bciers/utils/src/enums"; import { contactsUiSchema } from "@/administration/app/data/jsonSchema/contact"; +import { useSessionRole } from "@bciers/utils/src/sessionUtils"; interface Props { schema: any; @@ -32,6 +33,7 @@ export default function ContactForm({ const [formState, setFormState] = useState(formData ?? {}); const [isCreatingState, setIsCreatingState] = useState(isCreating); const [key, setKey] = useState(Math.random()); + const role = useSessionRole(); return ( } + inlineMessage={ + isCreatingState && !role.includes("cas") && + } onSubmit={async (data: { formData?: any }) => { const updatedFormData = { ...formState, ...data.formData }; setFormState(updatedFormData); diff --git a/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx b/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx index 3aae947af6..d390ca5dd2 100644 --- a/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx +++ b/bciers/apps/administration/app/components/operations/OperationInformationForm.tsx @@ -51,6 +51,7 @@ const OperationInformationForm = ({ ); if (response?.error) { + // Users get this error when they select a contact that's missing address information. We include a link to the Contacts page because the user has to fix the error from there, not here in the operation form. if (response.error.includes("Please return to Contacts")) { const splitError = response.error.split("Contacts"); response.error = ( @@ -81,7 +82,7 @@ const OperationInformationForm = ({ }; return ( <> - {isRedirectedFromContacts && ( + {isRedirectedFromContacts && !role.includes("cas_") && ( To remove the current operation representative, please select a new contact to replace them. diff --git a/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts b/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts index d0229a3507..2d534eef68 100644 --- a/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts +++ b/bciers/apps/administration/app/data/jsonSchema/operationInformation/administrationRegistrationInformation.ts @@ -10,9 +10,14 @@ export const createAdministrationRegistrationInformationSchema = async ( // fetch db values that are dropdown options const regulatedProducts: { id: number; name: string }[] = await getRegulatedProducts(); + if (regulatedProducts && "error" in regulatedProducts) + throw new Error("Failed to retrieve regulated products information"); const contacts: { items: [{ id: number; first_name: string; last_name: string }]; } = await getContacts(); + if (contacts && "error" in contacts) + throw new Error("Failed to retrieve contacts information"); + const isRegulatedProducts = registrationPurposeValue === RegistrationPurposes.OBPS_REGULATED_OPERATION.valueOf(); diff --git a/bciers/apps/administration/tests/components/contacts/ContactForm.test.tsx b/bciers/apps/administration/tests/components/contacts/ContactForm.test.tsx index 6674d140a2..c013b5d60f 100644 --- a/bciers/apps/administration/tests/components/contacts/ContactForm.test.tsx +++ b/bciers/apps/administration/tests/components/contacts/ContactForm.test.tsx @@ -1,6 +1,6 @@ import { render, screen, act, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { actionHandler, useRouter } from "@bciers/testConfig/mocks"; +import { actionHandler, useRouter, useSession } from "@bciers/testConfig/mocks"; import { contactsSchema, contactsUiSchema, @@ -98,6 +98,13 @@ export const fillContactForm = async () => { describe("ContactForm component", () => { beforeEach(async () => { vi.clearAllMocks(); + useSession.mockReturnValue({ + data: { + user: { + app_role: "industry_user_admin", + }, + }, + }); }); it("renders the empty contact form when creating a new contact", async () => { @@ -123,7 +130,14 @@ describe("ContactForm component", () => { expect(screen.getByRole("button", { name: /submit/i })).toBeEnabled(); expect(screen.getByRole("button", { name: /cancel/i })).toBeEnabled(); }); - it("loads existing readonly contact form data", async () => { + it("loads existing readonly contact form data for an internal user", async () => { + useSession.mockReturnValue({ + data: { + user: { + app_role: "industry_user_admin", + }, + }, + }); const readOnlyContactSchema = createContactSchema(contactsSchema, false); const { container } = render( { isCreating={false} />, ); - // form fields + + // Inline message expect( - screen.queryByText(/Is this contact a user in BCIERS/i), + screen.queryByText( + /To assign this representative to an operation, go to the operation information form/i, + ), ).not.toBeInTheDocument(); - expect(screen.queryByText(/Select the user/i)).not.toBeInTheDocument(); expect( container.querySelector("#root_section1_first_name"), diff --git a/bciers/apps/administration/tests/components/contacts/ContactPage.test.tsx b/bciers/apps/administration/tests/components/contacts/ContactPage.test.tsx index 8020a34d24..152d9d380a 100644 --- a/bciers/apps/administration/tests/components/contacts/ContactPage.test.tsx +++ b/bciers/apps/administration/tests/components/contacts/ContactPage.test.tsx @@ -3,10 +3,6 @@ import { useSession, useRouter } from "@bciers/testConfig/mocks"; import { getContact, getUserOperatorUsers } from "./mocks"; import ContactPage from "apps/administration/app/components/contacts/ContactPage"; -useSession.mockReturnValue({ - get: vi.fn(), -}); - useRouter.mockReturnValue({ query: {}, replace: vi.fn(), @@ -29,6 +25,14 @@ const contactFormData = { describe("Contact component", () => { beforeEach(async () => { vi.resetAllMocks(); + useSession.mockReturnValue({ + get: vi.fn(), + data: { + user: { + app_role: "industry_user_admin", + }, + }, + }); }); it("renders the appropriate error component when getContact fails", async () => { diff --git a/bciers/apps/administration/tests/components/contacts/mocks.ts b/bciers/apps/administration/tests/components/contacts/mocks.ts index f64c958ca1..25516beea5 100644 --- a/bciers/apps/administration/tests/components/contacts/mocks.ts +++ b/bciers/apps/administration/tests/components/contacts/mocks.ts @@ -1,6 +1,7 @@ const fetchContactsPageData = vi.fn(); const getContact = vi.fn(); const getUserOperatorUsers = vi.fn(); +const getContacts = vi.fn(); vi.mock( "apps/administration/app/components/contacts/fetchContactsPageData", @@ -20,4 +21,8 @@ vi.mock( }), ); -export { fetchContactsPageData, getContact, getUserOperatorUsers }; +vi.mock("libs/actions/src/api/getContacts", () => ({ + default: getContacts, +})); + +export { fetchContactsPageData, getContact, getUserOperatorUsers, getContacts }; diff --git a/bciers/apps/administration/tests/components/operations/OperationInformationForm.test.tsx b/bciers/apps/administration/tests/components/operations/OperationInformationForm.test.tsx index f168fbad90..a6312d2bdd 100644 --- a/bciers/apps/administration/tests/components/operations/OperationInformationForm.test.tsx +++ b/bciers/apps/administration/tests/components/operations/OperationInformationForm.test.tsx @@ -12,13 +12,13 @@ import { getOperationWithDocuments, getRegulatedProducts, getReportingActivities, - getContacts, } from "./mocks"; import { createAdministrationOperationInformationSchema } from "apps/administration/app/data/jsonSchema/operationInformation/administrationOperationInformation"; import { FrontEndRoles, OperationStatus } from "@bciers/utils/src/enums"; import { expect } from "vitest"; import userEvent from "@testing-library/user-event"; import { RegistrationPurposes } from "@/registration/app/components/operations/registration/enums"; +import { getContacts } from "../contacts/mocks"; useSession.mockReturnValue({ data: { diff --git a/bciers/apps/administration/tests/components/operations/mocks.ts b/bciers/apps/administration/tests/components/operations/mocks.ts index e3ec3ad6c7..f6f1e45ab6 100644 --- a/bciers/apps/administration/tests/components/operations/mocks.ts +++ b/bciers/apps/administration/tests/components/operations/mocks.ts @@ -6,7 +6,6 @@ const getRegulatedProducts = vi.fn(); const getRegistrationPurposes = vi.fn(); const getBusinessStructures = vi.fn(); const fetchOperationsPageData = vi.fn(); -const getContacts = vi.fn(); vi.mock("libs/actions/src/api/getOperation", () => ({ default: getOperation, @@ -35,9 +34,6 @@ vi.mock("libs/actions/src/api/getRegistrationPurposes", () => ({ vi.mock("libs/actions/src/api/getBusinessStructures", () => ({ default: getBusinessStructures, })); -vi.mock("libs/actions/src/api/getContacts", () => ({ - default: getContacts, -})); vi.mock("libs/actions/src/api/fetchOperationsPageData", () => ({ default: fetchOperationsPageData, @@ -52,5 +48,4 @@ export { getRegistrationPurposes, getBusinessStructures, fetchOperationsPageData, - getContacts, }; diff --git a/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.test.tsx b/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.test.tsx index eb8befe60d..0dd3f61a58 100644 --- a/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.test.tsx +++ b/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.test.tsx @@ -62,16 +62,22 @@ const mockFormData = { describe("RJSF PlacesAssignedFieldTemplate", () => { it("should render the field template when there are no places assigned", async () => { render( - , + , ); expect(screen.getByText("None")).toBeVisible(); }); - it("should render the field template when formData is provided", async () => { + it("should render the field template when formData is provided for an external user", async () => { render( , ); expect(screen.getByText("testrole -")).toBeVisible(); @@ -87,6 +93,23 @@ describe("RJSF PlacesAssignedFieldTemplate", () => { ).toBeVisible(); }); + it("should render the field template when formData is provided for an internal user (no note)", async () => { + render( + , + ); + + expect( + screen.queryByText( + "You cannot delete this contact unless you replace them with other contact(s) in the place(s) above.", + ), + ).not.toBeInTheDocument(); + }); + it("should throw an error if given bad formData", async () => { expect(() => render( @@ -94,6 +117,7 @@ describe("RJSF PlacesAssignedFieldTemplate", () => { schema={mockSchema} uiSchema={mockUiSchema} formData={{ places_assigned: ["garbage"] }} + formContext={{ userRole: "industry_user" }} />, ), ).toThrow("Invalid places assigned data"); diff --git a/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.tsx b/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.tsx index 9519c40934..b4758e7032 100644 --- a/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.tsx +++ b/bciers/libs/components/src/form/fields/PlacesAssignedFieldTemplate.tsx @@ -1,36 +1,39 @@ import { ArrayFieldTemplateProps } from "@rjsf/utils"; import Link from "next/link"; -const PlacesAssignedFieldTemplate = ({ items }: ArrayFieldTemplateProps) => { +const PlacesAssignedFieldTemplate = ({ + items, + formContext, +}: ArrayFieldTemplateProps) => { if (items.length < 1) { return
None
; } return (
{items?.map((item) => { - const formData = item.children.props.formData; - if ( - !formData.role_name || - !formData.operation_name || - !formData.operation_id - ) { + // eslint-disable-next-line @typescript-eslint/naming-convention + const { role_name, operation_name, operation_id } = + item.children.props.formData; + if (!role_name || !operation_name || !operation_id) { throw new Error(`Invalid places assigned data`); } return (
- {formData.role_name} -{" "} + {role_name} -{" "} - {formData.operation_name} + {operation_name}
); })} -
- Note: You cannot delete this contact unless you replace them with - other contact(s) in the place(s) above. -
+ {!formContext.userRole.includes("cas") && ( +
+ Note: You cannot delete this contact unless you replace them + with other contact(s) in the place(s) above. +
+ )}
); }; From 5d7983957e97b3eb21a4a04221500549e804c7c7 Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 23 Jan 2025 15:32:17 -0800 Subject: [PATCH 13/13] chore: apply more suggestions from code review --- bc_obps/registration/schema/v2/operation.py | 1 - bc_obps/service/contact_service_v2.py | 8 ++------ bc_obps/service/tests/test_contact_service_v2.py | 4 ++-- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/bc_obps/registration/schema/v2/operation.py b/bc_obps/registration/schema/v2/operation.py index c3d50c2094..1025b50238 100644 --- a/bc_obps/registration/schema/v2/operation.py +++ b/bc_obps/registration/schema/v2/operation.py @@ -194,7 +194,6 @@ class Meta: 'status', 'activities', 'opted_in_operation', - # 'contacts', ] from_attributes = True diff --git a/bc_obps/service/contact_service_v2.py b/bc_obps/service/contact_service_v2.py index b5960505bd..4981d2e609 100644 --- a/bc_obps/service/contact_service_v2.py +++ b/bc_obps/service/contact_service_v2.py @@ -32,13 +32,9 @@ def list_contacts_v2( ) return cast(QuerySet[Contact], queryset) - @classmethod - def get_by_id(cls, contact_id: int) -> Contact: - return Contact.objects.get(id=contact_id) - @classmethod def get_with_places_assigned_v2(cls, contact_id: int) -> Optional[ContactWithPlacesAssigned]: - contact = cls.get_by_id(contact_id) + contact = ContactDataAccessService.get_by_id(contact_id) places_assigned = [] if contact: role_name = contact.business_role.role_name @@ -58,7 +54,7 @@ def get_with_places_assigned_v2(cls, contact_id: int) -> Optional[ContactWithPla @classmethod def raise_exception_if_contact_missing_address_information(cls, contact_id: int) -> None: """This function checks that a contact has a complete address record (contact.address exists and all fields in the address model have a value). In general in the app, address is not mandatory, but in certain cases (e.g., when a contact is assigned to an operation as the Operation Representative), the business area requires the contact to have an address.""" - contact = cls.get_by_id(contact_id) + contact = ContactDataAccessService.get_by_id(contact_id) address = contact.address if not address or any( not getattr(address, field, None) for field in ['street_address', 'municipality', 'province', 'postal_code'] diff --git a/bc_obps/service/tests/test_contact_service_v2.py b/bc_obps/service/tests/test_contact_service_v2.py index cdbb31b9d1..ac18f8e239 100644 --- a/bc_obps/service/tests/test_contact_service_v2.py +++ b/bc_obps/service/tests/test_contact_service_v2.py @@ -47,7 +47,7 @@ def test_get_with_places_assigned_with_contacts(): operation = baker.make_recipe('utils.operation', operator=approved_user_operator.operator) operation.contacts.set([contact]) - result = ContactServiceV2.get_with_places_assigned_v2(approved_user_operator.user.user_guid, contact.id) + result = ContactServiceV2.get_with_places_assigned_v2(contact.id) assert result.places_assigned == [ PlacesAssigned( role_name=contact.business_role.role_name, operation_name=operation.name, operation_id=operation.id @@ -63,7 +63,7 @@ def test_get_with_places_assigned_with_no_contacts(): # add contact to operator (they have to be associated with the operator or will throw unauthorized) approved_user_operator.operator.contacts.set([contact]) - result = ContactServiceV2.get_with_places_assigned_v2(approved_user_operator.user.user_guid, contact.id) + result = ContactServiceV2.get_with_places_assigned_v2(contact.id) assert not hasattr(result, 'places_assigned') @staticmethod