Commit Graph

261 Commits

Author SHA1 Message Date
Rebecca Law
0798154fa2 Optimize the query used to return the services and todays notification totals.
By changing the created_at filter to use a specific date range I found a significant improvement to the queries performance.
The unit test needed to change because now were are returning todays date as BST the local timezone.

Query plan before

Merge Left Join  (cost=1226133.76..1226143.77 rows=1753 width=70) (actual time=5800.160..5801.657 rows=1849 loops=1)
Merge Cond: (services.id = anon_1.service_id)
->  Sort  (cost=152.99..157.37 rows=1753 width=46) (actual time=2.205..2.631 rows=1762 loops=1)
Sort Key: services.id
Sort Method: quicksort  Memory: 224kB
->  Seq Scan on services  (cost=0.00..58.54 rows=1753 width=46) (actual time=0.011..1.156 rows=1762 loops=1)
Filter: active
Rows Removed by Filter: 101
->  Sort  (cost=1225980.77..1225980.99 rows=86 width=40) (actual time=5797.949..5797.984 rows=198 loops=1)
Sort Key: anon_1.service_id
Sort Method: quicksort  Memory: 40kB
->  Subquery Scan on anon_1  (cost=1225976.29..1225978.01 rows=86 width=40) (actual time=5797.682..5797.823 rows=198 loops=1)
->  HashAggregate  (cost=1225976.29..1225977.15 rows=86 width=48) (actual time=5797.681..5797.747 rows=198 loops=1)
Group Key: notifications.notification_type, notifications.notification_status, notifications.service_id
->  Seq Scan on notifications  (cost=0.00..1220610.86 rows=536543 width=48) (actual time=0.064..5482.975 rows=643799 loops=1)
Filter: (((key_type)::text <> 'TEST'::text) AND (date(created_at) = '2018-03-20'::date))
Rows Removed by Filter: 6804774
Planning time: 1.106 ms
Execution time: 5802.130 ms

Query plan after
Merge Left Join  (cost=953378.30..953388.30 rows=1753 width=70) (actual time=2380.144..2382.499 rows=1852 loops=1)
Merge Cond: (services.id = anon_1.service_id)
->  Sort  (cost=152.99..157.37 rows=1753 width=46) (actual time=2.944..3.570 rows=1762 loops=1)
Sort Key: services.id
Sort Method: quicksort  Memory: 224kB
->  Seq Scan on services  (cost=0.00..58.54 rows=1753 width=46) (actual time=0.006..1.294 rows=1762 loops=1)
Filter: active
Rows Removed by Filter: 101
->  Sort  (cost=953225.31..953225.53 rows=86 width=40) (actual time=2377.194..2377.262 rows=201 loops=1)
Sort Key: anon_1.service_id
Sort Method: quicksort  Memory: 40kB
->  Subquery Scan on anon_1  (cost=953220.83..953222.55 rows=86 width=40) (actual time=2376.797..2377.034 rows=201 loops=1)
->  HashAggregate  (cost=953220.83..953221.69 rows=86 width=48) (actual time=2376.795..2376.905 rows=201 loops=1)
Group Key: notifications.notification_type, notifications.notification_status, notifications.service_id
->  Bitmap Heap Scan on notifications  (cost=29883.14..947856.24 rows=536459 width=48) (actual time=270.061..1887.754 rows=644735 loops=1)
Recheck Cond: ((created_at >= '2018-03-20 00:00:00'::timestamp without time zone) AND (created_at < '2018-03-21 00:00:00'::timestamp without time zone))
Rows Removed by Index Recheck: 947427
Filter: ((key_type)::text <> 'TEST'::text)
Heap Blocks: exact=40882 lossy=186483
->  Bitmap Index Scan on ix_notifications_created_at  (cost=0.00..29749.02 rows=536459 width=0) (actual time=258.631..258.631 rows=644849 loops=1)
Index Cond: ((created_at >= '2018-03-20 00:00:00'::timestamp without time zone) AND (created_at < '2018-03-21 00:00:00'::timestamp without time zone))
Planning time: 0.548 ms
Execution time: 2383.485 ms
2018-03-20 15:48:32 +00:00
Chris Hill-Scott
c45f0ac516 Fix code style 2018-03-09 09:53:05 +00:00
Rebecca Law
031a19d387 Added a missing test, letter search has not yet been implemented. 2018-03-09 09:53:05 +00:00
Rebecca Law
e3a75d1b7d Notification_type is a required parameter, admin app always passes it in.
Normalise for notificaiton type.
Throw InvalidRequest exception is the notification type is invalid.
2018-03-09 09:53:05 +00:00
Rebecca Law
5f25fc0db4 The template type should be passed in when doing a search by recipent.
It is possible to search for a phone number when from the email notification page and get a SMS message in return.
This also helps to optimise the query.
2018-03-09 09:53:05 +00:00
Ken Tsang
7011b90bd4 Refactor is_precompiled_letter to model 2018-03-07 23:03:03 +00:00
Ken Tsang
bca858f4a8 Return precompiled_letter flag rather than hidden 2018-03-06 12:46:06 +00:00
Ken Tsang
564504bf97 Add template hidden field in response 2018-03-06 12:46:06 +00:00
Ken Tsang
ee9b6f1fe0 Return hidden field as part of json for notification by id
- required by Admin app so that it can distinguish between created and precompiled letters
2018-03-06 12:46:06 +00:00
Leo Hemsted
aedbd37d8b don't return or accept reply_to_email in update service anymore
either
2018-02-22 11:03:35 +00:00
Leo Hemsted
703cee66f0 don't return or accept sms_sender in update service anymore
it's no longer a field on the service
2018-02-21 15:45:19 +00:00
Katie Smith
a6dae2c81b Remove unnecessary code to do with testing service name uniqueness
notifications-admin has now been changed to always pass the service_id
to the 'service/unique' endpoint. This means we don't need to cover the
case of there being no service_id and the tests can also be updated.
2018-02-15 16:32:09 +00:00
Katie Smith
e1cc8175d7 Allow services to add puntuation to or change the case of their name
Changed the '/service/unique' endpoint to optionally accept the
service_id parameter. It now doesn't matter if a user tries to change
the capitalization or add punctuation to their own service name. But
there should still be an error if a user tries to change the punctuation
or capitalization of another service.

service_id needs to be allowed to be None until notifications-admin is
updated to always pass in the service_id.
2018-02-14 10:11:46 +00:00
Ken Tsang
44f9143d08 Organisation services API endpoints 2018-02-13 14:06:43 +00:00
Leo Hemsted
ba20010f27 remove organisation from api 2018-02-07 11:39:33 +00:00
Leo Hemsted
5d00abd6bc update service_email_branding mapping table alongside organisation_id 2018-02-02 12:50:53 +00:00
Chris Hill-Scott
e9aba34c5e Turn letters on by default for new services
Letters is a mature enough feature now – and one that we’ve been talking
about offering for long enough – that we shouldn’t make people dig
around in the settings.

I think we’d want to wait a bit longer/indefinitely before deciding to
turn it on for existing services across the platform.
2018-01-24 13:42:11 +00:00
Katie Smith
b07db16cd1 Get rate limit from service.rate_limit column (not config)
PR #1550 added the rate_limit column to the Service table.

This PR removes the rate limits from the config and uses rate_limit from
the Service model instead. Rate limits are still separated into 'team',
'normal' and 'test', but these values are the same for a service.

Pivotal story https://www.pivotaltracker.com/story/show/153992529
2018-01-11 10:28:11 +00:00
Leo Hemsted
4cdcc4e035 no longer add an annual billing entry creating a service
this is now handled by a separate call from the admin app
2017-12-06 14:45:44 +00:00
Leo Hemsted
a4e2a40134 update dao tests to pass in free sms fragment limit, remove command 2017-12-06 14:45:43 +00:00
Leo Hemsted
b0d4044ff5 remove free_sms_fragment_limit from service
* remove from model
* still required when calling POST /service - we just call through
  from dao_create_service to add a new annual billing entry.
* removed from POST /service/<id> update_service - if you want to
  update/add a new one, use POST /service/<id>/free-sms-fragment-limit
* made sure tests create services with default 250k limit.
2017-12-06 14:45:43 +00:00
Rebecca Law
06a6d37595 Merge branch 'master' into add-crown-column-to-services 2017-12-05 10:28:26 +00:00
Rebecca Law
7fa4e7ffc7 Set crown if organisation_type is updated on service.
Add some tests.
Update the initial values of crown.
2017-12-04 16:07:26 +00:00
Leo Hemsted
5bb6b68e57 fix tests that expect to be rate limited (which is off if redis is disabled) 2017-12-04 14:13:43 +00:00
venusbb
d03280776d CodeStyle correction 2017-11-30 09:38:57 +00:00
venusbb
6a994793dc Merge branch 'master' of https://github.com/alphagov/notifications-api into vb-receipt-callback-dao 2017-11-29 17:39:33 +00:00
venusbb
8dc0455f70 move inbound_api tests into test_callback_rest.py 2017-11-29 16:43:09 +00:00
Leo Hemsted
08494ef206 more flake8. lots of unused imports and variables that didn't get used. i tried to preserve old variable names as comments when it looked like they were useful (eg when they were describing timestamps) 2017-11-28 17:23:09 +00:00
Leo Hemsted
b1cccdcc6b First batch of flake8 changes.
Many unused variables, and replacing some old fixtures with
admin_request before I realised just how many there where 😩
2017-11-28 17:00:01 +00:00
venusbb
a97d946d2d Merge branch 'master' of https://github.com/alphagov/notifications-api into add-reply-to-notifications 2017-11-27 13:53:37 +00:00
Richard Chapman
adfba208c4 Removed the templates/monthly endpoint
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
2017-11-27 11:06:43 +00:00
Rebecca Law
049daa0cb8 Save reply_to_text for one off notiications and csv notificaitons. 2017-11-25 11:31:36 +00:00
Richard Chapman
a812ae1e6f Merge pull request #1412 from alphagov/rc-monthly-template-usage-endpoint
Template usage always aggregating today's stats
2017-11-22 14:05:58 +00:00
Richard Chapman
b453669a7c Added test to ensure todays data isn't aggregated to template usage
The integration tests did test for zero return. Added a method to test
for a year which has no data and also tweaked the existing tests
to ensure they are testing the year more fully.
2017-11-21 16:46:29 +00:00
Chris Hill-Scott
22ec134668 Merge pull request #1376 from alphagov/remove-computed-sms-prefix
Remove computed SMS prefix
2017-11-21 14:29:33 +00:00
Chris Hill-Scott
ac885dd4e6 Remove computed SMS prefix
This is no longer used no that the column in the table (`prefix_sms`) is
populated.
2017-11-21 14:12:35 +00:00
Rebecca Law
577463b0ac Remove create_or_update_email_reply_to and create_or_update_letter_contact - no longer needed.
Remove Services.reply_to_email_address and Services.letter_contact_block
2017-11-20 14:33:15 +00:00
Chris Hill-Scott
3f7aca5f78 Merge pull request #1372 from alphagov/sms-prefix-non-nullable
Make SMS prefix setting non-nullable
2017-11-20 10:15:07 +00:00
Richard Chapman
e7de0c0900 Added a template_type to the rest call
Added a template type which is required for the admin UI and updated the
tests. The rest tests needed updated because of the bug fix for
aggregation.
2017-11-16 15:17:27 +00:00
Chris Hill-Scott
492462d4f9 Make getting a service return prefix_sms column
This gives us the confidence to remove references to the computed
column in https://github.com/alphagov/notifications-admin/pull/1619
2017-11-16 14:29:32 +00:00
Chris Hill-Scott
4eaa482b2d Rewrite existing test to use admin_request fixture
Makes it consistent with the other, related, test in this PR.
2017-11-16 14:29:32 +00:00
Chris Hill-Scott
e8a94422e5 Make SMS prefix setting non-nullable
We have now:
- defaulted new services to start with this column set to `true`
- migrated all preexisting services[1] to have either `true` or `false` set
  for this column

There is no way for a service to switch back from `true`/`false` to
`null`.

This means that we can safely enforce a non-nullable constraint on this
column now.

1. There is a little gotcha: the GOV.UK Notify service still relies on
   the `sms_sender` column. It doesn’t have a row in the
   `service_sms_senders` table. This means the previous migration
   never changed this service’s value for `prefix_sms` from `null`. So
   this commit also changes its value to `False`, so that the rest of
   the migration, of the whole column, is possible.
2017-11-16 14:29:32 +00:00
Richard Chapman
38e8451627 Merge pull request #1390 from alphagov/rc-monthly-template-usage-endpoint
Added new endpoint to get the new template stats
2017-11-15 16:20:04 +00:00
Richard Chapman
1e4850b70f Add Template name to response and filter by year
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.
2017-11-15 15:55:00 +00:00
Richard Chapman
c27edf5e3c Added new endpoint to get the new template stats
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
2017-11-15 12:58:39 +00:00
Rebecca Law
fd217e67f5 Merge branch 'master' into remove-initial-update-sms-sender 2017-11-15 12:33:38 +00:00
Rebecca Law
45bf7e1938 Fix db migration merge conflict
Update create_service_with_inbound_number
2017-11-15 11:00:56 +00:00
Alexey Bezhan
0825177f2d Add tests for notification.template returning the right version
Adds test for:
* checking the template version foreign key constraint
* checking that template changes don't affect existing notifications
* notification statistics aren't affected by different template versions
* notification stats always return the current template name
2017-11-10 15:13:55 +00:00
Rebecca Law
3713cc2353 Merge branch 'master' into remove-initial-update-sms-sender 2017-11-09 11:53:29 +00:00
Rebecca Law
0054361044 Refactor the sms sender code to remove any unused methods.
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.
2017-11-07 14:26:18 +00:00