This changeset pulls in all of the notification_utils code directly into the admin and removes it as an external dependency. We are doing this to cut down on operational maintenance of the project and will begin removing parts of it no longer needed for the admin.
Signed-off-by: Carlo Costino <carlo.costino@gsa.gov>
This is a straight move with a few minor tweaks:
- Some references to self.X in the code from Service now need to
become self.service.X.
- Some reference to self.service.Y in TemplateList can now become
self.Y for the methods that were migrated.
The remaining _template_ methods in Service are still called from
multiple places and there's no performance gain to motivate moving
them out of the Service class, which is now a more manageable size.
Part of moving "get_template_folders" et al. into TemplateList so we
can cache it more effectively. This is slightly less efficient as
iterating a TemplateList will instantiate an object for each item
in the folder; but the difference is minimal.
Note that:
- The default template_type for TemplateList is "all".
- We need to pass realistic template "JSON" in the test now.
This is a very low impact bug since a user can always create such
templates after their service is live and not be subject to checks
we do before that point. Still, we may as well fix it.
The main benefit of this change is actually to contribute towards
moving methods like "get_templates" out of the Service class so
we can simplify and cache their results more effectively.
Note: I wanted to simplify the Service class further as the two
"has_" properties are only used once in the app code. Unfortunately
they are tightly coupled in one of the tests as well [^1].
[^1]: bef0382cca/tests/app/main/views/service_settings/test_service_settings.py (L1961-L1962)
The new way of creating support tickets can be seen in
[notifications-utils](https://github.com/alphagov/notifications-utils/pull/899).
This changes tickets created when making a request to go live to use
the new way, while other tickets stay the same for now.
The go live tags have been removed. Some of these had become
unneccessary since you can't make the request to go live unless they are
true (e.g. `notify_go_live_email_reply_to`). Others will always get
added by a Zendesk macro when the ticket is replied to, so we don't need
to add them here.
Note, no option at the moment to set the service broadcast account type
as None, or back to without the broadcast permission. This has been done
for speed of development given the chance of us needing this is very
low. We can add it later if we need to.
We wrote custom `__getattr__` and `__getitem__` implementation to make
it easier to find code that was accessing fields in the dictionary that
we weren’t aware of. See this commit for details:
b48305c50d
Now that no view-layer code accesses the service dictionary directly we
don’t need to support this behaviour any more. By removing it we can
make the model code simpler, and closer to the `SerialisedModel` it
inherits from.
It still needs some custom implementation because:
- a lot of our test fixtures are lazy and don’t set up all the expected
fields, so we need to account for fields sometimes being present in
the underlying dictionary and sometimes not
- we often implement a property that has the same name as one of the
fields in the JSON, so we have to be careful not to try to override
this property with the value from the underlying JSON
we want to keep track of all broadcast services across govt easily. As
such, when broadcasting is enabled for a service, we've decided we're
going to add the service to a special broadcasting organisation.
This organisation is defined in the config file. It's hard coded for
production, if you want to test locally, you should set
BROADCAST_ORGANISATION_ID in your local environment.
This was broken because current_service doesn’t update itself after
calling the `update` method of the API. So we thought we were changing
the permissions like this:
```
{'email', 'sms', 'letter'}
{'email', 'sms', 'letter', 'broadcast'}
{'sms', 'letter', 'broadcast'}
{'letter', 'broadcast'}
{'broadcast'}
```
But actually we were doing this:
```
{'email', 'sms', 'letter'}
{'email', 'sms', 'letter', 'broadcast'}
{'sms', 'letter'}
{'email', 'letter'}
{'email', 'sms'}
```
This commit changes the code to update the permissions like this:
```
{'email', 'sms', 'letter'}
{'broadcast'}
```
It does so by adding a new method to the service model which changes all
the permissions in one API call, and updates the tests to mock the
underlying API call, not the method on the model.
At the moment the page is the same as for text message templates,
except:
- different H1
- no guidance about personalisation, links, etc (until we decide how
these should work)
For now you won’t be able to really create a broadcast template, because
the API doesn’t support it (the API will respond with a 400). But that’s
OK because no real services have the broadcast permission yet.
This required a bit of refactoring of how we check which template types
a service can use, because there were some hard-coded assumptions about
emails and text messages.
We’re removing it for performance reasons.
This means removing the old pages that edited the letter contact block
when it was stored directly on the service, rather than the current
model where a service can have multiple contact blocks.
All the constructor of the service model is doing is setting a default
value of a property, this is more idiomatically expressed with a custom
property, and means we can get rid of the custom constructor entirely.
We’re caching the organisation name, but still talking to the API
to see if the organisation exists.
`Service().organisation_id` only goes to the JSON for the service.
`Service().organisation` makes a separate API call.
We only need the former to know if a service belongs to an organisation.
