When we get a request for new branding it’s helpful to quickly see what
the service’s current branding is, so we can get a better sense of why
they want to change it.
At the moment it 500s because it can’t format the `None` values as
numbers.
In the future we will stop people requesting to go live until they’ve
provided this info. For now it has to be optional.
We get a bunch of requests to go live where people have told us they're
going to send email but there is no email reply-to address present.
These come from 2 scenarios:
1. when there are email templates, and no reply to address – but they
ignore the checklist
2. when there are no email templates (yet) but they provide anticipated
volumes for email
At the moment we only auto-check for a reply to address when they have
email templates. And because the question about anticipated volumes
follows the checklist, you'll get a checklist that passes (reply
addresses not required as no templates present) - but your future intent
that differs (reply address IS required because you have anticipated
volumes).
So let’s bring the request for anticipated volumes into the checklist,
that way we can dynamically add the requirement for a reply to address
if they say they will send email but don't have templates yet.
We should begin storing it in the database against the service to stop
people having to re-enter it each time they try to complete the go live
screens.
This also means moving the ‘consent to research question’ along with
the questions about volume, because
- we want people to answer both before going live
- we don’t want to clutter up the summary page by asking questions there
too
Most of the existing platform admin buttons on the service settings
page used to issue GET requests to switch service settings. This
means they weren't protected by CSRF. On top of that as our number
of service permissions increases over time a lot of buttons on the
page made it hard to work with.
To fix these issues we replace most of the buttons with rows in the
platform admin settings table. Each setting has a 'Change' link that
leads to a page with an On/Off switch form.
This removes "research mode" switch completely since we're planning
to deprecate it in the future and we don't expect to switch any new
services into research mode at the moment.
Most service permissions are now handled by a shared endpoint that
is parameterized with the permission name. Some permissions that
require some additional logic before they can be toggled (like document
upload, which requires setting a contact address) have separate
initial endpoints that redirect to `set_service_permission`.
"Archive", "Suspend" and "Resume" actions are kept as buttons since
they display a confirmation banner (which is a CSRF-protected form)
and they're not easily represented as an On/Off switch.
This adds a new OnOffField class that implements a boolean field
that is rendered as two On / Off radio buttons. This allows us to
avoid comparing 'on' and 'off' string values in the views since
the field takes care of transforming form data into python booleans.
This also adds a form class that can be used for any single On / Off
switch forms (e.g. service permissions).
This adds a preview pane which is visible when updating a letter brand.
If JavaScript is enabled, the preview pane shows on the set-letter-branding
page, and submitting the form saves updates the letter brand for a service
immediately. If Javascript is not enabled, there is a separate 'Preview email
branding' page which shows a preview of the brand and has a 'Save' button on it.
when clients are defined in app/__init__.py, it increases the chance of
cyclical imports. By moving module level client singletons out to a
separate extensions file, we stop cyclical imports, but keep the same
code flow - the clients are still initialised in `create_app` in
`__init__.py`.
The redis client in particular is no longer separate - previously redis
was set up on the `NotifyAdminAPIClient` base class, but now there's one
singleton in `app.extensions`. This was done so that we can access redis
from outside of the existing clients.
Because some people don’t know they can put their own logo on letters:
> The HM Government Logo is at the top of the letter and we can't see
> a way of putting the [organisation] logo on
> We are intending to use the letter template feature for the first time
> and wondered whether the branding is configurable or whether the HM
> Government header is the standard default.
> Can we replace HM Government logo with our own in the letter? IF yes,
> then how?
> I don't seem to be able to set the branding on the letters to be
> [organisation]. it's always HM government. Is there something that
> needs enabling for this account?
No-one actually wants the HM Government logo (no-one is sending real
letters using it). So we should leave the space blank and put a button
there prompting people to add their own logo.
it wouldn't show search if there were under a certain amount of letter
or email branding options - however we know there will always be more
than that amount so lets remove some complexity.
Also, rename the SearchTemplatesForm because it can search anything -
it just prompts you to search by name is all.
new code is copied stylistically from the email branding patterns.
Instead of `service.dvla_organisation`, there's now
`service.letter_branding` and `service.letter_branding_id`. However,
unlike email branding we're not currently showing a preview of the
logo. That can come later when we work out how we want to do it.
We were getting all letter logos from a method in the email branding
client. Since we will be adding more client methods to deal with
letters, it makes things clearer to separate the email and letter
branding clients.
We are moving from the postage being set on the service to being set on
the template. Once a service has been migrated to have the new
permission they should no longer be able to set the postage at a service
level, only at the template level.
Only users who work for government can accept the terms of use. This
will save us from having to email these requesters back telling them
they need to find someone else to submit the request.
We suggest people format their numbers with commas when telling us how
many things they’re going to send.
This causes problems when we paste these values into a spreadsheet,
because the commas get interpreted as column separators.
If the browser posts the value of `<input value='None'>` to the server
it does so as a string.
We want to post a value of `None` (actually JSON `null`) to the API. To
do this we:
- set the value in the form class to `'None'` (ie a string)
- convert to `None` (as a type) afterwards
However seeing `x = 'None'` in code looks a bit like a mistake. So to
make sure it looks deliberate and clear what is happening this commit:
- makes a reusable constant for `'None'`
- adds a comment explaining why it’s a string
WTForms lets you pre-populate a form’s value by passing it in as an
argument to the constructor.
This will be good for us because it will let us access that value in
the constructor later on.
These helper functions for modifying a service permission were just
floating around loose in the view code.
A much better home for them is on the model. This will also make it
easy to reuse them in other views if we ever need to.
We have a lot of places in settings where we update something by passing
in the `service_id`. `current_service` already knows about `service_id`,
so it’s cleaner to encapsulate these updates inside the model.
Making people use a property is a sure way to make sure they’re spelling
the name of the property correctly, and allows us to easily swap out
properties that call through to the underlying JSON, and properties
which are implemented as methods.
The API should always return something in the JSON for a property, even
if it’s just `None`.
There’s a lot of code in service settings which:
- talks to the API directly through the clients
- passes that information through to the Jinja template
By encapsulating this logic in the service model:
- the Jinja template can access the data directly
- the logic can be reused across multiple methods
Making people use a property is a sure way to make sure they’re spelling
the name of the property correctly, and allows us to easily swap out
properties that call through to the underlying JSON, and properties
which are implemented as methods.
2/3 of our incomplete requests to go live are incomplete because the
Data Sharing and Financial Agreement isn’t signed.
We reckon we can be pushier about this by saying it’s ‘incomplete’ where
we know the agreement is signed.
Where the agreement is signed we should confirm this, rather than make
the line disappear. This is so it makes more sense to someone who sees
it as ‘incomplete’, signs it, then comes back to the page.
If we don’t know whether or not the agreement is signed we should wait
until someone has got in touch with us by requesting to go live to
figure it out. So that’s why we’re not showing that line at all.
This tag was not showing up in the call to the Zendesk API because the
return value of a generator is not included as a member of that
generator (on things `yield`ed from it are).
