-
Notifications
You must be signed in to change notification settings - Fork 1
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 5653 desnz multiple licence conditions #2243
Conversation
6e48010
to
26dafad
Compare
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.
Really great work - I think it's almost there, I just have a bunch of (mostly minor) enhancements to suggest. The only real major is the comment about being put on the edit approval view when editing refusal advice.
422a817
to
185de88
Compare
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.
Nice one. Just a few minor comments/re-comments which I will leave with you
caseworker/advice/forms.py
Outdated
|
||
def __init__(self, *args, **kwargs): | ||
kwargs.pop("approval_reason") | ||
kwargs.pop("proviso") |
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.
Still wondering about this
"footnote": data["footnote_details"], | ||
"proviso": data.get("proviso", ""), | ||
"note": data.get("instructions_to_exporter", ""), | ||
"footnote_required": True if data.get("footnote_details") else False, |
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.
I meant the field in general - footnote
and footnote_required
seems odd
d3dc88e
to
6ed6bd7
Compare
5bead11
to
8c6fede
Compare
caseworker/advice/forms.py
Outdated
|
||
def __init__(self, *args, **kwargs): | ||
kwargs.pop("approval_reason") | ||
kwargs.pop("proviso") |
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.
I'm seeing this in all of the forms and it makes me wonder if there's a better way of solving this than just popping things to make them go away.
I also prefer del
in cases where we are not using the returned value.
unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
Outdated
Show resolved
Hide resolved
d5d5833
to
94b6d62
Compare
…e other forms too
…ion everywhere soon
…d a backlink and moved some logic to rules
94b6d62
to
915e82e
Compare
Converted GiveApprovalAdviceView to a sessionwizard
Added conditional checkboxes to the licence conditions
Added a DESNZeditview to handle the prepopulation of the sessionwizard
Renamed the old views to be -legacy to move us a step closer toward this flow being the default (because of this it may be difficult to read the file changes, I'd recommend checking the commit history for a simpler view of the changes)
This is hopefully the first step to converting this whole are to a sessionwizard and removing the factories
LTD-5653