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 Jan 28, 2025
1 parent 8f1f47a commit adedb9b
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 8 deletions.
2 changes: 0 additions & 2 deletions bc_obps/registration/models/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,3 @@ def delete(self, *args, **kwargs):
default_storage.delete(self.file.name)

super().delete(*args, **kwargs)

#
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ def test_cas_director_approves_admin_access_request_with_new_operator(self, mock
)

def test_cas_analyst_declines_access_request(self, mocker):
# brianna this will be fixed after rebase
user = baker.make(User)
operator = operator_baker()
operator.status = Operator.Statuses.APPROVED
Expand Down
5 changes: 4 additions & 1 deletion bc_obps/registration/tests/utils/baker_recipes.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from registration.models.bc_greenhouse_gas_id import BcGreenhouseGasId
from registration.models.facility_designated_operation_timeline import FacilityDesignatedOperationTimeline
from registration.models.document import Document
from registration.models.document_type import DocumentType
from registration.models.event.transfer_event import TransferEvent
from registration.models.facility import Facility
from registration.models.multiple_operator import MultipleOperator
Expand All @@ -32,7 +33,9 @@

naics_code = Recipe(NaicsCode, naics_code='486210')
address = Recipe(Address, street_address='Dreary Lane', municipality='Candyland', province='BC', postal_code='HOHOHO')
document = Recipe(Document, file='test.pdf')
document = Recipe(
Document, file='test.pdf', type=DocumentType.objects.get(name='boundary_map')
) # DocumentType records are loaded in the migrations
bcghg_id = Recipe(BcGreenhouseGasId, id='23219990023')
boro_id = Recipe(BcObpsRegulatedOperation, id=seq("99-", start=1001))

Expand Down
1 change: 0 additions & 1 deletion bc_obps/service/data_access_service/document_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ class DocumentDataAccessService:
@classmethod
def get_operation_document_by_type(cls, operation_id: UUID, document_type: str) -> Document | None:
operation = OperationDataAccessService.get_by_id(operation_id=operation_id)

try:
document = operation.documents.get(type=DocumentType.objects.get(name=document_type))
except Document.DoesNotExist:
Expand Down
10 changes: 9 additions & 1 deletion bc_obps/service/document_service_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,17 @@
from registration.models import Document
from registration.utils import files_have_same_hash
from django.core.files.base import ContentFile
from service.operation_service import OperationService


class DocumentServiceV2:
@classmethod
def get_operation_document_by_type_if_authorized(
cls, user_guid: UUID, operation_id: UUID, document_type: str
) -> Document | None:
operation = OperationService.get_if_authorized(user_guid, operation_id)
return DocumentDataAccessService.get_operation_document_by_type(operation.id, document_type)

@classmethod
def create_or_replace_operation_document(
cls, user_guid: UUID, operation_id: UUID, file_data: ContentFile, document_type: str
Expand All @@ -18,7 +26,7 @@ def create_or_replace_operation_document(
This function does NOT set any m2m relationships.
:returns:c Tuple[Document, bool] where the bool is True if a new document was created, False if an existing document was updated
"""
existing_document = DocumentDataAccessService.get_operation_document_by_type(operation_id, document_type)
existing_document = cls.get_operation_document_by_type_if_authorized(user_guid, operation_id, document_type)
# if there is an existing document, check if the new one is different
if existing_document:
# We need to check if the file has changed, if it has, we need to delete the old one and create a new one
Expand Down
18 changes: 18 additions & 0 deletions bc_obps/service/tests/test_document_service_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,24 @@


class TestDocumentServiceV2:
@staticmethod
def test_get_operation_document_by_type_if_authorized():
approved_user_operator = baker.make_recipe('utils.approved_user_operator')
operation = baker.make_recipe('utils.operation', operator=approved_user_operator.operator)
document = baker.make_recipe('utils.document', operation=operation)
retrieved_document = DocumentServiceV2.get_operation_document_by_type_if_authorized(
approved_user_operator.user.user_guid, operation.id, 'boundary_map'
)
assert document.id == retrieved_document.id

@staticmethod
def test_cannot_get_operation_document_by_type_if_unauthorized():
user = baker.make_recipe('utils.industry_operator_user')
operation = baker.make_recipe('utils.operation')
baker.make_recipe('utils.document')
with pytest.raises(Exception, match='Unauthorized.'):
DocumentServiceV2.get_operation_document_by_type_if_authorized(user.user_guid, operation.id, 'boundary_map')

@staticmethod
def test_create_operation_document():
# the value received by the service is a File (transformed into this in the django ninja schema)
Expand Down
1 change: 0 additions & 1 deletion bc_obps/service/user_operator_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ def update_status(
if user_operator.status == UserOperator.Statuses.DECLINED:
# Set role to pending for now but we may want to add a new role for declined
user_operator.role = UserOperator.Roles.PENDING
# brianna you're going to have to go deal with thiss
# hard delete contacts (Senior Officers) associated with the operator and the user who requested access
user_operator.operator.contacts.filter(
created_by=user_operator.user,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ const ContactsDataGrid = ({
const operator = row?.operator__legal_name ? row.operator__legal_name : "";
return row.id + operator;
}
console.log("initialdata", initialData);
return (
<DataGrid
columns={columns}
Expand Down

0 comments on commit adedb9b

Please sign in to comment.