Flake8 Bugbear checks for some extra things that aren’t code style
errors, but are likely to introduce bugs or unexpected behaviour. A
good example is having mutable default function arguments, which get
shared between every call to the function and therefore mutating a value
in one place can unexpectedly cause it to change in another.
This commit enables all the extra warnings provided by Flake8 Bugbear,
except for the line length one (because we already lint for that
separately).
It disables:
- _B003: Assigning to os.environ_ because I don’t really understand this
- _B306: BaseException.message is removed in Python 3_ because I think
our exceptions have a custom structure that means the `.message`
attribute is still present
We now use the pattern of showing a box at the top of the page with the
error. The error message has a heading and can have additional details.
Error messages and the invalid pages get stored in the S3 metadata.
If there aren’t a range of options (normally presented as radio buttons)
to show the user on the email branding request page then we just show
the textbox. But we were still doing form validation on the radio
buttons, even though the user couldn’t see them to click them. This
stopped the user from being able to submit the form.
This commit fixes the problem by, in this specific case, pre-ticking the
‘Something else’ radio button.
Letting people input a bit of free text should reduce the amount of back
and forth we have to do over support tickets when setting up someone’s
branding.
If something else is the only option then we don’t show the radio button
at all and have just the free text input on the page (not behind a
progressive disclosure).
Users who work in local government can’t have GOV.UK branding on their
emails. And only those working for Companies House (for example) can
request the Companies House branding.
This commit adds:
- new choices of email branding, which offer the name of the branding,
rather than the style
- logic to filter this list to only the applicable options, based on
what we know about the user, service and organisation
This is a change from the previous approach which put the onus on users
to figure out the style of branding they wanted, when we might already
know that a lot of the options weren’t available to them, or would be
inconsistent with the branding of other services in their organisation.
Added a form to upload a single letter. Currently this only uses the
form to validate that a file is submitted and that the file is a PDF. If
either of these validations fail, the form will display an error.
Otherwise, we redirect to a new preview page which just has the filename
as the heading for now.
All we do via support is ask which organisation they work for and
manually assign their service to it. This commit makes that process self
service.
We think we have all the trusts and clinical commissioning groups
loaded into the database now.
This will make the go live process smoother for these teams.
Most GP practice services are named after the practice, which is the
organisation.
So rather than make people re-type the name of their organisation (and
potentially make a typo) let’s just let them say ‘yes, that’s the name
of my organisation’.
Although their allowances are the same as what we call `nhs_local` it
makes more sense to store them separately because:
- we already present them as two separate choices to the user
- we may want to handle them differently in the future, eg in terms of
what branding choices are available to them
Once the API is updated we can start passing in this new value from
the admin app.
We’re defining the list of org types in a few different places. This
makes it more likely we’ll forget to update one of these places, thereby
introducing a bug.
This commit moves the definition to be on the organisation model, which
feels like a sensible enough place for it.
This report will be used by the engagement team. There is a form to give
a start and end date for the report, and the form is then downloaded
as a CSV file when the form is submitted.
The service organisation type will either be the same as the org type of
the service's organisation or will be set by a user when creating a new
service. This removes the ability to change it from the platform admin
settings table.
Currently we set not-very-useful defaults for organisation type and
crown status when creating an organisation. This commit adds two field
to the form (in addition to the existing name field) to explicitly ask
for:
- organisation type
- crown status
We need these for all organisations before we can make any of their
services live.
This commit also records any new organisation as not having accepted the
data sharing and financial agreement, because if we don’t know about the
organisation already then they definitely won’t have signed it.
If the user has selected that they are accepting the agreement on behalf
of someone else then we need to make the they provide that person’s
details.
If they’ve selected that they are accepting the agreement themselves
then we have to ignore what they might have put in the ‘on behalf of
boxes’ (for example if they filled them out but then changed their
mind).
At the moment, the process for accepting the data sharing and financial
agreement is:
1. download a pdf
* print it out
* get someone to sign it
* scan it
* email it back to us
* we rename the file and save it in Google Drive
* we then update the organisation to say the MOU is signed
* sometimes we also:
* print it out and get it counter-signed
* scan it again
* email it back to the service
Let's not do that any more.
When the first service for an organisation that doesn't have the
agreement in place is in the process of going live, then they should
be able to accept the agreement online as part of the go live flow. This
commit adds the pages that let someone do that.
Where the checklist shows the agreement as **[not completed]** then
they can follow a link where they can download it (as happens now).
From here, they should then also be able to provide some info to accept
it. The info that we need is:
**Version** – because we version the agreements occasionally, we need to
know which version they are accepting. It may not be the latest one if
they downloaded it a while ago and it took time to be signed off
**Who is accepting the agreement** – this will often be someone in the
finance team, and not necessarily a team member, so we should let the
person either accept as themselves, or on behalf of someone else. If
it's on behalf of someone else we need to the name and email address of
that person so we have that on record. Obvs if it's them accepting it
themselves, we have that already (so we just store their user ID and
not their name or email address).
We then replay the collected info back in a sort of legally
binding kind of way pulling in the organisation name too. The wording
we’re using is inspired by what GOV.UK Pay have. Then there’s a big
green button they can click to accept the agreement, which stores their
user ID and and timestamp.
The data flow of other bits of our application looks like this:
```
API (returns JSON)
⬇
API client (returns a built in type, usually `dict`)
⬇
Model (returns an instance, eg of type `Service`)
⬇
View (returns HTML)
```
The user API client was architected weirdly, in that it returned a model
directly, like this:
```
API (returns JSON)
⬇
API client (returns a model, of type `User`, `InvitedUser`, etc)
⬇
View (returns HTML)
```
This mixing of different layers of the application is bad because it
makes it hard to write model code that doesn’t have circular
dependencies. As our application gets more complicated we will be
relying more on models to manage this complexity, so we should make it
easy, not hard to write them.
It also means that most of our mocking was of the User model, not just
the underlying JSON. So it would have been easy to introduce subtle bugs
to the user model, because it wasn’t being comprehensively tested. A lot
of the changed lines of code in this commit mean changing the tests to
mock only the JSON, which means that the model layer gets implicitly
tested.
For those reasons this commit changes the user API client to return
JSON, not an instance of `User` or other models.
- This brings in the latest version of notifications-utils which
allows Welsh characters in SMS templates.
- Updated the pricing page to show the new prices for SMS with certain
Welsh characters
This adds an option on the organisation settings page to add
'request_to_go_live_notes'. When a service belonging to this
organisation requests to go live, any go live notes for the
organisation will be added to the Zendesk ticket in the 'Agreement
signed' section.
When a user's email address is updated, we not allowing it to be changed
to a non-government email address. We now allow a non-gov email address
to be changed to another non-gov email address. Government email
addresses still cannot be changed to non-government email addresses.
Also fixes the link in the error message on the ChangeEmailAddress form -
this was being escaped before.
We’re deprecating storing the domain as text on a branding in favour of
a database relationship between branding and organisation.
We need to do this now in order to remove the validation on these fields
(which depends on the data in `domains.yml`)
Adds a front end for:
https://github.com/alphagov/notifications-api/pull/2417
> Sometimes we have to make a few services for what really is one
> service, for example GOV.UK Pay and GOV.UK Pay Direct Debit. We also
> have our own test services which aren’t included in the count of live
> services. We currently count these as one service by not including
> them in the beta partners spreadsheet.
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`.
