I think it’s better to raise an exception and force us to fix it if we ever add a new auth_type, or use this formatter on something that isn’t a valid auth_type.
Co-authored-by: Ben Thorner <benthorner@users.noreply.github.com>
On the ‘find user’ page it says ‘sms_auth’ instead of ‘Text message
code’.
This commit fixes that, and adds a handy formatter so it’s easier to do
the right thing in the future.
Based on comments on the associated pull request,
(and my realisation that I was using flexbox
wrong):
https://github.com/alphagov/notifications-admin/pull/4171
Removal of flex-grow
I added `flex-grow: 1` to make the description and
status flex items grow to fill the container, at
least until they hit their `max-width`. They
already have `width: 100%` which achieves the same
thing so this removes it.
Using `width` doesn't work for flex-items with
flex-grow set above 0 because they will expand to
fill the space, whatever its value.
The intention for this line was always to
constrain how much these ones can grow to half the
space minus the gap between. `max-width`, which is
still honored by flex-items with `flex-grow` above
0, meaning they will expand up to that size.
Having a grid in the click target stops the hint
and status text being clickable. This removes it
in favour using flexbox to position the elements
in a similar way.
Includes fallback styles for browsers that don't
support flexbox that positions the child elements
inline, laid out justified, to mimic the flexbox
positioning of justify-content: space-between.
Note: display is overriden on child items under
flexbox so setting display: inline-block is
ignored.
This makes it clearer we have tests for the code in forms.py, which
I missed initially. In future we could also split up forms.py in a
similar way, as it's currently _very long_.
As part of grouping tests for code in forms.py, I've extracted some
from test_validators.py, so that what remains is focussed on testing
the code in validators.py.
Daily volumes report: total volumes across the platform aggregated by whole business day (bst_date)
Volumes by service report: total volumes per service aggregated by the date range given.
NB: start and end dates are inclusive
This is consistent with all other methods: we clear the cache after
the actual change, not before it.
Since the new version of -utils, we're now catching redis exceptions
on delete, so this change has little effect on behaviour.
This was causing some tests for the "estimate_volume" endpoint to
fail due to the surprising way that form handles "''":
- The form is the exclusive user of the ForgivingIntegerField [^1].
- The field secretly/silently converts "''" to the integer 0 [^2].
If the validations fail, we don't want to surprise the user with a
"0" when they didn't enter one. The field already handles this by
massaging the values in the __call__ method that generates the HTML
for the form [^3]. However, there are two scenarios:
- User submits field with '' - converted to integer 0.
- User submits field with '0' - remains as a string.
In the case where "value" is "''", the parent class will use the
converted value from form.data instead [^4]. This seems to be an
oversight and so we get either the integer 0 (from form.data) or
the string '0' (from the value kwarg). Complicado!
Previously it was a fluke that we avoided replaying the integer 0
to the user; the previous commit removes the fluke. This fixes the
conditional to always use the data in the "value" kwarg if it has
been provided, as it's meant to override "form.data".
[^1]: 9f63449384
[^2]: a22b8cf684/app/main/forms.py (L364)
[^3]: a22b8cf684/app/main/forms.py (L393)
[^4]: a22b8cf684 (diff-a1c8d24b22d4478fe71f75fd43b71b18dd82aae97bc63de84473a6da1902909bR215)
This is to fix a bug where a user creates an account but doesn't
complete registration, then they are invited to a service that
changes their auth to email_auth, and then when they try to
complete registration they are still asked for sms code.
It should save users some pain, and reduce number of support tickets.
So we do not have to go into the db when we need to change user
auth.
We do not allow this for users who use webauthn. We do not want to
enable security downgrade for those users.
Previously we duplicated the "something else" email branding form
on its own page and embedded in the choices form (if it was the
only option). See [^1] for how this looks - it's inconsistent.
This DRYs-up the "something else" form by bypassing the choices
form when "something else" is the only option. I've also tweaked
the "Back" button to be consistent with this behaviour.
Making this change also simplifies the choices form, which we'll
be adding pool options to shortly. I'd like to make the letters
form consistent, but let's see how emails pan out first.
Note that the choices form will now show a single radio button if
"something else" is the only option. I think that's OK as nothing
will link to the page, and the form still works.
[^1]: https://github.com/alphagov/notifications-admin/pull/4163#issuecomment-1050088088
In user research we quite often saw people accidentally scroll the
branding preview `<iframe>` when trying to scroll the page.
This is suboptimal because they:
- were confused why the page wasn’t scrolling
- lost visibility of the branding they were trying to preview because
it scrolled outside the bounds of the `<iframe>`
