Update fetch_todays_total_sms_count no longer count by fragment

- Removed a number of tests involving counting of fragments vs limits - Fixed a call to check_sms_daily_limit that was still passing fragments into it's parameters
cds-snc · Nov 17, 2023 · c13045c · c13045c
1 parent 0834321
commit c13045c
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 248 deletions.
diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py
@@ -443,7 +443,7 @@ def fetch_todays_total_message_count(service_id):
 def fetch_todays_total_sms_count(service_id):
     midnight = get_midnight(datetime.now(tz=pytz.utc))
     result = (
-        db.session.query(func.sum(Notification.billable_units).label("sum_billable_units"))
+        db.session.query(func.count(Notification.id).label("total_sms_notifications"))
         .filter(
             Notification.service_id == service_id,
             Notification.key_type != KEY_TYPE_TEST,
@@ -452,7 +452,7 @@ def fetch_todays_total_sms_count(service_id):
         )
         .first()
     )
-    return 0 if result is None or result.sum_billable_units is None else result.sum_billable_units
+    return 0 if result is None or result.total_sms_notifications is None else result.total_sms_notifications
 
 
 def fetch_service_email_limit(service_id: uuid.UUID) -> int:

diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py
@@ -297,7 +297,7 @@ def post_notification(notification_type: NotificationType):
     if template.template_type == SMS_TYPE:
         is_test_notification = api_user.key_type == KEY_TYPE_TEST or simulated_recipient(form["phone_number"], notification_type)
         if not is_test_notification:
-            check_sms_daily_limit(authenticated_service, template_with_content.fragment_count)
+            check_sms_daily_limit(authenticated_service, 1)
 
     current_app.logger.info(f"Trying to send notification for Template ID: {template.id}")
 

diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py
@@ -1090,13 +1090,6 @@ def test_only_counts_sms(self):
         save_notification(create_notification(template=email_template))
         assert fetch_todays_total_sms_count(service.id) == 2
 
-    def test_sums_billable_units(self):
-        service = create_service()
-        sms_template = create_template(service=service, template_type=SMS_TYPE)
-        save_notification(create_notification(template=sms_template, billable_units=3))
-        save_notification(create_notification(template=sms_template, billable_units=10))
-        assert fetch_todays_total_sms_count(service.id) == 13
-
     def test_returns_0_when_no_messages_for_today(self):
         assert fetch_todays_total_sms_count(uuid.uuid4()) == 0
 

diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py
@@ -1450,7 +1450,7 @@ def test_post_notification_without_document_upload_permission(self, client, noti
 
 
 class TestSMSSendFragments:
-    def test_post_sms_enough_fragments_left(self, notify_api, client, notify_db, notify_db_session, mocker):
+    def test_post_sms_enough_messages_left(self, notify_api, client, notify_db, notify_db_session, mocker):
         mocker.patch("app.sms_normal_publish.publish")
         service = create_service(sms_daily_limit=10, message_limit=100)
         template = create_sample_template(notify_db, notify_db_session, content=500 * "a", service=service, template_type="sms")
@@ -1459,7 +1459,7 @@ def test_post_sms_enough_fragments_left(self, notify_api, client, notify_db, not
             "template_id": str(template.id),
             "personalisation": {" Name": "Jo"},
         }
-        for x in range(2):
+        for x in range(6):
             create_sample_notification(notify_db, notify_db_session, service=service)
         auth_header = create_authorization_header(service_id=template.service_id)
 
@@ -1471,7 +1471,7 @@ def test_post_sms_enough_fragments_left(self, notify_api, client, notify_db, not
             )
         assert response.status_code == 201
 
-    def test_post_sms_not_enough_fragments_left(self, notify_api, client, notify_db, notify_db_session, mocker):
+    def test_post_sms_not_enough_messages_left(self, notify_api, client, notify_db, notify_db_session, mocker):
         mocker.patch("app.sms_normal_publish.publish")
         service = create_service(sms_daily_limit=10, message_limit=100)
         template = create_sample_template(notify_db, notify_db_session, content=500 * "a", service=service, template_type="sms")
@@ -1480,7 +1480,7 @@ def test_post_sms_not_enough_fragments_left(self, notify_api, client, notify_db,
             "template_id": str(template.id),
             "personalisation": {" Name": "Jo"},
         }
-        for x in range(7):
+        for x in range(10):
             create_sample_notification(notify_db, notify_db_session, service=service)
         auth_header = create_authorization_header(service_id=template.service_id)
 
@@ -1493,7 +1493,7 @@ def test_post_sms_not_enough_fragments_left(self, notify_api, client, notify_db,
         assert response.status_code == 429
 
 
-class TestSMSFragmentCounter:
+class TestSMSMessageCounter:
     # Testing API one-off:
     #   - Sending using TEST, NORMAL, and TEAM API keys with a simulated phone number should not count towards limits
     # TODO: update these params when we fix https://github.com/cds-snc/notification-planning/issues/855 and remove the xfao;
@@ -1772,40 +1772,6 @@ def __send_sms():
         response = __send_sms()  # send the 11th fragment
         assert response.status_code == 429  # Ensure send is blocked
 
-    def test_API_ONEOFF_cant_hop_over_limit_using_3_fragment_sms(self, notify_api, client, notify_db, notify_db_session, mocker):
-        # test setup
-        mocker.patch("app.sms_normal_publish.publish")
-        send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email")
-
-        def __send_sms():
-            auth_header = create_authorization_header(service_id=template.service_id)
-            with set_config_values(notify_api, {"REDIS_ENABLED": True}):
-                response = client.post(
-                    path="/v2/notifications/sms",
-                    data=json.dumps(data),
-                    headers=[("Content-Type", "application/json"), auth_header],
-                )
-                return response
-
-        # Create a service, Set limit to 10 fragments
-        service = create_service(sms_daily_limit=10, message_limit=100)
-
-        # Create 5 notifications in the db
-        template = create_sample_template(notify_db, notify_db_session, content="a" * 400, service=service, template_type="sms")
-        data = {
-            "phone_number": "+16502532222",
-            "template_id": str(template.id),
-            "personalisation": {" Name": "Jo"},
-        }
-        for x in range(7):
-            create_sample_notification(notify_db, notify_db_session, service=service)
-
-        __send_sms()  # send 1 sms (3 fragments) should be at 80%
-        assert send_warning_email.called
-
-        __send_sms()
-        response = __send_sms()  # send one more, puts us at 11/10 fragments
-        assert response.status_code == 429  # Ensure send is blocked
 
     def test_API_BULK_sends_warning_emails_and_blocks_sends(self, notify_api, client, notify_db, notify_db_session, mocker):
         # test setup
@@ -1850,52 +1816,6 @@ def __send_sms():
         response = __send_sms()  # send the 11th fragment
         assert response.status_code == 400  # Ensure send is blocked - not sure why we send a 400 here and a 429 everywhere else
 
-    def test_API_BULK_cant_hop_over_limit_1_fragment(self, notify_api, client, notify_db, notify_db_session, mocker):
-        # test setup
-        mocker.patch("app.sms_normal_publish.publish")
-        mocker.patch("app.v2.notifications.post_notifications.create_bulk_job", return_value=str(uuid.uuid4()))
-        send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email")
-        send_limit_reached_email = mocker.patch("app.notifications.validators.send_sms_limit_reached_email")
-
-        def __send_sms(number_to_send=1):
-            with set_config_values(notify_api, {"REDIS_ENABLED": True}):
-                numbers = [["9025551234"]] * number_to_send
-                data = {
-                    "name": "job_name",
-                    "template_id": str(template.id),
-                    "rows": [["phone number"], *numbers],
-                }
-
-                response = client.post(
-                    "/v2/notifications/bulk",
-                    data=json.dumps(data),
-                    headers=[
-                        ("Content-Type", "application/json"),
-                        create_authorization_header(service_id=template.service_id),
-                    ],
-                )
-            return response
-
-        # Create a service, Set limit to 10 fragments
-        service = create_service(sms_daily_limit=10, message_limit=100)
-
-        # Create 7 notifications in the db
-        template = create_sample_template(notify_db, notify_db_session, content="Hello", service=service, template_type="sms")
-        for x in range(7):
-            create_sample_notification(notify_db, notify_db_session, service=service)
-
-        __send_sms(1)  # send 1 sms (1 fragment) should be at 80%
-        assert send_warning_email.called
-
-        response = __send_sms(3)  # attempt to send over limit (11 with max 10)
-        assert response.status_code == 400
-
-        __send_sms(2)  # attempt to send at limit (10 with max 10)
-        assert send_limit_reached_email.called
-
-        response = __send_sms(1)  # attempt to send over limit (11 with max 10)1
-        assert response.status_code == 400  # Ensure send is blocked - not sure why we send a 400 here and a 429 everywhere else
-
     # ADMIN
     def test_ADMIN_ONEOFF_sends_warning_emails_and_blocks_sends(self, notify_api, client, notify_db, notify_db_session, mocker):
         # test setup
@@ -1940,47 +1860,6 @@ def __send_sms():
         response = __send_sms()  # 11/10 fragments
         assert response.status_code == 429  # Ensure send is blocked
 
-    def test_ADMIN_ONEOFF_cant_hop_over_limit_using_3_fragment_sms(
-        self, notify_api, client, notify_db, notify_db_session, mocker
-    ):
-        # test setup
-        mocker.patch("app.sms_normal_publish.publish")
-
-        mocker.patch("app.service.send_notification.send_notification_to_queue")
-        send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email")
-
-        def __send_sms():
-            with set_config_values(notify_api, {"REDIS_ENABLED": True}):
-                token = create_jwt_token(
-                    current_app.config["ADMIN_CLIENT_SECRET"], client_id=current_app.config["ADMIN_CLIENT_USER_NAME"]
-                )
-                response = client.post(
-                    f"/service/{template.service_id}/send-notification",
-                    json={
-                        "to": "9025551234",
-                        "template_id": str(template.id),
-                        "created_by": service.users[0].id,
-                        "personalisation": {"var": "var"},
-                    },
-                    headers={"Authorization": f"Bearer {token}"},
-                )
-            return response
-
-        # Create a service, Set limit to 10 fragments
-        service = create_service(sms_daily_limit=10, message_limit=100)
-
-        # Create 5 notifications in the db
-        template = create_sample_template(notify_db, notify_db_session, content="a" * 400, service=service, template_type="sms")
-        for x in range(8):
-            create_sample_notification(notify_db, notify_db_session, service=service)
-
-        __send_sms()  # 9/10 fragments
-        assert send_warning_email.called
-        __send_sms()
-
-        response = __send_sms()  # 11/10 fragments
-        assert response.status_code == 429  # Ensure send is blocked
-
     def test_ADMIN_CSV_sends_warning_emails_and_blocks_sends(self, notify_api, client, notify_db, notify_db_session, mocker):
         # test setup
         mocker.patch("app.sms_normal_publish.publish")
@@ -2036,118 +1915,6 @@ def __send_sms():
         response = __send_sms()  # 11/10 fragments
         assert response.status_code == 429  # Ensure send is blocked
 
-    def test_ADMIN_CSV_cant_hop_over_limit_using_1_fragment_sms(self, notify_api, client, notify_db, notify_db_session, mocker):
-        # test setup
-        mocker.patch("app.sms_normal_publish.publish")
-        mocker.patch("app.service.send_notification.send_notification_to_queue")
-        mocker.patch("app.celery.tasks.process_job.apply_async")
-
-        send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email")
-        send_limit_reached_email = mocker.patch("app.notifications.validators.send_sms_limit_reached_email")
-
-        def __send_sms(number_to_send=1):
-            with set_config_values(notify_api, {"REDIS_ENABLED": True}):
-                phone_numbers = "\r\n6502532222" * number_to_send
-                mocker.patch("app.job.rest.get_job_from_s3", return_value=f"phone number{phone_numbers}")
-                mocker.patch(
-                    "app.job.rest.get_job_metadata_from_s3",
-                    return_value={
-                        "template_id": str(template.id),
-                        "original_file_name": "thisisatest.csv",
-                        "notification_count": f"{number_to_send}",
-                        "valid": "True",
-                    },
-                )
-
-                token = create_jwt_token(
-                    current_app.config["ADMIN_CLIENT_SECRET"], client_id=current_app.config["ADMIN_CLIENT_USER_NAME"]
-                )
-                response = client.post(
-                    f"/service/{template.service_id}/job",
-                    json={
-                        "id": str(uuid.uuid4()),
-                        "created_by": service.users[0].id,
-                    },
-                    headers={"Authorization": f"Bearer {token}"},
-                )
-            return response
-
-        # Create a service, Set limit to 10 fragments
-        service = create_service(sms_daily_limit=10, message_limit=100)
-
-        # Create 7 notifications in the db
-        template = create_sample_template(notify_db, notify_db_session, content="Hello", service=service, template_type="sms")
-        for x in range(7):
-            create_sample_notification(notify_db, notify_db_session, service=service)
-
-        __send_sms(1)  # 8/10 fragments
-        assert send_warning_email.called
-
-        response = __send_sms(3)  # 11/10 fragments
-        assert response.status_code == 429
-
-        __send_sms(2)  # 10/10 fragments
-        assert send_limit_reached_email.called
-
-        response = __send_sms(1)  # 11/10 fragments
-        assert response.status_code == 429  # Ensure send is blocked - not sure why we send a 400 here and a 429 everywhere else
-
-    def test_ADMIN_CSV_cant_hop_over_limit_using_2_fragment_sms(self, notify_api, client, notify_db, notify_db_session, mocker):
-        # test setup
-        mocker.patch("app.sms_normal_publish.publish")
-        mocker.patch("app.service.send_notification.send_notification_to_queue")
-        mocker.patch("app.celery.tasks.process_job.apply_async")
-
-        send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email")
-        send_limit_reached_email = mocker.patch("app.notifications.validators.send_sms_limit_reached_email")
-
-        def __send_sms(number_to_send=1):
-            with set_config_values(notify_api, {"REDIS_ENABLED": True}):
-                phone_numbers = "\r\n6502532222" * number_to_send
-                mocker.patch("app.job.rest.get_job_from_s3", return_value=f"phone number{phone_numbers}")
-                mocker.patch(
-                    "app.job.rest.get_job_metadata_from_s3",
-                    return_value={
-                        "template_id": str(template.id),
-                        "original_file_name": "thisisatest.csv",
-                        "notification_count": f"{number_to_send}",
-                        "valid": "True",
-                    },
-                )
-
-                token = create_jwt_token(
-                    current_app.config["ADMIN_CLIENT_SECRET"], client_id=current_app.config["ADMIN_CLIENT_USER_NAME"]
-                )
-                response = client.post(
-                    f"/service/{template.service_id}/job",
-                    json={
-                        "id": str(uuid.uuid4()),
-                        "created_by": service.users[0].id,
-                    },
-                    headers={"Authorization": f"Bearer {token}"},
-                )
-            return response
-
-        # Create a service, Set limit to 10 fragments
-        service = create_service(sms_daily_limit=10, message_limit=100)
-
-        # Create 6 notifications in the db
-        template = create_sample_template(notify_db, notify_db_session, content="A" * 200, service=service, template_type="sms")
-        for x in range(6):
-            create_sample_notification(notify_db, notify_db_session, service=service)
-
-        __send_sms(2)  # 8/10 fragments
-        assert send_warning_email.called
-
-        response = __send_sms(4)  # 12/10 fragments
-        assert response.status_code == 429
-
-        __send_sms(2)  # 10/10 fragments
-        assert send_limit_reached_email.called
-
-        response = __send_sms(1)  # 11/10 fragments
-        assert response.status_code == 429  # Ensure send is blocked - not sure why we send a 400 here and a 429 everywhere else
-
 
 class TestBulkSend:
     @pytest.mark.parametrize("args", [{}, {"rows": [1, 2], "csv": "foo"}], ids=["no args", "both args"])