From 4212c2d61fe86324b984517deda34c63396ad9c5 Mon Sep 17 00:00:00 2001 From: Brianna Cerkiewicz Date: Thu, 6 Feb 2025 14:50:57 -0800 Subject: [PATCH] chore: apply suggestions from code review --- .../registration/fixtures/mock/operation.json | 28 +------------------ ...alter_historicalfacility_type_and_more.py} | 4 +-- bc_obps/service/facility_service.py | 7 ++--- bc_obps/service/operation_service_v2.py | 3 +- .../service/tests/test_facility_service.py | 2 +- .../tests/test_operation_service_v2.py | 14 ++++------ .../operationInformation.ts | 26 +++++++---------- 7 files changed, 23 insertions(+), 61 deletions(-) rename bc_obps/registration/migrations/{0070_alter_facility_type_alter_historicalfacility_type_and_more.py => 0076_alter_facility_type_alter_historicalfacility_type_and_more.py} (93%) diff --git a/bc_obps/registration/fixtures/mock/operation.json b/bc_obps/registration/fixtures/mock/operation.json index bd11828a73..dbc2ff4d97 100644 --- a/bc_obps/registration/fixtures/mock/operation.json +++ b/bc_obps/registration/fixtures/mock/operation.json @@ -11,7 +11,6 @@ "swrs_facility_id": 1001, "status": "Registered", "bc_obps_regulated_operation": "24-0014", - "created_at": "2024-2-01T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "Reporting Operation", "contacts": [1, 2] @@ -31,7 +30,6 @@ "regulated_products": [1], "status": "Registered", "bc_obps_regulated_operation": "24-0015", - "created_at": "2024-2-02T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "OBPS Regulated Operation", "contacts": [3, 4] @@ -49,7 +47,6 @@ "opt_in": false, "regulated_products": [1], "status": "Draft", - "created_at": "2024-2-02T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "OBPS Regulated Operation" } @@ -69,7 +66,6 @@ "regulated_products": [2, 6, 7, 8], "status": "Registered", "bc_obps_regulated_operation": "23-0001", - "created_at": "2024-1-31T15:27:00.000Z", "activities": [1, 3], "registration_purpose": "OBPS Regulated Operation", "contacts": [3, 4] @@ -90,7 +86,6 @@ "regulated_products": [1], "status": "Registered", "bc_obps_regulated_operation": "23-0002", - "created_at": "2024-1-30T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "OBPS Regulated Operation", "contacts": [1] @@ -112,7 +107,6 @@ "registration_purpose": "OBPS Regulated Operation", "status": "Registered", "bc_obps_regulated_operation": "24-0003", - "created_at": "2024-1-29T15:27:00.000Z", "activities": [1, 5], "operation_has_multiple_operators": true, "contacts": [3] @@ -133,7 +127,6 @@ "regulated_products": [1], "status": "Registered", "bc_obps_regulated_operation": "24-0004", - "created_at": "2024-1-28T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "Opted-in Operation", "contacts": [1] @@ -153,7 +146,6 @@ "bcghg_id": "23219990006", "status": "Registered", "bc_obps_regulated_operation": "24-0005", - "created_at": "2024-1-27T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "Potential Reporting Operation", "contacts": [2] @@ -173,7 +165,6 @@ "bcghg_id": "23219990007", "status": "Registered", "bc_obps_regulated_operation": "24-0006", - "created_at": "2024-1-26T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "Electricity Import Operation", "contacts": [1] @@ -194,7 +185,6 @@ "regulated_products": [2], "status": "Registered", "bc_obps_regulated_operation": "24-0007", - "created_at": "2024-1-25T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "New Entrant Operation", "contacts": [1] @@ -213,7 +203,6 @@ "opt_in": false, "status": "Registered", "bc_obps_regulated_operation": "24-0008", - "created_at": "2024-1-24T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "Reporting Operation", "contacts": [2] @@ -234,7 +223,6 @@ "regulated_products": [3], "status": "Registered", "bc_obps_regulated_operation": "24-0009", - "created_at": "2024-1-23T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "OBPS Regulated Operation", "contacts": [1, 2] @@ -254,7 +242,6 @@ "bcghg_id": "23219990011", "status": "Registered", "bc_obps_regulated_operation": "24-0010", - "created_at": "2024-1-22T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "Reporting Operation", "contacts": [1, 2] @@ -274,7 +261,6 @@ "regulated_products": [3, 4], "status": "Registered", "bc_obps_regulated_operation": "24-0011", - "created_at": "2024-1-21T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "OBPS Regulated Operation", "contacts": [1] @@ -292,7 +278,6 @@ "naics_code": 21, "opt_in": false, "bcghg_id": "23219990013", - "created_at": "2024-1-20T15:27:00.000Z", "status": "Not Started", "registration_purpose": "Electricity Import Operation" } @@ -311,7 +296,6 @@ "bcghg_id": "23219990014", "regulated_products": [3], "status": "Not Started", - "created_at": "2024-1-19T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "OBPS Regulated Operation" } @@ -329,7 +313,6 @@ "opt_in": false, "bcghg_id": "23219990015", "status": "Draft", - "created_at": "2024-1-18T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "Potential Reporting Operation" } @@ -349,7 +332,6 @@ "regulated_products": [5], "status": "Registered", "bc_obps_regulated_operation": "24-0012", - "created_at": "2024-1-17T15:27:00.000Z", "activities": [1, 5], "registration_purpose": "OBPS Regulated Operation", "contacts": [3] @@ -368,7 +350,6 @@ "opt_in": false, "bcghg_id": "23219990017", "status": "Draft", - "created_at": "2024-1-16T15:27:00.000Z", "activities": [1, 5] } }, @@ -388,7 +369,6 @@ "status": "Registered", "bc_obps_regulated_operation": "24-0013", "registration_purpose": "New Entrant Operation", - "created_at": "2024-1-15T15:27:00.000Z", "activities": [1, 3], "contacts": [4] } @@ -407,7 +387,6 @@ "bcghg_id": "23219990019", "status": "Registered", "bc_obps_regulated_operation": "24-0016", - "created_at": "2024-1-14T15:27:00.000Z", "registration_purpose": "Reporting Operation", "activities": [1, 5], "contacts": [3] @@ -426,7 +405,6 @@ "bcghg_id": "23219990020", "status": "Registered", "bc_obps_regulated_operation": "24-0017", - "created_at": "2024-1-13T15:27:00.000Z", "registration_purpose": "Reporting Operation", "activities": [1, 5], "contacts": [3] @@ -445,7 +423,6 @@ "bcghg_id": "23219990021", "status": "Registered", "bc_obps_regulated_operation": "24-0018", - "created_at": "2024-1-12T15:27:00.000Z", "registration_purpose": "Reporting Operation", "activities": [1, 5], "contacts": [4] @@ -457,11 +434,9 @@ "fields": { "point_of_contact": 1, "operator": "4242ea9d-b917-4129-93c2-db00b7451051", - "documents": [], "name": "Blight EIO - Draft", "type": "Electricity Import Operation", - "status": "Draft", - "created_at": "2024-1-11T15:27:00.000Z" + "status": "Draft" } }, { @@ -475,7 +450,6 @@ "naics_code": 21, "opt_in": false, "status": "Draft", - "created_at": "2024-1-10T15:27:00.000Z", "activities": [1, 3] } }, diff --git a/bc_obps/registration/migrations/0070_alter_facility_type_alter_historicalfacility_type_and_more.py b/bc_obps/registration/migrations/0076_alter_facility_type_alter_historicalfacility_type_and_more.py similarity index 93% rename from bc_obps/registration/migrations/0070_alter_facility_type_alter_historicalfacility_type_and_more.py rename to bc_obps/registration/migrations/0076_alter_facility_type_alter_historicalfacility_type_and_more.py index e8e5e7be85..b6f420e2b7 100644 --- a/bc_obps/registration/migrations/0070_alter_facility_type_alter_historicalfacility_type_and_more.py +++ b/bc_obps/registration/migrations/0076_alter_facility_type_alter_historicalfacility_type_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 5.0.11 on 2025-01-28 23:32 +# Generated by Django 5.0.11 on 2025-02-06 19:28 import django.db.models.deletion from django.db import migrations, models @@ -7,7 +7,7 @@ class Migration(migrations.Migration): dependencies = [ - ('registration', '0069_V1_19_0'), + ('registration', '0075_facility_operation_historicalfacility_operation'), ] operations = [ diff --git a/bc_obps/service/facility_service.py b/bc_obps/service/facility_service.py index 3734cc3c6d..24b759ea7a 100644 --- a/bc_obps/service/facility_service.py +++ b/bc_obps/service/facility_service.py @@ -162,10 +162,9 @@ def create_facility_with_designated_operation(cls, user_guid: UUID, payload: Fac operation = OperationDataAccessService.get_by_id(payload.operation_id) cls.check_user_access(user_guid, operation) - # Validate that SFO can only make one facility - num_facilities = FacilityDataAccessService.get_current_facilities_by_operation(operation).count() - if num_facilities > 0 and operation.type != OperationTypes.LFO.value: - raise RuntimeError( + # Validate that SFO and EIO can only have one facility + if operation.facilities.count() > 0 and operation.type != OperationTypes.LFO.value: + raise Exception( "This type of operation (SFO or EIO) can only have one facility, this page should not be accessible" ) diff --git a/bc_obps/service/operation_service_v2.py b/bc_obps/service/operation_service_v2.py index 7fea73370d..2346947dd8 100644 --- a/bc_obps/service/operation_service_v2.py +++ b/bc_obps/service/operation_service_v2.py @@ -4,7 +4,6 @@ from registration.schema.v1.facility import FacilityIn from registration.schema.v2.operation_timeline import OperationTimelineFilterSchema from service.contact_service_v2 import ContactServiceV2 -from service.data_access_service.facility_service import FacilityDataAccessService from service.data_access_service.operation_designated_operator_timeline_service import ( OperationDesignatedOperatorTimelineDataAccessService, ) @@ -329,7 +328,7 @@ def register_operation_information( eio_payload = FacilityIn( name=payload.name, type=Facility.Types.ELECTRICITY_IMPORT, operation_id=operation.id ) - facility = FacilityDataAccessService.get_current_facilities_by_operation(operation).first() + facility = operation.facilities.first() if not facility: FacilityService.create_facilities_with_designated_operations(user_guid, [eio_payload]) diff --git a/bc_obps/service/tests/test_facility_service.py b/bc_obps/service/tests/test_facility_service.py index bdb49ce13f..a4a0c61d7b 100644 --- a/bc_obps/service/tests/test_facility_service.py +++ b/bc_obps/service/tests/test_facility_service.py @@ -181,7 +181,7 @@ def test_create_second_sfo_facility_error(): # test if second facility raises proper exception with pytest.raises( - RuntimeError, + Exception, match=re.escape( "This type of operation (SFO or EIO) can only have one facility, this page should not be accessible" ), diff --git a/bc_obps/service/tests/test_operation_service_v2.py b/bc_obps/service/tests/test_operation_service_v2.py index c241b0d8b5..b40d914f45 100644 --- a/bc_obps/service/tests/test_operation_service_v2.py +++ b/bc_obps/service/tests/test_operation_service_v2.py @@ -361,7 +361,7 @@ def test_create_or_replace_new_entrant_application(): class TestRegisterOperationInformation: @staticmethod def test_register_operation_information_new_eio(): - approved_user_operator = baker.make_recipe('utils.approved_user_operator') + approved_user_operator = baker.make_recipe('registration.tests.utils.approved_user_operator') payload = OperationInformationIn( registration_purpose='Electricity Import Operation', name="TestEIO", @@ -373,13 +373,11 @@ def test_register_operation_information_new_eio(): ) operation.refresh_from_db() assert Operation.objects.count() == 1 - assert operation.created_by == approved_user_operator.user - assert operation.created_at is not None # check purpose and status assert operation.registration_purpose == Operation.Purposes.ELECTRICITY_IMPORT_OPERATION assert operation.status == Operation.Statuses.DRAFT # check facility - facilities = Facility.objects.filter(designated_operations__operation=operation).all() + facilities = operation.facilities.all() assert facilities.count() == 1 assert facilities[0].name == "TestEIO" assert facilities[0].type == Facility.Types.ELECTRICITY_IMPORT @@ -404,13 +402,11 @@ def test_register_operation_information_existing_eio(): ) operation.refresh_from_db() assert Operation.objects.count() == 1 - assert operation.updated_by == approved_user_operator.user - assert operation.updated_at is not None # check purpose and status assert operation.registration_purpose == Operation.Purposes.ELECTRICITY_IMPORT_OPERATION assert operation.status == Operation.Statuses.DRAFT # check facility - facilities = Facility.objects.filter(designated_operations__operation=operation).all() + facilities = operation.facilities.all() assert facilities.count() == 1 assert facilities[0].name == "UpdatedEIO" assert facilities[0].type == Facility.Types.ELECTRICITY_IMPORT @@ -439,7 +435,7 @@ def test_register_operation_information_new_operation(): # check purpose assert operation.registration_purpose == Operation.Purposes.REPORTING_OPERATION assert operation.status == Operation.Statuses.DRAFT - facilities = Facility.objects.filter(designated_operations__operation=operation).all() + facilities = operation.facilities.all() assert facilities.count() == 0 @staticmethod @@ -469,7 +465,7 @@ def test_register_operation_information_existing_operation(): # check purpose assert operation.registration_purpose == Operation.Purposes.POTENTIAL_REPORTING_OPERATION assert operation.status == Operation.Statuses.DRAFT - facilities = Facility.objects.filter(designated_operations__operation=operation).all() + facilities = operation.facilities.all() assert facilities.count() == 0 @staticmethod diff --git a/bciers/apps/administration/app/data/jsonSchema/operationInformation/operationInformation.ts b/bciers/apps/administration/app/data/jsonSchema/operationInformation/operationInformation.ts index 305e204993..45d7c8ca58 100644 --- a/bciers/apps/administration/app/data/jsonSchema/operationInformation/operationInformation.ts +++ b/bciers/apps/administration/app/data/jsonSchema/operationInformation/operationInformation.ts @@ -118,22 +118,16 @@ export const createOperationInformationSchema = async ( title: "Boundary Map", format: "data-url", }, - ...(app === Apps.ADMINISTRATION - ? { - bc_obps_regulated_operation: { - type: "string", - title: "BORO ID", - }, - } - : {}), - ...(app === Apps.ADMINISTRATION - ? { - bcghg_id: { - type: "string", - title: "BCGHGID", - }, - } - : {}), + ...(app === Apps.ADMINISTRATION && { + bc_obps_regulated_operation: { + type: "string", + title: "BORO ID", + }, + bcghg_id: { + type: "string", + title: "BCGHGID", + }, + }), }; return sfoAndLfoSchema; };