-
Notifications
You must be signed in to change notification settings - Fork 2
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 5681 update licence decision refusal criteria #2333
base: dev
Are you sure you want to change the base?
Changes from all commits
1333da2
ff49396
1455428
1cbd2f5
96a0c55
f6417f7
b0f1515
5c35e70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
# Generated by Django 4.2.16 on 2024-12-03 14:48 | ||
|
||
from django.db import migrations | ||
|
||
from api.audit_trail.enums import AuditType | ||
from api.cases.enums import ( | ||
AdviceLevel, | ||
AdviceType, | ||
) | ||
|
||
|
||
LICENSING_UNIT_ID = "58e77e47-42c8-499f-a58d-94f94541f8c6" | ||
|
||
|
||
def update_licencedecision_denial_reasons(apps, schema_editor): | ||
LicenceDecision = apps.get_model("cases", "LicenceDecision") | ||
Advice = apps.get_model("cases", "Advice") | ||
|
||
denial_reasons = ( | ||
Advice.objects.filter( | ||
case__licence_decisions__decision="refused", | ||
level=AdviceLevel.FINAL, | ||
team_id=LICENSING_UNIT_ID, # Just care about LU advice | ||
type=AdviceType.REFUSE, | ||
) | ||
.only("denial_reasons__id", "case__licence_decisions__id") | ||
.values_list("denial_reasons__id", "case__licence_decisions__id") | ||
# The AdviceManager orders by `created_at` and this affects the distinct | ||
# so we remove the ordering completely to ensure the distinct workds as | ||
# expected | ||
.order_by() | ||
.distinct() | ||
) | ||
|
||
updated_cases = set() | ||
for denial_reason, licence_decision_id in denial_reasons: | ||
licence_decision = LicenceDecision.objects.get(pk=licence_decision_id) | ||
licence_decision.denial_reasons.add(denial_reason) | ||
updated_cases.add(licence_decision.case.pk) | ||
|
||
Audit = apps.get_model("audit_trail", "Audit") | ||
Case = apps.get_model("cases", "Case") | ||
ContentType = apps.get_model("contenttypes", "ContentType") | ||
case_content_type = ContentType.objects.get_for_model(Case) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's better to use Directly importing models in migrations is flaky and in a few migrations time will probably fail because during migrations the code in models.py is out of step with the database schema: the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed with this |
||
|
||
refusal_criteria_audits = Audit.objects.exclude( | ||
target_content_type_id=case_content_type.pk, | ||
target_object_id__in=updated_cases, | ||
).filter(verb=AuditType.CREATE_REFUSAL_CRITERIA) | ||
for audit in refusal_criteria_audits: | ||
case = Case.objects.get(pk=audit.target_object_id) | ||
refusal_licence_decisions = case.licence_decisions.filter(decision="refused") | ||
if not refusal_licence_decisions.exists(): | ||
continue | ||
for licence_decision in refusal_licence_decisions: | ||
denial_reasons = audit.payload["additional_text"].replace(".", "").replace(" ", "").split(",") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These need to be valid There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It hasn't failed when running on the production-like data so far so I've not worried about this. |
||
licence_decision.denial_reasons.set(denial_reasons) | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
("cases", "0073_licencedecision_denial_reasons"), | ||
] | ||
|
||
operations = [ | ||
migrations.RunPython( | ||
update_licencedecision_denial_reasons, | ||
migrations.RunPython.noop, | ||
), | ||
] |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
import pytest | ||
|
||
from api.applications.tests.factories import StandardApplicationFactory | ||
from api.audit_trail.enums import AuditType | ||
from api.audit_trail.tests.factories import AuditFactory | ||
from api.cases.enums import ( | ||
AdviceType, | ||
LicenceDecisionType, | ||
) | ||
from api.cases.tests.factories import ( | ||
FinalAdviceFactory, | ||
LicenceDecisionFactory, | ||
TeamAdviceFactory, | ||
UserAdviceFactory, | ||
) | ||
from api.users.tests.factories import ( | ||
GovUserFactory, | ||
RoleFactory, | ||
) | ||
|
||
|
||
@pytest.mark.django_db() | ||
def test_attach_licence_to_licence_decisions(migrator): | ||
migrator.apply_initial_migration(("cases", "0073_licencedecision_denial_reasons")) | ||
|
||
LICENSING_UNIT_ID = "58e77e47-42c8-499f-a58d-94f94541f8c6" | ||
FCDO_ID = "67b9a4a3-6f3d-4511-8a19-23ccff221a74" | ||
|
||
RoleFactory(id="00000000-0000-0000-0000-000000000001") | ||
lu_user = GovUserFactory(team_id=LICENSING_UNIT_ID) | ||
fcdo_user = GovUserFactory(team_id=FCDO_ID) | ||
|
||
refused_application = StandardApplicationFactory() | ||
refused_licence_decision = LicenceDecisionFactory( | ||
case=refused_application, | ||
decision=LicenceDecisionType.REFUSED, | ||
) | ||
UserAdviceFactory( | ||
case=refused_application, | ||
denial_reasons=["1"], | ||
team_id=FCDO_ID, | ||
type=AdviceType.REFUSE, | ||
user=fcdo_user, | ||
) | ||
UserAdviceFactory( | ||
case=refused_application, | ||
denial_reasons=["2"], | ||
team_id=LICENSING_UNIT_ID, | ||
type=AdviceType.REFUSE, | ||
user=lu_user, | ||
) | ||
TeamAdviceFactory( | ||
case=refused_application, | ||
denial_reasons=["3"], | ||
team_id=FCDO_ID, | ||
type=AdviceType.REFUSE, | ||
user=fcdo_user, | ||
) | ||
TeamAdviceFactory( | ||
case=refused_application, | ||
denial_reasons=["4"], | ||
team_id=LICENSING_UNIT_ID, | ||
type=AdviceType.REFUSE, | ||
user=lu_user, | ||
) | ||
FinalAdviceFactory( | ||
case=refused_application, | ||
denial_reasons=["5"], | ||
team_id=LICENSING_UNIT_ID, | ||
type=AdviceType.REFUSE, | ||
user=lu_user, | ||
) | ||
assert list(refused_licence_decision.denial_reasons.values_list("id", flat=True)) == [] | ||
|
||
migrator.apply_tested_migration(("cases", "0074_update_licence_decision_denial_reasons")) | ||
refused_licence_decision.refresh_from_db() | ||
assert list(refused_licence_decision.denial_reasons.values_list("id", flat=True)) == ["5"] | ||
|
||
|
||
@pytest.mark.django_db() | ||
def test_attach_licence_to_licence_decisions_without_denial_reasons_on_advice(migrator): | ||
old_state = migrator.apply_initial_migration(("cases", "0073_licencedecision_denial_reasons")) | ||
|
||
refused_application = StandardApplicationFactory() | ||
refused_licence_decision = LicenceDecisionFactory( | ||
case=refused_application, | ||
decision=LicenceDecisionType.REFUSED, | ||
) | ||
assert list(refused_licence_decision.denial_reasons.values_list("id", flat=True)) == [] | ||
assert not refused_application.advice.exists() | ||
|
||
Case = old_state.apps.get_model("cases", "Case") | ||
ContentType = old_state.apps.get_model("contenttypes", "ContentType") | ||
case_content_type = ContentType.objects.get_for_model(Case) | ||
|
||
AuditFactory( | ||
payload={ | ||
"additional_text": "1, 5, 7.", | ||
}, | ||
target_content_type_id=case_content_type.pk, | ||
target_object_id=refused_application.pk, | ||
verb=AuditType.CREATE_REFUSAL_CRITERIA, | ||
) | ||
|
||
migrator.apply_tested_migration(("cases", "0074_update_licence_decision_denial_reasons")) | ||
refused_licence_decision.refresh_from_db() | ||
assert list(refused_licence_decision.denial_reasons.values_list("id", flat=True)) == ["1", "5", "7"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
import factory | ||
|
||
|
||
def get_model_for_factory(factory_class, apps): | ||
model_meta = factory_class._meta.model._meta | ||
|
||
return apps.get_model(model_meta.app_label, model_meta.object_name) | ||
|
||
|
||
def fullname(cls): | ||
return f"{cls.__module__}.{cls.__name__}" | ||
|
||
|
||
def generate_factory(apps, factory_class, **defaults): | ||
for field_name, field_value in factory_class._meta.declarations.items(): | ||
if field_name in defaults: | ||
continue | ||
if not isinstance(field_value, factory.declarations.SubFactory): | ||
continue | ||
generated_factory = generate_factory( | ||
apps, | ||
field_value.get_factory(), | ||
**field_value.defaults, | ||
) | ||
defaults[field_name] = factory.declarations.SubFactory(generated_factory) | ||
|
||
updated_factory = factory.make_factory( | ||
get_model_for_factory(factory_class, apps), | ||
**defaults, | ||
FACTORY_CLASS=factory_class, | ||
) | ||
return updated_factory | ||
Comment on lines
+1
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this get used? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't currently. I can get rid of it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you plan to use it somehow I'm ok with it staying, was just trying to work out if there was something else going on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has this been run against a recent anonymised copy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been.