rework get_fragment_count to not use ProviderStatistics

use NotficationHistory instead. Unfortunately this means the SQL gets a bit gnarly, as we have to repeat notifications_utils' `get_sms_fragment_count` functionality inside a SELECT 😱
2025-12-20 23:41:17 -05:00 · 2016-07-29 16:39:51 +01:00
parent bac27d688b
commit 1617f058e2
4 changed files with 118 additions and 100 deletions
--- a/app/dao/provider_statistics_dao.py
+++ b/app/dao/provider_statistics_dao.py
@@ -1,42 +1,51 @@
-from sqlalchemy import func
-from app.models import (ProviderStatistics, SMS_PROVIDERS, EMAIL_PROVIDERS, ProviderDetails)
+from sqlalchemy import func, cast, Float, case
+
+from app import db
+from app.models import (
+    ProviderStatistics,
+    ProviderDetails,
+    NotificationHistory,
+    SMS_TYPE,
+    EMAIL_TYPE,
+    NOTIFICATION_STATUS_TYPES_BILLABLE
+    )


 def get_provider_statistics(service, **kwargs):
-    return filter_query(ProviderStatistics.query, service, **kwargs)
-
-
-def get_fragment_count(service, date_from, date_to):
-    sms_query = filter_query(
-        ProviderStatistics.query,
-        service,
-        providers=SMS_PROVIDERS,
-        date_from=date_from,
-        date_to=date_to
-    )
-    email_query = filter_query(
-        ProviderStatistics.query,
-        service,
-        providers=EMAIL_PROVIDERS,
-        date_from=date_from,
-        date_to=date_to
-    )
-    return {
-        'sms_count': int(sms_query.with_entities(
-            func.sum(ProviderStatistics.unit_count)).scalar()) if sms_query.count() > 0 else 0,
-        'email_count': int(email_query.with_entities(
-            func.sum(ProviderStatistics.unit_count)).scalar()) if email_query.count() > 0 else 0
-    }
-
-
-def filter_query(query, service, **kwargs):
-    query = query.filter_by(service=service)
+    query = ProviderStatistics.query.filter_by(service=service)
    if 'providers' in kwargs:
        providers = ProviderDetails.query.filter(ProviderDetails.identifier.in_(kwargs['providers'])).all()
        provider_ids = [provider.id for provider in providers]
        query = query.filter(ProviderStatistics.provider_id.in_(provider_ids))
-    if 'date_from' in kwargs:
-        query.filter(ProviderStatistics.day >= kwargs['date_from'])
-    if 'date_to' in kwargs:
-        query.filter(ProviderStatistics.day <= kwargs['date_to'])
    return query
+
+
+def get_fragment_count(service_id):
+    sms_count = db.session.query(
+        func.sum(
+            case(
+                [
+                    (
+                        NotificationHistory.content_char_count <= 160,
+                        func.ceil(cast(NotificationHistory.content_char_count, Float) / 153)
+                    )
+                ],
+                else_=1
+            )
+        )
+    ).filter(
+        NotificationHistory.service_id == service_id,
+        NotificationHistory.notification_type == SMS_TYPE,
+        NotificationHistory.status.in_(NOTIFICATION_STATUS_TYPES_BILLABLE)
+    )
+    email_count = db.session.query(
+        func.count(NotificationHistory.id)
+    ).filter(
+        NotificationHistory.service_id == service_id,
+        NotificationHistory.notification_type == EMAIL_TYPE,
+        NotificationHistory.status.in_(NOTIFICATION_STATUS_TYPES_BILLABLE)
+    )
+    return {
+        'sms_count': int(sms_count.scalar() or 0),
+        'email_count': email_count.scalar() or 0
+    }
--- a/app/models.py
+++ b/app/models.py
@@ -329,9 +329,34 @@ class VerifyCode(db.Model):
    def check_code(self, cde):
        return check_hash(cde, self._code)

+NOTIFICATION_CREATED = 'created'
+NOTIFICATION_SENDING = 'sending'
+NOTIFICATION_DELIVERED = 'delivered'
+NOTIFICATION_PENDING = 'pending'
+NOTIFICATION_FAILED = 'failed'
+NOTIFICATION_TECHNICAL_FAILURE = 'technical-failure'
+NOTIFICATION_TEMPORARY_FAILURE = 'temporary-failure'
+NOTIFICATION_PERMANENT_FAILURE = 'permanent-failure'

-NOTIFICATION_STATUS_TYPES = ['created', 'sending', 'delivered', 'pending', 'failed',
-                             'technical-failure', 'temporary-failure', 'permanent-failure']
+NOTIFICATION_STATUS_TYPES_BILLABLE = [
+    NOTIFICATION_SENDING,
+    NOTIFICATION_DELIVERED,
+    NOTIFICATION_FAILED,
+    NOTIFICATION_TECHNICAL_FAILURE,
+    NOTIFICATION_TEMPORARY_FAILURE,
+    NOTIFICATION_PERMANENT_FAILURE
+]
+
+NOTIFICATION_STATUS_TYPES = [
+    NOTIFICATION_CREATED,
+    NOTIFICATION_SENDING,
+    NOTIFICATION_DELIVERED,
+    NOTIFICATION_PENDING,
+    NOTIFICATION_FAILED,
+    NOTIFICATION_TECHNICAL_FAILURE,
+    NOTIFICATION_TEMPORARY_FAILURE,
+    NOTIFICATION_PERMANENT_FAILURE
+]
 NOTIFICATION_STATUS_TYPES_ENUM = db.Enum(*NOTIFICATION_STATUS_TYPES, name='notify_status_type')


--- a/app/service/rest.py
+++ b/app/service/rest.py
@@ -168,13 +168,7 @@ def remove_user_from_service(service_id, user_id):

@service.route('/<uuid:service_id>/fragment/aggregate_statistics')
 def get_service_provider_aggregate_statistics(service_id):
-    service = dao_fetch_service_by_id(service_id)
-    data = from_to_date_schema.load(request.args).data
-    return jsonify(data=get_fragment_count(
-        service,
-        date_from=(data.pop('date_from') if 'date_from' in data else date.today()),
-        date_to=(data.pop('date_to') if 'date_to' in data else date.today())
-    ))
+    return jsonify(data=get_fragment_count(service_id))


 # This is placeholder get method until more thought
--- a/tests/app/dao/test_notifications_dao_provider_statistics.py
+++ b/tests/app/dao/test_notifications_dao_provider_statistics.py
@@ -1,5 +1,7 @@
-from datetime import (date, timedelta)
-from app.models import ProviderStatistics
+from datetime import datetime
+import uuid
+
+from app.models import NotificationHistory, KEY_TYPE_NORMAL, NOTIFICATION_STATUS_TYPES
 from app.dao.notifications_dao import update_provider_stats
 from app.dao.provider_statistics_dao import (
    get_provider_statistics, get_fragment_count)
@@ -89,63 +91,51 @@ def test_should_update_provider_statistics_email_multi(notify_db,
    assert provider_stats.unit_count == 3


-def test_should_aggregate_fragment_count(notify_db,
-                                         notify_db_session,
-                                         sample_service,
-                                         mmg_provider,
-                                         firetext_provider,
-                                         ses_provider):
-    day = date.today()
-    stats_mmg = ProviderStatistics(
-        service=sample_service,
-        day=day,
-        provider_id=mmg_provider.id,
-        unit_count=2
-    )
+def test_get_fragment_count_with_no_data(sample_template):
+    assert get_fragment_count(sample_template.service_id)['sms_count'] == 0
+    assert get_fragment_count(sample_template.service_id)['email_count'] == 0

-    stats_firetext = ProviderStatistics(
-        service=sample_service,
-        day=day,
-        provider_id=firetext_provider.id,
-        unit_count=3
-    )

-    stats_ses = ProviderStatistics(
-        service=sample_service,
-        day=day,
-        provider_id=ses_provider.id,
-        unit_count=1
+def test_get_fragment_count_separates_sms_and_email(notify_db, sample_template, sample_email_template):
+    noti_hist(notify_db, sample_template)
+    noti_hist(notify_db, sample_template)
+    noti_hist(notify_db, sample_email_template)
+    assert get_fragment_count(sample_template.service_id) == {
+        'sms_count': 2,
+        'email_count': 1
+    }
+
+
+def test_get_fragment_count_filters_on_status(notify_db, sample_template):
+    for status in NOTIFICATION_STATUS_TYPES:
+        noti_hist(notify_db, sample_template, status=status)
+    # sending, delivered, failed, technical-failure, temporary-failure, permanent-failure
+    assert get_fragment_count(sample_template.service_id)['sms_count'] == 6
+
+
+def test_get_fragment_count_sums_char_count_for_sms(notify_db, sample_template):
+    noti_hist(notify_db, sample_template, content_char_count=1)  # 1
+    noti_hist(notify_db, sample_template, content_char_count=159)  # 1
+    noti_hist(notify_db, sample_template, content_char_count=310)  # 2
+    assert get_fragment_count(sample_template.service_id)['sms_count'] == 4
+
+
+def noti_hist(notify_db, template, status='delivered', content_char_count=None):
+    if not content_char_count and template.template_type == 'sms':
+        content_char_count = 1
+
+    notification_history = NotificationHistory(
+        id=uuid.uuid4(),
+        service=template.service,
+        template=template,
+        template_version=template.version,
+        status=status,
+        created_at=datetime.utcnow(),
+        content_char_count=content_char_count,
+        notification_type=template.template_type,
+        key_type=KEY_TYPE_NORMAL
    )
-    notify_db.session.add(stats_mmg)
-    notify_db.session.add(stats_firetext)
-    notify_db.session.add(stats_ses)
+    notify_db.session.add(notification_history)
    notify_db.session.commit()
-    results = get_fragment_count(sample_service, day, day)
-    assert results['sms_count'] == 5
-    assert results['email_count'] == 1

-
-def test_should_aggregate_fragment_count_over_days(notify_db,
-                                                   notify_db_session,
-                                                   sample_service,
-                                                   mmg_provider):
-    today = date.today()
-    yesterday = today - timedelta(days=1)
-    stats_today = ProviderStatistics(
-        service=sample_service,
-        day=today,
-        provider_id=mmg_provider.id,
-        unit_count=2
-    )
-    stats_yesterday = ProviderStatistics(
-        service=sample_service,
-        day=yesterday,
-        provider_id=mmg_provider.id,
-        unit_count=3
-    )
-    notify_db.session.add(stats_today)
-    notify_db.session.add(stats_yesterday)
-    notify_db.session.commit()
-    results = get_fragment_count(sample_service, yesterday, today)
-    assert results['sms_count'] == 5
-    assert results['email_count'] == 0
+    return notification_history