Integrates the folder permissions form with the updated API endpoint
to store changes in the user folders.
Since user folder permissions are returned in the full list of template
folders for the service we need to invalidate the cache key for it each
time we update user permissions.
We're reusing the logic for the `move_to` nested radios field for the
user folder permissions nested checkboxes.
The main difference between the two forms (aside from the different
input type) is that "Move" form contains the root "Templates" as an
option, whereas the folder permissions doesn't.
It turns out that, because of the way NestedFieldMixin.children and
select_nested macro are implemented the easiest way to get the desired
folder permissions behaviour is to add the root folder as a choice with
a `None` value and `NONE_OPTION_VALUE = None` set on the field, which
allows the `child_map` to be constructed but doesn't display the root
folder checkbox itself since it gets overwritten in the final `child_map`.
At the moment it 500s because it can’t format the `None` values as
numbers.
In the future we will stop people requesting to go live until they’ve
provided this info. For now it has to be optional.
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?
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.
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
Currently when you load the ‘edit user’ page (which has a URL like
`/service/<service_id>/users/<user_id>`) we check that:
- you belong to the service represented by `service_id`
- you have permission to edit users on this service
We don’t check that:
- the user represented by `user_id` belongs to this service
This means that if you could somehow determine another user’s `user_id`
(which I don’t think is possible if you don’t already have the manage
service permission for that service) then you could:
- edit their permissions on your service (weird, but wouldn’t have any
effect)
- change their email address (bad)
This commit adds checks to return a `404` any time you’re looking at a
service and trying to do stuff to a user who doesn’t belong to that
service.
We can’t add this check to the API easily because there are still times
that we want to get/modify users outside the context of a service (eg
platform admin pages, or users who have no services).
The endpoint for setting permissions in api will now be used for both
user permissions and a user's folder permissions, so this changes the
format of the data we pass through.
When updating a user’s email address you currently get an validation
error if you save without changing it. Instead it should just obey your
command. And no need for the confirmation step because nothing is
actually changing.
Most of the existing platform admin buttons on the service settings
page used to issue GET requests to switch service settings. This
means they weren't protected by CSRF. On top of that as our number
of service permissions increases over time a lot of buttons on the
page made it hard to work with.
To fix these issues we replace most of the buttons with rows in the
platform admin settings table. Each setting has a 'Change' link that
leads to a page with an On/Off switch form.
This removes "research mode" switch completely since we're planning
to deprecate it in the future and we don't expect to switch any new
services into research mode at the moment.
Most service permissions are now handled by a shared endpoint that
is parameterized with the permission name. Some permissions that
require some additional logic before they can be toggled (like document
upload, which requires setting a contact address) have separate
initial endpoints that redirect to `set_service_permission`.
"Archive", "Suspend" and "Resume" actions are kept as buttons since
they display a confirmation banner (which is a CSRF-protected form)
and they're not easily represented as an On/Off switch.
This adds a new OnOffField class that implements a boolean field
that is rendered as two On / Off radio buttons. This allows us to
avoid comparing 'on' and 'off' string values in the views since
the field takes care of transforming form data into python booleans.
This also adds a form class that can be used for any single On / Off
switch forms (e.g. service permissions).
The current user already has a list of service IDs. The current user
- is an API call we have to make anyway to render this page
- is usually cached in Redis
This adds a preview pane which is visible when updating a letter brand.
If JavaScript is enabled, the preview pane shows on the set-letter-branding
page, and submitting the form saves updates the letter brand for a service
immediately. If Javascript is not enabled, there is a separate 'Preview email
branding' page which shows a preview of the brand and has a 'Save' button on it.
I did the update following instructions from this commit:
https://github.com/alphagov/notifications-admin/
commit/136662bd309d986a9b7c3e0ee76588612c1ab761
Password repositiories I used were:
darkweb2017-top10000.txt
probable-v2-top12000.txt
twitter-banned.txt
when clients are defined in app/__init__.py, it increases the chance of
cyclical imports. By moving module level client singletons out to a
separate extensions file, we stop cyclical imports, but keep the same
code flow - the clients are still initialised in `create_app` in
`__init__.py`.
The redis client in particular is no longer separate - previously redis
was set up on the `NotifyAdminAPIClient` base class, but now there's one
singleton in `app.extensions`. This was done so that we can access redis
from outside of the existing clients.
