Frontload estimate usage #2774
Merged
Frontload estimate usage #2774
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
quis
force-pushed
the
frontload-estimate-usage
branch
from
4b1a582 to
96ed36f
Compare
leohemsted
reviewed
Feb 19, 2019
app/main/forms.py
Outdated
| error = None | ||
| try: | ||
| if int(self.data) > self.POSTGRES_MAX_INT: | ||
| error = 'Must be less than {:,.0f}'.format(self.POSTGRES_MAX_INT) |
There was a problem hiding this comment.
i appreciate the reason why you've chosen the number, but is this a bit weird and scary as a message?
There was a problem hiding this comment.
Rounded down to 2,000,000,000 in 4dd1a35
app/main/forms.py
Outdated
| pass | ||
|
|
||
| if valuelist[0] == '': | ||
| valuelist[0] = 0 |
There was a problem hiding this comment.
this means that if there's a validation error, other fields are pre-populated with 0 for the reload. Is that okay or would it be more elegant to handle this on API side (or at least post-validation, pre-submitting to API)?
There was a problem hiding this comment.
Made it so it doesn’t re-format or zero-populate if there are errors in 61ac7fa
| <h1 class='banner-title'> | ||
| no things supplied | ||
| </h1> | ||
| <p>Tell us some things</p> |
There was a problem hiding this comment.
not sure that this content's ready for the big leagues just yet 😉
There was a problem hiding this comment.
Up to date content added in cdf437c
app/main/forms.py
Outdated
|
|
||
| if valuelist: | ||
|
|
||
| valuelist[0] = valuelist[0].replace(',', '') |
There was a problem hiding this comment.
do we also want to replace spaces?
thinking of french number system type things, where you have 5 000 000
We get a bunch of requests to go live where people have told us they're going to send email but there is no email reply-to address present. These come from 2 scenarios: 1. when there are email templates, and no reply to address – but they ignore the checklist 2. when there are no email templates (yet) but they provide anticipated volumes for email At the moment we only auto-check for a reply to address when they have email templates. And because the question about anticipated volumes follows the checklist, you'll get a checklist that passes (reply addresses not required as no templates present) - but your future intent that differs (reply address IS required because you have anticipated volumes). So let’s bring the request for anticipated volumes into the checklist, that way we can dynamically add the requirement for a reply to address if they say they will send email but don't have templates yet. We should begin storing it in the database against the service to stop people having to re-enter it each time they try to complete the go live screens. This also means moving the ‘consent to research question’ along with the questions about volume, because - we want people to answer both before going live - we don’t want to clutter up the summary page by asking questions there too
It’s annoying and very ‘computer says no’ to make people type `0` in a box. We can see from our analytics that this error is affecting about 7% of users trying to go live. This commit relaxes the validation to only require a number greater than 1 for at least one of the questions. It also lets people enter their numbers comma-separated – like our examples suggest – but normalises them to integers before sending them over to the API.
Apparently this is a French thing.
It’s confusing to at the same time: 1. change what you’ve inputted 2. tell you it’s wrong This commit makes it so that 1. only happens if 2. doesn’t.
quis
force-pushed
the
frontload-estimate-usage
branch
from
a8cb95d to
4dd1a35
Compare
|
With latest content:
|
A round number feels better than a very arbitrary-looking one.
Things we talked about: • asking users to write the number 'as numerals' or 'using digits' isn't very plain English • the style guide says to use an example in the error `..., like 5,000` but not if you have an example in the hint text, so we can't do that • I have reservations about 'correct format', because it sounds odd if you're not describing something like a phone number, NI number or credit card number. Looking back through Request to Go Live tickets on Zendesk. --- I got to September before I found anything that would count as invalid under our new rules: > Possibly around 1,000,000- not planning on implementing emails yet but might change I'll keep looking, but if most people enter the number according to the hint example we might be able to go with a much simpler error just prompting them to enter a number – no convoluted descriptions of what we mean by a number There seemed to be more problems when the Qs were about start volume and peak volume. Users felt the need to explain their plans more. Using 'number' instead of 'volume' is more explicit too – so that probably helps. In terms of errors: `Enter the number of emails you expect to send` `Enter the number of text messages you expect to send` `Enter the number of letters you expect to send` – will probably do it, right?
Follows what we’re doing elsewhere; make things as easy to click as possible.
quis
force-pushed
the
frontload-estimate-usage
branch
from
4dd1a35 to
ae30d61
Compare
|
Making the clickable areas full-width, and the task list indicators line up:
|
leohemsted
approved these changes
Feb 27, 2019
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.
We get a bunch of requests to go live where people have told us they're going to send email but there is no email reply-to address present.
These come from 2 scenarios:
At the moment we only auto-check for a reply to address when they have email templates. And because the question about anticipated volumes follows the checklist, you'll get a checklist that passes (reply addresses not required as no templates present) - but your future intent that differs (reply address IS required because you have anticipated volumes).
So let’s bring the request for anticipated volumes into the checklist, that way we can dynamically add the requirement for a reply to address if they say they will send email but don't have templates yet (later, not as part of this PR).
We should begin storing it in the database against the service to stop people having to re-enter it each time they try to complete the go live screens.
This also means moving the ‘consent to research question’ along with the questions about volume, because