It’s weird to have a method just for updating one attribute. I think
the reason for doing this was to only invalidate the
`organisation-{}-name` cache when absolutely necessary, but:
- we don’t need a separate method to check whether it’s the name being
updated
- it was easy to get around this by calling
`OrganisationsClient.update_organisation` directly, leaving a stale
value in the cache
Note that this is copied from the same change made to the rename service
page:
1190e4541b
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’ and ‘rename organisation’
features
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
unattended laptop, for example.
So this commit removes the need to re-enter your password when renaming
an organisation.
If you submit the rename organisation form without making any changes
you will get an error saying that the name is currently in use. This is
true because it’s being used by the current organisation.
However your intention is probably not to actually change anything, so
we can just redirect back to the settings page.
This is the same thing we do when renaming services:
60f5b74904/app/main/views/service_settings.py (L99-L100)
We have a `client_request` fixture which does a bunch of useful stuff
like:
- checking the status code of the response
- returning a `BeautifulSoup` object
Lots of our tests still use an older fixture called `client`. This is
not as good because it:
- returns a raw `Response` object
- doesn’t do the additional checks
- means our tests contain a lot of repetetive boilerplate like `page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')`
This commit converts all the tests which had a `client.get(…)` or
`client.post(…)` statement to use their equivalents on `client_request`
instead.
Subsequent commits will remove uses of `client` in other tests, but
doing it this way means the work can be broken up into more manageable
chunks.
We have a `client_request` fixture which does a bunch of useful stuff
like:
- checking the status code of the response
- returning a `BeautifulSoup` object
Lots of our tests still use an older fixture called `client`. This is
not as good because it:
- returns a raw `Response` object
- doesn’t do the additional checks
- means our tests contain a lot of repetetive boilerplate like `page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')`
This commit converts all the tests which had a `client.login(…)`
statement to use `client_request` (which is already logged in by
default).
Subsequent commits will remove uses of `client` in other tests, but
doing it this way means the work can be broken up into more manageable
chunks.
We have a `client_request` fixture which does a bunch of useful stuff
like:
- checking the status code of the response
- returning a `BeautifulSoup` object
Lots of our tests still use an older fixture called `logged_in_client`.
This is not as good because:
- it returns a raw `Response` object
- doesn’t do the additional checks
- means our tests contain a lot of repetetive boilerplate like `page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')`
This commit converts all the tests using `logged_in_client` to:
use `client_request` instead.
We have a `client_request` fixture which does a bunch of useful stuff
like:
- checking the status code of the response
- returning a `BeautifulSoup` object
For most tests of a platform admin view we used `platform_admin_client`
instead. This is not as good because it returns a raw `Response` object
and doesn’t do the additional checks.
This commit converts all the tests using `platform_admin_client` to:
use new `client_request` and log in as `platform_admin_user` before
making any requests.
This is also nice because it makes any test easy to parametrize with
additional users, for example to test differences in behaviour dependant
on being platform admin or not.
To render text in an SVG consistently the system rendering the SVG must
have the fonts specified by the SVG installed.
If the fonts are not installed then the renderer will fall back to a
system font and the text will look different. This is especially bad
news for branding where the right font is an integral part of any brand.
To fix this, the text should instead be converted to `<path>` elements.
This process is sometimes called ‘outlining’.
A few of our logos had this problem, and I’ve fixed most of them by
hand. Adding this validation will stop the problem, coming up again.
Some users have reported a problem with the received text message
report:
> I have tested the reply service but in the excel report the mobile
> number is showing as 4.47900E+23. How can I change the format so that
> it is show the mobile number that has replied?
This is happening because Excel is interpreting a phone number in the
format `447900900123` as a number in
[scientific notation](https://en.wikipedia.org/wiki/Scientific_notation),
in other words 4.479 × 10<sup>23</sup>.
`447900900123` is the format that our provider is giving us the number
in – there’s no guarantee it will always be in this format.
We can prevent this behaviour by putting spaces in the numbers. Excel
and Google Sheets won’t try to convert a string with spaces into a
number.
I think we used to do this for the sent text messages report but
probably stopped because we decided it was better to keep the phone
number in the same format as it had been supplied to us for
reconcilliation purposes.
This updates the `get_notifications_for_service` of the
`NotificationApiClient` to make POST request to the api when the `to`
field is provided. This is done so that we avoid logging personal
details such as email addreses.
If the `to` field is not present, the method will make a GET reqeust and
there will be no change to how it works.
If an organisation doesn’t have any live services then there’s no data
to download. To make things less confusing we should hide the link in
this case.
This commit also modifies the existing test so that the assertions are
consistent.
We can't assign `hscni.net` to an organisation because it is used
by GP surgeries, so we don't always know which org it should be
associated with. This change allows people with the an `hscni.net`
email address to sign up and create a service.
If the organisation table contains an entry for `agreement_signed_by_id`
and for `agreement_signed_on_behalf_of_name` then we should the person
who signed the MOU as being the `agreement_signed_on_behalf_of_name`.
This was wrongly showing the `agreement_signed_by_id` as the person who
signed the agreement.
We use the `ChangeEmailForm` if you want to change your own email
address or someone else's email address. This has various validators
which get run. We check if the email address is valid (by using a
function from utils) and if the email address is already in use
(by calling API).
If the email address is not valid, we should not call API to see if it's
already in use because this will cause an exception in API leading to a
`500` in admin. We now only call API if there were no other errors with
the email address.
(The `test_should_redirect_after_name_change` test didn't need the
`mock_email_is_not_already_in_use` fixture, so this has been removed.)
This brings a few performance improvements for RecipientCSV, which
we use to preview and process CSVs. One change also renames one of
the attributes for the class to "guestlist".
We don't currently have any radio fields or check boxes where it's
possible to get more than one validation error. However, since we
never want to show more than one error at a time for a field, this
changes the error messages for the relevant widgets to only show the
first error if there ever were multiple.
If there were multiple errors, this widget was joining the messages
together and displaying all error messages. If a text input field does
have more than one validation error, we only want to show one.
This adds a link to the user profile of the person who requested to go
live for "Request to go live" Zendesk tickets. Viewing a user's profile
page helps us to check for duplicate organisations and services from
that user.
We want organisation team members to be able to see the MOU details for
their organisation. This change creates a new page called billing, which
contains these details. It's only visible to platform admin users now -
the plan is to add more information to this page, then to make it visible
to all organisation users.
The page showing the MOU covers the case of when agreement_signed is
True, when an agreement_signed is False, and when agreement_signed is
None. The case when an agreement_signed is None is very rare - it
signifies that the agreement is not signed but that we have some
service-specific agreements in place. We only have a few organisations
in this state, so it's unlikely that the content for this scenario will
be seen.
When an organisation has signed the agreement we may know the full
details (signing date, version signed, the person who signed it or who it
was signed on behalf of), or we may only have the name of the person who
signed the agreement. We show the more detailed content if possible, and
a less detailed version of the content if not.
There's a new route for downloading the agreement which is almost
identical to the existing `.service_download_agreement` route (plus the
test is almost the same), except that it takes an organisation ID
instead of a service ID.
