-
-
Notifications
You must be signed in to change notification settings - Fork 272
[TfL] Add scaffold licence application form #5807
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
base: master
Are you sure you want to change the base?
Conversation
Add foundation for TfL's digital licence application system, replacing their current process. - Licence controller that discovers and routes to specific licence form types (scaffold, etc.) - Base Licence form role providing shared functionality for all licence forms (file uploads, summary display, field formatting) - Scaffold licence form as the first example - Form submission creates non-public Problem records with licence data stored in extra metadata
This removes the duplication between these two forms, and makes it easier for other forms to handle file uploads in the future.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #5807 +/- ##
==========================================
+ Coverage 82.66% 82.96% +0.30%
==========================================
Files 458 467 +9
Lines 35802 37237 +1435
Branches 5844 6114 +270
==========================================
+ Hits 29594 30892 +1298
- Misses 4506 4620 +114
- Partials 1702 1725 +23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Manually created reports can stick in any category they want, there's no database linking there. |
dracos
left a comment
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.
This is all looking good - a number of suggestions and things to change; messaged on Slack in terms of the bits (only little, I think) that we should do before letting them know
| ? $c->user->obj | ||
| : $c->model('DB::User')->find_or_new({ email => $data->{email} }); | ||
| $user->name($data->{name}) if $data->{name}; | ||
| $user->phone($data->{phone}) if $data->{phone}; |
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 think this may be a problem in Claims as well; if the user already exists, its name/phone are not updated (which I think is right, because otherwise someone could override it only by specifying their email, if there's no confirmation). It looks like in Waste we're now setting a $report->set_extra_metadata(phone => $c->stash->{phone}); in these cases, maybe? Bit unclear, and hopefully won't matter, but thought worth noting to have a think.
| # Fields: name, email, phone from AboutYou role | ||
| # ========================================================================== | ||
| has_page applicant => ( | ||
| fields => [ |
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.
The field order on the about applicant page is wrong. This came up in waste with bexley verifications, perhaps now is an opportunity to fix it so the order is always that specified in fields ; at present it’s done on order and then somehow on MRO import order - which makes including things in multiple places harder than need be (but for now, using manual order numbering could provide a fix if necessary here).
| @@ -0,0 +1,67 @@ | |||
| body.formflow { | |||
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.
Could we have a shared CSS, as this is the same as claims, and I don't think it's going to differ a lot
|
Have done a first pass of changes. I added a 👍 reaction to comments I've addressed, and 👀 to ones I still need to address. I think the two things I still need to address before sharing with them are:
The other bits I'd like to do as well, but not as critical before sharing. |
dracos
left a comment
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 can't seem to see the single comment I just made, so adding again here.] Changes so far all look good, only thing left within those is the help text should be "If the response to question 1 is 'no', or any response from 2 through to 7 is 'yes', then..."
|
@dracos I think I've addressed everything that needs to be done before showing them the scaffold form. Would you mind checking over my fixups? |
dracos
left a comment
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.
Yep, all looks good to me, thanks :-)
So I think only non-internal thing left is the form ordering on that one page, which is fine (could tell them we know it's an issue, or not even mention it, give them something to spot ;) ).
|
Have put this on staging and setup a "Scaffold licence" category. For now I've used "Red claim " as the category type, but we should make this more generic, have opened https://github.com/mysociety/societyworks/issues/5348 to track that. |
As part of the application forms project this adds the foundations for the new licencing forms, and implements the first form, scaffolding, along with the shared fields etc for them. Also refactors some bits of the claims form around file uploads to be shared between the forms.
In order to test this locally you'll need to enable it in the config:
I think you might need to create the category as well, but I was able to create reports without the category existing, so not sure if I've done something wrong there?
I haven't done anything about the PDF generation (https://github.com/mysociety/societyworks/issues/5343) yet, this is just the form itself.
Fixes https://github.com/mysociety/societyworks/issues/5304