Which means the sent_at date for the notification could be empty causing the service callback to fail.
- Allow code to work if notification.sent_at or updated_at is None
- Update calls to send_delivery_status_to_service to send the data encrypted so that the task does not need to use the db.
This PR will optimize this query to use a more efficient index.
- Add notification_type to the dao_get_last_template_usage to optimize the query.
- Tested and analyzed query on production database with very significant results.
Before:
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Limit (cost=0.43..1711.35 rows=1 width=935) (actual time=21186.053..21186.053 rows=0 loops=1)
-> Index Scan Backward using ix_notifications_created_at on notifications (cost=0.43..4607493.80 rows=2693 width=935) (actual time=21186.052..21186.052 rows=0 loops=1)
Filter: (((key_type)::text <> 'test'::text) AND (template_id = 'xxxxxx'::uuid))
Rows Removed by Filter: 8244071
Planning time: 0.112 ms
Execution time: 21186.082 ms
After:
QUERY PLAN
---------------------------------------------------------------------------------------------------------------------
Limit (cost=5323.10..5323.10 rows=1 width=935)
-> Sort (cost=5323.10..5323.74 rows=258 width=935)
Sort Key: created_at DESC
-> Index Scan using ix_notifications_template_id on notifications (cost=0.56..5321.81 rows=258 width=935)
Index Cond: (template_id = 'xxxxx'::uuid)
Filter: (((key_type)::text <> 'test'::text) AND (notification_type = 'sms'::notification_type))
Planning time: 1.102 ms
Execution time: 0.584 ms
We need to deal with this, it's ok when updating a notification status from delivered to delivered. But the DailySortedLetter counts are being doubled.
Adding the file_name to the table as a unique key to get around this issue. It will mean we have multiple rows for each billing_day, but that's ok we can aggregate that.
This will also give us a way to see which file created which count.
this is so that we can filter out inactive organisations and services
note: can't remove user schema completely, as we still use it in
POST /user to create new users
and the client reference. It could confuse the client consumer so best
to remove it from the response altogether.
* Respond with only the notification_id and client_reference
* Updated the test to check for the response without the template
The template in precompiled is only used internally and cannot be
accessed from the UI. It could confuse the client consumer so best to
remove it from the response altogether.
* Remove the template from the dict before returning it and therefore
ensuring it still stays similar to letter
* Updated the test to check for the response without the template
SQLAlchemy handles escaping anything that could allow a SQL injection
attack. But it doesn’t escape the characters used for wildcard
searching. This is the reason we’re able to do `.like('%example%')`
at all.
But we shouldn’t be letting our users search with wildcard characters,
so we need to escape them. Which is what this commit does.
We were previously expecting the letter response files to be in the
format of 'NOTIFY.<datetime>.RSP.TXT' but the response files we receive
use '-' in the filenames instead of '.' which was causing an error when
we tried to get the date from the filename.
In the 'update-letter-notifications-statuses' task we want to ensure
that temporary failures are always handled, regardless of whether the
response file we receive contains unknown Sorted statuses or not.
Added the following new notification statuses:
* pending_virus_check
* virus_scan_failed
If we decide to remove these statuses in future, we will need to replace
them with a different status in the notifications and
Notification_history tables where they are referenced, so
pending-virus-check will be replaced with sending, and virus-scan-failed
will be replaced with permanent-failure.
When the notification is timedout by the scheduled task if the service is expecting a status update, that update to the service would fail.
A test has been added.
* Removed extra log messages so there are not two log messages being
generated per exception, as InvalidRequest also logs, updated the
InvalidRequest log message to include the exception type and exception
information
* Added extra asserts to ensure the exception messages are printed
* Deleted the statistics DAO
(this was used for the job statistics tasks)
* Deleted the functions in the jobs DAO which are no longer used
(the functions that were used for the job-stats endpoints)
The tasks are no longer being used, so can be deleted safely:
* record_initial_job_statistics
* record_outcome_job_statistics
* timeout-job-statistics
The test file for the statistics tasks was deleted in a previous commit.
process_incomplete_jobs loops through jobs processing them in a single
task. This means that if the job statuses are all 'error', and then the
process_incomplete_jobs task fails, the later jobs in the list that
never got picked up won't have their status set back to in progress, or
their processing_started time - so will be stuck in 'error' forever.
Instead, we set the job statuses to in progress and the start time to
now before we process any - so if the incomplete_jobs task fails later,
the jobs will be picked up (again) by the check_job_statuses task in
half an hour's time
the process_incomplete_jobs task runs through all incomplete jobs in
a loop, so it might not get a chance to update the processing_started
time of the last job before check_job_status runs again (every minute).
So before we even trigger the process_incomplete_jobs task, lets set
the status of the jobs to error, so that we don't identify them for
re-processing again.
we might stop processing jobs mid-way through if, for example, a
deploy or downscale kills the box working on it. We have a scheduled
task that identifies any job that we started processing more than half
an hour ago that is still processing.
However, we encountered a bug where we triggered the
process_incomplete_job multiple times, because the processing_started
of the job was still set to half an hour ago. If we reset the
processing_started to the current time, then it won't get picked up by
future runs of the check_job_status scheduled task.
before sending it to template preview. This stops the whole pdf file
being sent to template preview for each page which is really inefficient
on network traffic and memory usage.
* Added logic to the endpoint to extract the specific page requested
* Updated tests to add a mock for the new call to utils
* Added a new test case for exceptions in the PDF extraction process
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.
Phone numbers sometimes contain stuff we normalise out. This matches
perfectly if we have a full phone number, because we can normalise the
thing we’re searching for in the same way as the search term.
With partial search terms we can’t do this completely, because we can’t
work out if ‘123’ is part of a UK number, an international number, the
start of the phone number, the last 3 digits, etc.
What we can do is remove some stuff that we can know will cause partial
search terms to not match:
- leading pluses
- leading `0`s
- any brackets
- any spaces
Users expect the search to work on partial email addresses ‘similar to
Gov.Pay’. We can’t have a situation where Pay are doing something better
than us 😜
Which means we can remove the need to request the data from the database.
In order for the PR to be backwards compatible I have added an optional parameter "encrypted_status_update".
If this is not None then the new code is called.
The next PR will send the encrypted data to this task.
A final PR will remove the code that uses the database to get the notification and service callback api.
