New API endpoint to count SMS parts and total cost for a service

[whabanks]

Summary | Résumé

This PR is a simple example of the rest-endpoint-builder agent adding a new API endpoint /service/<id>/billing/sms-cost which tallies up the fragment_count and total_cost for a service within a given date range.

Example output:

Promp & chat

Add a GET endpoint to the billing module that returns the total SMS cost for a service in a given date range. 
It should accept start_date and end_date as query parameters and return the total cost and fragment count.

I have a good understanding of the existing patterns. Let me check the FactBilling model to understand the columns available for the SMS cost query, and also read definitions.py for shared definitions.

Now I have full context. The FactBilling model has service_id, bst_date, notification_type, billable_units, rate_multiplier, rate, and billing_total columns.

I'll add:

1. A validation schema for the query params
2. A DAO function to query SMS cost by date range
3. A GET route in the billing rest.py
4. Test stubs
5. Let me implement all the changes:

Now let me add test stubs. Let me check how the existing tests import and structure things:

Now I'll append tests for the new endpoint at the end of the test file:

The import ordering in rest.py needs fixing — from datetime import date should come before Flask imports per standard Python import ordering (stdlib before third-party).

All changes are complete. Here's a summary of what was added:

Edited files:

• billing_schemas.py — added get_sms_cost_for_service_schema requiring start_date and end_date string properties with "format": "date"
• fact_billing_dao.py — added dao_fetch_sms_cost_for_service_in_range(service_id, start_date, end_date) that queries FactBilling for SMS-type rows, returning total fragment_count (billable_units × rate_multiplier) and total_cost (fragments × rate)
• rest.py — added GET /service/uuid:service_id/billing/sms-cost?start_date=YYYY-MM-DD&end_date=YYYY-MM-DD route that validates params, checks start < end, calls the DAO, and returns {start_date, end_date, fragment_count, total_cost}
• test_billing.py — added TestGetSmsCostForService class with 4 tests: happy path with data, zero-result case, missing dates (400), and start_date after end_date (400)

Checklist:

• billing_schemas.py — edited (added get_sms_cost_for_service_schema)
• fact_billing_dao.py — edited (added dao_fetch_sms_cost_for_service_in_range)
• rest.py — edited (added new route + updated imports)
• test_billing.py — edited (added test class)
• No __init__.py change needed — the billing_blueprint is already registered

Test instructions | Instructions pour tester la modification

TODO: Fill in test instructions for the reviewer.

Release Instructions | Instructions pour le déploiement

None.

Reviewer checklist | Liste de vérification du réviseur

• This PR does not break existing functionality.
• This PR does not violate GCNotify's privacy policies.
• This PR does not raise new security concerns. Refer to our GC Notify Risk Register document on our Google drive.
• This PR does not significantly alter performance.
• Additional required documentation resulting of these changes is covered (such as the README, setup instructions, a related ADR or the technical documentation).

⚠ If boxes cannot be checked off before merging the PR, they should be moved to the "Release Instructions" section with appropriate steps required to verify before release. For example, changes to celery code may require tests on staging to verify that performance has not been affected.

[Copilot]

Pull request overview

Adds a new billing API endpoint to return total SMS fragments and total SMS cost for a service over a requested date range, using ft_billing for historical days and notifications for “today”.

Changes:

• Adds GET /service/<uuid:service_id>/billing/sms-cost with start_date/end_date query validation.
• Introduces DAO aggregation dao_fetch_sms_cost_for_service_in_range(...) combining ft_billing + same-day notifications costs.
• Adds unit tests for the DAO and the new REST endpoint.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
app/billing/billing_schemas.py	Adds JSON schema for start_date/end_date query params.
app/dao/fact_billing_dao.py	Implements SMS cost + fragment aggregation across ft_billing and notifications.
app/billing/rest.py	Exposes the new /billing/sms-cost route and wires it to the DAO.
tests/app/dao/test_ft_billing_dao.py	Adds DAO-level tests for historical/today/combined cases and filters.
tests/app/billing/test_billing.py	Adds endpoint-level tests for success and validation failures.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This assertion compares floats computed via 15 * 0.0162, which can be sensitive to float representation/rounding (and may vary depending on how the DB value is stored/returned). To avoid flaky tests, assert using pytest.approx(...) or compare using Decimal (eg serialize total_cost consistently and compare as a string/Decimal).

Suggested change

	assert json_resp["total_cost"] == 15 * 0.0162
	assert json_resp["total_cost"] == pytest.approx(15 * 0.0162)

[Copilot]

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.

Suggested change

	raise InvalidRequest("start_date must be before end_date", 400)
	raise InvalidRequest("start_date must be on or before end_date", 400)

[Copilot]

fragment_count for historical ft_billing rows is currently computed as sum(FactBilling.billable_units), but elsewhere in this module SMS “billable units/fragments” are consistently billable_units * rate_multiplier (see fetch_sms_free_allowance_remainder and fetch_sms_billing_for_all_services). As written, this endpoint undercounts fragments for any row with rate_multiplier != 1 and makes fragment_count inconsistent with the total_cost calculation (which does apply the multiplier). Update the query to sum FactBilling.billable_units * FactBilling.rate_multiplier (with appropriate coalesce) and adjust downstream tests accordingly.

Suggested change

	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]

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.

Suggested change

	if end_date >= today:
	if start_date <= today and end_date >= today:

[Copilot]

This test documents/locks in the current behavior that fragment_count ignores rate_multiplier (expecting 3 + 5 = 8). If fragment_count is intended to represent SMS fragments/billable units (consistent with other billing queries), the expectation should include the multiplier (31 + 52 = 13) and the comment should be updated accordingly.

Suggested change

	# fragment_count = sum of billable_units (no multiplier) = 3 + 5 = 8
	assert result["fragment_count"] == 8
	# fragment_count = sum of multiplier-adjusted billable_units = (3 * 1) + (5 * 2) = 13
	assert result["fragment_count"] == 13