diff --git a/bc_obps/registration/schema/v2/operation.py b/bc_obps/registration/schema/v2/operation.py index c62fc3f412..e66f4b5857 100644 --- a/bc_obps/registration/schema/v2/operation.py +++ b/bc_obps/registration/schema/v2/operation.py @@ -80,8 +80,8 @@ class OperationInformationIn(ModelSchema): registration_purpose: Optional[Operation.Purposes] = None regulated_products: Optional[List[int]] = None activities: List[int] - boundary_map: str - process_flow_diagram: str + boundary_map: Optional[str] = None + process_flow_diagram: Optional[str] = None naics_code_id: int opt_in: Optional[bool] = False secondary_naics_code_id: Optional[int] = None diff --git a/bc_obps/registration/tests/integration/test_changing_registration_purpose.py b/bc_obps/registration/tests/integration/test_changing_registration_purpose.py index 50510fc5f8..148bf60031 100644 --- a/bc_obps/registration/tests/integration/test_changing_registration_purpose.py +++ b/bc_obps/registration/tests/integration/test_changing_registration_purpose.py @@ -1,4 +1,5 @@ import pytest +from service.operation_service_v2 import OperationServiceV2 from service.data_access_service.document_service import DocumentDataAccessService from registration.enums.enums import OperationTypes from model_bakery import baker @@ -129,6 +130,12 @@ def _set_operation_representative(self): self.operation_representative_id = response.json()['id'] self.operation.refresh_from_db() + def _set_operation_status(self, status): + # manually setting the operation status this way because normally industry users don't have control + # over the status of their operation + self.operation.status = status + self.operation.refresh_from_db() + def _set_opted_in_operation_detail(self): response = TestUtils.mock_put_with_auth_role( self, @@ -194,8 +201,7 @@ def _test_reporting_to_eio(self): assert self.operation.registration_purpose == Operation.Purposes.ELECTRICITY_IMPORT_OPERATION assert self.operation.activities.count() == 0 assert self.operation.regulated_products.count() == 0 - assert DocumentDataAccessService.get_active_operation_documents(self.operation.id).count() == 0 - assert self.operation.facilities.count() == 1 + assert self.operation.documents.count() == 0 assert self.operation.naics_code is None def _test_reporting_to_new_entrant(self): @@ -301,7 +307,7 @@ def _test_regulated_to_eio(self): """ assert self.operation.registration_purpose == Operation.Purposes.ELECTRICITY_IMPORT_OPERATION assert self.operation.activities.count() == 0 - assert DocumentDataAccessService.get_active_operation_documents(self.operation.id).count() == 0 + assert self.operation.documents.count() == 0 assert self.operation.naics_code is None ### Tests for Original Purpose = Opted-in @@ -313,8 +319,11 @@ def _test_opted_in_to_reporting(self): """ assert self.operation.registration_purpose == Operation.Purposes.REPORTING_OPERATION assert self.operation.opt_in is False - assert self.operation.opted_in_operation.archived_at is not None - assert self.operation.opted_in_operation.archived_by is not None + if self.operation.status == Operation.Statuses.REGISTERED: + assert self.operation.opted_in_operation.archived_at is not None + assert self.operation.opted_in_operation.archived_by is not None + else: + assert self.operation.opted_in_operation is None def _test_opted_in_to_potential_reporting(self): """ @@ -323,8 +332,11 @@ def _test_opted_in_to_potential_reporting(self): """ assert self.operation.registration_purpose == Operation.Purposes.POTENTIAL_REPORTING_OPERATION assert self.operation.opt_in is False - assert self.operation.opted_in_operation.archived_at is not None - assert self.operation.opted_in_operation.archived_by is not None + if self.operation.status == Operation.Statuses.REGISTERED: + assert self.operation.opted_in_operation.archived_at is not None + assert self.operation.opted_in_operation.archived_by is not None + else: + assert self.operation.opted_in_operation is None def _test_opted_in_to_regulated(self): """ @@ -333,8 +345,11 @@ def _test_opted_in_to_regulated(self): """ assert self.operation.registration_purpose == Operation.Purposes.OBPS_REGULATED_OPERATION assert self.operation.opt_in is False - assert self.operation.opted_in_operation.archived_at is not None - assert self.operation.opted_in_operation.archived_by is not None + if self.operation.status == Operation.Statuses.REGISTERED: + assert self.operation.opted_in_operation.archived_at is not None + assert self.operation.opted_in_operation.archived_by is not None + else: + assert self.operation.opted_in_operation is None def _test_opted_in_to_eio(self): """ @@ -343,11 +358,14 @@ def _test_opted_in_to_eio(self): """ assert self.operation.registration_purpose == Operation.Purposes.ELECTRICITY_IMPORT_OPERATION assert self.operation.opt_in is False - assert self.operation.opted_in_operation.archived_at is not None - assert self.operation.opted_in_operation.archived_by is not None + if self.operation.status == Operation.Statuses.REGISTERED: + assert self.operation.opted_in_operation.archived_at is not None + assert self.operation.opted_in_operation.archived_by is not None + else: + assert self.operation.opted_in_operation is None assert self.operation.regulated_products.count() == 0 assert self.operation.naics_code is None - assert DocumentDataAccessService.get_active_operation_documents(self.operation.id).count() == 0 + assert self.operation.documents.count() == 0 def _test_opted_in_to_new_entrant(self): """ @@ -356,8 +374,11 @@ def _test_opted_in_to_new_entrant(self): """ assert self.operation.registration_purpose == Operation.Purposes.NEW_ENTRANT_OPERATION assert self.operation.opt_in is False - assert self.operation.opted_in_operation.archived_at is not None - assert self.operation.opted_in_operation.archived_by is not None + if self.operation.status == Operation.Statuses.REGISTERED: + assert self.operation.opted_in_operation.archived_at is not None + assert self.operation.opted_in_operation.archived_by is not None + else: + assert self.operation.opted_in_operation is None assert self.operation.date_of_first_shipment is not None assert ( self.operation.documents.filter(type=DocumentType.objects.get(name='new_entrant_application')).count() > 0 @@ -402,7 +423,6 @@ def _test_new_entrant_to_eio(self): In addition, data for process flow diagram, boundary map, regulated products, and reporting activities should be removed. """ assert self.operation.registration_purpose == Operation.Purposes.ELECTRICITY_IMPORT_OPERATION - assert DocumentDataAccessService.get_active_operation_documents(self.operation.id).count() == 0 assert self.operation.documents.filter(type=DocumentType.objects.get(name="process_flow_diagram")).count() == 0 assert self.operation.documents.filter(type=DocumentType.objects.get(name="boundary_map")).count() == 0 assert ( @@ -576,15 +596,18 @@ def _test_potential_reporting_to_eio(self): """ assert self.operation.registration_purpose == Operation.Purposes.ELECTRICITY_IMPORT_OPERATION assert self.operation.activities.count() == 0 - assert DocumentDataAccessService.get_active_operation_documents(self.operation.id).count() == 0 - assert self.operation.facilities.count() == 1 + assert self.operation.documents.count() == 0 assert self.operation.naics_code is None @pytest.mark.parametrize("original_purpose", list(Operation.Purposes)) @pytest.mark.parametrize("new_purpose", list(Operation.Purposes)) @pytest.mark.parametrize("operation_type", [OperationTypes.SFO.value, OperationTypes.LFO.value]) - # @pytest.mark.parametrize("was_submitted", ["Submitted", "Not Submitted"]) - def test_changing_registration_purpose(self, original_purpose, new_purpose, operation_type): + @pytest.mark.parametrize("registration_status", [Operation.Statuses.REGISTERED, Operation.Statuses.DRAFT]) + def test_changing_registration_purpose(self, original_purpose, new_purpose, operation_type, registration_status): + """ + If the registration_status == Registered, the irrelevant data should be archived + If the registration_status == Draft, the irrelevant data should be deleted + """ if original_purpose == new_purpose: pytest.skip() @@ -597,10 +620,7 @@ def test_changing_registration_purpose(self, original_purpose, new_purpose, oper self._set_new_entrant_info() self._set_facilities() self._set_operation_representative() - - ### mock registration submission if was_submitted == True - # if was_submitted == "Submitted": - # self._set_registration_submission() + self._set_operation_status(registration_status) # make a copy of the original operation record to compare against later self.original_operation_record = deepcopy(self.operation) @@ -612,9 +632,6 @@ def test_changing_registration_purpose(self, original_purpose, new_purpose, oper elif new_purpose == Operation.Purposes.NEW_ENTRANT_OPERATION: self._set_new_entrant_info() - # if the registration was_submitted == True, the irrelevant data should be archived - # if the registration wasn't submitted, (was_submitted == False), the irrelevant data should be deleted - ### Assertions applicable to all purposes # assert that we're patching the same operation, not creating a new one assert self.operation.id == self.original_operation_record.id diff --git a/bc_obps/service/data_access_service/document_service.py b/bc_obps/service/data_access_service/document_service.py index 11aea4fe20..f00305729d 100644 --- a/bc_obps/service/data_access_service/document_service.py +++ b/bc_obps/service/data_access_service/document_service.py @@ -17,12 +17,6 @@ def get_operation_document_by_type(cls, operation_id: UUID, document_type: str) return document - @classmethod - def get_active_operation_documents(cls, operation_id: UUID) -> List[Document]: - operation = OperationDataAccessService.get_by_id(operation_id=operation_id) - - return operation.documents.filter(archived_at__isnull=True) - @classmethod def create_document(cls, user_guid: UUID, file_data: Optional[ContentFile], document_type_name: str) -> Document: document = Document.objects.create( diff --git a/bc_obps/service/document_service.py b/bc_obps/service/document_service.py index 8063f1fb29..f964697839 100644 --- a/bc_obps/service/document_service.py +++ b/bc_obps/service/document_service.py @@ -36,16 +36,20 @@ def create_or_replace_operation_document( return document, True @classmethod - def archive_operation_document(cls, user_guid: UUID, operation_id: UUID, document_type: str) -> bool: + def archive_or_delete_operation_document(cls, user_guid: UUID, operation_id: UUID, document_type: str) -> bool: """ This function receives an operation ID and document type. - If the specified document_type for the operation_id can be found and hasn't already been archived, this + If the operation's status != "Registered", the specified document will be deleted. + If the operation's status == "Registered", and the specified document_type for the operation_id can be found, this function will archive that document. - :returns: bool to indicate whether the document was successfully archived. + :returns: bool to indicate whether the document was successfully archived or deleted. """ + operation = OperationDataAccessService.get_by_id(operation_id) document = DocumentDataAccessService.get_operation_document_by_type(operation_id, document_type) - if document: - if document.archived_at is None and document.archived_by is None: - document.set_archive(user_guid) - return True + if document and operation.status == Operation.Statuses.REGISTERED: + document.set_archive(user_guid) + return True + elif document: + document.delete() + return True return False diff --git a/bc_obps/service/operation_service_v2.py b/bc_obps/service/operation_service_v2.py index 15a2e1afc3..040130a604 100644 --- a/bc_obps/service/operation_service_v2.py +++ b/bc_obps/service/operation_service_v2.py @@ -197,9 +197,8 @@ def create_or_update_operation_v2( # will need to retrieve operation as it exists currently in DB first, to determine whether there's been a change to the RP if operation_id: - original_operation = Operation.objects.get(id=operation_id) + original_operation = OperationService.get_if_authorized(user_guid, operation_id) if payload.registration_purpose != original_operation.registration_purpose: - print('New RP detected!!!') payload = cls.handle_change_of_registration_purpose(user_guid, original_operation, payload) print(payload) @@ -230,17 +229,29 @@ def create_or_update_operation_v2( operation_documents = [ doc for doc, created in [ - DocumentService.create_or_replace_operation_document( - user_guid, - operation.id, - payload.boundary_map, # type: ignore # mypy is not aware of the schema validator - 'boundary_map', + *( + [ + DocumentService.create_or_replace_operation_document( + user_guid, + operation.id, + payload.boundary_map, # type: ignore # mypy is not aware of the schema validator + 'boundary_map', + ) + ] + if payload.boundary_map + else [] ), - DocumentService.create_or_replace_operation_document( - user_guid, - operation.id, - payload.process_flow_diagram, # type: ignore # mypy is not aware of the schema validator - 'process_flow_diagram', + *( + [ + DocumentService.create_or_replace_operation_document( + user_guid, + operation.id, + payload.process_flow_diagram, # type: ignore # mypy is not aware of the schema validator + 'process_flow_diagram', + ) + ] + if payload.process_flow_diagram + else [] ), *( [ @@ -431,12 +442,15 @@ def check_conditions() -> Generator[Tuple[Callable[[], bool], str], None, None]: "Operation must have at least one reporting activity.", ) - # Check if the operation has both a process flow diagram and a boundary map + # Check if the operation has both a process flow diagram and a boundary map (unless it is an EIO) yield ( - lambda: operation.documents.filter(Q(type__name='process_flow_diagram') | Q(type__name='boundary_map')) - .distinct() - .count() - == 2, + lambda: ( + operation.registration_purpose != Operation.Purposes.ELECTRICITY_IMPORT_OPERATION + and operation.documents.filter(Q(type__name='process_flow_diagram') | Q(type__name='boundary_map')) + .distinct() + .count() + == 2 + ), "Operation must have a process flow diagram and a boundary map.", ) yield ( @@ -497,10 +511,15 @@ def handle_change_of_registration_purpose( if original_operation.registration_purpose == Operation.Purposes.OPTED_IN_OPERATION: payload.opt_in = False opted_in_detail: OptedInOperationDetail = original_operation.opted_in_operation - opted_in_detail.set_archive(user_guid) + if original_operation.status == Operation.Statuses.REGISTERED: + opted_in_detail.set_archive(user_guid) + else: + opted_in_detail.delete() elif original_operation.registration_purpose == Operation.Purposes.NEW_ENTRANT_OPERATION: payload.date_of_first_shipment = None - DocumentService.archive_operation_document(user_guid, original_operation.id, 'new_entrant_application') + DocumentService.archive_or_delete_operation_document( + user_guid, original_operation.id, 'new_entrant_application' + ) if payload.registration_purpose == Operation.Purposes.ELECTRICITY_IMPORT_OPERATION: payload.activities = [] @@ -508,8 +527,20 @@ def handle_change_of_registration_purpose( payload.naics_code_id = None payload.secondary_naics_code_id = None payload.tertiary_naics_code_id = None - DocumentService.archive_operation_document(user_guid, original_operation.id, 'process_flow_diagram') - DocumentService.archive_operation_document(user_guid, original_operation.id, 'boundary_map') + payload.boundary_map = None + payload.process_flow_diagram = None + if original_operation.status == Operation.Statuses.REGISTERED: + DocumentService.archive_or_delete_operation_document( + user_guid, original_operation.id, 'process_flow_diagram' + ) + DocumentService.archive_or_delete_operation_document(user_guid, original_operation.id, 'boundary_map') + else: + boundary_map = original_operation.get_boundary_map() + if boundary_map: + boundary_map.delete() + process_flow_diagram = original_operation.get_process_flow_diagram() + if process_flow_diagram: + process_flow_diagram.delete() elif payload.registration_purpose in [ Operation.Purposes.REPORTING_OPERATION, Operation.Purposes.POTENTIAL_REPORTING_OPERATION, diff --git a/bc_obps/service/tests/test_document_service.py b/bc_obps/service/tests/test_document_service.py index 937574a883..000c91de5c 100644 --- a/bc_obps/service/tests/test_document_service.py +++ b/bc_obps/service/tests/test_document_service.py @@ -1,6 +1,7 @@ from service.data_access_service.document_service import DocumentDataAccessService from registration.utils import data_url_to_file from registration.models.document import Document +from registration.models.operation import Operation from registration.tests.constants import MOCK_DATA_URL, MOCK_DATA_URL_2 from service.document_service import DocumentService import pytest @@ -67,26 +68,26 @@ def test_update_operation_document(): assert document.file.name.find("test") != -1 assert created is True - @staticmethod - def test_get_active_documents(): + @pytest.mark.parametrize("registration_status", [Operation.Statuses.REGISTERED, Operation.Statuses.DRAFT]) + def test_archive_or_delete_operation_document(self, registration_status): approved_user_operator = baker.make_recipe('utils.approved_user_operator') - operation = baker.make_recipe('utils.operation', operator=approved_user_operator.operator) + operation = baker.make_recipe( + 'utils.operation', operator=approved_user_operator.operator, status=registration_status + ) b_map = DocumentDataAccessService.create_document( approved_user_operator.user_id, data_url_to_file(MOCK_DATA_URL), 'boundary_map' ) pfd = DocumentDataAccessService.create_document( approved_user_operator.user_id, data_url_to_file(MOCK_DATA_URL), 'process_flow_diagram' ) - nea = DocumentDataAccessService.create_document( - approved_user_operator.user_id, data_url_to_file(MOCK_DATA_URL), 'new_entrant_application' - ) - operation.documents.set([b_map.id, pfd.id, nea.id]) + operation.documents.set([b_map.id, pfd.id]) - assert Document.objects.count() == 3 + assert Document.objects.count() == 2 + assert operation.documents.count() == 2 - DocumentService.archive_operation_document( - approved_user_operator.user_id, operation.id, 'new_entrant_application' + DocumentService.archive_or_delete_operation_document( + approved_user_operator.user_id, operation.id, 'boundary_map' ) - assert Document.objects.count() == 2 - assert DocumentDataAccessService.get_active_operation_documents(operation.id).count() == 2 + assert Document.objects.count() == 1 + assert operation.documents.count() == 1