The original idea behind was to always ask users to re-enter their
password any time:
- we want them to be sure that they want to do what they’re about to do
- we want to be sure it’s really the user trying to do the thing (and
not someone malicious)
In reality we:
- removed this from the initial place it was added (a descendent of the
‘suspend service’ feature)
- only ever added it to the ‘rename service’ feature
So in reality it’s not a pattern we have persisted with. Arguably there
are several things you can now do in the admin app without re-entering
your password which are much more high consequence than changing the
service name.
Also, with browser autofill there’s a lot less chance that forcing
someone to re-enter a password really gives much defence against an
unatteneded laptop, for example.
I also wonder whether we might get people to give better service names
if we make the process of renaming the service less intimidating.
So this commit removes the need to re-enter your password when renaming
a service.
Note that re-naming an organisation still has the same check, but I
haven’t removed that too for the sake of keeping scope of the PR small.
https://www.pivotaltracker.com/story/show/180026726
There was a bug where if you enter an invalid email address in
to the edit reply to email address form and click save, the
form you get shown with your error message will always contain
the field to set as default the reply to and also delete. This should
not have been the case. If you make an error on the form when
changing a reply to that is already a default, then you should not
be given the chance to change it to not default, nor should you
be able to delete it.
This commit fixes that bug by making sure the additional form fields
are only shown if the reply to being changed is not the default.
Jinja is a better language for doing complex templating. And we can use
the global Jinja scope to automatically get access to things like
`current_user` and our formatters.
The new way of creating support tickets can be seen in
[notifications-utils](https://github.com/alphagov/notifications-utils/pull/899).
This changes tickets created when making a request to go live to use
the new way, while other tickets stay the same for now.
The go live tags have been removed. Some of these had become
unneccessary since you can't make the request to go live unless they are
true (e.g. `notify_go_live_email_reply_to`). Others will always get
added by a Zendesk macro when the ticket is replied to, so we don't need
to add them here.
Previously the backend would never validate permissions because the
"not service.active" part would (usually) fail. I've updated it to
match the (inverse of the) conditional we have in the HTML [1].
[1]: 6ac593aa5f/app/templates/views/service-settings.html (L455)
When the broadcast service settings form is submitted it now removes all
permissions for users in notifications-api. This means it should be
clearing the user cache.
For most events this makes the purpose of each argument clearer at
the point the event is called. It's still worth having a function
for each event type, as this abstracts knowledge of the event label.
Using a schema approach will make adding new events easier.
In the next commit we'll DRY-up the duplication in the tests as well.
This is particularly important for broadcast services, where a rogue
service or platform admin could launch a DoS attack by suspending a
service at a critical moment when it needs to send alerts.
We put some content in the go live ticket which is for our benefit, for
example notes about the organisation.
It’s hard for us to be able to say what we want here if we know that the
person making a go live request is going to see those notes.
This commit changes go live requests so that the initial content of the
ticket is hidden from the person raising it (in Zendesk it will appear
as an ‘internal note’, rather than a ‘public reply’).
---
Depends on:
- [ ] https://github.com/alphagov/notifications-utils/pull/877/files
We have a label saying "other live services". This label means
other live services for a user making the request, but it could
also be interpreted as other live services for an organisation.
Hence, we are changing the label to "other live services for
that user" to avoid confusion
This takes a similar approach as in the previous commit. Since the
"training channel" doesn't really exist, we need some extra code
to pre-select it if a service is already in training mode. As in
the previous commit, I've removed a few non-critical test cases
where we really don't need to test exhaustively.
Note that we also need some specific code to avoid pre-selecting an
option for non-broadcast services, which only used to work by fluke:
we would try to populate the field with (False, None, 'all'), which
isn't a valid combination, so nothing was selected.
Previously this field had to mimic the final hyphenated string of
the broadcast account type, even though it was only used to select
one of its components. The new, shorter choices make it easier to
simplify the test for the POST request.
I've also deleted a number of test cases for pre-selected radios.
This functionality isn't critical, so we don't need to exhaustively
test every single possible combination of values.
This allows us to start decoupling the form fields from the final,
hyphenated string, which we'll do in the next commits.
Note that I've also removed the conditional that changes the data
of the network field as part of validating it. We shouldn't change
data in validations, and having the new property directly above
makes it clear there's no need for this code.
without the need for verification.
This is for when the email takes too long to arrive and the service
users cannot update it as a result.
A more streamlined solution has been proposed where we could send
a link in the verification email to the users and clicking that
link would add/update reply-email-to address.
That would require a bit more work so right now I am proposing this
as a quick stop gap so that we don't have to go to the database
manually to add the reply-to email address.
Previously we could only select a provider when using the test
channel, but this is also required for others channels when we
do tests on the production network with individual MNOs.
In order to reduce duplication and improve consistency, I've reused
the new broadcast_service_name_tag macro to show the setting.
Only the test channel has the option to isolate messages to one network.
This commits makes the choices less confusing by only showing the
network choice to those who have selected the test channel.
It feels quite dangerous that it’s just one click to make an emergency
alerts service live.
This commit adds a confirmation step which explains the consequences of
what you’re about to do.
This adds an audit event to the `events` table when the broadcast
permissions for a service (the service mode, channel or provider
restriction) changes.
Note, no option at the moment to set the service broadcast account type
as None, or back to without the broadcast permission. This has been done
for speed of development given the chance of us needing this is very
low. We can add it later if we need to.
As per ticket and as per Caley's request, so everything can be
edited together.
Also pluralise labels for billing contact info, to indicate
that putting multiple in is ok, and widen the input fields so
that it is more comfortable to input multiple contact details.
Form can be pre-filled with existing data upon instantiation.
WTForms will know not to do this on POST request.
Co-authored-by: Chris Hill-Scott <me@quis.cc>
We have lots of functions for converting various types of data into
strings to be displayed to the user somewhere.
This commit collects all these functions into their own module, rather
than having them cluttering up `app/__init__.py` or buried amongst
various other things that have ended up in `app/utils.py`.
