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’ feature
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
unatteneded laptop, for example.
I also wonder whether we might get people to give better service names
if we make the process of renaming the service less intimidating.
So this commit removes the need to re-enter your password when renaming
a service.
Note that re-naming an organisation still has the same check, but I
haven’t removed that too for the sake of keeping scope of the PR small.
We think that the API is returning incorrect data for this column.
It’s going to take a while to figure out what’s going on with the
queries in the API, so this pull request temporarily removes the column
so we’ve not giving people incorrect data.
We are starting to see lots of 100.0%s in the current table
and we think this looks suspiciously too good so think it is
beneficial to change it to be 2dp such that we get a few more
non 100.0% values.
This will put all the values in to having 2dp, however it will
also require the API to have a change to
https://github.com/alphagov/notifications-api/blob/master/app/performance_dashboard/rest.py#L81
where it is currently losing the granularity down to a single
decimal place (meaning that if we were really at 99.894% then that
would be shown on the page as 99.90% rather than 99.89%). However,
I don't think it is a blocker that we get that sorted before this
can be merged.
This means that the data in the report will match what’s on the page,
where the values are rounded to the nearest penny.
This uses the same string formatting to round the numbers which the
`big_number` component does, so it should round the numbers in the same
way.
This is so org level users can use this data easier for things
like determining spending per service.
We do not include sms fragments sent column and remove other sms columns
consistency.
Do not add sms fragments sent column for now until we agree on an
unambiguous name for it. The data in this column is sms billing units
multiplied by international sms weighing. My favourite for a clear
name would be 'text message credits used', but we need a naming
strategy for this.
This makes it clearer that this model collection isn’t the organisations
for a user or a service or some other entity, like most model
collections are.
It will also lets us make a separate Organisations model, without the
name conflicting.
This commits adds test coverage for ther HTML in several of the forms
which had broken autofocus.
It means that if we make changes to the HTML which triggers autofocus in
the future it should be more obvious that something is depending on the
attributes being added/removed.
This moves things out of the parametrization that didn't need to be
in there and also makes the test match the test below,
test_shows_delete_link_for_error_on_post_request_for_edit_email_reply_to_address,
more closely.
https://www.pivotaltracker.com/story/show/180026726
There was a bug where if you enter an invalid email address in
to the edit reply to email address form and click save, the
form you get shown with your error message will always contain
the field to set as default the reply to and also delete. This should
not have been the case. If you make an error on the form when
changing a reply to that is already a default, then you should not
be given the chance to change it to not default, nor should you
be able to delete it.
This commit fixes that bug by making sure the additional form fields
are only shown if the reply to being changed is not the default.
The current link button for removing an area is
created using a psuedo element with an 'x' as
content.
The inline box for the 'x' overlapped its parent.
This is visible in high contrast mode, breaking the
parent's border. Despite existing in CSS, the 'x'
is also announced by screen readers, which is not
what we want.
This changes it to be an inline SVG with a role of
image. It doesn't require as large an inline box
so doesn't cause visual issues in high contrast
modes. It also means we can set it's label similar to
how you would an image's alt text, giving us
control over what is announced by screen readers.
This commit also includes some extra CSS,
targeting high contrast modes, giving the
link button the following when viewed in those
modes:
- a complete border so it is
distinguishable from the list item
- a focus style
`reference` isn’t very human-friendly – the Environment Agency just
supply a UUID in this field.
The Environment Agency also populate the `<event>`` field with some
human readable text, for example:
> 013 Issue Severe Flood Warning EA
(013 is an ‘area code’ which will be meaningful to the Flood Warning Service team)
This commit changes the frontend to display the value of the `cap_event`
field, if it’s present, which is where the API stores the value of the
`<event>` field from the original CAP XML.
***
Depends on:
- [x] https://github.com/alphagov/notifications-api/pull/3344/files
You’re supposed to see the two column layout on this page if you have
multiple categories of things to show.
We weren’t counting the ‘platform admin’ section as one of these
categories so platform admin users with only live services or only trial
mode services were inadvertenly seeing a mixture of the one column and
two column layout.
Also this logic around the headings wasn’t tested before – now it is.
I recently found that there is a problem with the
overuse of lists online for sequences of content
that don't need to be in a list. I think that's
what we're doing with our map key.
The problem is large enough that browsers include
heuristics to determine if lists should be
presented as such to the accessibility API (and so
to assistive tech' like screen readers). This
thread contains more details:
https://twitter.com/cookiecrook/status/1337226933822603270
Based on the metric described in the thread:
'If all of the styles that make it “list-like”
have been removed, it’s no longer relevant to
convey it as a list.'
...or
'if a sighted user doesn’t need to know it’s a
list, why would a screen reader user need to know
or want to know?'
Based on that, I think the items in our map key
should be paragraphs, not items in a list.
Also, they read really well as sentences when
announced by a screen reader, partly because of
the extra (hidden) info @quis added to them.
This also doesn't change their visual appearance.
Since we’ve introduced the ‘on behalf of’ wording to the go live ticket
(to talk about who the agreement has been signed on behalf of) it’s
confusing to use the same terminology to talk about the organisation
for whom the agreement has been accepted.
Previously this test asserted on `current_user.is_authenticated`. That
isn’t possible now because the object imported into tests isn’t the same
one the app is using.
A different proxy for whether the user is signed in is whether they have
a user id in their session, because we set this every time they sign in:
ff32e73d9b/app/models/user.py (L162)
This is the newest version.
Pyup is complaining about vulnerabilities in version 1.0.1, specifically
> Werkzeug version 2.0.2 improves the security of the debugger cookies.
> "SameSite" attribute is set to "Strict" instead of "None", and the
> secure flag is added when on HTTPS.
Previously we were using whatever version of Werkzeug that Flask
specified this pins it to get rid of the vulnerability without having to
upgrade everything at once.
This requires a few changes to tests which were relying on importing
`session` and `current_user` from Flask. Previously it seemed that
importing these in the tests referred to the same object that was being
used in the app. This appears to no longer be the case. This commit
works around that by:
- using a context manager to get the contents of the session, like we
already do in most tests
- asserting that the mock which logs the user in is being called with
the right values, rather than looking at the state of the
`current_user` object (which was probably giving false certainty
anyway)
In https://github.com/alphagov/notifications-admin/pull/3663/files we
made specific routes for sending the ‘tour’ text message, rather than
sharing the ‘one-off’ routes in `send.py`.
This commit moves the final route in the tour journey into `tour.py` as
well, which is where I expected to find it when I was looking for it
just now.
The feedback endpoints use `ticket_type` to decide what to display and
whether or not a ticket should be escalated. We were using the
ticket_type as the value for the Zendesk ticket_type. However, the Zendesk
API accepts 4 values for its ticket_type and these are different from
the ticket_type values we use in our code.
This change converts the Notify ticket_type value to a valid Zendesk
ticket_type value when creating a Notify feedback ticket.
The new way of creating support tickets can be seen in
[notifications-utils](https://github.com/alphagov/notifications-utils/pull/899).
This changes tickets created when making a request to go live to use
the new way, while other tickets stay the same for now.
The go live tags have been removed. Some of these had become
unneccessary since you can't make the request to go live unless they are
true (e.g. `notify_go_live_email_reply_to`). Others will always get
added by a Zendesk macro when the ticket is replied to, so we don't need
to add them here.
the api returns UTC timestamps, we should keep them as UTC timestamps
until the very last moment, and only convert them into BST when we know
we want to return to a user (ie: in contact-list.html and other places
like that)
since we are hard-coding a generic error message on the front-end, we
have no need to do anything on the back end. This is also nice as it
standardises the two flows to behave more like each other (rather than
previously where one would `flash` an error message and the other would
return CBOR for the js to decode).
Note that the register flow returns 400 while the auth flow returns 403.
The js for both just checks `response.ok` so will handle both. The JS
completely discards any body returned if the status isn't 200 now.
Add additional instructions for the service name - this is more consistent with the local government version of this page
Also update tests to use the new content.
