The radio buttons to select the type of service - central, etc. -
are only shown if we can't infer the type based on the user's email
/ default organisation. However, the code to render the page in the
error case didn't accommodate this, nor did it show the version of
the page for adding a local government service.
This fixes the bug by DRYing-up the logic to render the pages. I've
not added a test for this for a couple of reasons:
- It's not a critical bug: no one has complained about it and it
doesn't block the user from adding service.
- It's unlikely to reoccur because the bug involved writing _more_
code than was necessary.
- It's not trivial to test this due to the 3 versions of the page
involved - these are tested for the happy path.
The API has a method to handle setting the default SMS free allowance. This will save a call to the API and remove some code duplication between the two apps.
Needs to be merged after https://github.com/alphagov/notifications-api/pull/3197
🚨 Do not merge until after 1 April 2020 🚨
Once this date has past we no longer need to give any services the
previous allowances, so we can remove them from the codebase to avoid
confusion.
It’s possible we change the allowance structure again, but it might
change in a way that this config-based logic doesn’t account for (what
if we did a per-organisation allowance for example). Having both years’
allowances in the config was a quick fix, not a foundation to build on.
We’re going to have different allowances next financial year. This means
that when someone adds a service, we’ll need to check which year it is,
so we can give them the right allowance.
This commit changes the config structure so that the current allowances
are explicitly assigned to the 2020/21 financial year.
It freezes the tests to the 2020/21 financial year, so they won’t start
failing automatically when next financial year comes around.
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`.
Rather than force us to write the decorators in a specific order let’s
just have one decorator call the other. This should make fewer lines of
code, and fewer annoying test failures. It also means that the same way
of raising a `401` (through the `current_app` method) is used
everywhere.
We get people signing up for Notify who work for the NHS, but whose
organisation we don’t know about. For example
`name@gloshospitals.nhs.uk` will be someone working for Gloucestershire
Hospitals NHS Foundation Trust, which is not an organisation we have in
the database.
Currently we rely on knowing the specific organisation (NHS as a whole
isn’t an organisation) in order to set the organisation type for any
services they create. This commit adds a special case for anyone with an
NHS email address to set the organisation type to be NHS, even when we
don’t know which specific part of the NHS they work for.
This is the same thing we do on the API side for NHS email and letter
branding:
a4ae5a0a90/app/dao/services_dao.py (L310-L313)
Every time someone adds a new service we ask them what kind of
organisation they work for.
We can look this up based on the user’s email address now. So we should
only ask the question if:
- we don’t know about the organisation
- or we haven’t set what type of organisation it is (this shouldn’t be
possible on productions because we’ve populated the column for all
existing organisations and it’s impossible to add a new one without
setting it
We were adding invited users to services in the `main.add_service` view
function as the last step in the process of inviting users. Since this
view function is decorated with `@user_is_gov_user`, invited users with
non-governmental email addresses would never reach this point and would
be able to register an account but would not get linked to a service.
To fix this, we now add the invited user to the service at the point at
which the user gets activated and also ensure that non-gov users don't
get redirected to a page which they don't have permission to view.
This commit is the first step to disentangling the models from the API
clients. With the models in the same folder as the API clients it makes
it hard to import the API clients within the model without getting a
circular import.
After this commit the user API clients still has this problem, but at
least the service API client doesn’t.
Anyone choosing ‘NHS’ for their organisation type gets should get the
NHS branding. We don’t want to hard-code an ID for NHS branding anywhere
because it won’t be consistent between environments.
So instead we can say that anyone who chooses ‘NHS’ as their
organisation type should get whatever branding has `nhs.uk` as its
domain.
This allows us to easily manage the branding the same way we do other
brands, but gives us the efficiency of having it auto applied.
When a user creates a service we can take a pretty good guess at what
organisation they’re from.
For many organisations, especially local councils, GOV.UK branding is
not appropriate for their service. But right now every service:
- gets created with GOV.UK branding
- has to ask us to change it, even if they’ve already done so for other
services they run
This commit starts using the `domain` field on the email branding table
to lookup what email branding to assign to a service automatically,
where we’re sure there’s a sensible default.
Since we send all one off messages as priority now[1], we don’t need to
explicitly mark this template as being priority.
This stops the (potential) problem of people skipping the tour, still
having this template and then modifying it to send other messages,
potentially in high volumes from CSV files or the API. I don’t think
this is a real problem now, but worth cleaning this up.
Currently:
- 827 priority templates in the database
- 195 of which are not deleted
- 18 of which are not called ‘Example text message template’
- 3 of which look like genuine use cases, not from services that we run
[1]: https://github.com/alphagov/notifications-api/pull/1722
Done using isort[1], with the following command:
```
isort -rc ./app ./tests
```
Adds linting to the `run_tests.sh` script to stop badly-sorted imports
getting re-introduced.
Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because
I think it gives the cleanest diffs, eg:
```
from third_party import (
lib1,
lib2,
lib3,
lib4,
)
```
1. https://pypi.python.org/pypi/isort
Different parts of government get billed slightly differently, and
there’s differences in how much money we’re allowed to give them.
Think these numbers are right, but should be double checked.
So that we can default services to their appropriate text allowance, we
need to find out what sector they're in. So let's start collecting that
from teams as they create new services.
I think Central/Local/NHS are the right options, but these can be easily
changed if not.
- This is done using a new endpoint in the api.
- Removed the AddServiceForm in favor or using the ServiceNameForm
- Removed ServiceApiClient.find_all_service_email_from
We’ve made a few changes to the tour recently, without changing the
help text on the left hand side of the screen. So the stuff you see on
the right side of the screen doesn’t quite sync up any more.
This commit adds an extra, introductory page that just shows the
template and a next button, which better matches the ‘every message
starts with a template’ help text.