mirror of
https://github.com/GSA/notifications-api.git
synced 2026-09-11 10:28:55 -04:00
Archive, don’t delete contact lists
So we keep a record of who first uploaded a list it’s better to archive a list than completely delete it. The list in the database doesn’t contain any recipient info so this isn’t a change to what data we’re retaining. This means updating the endpoints that get contact lists to exclude ones that are archived.
This commit is contained in:
@@ -135,14 +135,6 @@ def dao_update_job(job):
|
|||||||
db.session.commit()
|
db.session.commit()
|
||||||
|
|
||||||
|
|
||||||
def dao_unlink_jobs_from_contact_list_id(contact_list_id):
|
|
||||||
Job.query.filter(
|
|
||||||
Job.contact_list_id == contact_list_id,
|
|
||||||
).update({
|
|
||||||
'contact_list_id': None,
|
|
||||||
})
|
|
||||||
|
|
||||||
|
|
||||||
def dao_get_jobs_older_than_data_retention(notification_types):
|
def dao_get_jobs_older_than_data_retention(notification_types):
|
||||||
flexible_data_retention = ServiceDataRetention.query.filter(
|
flexible_data_retention = ServiceDataRetention.query.filter(
|
||||||
ServiceDataRetention.notification_type.in_(notification_types)
|
ServiceDataRetention.notification_type.in_(notification_types)
|
||||||
|
|||||||
@@ -5,7 +5,8 @@ from app.models import ServiceContactList
|
|||||||
def dao_get_contact_list_by_id(service_id, contact_list_id):
|
def dao_get_contact_list_by_id(service_id, contact_list_id):
|
||||||
contact_list = ServiceContactList.query.filter_by(
|
contact_list = ServiceContactList.query.filter_by(
|
||||||
service_id=service_id,
|
service_id=service_id,
|
||||||
id=contact_list_id
|
id=contact_list_id,
|
||||||
|
archived=False,
|
||||||
).one()
|
).one()
|
||||||
|
|
||||||
return contact_list
|
return contact_list
|
||||||
@@ -13,7 +14,8 @@ def dao_get_contact_list_by_id(service_id, contact_list_id):
|
|||||||
|
|
||||||
def dao_get_contact_lists(service_id):
|
def dao_get_contact_lists(service_id):
|
||||||
contact_lists = ServiceContactList.query.filter_by(
|
contact_lists = ServiceContactList.query.filter_by(
|
||||||
service_id=service_id
|
service_id=service_id,
|
||||||
|
archived=False,
|
||||||
).order_by(
|
).order_by(
|
||||||
ServiceContactList.created_at.desc()
|
ServiceContactList.created_at.desc()
|
||||||
)
|
)
|
||||||
@@ -25,6 +27,7 @@ def save_service_contact_list(service_contact_list):
|
|||||||
db.session.commit()
|
db.session.commit()
|
||||||
|
|
||||||
|
|
||||||
def dao_delete_contact_list(service_contact_list):
|
def dao_archive_contact_list(service_contact_list):
|
||||||
db.session.delete(service_contact_list)
|
service_contact_list.archived = True
|
||||||
|
db.session.add(service_contact_list)
|
||||||
db.session.commit()
|
db.session.commit()
|
||||||
|
|||||||
@@ -2135,6 +2135,7 @@ class ServiceContactList(db.Model):
|
|||||||
created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=True)
|
created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=True)
|
||||||
created_at = db.Column(db.DateTime, nullable=False)
|
created_at = db.Column(db.DateTime, nullable=False)
|
||||||
updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow)
|
updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow)
|
||||||
|
archived = db.Column(db.Boolean, nullable=False, default=False)
|
||||||
|
|
||||||
def serialize(self):
|
def serialize(self):
|
||||||
created_at_in_bst = convert_utc_to_bst(self.created_at)
|
created_at_in_bst = convert_utc_to_bst(self.created_at)
|
||||||
|
|||||||
+3
-5
@@ -30,7 +30,6 @@ from app.dao.fact_notification_status_dao import (
|
|||||||
fetch_stats_for_all_services_by_date_range, fetch_monthly_template_usage_for_service
|
fetch_stats_for_all_services_by_date_range, fetch_monthly_template_usage_for_service
|
||||||
)
|
)
|
||||||
from app.dao.inbound_numbers_dao import dao_allocate_number_for_service
|
from app.dao.inbound_numbers_dao import dao_allocate_number_for_service
|
||||||
from app.dao.jobs_dao import dao_unlink_jobs_from_contact_list_id
|
|
||||||
from app.dao.organisation_dao import dao_get_organisation_by_service_id
|
from app.dao.organisation_dao import dao_get_organisation_by_service_id
|
||||||
from app.dao.returned_letters_dao import (
|
from app.dao.returned_letters_dao import (
|
||||||
fetch_most_recent_returned_letter,
|
fetch_most_recent_returned_letter,
|
||||||
@@ -39,10 +38,10 @@ from app.dao.returned_letters_dao import (
|
|||||||
fetch_returned_letters,
|
fetch_returned_letters,
|
||||||
)
|
)
|
||||||
from app.dao.service_contact_list_dao import (
|
from app.dao.service_contact_list_dao import (
|
||||||
|
dao_archive_contact_list,
|
||||||
dao_get_contact_lists,
|
dao_get_contact_lists,
|
||||||
save_service_contact_list,
|
|
||||||
dao_get_contact_list_by_id,
|
dao_get_contact_list_by_id,
|
||||||
dao_delete_contact_list,
|
save_service_contact_list,
|
||||||
)
|
)
|
||||||
from app.dao.service_data_retention_dao import (
|
from app.dao.service_data_retention_dao import (
|
||||||
fetch_service_data_retention,
|
fetch_service_data_retention,
|
||||||
@@ -1046,8 +1045,7 @@ def delete_contact_list_by_id(service_id, contact_list_id):
|
|||||||
service_id=service_id,
|
service_id=service_id,
|
||||||
contact_list_id=contact_list_id,
|
contact_list_id=contact_list_id,
|
||||||
)
|
)
|
||||||
dao_unlink_jobs_from_contact_list_id(contact_list.id)
|
dao_archive_contact_list(contact_list)
|
||||||
dao_delete_contact_list(contact_list)
|
|
||||||
s3.remove_contact_list_from_s3(service_id, contact_list_id)
|
s3.remove_contact_list_from_s3(service_id, contact_list_id)
|
||||||
|
|
||||||
return '', 204
|
return '', 204
|
||||||
|
|||||||
@@ -0,0 +1,26 @@
|
|||||||
|
"""
|
||||||
|
|
||||||
|
Revision ID: 0319_contact_list_archived
|
||||||
|
Revises: 0318_service_contact_list
|
||||||
|
Create Date: 2020-03-26 11:16:12.389524
|
||||||
|
|
||||||
|
"""
|
||||||
|
from alembic import op
|
||||||
|
import sqlalchemy as sa
|
||||||
|
|
||||||
|
revision = '0319_contact_list_archived'
|
||||||
|
down_revision = '0318_service_contact_list'
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade():
|
||||||
|
op.add_column(
|
||||||
|
'service_contact_list',
|
||||||
|
sa.Column('archived', sa.Boolean(), nullable=False, server_default=sa.false()),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade():
|
||||||
|
op.drop_column(
|
||||||
|
'service_contact_list',
|
||||||
|
'archived',
|
||||||
|
)
|
||||||
@@ -4,6 +4,11 @@ from tests.app.db import create_service_contact_list
|
|||||||
|
|
||||||
def test_dao_get_contact_lists(notify_db_session):
|
def test_dao_get_contact_lists(notify_db_session):
|
||||||
contact_list = create_service_contact_list()
|
contact_list = create_service_contact_list()
|
||||||
|
create_service_contact_list(
|
||||||
|
service=contact_list.service,
|
||||||
|
archived=True,
|
||||||
|
)
|
||||||
|
|
||||||
fetched_list = dao_get_contact_lists(contact_list.service_id)
|
fetched_list = dao_get_contact_lists(contact_list.service_id)
|
||||||
|
|
||||||
assert len(fetched_list) == 1
|
assert len(fetched_list) == 1
|
||||||
|
|||||||
+3
-1
@@ -976,7 +976,8 @@ def create_service_contact_list(
|
|||||||
original_file_name='EmergencyContactList.xls',
|
original_file_name='EmergencyContactList.xls',
|
||||||
row_count=100,
|
row_count=100,
|
||||||
template_type='email',
|
template_type='email',
|
||||||
created_by_id=None
|
created_by_id=None,
|
||||||
|
archived=False,
|
||||||
):
|
):
|
||||||
if not service:
|
if not service:
|
||||||
service = create_service(service_name='service for contact list', user=create_user())
|
service = create_service(service_name='service for contact list', user=create_user())
|
||||||
@@ -988,6 +989,7 @@ def create_service_contact_list(
|
|||||||
template_type=template_type,
|
template_type=template_type,
|
||||||
created_by_id=created_by_id or service.users[0].id,
|
created_by_id=created_by_id or service.users[0].id,
|
||||||
created_at=datetime.utcnow(),
|
created_at=datetime.utcnow(),
|
||||||
|
archived=archived,
|
||||||
)
|
)
|
||||||
db.session.add(contact_list)
|
db.session.add(contact_list)
|
||||||
db.session.commit()
|
db.session.commit()
|
||||||
|
|||||||
@@ -238,12 +238,21 @@ def test_create_scheduled_job(client, sample_template, mocker, fake_uuid):
|
|||||||
assert resp_json['data']['notification_count'] == 1
|
assert resp_json['data']['notification_count'] == 1
|
||||||
|
|
||||||
|
|
||||||
def test_create_job_with_contact_list_id(client, mocker, sample_template, fake_uuid):
|
@pytest.mark.parametrize('contact_list_archived', (
|
||||||
|
True, False,
|
||||||
|
))
|
||||||
|
def test_create_job_with_contact_list_id(
|
||||||
|
client,
|
||||||
|
mocker,
|
||||||
|
sample_template,
|
||||||
|
fake_uuid,
|
||||||
|
contact_list_archived,
|
||||||
|
):
|
||||||
mocker.patch('app.celery.tasks.process_job.apply_async')
|
mocker.patch('app.celery.tasks.process_job.apply_async')
|
||||||
mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
|
mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
|
||||||
'template_id': str(sample_template.id)
|
'template_id': str(sample_template.id)
|
||||||
})
|
})
|
||||||
contact_list = create_service_contact_list()
|
contact_list = create_service_contact_list(archived=contact_list_archived)
|
||||||
data = {
|
data = {
|
||||||
'id': fake_uuid,
|
'id': fake_uuid,
|
||||||
'valid': 'True',
|
'valid': 'True',
|
||||||
|
|||||||
@@ -75,6 +75,7 @@ def test_get_contact_list_returns_for_service(admin_request, notify_db_session):
|
|||||||
expected_list_2 = create_service_contact_list(service=service_1)
|
expected_list_2 = create_service_contact_list(service=service_1)
|
||||||
# not included in results
|
# not included in results
|
||||||
create_service_contact_list(service=service_2)
|
create_service_contact_list(service=service_2)
|
||||||
|
create_service_contact_list(service=service_1, archived=True)
|
||||||
|
|
||||||
response = admin_request.get(
|
response = admin_request.get(
|
||||||
'service.get_contact_list',
|
'service.get_contact_list',
|
||||||
@@ -101,6 +102,16 @@ def test_dao_get_contact_list_by_id(admin_request, sample_service):
|
|||||||
assert response == expected_list_1.serialize()
|
assert response == expected_list_1.serialize()
|
||||||
|
|
||||||
|
|
||||||
|
def test_dao_get_archived_contact_list_by_id(admin_request):
|
||||||
|
contact_list = create_service_contact_list(archived=True)
|
||||||
|
admin_request.get(
|
||||||
|
'service.get_contact_list_by_id',
|
||||||
|
service_id=contact_list.service.id,
|
||||||
|
contact_list_id=contact_list.id,
|
||||||
|
_expected_status=404,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_dao_get_contact_list_by_id_does_not_return_if_contact_list_id_for_another_service(
|
def test_dao_get_contact_list_by_id_does_not_return_if_contact_list_id_for_another_service(
|
||||||
admin_request, sample_service
|
admin_request, sample_service
|
||||||
):
|
):
|
||||||
@@ -120,7 +131,7 @@ def test_dao_get_contact_list_by_id_does_not_return_if_contact_list_id_for_anoth
|
|||||||
assert response['message'] == "No result found"
|
assert response['message'] == "No result found"
|
||||||
|
|
||||||
|
|
||||||
def test_dao_delete_contact_list_by_id(mocker, admin_request, sample_service):
|
def test_archive_contact_list_by_id(mocker, admin_request, sample_service):
|
||||||
mock_s3 = mocker.patch('app.service.rest.s3.remove_contact_list_from_s3')
|
mock_s3 = mocker.patch('app.service.rest.s3.remove_contact_list_from_s3')
|
||||||
service_1 = create_service(service_name='Service under test')
|
service_1 = create_service(service_name='Service under test')
|
||||||
template_1 = create_template(service=service_1)
|
template_1 = create_template(service=service_1)
|
||||||
@@ -137,8 +148,12 @@ def test_dao_delete_contact_list_by_id(mocker, admin_request, sample_service):
|
|||||||
contact_list_id=expected_list.id,
|
contact_list_id=expected_list.id,
|
||||||
)
|
)
|
||||||
|
|
||||||
assert job_1.contact_list_id is None
|
assert job_1.contact_list_id == expected_list.id
|
||||||
|
assert expected_list.archived is True
|
||||||
|
|
||||||
assert job_2.contact_list_id == other_list.id
|
assert job_2.contact_list_id == other_list.id
|
||||||
|
assert other_list.archived is False
|
||||||
|
|
||||||
assert job_3.contact_list_id is None
|
assert job_3.contact_list_id is None
|
||||||
|
|
||||||
mock_s3.assert_called_once_with(
|
mock_s3.assert_called_once_with(
|
||||||
@@ -147,7 +162,7 @@ def test_dao_delete_contact_list_by_id(mocker, admin_request, sample_service):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_dao_delete_contact_list_when_unused(mocker, admin_request, sample_service):
|
def test_archive_contact_list_when_unused(mocker, admin_request, sample_service):
|
||||||
mock_s3 = mocker.patch('app.service.rest.s3.remove_contact_list_from_s3')
|
mock_s3 = mocker.patch('app.service.rest.s3.remove_contact_list_from_s3')
|
||||||
service = create_service(service_name='Service under test')
|
service = create_service(service_name='Service under test')
|
||||||
expected_list = create_service_contact_list(service=service)
|
expected_list = create_service_contact_list(service=service)
|
||||||
@@ -160,7 +175,8 @@ def test_dao_delete_contact_list_when_unused(mocker, admin_request, sample_servi
|
|||||||
contact_list_id=expected_list.id
|
contact_list_id=expected_list.id
|
||||||
)
|
)
|
||||||
|
|
||||||
assert ServiceContactList.query.count() == 0
|
assert ServiceContactList.query.count() == 1
|
||||||
|
assert expected_list.archived is True
|
||||||
|
|
||||||
mock_s3.assert_called_once_with(
|
mock_s3.assert_called_once_with(
|
||||||
expected_list.service.id,
|
expected_list.service.id,
|
||||||
@@ -168,7 +184,7 @@ def test_dao_delete_contact_list_when_unused(mocker, admin_request, sample_servi
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_dao_delete_contact_list_by_id_for_different_service(mocker, admin_request, sample_service):
|
def test_archive_contact_list_by_id_for_different_service(mocker, admin_request, sample_service):
|
||||||
|
|
||||||
mock_s3 = mocker.patch('app.service.rest.s3.remove_contact_list_from_s3')
|
mock_s3 = mocker.patch('app.service.rest.s3.remove_contact_list_from_s3')
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user