diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 6e1b6ceed..c5a0492c5 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -2,7 +2,7 @@ import uuid from datetime import date, datetime, timedelta from notifications_utils.statsd_decorators import statsd -from sqlalchemy import asc, func, case +from sqlalchemy.sql.expression import asc, case, func from sqlalchemy.orm import joinedload from flask import current_app @@ -77,25 +77,25 @@ def dao_count_live_services(): def dao_fetch_live_services_data(): year_start_date, year_end_date = get_current_financial_year() + this_year_ft_billing = FactBilling.query.filter( FactBilling.bst_date >= year_start_date, FactBilling.bst_date <= year_end_date, ).subquery() + data = db.session.query( - Service.id, - Organisation.name.label("organisation_name"), - Organisation.organisation_type, + Service.id.label('service_id'), Service.name.label("service_name"), - Service.consent_to_research, - Service.go_live_user_id, - Service.count_as_live, - User.name.label('user_name'), - User.email_address, - User.mobile_number, + Organisation.name.label("organisation_name"), + Organisation.organisation_type.label('organisation_type'), + Service.consent_to_research.label('consent_to_research'), + User.name.label('contact_name'), + User.email_address.label('contact_email'), + User.mobile_number.label('contact_mobile'), Service.go_live_at.label("live_date"), - Service.volume_sms, - Service.volume_email, - Service.volume_letter, + Service.volume_sms.label('sms_volume_intent'), + Service.volume_email.label('email_volume_intent'), + Service.volume_letter.label('letter_volume_intent'), case([ (this_year_ft_billing.c.notification_type == 'email', func.sum(this_year_ft_billing.c.notifications_sent)) ], else_=0).label("email_totals"), @@ -130,42 +130,20 @@ def dao_fetch_live_services_data(): Service.volume_sms, Service.volume_email, Service.volume_letter, - this_year_ft_billing.c.notification_type + this_year_ft_billing.c.notification_type, ).order_by( asc(Service.go_live_at) ).all() results = [] for row in data: - is_service_in_list = None - i = 0 - while i < len(results): - if results[i]["service_id"] == row.id: - is_service_in_list = i - break - else: - i += 1 - if is_service_in_list is not None: - results[is_service_in_list]["email_totals"] += row.email_totals - results[is_service_in_list]["sms_totals"] += row.sms_totals - results[is_service_in_list]["letter_totals"] += row.letter_totals + existing_service = next((x for x in results if x['service_id'] == row.service_id), None) + + if existing_service is not None: + existing_service["email_totals"] += row.email_totals + existing_service["sms_totals"] += row.sms_totals + existing_service["letter_totals"] += row.letter_totals else: - results.append({ - "service_id": row.id, - "service_name": row.service_name, - "organisation_name": row.organisation_name, - "organisation_type": row.organisation_type, - "consent_to_research": row.consent_to_research, - "contact_name": row.user_name, - "contact_email": row.email_address, - "contact_mobile": row.mobile_number, - "live_date": row.live_date, - "sms_volume_intent": row.volume_sms, - "email_volume_intent": row.volume_email, - "letter_volume_intent": row.volume_letter, - "sms_totals": row.sms_totals, - "email_totals": row.email_totals, - "letter_totals": row.letter_totals, - }) + results.append(row._asdict()) return results diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index eab886f3f..8e199ae0e 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1,5 +1,6 @@ import uuid from datetime import datetime +from unittest import mock import pytest from freezegun import freeze_time @@ -386,11 +387,11 @@ def test_get_all_user_services_should_return_empty_list_if_no_services_for_user( @freeze_time('2019-04-23T10:00:00') -def test_dao_fetch_live_services_data(sample_user, mock): +def test_dao_fetch_live_services_data(sample_user): org = create_organisation(organisation_type='crown') service = create_service(go_live_user=sample_user, go_live_at='2014-04-20T10:00:00') template = create_template(service=service) - service_2 = create_service(service_name='second', go_live_user=sample_user, go_live_at='2017-04-20T10:00:00') + service_2 = create_service(service_name='second', go_live_at='2017-04-20T10:00:00', go_live_user=sample_user) create_service(service_name='third', go_live_at='2016-04-20T10:00:00') # below services should be filtered out: create_service(service_name='restricted', restricted=True) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 7afb7786f..9d450dfc7 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -137,28 +137,57 @@ def test_get_service_list_should_return_empty_list_if_no_services(admin_request) assert len(json_resp['data']) == 0 -def test_get_live_services_data(sample_user, admin_request, mock): +def test_get_live_services_data(sample_user, admin_request): org = create_organisation() - service = create_service(go_live_user=sample_user) + + service = create_service(go_live_user=sample_user, go_live_at=datetime(2018, 1, 1)) + create_service(service_name='second', go_live_at=datetime(2019, 1, 1), go_live_user=sample_user) + template = create_template(service=service) - create_service(service_name='second', go_live_user=sample_user) template2 = create_template(service=service, template_type='email') dao_add_service_to_organisation(service=service, organisation_id=org.id) create_ft_billing(bst_date='2019-04-20', notification_type='sms', template=template, service=service) - create_ft_billing(bst_date='2019-04-20', notification_type='email', template=template2, - service=service) + create_ft_billing(bst_date='2019-04-20', notification_type='email', template=template2, service=service) + response = admin_request.get('service.get_live_services_data')["data"] + assert len(response) == 2 - assert {'consent_to_research': None, 'contact_email': 'notify@digital.cabinet-office.gov.uk', - 'contact_mobile': '+447700900986', 'contact_name': 'Test User', 'email_totals': 0, - 'email_volume_intent': None, 'letter_totals': 0, 'letter_volume_intent': None, 'live_date': None, - 'organisation_name': None, 'service_id': mock.ANY, 'service_name': 'second', 'sms_totals': 0, - 'sms_volume_intent': None, 'organisation_type': None} in response - assert {'consent_to_research': None, 'contact_email': 'notify@digital.cabinet-office.gov.uk', - 'contact_mobile': '+447700900986', 'contact_name': 'Test User', 'email_totals': 1, - 'email_volume_intent': None, 'letter_totals': 0, 'letter_volume_intent': None, 'live_date': None, - 'organisation_name': 'test_org_1', 'service_id': mock.ANY, 'service_name': 'Sample service', - 'sms_totals': 1, 'sms_volume_intent': None, 'organisation_type': None} in response + assert response == [ + { + 'consent_to_research': None, + 'contact_email': 'notify@digital.cabinet-office.gov.uk', + 'contact_mobile': '+447700900986', + 'contact_name': 'Test User', + 'email_totals': 1, + 'email_volume_intent': None, + 'letter_totals': 0, + 'letter_volume_intent': None, + 'live_date': 'Mon, 01 Jan 2018 00:00:00 GMT', + 'organisation_name': 'test_org_1', + 'service_id': ANY, + 'service_name': 'Sample service', + 'sms_totals': 1, + 'sms_volume_intent': None, + 'organisation_type': None, + }, + { + 'consent_to_research': None, + 'contact_email': 'notify@digital.cabinet-office.gov.uk', + 'contact_mobile': '+447700900986', + 'contact_name': 'Test User', + 'email_totals': 0, + 'email_volume_intent': None, + 'letter_totals': 0, + 'letter_volume_intent': None, + 'live_date': 'Tue, 01 Jan 2019 00:00:00 GMT', + 'organisation_name': None, + 'service_id': ANY, + 'service_name': 'second', + 'sms_totals': 0, + 'sms_volume_intent': None, + 'organisation_type': None, + }, + ] def test_get_service_by_id(admin_request, sample_service):