Skip to content

Commit

Permalink
chore: apply suggestions from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
BCerki committed Feb 6, 2025
1 parent e7c28c2 commit 4212c2d
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 61 deletions.
28 changes: 1 addition & 27 deletions bc_obps/registration/fixtures/mock/operation.json
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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"
}
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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"
}
Expand All @@ -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"
}
Expand All @@ -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"
}
Expand All @@ -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]
Expand All @@ -368,7 +350,6 @@
"opt_in": false,
"bcghg_id": "23219990017",
"status": "Draft",
"created_at": "2024-1-16T15:27:00.000Z",
"activities": [1, 5]
}
},
Expand All @@ -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]
}
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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"
}
},
{
Expand All @@ -475,7 +450,6 @@
"naics_code": 21,
"opt_in": false,
"status": "Draft",
"created_at": "2024-1-10T15:27:00.000Z",
"activities": [1, 3]
}
},
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -7,7 +7,7 @@
class Migration(migrations.Migration):

dependencies = [
('registration', '0069_V1_19_0'),
('registration', '0075_facility_operation_historicalfacility_operation'),
]

operations = [
Expand Down
7 changes: 3 additions & 4 deletions bc_obps/service/facility_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down
3 changes: 1 addition & 2 deletions bc_obps/service/operation_service_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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])
Expand Down
2 changes: 1 addition & 1 deletion bc_obps/service/tests/test_facility_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
),
Expand Down
14 changes: 5 additions & 9 deletions bc_obps/service/tests/test_operation_service_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down

0 comments on commit 4212c2d

Please sign in to comment.