This was a minor issue in a recent incident [1].
We don't want people emailing us directly:
- It's harder to collaborate on replies than in Zendesk, where I
can see who's editing.
- We can't take advantage of macros like we can in Zendesk.
- People emailing us directly creates an unnecessary distraction
and it's easy to lose track. We want to minimise distractions in
an incident situation.
- We use notify-support@ for monitoring, so we don't want noise.
[1]: https://docs.google.com/document/d/1eArpcYoCmhlgB45FZpGj1_HnkAgpgtgwr-KBiUq7h8c/edit#
for a job to be finished there are two requirements:
* the status to be "finished"
* the percentage complete to be 100%
The job status is set to finished when the process_job task finishes
(even though not all process_row may have finished). The
percentage_complete is calculated by comparing the number of
notifications in the database with the number of rows in the
spreadsheet.
This was inadvertently changed from an "and" to an "or" clause two years
ago. This meant that people could download a report when the status was
finished but not all notifications were present in the database. Lets
change it back.
7d52ac97f1 (diff-44b012cad205379c481bed244ddb2294bae5ee85dcd01f4aee932a2bd85b67b2L86-R100)
This deletes the `test_view_team_members` test since it is now
duplicated by a new test. It also moves a test relating to org
invites to test_organisation_invites.py and one that wasn't related to
invites from that file to test_organisations.py.
This adds a link next to the organisation team members which lets
them be removed from the organisation. Service team members have
their own page and the link to remove them appears there. For
organisation team members, we don't currently have any other
information we want to show or any other actions to perform. As
a result, this change uses the 'Team members' page to show the
confirmation banner.
The endpoint called 'edit_user_org_permissions' was renamed to
'edit_organisation_user' and some of the existing code around deleting
org users (which didn't work) was changed.
This stops adding the current user to the data sent to the API when
removing a user from an organisation. API only needs to know the
organisation_id and the id of the user we want to remove from
the organisation, so we don't need to pass through the id of the
current user too.
The other change made is to clear the user cache of the user who has
been removed from the org. We don't need to clear any of the organisation
caches, since these values don't contain lists of users for the orgs.
When checking the service or organisation name for uniqueness before
changing it, it would be necessary to exclude the current name from
this check. However now we are changing it immediately we don’t need
to guard around this behaviour of the uniqueness check.
So this commit removes the guard for both renaming a service and an
organisation.
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)
No tests are now using the `client` fixture directly so we can rename
it.
Python convention is to use an `_underscore` for things which should be
considered semi private.
This should discourage people from writing new tests with these old
fixtures.
New tests should always use `client_request`.
Want to be logged in with a different user? Call
`client_request.login(user)` first.
Don’t want to be logged in? Call `client_request.logout()` first (most
of our tests need to be logged in).
Need an instance of `Response` object not an instance of
`BeautifulSoup`? Use `client_request.get_response` or
`client_request.post_response`.
Need to pass in a URL, not arguments to `url_for`? Use
`client_request.get_url(…)` or `client_request.post_url(…)`.
Need to pass in a URL and get a response back? Use
`client_request.get_response_from_url(…)` or
`client_request.post_response_from_url(…)`.
Some tests use the `client` fixture but don’t call any of its methods.
The reason for doing this is because the test depends on something in
the request context.
This commit replaces all those instances with `client_request`, which
also sets the request context.
These tests are the last ones that still use the `client` fixture. By
replacing it with `client_request` we will be able to say that no tests
should be using the `client` fixture directly.
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 added a new argument to `client_request.get` and
`client_request.post` to specify that it should return a raw `Response`
object rather than an instance of `BeautifulSoup`.
This is useful because sometimes we need to look at stuff like the
response headers.
However it turns out we already have a separate method for this, so
rather than invent something new I think it’s better to stick with the
thing we already have.
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
A few of our tests still use an older fixture called
`logged_in_client_with_session`. It’s not clear how this is different
from `logged_in_client`, which we have replaced with `client_request`.
So this commit goes ahead and converts all the tests using
`logged_in_client_with_session` 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
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.
