- Added the boolean 'crown' column to services and services_history tables
- We populate this column in the same migration script by checking the
'organisation_type' of a service
Removed the REST endpoint and the DAO that it uses as the endpoint is
no longer used by the Admin UI and the DAO is not reused anywhere
else.
- Removed REST endpoint
- Removed DAO which gets the stats
- Removed associated tests of both methods
Currently templates are ordered by the newest created first. This made
sense when, after creating a new template, you were landed on the page
that listed all the templates. In other words, you needed to see
confirmation of the thing that you’ve just done.
Now (since https://github.com/alphagov/notifications-admin/pull/1195)
you get landed on the page for just that template. So you always see
the template you’ve just created, no matter how the list of templates is
ordered. So we are free to change the order of the templates.
Ordering by time created is not great, because it gives users no control
over which templates appear first. For example, our research reported
this from one team:
> One frustration they have is that when you add a new template it
> automatically goes to the top of the list. To get round this, whenever
> they add a new template they delete all of the existing ones and start
> again because they want to keep their templates in numerical order.
> They'd like to be able to control the order of templates in the list.
We _could_ give people some sort of drag-and-drop template ordering
thing. But this feels like overkill. I think that alphabetical order is
better because:
- it’s easily discoverable – anyone who wants to know how a list is
ordered can quickly tell just by looking at it
- it’s universal – everyone knows how alphabetical ordering works
- it’s familiar – this is how people documents on their computer are
ordered; there’s no new UI to learn
- it’s what users are doing already – from the same service as above:
> They number their templates 1,2a, 2b, 3a etc
So this commit changes the ordering from newest created first to
alphabetical.
Previous changes to template order and navigation:
- https://github.com/alphagov/notifications-admin/pull/1163
- https://github.com/alphagov/notifications-admin/pull/1195
- https://github.com/alphagov/notifications-admin/pull/1330
Implementation notes
---
I refactored some of the tests here. I still don’t think they’re great
tests, but they’re a little more Pythonic now at least.
I also added a sort by template type, so that the order is deterministic
when you have, for example, an email template and a text message
template with the same name. If you have two text message templates with
the same name you’re on your own.
Added a check to ensure that the current date falls in between the
financial year for the year supplied by the method, so that the todays
stats will only be appended in that situation.
correctly
The dao_fetch_monthly_historical_usage_by_template_for_service code
wasn't being tested properly due to an bug on the test which created a
notification with the same template and hence was not testing that
a specific service would have a different template id.
- Fixed the bug in the test
- Update services_dao so that the service id check is made
are not returned from queries
- Updated stats_template_usage_by_month_dao.py to return the results for
financial year not calendar, as the report os for FY only and hence
only the FY data is required
- Updated services_dao.py to ensure double precision values are converted
to an int as the 'exact' function returns double precision from the
database query, as the admin code requires the value for month to be an
int
The check for aggregation was too broad and hence was adding together
totals based on template_id and not the unqiue combination of
template id, month and year.
- Added test to test for the failure
- Added check and a test to for template_id, mon and year matches
- Celery process name did not match the task
The template name should be returned for the response and the user will
pick a year, so ths adds those two features to the
notifications/templates_usage/monthly endpoint and added some tests to
test the functionality.
Added a new endpoint which combines the usage of the stats table and the
data from the notifications tables, instead of using all the data from
the notification_history table. This should speed up the query times
and improve the page performance.
- Updated to make the stats create and update function transactional as
it actually wasn't committing the data to the table
- Added the get from the stats table
- Add a a method to combine the two results
- Added the endpoint
Relationship attribute is not used by the application code, so we
can remove it without replacing it with a TemplateHistory one.
This also updates the foreign key constraint to refer to the composite
primary key for the TemplateHistory records.
`Notification.template` changed from being a Template relationship
to TemplateHistory. When a relationship attribute is being assigned,
SQLAlchemy checks that the assigned value type matches the relationship
type. Since most tests at the moment create a notification using a
Template instance this check fails.
Rewriting the tests to use TemplateHistory objects would require
changes to the majority of tests. Instead, when creating a notification
objects we assign the foreign key attributes directly. This skips the
SQLAlchemy type check, but we still get the constraint check on the
foreign keys, so a matching TemplateHistory object needs to exist in
the database.
- Modified the services_dao to return an int instead of a datetime to
make usage easier and removed the BST function on year as it is not
relevant for year
- Improved tests do there is less logic by ordering the result so there
is less reliance on the template id
- Renamed variable in stats_template_usage_by_month_dao.py to make it
consistent with the method
- Some tests for failing because of an import which was overriding the
one above it in test_scheduled_tasks.py
- Import error fixed in test_services_dao.py after a rename of a method
Currently some pages are timing out due to the time it takes to perform
database queries. This is an attempt to improve the performance by
performing the query against the notification history table once a day
and use the notification table for a delta between midnight and the when
the page is run and combine the results.
- Added Celery task for doing the work
- Added a dao to handle the insert and update of the stats table
- Updated tests to test the new functionality
Refactor tests/db/create_service() to behave more like the real world.
Created new create_service_with_inbound_number and create_service_with_defined_sms_sender() test/db methods.
We want new services, when they do the tour, to see how the service name
they just made shows up in the messages. This is how it (should) work
at the moment (although got broken because of the multiple senders
stuff).
Need to do this before we do the migration now otherwise a new service
could sneak in with this setting still set to `null`.
- moved get_current_financial_year_start_year from service.utils to dao.date_utils
- Moved logic for data persistence from rest to dao when updating records in db
NotificationStatistics was added as a spike but didn't work out as expected. This is finally removing all that unused code.
I'll drop the table in the next PR
