Broadcasts created by the API are different in that:
- they aren’t created by any user, so don’t have a `created_by_id`
- they are created instantly, not in steps, so don’t have an
`updated_at` time
This commit alters the views to account for when these pieces of
information aren’t present.
The following were added without the macro ever
being called:
- app/templates/views/add-service-local.html
(added in
e6f49825e5)
- app/templates/views/service-settings/data-retention/edit.html
(added in
4b8b571a87)
- app/templates/views/organisations/organisation/settings/edit-domains.html
(added in
936883bf7b)
The following were used when they were first added
but not removed when the macro stopped being used:
- app/templates/views/edit-letter-template.html
(macro removed in
20ae200de9)
- app/templates/views/organisations/organisation/settings/edit-agreement.html
(macro removed in
45526598c6)
The last_dest_idx variable should always have been
tracking the last index in the source list. The
original intention, implemented incorrectly, was
to just append any items which source has no item
at that index.
Update all methods that were previous calling @cache.delete('service-{service-id}-template-None') to instead call _delete_template_cache_for_service
Remove call to get service templates, it's not needed since all template version cache is being deleted.
If a user has only send_message permissions, when they click on a
template name they are currently taken to the `send_one_off` page. This
is incorrect as if there is more than one SMS sender or email reply to
address, then they should pick the address they wish to use.
This commit fixes that bug by redirecting them to the `set_sender`
route. Note, if there is only one sender then the `set_sender` will
redirect the user on to the `send_one_off` route.
https://www.pivotaltracker.com/story/show/176541486
A comment on the pull request for this branch
pointed out that it's not clear why the 'items'
list is deleted and then reassigned in
extend_params:
https://github.com/alphagov/notifications-admin/pull/3770#pullrequestreview-573067465
The simple reason is that we want to use
merge_jsonlike to merge params and
param_extensions (passed in as extensions) but
merge_jsonlike doesn't merge lists correctly.
I realised that if we just make merge_jsonlike
merge lists correctly, we can use it for
everything extend_params does.
This commit does that, and replaces all calls to
extend_params with merge_jsonlike.
Because extend_params is used across many form
field classes, and so many pages, I took the
following precautions after making those changes:
1. found every use of param_extensions
2. looked at the merges onto params that each would
cause and deduped them to a final list of 6(!)
3. tested pages containing fields from that list
4. added new testcases to the merge_jsonlike tests
for any merges that exist in our codebase but
not in our tests
Current behaviour is to check item-against-item
and merge based on whether items match, irrelevant
of position. This doesn't produce the results we
need for our usecases (merging data to send to
GOVUK Frontend components).
We actually want:
- items to be compared based on their position
- new primitive items at the same position to
overwrite existing ones
- dicts or lists at the same position to be merged
For example,
Starting with this list:
[{"name": "option-1", "value": "1"}]
Merging in this list:
[{"hint": {"text": "Choose one option"}}]
You currently get this:
[
{"name": "option-1", "value": "1"},
{"hint": {"text": "Choose one option"}}
]
We want to get this:
[
{
"name": "option-1", "value": "1",
"hint": {"text": "Choose one option"}
}
]
After talking with the reviewer, it was decided
that:
1. the JS could do with some comments to explain
its structure and what various functions do
better
2. some CSS selectors in the tests don't need to
be as complex and simplifying them makes the
test easier to read
At the moment the admin app expects all broadcasts to have a template,
and expects the content of the alert to come from the template.
This commit makes it so those pages can still get a `Template` instance,
but populated with content straight from the `content` field in the
database.
Form can be pre-filled with existing data upon instantiation.
WTForms will know not to do this on POST request.
Co-authored-by: Chris Hill-Scott <me@quis.cc>
Makes focus shift to the first time in the range
when you select a day.
Also rewrites the code for controlling focus so it
explains itself better, now it has different
settings.
All buttons that open or close a region of the
component should have aria-expanded attributes to
show:
- they have that control
- the state of the region
We think that in some cases alerts will be composed in the moment, and
therefore making people first create a template is:
- not a good use of their time
- adding some conceptual complexity which they don’t need
This commit makes it possible to type some words and have them go
straight into the `content` field in the database.
In the future we might want to progressively enhance the radio buttons
so they show on the same page (like we do with the grey buttons on the
templates page).
The OrganisationAgreementSignedForm class has a
bug causing it to render different HTML when the
page loads to when you subsequently refresh it.
This commit proposes a change to the extend_params
function to fix it.
extend_params, is used by the
OrganisationAgreementSignedForm, as well as all
the other WTForms field classes we added to wrap
GOVUK Frontend components. Fixing it should
therefore fix any similar bugs with them.
All of these fields send a dict of configuration
data to the GOVUK Frontend component when they
call it, at render time. This dict is 'JSON-like',
meaning it's values can be all the primitives as
well as lists and dicts. This also means it can go
quite deep.
Extending the default configuration
The classes have a default dict of this data kept
privately in the params variable. They let you
change it by passing in an argument called
param_extensions on instantiation, after that,
through an attribute of the same name and at
render time as the same argument (in templates).
The extend_params function
The param_extensions dict is used as a collection
of changes to make to the default params dict.
The changes are applied by the extend_params
function. Its code deletes part of the
param_extensions, a side effect that didn't seem a
problem because it isn't used after the function
has run.
The bug
The bug was only with the part of the HTML that
got its data from the part of the param_extensions
dict that was deleted by extend_params. The class
with the bug set param_extensions when the field
is instantiated, as part of its parent form
definition.
My guess is that param_extensions was stored in
memory, as part of the form class, and reused
when the page refreshed. At that point,
extend_params had deleted part of its data,
causing the bug.
jQuery.attr returns `undefined` if an element does not have an
attribute. We want an empty string, rather than the default of coercing
`undefined` to the string `'undefined'`.
