The fieldset that wraps the collapsible checkboxes
has an aria-describedby to make the summary its
accessible description.
This needs to point to the id of the summary but
the summary didn't have one.
These changes add the id and fix a fixture in the
tests for this module.
The links in the blue boxes on the job page needed hidden text so that
they work out of context. This changes the text from "10 sending" to "10
sending text messages" (with the message type hidden text).
WTForms now renders the `required` attribute if there is a validator
such as `DataRequired`. This was flagged in an accessibility audit as
being unnecessary since it doesn't conform to the Design System
recommendations, which state that "all form fields are considered
mandatory when navigating a government service unless otherwise denoted
by the word ‘(optional)’."
This uses the approach here https://github.com/wtforms/wtforms/pull/361
to overwrite the `render_field` method.
All services have the `upload_document` permission now, so we don't need
to check for it on the email formatting page. This also deletes a test
which is not needed now.
Moves a commonly used fixture into
`mock_get_service_settings_page_common` so we don't need to keep writing
it every time.
Note, we may be able to do similar in the future with
- single_reply_to_email_address
- single_letter_contact_block
- single_sms_sender
but need a bit more thought about fixing tests that would fail due to
this change and need tweaks to the order of their arguments.
Note, no option at the moment to set the service broadcast account type
as None, or back to without the broadcast permission. This has been done
for speed of development given the chance of us needing this is very
low. We can add it later if we need to.
* We don't need the inset text to be inside `<div>` tags because the
component adds its own
* Replaced `bottom-gutter` with `govuk-!-margin-bottom-6` since this
change will be needed across the app at some point.
This adds spacing classes from the design system where necessary to keep
the spacing looking the same.
It also replaces the `<aside>` elements with a `<div>` on the edit
template pages. The accessibility audit noted that these were inside a
`<main>` element, so screen readers may not be able to navigate the
elements correctly.
This importantly upgrades pyyaml which has a security bug in 5.3.1
Note, strangely, I had to delete the requirements.txt file and rerun to
get these requirements to upgrade, otherwise it kept them in place
(maybe some piptools caching stuff not calculating things if it doesn't
think it has been asked to change them).
This makes a few additional changes to curb the length of this file,
by moving length documentation (the picture is large) into a separate
file, where other documentation could go. It also corrects the section
on requirements, and attempts to make it more concise.
These aren't specific to this repo, and are covered more generally
in the Wiki [1]. Note that:
- The claim about needing multiple Python versions is not true.
- The NPM instructions should be covered by the "make bootstrap".
- The version of Node/NPM is covered by installing the latest one.
[1]: https://github.com/alphagov/notifications-manuals/wiki/Getting-Started
This is more consistent with how we run all other tasks. Note that
the virtual env setup is not generally applicable, and developers
of this repo should follow the guidance in the README.
The fullscreenTable component has 2 layers to the
table you see onscreen:
1. the actual data table
2. a clone, with only the first column showing,
that sits on top so the row numbers stay in
place while you scroll
Table 1. has an id attribute on its caption. The
region wrapping it has an aria-describedby
attribute with the id as its value. This makes the
caption the description for the region.
This isn't needed for the clone and
makes the HTML invalid because ids should be
unique.
This removes the id from the cloned table.
These are leading to multiple failures:
- flake8 fails with various issues
- isort fails with various issues
- pytest fails on a couple of 2FA tests
While we can and should upgrade these dependencies, the priority is
fixing the build so that we can do this reliably.
This downgrades various packages so they are mutually compatible
and "pip install -r requirements.txt" succeeds.
This downgrades to pyyaml 5.3.1, despite it having a security issue,
in order to fix the build for the time being. This also downgrades
dnspython, due to a suspected issue with eventlet [1], which caused
the admin app to start failing with errors like this:
File "/home/vcap/deps/0/python/lib/python3.6/site-packages/urllib3/connectionpool.py", line 1010, in _validate_conn
conn.connect()
File "/home/vcap/deps/0/python/lib/python3.6/site-packages/urllib3/connection.py", line 394, in connect
cert_reqs=resolve_cert_reqs(self.cert_reqs),
File "/home/vcap/deps/0/python/lib/python3.6/site-packages/urllib3/util/ssl_.py", line 303, in create_urllib3_context
context.options |= options
File "/home/vcap/deps/0/python/lib/python3.6/ssl.py", line 465, in options
super(SSLContext, SSLContext).options.__set__(self, value)
File "/home/vcap/deps/0/python/lib/python3.6/ssl.py", line 465, in options
super(SSLContext, SSLContext).options.__set__(self, value)
File "/home/vcap/deps/0/python/lib/python3.6/ssl.py", line 465, in options
super(SSLContext, SSLContext).options.__set__(self, value)
[Previous line repeated 280 more times]
RecursionError: maximum recursion depth exceeded while calling a Python object
[1]: https://github.com/eventlet/eventlet/pull/684
Currently we have a situation where we're not running tests against
new versions of dependencies, as requirements_for_test.txt is not
being kept in-sync with requirements.txt by pyup. Deploys are only
working because Concourse silently ignores version issues.
From a deployment log:
awscli 1.18.211 has requirement PyYAML<5.4,>=3.10; python_version != "3.4", but you'll have pyyaml 5.4 which is incompatible.
This switches to a single requirements file for test dependencies,
in order to keep it in-sync with requirements.txt i.e. we run our
tests against the same versions of dependencies that we deploy with,
and the build fails if we try to use package versions that are not
mutually compatible, as this example PR shows [1].
ERROR: Cannot install -r requirements_for_test.txt (line 17), -r requirements_for_test.txt (line 198) and pyyaml==5.4.1 because these package versions have conflicting dependencies.
We shouldn't need to have fine-grained locking on test dependencies,
beyond those we want to list manually in the file.
[1]: https://github.com/alphagov/notifications-admin/pull/3804
