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.
When running tests locally Pytest returns a lot of captured logging info.
This is redundant because Pytest also captures stdout.
This commit effectively disables logging output when running tests by
setting the log level to higher than anything a real logging call would
ever emit.
The logging output is still captured by stdout, so nothing is lost here,
we’re just reducing duplication.
This replicates how we let large spreadsheets scroll horizontally.
Pro: this looks nicer and is more usable
Con: the code for this feels a bit fragile, especially the calling of
`.maintainWidth` twice, ie as many times as a it takes to get stuff to
render properly.
In trial mode you can’t send letters. But it’s still useful to be able
to build up a letter to see how it work.
Best place to put this error is before someone tries to send a letter
for real.
We didn’t used to allow this because it wasn’t really possible with the
old DVLA set up and we didn’t think there’s a need.
We think it’s possible now because, even though it’s cumbersome, it’s
better than the manual process.
During the migration from the model inhereting from a `dict` to being a
plain object it was useful to have these exceptions raised for quicker
debugging.
Now that all the code which relied on these methods has gone it’s OK
to remove them (attempts to call them will fall through to Python’s
native exception handling).
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
The view here is rebuilding a pseudo-service object. Now that service
objects have templates it’s cleaner to use the actual service object.
Requires a small change to the `templates_by_type` method so that it can
filter by one or many template types (a user should be able to copy any
template whose type is enabled for their service, and the service
they’re copying from).
This commit is the first step to disentangling the models from the API
clients. With the models in the same folder as the API clients it makes
it hard to import the API clients within the model without getting a
circular import.
After this commit the user API clients still has this problem, but at
least the service API client doesn’t.
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.
This means we can have a method on the service model which hits the API
(or Redis) but can be called multiple times (within the context of a
request) without making multiple network requests.
It does this by storing the results of the method on the object’s
internal `__dict__` the first time the method is called.
Inheriting from `dict` has some unexpected side effects that don’t
happen with plain object. The one we want to avoid right now is that
a dict doesn’t seem to implement `__dict__` in a normal way, which
is required by `werkzeug.utils.cached_property`.
We do a lot of logic around choosing which templates to show. This logic
is all inside one view method.
It makes it cleaner to break this logic up into functions. But this
would mean passing around variables from one function to another.
Putting these methods onto a class (the service model) means that
there’s a place to store this data (rather than having to pass it around
a lot).
Making this code more manageable is important so that when we have
templates and folders it’s easy to encapsulate the logic around
combining the two.
`_get_current_service` is a function which gets called every time
`current_service` is referenced in a view method or Jinja template.
Because the service model was getting initialised inside this function
it was being reconstructed many times in one request. On the service
settings page, for example, it was getting initialised 43 times, adding
about 200ms to the response time.
This commit moves its initialisation to the point where we’re getting
the data from the API, which only happens once per request.
We already have tests for the
/services/<service_id>/notifications/<notification_type> page, but these
were not testing the page when there were failed notifications, so this
adds a test for the page in this situation.
There was an awkward line break in this text, and the line length was
too long because there was no grid on the page.
Splitting it into two paragraphs makes it:
- read nicer
- avoids any awkward line breaks
It’s annoying that this button moves after you click on it. It’s
happening because the API key is wrapping onto multiple lines.
This commit fixes the height of the container so that it doesn’t reflow
when it has less content in it.
Uses a bit of flexbox to vertically centre the text.
