feat: Remove inbound sms from service #4312
Conversation
joybytes
changed the title
joybytes
force-pushed
the
feat/remove-inbound-sms-from-service
branch
3 times, most recently
from
ffede18 to
c271b16
Compare
app/inbound_sms/rest.py
Outdated
| dao_remove_service_permission(service_id, INBOUND_SMS_TYPE) | ||
| dao_remove_inbound_sms_senders(service_id) | ||
| archive_or_release_inbound_number_for_service(service_id, archive) |
There was a problem hiding this comment.
Here are three database operations. If the second or last transaction fails, data inconsistency may arise. For instance, if a user attempts to archive a number and dao_remove_inbound_sms_senders encounters an exception, archive_or_release_inbound_number_for_service will not be executed. Consequently, only dao_remove_service_permission will be successful, leading to a partial update and potential data corruption.
Transaction allows series of database operations are either all committed to the database or all rolled back if an error occurs. We can follow similar approach https://github.com/alphagov/notifications-api/blob/81af15c03ca0ec7557101752291d8305e658217f/app/dao/services_dao.py#L310C1-L331C1. May be @klssmith or @leohemsted can advise.
There was a problem hiding this comment.
that's a good point actually! well spotted
There was a problem hiding this comment.
joybytes
force-pushed
the
feat/remove-inbound-sms-from-service
branch
2 times, most recently
from
621dd4d to
022a969
Compare
We are automating the following query ``` select * from service_sms_senders where service_id= 'service_id'; select * from inbound_numbers where service_id = 'service_id'; select * from service_permissions where service_id= 'service_id'; delete from service_permissions where service_id = 'service_id' and permission = 'inbound_sms'; delete from service_sms_senders where service_id = 'service_id' and inbound_number_id is not null; -- run one of the two following queries -- if we wish to release the number (if it has never been used) update inbound_numbers set service_id = null, updated_at = now() where service_id = 'service_id' and active; -- if we wish to archive the number instead (if it has been used before so we dont want new services receiving the prev service's messages) update inbound_numbers set service_id = null, updated_at = now(), active = false where service_id = 'service_id' and active; ``` - Remove inbound_sms permission from service - Remove service_sms_sender with inbound_number_id - Disassociate inbound_number with service - If the number is being release, leave 'active' status as True, if being archived, update 'active' to False
…vice - Look through the following tables to get most recent date for inbound sms use: notification, inbound sms and inbound sms history - Endpoint created to showcase information on the front-end in regards to number activity to prevent us from going into the DB to check the latest use for the number when deciding to archive or release this number
joybytes
force-pushed
the
feat/remove-inbound-sms-from-service
branch
from
d5cb226 to
76e4666
Compare
…t/remove-inbound-sms-from-service
| @@ -106,3 +108,15 @@ def _raise_when_no_default(old_default): | |||
| # check that the update is not updating the only default to false | |||
| if not old_default: | |||
| raise Exception("You must have at least one SMS sender as the default.", 400) | |||
|
|
|||
|
|
|||
| def dao_remove_inbound_sms_senders(service_id: UUID, commit=True): | |||
There was a problem hiding this comment.
| def dao_remove_inbound_sms_senders(service_id: UUID, commit=True): | |
| @autocommit | |
| def dao_remove_inbound_sms_senders(service_id: UUID): | |
| result = ( | |
| ServiceSmsSender.query.filter_by(service_id=service_id) | |
| .filter(ServiceSmsSender.inbound_number_id.isnot(None)) | |
| .delete(synchronize_session="fetch") | |
| ) | |
| return result |
I am not sure but is commit=True and flag necessary? transaction is a generator function and it handles the sessions and commits.
remove_inbound_sms_for_service)This process is currently done manually with this query https://github.com/alphagov/notifications-manuals/wiki/Support-Runbook#query-to-remove-the-ability-to-receive-text-messages
This endpoint:
inbound_numbertable disassociates service id from the number and based on the payload (archive: bool) we archive the number by setting theactivefield to false or release by leaving theactivefield as trueget_most_recent_inbound_usage_date):Ticket:
https://trello.com/c/jC9B8l27/1056-build-platform-admin-endpoint-to-remove-inbound-sms-from-service