For two reasons:
- it’s extra stuff in tour that users dont yet need to know about
- test messages are hidden from the dashboard, so you’d have no
visibility of when they were sending once you’d scheduled them
Users need to pick a time in the next 24hrs, or send a file immediately.
Rationale for this is a bit lost in time-before-holiday, but generally:
‘Now’ and ‘later’ as the inital choices makes it really clear what
this feature is about conceptually.
The choice of times is absolute, eg ‘1pm’ not ‘in 3 hours’
Our templates are a littered with `request.args.get('help', '0')`.
This commit refactors these into a single helper method, which can be
used by the view functions, then passed to the template.
This makes the templates cleaner, and should make it easier to refactor
`help` out of the query parameters entirely in the future.
This changes it back to how it was when we first introduced this
feature:
https://github.com/alphagov/notifications-admin/pull/181
It’s kind of lost on the page where you upload a file, which is a shame
because it’s a good teaching aid.
After the tour we should ground users by dropping them on the dashboard.
In the background, we delete the example text message template. This
means that users start from a clean slate when they go to add their own
templates
This also means some wording changes to the tour so it still makes
(some) sense:
- 1, 2 and 3 should refer to the current step, not describe the next
one
- the link should take you to the dashboard
- change from ‘Get started’ to ‘Try this example’ because we’re using
‘Get started on the dashboard’
Steps to reproduce:
- make a template with a placeholder
- click ‘send yourself a test’
- leave fields blank
- click ‘check’
- see error, click ‘back’
Expected: previous page
Actual: previous page with blue help sidebar
When the URL contains `help=0`, `request.args.get('help')` returns '0'.
Doing `if '0':` is the same as doing any `if <non empty string>:` which
returns `True`.
So we should only display the help when the help query parameter is:
- not missing
- AND a string that isn’t `'0'`
While test messages technically have a file and are a job, there’s not
much reason to ever revisit them. So all they end up doing is cluttering
the dashboard and making it harder to find the actual files you’ve
actually uploaded from your computer.
So this commit:
- abstracts the name of test messages into config
- filters out any files whose filename represents a test message
- adds some more thorough tests for the jobs on the dashboard
For the button on the check page, we need to be able to say ‘1 text message’ or
‘55 emails’. We already have the logic to do this on the dashboard (101 text
messages sent), and it’s already in a component. So this commit makes the check
page use the same component.
Row-level errors are:
- bad phone number/email address
- missing data
I think it’s distracting to show these on the page if there’s something more
fundamentally wrong with the file, eg placeholders don’t match.
So this commit makes sure that these error messages are only displayed when the
top-level error says ‘There is a problem with your data’
This commit rearranges the CSV errors (again) to make them geared towards
teaching you how to match placeholders to the column headers.
So the order of errors now is:
1. No phone number/email column
2. Column headers don’t match placeholders
3. Missing or bad data
4. Trial mode
5. Daily limit reached
This depends on:
- [x] https://github.com/alphagov/notifications-utils/pull/39 for 1.
Now that the example on the breaking changes page looks more like a spreadsheet,
we should do the same thing for the downloadable example on the send page.
_The code for this is quite hacky and light on tests. But I’d really like to get
it in the app for the research tomorrow to see how well the feature works._
This commit changes the tour from being a set of static screens to some help
which guides you through the process of sending your first test message.
The theory behind this is that what users are really struggling with is the
concept of a variable, rather than the relationship between the placeholders and
the column headers. And like learning to program, the best way to learn is by
taking an example and modifying it to your own needs.
This means that when someone adds their first service we set them up an
example email template and an example text message template. Then there is a
guided, three step process where _all_ the user can do is send a test message to
themselves.
Once the message is sent, the user still has the example templates which they
can edit, rather than having to remember what they’re supposed to be doing.
Generating the UUID can be can be contained within this function,
thus any other part of the code using it doesn’t have to do the
ID-generating stuff itself.
There shouldn’t be a case where we see a `ValueError` on upload any
more. Our file handling should be robust enough to deal with whatever is
thrown at it.
This commit:
- adds test files with bad data (PNG files with their extensions changed to look
like spreadsheets)
- catches whatever exceptions are raised by trying to parse these files
- returns a helpful flash message to the user
Anything else should raise a `500`, eg if the file can’t be uploaded to S3.
We require users to export their spreadsheets as CSV files before
uploading them. But this seems like the sort of thing a computer should
be able to do.
So this commit adds a wrapper class which:
- takes a the uploaded file
- returns it in a normalised format, or reads it using pyexcel[1]
- gives the data back in CSV format
This allows us to accept `.csv`, `.xlsx`, `.xls` (97 and 95), `.ods`,
`.xlsm` and `.tsv` files. We can upload the resultant CSV just like
normal, and process it for errors as before.
Testing
---
To test this I’ve added a selection of common spreadsheet files as test
data. They all contain the same data, so the tests look to see that the
resultant CSV output is the same for each.
UI changes
---
This commit doesn’t change the UI, apart from to give a different error
message if a user uploads a file type that we still don’t understand.
I intend to do this as a separate pull request, in order to fulfil
https://www.pivotaltracker.com/story/show/119371637
This commit does two main things:
- adds textboxes to the send yourself a test page, so you can replace
((name)) with ‘Chris’, or whatever your name is
- rejigs the send page a bit to make it clearer what the CSV upload is
for and how to use it
The idea being that, since most users go into ‘send yourself a test’
first, it teaches them about how placeholders work by making them do the
replacing themselves.
When previewing a template on the send page, having the recipient appear
as a placeholder should help reinforce the relationship between the
columns in the CSV and the placeholders.
Then, when previewing a message, having the template populated with the
first recipient’s email address/phone number should reinforce that
relationship again.
https://www.pivotaltracker.com/story/show/117630691
There is a limit of 50 messages per day in trial mode. Right now, we
don’t tell you this, we just start failing your messages.
This commit adds an error message if you upload a CSV file that has too
many rows in it.
Depending on the size of the uploaded file, Flask will temporarily store
it in different ways. This means that it comes back as a `TempFile` if
the file is roughly <500k and as `BytesIO` if the file is larger.
`TempFile` supports the `.getvalue()` method, but `BytesIO` does not.
Both support the `.read()` method, so this commit changes to use that
instead.
For users who:
- want to send messages from a template
- want to edit templates
For developers:
- who need to get the ID of a template
This commit mainly cleans up the choose template page so there are less
options, and the options that are there are less wordy.
This means:
- moving ‘send yourself a test’ onto the send messages page, and making
it button
- stripping a lot of stuff out of the ‘send from API’ page, so it’s more
obvious what the template ID is
> At the moment, we have an all email templates page, and an edit an
> individual page.
>
> This gets messy when we refer to templates like the dashboard and the
> activity views. We solve this currently by using anchor links to the
> list page, but this is clunky.
>
> So lets add it, then update the links on the dash and activity to the
> new view page.
>
> Should be a link from the view a single template page, to the template
> hub page.
https://www.pivotaltracker.com/story/show/117349227
We probably shouldn’t hide the contents of the CSV when people are just
testing the app, or if they’re starting off with small jobs.
A limit of 15 rows displayed was awkwardly on the cusp between just
testing and sending a small batch.
This commit increases the limit to 50; I reckon that over 50 recipients
no-one will be wanting to check them all individually.
The problem
---
1. Upload a CSV with problems
2. See the validation errors on the page
3. Try to re-upload the file
4. Get this error:
> Method Not Allowed
> The method is not allowed for the requested URL.
https://www.pivotaltracker.com/story/show/116882241
The cause
---
The POST route for the check page (where you see errors in your files)
was removed here:
f3fd5f6b15 (diff-1dd8b3bf53dfd9382c7721051f3d930dR280)
The fix
---
This commit adds:
- a simple route which re-calls the initial ‘I have uploaded a file’ route
- a test to make sure that both of these routes are POSTed to
