Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LTD: Imrove validation during submission #2337

Open
wants to merge 19 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
6bee13f
Add basic validator to validate an application for submission
saruniitr Mar 25, 2024
c3714dd
Update validator to validate end-user details
saruniitr Mar 25, 2024
c7b6edb
Add locations validator for SIEL applications
saruniitr Mar 25, 2024
b7d915f
Add validator to check if security approvals are completed or not
saruniitr Mar 25, 2024
52c7fbd
Merge branch 'dev' into LTD-improve-validation-during-submission
saruniitr May 16, 2024
4bcafc1
Add goods and other validators for SIEL applications
saruniitr May 16, 2024
6609026
Refactor StandardApplication validators functions
saruniitr May 17, 2024
1539921
Fix linter error and raise not implemented error if no validators are…
saruniitr May 17, 2024
889222b
Merge branch 'dev' into LTD-improve-validation-during-submission
saruniitr Jun 4, 2024
a229adb
Delete unused validation helper functions
saruniitr Jun 4, 2024
e7f3dc6
Update test data
saruniitr Jun 5, 2024
2417bf7
Merge branch 'dev' into LTD-improve-validation-during-submission
saruniitr Jun 5, 2024
e71affa
Add/update tests to fix coverage issues
saruniitr Jun 6, 2024
7191da4
Merge branch 'dev' into LTD-improve-validation-during-submission
saruniitr Jun 24, 2024
acc05f5
Merge branch 'dev' into LTD-improve-validation-during-submission
saruniitr Jul 9, 2024
ed49b60
Merge branch 'dev' into LTD-improve-validation-during-submission
saruniitr Dec 9, 2024
b768b39
Move the import into the class to avoid circular imports
saruniitr Dec 9, 2024
cee13c6
Correct an error message
saruniitr Dec 9, 2024
83f21be
Fix sorting issue
saruniitr Dec 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
271 changes: 2 additions & 269 deletions api/applications/creators.py
Original file line number Diff line number Diff line change
@@ -1,40 +1,6 @@
from django.db.models import Q

from api.applications.enums import ApplicationExportType, GoodsTypeCategory
from api.applications.models import (
ApplicationDocument,
GoodOnApplication,
SiteOnApplication,
ExternalLocationOnApplication,
StandardApplication,
)
from api.cases.enums import CaseTypeSubTypeEnum
from api.applications.models import ApplicationDocument
from api.core.helpers import str_to_bool
from api.goods.models import GoodDocument
from lite_content.lite_api import strings
from api.parties.models import PartyDocument
from api.parties.enums import PartyType


def _validate_siel_locations(application, errors):
old_locations_invalid = (
not SiteOnApplication.objects.filter(application=application).exists()
and not ExternalLocationOnApplication.objects.filter(application=application).exists()
and not getattr(application, "have_goods_departed", False)
and not getattr(application, "goodstype_category", None) == GoodsTypeCategory.CRYPTOGRAPHIC
)

new_locations_invalid = (
not getattr(application, "export_type", False)
and not getattr(application, "goods_recipients", False)
and not getattr(application, "goods_starting_point", False)
and getattr(application, "is_shipped_waybill_or_lading") is None
)

if old_locations_invalid and new_locations_invalid:
errors["location"] = [strings.Applications.Generic.NO_LOCATION_SET]

return errors


def _get_document_errors(documents, processing_error, virus_error):
Expand All @@ -49,154 +15,6 @@ def _get_document_errors(documents, processing_error, virus_error):
return virus_error


def check_party_document(party, is_mandatory):
"""
Checks for existence of and status of document (if it is mandatory) and return any errors
"""
documents_qs = PartyDocument.objects.filter(party=party).values_list("safe", flat=True)
if not documents_qs.exists():
# End-user document is mandatory but we are providing an option to not upload
# if there is a valid reason
if party.type == PartyType.END_USER and party.end_user_document_available is False:
return None

if is_mandatory:
return getattr(strings.Applications.Standard, f"NO_{party.type.upper()}_DOCUMENT_SET")
else:
return None

if None in documents_qs:
return build_document_processing_error_message(
get_document_type_description_from_party_type(party_type=party.type)
)
elif False in documents_qs:
return getattr(strings.Applications.Standard, f"{party.type.upper()}_DOCUMENT_INFECTED")

return None


def check_parties_documents(parties, is_mandatory=True):
"""Check a given list of parties all have documents if is_mandatory. Also checks all documents are safe"""

for poa in parties:
error = check_party_document(poa.party, is_mandatory)
if error:
return error
return None


def check_party_error(party, object_not_found_error, is_mandatory, is_document_mandatory=True):
"""Check a given party exists and has a document if is_document_mandatory"""

if is_mandatory and not party:
return object_not_found_error
elif party:
document_error = check_party_document(party, is_document_mandatory)
if document_error:
return document_error


def _validate_end_user(draft, errors, is_mandatory, open_application=False):
"""Validates end user. If a document is mandatory, this is also validated."""

# Document is only mandatory if application is standard permanent or HMRC query
is_document_mandatory = (
draft.case_type.sub_type == CaseTypeSubTypeEnum.STANDARD
and draft.export_type == ApplicationExportType.PERMANENT
) or draft.case_type.sub_type == CaseTypeSubTypeEnum.HMRC

end_user_errors = check_party_error(
draft.end_user.party if draft.end_user else None,
object_not_found_error=strings.Applications.Standard.NO_END_USER_SET,
is_mandatory=is_mandatory,
is_document_mandatory=is_document_mandatory,
)
if end_user_errors:
errors["end_user"] = [end_user_errors]

return errors


def _validate_consignee(draft, errors, is_mandatory):
"""
Checks there is an consignee if goods_recipients is set to VIA_CONSIGNEE or VIA_CONSIGNEE_AND_THIRD_PARTIES
(with a document if is_document_mandatory)
"""
# This logic includes old style applications where the goods_recipients field will be ""
if draft.goods_recipients != StandardApplication.DIRECT_TO_END_USER:
consignee_errors = check_party_error(
draft.consignee.party if draft.consignee else None,
object_not_found_error=strings.Applications.Standard.NO_CONSIGNEE_SET,
is_mandatory=is_mandatory,
is_document_mandatory=False,
)
if consignee_errors:
errors["consignee"] = [consignee_errors]
return errors


def _validate_security_approvals(draft, errors, is_mandatory):
"""Checks there are security approvals for the draft"""
if is_mandatory:
if draft.is_mod_security_approved is None:
errors["security_approvals"] = [
"To submit the application, complete the 'Do you have a security approval?' section"
]
return errors


def _validate_ultimate_end_users(draft, errors, is_mandatory):
"""
Checks all ultimate end users have documents if is_mandatory is True.
Also checks that at least one ultimate_end_user is present if there is an incorporated good
"""
# Document is always optional even if there are incorporated goods
ultimate_end_user_documents_error = check_parties_documents(draft.ultimate_end_users.all(), is_mandatory=False)
if ultimate_end_user_documents_error:
errors["ultimate_end_user_documents"] = [ultimate_end_user_documents_error]

if is_mandatory:
ultimate_end_user_required = GoodOnApplication.objects.filter(
Q(application=draft), Q(is_good_incorporated=True) | Q(is_onward_incorporated=True)
).exists()

if ultimate_end_user_required:
if len(draft.ultimate_end_users.values_list()) == 0:
errors["ultimate_end_users"] = ["To submit the application, add an ultimate end-user"]
else:
# We make sure that an ultimate end user is not also the end user
for ultimate_end_user in draft.ultimate_end_users.values_list("id", flat=True):
if "end_user" not in errors and str(ultimate_end_user) == str(draft.end_user.party.id):
errors["ultimate_end_users"] = [
"To submit the application, an ultimate end-user cannot be the same as the end user"
]

return errors


def _validate_end_use_details(draft, errors, application_type):
if application_type in [CaseTypeSubTypeEnum.STANDARD, CaseTypeSubTypeEnum.OPEN]:
if (
draft.is_military_end_use_controls is None
or draft.is_informed_wmd is None
or draft.is_suspected_wmd is None
or not draft.intended_end_use
) and not getattr(draft, "goodstype_category", None) == GoodsTypeCategory.CRYPTOGRAPHIC:
errors["end_use_details"] = [strings.Applications.Generic.NO_END_USE_DETAILS]

if application_type == CaseTypeSubTypeEnum.STANDARD:
if draft.is_eu_military is None:
errors["end_use_details"] = [strings.Applications.Generic.NO_END_USE_DETAILS]
elif draft.is_eu_military and draft.is_compliant_limitations_eu is None:
errors["end_use_details"] = [strings.Applications.Generic.NO_END_USE_DETAILS]

elif application_type == CaseTypeSubTypeEnum.F680:
if not draft.intended_end_use:
errors["end_use_details"] = [strings.Applications.Generic.NO_END_USE_DETAILS]

return errors


def _validate_agree_to_declaration(request, errors):
"""Checks the exporter has agreed to the T&Cs of the licence"""

Expand All @@ -215,76 +33,6 @@ def _validate_agree_to_declaration(request, errors):
return errors


def _validate_temporary_export_details(draft, errors):
if (
draft.case_type.sub_type in [CaseTypeSubTypeEnum.STANDARD, CaseTypeSubTypeEnum.OPEN]
and draft.export_type == ApplicationExportType.TEMPORARY
):
if not draft.temp_export_details or draft.is_temp_direct_control is None or draft.proposed_return_date is None:
errors["temporary_export_details"] = [strings.Applications.Generic.NO_TEMPORARY_EXPORT_DETAILS]

return errors


def _validate_third_parties(draft, errors, is_mandatory):
"""Checks all third parties have documents if is_mandatory is True"""

third_parties_documents_error = check_parties_documents(draft.third_parties.all(), is_mandatory)
if third_parties_documents_error:
errors["third_parties_documents"] = [third_parties_documents_error]

return errors


def _validate_goods(draft, errors, is_mandatory):
"""Checks Goods"""

goods_on_application = GoodOnApplication.objects.filter(application=draft)

if is_mandatory:
if not goods_on_application:
errors["goods"] = [strings.Applications.Standard.NO_GOODS_SET]

# Check goods documents
if goods_on_application.exists():
goods = goods_on_application.values_list("good", flat=True)
document_errors = _get_document_errors(
GoodDocument.objects.filter(good__in=goods),
processing_error=build_document_processing_error_message("a good"),
virus_error=strings.Applications.Standard.GOODS_DOCUMENT_INFECTED,
)
if document_errors:
errors["goods"] = [document_errors]

return errors


def _validate_standard_licence(draft, errors):
"""Checks that a standard licence has all party types & goods"""

errors = _validate_siel_locations(draft, errors)
errors = _validate_end_user(draft, errors, is_mandatory=True)
errors = _validate_security_approvals(draft, errors, is_mandatory=True)
errors = _validate_consignee(draft, errors, is_mandatory=True)
errors = _validate_third_parties(draft, errors, is_mandatory=False)
errors = _validate_goods(draft, errors, is_mandatory=True)
errors = _validate_ultimate_end_users(draft, errors, is_mandatory=True)
errors = _validate_end_use_details(draft, errors, draft.case_type.sub_type)
errors = _validate_route_of_goods(draft, errors)
errors = _validate_temporary_export_details(draft, errors)

return errors


def _validate_route_of_goods(draft, errors):
if (
draft.is_shipped_waybill_or_lading is None
and not getattr(draft, "goodstype_category", None) == GoodsTypeCategory.CRYPTOGRAPHIC
):
errors["route_of_goods"] = [strings.Applications.Generic.NO_ROUTE_OF_GOODS]
return errors


def _validate_additional_documents(draft, errors):
"""Validate additional documents"""
documents = ApplicationDocument.objects.filter(application=draft)
Expand All @@ -305,26 +53,11 @@ def _validate_additional_documents(draft, errors):
def validate_application_ready_for_submission(application):
errors = {}

# Perform additional validation and append errors if found
if application.case_type.sub_type == CaseTypeSubTypeEnum.STANDARD:
_validate_standard_licence(application, errors)
else:
errors["unsupported_application"] = ["You can only validate a supported application type"]

errors = application.validate()
errors = _validate_additional_documents(application, errors)

return errors


def build_document_processing_error_message(document_type_description):
return f"We are still processing {document_type_description} document. Try submitting again in a few minutes."


def get_document_type_description_from_party_type(party_type):
document_type_description = {
PartyType.CONSIGNEE: "a consignee",
PartyType.END_USER: "an end-user",
PartyType.THIRD_PARTY: "a third party",
PartyType.ULTIMATE_END_USER: "an ultimate end-user",
}
return document_type_description[party_type]
10 changes: 10 additions & 0 deletions api/applications/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,17 @@ def set_appealed(self, appeal, exporter_user):
self.set_sub_status(CaseSubStatusIdEnum.UNDER_APPEAL__APPEAL_RECEIVED)
self.add_to_queue(Queue.objects.get(id=QueuesEnum.LU_APPEALS))

def validate(self):
raise NotImplementedError("Validator to validate application attributes is not implemented")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice


def create_amendment(self, user):
raise NotImplementedError()


# Licence Applications
class StandardApplication(BaseApplication, Clonable):
from api.applications.validators import StandardApplicationValidator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this import need to be here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually added this to avoid circular dependencies but they are not there atm, I will move this outside of this.


GB = "GB"
NI = "NI"
GOODS_STARTING_POINT_CHOICES = [
Expand All @@ -289,6 +294,7 @@ class StandardApplication(BaseApplication, Clonable):
(VIA_CONSIGNEE, "To an end-user via a consignee"),
(VIA_CONSIGNEE_AND_THIRD_PARTIES, "To an end-user via a consignee, with additional third parties"),
]
validator_class = StandardApplicationValidator

export_type = models.TextField(choices=ApplicationExportType.choices, blank=True, default="")
reference_number_on_information_form = models.CharField(blank=True, null=True, max_length=255)
Expand Down Expand Up @@ -422,6 +428,10 @@ def create_amendment(self, user):
self.case_ptr.change_status(system_user, get_case_status_by_status(CaseStatusEnum.SUPERSEDED_BY_EXPORTER_EDIT))
return amendment_application

def validate(self):
validator = self.validator_class(self)
return validator.validate()


class ApplicationDocument(Document, Clonable):
application = models.ForeignKey(BaseApplication, on_delete=models.CASCADE)
Expand Down
2 changes: 1 addition & 1 deletion api/applications/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
from api.documents.tests.factories import DocumentFactory
from api.staticdata.statuses.models import CaseStatus
from api.goods.tests.factories import GoodFactory
from api.organisations.tests.factories import OrganisationFactory, SiteFactory, ExternalLocationFactory
from api.parties.tests.factories import ConsigneeFactory, EndUserFactory, PartyFactory, ThirdPartyFactory
from api.organisations.tests.factories import OrganisationFactory, SiteFactory, ExternalLocationFactory
from api.users.tests.factories import ExporterUserFactory, GovUserFactory
from api.staticdata.units.enums import Units
from api.staticdata.control_list_entries.helpers import get_control_list_entry
Expand Down
6 changes: 3 additions & 3 deletions api/applications/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
GoodOnApplicationFactory,
PartyOnApplicationFactory,
)
from api.users.models import GovUser, ExporterUser
from api.users.models import BaseUser, GovUser, ExporterUser
from api.goods.tests.factories import FirearmFactory
from api.organisations.tests.factories import OrganisationFactory
from api.staticdata.control_list_entries.models import ControlListEntry
Expand All @@ -43,7 +43,6 @@
CaseStatusEnum,
)
from api.users.enums import SystemUser
from api.users.models import ExporterUser
from api.users.tests.factories import BaseUserFactory


Expand Down Expand Up @@ -603,7 +602,8 @@ def test_clone_with_party_override(self):
@pytest.mark.requires_transactions
class TestStandardApplicationRaceConditions(TransactionTestCase):
def test_create_amendment_race_condition_success(self):
BaseUserFactory(id=SystemUser.id)
if not BaseUser.objects.filter(id=SystemUser.id).exists():
BaseUserFactory(id=SystemUser.id)

original_application = StandardApplicationFactory()

Expand Down
Loading
Loading