darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-01-30 14:31:57 -05:00
Code Issues Packages Projects Releases Wiki Activity

Standardise timezones for service usage APIs

Browse Source 
We want to query for service usage in the BST financial year:

    2022-04-01T00:00:00+01:00 to 2023-03-31T23:59:59+01:00 =>
    2022-04-01 to 2023-03-31  # bst_date

Previously we were only doing this explicitly for the monthly API
and it seemed like the yearly usage API was incorrectly querying:

    2022-03-31T23:00:00+00:00 to 2023-03-30T23:00:00+00:00 =>
    2022-03-31 to 2023-03-30  # "bst_date"

However, it turns out this isn't a problem for two reasons:

1. We've been lucky that none of our rates have changed since 2017,
which is long ago enough that no one would care.

2. There's a quirk somewhere in Sqlalchemy / Postgres that has been
compensating for the lack of explicit BST conversion.

To help ensure we do this consistently in future I've DRYed-up the
BST conversion into a new utility. I could have just hard-coded the
dates but it seemed strange to have the knowledge twice.

I've also adjusted the tests so they detect if we accidentally use
data from a different financial year. (2) is why none of the test
assertions actually need changing and users won't be affected.

Sqlalchemy / Postgres quirk
===========================

The following queries were run on the same data but results differ:

    FactBilling.query.filter(FactBilling.bst_date >= datetime(2021,3,31,23,0), FactBilling.bst_date <= '2021-04-05').order_by(FactBilling.bst_date).first().bst_date
    datetime.date(2021, 4, 1)

    FactBilling.query.filter(FactBilling.bst_date >= '2021-03-31 23:00:00', FactBilling.bst_date <= '2021-04-05').order_by(FactBilling.bst_date).first().bst_date
    datetime.date(2021, 3, 31)

Looking at the actual query for the first item above still suggests
the results should be the same, but for the use of "timestamp".

    SELECT ...
    FROM ft_billing
    WHERE ft_billing.service_id = '16b60315-9dab-45d3-a609-e871fbbf5345'::uuid AND ft_billing.bst_date >= '2016-03-31T23:00:00'::timestamp AND ft_billing.bst_date <= '2017-03-31T22:59:59.999999'::timestamp AND ft_billing.notification_type IN ('email', 'letter') GROUP BY ft_billing.rate, ft_billing.notification_type UNION ALL SELECT sum(ft_billing.notifications_sent) AS notifications_sent, sum(ft_billing.billable_units * ft_billing.rate_multiplier) AS billable_units, ft_billing.rate AS ft_billing_rate, ft_billing.notification_type AS ft_billing_notification_type
    FROM ft_billing
    WHERE ft_billing.service_id = '16b60315-9dab-45d3-a609-e871fbbf5345'::uuid AND ft_billing.bst_date >= '2016-03-31T23:00:00'::timestamp AND ft_billing.bst_date <= '2017-03-31T22:59:59.999999'::timestamp AND ft_billing.notification_type = 'sms' GROUP BY ft_billing.rate, ft_billing.notification_type) AS anon_1 ORDER BY anon_1.notification_type, anon_1.rate

If we try some manual queries with and without '::timestamp' we get:

    select distinct(bst_date) from ft_billing where bst_date >= '2022-04-20T23:00:00' order by bst_date desc;
      bst_date
    ------------
     2022-04-21
     2022-04-20

    select distinct(bst_date) from ft_billing where bst_date >= '2022-04-20T23:00:00'::timestamp order by bst_date desc;
      bst_date
    ------------
     2022-04-21
     2022-04-20

It looks like this is happening because all client connections are
aware of the local timezone, and naive datetimes are interpreted as
being in UTC - not necessarily true, but saves us here!

The monthly API datetimes were pre-converted to dates, so none of
this was relevant for deciding exactly which date to use.
This commit is contained in:
Ben Thorner
2022-04-21 16:56:28 +01:00
parent fe6afd18d6
commit ee4da698fe
3 changed files with 40 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
app/dao/date_util.py
View File

@@ -1,7 +1,7 @@
from datetime import date, datetime, time, timedelta
import pytz
from notifications_utils.timezones import convert_bst_to_utc
from notifications_utils.timezones import convert_bst_to_utc, convert_utc_to_bst
def get_months_for_financial_year(year):
@@ -22,6 +22,15 @@ def get_financial_year(year):
return get_april_fools(year), get_april_fools(year + 1) - timedelta(microseconds=1)
def get_financial_year_dates(year):
year_start_datetime, year_end_datetime = get_financial_year(year)
return (
convert_utc_to_bst(year_start_datetime).date(),
convert_utc_to_bst(year_end_datetime).date()
)
def get_current_financial_year():
now = datetime.utcnow()
current_month = int(now.strftime('%-m'))

44
app/dao/fact_billing_dao.py
View File

@@ -8,7 +8,7 @@ from sqlalchemy.sql.expression import case, literal
from app import db
from app.dao.date_util import (
get_financial_year,
get_financial_year_dates,
get_financial_year_for_datetime,
)
from app.dao.organisation_dao import dao_get_organisation_live_services
@@ -197,7 +197,7 @@ def fetch_letter_line_items_for_all_services(start_date, end_date):
def fetch_billing_totals_for_year(service_id, year):
year_start_date, year_end_date = get_financial_year(year)
year_start, year_end = get_financial_year_dates(year)
"""
Billing for email: only record the total number of emails.
Billing for letters: The billing units is used to fetch the correct rate for the sheet count of the letter.
@@ -211,8 +211,8 @@ def fetch_billing_totals_for_year(service_id, year):
FactBilling.notification_type.label('notification_type')
).filter(
FactBilling.service_id == service_id,
FactBilling.bst_date >= year_start_date,
FactBilling.bst_date <= year_end_date,
FactBilling.bst_date >= year_start,
FactBilling.bst_date <= year_end,
FactBilling.notification_type.in_([EMAIL_TYPE, LETTER_TYPE])
).group_by(
FactBilling.rate,
@@ -228,8 +228,8 @@ def fetch_billing_totals_for_year(service_id, year):
FactBilling.notification_type
).filter(
FactBilling.service_id == service_id,
FactBilling.bst_date >= year_start_date,
FactBilling.bst_date <= year_end_date,
FactBilling.bst_date >= year_start,
FactBilling.bst_date <= year_end,
FactBilling.notification_type == SMS_TYPE
).group_by(
FactBilling.rate,
@@ -245,14 +245,11 @@ def fetch_billing_totals_for_year(service_id, year):
def fetch_monthly_billing_for_year(service_id, year):
year_start_datetime, year_end_datetime = get_financial_year(year)
year_start_date = convert_utc_to_bst(year_start_datetime).date()
year_end_date = convert_utc_to_bst(year_end_datetime).date()
year_start, year_end = get_financial_year_dates(year)
today = convert_utc_to_bst(datetime.utcnow()).date()
# if year end date is less than today, we are calculating for data in the past and have no need for deltas.
if year_end_date >= today:
if year_end >= today:
data = fetch_billing_data_for_day(process_day=today, service_id=service_id, check_permissions=True)
for d in data:
update_fact_billing(data=d, process_day=today)
@@ -266,8 +263,8 @@ def fetch_monthly_billing_for_year(service_id, year):
FactBilling.postage
).filter(
FactBilling.service_id == service_id,
FactBilling.bst_date >= year_start_date,
FactBilling.bst_date <= year_end_date,
FactBilling.bst_date >= year_start,
FactBilling.bst_date <= year_end,
FactBilling.notification_type.in_([EMAIL_TYPE, LETTER_TYPE])
).group_by(
'month',
@@ -285,8 +282,8 @@ def fetch_monthly_billing_for_year(service_id, year):
FactBilling.postage
).filter(
FactBilling.service_id == service_id,
FactBilling.bst_date >= year_start_date,
FactBilling.bst_date <= year_end_date,
FactBilling.bst_date >= year_start,
FactBilling.bst_date <= year_end,
FactBilling.notification_type == SMS_TYPE
).group_by(
'month',
@@ -649,15 +646,12 @@ def fetch_sms_billing_for_organisation(organisation_id, start_date, end_date):
def fetch_usage_year_for_organisation(organisation_id, year):
year_start_datetime, year_end_datetime = get_financial_year(year)
year_start_date = convert_utc_to_bst(year_start_datetime).date()
year_end_date = convert_utc_to_bst(year_end_datetime).date()
year_start, year_end = get_financial_year_dates(year)
today = convert_utc_to_bst(datetime.utcnow()).date()
services = dao_get_organisation_live_services(organisation_id)
# if year end date is less than today, we are calculating for data in the past and have no need for deltas.
if year_end_date >= today:
if year_end >= today:
for service in services:
data = fetch_billing_data_for_day(process_day=today, service_id=service.id)
for d in data:
@@ -677,9 +671,9 @@ def fetch_usage_year_for_organisation(organisation_id, year):
'emails_sent': 0,
'active': service.active
}
sms_usages = fetch_sms_billing_for_organisation(organisation_id, year_start_date, year_end_date)
letter_usages = fetch_letter_costs_for_organisation(organisation_id, year_start_date, year_end_date)
email_usages = fetch_email_usage_for_organisation(organisation_id, year_start_date, year_end_date)
sms_usages = fetch_sms_billing_for_organisation(organisation_id, year_start, year_end)
letter_usages = fetch_letter_costs_for_organisation(organisation_id, year_start, year_end)
email_usages = fetch_email_usage_for_organisation(organisation_id, year_start, year_end)
for usage in sms_usages:
service_with_usage[str(usage.service_id)] = {
'service_id': usage.service_id,