Skip to content

Commit

Permalink
chore: implemented some of the code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
andrea-williams committed Jan 7, 2025
1 parent 67e4544 commit d4070b3
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def _test_reporting_to_opted_in(self):
def _test_reporting_to_obps_regulated(self):
"""
Tests operation registration data for situation where registration_purpose changes from Reporting to OBPS Regulated.
Should have at least one regulated product for OBPS Regulated - no products were required for Reporting..
Should have at least one regulated product for OBPS Regulated - no products were required for Reporting.
"""
assert self.operation.registration_purpose == Operation.Purposes.OBPS_REGULATED_OPERATION
assert self.operation.regulated_products is not None
Expand Down
21 changes: 10 additions & 11 deletions bc_obps/service/operation_service_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,6 @@ def list_current_users_unregistered_operations(
~Q(status=Operation.Statuses.REGISTERED)
)

@classmethod
def check_current_users_registered_operation(cls, operator_id: UUID) -> bool:
"""
Returns True if the userOperator's operator has at least one operation with status 'Registered', False otherwise.
"""
return Operation.objects.filter(operator_id=operator_id, status="Registered").exists()

@classmethod
@transaction.atomic()
def update_status(cls, user_guid: UUID, operation_id: UUID, status: Operation.Statuses) -> Operation:
Expand Down Expand Up @@ -531,13 +524,19 @@ def update_operator(cls, user_guid: UUID, operation: Operation, operator_id: UUI
def handle_change_of_registration_purpose(
cls, user_guid: UUID, original_operation: Operation, payload: OperationInformationIn
) -> OperationInformationIn:
"""
Logic to handle the situation when an industry user changes the selected registration purpose (RP) for their operation.
Changing the RP can happen during or after submitting the operation's registration info.
Depending on what the old RP was, some operation data may need to be removed.
Depending on what the new RP is, some new operation data may need to be added.
Generally, if the operation was already registered when the RP changed, the original data will be archived.
If the operation wasn't yet registered when the selected RP changed, the original data will be deleted.
"""
if original_operation.registration_purpose == Operation.Purposes.OPTED_IN_OPERATION:
payload.opt_in = False
opted_in_detail = original_operation.opted_in_operation
if opted_in_detail and original_operation.status == Operation.Statuses.REGISTERED:
opted_in_detail.set_archive(user_guid)
elif opted_in_detail:
opted_in_detail.delete()
if opted_in_detail:
OperationServiceV2.remove_opted_in_operation_detail(user_guid, original_operation.id)
elif original_operation.registration_purpose == Operation.Purposes.NEW_ENTRANT_OPERATION:
payload.date_of_first_shipment = None
DocumentService.archive_or_delete_operation_document(
Expand Down
11 changes: 11 additions & 0 deletions bc_obps/service/tests/test_document_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,14 @@ def test_archive_or_delete_operation_document(self, registration_status):

assert Document.objects.count() == 1
assert operation.documents.count() == 1

if registration_status == Operation.Statuses.REGISTERED:
"""if the registration has been completed, the document should be archived"""
print(b_map.file)
print(b_map.archived_at, b_map.archived_by)
assert b_map.archived_at is not None
assert b_map.archived_by is not None
elif registration_status == Operation.Statuses.DRAFT:
"""if the registration wasn't completed, the document should be deleted"""
print(b_map.file)
assert b_map is None
6 changes: 3 additions & 3 deletions bc_obps/service/tests/test_operation_service_v2.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from datetime import datetime, timedelta
from unittest.mock import patch, MagicMock
import pytest
from uuid import uuid4
from zoneinfo import ZoneInfo
from registration.schema.v2.operation_timeline import OperationTimelineFilterSchema
Expand All @@ -21,7 +22,6 @@
)
from service.data_access_service.operation_service_v2 import OperationDataAccessServiceV2
from service.operation_service_v2 import OperationServiceV2
import pytest
from registration.models.multiple_operator import MultipleOperator
from registration.schema.v2.multiple_operator import MultipleOperatorIn
from registration.models.operation import Operation
Expand Down Expand Up @@ -168,7 +168,8 @@ def test_remove_opted_in_operation_detail():

assert operation.opt_in is False
assert operation.opted_in_operation is None
assert opted_in_operation_detail is not None
# operation.status is 'Draft', so opted_in_operation_detail should be deleted
assert not OptedInOperationDetail.objects.filter(id=opted_in_operation_detail.id).exists()

@staticmethod
def test_assigning_opted_in_operation_will_create_and_opted_in_operation_detail():
Expand Down Expand Up @@ -971,7 +972,6 @@ def test_raise_exception_if_user_unapproved():

@staticmethod
def test_gets_unfiltered_sorted_list_for_industry_user():

approved_user_operator = baker.make_recipe('utils.approved_user_operator')

baker.make_recipe(
Expand Down

0 comments on commit d4070b3

Please sign in to comment.