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.
There are no links to the `webauthn_begin_register` route - you are only
taken there if you are logged in and have clicked to register a key.
However, we have seen this route being crawled by bots making a GET
request which gives a `500` status code error because there isn't a
logged in current_user. For consistency, this also adds teh decorator to
the POST route.
The browser uses the `width` and `height` attributes of the image tag to
allocate space on the page for the image.
If these aren’t provided then the browser will assume the image takes up
no space, until it’s downloaded it and had a look at what the file’s
dimensions are. This causes the layout of the page to jump once the
image downloads.
`149 × 150px` is the native size of the image. But we don’t want it to
display at that size, so this commit also adds some extra CSS which
keeps it looking the same, namely:
- the full width of the 1/4 page column on desktop
- the full width of the column minus a `40px` gutter either side on
mobile (by using `box-sizing: border-box` the `40px` of padding is
subtracted from the 100% width, rather than added to it)
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.
When the component was renamed from ‘API key’ to ‘Copy to clipboard’ the class for the thing to be copied changed from `api-key__key` to `copy-to-clipboard__value`. While the CSS was updated to reflect the change from `api-key` to `copy-to-clipboard` the change from `__key` to `__value` was not made.
Before: 4921e6d46e/app/templates/components/api-key.html
After: 85f6881a56/app/templates/components/copy-to-clipboard.html
This commit changes updates the CSS to reflect the latter change, so that the styles get applied properly.
We now use `sass`, a JavaScript version of Sass,
compiled from dart-sass*, so shouldn't need to
rebuild it to solve issues with C libraries.
*https://github.com/sass/dart-sass
The intention behind the version of node in the
engines property was for that version to be the
minimum required so it was always missing the `>=`
prefix.
This adds that prefix and also adds a setting for
npm, to prevent use of insecure versions. See this
article for details:
https://github.blog/2021-09-08-github-security-update-vulnerabilities-tar-npmcli-arborist/
The way we're using the updateContent.js code is
slightly different to expected and to the
scenarios in our tests. This changes the
tests to match that use.
The expected behaviour was for updates to a
module's HTML to happen to the HTML inside of the
div[data-module=update-content] element.
So with initial HTML of:
<div data-module="update-content" data-key="one">
<div class="ajax-block-container">
Existing content
</div>
</div>
...should be updated to be:
<div data-module="update-content" data-key="one">
<div class="ajax-block-container">
New content
</div>
</div>
Instead the HTML returned by the AJAX requests
replaced the div[data-module=update-content]
element.
So with initial HTML of:
<div data-module="update-content" ..>
<div class="ajax-block-container">
Existing content
</div>
</div>
...will be updated to be:
<div class="ajax-block-container">
New content
</div>
This doesn't seem to create any noticable changes
to the visual interface so, I think, went
unnoticed. The assumption I am making, of this
being unintended, is based on the fact that the
div[data-module=update-content] element has an
aria-live attribute, which authors would normally
want to stay in the page when updates happen.
Note: This commit doesn't try and fix the problem,
as the behaviour still largely works and the lack
of aria-live actually seems to be a positive
thing, meaning non-visual users aren't told of
every update but can discover it themselves if
needed.
A while ago diffDOM moved its code to use ES6
modules and started using various language
features specific to ES6. These two things
happened independently btw.
The result of this is that the version of diffDOM
suitable for our build pipeline, structured as an
immediately invoked function evocation (IIFE),
now requires polyfills of some ES6 features to
work in the older browsers we support, like IE11.
It's also worth noting that in the move to ES6
the maintainers of diffDOM have adopted a process
whereby users who need to support older browsers
now have to add polyfill code for any ES6 features
they choose to use.
This commmit proposes a move to the domdiff
library instead because:
- it runs on all javascript runtimes with no
polyfills
- it is 2KB instead of diffDOM's 25KB
Domdiff takes a different approach to diffDOM, in
that it compares existing nodes and new nodes and
replaces the existing ones with the new ones if
there are differences. By contrast, diffDOM will
make in-place changes to nodes if there are enough
similarities. In other words, in most situations,
diffDOM won't change the node in $component
whereas domdiff will.
Because of this, I've had to change the
updateContent.js code to cache the data-key
attribute's value so we don't lose access to it by
overwrite the $component variable with a different
jQuery selection.
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 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 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.
turns out that we're only using errorBanner with a static message, and
it's also full of rich html content. This means that it's probably
better to put it in the html templates with other content, rather than
hidden away in js files if we can help it.
Since there are two places, had to dupe the error message but i think
that's fine as i don't anticipate this error message being used in
significantly more places.
making it a string is a bit gross and means we don't get nice syntax
highlighting on it, but as it needs to be passed in to a jinja macro
that's the way it has to go unfortunately.
the banner is a nicer user experience, and consistent with how we
display errors elsewhere in notify. For now pass through the error
message from JS, but we'll probably want to change that since the erorr
messages themselves are often a bit cryptic and unhelpful
this ensures it's reusable by other components, and easier to unit test
by isolating the separate concerns
note: this is not in Modules since that's designed for classes that are
then bound to an element in the DOM as indicated by a data-module
attribute. This will just live at the window.GOVUK level since we want
there to only ever be one `.banner-dangerous` warning.
