-
Notifications
You must be signed in to change notification settings - Fork 19
New API endpoint to count SMS parts and total cost for a service #2812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5d5634e
486acc0
22388d8
f905382
008cac7
49f2678
08361c3
be6dd40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -32,6 +32,78 @@ | |||||||||
| from app.utils import get_local_timezone_midnight_in_utc | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def dao_fetch_sms_cost_for_service_in_range(service_id, start_date, end_date): | ||||||||||
| """Return the total SMS cost and fragment count for a service in the given date range (inclusive). | ||||||||||
|
|
||||||||||
| FactBilling is populated by a nightly task, so it is always ~1 day behind. | ||||||||||
| For the current day we fall back to the Notification table and use the | ||||||||||
| carrier-reported costs (sms_total_carrier_fee + sms_total_message_price). | ||||||||||
|
|
||||||||||
| Minor edge case: | ||||||||||
| If this API is called at 00:15 EST and prior to `create_nightly_billing_for_day` completing, then | ||||||||||
| this endpoint could return inaccurate data. | ||||||||||
|
|
||||||||||
| Future improvements could include: | ||||||||||
| 1. Check if `ft_billing` has been populated for the requested date range and if not, fall back to the `notifications` table | ||||||||||
| 2. Always use the `notifications` table for today and yesterday's data, and `ft_billing` for anything older | ||||||||||
| """ | ||||||||||
| today = convert_utc_to_local_timezone(datetime.utcnow()).date() | ||||||||||
|
|
||||||||||
| fragment_count = 0 | ||||||||||
| total_cost = Decimal(0) | ||||||||||
|
|
||||||||||
| # Historical data from FactBilling (up to yesterday) | ||||||||||
| if start_date < today: | ||||||||||
| fact_end = min(end_date, today - timedelta(days=1)) | ||||||||||
| fact_result = ( | ||||||||||
| db.session.query( | ||||||||||
| func.coalesce(func.sum(FactBilling.billable_units), 0).label("fragment_count"), | ||||||||||
|
||||||||||
| func.coalesce(func.sum(FactBilling.billable_units), 0).label("fragment_count"), | |
| func.coalesce( | |
| func.sum(FactBilling.billable_units * func.coalesce(FactBilling.rate_multiplier, 1)), 0 | |
| ).label("fragment_count"), |
Copilot
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Notification-table branch runs whenever end_date >= today, but it doesn’t check start_date. For a future-only range (eg start_date tomorrow, end_date tomorrow), this will still include today’s notifications because it always queries [today_start, today_end). Gate this branch on start_date <= today <= end_date (or start_date <= today in addition to the existing end_date >= today) so results don’t include out-of-range data.
| if end_date >= today: | |
| if start_date <= today and end_date >= today: |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -353,6 +353,62 @@ def test_get_yearly_billing_usage_summary_from_ft_billing(client, notify_db_sess | |||||
| assert json_response[2]["letter_total"] == 0 | ||||||
|
|
||||||
|
|
||||||
| class TestGetSmsCostForService: | ||||||
| def test_get_sms_cost_for_service(self, client, notify_db_session): | ||||||
| service = create_service() | ||||||
| template = create_template(service=service, template_type="sms") | ||||||
| create_ft_billing( | ||||||
| utc_date="2024-06-01", | ||||||
| service=service, | ||||||
| template=template, | ||||||
| notification_type="sms", | ||||||
| billable_unit=10, | ||||||
| rate=0.0162, | ||||||
| ) | ||||||
| create_ft_billing( | ||||||
| utc_date="2024-06-15", | ||||||
| service=service, | ||||||
| template=template, | ||||||
| notification_type="sms", | ||||||
| billable_unit=5, | ||||||
| rate=0.0162, | ||||||
| ) | ||||||
| response = client.get( | ||||||
| "/service/{}/billing/sms-cost?start_date=2024-06-01&end_date=2024-06-30".format(service.id), | ||||||
| headers=[create_authorization_header()], | ||||||
| ) | ||||||
| assert response.status_code == 200 | ||||||
| json_resp = json.loads(response.get_data(as_text=True)) | ||||||
| assert json_resp["start_date"] == "2024-06-01" | ||||||
| assert json_resp["end_date"] == "2024-06-30" | ||||||
| assert json_resp["fragment_count"] == 15 | ||||||
| assert json_resp["total_cost"] == 15 * 0.0162 | ||||||
|
||||||
| assert json_resp["total_cost"] == 15 * 0.0162 | |
| assert json_resp["total_cost"] == pytest.approx(15 * 0.0162) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation error message says “start_date must be before end_date”, but the code only rejects
start_date > end_date(same-day ranges are allowed). Consider changing the message to reflect the actual constraint (eg “start_date must be on or before end_date”) to avoid confusing API consumers.