Commit Graph

165 Commits

Author SHA1 Message Date
Katie Smith
aaadd867f7 Refactor code to check whether email/sms is enabled 2017-07-05 09:15:58 +01:00
Katie Smith
42a8474415 Stop users from editing/adding templates without the correct permission
If sending SMS is disabled for a service, it should not be possible to
add or modify SMS templates. If a user tries to do this, they should see
a different page with a link to go back. The same thing should happen
with email templates.
2017-07-05 09:15:32 +01:00
Rebecca Law
3f6930d176 Fix permissions for sending one off notifications. 2017-07-03 16:39:57 +01:00
Leo Hemsted
0b6b659bc0 if on the tour check notification page (step 2), show back link
Back link redirects to beginning of tour.

Additionally, this fixes a problem where you'd get a 500 when using
the browser back button
2017-06-30 12:48:44 +01:00
Leo Hemsted
ef4d3d111f help (aka tour) now works correctly throughout the notification flow 2017-06-30 11:49:03 +01:00
Leo Hemsted
87aeec1b8d add other send notification error branches and tests 2017-06-29 16:10:59 +01:00
Leo Hemsted
70914bfe8a display errors on the end page.
The following errors may happen:
* Number outside of service if service in trial mode
* Message too long for sms
* Service over daily limit

We need to handle these. They only return on send, rather than in a
separate validation step (for now).
2017-06-29 15:31:44 +01:00
Leo Hemsted
a31bf0c5ce add tests for new check and send notification endpoints 2017-06-29 12:40:03 +01:00
Leo Hemsted
de92950d8e make sure recipient details always rendered 2017-06-29 12:40:03 +01:00
Leo Hemsted
47aaebacc3 iron out kinks in recipient/placeholder handling
where we were previously setting the placeholder when going through
the send self a test - however, should be setting recipient. Also,
only do this on step-0 of the one-off route, not the send-test route,
since step-0 of send-test is the first normal placeholder. Phew!
2017-06-29 12:40:03 +01:00
Leo Hemsted
e46b635551 separate recipient from rest of placeholders when sending notification
note: in the case of letters, we still want to create a CSV file. This
only modifies the code flow when it's an email or template 😩

renamed `send_test_values` to `placeholders` because a) that's what
they are and b) this isn't just for sending a test message any more
2017-06-29 12:40:03 +01:00
Leo Hemsted
40f805c230 add new endpoints for one-off flow
rather than creating a job, after entering the placeholders, you now
send a single notification. This means we don't clog up s3 by creating
lots of one line CSV files.
2017-06-29 12:40:03 +01:00
Leo Hemsted
16df91766f rename template var in send_test_step
it's confusing reassigining one template (json from api) to another
type (utils object) on one line.

Also removed an unnecessary bounds check (since if placeholders is
empty the IndexError will throw on the next line anyway and it'll
be handled the same), and moved get_back_link out to its own function
2017-06-29 12:40:03 +01:00
Ken Tsang
bcc45ede9c Refactored code for new service permissions 2017-06-23 14:27:41 +01:00
Chris Hill-Scott
f12e0fde39 Ensure the tour sidebar gets shown
I accidentally broke it by removing a parameter. This commit reinstates
that parameter and adds some tests to make sure it doesn’t happen again.
2017-06-01 14:27:55 +01:00
Chris Hill-Scott
6425dcbc94 Rename the feature to ‘send to one recipient’
‘One-off’ is a bit wooly. Feels like our name for the thing. ‘Send to
one recipient’ matches ‘Upload recipients’.

This also means making the `<h1>` on job page ‘Report’ for one-off
messages. It doesn’t make sense to call the feature ‘send to one
recipient’ when we’re not using the language of one-off any more.
2017-06-01 14:27:25 +01:00
Chris Hill-Scott
43f6d21e1d Replace test with one-off
It’s a confusing proposition to have two features which are almost
identical. Even differentiating between them in the template menu would
be tricky.

I think the better thing to do is rename the whole feature to ‘send
one-off message’.

Then if someone wants to use there own phone number or email address,
give them a quick shortcut to doing that, once they’re in the flow.
In the background this reuses the ‘send yourself a test’ code, but
the user is never aware that they’re going through a different route
to send an individual message. So the proposition stays nice and clean.
2017-06-01 14:26:49 +01:00
Chris Hill-Scott
82549c817a Ensure correct page titles on send test flows
We have some fairly complicated nested if statements in our Jinja that
decide what the page titles should be. It’s only going to get more
complicated with the send individual message routes.

So this commit:
- moves the logic from Jinja to Python
- adds tests to check things are working as expected
- sets the page titles to the right thing for each flow
2017-06-01 14:26:14 +01:00
Chris Hill-Scott
27c1463213 Validate recipients in send a one-off message
It would be annoying to get all the way to the end of the flow and get
told that the phone number or email address you entered isn’t valid.

So this commit reuses the existing WTForms objects that we have to do
some extra validation on the first step in the send one-off message
flow. It also accounts for international phone numbers, if the service
is allowed to send them.

It doesn’t reject other people’s phone numbers if your service is
restricted, because I think it’s better to let users play with the
feature – it’s good for learning.
2017-06-01 14:25:41 +01:00
Chris Hill-Scott
7310e8b435 Make send one-off route ask for phone number/email
This commit adds an extra, initial, step to the ‘send one-off message’
flow to ask for a phone number or email address. This is the first pass
at making a feature which caseworkers or similar could use Notify to
send individual messages while they’re working a case.
2017-06-01 14:25:04 +01:00
Chris Hill-Scott
eabd9fcbf1 Add a third route through populating a template
This commit adds a route which is identical to send yourself a test, but
with its own endpoint. This will let us add a slightly different
‘send a one-off message’ flow. This commit just adds the route though,
and makes sure that the tests pass for both routes.
2017-05-30 10:33:48 +01:00
Chris Hill-Scott
cd7c27925c Check that users can’t skip steps in send test
Because we put the step in the URL, users could:
- skip ahead to a later step
- navigate to a step which doesn’t exist (ie an index greater than the
  number of placeholders)

This commit adds some checks to do the sensible thing in the unlikely
event that either of these situations occur.
2017-05-22 12:12:22 +01:00
Chris Hill-Scott
e7896f283a Cache letter page count on send yourself test
Calculating the number of pages in a letter is quite slow. And the send
yourself a test pages need to load _fast_. Since filling in placeholders
is very unlikely to change the number of pages in the resultant letter,
it’s pretty safe to cache that count, and makes the subsequent pages
load a lot faster.
2017-05-22 10:52:06 +01:00
Chris Hill-Scott
16c7d17329 Unfill current placeholder if revisiting page
If we don’t do this then you can’t see where in the composed message
the value for your placeholder will appear.
2017-05-22 10:51:39 +01:00
Chris Hill-Scott
8c03feb334 Show one field at a time on send yourself a test
The send yourself a test feature is useful for two things:
- constructing an email/text message/letter without uploading a CSV file
- seeing what the thing your going to send will look like (either by
  getting it in your inbox or downloading the PDF)
- learning the concept of placeholders, ie understanding they’re thing
  that gets populated with _stuff_

The problem we’re seeing is that the current UI breaks when a template
has a lot of placeholders. This is especially apparent with letter
templates, which have a minimum of 7 placeholders by virtue of the
address.

The idea behind having the form fields side-by-side was to help people
understand the relationship between their spreadsheet columns and the
placeholders. But this means that the page was doing a lot of work,
trying to teach:
- replacement of placeholders
- link between placeholders and spreadsheet columns

The latter is better explained by the example spreadsheet shown on the
upload page. So it can safely be removed from the send yourself a test
page – in other words the fields don’t need to be shown side by side.

Showing them one-at-a-time works well because:
- it’s really obvious, even on first use, what the page is asking you to
  do
- as your step through each placeholder, you see the message build up
  with the data you’ve entered – you’re learning how replacement of
  placeholders works by repetition

This also means adding a matching endpoint for viewing each step of
making the test letter as a PDF/PNG because we can’t reuse the view of
the template without any placeholders filled any more.
2017-05-22 10:50:10 +01:00
Chris Hill-Scott
acae449ab9 Show all pages of a letter everywhere
Right now showing all the pages in full is the only way we have of
showing a letter that makes sense to our users. Maybe in the future we
show some kind of truncated version, but the end of the first page is
not a good place to truncate the letter.

This commit just extracts the code for showing multiple pages from the
template view, refactors it for reuse, and includes it in the send
views.
2017-05-16 11:09:34 +01:00
Chris Hill-Scott
d1cd74daa8 Update utils with to make letters unclickable
Brings in:
- [ ] https://github.com/alphagov/notifications-utils/pull/159

Means:
- renaming a bunch of things
- removing any CSS that targeted the link, and making it target the
  containing element too
2017-05-03 12:20:45 +01:00
Chris Hill-Scott
fd7a34f1e4 Allow international phone numbers in spreadsheet
If a service can send internationally, our CSV validation should not
catch valid international phone numbers. This means calling through
to code added to utils in:
- [ ] https://github.com/alphagov/notifications-utils/pull/156
2017-04-27 13:52:04 +01:00
Chris Hill-Scott
de2886dbaf Remove ‘human-readable’ phone number formatting
This has been removed from utils (so things will break if utils is
upgraded without this change isn’t made). I think it’s friendlier to
present the phone number as the user entered it anyway – because this is
what they think a ‘correct’ phone number representation looks like
anyway.
2017-04-27 13:50:55 +01:00
Leo Hemsted
d655a38e14 don't allow bad filetypes to preview 2017-04-12 12:12:11 +01:00
Leo Hemsted
9391181b2c Add cloudfoundry config values
also added tests
2017-04-10 19:25:08 +01:00
Leo Hemsted
a6318f2674 remove pdf/png code from admin app entirely
while PDFs work on paas, they only do that because it turns out the
python buildpack happens to have imagemagick preinstalled - if that
ever changes then it'd break. so move those to the template preview
service. This also means we can get rid of weazyprint and wand
dependencies
2017-04-10 17:45:22 +01:00
Chris Hill-Scott
db37a16eda Fix broken journey when going back from check page
We’ve seen in letters usability testing that people get stuck in a
“no-man’s land” when trying to go back from the _Send yourself a test_
page.

This was broken for two reasons:
- we hadn’t considered that a letter template without placeholder still
  requires you to fill in
- we’ve changed subsequently made the _view template_ page the place
  where you do your actions, rather than the (old) page with all the
  templates shown

So this commit fixes it so that the back link always take you back to
the page you were previously on, and adds some more test cases so we
have all the scenarios accounted for.
2017-04-07 09:18:00 +01:00
Chris Hill-Scott
0b2a11daf2 Don’t fill out address line 3 in the example
Address line 3 is optional. Currently the only way we have of indicating
to users what is/isn’t optional is by using the example. Which probably
isn’t ideal, but should at least be correct.
2017-04-06 10:25:05 +01:00
Chris Hill-Scott
44a3a81305 Put template ID on the single template page
Not necessary to have it on its own page – it’s one line of stuff. And
definitely not as frequent use as the ‘Upload recipients’ or ‘Send
yourself a test’ links.
2017-03-20 11:55:48 +00:00
Chris Hill-Scott
43a922638b Merge email, text message + letter templates pages
Right now we have separate pages for email and text message templates.
In the future we will also have a separate page for letter templates.

This commit changes Notify to only have one page for all templates.

What is the problem?
---

The left-hand navigation is getting quite crowded, at 8 items for a
service that can send letters. Research suggests that the number of
objects an average human can hold in working memory is 7 ± 2 [1]. So
we’re at the limit of how many items the navigation should have.

In the future we will need to search/sort/filter templates by attributes
other than type, for example:
- show me the ‘confirmation’ templates
- show me the most recently used templates
- show me all templates containing the placeholder `((ref_no))`

These are hypothetical for now, but these needs (or others) may become
real in the future. At this point pre-filtering the list of templates
by type would restrict what searches a user could do. So by making this
change now we’re in a better position to iterate the design in the
future.

What’s the change?
---

This commit replaces the ‘Email templates’, ‘Text message templates’ and
‘Letter templates’ pages with one page called ‘Templates’.

This new templates page shows all the templates for the service, sorted
by most recently created first (as before).

To add a new template there is a new page with a form asking you what
kind of template you want to create. This is necessary because in the
past we knew what kind of template you wanted to create based on the
kind you were looking at.

What’s the impact of this change on new users?
---

This change alters the onboarding process slightly. We still want to
take people through the empty templates page from the call-to-action on
the dashboard because it helps them understand that to send a message
using Notify you need a template. But because we don’t have separate
pages for emails/text messages we will have to send users through the
extra step of choosing what kind of template to create. This is a bit
clunkier on first use but:

- it still gets the point across
- it takes them through the actual flow they will be using to create new
  templates in the future (ie they’re learning how to use Notify, not
  just being taken through a special onboarding route)

I’m not too worried about this change in terms of the experience for new
users. Furthermore, by making it now we get to validate whether it’s
causing any problems in the lab research booked for next week.

What’s the impact of this change on current services?
---

Looking at the top 15 services by number of templates[2], most are using
either text messages or emails. So this change would not have a
significant impact on these services because the page will not get any
longer. In other words we wouldn’t be making it worse for them.

Those services who do use both are not using as many templates. The
worst-case scenario is SSCS, who have 16 templates, evenly split between
email and text messages. So they would go from having 8 templates per
page to 16, which is still less than half the number that HMPO or
Digital Marketplace are managing.

References
---

1. https://en.wikipedia.org/wiki/The_Magical_Number_Seven,_Plus_or_Minus_Two

2. Template usage by service

Service name                           | Template count | Template types
---------------------------------------|----------------|---------------
Her Majesty's Passport Office          |             40 | sms
Digital Marketplace                    |             40 | email
GovWifi-Staging                        |             19 | sms
GovWifi                                |             18 | sms
Digital Apprenticeship Service         |             16 | email
SSCS                                   |             16 | both
Crown Commercial Service MI Collection |             15 | email
Help with Prison Visits                |             12 | both
Digital Future                         |             12 | email
Export Licensing Service               |             11 | email
Civil Money Claims                     |              9 | both
DVLA Drivers Medical Service           |              9 | sms
GOV.UK Notify                          |              8 | both
Manage your benefit overpayments       |              8 | both
Tax Renewals                           |              8 | both
2017-03-01 15:17:06 +00:00
Chris Hill-Scott
888821d1b4 Don’t 500 when a CSV is missing rows
> When the CSV is missing the header row, we get an error and the user
> will see "Sorry, we are experiencing technical difficulties..."
>
> We should return a better error message for the user.

– https://www.pivotaltracker.com/story/show/140668615

This was caused by an attempt to access the `first_recipient` variable
before it was assigned. It would only be assigned when there was at
least one row in the file.

Fixing this means doing two things:
- defaulting `first_recipient` to be `None` before looking in the file
- adding an error message for when we can’t extract any rows out of the
  file (which is more nuanced than the file just being completely empty)

(There’s a nasty `sort` in the Jinja template because when there are no
rows in the file the order of the required column headers is not
deterministic.)
2017-02-27 14:46:01 +00:00
Leo Hemsted
2cd8304061 Merge pull request #1082 from alphagov/persist-letters
allow creation of letter jobs
2017-01-31 10:31:34 +00:00
Leo Hemsted
51f3e0478a allow creation of letter jobs 2017-01-20 16:20:37 +00:00
Leo Hemsted
ef91c374a5 send users back to beginning of tour if they hit back at end
(previously it would have sent them to the choose template page)
if the user has added new templates or deleted the example one,
they're clearly competent enough to use the app so don't worry
(we wouldn't know what URL the tour starts on since the UUID of
the example template is random)
2017-01-20 10:45:29 +00:00
Leo Hemsted
9e414ef3c0 when pressing back after sending CSV redirect properly
previously we were issuing a flask redirect (302) from the function,
which we then attempted to unpack as a dict further down the line.

raise a werkzeug.routing.RequestRedirect (301 MOVED PERMANENTLY)
instead. note: only use this pattern when the URL they attempted to
access will *NEVER* be valid, as 301s are cached by browsers.
2017-01-19 15:20:30 +00:00
Chris Hill-Scott
2854dbe3dd Refactor PNG generating endpoint for code reuse
The way we generate PNGs is the same for all the different endpoints,
so the code to do this should be shared.
2017-01-04 15:45:59 +00:00
Chris Hill-Scott
559433c5d2 Add remaining endpoints for PDFs and PNGs
Right now we can show what a letter template looks like as a PDF or PNG.

This commit completes the work so this is also possible when:

- showing a template with the placeholders replaced
- showing any version of a template

Also removes dependency on `Exception().message`, which was deprecated
in Python 2.6. See
97f82d565f
for full details.
2017-01-04 15:45:59 +00:00
Chris Hill-Scott
a159f351a8 Split endpoints that use templates
We’ll need to reuse the logic for viewing images and PDFs of templates,
so let’s have it in a shared method which the HTML endpoint then calls.
2017-01-04 15:45:58 +00:00
Chris Hill-Scott
dc63de0f4e Incorporate breaking utils changes
Brings in:
- [ ] https://github.com/alphagov/notifications-utils/pull/94
2016-12-14 14:32:07 +00:00
Chris Hill-Scott
ae356fc741 Use PDF link renderer to preview letters
Depends on and implements:
- [x] https://github.com/alphagov/notifications-utils/pull/84
- [ ] https://github.com/alphagov/notifications-utils/pull/86
2016-12-13 10:34:19 +00:00
Chris Hill-Scott
18d11aa013 Move code for rendering messages/templates → utils
Utils is better structured to handle the logic of what thing to show
for what template type, especially now that what we show for different
template types in different contexts has diverged significantly.

See https://github.com/alphagov/notifications-utils/commit/6b39c1a for
an example of this code moving into utils

Depends on and implements:
https://github.com/alphagov/notifications-utils/pull/84

The main reason for doing this is to get Paul’s fix for the misaligned
CSV columns: https://github.com/alphagov/notifications-utils/pull/87
2016-12-05 12:11:54 +00:00
Chris Hill-Scott
a108165e47 Give more realistic example address data
Posit that examples of where you can put different parts of the address
is more helpful than ‘example, example, example’. Also shows that you
don’t have to fill all of the address columns.

Spot the Easter egg 🎅
2016-11-14 14:53:07 +00:00
Chris Hill-Scott
69ad5fe156 Handle multi-column recipients
Implements https://github.com/alphagov/notifications-utils/pull/81

Handles addresses as multiple columns:
- in ‘Send yourself a test’
- in example CSV files
- in validating that a CSV file has recipients (eg at least an ‘address
  line 1’ and ‘postcode’ column)
- when showing the contents of a CSV file

As few UI changes as possible, once we have the thing working end-to-end
we can think about how the UI might need to work differently.
2016-11-14 14:53:06 +00:00
Chris Hill-Scott
70eec8fe73 Don’t let people actually start a letter job
Who knows what would happen if a job with a letter template actually
got into the database. `403`ing the page is a quick and dirty hack to
stop this from happening.
2016-11-14 14:51:27 +00:00