1962 eio form #2738

BCerki · 2025-01-27T18:45:06Z

card: https://github.com/orgs/bcgov/projects/123/views/16?pane=issue&itemId=80445774&issue=bcgov%7Ccas-registration%7C1962

This PR:

conditionally hides non-EIO fields when the registration purpose is EIO
EIO type is only available if purpose is EIO
makes an EIO in the mock data
makes a facility when creating an EIO
^this facility is not accessible via the operation grid
vitests
pytests
testing instructions in the card

BCerki · 2025-01-27T23:06:40Z

bc_obps/registration/schema/v2/operation.py

    registration_purpose: Optional[Operation.Purposes] = None
    regulated_products: Optional[List[int]] = None
-    activities: List[int]
+    activities: Optional[List[int]] = None


This is the only field that isn't already optional, so perhaps it's not worth making a separate EIO schema

BCerki · 2025-01-27T23:07:20Z

bc_obps/service/facility_service.py

+                "This type of operation (SFO or EIO) can only have one facility, this page should not be accessible"
+            )


I added this error for EIO too because I did manage to create a second facility while testing

Instead of get_current_facilities_by_operation can we just say: operation.facilities.count()? In a couple of places, we should change our approach and use the direct operation-facility/operator-operation relationships which are easier to understand and more readable.
Also, RuntimeError is typically used for unexpected runtime issues rather than business logic validation. Since this is a domain-specific constraint, using an Exception or ValueError might be more appropriate.

Sepehr-Sobhani

Looks great!
A couple of nits and comments but not blocker.

Sepehr-Sobhani · 2025-02-06T00:01:28Z

bc_obps/service/facility_service.py

+                "This type of operation (SFO or EIO) can only have one facility, this page should not be accessible"
+            )


Instead of get_current_facilities_by_operation can we just say: operation.facilities.count()? In a couple of places, we should change our approach and use the direct operation-facility/operator-operation relationships which are easier to understand and more readable.
Also, RuntimeError is typically used for unexpected runtime issues rather than business logic validation. Since this is a domain-specific constraint, using an Exception or ValueError might be more appropriate.

Sepehr-Sobhani · 2025-02-06T00:04:52Z

bc_obps/service/operation_service_v2.py

+            eio_payload = FacilityIn(
+                name=payload.name, type=Facility.Types.ELECTRICITY_IMPORT, operation_id=operation.id
+            )
+            facility = FacilityDataAccessService.get_current_facilities_by_operation(operation).first()


Here as well! We could say operation.facilities.first()

Sepehr-Sobhani · 2025-02-06T00:05:44Z

bc_obps/service/operation_service_v2.py

+        if operation.registration_purpose == Operation.Purposes.ELECTRICITY_IMPORT_OPERATION:
+            # EIO operations have a facility with the same data as the operation
+            eio_payload = FacilityIn(
+                name=payload.name, type=Facility.Types.ELECTRICITY_IMPORT, operation_id=operation.id
+            )
+            facility = FacilityDataAccessService.get_current_facilities_by_operation(operation).first()
+
+            if not facility:
+                FacilityService.create_facilities_with_designated_operations(user_guid, [eio_payload])
+            else:
+                FacilityService.update_facility(user_guid, facility.id, eio_payload)
+


Can we move this whole condition into a separate function?!

I'm currently doing work on this function in the optimization PR, so I'll handle this there instead to avoid merge conflicts/accidentally un-optimizing

Sepehr-Sobhani · 2025-02-06T00:09:05Z

bc_obps/service/tests/test_facility_service.py

+        approved_user_operator = baker.make_recipe('utils.approved_user_operator')
+        owning_operation: Operation = baker.make_recipe(
+            'utils.operation', operator=approved_user_operator.operator, type="Linear Facility Operation"


nit: suggest using the absolute path for recipes.(registration.tests.utils....)
For some reason Model Bakery can't find our recipes after adding the RLS app; The only workaround is to use the absolute path.

Yes, I've hit this a few times now, thanks!

Sepehr-Sobhani · 2025-02-06T00:26:37Z

bc_obps/service/tests/test_operation_service_v2.py

+        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()


operation.facilities.all()

Sepehr-Sobhani · 2025-02-06T00:27:42Z

bc_obps/service/tests/test_operation_service_v2.py

        assert operation.status == Operation.Statuses.DRAFT
+        facilities = Facility.objects.filter(designated_operations__operation=operation).all()


Sepehr-Sobhani · 2025-02-06T00:27:56Z

bc_obps/service/tests/test_operation_service_v2.py

@@ -411,6 +472,8 @@ 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()


And here! :)

Sepehr-Sobhani · 2025-02-06T00:43:54Z

bciers/apps/administration/app/data/jsonSchema/operationInformation/operationInformation.ts

+    ...(app === Apps.ADMINISTRATION
+      ? {
+          bc_obps_regulated_operation: {
+            type: "string",
+            title: "BORO ID",
+          },
+        }
+      : {}),
+    ...(app === Apps.ADMINISTRATION
+      ? {
+          bcghg_id: {
+            type: "string",
+            title: "BCGHGID",
+          },
+        }
+      : {}),


Can we say?

...(app === Apps.ADMINISTRATION && { bc_obps_regulated_operation: { type: "string", title: "BORO ID", }, bcghg_id: { type: "string", title: "BCGHGID", }, }),

Sepehr-Sobhani · 2025-02-06T00:47:45Z

bciers/apps/registration/app/components/operations/OperationRegistrationPage.tsx

    ) {
      steps.splice(2, 0, OperationRegistrationSteps.OPT_IN_APPLICATION);
    }
-    if (purpose == RegistrationPurposes.NEW_ENTRANT_OPERATION)
+    if (purpose === RegistrationPurposes.NEW_ENTRANT_OPERATION)


…t Operation

BCerki force-pushed the 1962-eio-form branch from 8151e39 to 82e4127 Compare January 27, 2025 21:53

BCerki commented Jan 27, 2025

View reviewed changes

BCerki force-pushed the 1962-eio-form branch 3 times, most recently from ae1f6d1 to c6baea2 Compare January 29, 2025 17:54

BCerki marked this pull request as ready for review January 29, 2025 17:55

BCerki force-pushed the 1962-eio-form branch from 90d8bd8 to 06a2018 Compare January 29, 2025 18:09

Sepehr-Sobhani approved these changes Feb 6, 2025

View reviewed changes

BCerki added 4 commits February 6, 2025 11:26

chore: add EIO support to models

d7de312

chore: services to set up EIOs

4817815

feat: external user can select Registration Purpose\Electricity Impor…

f3758d2

…t Operation

test: vitest EIOs

e7c28c2

BCerki force-pushed the 1962-eio-form branch from 06a2018 to 6084c81 Compare February 6, 2025 21:32

chore: apply suggestions from code review

4212c2d

BCerki force-pushed the 1962-eio-form branch from 6084c81 to 4212c2d Compare February 6, 2025 22:51

BCerki merged commit ffb6848 into develop Feb 6, 2025
42 checks passed

BCerki deleted the 1962-eio-form branch February 6, 2025 23:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1962 eio form #2738

1962 eio form #2738

BCerki commented Jan 27, 2025 •

edited

Loading

BCerki Jan 27, 2025

BCerki Jan 27, 2025 •

edited

Loading

Sepehr-Sobhani Feb 6, 2025

Sepehr-Sobhani left a comment

Sepehr-Sobhani Feb 6, 2025

Sepehr-Sobhani Feb 6, 2025

Sepehr-Sobhani Feb 6, 2025

BCerki Feb 6, 2025

Sepehr-Sobhani Feb 6, 2025

BCerki Feb 6, 2025

Sepehr-Sobhani Feb 6, 2025

Sepehr-Sobhani Feb 6, 2025

Sepehr-Sobhani Feb 6, 2025

Sepehr-Sobhani Feb 6, 2025

Sepehr-Sobhani Feb 6, 2025

		"This type of operation (SFO or EIO) can only have one facility, this page should not be accessible"
		)

		assert operation.status == Operation.Statuses.DRAFT
		facilities = Facility.objects.filter(designated_operations__operation=operation).all()

1962 eio form #2738

1962 eio form #2738

Conversation

BCerki commented Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

BCerki Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sepehr-Sobhani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BCerki commented Jan 27, 2025 •

edited

Loading

BCerki Jan 27, 2025 •

edited

Loading