From 100d47f4e8904c1c79578b0467bc293178109fd2 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 8 Mar 2021 17:44:26 +0000 Subject: [PATCH] Refactor test and fixture for getting billing report data Names of services and orgs were confusing, and variable setting was done in a way that made it easy to introduce errors. Now hopefully it is more readable and more error-proof. --- tests/app/dao/test_ft_billing_dao.py | 101 +++++++++++++++++++------- tests/app/db.py | 64 ++++++++++------ tests/app/platform_stats/test_rest.py | 16 ++-- 3 files changed, 123 insertions(+), 58 deletions(-) diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 0bf6250bb..cc83e9156 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -600,62 +600,111 @@ def test_fetch_sms_billing_for_all_services_with_remainder(notify_db_session): def test_fetch_sms_billing_for_all_services_without_an_organisation_appears(notify_db_session): - org, org_2, service, service_2, service_3, service_sms_only, \ - org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 5, 1)) + setup = set_up_usage_data(datetime(2019, 5, 1)) results = fetch_sms_billing_for_all_services(datetime(2019, 5, 1), datetime(2019, 5, 31)) assert len(results) == 2 # organisation_name, organisation_id, service_name, service_id, free_sms_fragment_limit, # sms_rate, sms_remainder, sms_billable_units, chargeable_billable_units, sms_cost - assert results[0] == (org.name, org.id, service.name, service.id, 10, Decimal('0.11'), 8, 3, 0, Decimal('0')) - assert results[1] == (None, None, service_sms_only.name, service_sms_only.id, 10, Decimal('0.11'), - 0, 3, 3, Decimal('0.33')) + assert results[0] == ( + setup["org_1"].name, + setup["org_1"].id, + setup["service_1_sms_and_letter"].name, + setup["service_1_sms_and_letter"].id, + 10, Decimal('0.11'), 8, 3, 0, Decimal('0')) + assert results[1] == ( + None, + None, + setup["service_with_sms_without_org"].name, + setup["service_with_sms_without_org"].id, + 10, Decimal('0.11'), 0, 3, 3, Decimal('0.33') + ) def test_fetch_letter_costs_for_all_services(notify_db_session): - org, org_2, service, service_2, service_3, service_sms_only, \ - org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 6, 1)) + setup = set_up_usage_data(datetime(2019, 6, 1)) results = fetch_letter_costs_for_all_services(datetime(2019, 6, 1), datetime(2019, 9, 30)) assert len(results) == 3 - assert results[0] == (org.name, org.id, service.name, service.id, Decimal('3.40')) - assert results[1] == (org_2.name, org_2.id, service_2.name, service_2.id, Decimal('14.00')) - assert results[2] == (None, None, service_3.name, service_3.id, Decimal('24.45')) + assert results[0] == ( + setup["org_1"].name, setup["org_1"].id, + setup["service_1_sms_and_letter"].name, setup["service_1_sms_and_letter"].id, + Decimal('3.40') + ) + assert results[1] == ( + setup["org_for_service_with_letters"].name, setup["org_for_service_with_letters"].id, + setup["service_with_letters"].name, setup["service_with_letters"].id, + Decimal('14.00') + ) + assert results[2] == ( + None, None, + setup["service_with_letters_without_org"].name, setup["service_with_letters_without_org"].id, + Decimal('24.45') + ) def test_fetch_letter_line_items_for_all_service(notify_db_session): - org_1, org_2, service_1, service_2, service_3, service_sms_only, \ - org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 6, 1)) + setup = set_up_usage_data(datetime(2019, 6, 1)) results = fetch_letter_line_items_for_all_services(datetime(2019, 6, 1), datetime(2019, 9, 30)) assert len(results) == 7 - assert results[0] == (org_1.name, org_1.id, service_1.name, service_1.id, Decimal('0.45'), 'second', 6) - assert results[1] == (org_1.name, org_1.id, service_1.name, service_1.id, Decimal("0.35"), 'first', 2) - assert results[2] == (org_2.name, org_2.id, service_2.name, service_2.id, Decimal("0.65"), 'second', 20) - assert results[3] == (org_2.name, org_2.id, service_2.name, service_2.id, Decimal("0.50"), 'first', 2) - assert results[4] == (None, None, service_3.name, service_3.id, Decimal("0.35"), 'second', 2) - assert results[5] == (None, None, service_3.name, service_3.id, Decimal("0.50"), 'first', 1) - assert results[6] == (None, None, service_3.name, service_3.id, Decimal("1.55"), 'international', 15) + assert results[0] == ( + setup["org_1"].name, setup["org_1"].id, + setup["service_1_sms_and_letter"].name, setup["service_1_sms_and_letter"].id, + Decimal('0.45'), 'second', 6 + ) + assert results[1] == ( + setup["org_1"].name, setup["org_1"].id, + setup["service_1_sms_and_letter"].name, setup["service_1_sms_and_letter"].id, + Decimal("0.35"), 'first', 2 + ) + assert results[2] == ( + setup["org_for_service_with_letters"].name, setup["org_for_service_with_letters"].id, + setup["service_with_letters"].name, setup["service_with_letters"].id, + Decimal("0.65"), 'second', 20 + ) + assert results[3] == ( + setup["org_for_service_with_letters"].name, setup["org_for_service_with_letters"].id, + setup["service_with_letters"].name, setup["service_with_letters"].id, + Decimal("0.50"), 'first', 2 + ) + assert results[4] == ( + None, None, + setup["service_with_letters_without_org"].name, setup["service_with_letters_without_org"].id, + Decimal("0.35"), 'second', 2 + ) + assert results[5] == ( + None, None, + setup["service_with_letters_without_org"].name, setup["service_with_letters_without_org"].id, + Decimal("0.50"), 'first', 1 + ) + assert results[6] == ( + None, None, + setup["service_with_letters_without_org"].name, setup["service_with_letters_without_org"].id, + Decimal("1.55"), 'international', 15 + ) @freeze_time('2019-06-01 13:30') def test_fetch_usage_year_for_organisation(notify_db_session): - org, org_2, service, service_2, service_3, service_sms_only, \ - org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 5, 1)) + setup = set_up_usage_data(datetime(2019, 5, 1)) service_with_emails_for_org = create_service(service_name='Service with emails for org') - dao_add_service_to_organisation(service=service_with_emails_for_org, organisation_id=org.id) + dao_add_service_to_organisation( + service=service_with_emails_for_org, + organisation_id=setup["org_1"].id + ) template = create_template(service=service_with_emails_for_org, template_type='email') create_ft_billing(bst_date=datetime(2019, 5, 1), template=template, notifications_sent=1100) - results = fetch_usage_year_for_organisation(org.id, 2019) + results = fetch_usage_year_for_organisation(setup["org_1"].id, 2019) assert len(results) == 2 - first_row = results[str(service.id)] - assert first_row['service_id'] == service.id - assert first_row['service_name'] == service.name + first_row = results[str(setup["service_1_sms_and_letter"].id)] + assert first_row['service_id'] == setup["service_1_sms_and_letter"].id + assert first_row['service_name'] == setup["service_1_sms_and_letter"].name assert first_row['free_sms_limit'] == 10 assert first_row['sms_remainder'] == 10 assert first_row['chargeable_billable_sms'] == 0 diff --git a/tests/app/db.py b/tests/app/db.py index b059cb690..77a0e233d 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -949,55 +949,62 @@ def set_up_usage_data(start_date): one_week_later = start_date + timedelta(days=7) one_month_later = start_date + timedelta(days=31) - service = create_service( + service_1_sms_and_letter = create_service( service_name='a - with sms and letter', purchase_order_number="service purchase order number", billing_contact_names="service billing contact names", billing_contact_email_addresses="service@billing.contact email@addresses.gov.uk", billing_reference="service billing reference" ) - letter_template_1 = create_template(service=service, template_type='letter') - sms_template_1 = create_template(service=service, template_type='sms') - create_annual_billing(service_id=service.id, free_sms_fragment_limit=10, financial_year_start=year) - org = create_organisation( - name="Org for {}".format(service.name), + letter_template_1 = create_template(service=service_1_sms_and_letter, template_type='letter') + sms_template_1 = create_template(service=service_1_sms_and_letter, template_type='sms') + create_annual_billing( + service_id=service_1_sms_and_letter.id, free_sms_fragment_limit=10, financial_year_start=year + ) + org_1 = create_organisation( + name="Org for {}".format(service_1_sms_and_letter.name), purchase_order_number="org1 purchase order number", billing_contact_names="org1 billing contact names", billing_contact_email_addresses="org1@billing.contact email@addresses.gov.uk", billing_reference="org1 billing reference" ) - dao_add_service_to_organisation(service=service, organisation_id=org.id) - - service_2 = create_service(service_name='b - emails') - email_template = create_template(service=service_2, template_type='email') - org_2 = create_organisation( - name='Org for {}'.format(service_2.name), + dao_add_service_to_organisation( + service=service_1_sms_and_letter, + organisation_id=org_1.id ) - dao_add_service_to_organisation(service=service_2, organisation_id=org_2.id) - service_3 = create_service(service_name='c - letters only') - letter_template_3 = create_template(service=service_3, template_type='letter') - org_3 = create_organisation( - name="Org for {}".format(service_3.name), + service_with_emails = create_service(service_name='b - emails') + email_template = create_template(service=service_with_emails, template_type='email') + org_2 = create_organisation( + name='Org for {}'.format(service_with_emails.name), + ) + dao_add_service_to_organisation(service=service_with_emails, organisation_id=org_2.id) + + service_with_letters = create_service(service_name='c - letters only') + letter_template_3 = create_template(service=service_with_letters, template_type='letter') + org_for_service_with_letters = create_organisation( + name="Org for {}".format(service_with_letters.name), purchase_order_number="org3 purchase order number", billing_contact_names="org3 billing contact names", billing_contact_email_addresses="org3@billing.contact email@addresses.gov.uk", billing_reference="org3 billing reference" ) - dao_add_service_to_organisation(service=service_3, organisation_id=org_3.id) + dao_add_service_to_organisation(service=service_with_letters, organisation_id=org_for_service_with_letters.id) - service_4 = create_service(service_name='d - service without org') - letter_template_4 = create_template(service=service_4, template_type='letter') + service_with_letters_without_org = create_service(service_name='d - service without org') + letter_template_4 = create_template(service=service_with_letters_without_org, template_type='letter') - service_sms_only = create_service( + service_with_sms_without_org = create_service( service_name='b - chargeable sms', purchase_order_number="sms purchase order number", billing_contact_names="sms billing contact names", billing_contact_email_addresses="sms@billing.contact email@addresses.gov.uk", billing_reference="sms billing reference" ) - sms_template = create_template(service=service_sms_only, template_type='sms') - create_annual_billing(service_id=service_sms_only.id, free_sms_fragment_limit=10, financial_year_start=year) + sms_template = create_template(service=service_with_sms_without_org, template_type='sms') + create_annual_billing( + service_id=service_with_sms_without_org.id, free_sms_fragment_limit=10, financial_year_start=year + ) create_ft_billing(bst_date=one_week_earlier, template=sms_template_1, billable_unit=2, rate=0.11) create_ft_billing(bst_date=start_date, template=sms_template_1, billable_unit=2, rate=0.11) @@ -1031,7 +1038,16 @@ def set_up_usage_data(start_date): create_ft_billing(bst_date=start_date, template=email_template, notifications_sent=10) - return org, org_3, service, service_3, service_4, service_sms_only, org_2, service_2 + return { + "org_1": org_1, + "service_1_sms_and_letter": service_1_sms_and_letter, + "org_2": org_2, + "service_with_emails": service_with_emails, + "org_for_service_with_letters": org_for_service_with_letters, + "service_with_letters": service_with_letters, + "service_with_letters_without_org": service_with_letters_without_org, + "service_with_sms_without_org": service_with_sms_without_org + } def create_returned_letter(service=None, reported_at=None, notification_id=None): diff --git a/tests/app/platform_stats/test_rest.py b/tests/app/platform_stats/test_rest.py index 3986fa6ae..9e3e2ec35 100644 --- a/tests/app/platform_stats/test_rest.py +++ b/tests/app/platform_stats/test_rest.py @@ -127,14 +127,13 @@ def test_validate_date_is_within_a_financial_year_when_input_is_not_a_date(start def test_get_usage_for_all_services(notify_db_session, admin_request): - org, org_3, service, service_3, service_without_org, service_sms_only, \ - org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 5, 1)) + setup = set_up_usage_data(datetime(2019, 5, 1)) response = admin_request.get("platform_stats.get_usage_for_all_services", start_date='2019-05-01', end_date='2019-06-30') assert len(response) == 4 - assert response[0]["organisation_id"] == str(org.id) - assert response[0]["service_id"] == str(service.id) + assert response[0]["organisation_id"] == str(setup["org_1"].id) + assert response[0]["service_id"] == str(setup["service_1_sms_and_letter"].id) assert response[0]["sms_cost"] == 0 assert response[0]["sms_fragments"] == 0 assert response[0]["letter_cost"] == 3.40 @@ -144,8 +143,8 @@ def test_get_usage_for_all_services(notify_db_session, admin_request): assert response[0]["contact_email_addresses"] == "service@billing.contact email@addresses.gov.uk" assert response[0]["billing_reference"] == "service billing reference" - assert response[1]["organisation_id"] == str(org_3.id) - assert response[1]["service_id"] == str(service_3.id) + assert response[1]["organisation_id"] == str(setup["org_for_service_with_letters"].id) + assert response[1]["service_id"] == str(setup["service_with_letters"].id) assert response[1]["sms_cost"] == 0 assert response[1]["sms_fragments"] == 0 assert response[1]["letter_cost"] == 14 @@ -156,7 +155,7 @@ def test_get_usage_for_all_services(notify_db_session, admin_request): assert response[1]["billing_reference"] == "org3 billing reference" assert response[2]["organisation_id"] == "" - assert response[2]["service_id"] == str(service_sms_only.id) + assert response[2]["service_id"] == str(setup["service_with_sms_without_org"].id) assert response[2]["sms_cost"] == 0.33 assert response[2]["sms_fragments"] == 3 assert response[2]["letter_cost"] == 0 @@ -167,10 +166,11 @@ def test_get_usage_for_all_services(notify_db_session, admin_request): assert response[2]["billing_reference"] == "sms billing reference" assert response[3]["organisation_id"] == "" - assert response[3]["service_id"] == str(service_without_org.id) + assert response[3]["service_id"] == str(setup["service_with_letters_without_org"].id) assert response[3]["sms_cost"] == 0 assert response[3]["sms_fragments"] == 0 assert response[3]["letter_cost"] == 24.45 assert response[3]["letter_breakdown"] == ( "2 second class letters at 35p\n1 first class letters at 50p\n15 international letters at £1.55\n" ) + assert response[3]["purchase_order_number"] is None