Merge pull request #191 from alphagov/wip-read-template-statistics

[WIP] Added dao method and rest endpoint

app/__init__.py

@@ -56,6 +56,7 @@ def create_app(app_name=None):
     from app.permission.rest import permission as permission_blueprint
     from app.accept_invite.rest import accept_invite
     from app.notifications_statistics.rest import notifications_statistics as notifications_statistics_blueprint
+    from app.template_statistics.rest import template_statistics as template_statistics_blueprint
 
     application.register_blueprint(service_blueprint, url_prefix='/service')
     application.register_blueprint(user_blueprint, url_prefix='/user')
@@ -67,6 +68,7 @@ def create_app(app_name=None):
     application.register_blueprint(permission_blueprint, url_prefix='/permission')
     application.register_blueprint(accept_invite, url_prefix='/invite')
     application.register_blueprint(notifications_statistics_blueprint)
+    application.register_blueprint(template_statistics_blueprint)
 
     return application
 

app/dao/notifications_dao.py

@@ -25,6 +25,23 @@ from app.clients import (
     STATISTICS_REQUESTED
 )
 
+from functools import wraps
+
+
+def transactional(func):
+    @wraps(func)
+    def commit_or_rollback(*args, **kwargs):
+        from flask import current_app
+        from app import db
+        try:
+            func(*args, **kwargs)
+            db.session.commit()
+        except Exception as e:
+            current_app.logger.error(e)
+            db.session.rollback()
+            raise
+    return commit_or_rollback
+
 
 def dao_get_notification_statistics_for_service(service_id):
     return NotificationStatistics.query.filter_by(
@@ -39,46 +56,55 @@ def dao_get_notification_statistics_for_service_and_day(service_id, day):
     ).order_by(desc(NotificationStatistics.day)).first()
 
 
+def dao_get_template_statistics_for_service(service_id, limit_days=None):
+    filter = [TemplateStatistics.service_id == service_id]
+    if limit_days:
+        latest_stat = TemplateStatistics.query.filter_by(service_id=service_id).order_by(
+            desc(TemplateStatistics.day)).limit(1).first()
+        if latest_stat:
+            last_date_to_fetch = latest_stat.day - timedelta(days=limit_days)
+        else:
+            last_date_to_fetch = date.today() - timedelta(days=limit_days)
+        filter.append(TemplateStatistics.day > last_date_to_fetch)
+    return TemplateStatistics.query.filter(*filter).order_by(desc(TemplateStatistics.day)).all()
+
+
+@transactional
 def dao_create_notification(notification, notification_type):
-    try:
-        if notification.job_id:
-            db.session.query(Job).filter_by(
-                id=notification.job_id
-            ).update({
-                Job.notifications_sent: Job.notifications_sent + 1,
-                Job.updated_at: datetime.utcnow()
-            })
+    if notification.job_id:
+        db.session.query(Job).filter_by(
+            id=notification.job_id
+        ).update({
+            Job.notifications_sent: Job.notifications_sent + 1,
+            Job.updated_at: datetime.utcnow()
+        })
 
-        update_count = db.session.query(NotificationStatistics).filter_by(
+    update_count = db.session.query(NotificationStatistics).filter_by(
+        day=notification.created_at.strftime('%Y-%m-%d'),
+        service_id=notification.service_id
+    ).update(update_query(notification_type, 'requested'))
+
+    if update_count == 0:
+        stats = NotificationStatistics(
             day=notification.created_at.strftime('%Y-%m-%d'),
-            service_id=notification.service_id
-        ).update(update_query(notification_type, 'requested'))
-
-        if update_count == 0:
-            stats = NotificationStatistics(
-                day=notification.created_at.strftime('%Y-%m-%d'),
-                service_id=notification.service_id,
-                sms_requested=1 if notification_type == TEMPLATE_TYPE_SMS else 0,
-                emails_requested=1 if notification_type == TEMPLATE_TYPE_EMAIL else 0
-            )
-            db.session.add(stats)
-
-        update_count = db.session.query(TemplateStatistics).filter_by(
-            day=date.today(),
             service_id=notification.service_id,
-            template_id=notification.template_id
-        ).update({'usage_count': TemplateStatistics.usage_count + 1})
+            sms_requested=1 if notification_type == TEMPLATE_TYPE_SMS else 0,
+            emails_requested=1 if notification_type == TEMPLATE_TYPE_EMAIL else 0
+        )
+        db.session.add(stats)
 
-        if update_count == 0:
-            template_stats = TemplateStatistics(template_id=notification.template_id,
-                                                service_id=notification.service_id)
-            db.session.add(template_stats)
+    update_count = db.session.query(TemplateStatistics).filter_by(
+        day=date.today(),
+        service_id=notification.service_id,
+        template_id=notification.template_id
+    ).update({'usage_count': TemplateStatistics.usage_count + 1})
 
-        db.session.add(notification)
-        db.session.commit()
-    except:
-        db.session.rollback()
-        raise
+    if update_count == 0:
+        template_stats = TemplateStatistics(template_id=notification.template_id,
+                                            service_id=notification.service_id)
+        db.session.add(template_stats)
+
+    db.session.add(notification)
 
 
 def update_query(notification_type, status):

app/job/rest.py

@@ -37,6 +37,8 @@ def get_job_by_service_and_job_id(service_id, job_id):
 def get_jobs_by_service(service_id):
     jobs = dao_get_jobs_by_service_id(service_id)
     data, errors = job_schema.dump(jobs, many=True)
+    if errors:
+        return jsonify(result="error", message=errors), 400
     return jsonify(data=data)
 
 

app/models.py

@@ -357,7 +357,7 @@ class TemplateStatistics(db.Model):
 
     id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4)
     service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, unique=False, nullable=False)
-    service = db.relationship('Service', backref=db.backref('template_statics', lazy='dynamic'))
+    service = db.relationship('Service', backref=db.backref('template_statistics', lazy='dynamic'))
     template_id = db.Column(db.BigInteger, db.ForeignKey('templates.id'), index=True, nullable=False, unique=False)
     template = db.relationship('Template')
     usage_count = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=1)

app/schemas.py

@@ -1,15 +1,27 @@
 from flask_marshmallow.fields import fields
-from . import ma
-from . import models
-from app.dao.permissions_dao import permission_dao
-from marshmallow import (post_load, ValidationError, validates, validates_schema, pre_load)
+
+from marshmallow import (
+    post_load,
+    ValidationError,
+    validates,
+    validates_schema,
+    pre_load
+)
+
 from marshmallow_sqlalchemy import field_for
+
 from utils.recipients import (
-    validate_email_address, InvalidEmailError,
-    validate_phone_number, InvalidPhoneError,
+    validate_email_address,
+    InvalidEmailError,
+    validate_phone_number,
+    InvalidPhoneError,
     validate_and_format_phone_number
 )
 
+from app import ma
+from app import models
+from app.dao.permissions_dao import permission_dao
+
 
 # TODO I think marshmallow provides a better integration and error handling.
 # Would be better to replace functionality in dao with the marshmallow supported
@@ -19,7 +31,7 @@ from utils.recipients import (
 
 
 class BaseSchema(ma.ModelSchema):
-    def __init__(self, *args, load_json=False, **kwargs):
+    def __init__(self, load_json=False, *args, **kwargs):
         self.load_json = load_json
         super(BaseSchema, self).__init__(*args, **kwargs)
 
@@ -241,6 +253,14 @@ class NotificationsFilterSchema(ma.Schema):
             raise ValidationError("Not a positive integer")
 
 
+class TemplateStatisticsSchema(BaseSchema):
+
+    template = fields.Nested(TemplateSchema, only=["id",  "name", "template_type"], dump_only=True)
+
+    class Meta:
+        model = models.TemplateStatistics
+
+
 user_schema = UserSchema()
 user_schema_load_json = UserSchema(load_json=True)
 service_schema = ServiceSchema()
@@ -264,3 +284,4 @@ permission_schema = PermissionSchema()
 email_data_request_schema = EmailDataSchema()
 notifications_statistics_schema = NotificationsStatisticsSchema()
 notifications_filter_schema = NotificationsFilterSchema()
+template_statistics_schema = TemplateStatisticsSchema()

app/template_statistics/rest.py

@@ -0,0 +1,36 @@
+from flask import (
+    Blueprint,
+    jsonify,
+    request,
+    current_app
+)
+
+from app.dao.notifications_dao import dao_get_template_statistics_for_service
+
+from app.schemas import template_statistics_schema
+
+template_statistics = Blueprint('template-statistics',
+                                __name__,
+                                url_prefix='/service/<service_id>/template-statistics')
+
+from app.errors import register_errors
+
+register_errors(template_statistics)
+
+
+@template_statistics.route('')
+def get_template_statistics_for_service(service_id):
+    if request.args.get('limit_days'):
+        try:
+            limit_days = int(request.args['limit_days'])
+        except ValueError as e:
+            error = '{} is not an integer'.format(request.args['limit_days'])
+            current_app.logger.error(error)
+            return jsonify(result="error", message={'limit_days': [error]}), 400
+    else:
+        limit_days = None
+    stats = dao_get_template_statistics_for_service(service_id, limit_days=limit_days)
+    data, errors = template_statistics_schema.dump(stats, many=True)
+    if errors:
+        return jsonify(result="error", message=errors), 400
+    return jsonify(data=data)

tests/app/dao/test_notification_dao.py

@@ -28,7 +28,8 @@ from app.dao.notifications_dao import (
     dao_get_notification_statistics_for_service_and_day,
     update_notification_status_by_id,
     update_notification_reference_by_id,
-    update_notification_status_by_reference
+    update_notification_status_by_reference,
+    dao_get_template_statistics_for_service
 )
 
 from tests.app.conftest import sample_job
@@ -886,10 +887,132 @@ def test_successful_notification_inserts_followed_by_failure_does_not_increment_
         db.session.execute('DROP TABLE TEMPLATE_STATISTICS')
         dao_create_notification(failing_notification, sample_template.template_type)
     except Exception as e:
-
         # There should be no additional notification stats or counts
         assert NotificationStatistics.query.count() == 1
         notication_stats = NotificationStatistics.query.filter(
             NotificationStatistics.service_id == sample_template.service.id
         ).first()
         assert notication_stats.sms_requested == 3
+
+
+@freeze_time("2016-03-30")
+def test_get_template_stats_for_service_returns_stats_in_reverse_date_order(sample_template, sample_job):
+
+    template_stats = dao_get_template_statistics_for_service(sample_template.service.id)
+    assert len(template_stats) == 0
+    data = {
+        'to': '+44709123456',
+        'job_id': sample_job.id,
+        'service': sample_template.service,
+        'service_id': sample_template.service.id,
+        'template': sample_template,
+        'template_id': sample_template.id,
+        'created_at': datetime.utcnow()
+    }
+
+    notification = Notification(**data)
+    dao_create_notification(notification, sample_template.template_type)
+
+    # move on one day
+    with freeze_time('2016-03-31'):
+        new_notification = Notification(**data)
+        dao_create_notification(new_notification, sample_template.template_type)
+
+    # move on one more day
+    with freeze_time('2016-04-01'):
+        new_notification = Notification(**data)
+        dao_create_notification(new_notification, sample_template.template_type)
+
+    template_stats = dao_get_template_statistics_for_service(sample_template.service_id)
+    assert len(template_stats) == 3
+    assert template_stats[0].day == date(2016, 4, 1)
+    assert template_stats[1].day == date(2016, 3, 31)
+    assert template_stats[2].day == date(2016, 3, 30)
+
+
+@freeze_time('2016-04-09')
+def test_get_template_stats_for_service_returns_stats_can_limit_number_of_days_returned(sample_template):
+
+    template_stats = dao_get_template_statistics_for_service(sample_template.service.id)
+    assert len(template_stats) == 0
+
+    # make 9 stats records from 1st to 9th April
+    for i in range(1, 10):
+        past_date = '2016-04-0{}'.format(i)
+        with freeze_time(past_date):
+            template_stats = TemplateStatistics(template_id=sample_template.id,
+                                                service_id=sample_template.service_id)
+            db.session.add(template_stats)
+            db.session.commit()
+
+    # Retrieve last week of stats
+    template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=7)
+    assert len(template_stats) == 7
+    assert template_stats[0].day == date(2016, 4, 9)
+    assert template_stats[6].day == date(2016, 4, 3)
+
+
+@freeze_time('2016-04-09')
+def test_get_template_stats_for_service_returns_stats_returns_all_stats_if_no_limit(sample_template):
+
+    template_stats = dao_get_template_statistics_for_service(sample_template.service.id)
+    assert len(template_stats) == 0
+
+    # make 9 stats records from 1st to 9th April
+    for i in range(1, 10):
+        past_date = '2016-04-0{}'.format(i)
+        with freeze_time(past_date):
+            template_stats = TemplateStatistics(template_id=sample_template.id,
+                                                service_id=sample_template.service_id)
+            db.session.add(template_stats)
+            db.session.commit()
+
+    template_stats = dao_get_template_statistics_for_service(sample_template.service_id)
+    assert len(template_stats) == 9
+    assert template_stats[0].day == date(2016, 4, 9)
+    assert template_stats[8].day == date(2016, 4, 1)
+
+
+@freeze_time('2016-04-30')
+def test_get_template_stats_for_service_returns_results_from_first_day_with_data(sample_template):
+
+    template_stats = dao_get_template_statistics_for_service(sample_template.service.id)
+    assert len(template_stats) == 0
+
+    # make 9 stats records from 1st to 9th April - no data after 10th
+    for i in range(1, 10):
+        past_date = '2016-04-0{}'.format(i)
+        with freeze_time(past_date):
+            template_stats = TemplateStatistics(template_id=sample_template.id,
+                                                service_id=sample_template.service_id)
+            db.session.add(template_stats)
+            db.session.commit()
+
+    # Retrieve one day of stats - read date is 2016-04-30
+    template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=1)
+    assert len(template_stats) == 1
+    assert template_stats[0].day == date(2016, 4, 9)
+
+    # Retrieve three days of stats
+    template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=3)
+    assert len(template_stats) == 3
+    assert template_stats[0].day == date(2016, 4, 9)
+    assert template_stats[1].day == date(2016, 4, 8)
+    assert template_stats[2].day == date(2016, 4, 7)
+
+    # Retrieve nine days of stats
+    template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=9)
+    assert len(template_stats) == 9
+    assert template_stats[0].day == date(2016, 4, 9)
+    assert template_stats[8].day == date(2016, 4, 1)
+
+    # Retrieve with no limit
+    template_stats = dao_get_template_statistics_for_service(sample_template.service_id)
+    assert len(template_stats) == 9
+    assert template_stats[0].day == date(2016, 4, 9)
+    assert template_stats[8].day == date(2016, 4, 1)
+
+
+def test_get_template_stats_for_service_with_limit_if_no_records_returns_empty_list(sample_template):
+    template_stats = dao_get_template_statistics_for_service(sample_template.service.id, limit_days=7)
+    assert len(template_stats) == 0

tests/app/template_statistics/test_rest.py

@@ -0,0 +1,136 @@
+import json
+from freezegun import freeze_time
+
+from app import db
+from app.models import TemplateStatistics
+
+from tests import create_authorization_header
+
+
+@freeze_time('2016-04-09')
+def test_get_template_statistics_for_service_for_last_week(notify_api, sample_template):
+
+    # make 9 stats records from 1st to 9th April
+    for i in range(1, 10):
+        past_date = '2016-04-0{}'.format(i)
+        with freeze_time(past_date):
+            template_stats = TemplateStatistics(template_id=sample_template.id,
+                                                service_id=sample_template.service_id)
+            db.session.add(template_stats)
+            db.session.commit()
+
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+
+            auth_header = create_authorization_header(
+                path='/service/{}/template-statistics'.format(sample_template.service_id),
+                method='GET'
+            )
+
+            response = client.get(
+                '/service/{}/template-statistics'.format(sample_template.service_id),
+                headers=[('Content-Type', 'application/json'), auth_header],
+                query_string={'limit_days': 7}
+            )
+
+            assert response.status_code == 200
+            json_resp = json.loads(response.get_data(as_text=True))
+            assert len(json_resp['data']) == 7
+            assert json_resp['data'][0]['day'] == '2016-04-09'
+            assert json_resp['data'][6]['day'] == '2016-04-03'
+
+
+@freeze_time('2016-04-30')
+def test_get_template_statistics_for_service_for_last_actual_week_with_data(notify_api, sample_template):
+
+    # make 9 stats records from 1st to 9th April
+    for i in range(1, 10):
+        past_date = '2016-04-0{}'.format(i)
+        with freeze_time(past_date):
+            template_stats = TemplateStatistics(template_id=sample_template.id,
+                                                service_id=sample_template.service_id)
+            db.session.add(template_stats)
+            db.session.commit()
+
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+
+            auth_header = create_authorization_header(
+                path='/service/{}/template-statistics'.format(sample_template.service_id),
+                method='GET'
+            )
+
+            # Date is frozen at 2016-04-30 and no data written since
+            response = client.get(
+                '/service/{}/template-statistics'.format(sample_template.service_id),
+                headers=[('Content-Type', 'application/json'), auth_header],
+                query_string={'limit_days': 7}
+            )
+
+            assert response.status_code == 200
+            json_resp = json.loads(response.get_data(as_text=True))
+            assert len(json_resp['data']) == 7
+            assert json_resp['data'][0]['day'] == '2016-04-09'
+            assert json_resp['data'][6]['day'] == '2016-04-03'
+
+
+def test_get_all_template_statistics_for_service(notify_api, sample_template):
+
+    # make 9 stats records from 1st to 9th April
+    for i in range(1, 10):
+        past_date = '2016-04-0{}'.format(i)
+        with freeze_time(past_date):
+            template_stats = TemplateStatistics(template_id=sample_template.id,
+                                                service_id=sample_template.service_id)
+            db.session.add(template_stats)
+            db.session.commit()
+
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+
+            auth_header = create_authorization_header(
+                path='/service/{}/template-statistics'.format(sample_template.service_id),
+                method='GET'
+            )
+
+            response = client.get(
+                '/service/{}/template-statistics'.format(sample_template.service_id),
+                headers=[('Content-Type', 'application/json'), auth_header]
+            )
+
+            assert response.status_code == 200
+            json_resp = json.loads(response.get_data(as_text=True))
+            assert len(json_resp['data']) == 9
+            assert json_resp['data'][0]['day'] == '2016-04-09'
+            assert json_resp['data'][8]['day'] == '2016-04-01'
+
+
+def test_get_all_template_statistics_with_bad_limit_arg_returns_400(notify_api, sample_template):
+
+    # make 9 stats records from 1st to 9th April
+    for i in range(1, 10):
+        past_date = '2016-04-0{}'.format(i)
+        with freeze_time(past_date):
+            template_stats = TemplateStatistics(template_id=sample_template.id,
+                                                service_id=sample_template.service_id)
+            db.session.add(template_stats)
+            db.session.commit()
+
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+
+            auth_header = create_authorization_header(
+                path='/service/{}/template-statistics'.format(sample_template.service_id),
+                method='GET'
+            )
+
+            response = client.get(
+                '/service/{}/template-statistics'.format(sample_template.service_id),
+                headers=[('Content-Type', 'application/json'), auth_header],
+                query_string={'limit_days': 'blurk'}
+            )
+
+            assert response.status_code == 400
+            json_resp = json.loads(response.get_data(as_text=True))
+            assert json_resp['result'] == 'error'
+            assert json_resp['message'] == {'limit_days': ['blurk is not an integer']}