Merge pull request #1933 from alphagov/delete-service-stuff

Delete service apis (callback + inbound)

app/commands.py

@@ -616,8 +616,11 @@ def migrate_data_to_ft_notification_status(start_date, end_date):
             """
         result = db.session.execute(sql, {"start": process_date, "end": process_date + timedelta(days=1)})
         db.session.commit()
-        print('ft_notification_status: --- Completed took {}ms. Migrated {} rows.'.format(datetime.now() - start_time,
+        print('ft_notification_status: --- Completed took {}ms. Migrated {} rows for {}.'.format(
-                                                                                          result.rowcount))
+            datetime.now() - start_time,
+            result.rowcount,
+            process_date
+        ))
         process_date += timedelta(days=1)
 
         total_updated += result.rowcount

app/dao/service_callback_api_dao.py

@@ -32,3 +32,8 @@ def get_service_callback_api(service_callback_api_id, service_id):
 
 def get_service_callback_api_for_service(service_id):
     return ServiceCallbackApi.query.filter_by(service_id=service_id).first()
+
+
+@transactional
+def delete_service_callback_api(service_callback_api):
+    db.session.delete(service_callback_api)

app/dao/service_inbound_api_dao.py

@@ -33,3 +33,8 @@ def get_service_inbound_api(service_inbound_api_id, service_id):
 
 def get_service_inbound_api_for_service(service_id):
     return ServiceInboundApi.query.filter_by(service_id=service_id).first()
+
+
+@transactional
+def delete_service_inbound_api(service_inbound_api):
+    db.session.delete(service_inbound_api)

app/service/callback_rest.py

@@ -6,7 +6,8 @@ from flask import (
 from sqlalchemy.exc import SQLAlchemyError
 
 from app.errors import (
-    register_errors
+    register_errors,
+    InvalidRequest
 )
 from app.models import (
     ServiceInboundApi,
@@ -20,12 +21,14 @@ from app.service.service_callback_api_schema import (
 from app.dao.service_inbound_api_dao import (
     save_service_inbound_api,
     get_service_inbound_api,
-    reset_service_inbound_api
+    reset_service_inbound_api,
+    delete_service_inbound_api,
 )
 from app.dao.service_callback_api_dao import (
     save_service_callback_api,
     get_service_callback_api,
-    reset_service_callback_api
+    reset_service_callback_api,
+    delete_service_callback_api,
 )
 
 service_callback_blueprint = Blueprint('service_callback', __name__, url_prefix='/service/<uuid:service_id>')
@@ -61,13 +64,25 @@ def update_service_inbound_api(service_id, inbound_api_id):
     return jsonify(data=to_update.serialize()), 200
 
 
-@service_callback_blueprint.route('/inbound-api/<uuid:inbound_api_id>', methods=["GET"])
+@service_callback_blueprint.route('/inbound-api/<uuid:inbound_api_id>', methods=['GET'])
 def fetch_service_inbound_api(service_id, inbound_api_id):
     inbound_api = get_service_inbound_api(inbound_api_id, service_id)
 
     return jsonify(data=inbound_api.serialize()), 200
 
 
+@service_callback_blueprint.route('/inbound-api/<uuid:inbound_api_id>', methods=['DELETE'])
+def remove_service_inbound_api(service_id, inbound_api_id):
+    inbound_api = get_service_inbound_api(inbound_api_id, service_id)
+
+    if not inbound_api:
+        error = 'Service inbound API not found'
+        raise InvalidRequest(error, status_code=404)
+
+    delete_service_inbound_api(inbound_api)
+    return '', 204
+
+
 @service_callback_blueprint.route('/delivery-receipt-api', methods=['POST'])
 def create_service_callback_api(service_id):
     data = request.get_json()
@@ -103,6 +118,18 @@ def fetch_service_callback_api(service_id, callback_api_id):
     return jsonify(data=callback_api.serialize()), 200
 
 
+@service_callback_blueprint.route('/delivery-receipt-api/<uuid:callback_api_id>', methods=['DELETE'])
+def remove_service_callback_api(service_id, callback_api_id):
+    callback_api = get_service_callback_api(callback_api_id, service_id)
+
+    if not callback_api:
+        error = 'Service delivery receipt callback API not found'
+        raise InvalidRequest(error, status_code=404)
+
+    delete_service_callback_api(callback_api)
+    return '', 204
+
+
 def handle_sql_error(e, table_name):
     if hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \
             and ('duplicate key value violates unique constraint "ix_{}_service_id"'.format(table_name)

tests/app/conftest.py

@@ -1106,7 +1106,7 @@ def admin_request(client):
                 url_for(endpoint, **(endpoint_kwargs or {})),
                 headers=[create_authorization_header()]
             )
-            json_resp = json.loads(resp.get_data(as_text=True))
+            json_resp = resp.json
             assert resp.status_code == _expected_status
             return json_resp
 
@@ -1118,7 +1118,7 @@ def admin_request(client):
                 headers=[('Content-Type', 'application/json'), create_authorization_header()]
             )
             if resp.get_data():
-                json_resp = json.loads(resp.get_data(as_text=True))
+                json_resp = resp.json
             else:
                 json_resp = None
             assert resp.status_code == _expected_status
@@ -1130,7 +1130,10 @@ def admin_request(client):
                 url_for(endpoint, **(endpoint_kwargs or {})),
                 headers=[create_authorization_header()]
             )
-            json_resp = json.loads(resp.get_data(as_text=True))
+            if resp.get_data():
+                json_resp = resp.json
+            else:
+                json_resp = None
             assert resp.status_code == _expected_status, json_resp
             return json_resp
 

tests/app/service/test_callback_rest.py

@@ -1,28 +1,27 @@
-import json
 import uuid
 
-from tests import create_authorization_header
-
 from tests.app.db import (
     create_service_inbound_api,
     create_service_callback_api
 )
 
+from app.models import ServiceInboundApi, ServiceCallbackApi
 
-def test_create_service_inbound_api(client, sample_service):
+
+def test_create_service_inbound_api(admin_request, sample_service):
     data = {
         "url": "https://some_service/inbound-sms",
         "bearer_token": "some-unique-string",
         "updated_by_id": str(sample_service.users[0].id)
     }
-    response = client.post(
+    resp_json = admin_request.post(
-        '/service/{}/inbound-api'.format(sample_service.id),
+        'service_callback.create_service_inbound_api',
-        data=json.dumps(data),
+        service_id=sample_service.id,
-        headers=[('Content-Type', 'application/json'), create_authorization_header()]
+        _data=data,
+        _expected_status=201
     )
-    assert response.status_code == 201
 
-    resp_json = json.loads(response.get_data(as_text=True))["data"]
+    resp_json = resp_json["data"]
     assert resp_json["id"]
     assert resp_json["service_id"] == str(sample_service.id)
     assert resp_json["url"] == "https://some_service/inbound-sms"
@@ -31,22 +30,22 @@ def test_create_service_inbound_api(client, sample_service):
     assert not resp_json["updated_at"]
 
 
-def test_set_service_inbound_api_raises_404_when_service_does_not_exist(client):
+def test_set_service_inbound_api_raises_404_when_service_does_not_exist(admin_request):
     data = {
         "url": "https://some_service/inbound-sms",
         "bearer_token": "some-unique-string",
         "updated_by_id": str(uuid.uuid4())
     }
-    response = client.post(
+    response = admin_request.post(
-        '/service/{}/inbound-api'.format(uuid.uuid4()),
+        'service_callback.create_service_inbound_api',
-        data=json.dumps(data),
+        service_id=uuid.uuid4(),
-        headers=[('Content-Type', 'application/json'), create_authorization_header()]
+        _data=data,
+        _expected_status=404
     )
-    assert response.status_code == 404
+    assert response['message'] == 'No result found'
-    assert json.loads(response.get_data(as_text=True))['message'] == 'No result found'
 
 
-def test_update_service_inbound_api_updates_url(client, sample_service):
+def test_update_service_inbound_api_updates_url(admin_request, sample_service):
     service_inbound_api = create_service_inbound_api(service=sample_service,
                                                      url="https://original_url.com")
 
@@ -54,53 +53,74 @@ def test_update_service_inbound_api_updates_url(client, sample_service):
         "url": "https://another_url.com",
         "updated_by_id": str(sample_service.users[0].id)
     }
-    response = client.post("/service/{}/inbound-api/{}".format(sample_service.id, service_inbound_api.id),
+
-                           data=json.dumps(data),
+    response = admin_request.post(
-                           headers=[('Content-Type', 'application/json'), create_authorization_header()])
+        'service_callback.update_service_inbound_api',
-    assert response.status_code == 200
+        service_id=sample_service.id,
-    resp_json = json.loads(response.get_data(as_text=True))["data"]
+        inbound_api_id=service_inbound_api.id,
-    assert resp_json["url"] == "https://another_url.com"
+        _data=data
+    )
+
+    assert response["data"]["url"] == "https://another_url.com"
     assert service_inbound_api.url == "https://another_url.com"
 
 
-def test_update_service_inbound_api_updates_bearer_token(client, sample_service):
+def test_update_service_inbound_api_updates_bearer_token(admin_request, sample_service):
     service_inbound_api = create_service_inbound_api(service=sample_service,
                                                      bearer_token="some_super_secret")
     data = {
         "bearer_token": "different_token",
         "updated_by_id": str(sample_service.users[0].id)
     }
-    response = client.post("/service/{}/inbound-api/{}".format(sample_service.id, service_inbound_api.id),
+
-                           data=json.dumps(data),
+    admin_request.post(
-                           headers=[('Content-Type', 'application/json'), create_authorization_header()])
+        'service_callback.update_service_inbound_api',
-    assert response.status_code == 200
+        service_id=sample_service.id,
+        inbound_api_id=service_inbound_api.id,
+        _data=data
+    )
     assert service_inbound_api.bearer_token == "different_token"
 
 
-def test_fetch_service_inbound_api(client, sample_service):
+def test_fetch_service_inbound_api(admin_request, sample_service):
     service_inbound_api = create_service_inbound_api(service=sample_service)
 
-    response = client.get("/service/{}/inbound-api/{}".format(sample_service.id, service_inbound_api.id),
+    response = admin_request.get(
-                          headers=[create_authorization_header()])
+        'service_callback.fetch_service_inbound_api',
-
+        service_id=sample_service.id,
-    assert response.status_code == 200
+        inbound_api_id=service_inbound_api.id,
-    assert json.loads(response.get_data(as_text=True))["data"] == service_inbound_api.serialize()
+    )
+    assert response["data"] == service_inbound_api.serialize()
 
 
-def test_create_service_callback_api(client, sample_service):
+def test_delete_service_inbound_api(admin_request, sample_service):
+    service_inbound_api = create_service_inbound_api(sample_service)
+
+    response = admin_request.delete(
+        'service_callback.remove_service_inbound_api',
+        service_id=sample_service.id,
+        inbound_api_id=service_inbound_api.id,
+    )
+
+    assert response is None
+    assert ServiceInboundApi.query.count() == 0
+
+
+def test_create_service_callback_api(admin_request, sample_service):
     data = {
         "url": "https://some_service/delivery-receipt-endpoint",
         "bearer_token": "some-unique-string",
         "updated_by_id": str(sample_service.users[0].id)
     }
-    response = client.post(
-        '/service/{}/delivery-receipt-api'.format(sample_service.id),
-        data=json.dumps(data),
-        headers=[('Content-Type', 'application/json'), create_authorization_header()]
-    )
-    assert response.status_code == 201
 
-    resp_json = json.loads(response.get_data(as_text=True))["data"]
+    resp_json = admin_request.post(
+        'service_callback.create_service_callback_api',
+        service_id=sample_service.id,
+        _data=data,
+        _expected_status=201
+    )
+
+    resp_json = resp_json["data"]
     assert resp_json["id"]
     assert resp_json["service_id"] == str(sample_service.id)
     assert resp_json["url"] == "https://some_service/delivery-receipt-endpoint"
@@ -109,22 +129,23 @@ def test_create_service_callback_api(client, sample_service):
     assert not resp_json["updated_at"]
 
 
-def test_set_service_callback_api_raises_404_when_service_does_not_exist(client, notify_db_session):
+def test_set_service_callback_api_raises_404_when_service_does_not_exist(admin_request, notify_db_session):
     data = {
         "url": "https://some_service/delivery-receipt-endpoint",
         "bearer_token": "some-unique-string",
         "updated_by_id": str(uuid.uuid4())
     }
-    response = client.post(
+
-        '/service/{}/delivery-receipt-api'.format(uuid.uuid4()),
+    resp_json = admin_request.post(
-        data=json.dumps(data),
+        'service_callback.create_service_callback_api',
-        headers=[('Content-Type', 'application/json'), create_authorization_header()]
+        service_id=uuid.uuid4(),
+        _data=data,
+        _expected_status=404
     )
-    assert response.status_code == 404
+    assert resp_json['message'] == 'No result found'
-    assert json.loads(response.get_data(as_text=True))['message'] == 'No result found'
 
 
-def test_update_service_callback_api_updates_url(client, sample_service):
+def test_update_service_callback_api_updates_url(admin_request, sample_service):
     service_callback_api = create_service_callback_api(service=sample_service,
                                                        url="https://original_url.com")
 
@@ -132,34 +153,54 @@ def test_update_service_callback_api_updates_url(client, sample_service):
         "url": "https://another_url.com",
         "updated_by_id": str(sample_service.users[0].id)
     }
-    response = client.post("/service/{}/delivery-receipt-api/{}".format(sample_service.id, service_callback_api.id),
+
-                           data=json.dumps(data),
+    resp_json = admin_request.post(
-                           headers=[('Content-Type', 'application/json'), create_authorization_header()])
+        'service_callback.update_service_callback_api',
-    assert response.status_code == 200
+        service_id=sample_service.id,
-    resp_json = json.loads(response.get_data(as_text=True))["data"]
+        callback_api_id=service_callback_api.id,
-    assert resp_json["url"] == "https://another_url.com"
+        _data=data
+    )
+    assert resp_json["data"]["url"] == "https://another_url.com"
     assert service_callback_api.url == "https://another_url.com"
 
 
-def test_update_service_callback_api_updates_bearer_token(client, sample_service):
+def test_update_service_callback_api_updates_bearer_token(admin_request, sample_service):
     service_callback_api = create_service_callback_api(service=sample_service,
                                                        bearer_token="some_super_secret")
     data = {
         "bearer_token": "different_token",
         "updated_by_id": str(sample_service.users[0].id)
     }
-    response = client.post("/service/{}/delivery-receipt-api/{}".format(sample_service.id, service_callback_api.id),
+
-                           data=json.dumps(data),
+    admin_request.post(
-                           headers=[('Content-Type', 'application/json'), create_authorization_header()])
+        'service_callback.update_service_callback_api',
-    assert response.status_code == 200
+        service_id=sample_service.id,
+        callback_api_id=service_callback_api.id,
+        _data=data
+    )
     assert service_callback_api.bearer_token == "different_token"
 
 
-def test_fetch_service_callback_api(client, sample_service):
+def test_fetch_service_callback_api(admin_request, sample_service):
     service_callback_api = create_service_callback_api(service=sample_service)
 
-    response = client.get("/service/{}/delivery-receipt-api/{}".format(sample_service.id, service_callback_api.id),
+    response = admin_request.get(
-                          headers=[create_authorization_header()])
+        'service_callback.fetch_service_callback_api',
+        service_id=sample_service.id,
+        callback_api_id=service_callback_api.id,
+    )
 
-    assert response.status_code == 200
+    assert response["data"] == service_callback_api.serialize()
-    assert json.loads(response.get_data(as_text=True))["data"] == service_callback_api.serialize()
+
+
+def test_delete_service_callback_api(admin_request, sample_service):
+    service_callback_api = create_service_callback_api(sample_service)
+
+    response = admin_request.delete(
+        'service_callback.remove_service_callback_api',
+        service_id=sample_service.id,
+        callback_api_id=service_callback_api.id,
+    )
+
+    assert response is None
+    assert ServiceCallbackApi.query.count() == 0