Ltd 5681 update licence decision refusal criteria #2333
+221
−21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevincarrogan
force-pushed
the
LTD-5681-update-licence-decision-refusal-criteria
branch
4 times, most recently
from
8f5dc3f to
c4216bc
Compare
![code-review-doctor[bot]](https://avatars.githubusercontent.com/in/83430?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
kevincarrogan
force-pushed
the
LTD-5681-update-licence-decision-refusal-criteria
branch
2 times, most recently
from
f57b5b8 to
ee8712a
Compare
saruniitr
reviewed
Dec 10, 2024
api/cases/migrations/0074_update_licence_decision_denial_reasons.py
Outdated
Show resolved
Hide resolved
saruniitr
reviewed
Dec 10, 2024
api/cases/migrations/0074_update_licence_decision_denial_reasons.py
Outdated
Show resolved
Hide resolved
kevincarrogan
force-pushed
the
LTD-5681-update-licence-decision-refusal-criteria
branch
from
30f198e to
624da9b
Compare
currycoder
reviewed
Dec 12, 2024
Comment on lines
35
to
39
| 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 = 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) |
There was a problem hiding this comment.
Looks like duplicate loops..?
| 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.
These need to be valid DenialReason.id values, right? Is it worth checking against a list of valid ids first? Or is the thought that this will fail with an IntegrityError or similar on the set() below?
There was a problem hiding this comment.
It hasn't failed when running on the production-like data so far so I've not worried about this.
| LICENSING_UNIT_ID = "58e77e47-42c8-499f-a58d-94f94541f8c6" | ||
|
|
||
|
|
||
| def update_licencedecision_denial_reasons(apps, schema_editor): |
There was a problem hiding this comment.
Has this been run against a recent anonymised copy?
Comment on lines
+1
to
+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 |
There was a problem hiding this comment.
How does this get used?
There was a problem hiding this comment.
It doesn't currently. I can get rid of it.
There was a problem hiding this comment.
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
saruniitr
approved these changes
Dec 12, 2024
This is to ensure that the ordering of migrations are correct and that data migrations have access to all of the model fields they require
kevincarrogan
force-pushed
the
LTD-5681-update-licence-decision-refusal-criteria
branch
from
624da9b to
5c35e70
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Aim
This updates refusal licence decisions with the relevant refusal criteria.
This is so that the reason for refusal is directly attached to the licence decision model itself instead of being attached to the advice record.
The migration initially just uses the final advice from LU but in some cases older cases don't have this advice record so we fall back on using audit entries for this instead.
LTD-5681