`all` is not a real template type, so for links to template folders that
apply to all template types we have a URL that looks like:
```
/services/<uuid:service_id>/templates
```
However Flask only generates this url when `url_for` is called with
`template_type=None`. If called with `template_type=all` then Flask will
generate a URL like
```
/services/<uuid:service_id>/templates/all
```
However attempting to load this URL will now 404, since `all` is not a
template type recognised by the regex introduced in
https://github.com/alphagov/notifications-admin/pull/3176
It would be nice to not have URLs with `all` in them at all, but since
people might have bookmarked them we need to support them indefinitely.
Also considered but decided against adding `all` to the set of template
types because it might cause other problems, for example attempting to
create a new template with a type of `all` would never work.
`all` is not a real template type, so for links to template folders that
apply to all template types we have a URL that looks like:
```
/services/<uuid:service_id>/templates/folders/<uuid:template_folder_id>
```
However Flask only generates this url when `url_for` is called with
`template_type=None`. If called with `template_type=all` then Flask will
generate a URL like
```
/services/<uuid:service_id>/templates/all/folders/<uuid:template_folder_id>
```
However attempting to load this URL will now 404, since `all` is not a
template type recognised by the regex introduced in
https://github.com/alphagov/notifications-admin/pull/3176
It would be nice to not have URLs with `all` in them at all, but since
people might have bookmarked them we need to support them indefinitely.
Also considered but decided against adding `all` to the set of template
types because it might cause other problems, for example attempting to
create a new template with a type of `all` would never work.
It looks weird to have two different visual treatments for showing a
navigable hierarchy.
I reckon losing the slash won’t make things less folder like – Windows
for example uses chevrons as foler separators.
The data flow of other bits of our application looks like this:
```
API (returns JSON)
⬇
API client (returns a built in type, usually `dict`)
⬇
Model (returns an instance, eg of type `Service`)
⬇
View (returns HTML)
```
The user API client was architected weirdly, in that it returned a model
directly, like this:
```
API (returns JSON)
⬇
API client (returns a model, of type `User`, `InvitedUser`, etc)
⬇
View (returns HTML)
```
This mixing of different layers of the application is bad because it
makes it hard to write model code that doesn’t have circular
dependencies. As our application gets more complicated we will be
relying more on models to manage this complexity, so we should make it
easy, not hard to write them.
It also means that most of our mocking was of the User model, not just
the underlying JSON. So it would have been easy to introduce subtle bugs
to the user model, because it wasn’t being comprehensively tested. A lot
of the changed lines of code in this commit mean changing the tests to
mock only the JSON, which means that the model layer gets implicitly
tested.
For those reasons this commit changes the user API client to return
JSON, not an instance of `User` or other models.
This removes the edit_folder_permission checks from the code, enabling
the folder permissions for all services.
This also fixes folder-related tests to set up appropriate user
permissions.
This should only be merged right after alphagov/notifications-api#2428,
when all other permission stories are done.
The template list wasn’t getting the right class applied because the
check was referring to an undefined variable (`can_manage_folders`) that
should have been removed when all other references to it were.
Sometimes, when filtering the view by template type, the message was
saying the folder was empty, when really it should have been saying that
there were no templates of the ‘type’ you’re looking for, eg:
> There are no email templates in this folder
This was because it was looking at the filtered list of templates, not
all templates to determine whether a folder was really empty.
It’s a bit rudimentary to only show the current place in the hierarchy
and the parent. You lose a sense of how deep you are.
But we can’t just show the full path, because it can be arbitrarily
long. So what this commit does is show the full path, but truncates the
display of any items. Further-up than the current folder or its parent.
This also helps disambiguate between folders and templates, because
folders are always shown with the folder icon.
This probably won’t affect many teams, because we don’t anticipate a lot
of deep nesting.
We already have a pattern for navigation folders and searching for
templates – let’s use it for the copy page too. And I reckon we can
represent services as folders if the user has multiple services they
could copy a template from.
Updated the move folder form to add a hint for the radio button for the
current folder saying 'current folder'. This hint does not get shown if
you are viewing all folders (so you are not inside a folder).
Also stopped a default radio button from being selected on the form.
so that they better align with the front-end, where they'll be used in
data attributes. Also, making the kebab case is nice because it doesn't
give favouritism to either JS or python naming conventions
WTForms coerces `None` as a choice to `'None'` as a string when
rendering form fields (form fields will only ever have string data
because that what the browser posts back).
But internally WTForms coerces `None` to mean an unset value, ie where
the user hasn’t selected a radio button:
283b280320/src/wtforms/utils.py (L1-L20)
We shouldn’t use `None` to mean two different things. And in fact we
can’t, because it in effect means that we’re always getting a value
for the `move_to` field, even if the user hasn’t chosen to move any
templates. Which results in some very expected behaviour.
Since we’re letting users add new folders directly from the choose page
it makes sense that they should also be able to add templates from
there.
This resolves the problem we saw in user research where people found it
hard to know where to go to add a new folder when they were all behind
one green button.
Since you can now see them when searching you should also be able to
select and move them. Which means that they needed to be included in
the `Form`’s list of possible choices of things to move.
If you have a folder structure like this:
> FA / FB / FC
and you search for ‘FA’, it’s very noisy to see:
> FA / FB
> FA / FB / T1
> FA / FB / T2
> FA / FB / FC
> FA / FB / FC / T3
> …
It’s clearer to just show:
> FA / FB
This also has the benefit of, if you type ‘template’ (for example) you
don’t get every item any more, because it’s only looking at the name of
the thing. It used to look at the entire description, ie:
> A / B / C / Example
> Text message template
Now it only searches on the name:
> A / B / C / *Example*
> Text message template
Things in subfolders need to be in the page so we can search for them.
But the default view should be only the things are the current level.
So we can use CSS to hide items that are below the current level.
So that the live search can filter things, they need to be on the page
when it loads. We want to make search work across folders, so all the
things in subfolders need to be in the page.
They also need the full path appending to them, so that you can tell
which ones are in which folders.
This won’t show items that are in a folder above the one you’re
currently in – my reckon is that when you’re narrowing down by clicking
into a folder that you only want to search for things in that folder.
The Jinja template for the ‘choose templates’ page is now pulling in
data from a lot of diparate places in order to work out what to show. As
we add more logic about what to show (in order to make the live search
work) it’s going to get harder to have all this logic in the Jinja
template.
This commit refactors it back into Python where we have more language
features for managing complex logic.
It’s a bit weird to call this file a model, in that it’s dealing with
some presentational logic, rather than just data. Conceptually it’s more
like a view model[1].
1. https://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93viewmodel
If the folder is completely empty, always say ‘This folder is empty’, so
we don’t suggest that you can click around and find other things.
If the current view is filtered, but there are templates available in
other filters, say ‘There are no text message templates in this folder’
to make it clear why you can’t see anything.
Currently if there’s nothing in a folder you just get an empty page.
This looks a bit broken, or like the page hasn’t finished loading.
This commit adds a message to the page to show that it’s intentionally
blank.
The message is contextual based on type of template, because there might
be templates in the current folder, even if you can’t see them at the
moment (because you’re filtering).
It’s weird that this page changes when you click delete – it looks like
you’re going to a different page.
It should feel like you’re on the same page, just with the confirmation
message.
The problem we had before is that the rename form would `POST` to
`…/delete`, which would then delete the template instead of updating
its name.
We can fix this by explicitly setting the `action` attribute on the
rename form to always post to `…/manage`, even if the user is currently
looking at the `…/delete` page.
