From a5cf8ff60f107968feec673d72f515ae9b9848f0 Mon Sep 17 00:00:00 2001 From: venusbb Date: Wed, 12 Jul 2017 13:49:20 +0100 Subject: [PATCH 01/34] put more log messages to view what env returns --- app/authentication/auth.py | 3 ++- tests/app/authentication/test_authentication.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 8242795b2..9c42f2431 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -52,7 +52,8 @@ def restrict_ip_sms(): ip_list = ip_route.split(',') if len(ip_list) >= 3: ip = ip_list[len(ip_list) - 3] - current_app.logger.info("Inbound sms ip route list {}".format(ip_route)) + current_app.logger.info("Inbound sms ip route list {} OS environ{}" + .format(ip_route, current_app.config.get('SMS_INBOUND_WHITELIST'))) if ip in current_app.config.get('SMS_INBOUND_WHITELIST'): current_app.logger.info("Inbound sms ip addresses {} passed ".format(ip)) diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index e423ab9a2..f0420e755 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -351,7 +351,7 @@ def test_reject_invalid_ips(restrict_ip_sms_app): assert exc_info.value.short_message == 'Unknown source IP address from the SMS provider' -@pytest.mark.xfail(reason='Currently not blocking invalid IPs', strict=True) +@pytest.mark.xfail(reason='Currently not blocking invalid senders', strict=True) def test_illegitimate_ips(restrict_ip_sms_app): with pytest.raises(AuthError) as exc_info: restrict_ip_sms_app.get( @@ -361,4 +361,4 @@ def test_illegitimate_ips(restrict_ip_sms_app): ] ) - assert exc_info.value.short_message == 'Unknown source IP address from the SMS provider' + assert exc_info.value.short_message == 'Unknown IP route not from known SMS provider' From 4b05c32b622d36ee8e5f3b1b9ca3d99735bc2c1f Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 13 Jul 2017 17:22:11 +0100 Subject: [PATCH 02/34] Create a new table to warehouse the monthly billing numbers --- app/dao/monthly_billing_dao.py | 6 ++++ app/models.py | 17 +++++++++- migrations/versions/0109_monthly_billing.py | 37 +++++++++++++++++++++ tests/app/dao/test_monthly_billing.py | 33 ++++++++++++++++++ 4 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 app/dao/monthly_billing_dao.py create mode 100644 migrations/versions/0109_monthly_billing.py create mode 100644 tests/app/dao/test_monthly_billing.py diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py new file mode 100644 index 000000000..c037b5c54 --- /dev/null +++ b/app/dao/monthly_billing_dao.py @@ -0,0 +1,6 @@ +from app import db + + +def update_monthly_billing(monthly_billing): + db.session.add(monthly_billing) + db.session.commit() diff --git a/app/models.py b/app/models.py index 0e3079c80..07ba505d8 100644 --- a/app/models.py +++ b/app/models.py @@ -4,7 +4,6 @@ import datetime from flask import url_for, current_app from sqlalchemy.ext.associationproxy import association_proxy -from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.dialects.postgresql import ( UUID, JSON @@ -1246,3 +1245,19 @@ class LetterRateDetail(db.Model): letter_rate = db.relationship('LetterRate', backref='letter_rates') page_total = db.Column(db.Integer, nullable=False) rate = db.Column(db.Numeric(), nullable=False) + + +class MonthlyBilling(db.Model): + __tablename__ = 'monthly_billing' + + 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, nullable=False) + service = db.relationship('Service', backref='monthly_billing') + month = db.Column(db.String, nullable=False) + year = db.Column(db.Float(asdecimal=False), nullable=False) + notification_type = db.Column(notification_types, nullable=False) + monthly_totals = db.Column(JSON, nullable=False) + + __table_args__ = ( + UniqueConstraint('service_id', 'month', 'year', 'notification_type', name='uix_monthly_billing'), + ) diff --git a/migrations/versions/0109_monthly_billing.py b/migrations/versions/0109_monthly_billing.py new file mode 100644 index 000000000..124a509cf --- /dev/null +++ b/migrations/versions/0109_monthly_billing.py @@ -0,0 +1,37 @@ +"""empty message + +Revision ID: 0109_monthly_billing +Revises: 0108_change_logo_not_nullable +Create Date: 2017-07-13 14:35:03.183659 + +""" + +# revision identifiers, used by Alembic. +revision = '0109_monthly_billing' +down_revision = '0108_change_logo_not_nullable' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + + +def upgrade(): + + op.create_table('monthly_billing', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('month', sa.String(), nullable=False), + sa.Column('year', sa.Float(), nullable=False), + sa.Column('notification_type', + postgresql.ENUM('email', 'sms', 'letter', name='notification_type', create_type=False), + nullable=False), + sa.Column('monthly_totals', postgresql.JSON(), nullable=False), + sa.ForeignKeyConstraint(['service_id'], ['services.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_monthly_billing_service_id'), 'monthly_billing', ['service_id'], unique=False) + op.create_index(op.f('uix_monthly_billing'), 'monthly_billing', ['service_id', 'month', 'year', 'notification_type'], unique=True) + + +def downgrade(): + op.drop_table('monthly_billing') diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py new file mode 100644 index 000000000..f04fa79ca --- /dev/null +++ b/tests/app/dao/test_monthly_billing.py @@ -0,0 +1,33 @@ +import uuid + +import pytest +from sqlalchemy.exc import IntegrityError + +from app.dao.monthly_billing_dao import update_monthly_billing +from app.models import MonthlyBilling + + +def test_add_monthly_billing_only_allows_one_row_per_service_month_type(sample_service): + first = MonthlyBilling(id=uuid.uuid4(), + service_id=sample_service.id, + notification_type='sms', + month='January', + year='2017', + monthly_totals={'billing_units': 100, + 'rate': 0.0158}) + + second = MonthlyBilling(id=uuid.uuid4(), + service_id=sample_service.id, + notification_type='sms', + month='January', + year='2017', + monthly_totals={'billing_units': 50, + 'rate': 0.0162}) + + update_monthly_billing(first) + with pytest.raises(IntegrityError): + update_monthly_billing(second) + monthly = MonthlyBilling.query.all() + assert len(monthly) == 1 + assert monthly[0].monthly_totals == {'billing_units': 100, + 'rate': 0.0158} From 9400988d72e97f9613f4681293f54018a3650a5b Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 18 Jul 2017 18:21:35 +0100 Subject: [PATCH 03/34] Monthly billing - part 1 This is still a work in progress but it would be good to get some eyes on it. This commit includes creating and updating a row in the monthly billing table and a method to fetch the results. There is a command to populate the monthly billing for a service and month so we can try it out. The total cost at the moment are wrong, they do not take into account the free allowance - see notes below about adding that to the table. Left to do: create a nightly task to run to update the monthly totals. create an endpoint to return the yearly billing, the current day will need to be calculated on the fly and added to the totals. Add the free allowance into the total costs. --- app/commands.py | 18 +++- app/dao/date_util.py | 13 +++ app/dao/monthly_billing_dao.py | 42 +++++++- app/dao/notification_usage_dao.py | 17 ++- app/dao/provider_rates_dao.py | 5 + app/delivery/send_to_providers.py | 2 +- app/models.py | 9 ++ app/service/rest.py | 1 - application.py | 1 + tests/app/dao/test_date_utils.py | 19 +++- tests/app/dao/test_monthly_billing.py | 128 ++++++++++++++++++----- tests/app/dao/test_provider_rates_dao.py | 13 ++- tests/app/db.py | 9 +- 13 files changed, 240 insertions(+), 37 deletions(-) diff --git a/app/commands.py b/app/commands.py index 6e4bc53d1..c7ec0567b 100644 --- a/app/commands.py +++ b/app/commands.py @@ -5,7 +5,8 @@ from flask.ext.script import Command, Manager, Option from app import db -from app.models import (PROVIDERS, Service, User, NotificationHistory) +from app.dao.monthly_billing_dao import create_or_update_monthly_billing_sms, get_monthly_billing_sms +from app.models import (PROVIDERS, User) from app.dao.services_dao import ( delete_service_and_all_associated_db_objects, dao_fetch_all_services_by_user @@ -146,3 +147,18 @@ class CustomDbScript(Command): print('Committed {} updates at {}'.format(len(result), datetime.utcnow())) db.session.commit() result = db.session.execute(subq_hist).fetchall() + + +class PopulateMonthlyBilling(Command): + option_list = ( + Option('-s', '-service-id', dest='service_id', + help="Service id to populate monthly billing for"), + Option('-m', '-month', dest="month", help="Use for integer value for month, e.g. 7 for July"), + Option('-y', '-year', dest="year", help="Use for integer value for year, e.g. 2017") + ) + + def run(self, service_id, month, year): + create_or_update_monthly_billing_sms(service_id, datetime(int(year), int(month), 1)) + results = get_monthly_billing_sms(service_id, datetime(int(year), int(month), 1)) + print("Finished populating data for {} for service id {}".format(month, service_id)) + print(results.monthly_totals) diff --git a/app/dao/date_util.py b/app/dao/date_util.py index 2471865bd..41ad9dc72 100644 --- a/app/dao/date_util.py +++ b/app/dao/date_util.py @@ -16,3 +16,16 @@ def get_april_fools(year): """ return pytz.timezone('Europe/London').localize(datetime(year, 4, 1, 0, 0, 0)).astimezone(pytz.UTC).replace( tzinfo=None) + + +def get_month_start_end_date(month_year): + """ + This function return the start and date of the month_year as UTC, + :param month_year: the datetime to calculate the start and end date for that month + :return: start_date, end_date, month + """ + import calendar + _, num_days = calendar.monthrange(month_year.year, month_year.month) + first_day = datetime(month_year.year, month_year.month, 1, 0, 0, 0) + last_day = datetime(month_year.year, month_year.month, num_days, 23, 59, 59, 99999) + return first_day, last_day diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index c037b5c54..8e3204584 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -1,6 +1,44 @@ +from datetime import datetime + from app import db +from app.dao.notification_usage_dao import get_billing_data_for_month +from app.models import MonthlyBilling, SMS_TYPE -def update_monthly_billing(monthly_billing): - db.session.add(monthly_billing) +def create_or_update_monthly_billing_sms(service_id, billing_month): + monthly = get_billing_data_for_month(service_id=service_id, billing_month=billing_month) + # update monthly + monthly_totals = _monthly_billing_data_to_json(monthly) + row = MonthlyBilling.query.filter_by(year=billing_month.year, + month=datetime.strftime(billing_month, "%B"), + notification_type='sms').first() + if row: + row.monthly_totals = monthly_totals + else: + row = MonthlyBilling(service_id=service_id, + notification_type=SMS_TYPE, + year=billing_month.year, + month=datetime.strftime(billing_month, "%B"), + monthly_totals=monthly_totals) + db.session.add(row) db.session.commit() + + +def get_monthly_billing_sms(service_id, billing_month): + monthly = MonthlyBilling.query.filter_by(service_id=service_id, + year=billing_month.year, + month=datetime.strftime(billing_month, "%B"), + notification_type=SMS_TYPE).first() + return monthly + + +def _monthly_billing_data_to_json(monthly): + # ('April', 6, 1, False, 'sms', 0.014) + # (month, billing_units, rate_multiplier, international, notification_type, rate) + # total cost must take into account the free allowance. + # might be a good idea to capture free allowance in this table + return [{"billing_units": x[1], + "rate_multiplier": x[2], + "international": x[3], + "rate": x[5], + "total_cost": (x[1] * x[2]) * x[5]} for x in monthly] diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 995462509..289a5a03d 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -6,7 +6,7 @@ from sqlalchemy import func, case, cast from sqlalchemy import literal_column from app import db -from app.dao.date_util import get_financial_year +from app.dao.date_util import get_financial_year, get_month_start_end_date from app.models import (NotificationHistory, Rate, NOTIFICATION_STATUS_TYPES_BILLABLE, @@ -35,6 +35,21 @@ def get_yearly_billing_data(service_id, year): return sum(result, []) +@statsd(namespace="dao") +def get_billing_data_for_month(service_id, billing_month): + start_date, end_date = get_month_start_end_date(billing_month) + rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + result = [] + # so the start end date in the query are the valid from the rate, not the month - this is going to take some thought + for r, n in zip(rates, rates[1:]): + result.extend(sms_billing_data_per_month_query(r.rate, service_id, max(r.valid_from, start_date), + min(n.valid_from, end_date))) + result.extend( + sms_billing_data_per_month_query(rates[-1].rate, service_id, max(rates[-1].valid_from, start_date), end_date)) + + return result + + @statsd(namespace="dao") def get_monthly_billing_data(service_id, year): start_date, end_date = get_financial_year(year) diff --git a/app/dao/provider_rates_dao.py b/app/dao/provider_rates_dao.py index 145bd431e..443543f86 100644 --- a/app/dao/provider_rates_dao.py +++ b/app/dao/provider_rates_dao.py @@ -9,3 +9,8 @@ def create_provider_rates(provider_identifier, valid_from, rate): provider_rates = ProviderRates(provider_id=provider.id, valid_from=valid_from, rate=rate) db.session.add(provider_rates) + + +@transactional +def create_sms_rate(rate): + db.session.add(rate) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index df4c664f9..5e33e2113 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -18,7 +18,7 @@ from app.dao.templates_dao import dao_get_template_by_id from app.models import SMS_TYPE, KEY_TYPE_TEST, BRANDING_ORG, EMAIL_TYPE, NOTIFICATION_TECHNICAL_FAILURE, \ NOTIFICATION_SENT, NOTIFICATION_SENDING -from app.celery.statistics_tasks import record_initial_job_statistics, create_initial_notification_statistic_tasks +from app.celery.statistics_tasks import create_initial_notification_statistic_tasks def send_sms_to_provider(notification): diff --git a/app/models.py b/app/models.py index 07ba505d8..e035acccb 100644 --- a/app/models.py +++ b/app/models.py @@ -1261,3 +1261,12 @@ class MonthlyBilling(db.Model): __table_args__ = ( UniqueConstraint('service_id', 'month', 'year', 'notification_type', name='uix_monthly_billing'), ) + + def serialized(self): + return { + "month": self.month, + "year": self.year, + "service_id": str(self.service_id), + "notification_type": self.notification_type, + "monthly_totals": self.monthly_totals + } diff --git a/app/service/rest.py b/app/service/rest.py index ae57f5390..deb37f525 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -68,7 +68,6 @@ from app.schemas import ( user_schema, permission_schema, notification_with_template_schema, - notification_with_personalisation_schema, notifications_filter_schema, detailed_service_schema ) diff --git a/application.py b/application.py index b2a59a6fd..5ff7dd1e4 100644 --- a/application.py +++ b/application.py @@ -16,6 +16,7 @@ manager.add_command('db', MigrateCommand) manager.add_command('create_provider_rate', commands.CreateProviderRateCommand) manager.add_command('purge_functional_test_data', commands.PurgeFunctionalTestDataCommand) manager.add_command('custom_db_script', commands.CustomDbScript) +manager.add_command('populate_monthly_billing', commands.PopulateMonthlyBilling) @manager.command diff --git a/tests/app/dao/test_date_utils.py b/tests/app/dao/test_date_utils.py index 760923c5a..4ede18853 100644 --- a/tests/app/dao/test_date_utils.py +++ b/tests/app/dao/test_date_utils.py @@ -1,4 +1,8 @@ -from app.dao.date_util import get_financial_year, get_april_fools +from datetime import datetime + +import pytest + +from app.dao.date_util import get_financial_year, get_april_fools, get_month_start_end_date def test_get_financial_year(): @@ -11,3 +15,16 @@ def test_get_april_fools(): april_fools = get_april_fools(2016) assert str(april_fools) == '2016-03-31 23:00:00' assert april_fools.tzinfo is None + + +@pytest.mark.parametrize("month, year, expected_end", + [(7, 2017, 31), + (2, 2016, 29), + (2, 2017, 28), + (9, 2018, 30), + (12, 2019, 31)]) +def test_get_month_start_end_date(month, year, expected_end): + month_year = datetime(year, month, 10, 13, 30, 00) + result = get_month_start_end_date(month_year) + assert result[0] == datetime(year, month, 1, 0, 0, 0, 0) + assert result[1] == datetime(year, month, expected_end, 23, 59, 59, 99999) diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index f04fa79ca..6a0399d67 100644 --- a/tests/app/dao/test_monthly_billing.py +++ b/tests/app/dao/test_monthly_billing.py @@ -1,33 +1,107 @@ -import uuid +from datetime import datetime -import pytest -from sqlalchemy.exc import IntegrityError - -from app.dao.monthly_billing_dao import update_monthly_billing +from app.dao.monthly_billing_dao import create_or_update_monthly_billing_sms, get_monthly_billing_sms from app.models import MonthlyBilling +from tests.app.db import create_notification, create_rate -def test_add_monthly_billing_only_allows_one_row_per_service_month_type(sample_service): - first = MonthlyBilling(id=uuid.uuid4(), - service_id=sample_service.id, - notification_type='sms', - month='January', - year='2017', - monthly_totals={'billing_units': 100, - 'rate': 0.0158}) +def test_add_monthly_billing(sample_template): + jan = datetime(2017, 1, 1) + feb = datetime(2017, 2, 15) + create_rate(start_date=jan, value=0.0158, notification_type='sms') + create_notification(template=sample_template, created_at=jan, billable_units=1, status='delivered') + create_notification(template=sample_template, created_at=feb, billable_units=2, status='delivered') - second = MonthlyBilling(id=uuid.uuid4(), - service_id=sample_service.id, - notification_type='sms', - month='January', - year='2017', - monthly_totals={'billing_units': 50, - 'rate': 0.0162}) + create_or_update_monthly_billing_sms(service_id=sample_template.service_id, + billing_month=jan) + create_or_update_monthly_billing_sms(service_id=sample_template.service_id, + billing_month=feb) + monthly_billing = MonthlyBilling.query.all() + assert len(monthly_billing) == 2 + assert monthly_billing[0].month == 'January' + assert monthly_billing[1].month == 'February' - update_monthly_billing(first) - with pytest.raises(IntegrityError): - update_monthly_billing(second) - monthly = MonthlyBilling.query.all() - assert len(monthly) == 1 - assert monthly[0].monthly_totals == {'billing_units': 100, - 'rate': 0.0158} + january = get_monthly_billing_sms(service_id=sample_template.service_id, billing_month=jan) + expected_jan = {"billing_units": 1, + "rate_multiplier": 1, + "international": False, + "rate": 0.0158, + "total_cost": 1 * 0.0158} + assert_monthly_billing(january, 2017, "January", sample_template.service_id, 1, expected_jan) + + february = get_monthly_billing_sms(service_id=sample_template.service_id, billing_month=feb) + expected_feb = {"billing_units": 2, + "rate_multiplier": 1, + "international": False, + "rate": 0.0158, + "total_cost": 2 * 0.0158} + assert_monthly_billing(february, 2017, "February", sample_template.service_id, 1, expected_feb) + + +def test_add_monthly_billing_multiple_rates_in_a_month(sample_template): + rate_1 = datetime(2016, 12, 1) + rate_2 = datetime(2017, 1, 15) + create_rate(start_date=rate_1, value=0.0158, notification_type='sms') + create_rate(start_date=rate_2, value=0.0124, notification_type='sms') + + create_notification(template=sample_template, created_at=datetime(2017, 1, 1), billable_units=1, status='delivered') + create_notification(template=sample_template, created_at=datetime(2017, 1, 14, 23, 59), billable_units=1, + status='delivered') + + create_notification(template=sample_template, created_at=datetime(2017, 1, 15), billable_units=2, + status='delivered') + create_notification(template=sample_template, created_at=datetime(2017, 1, 17, 13, 30, 57), billable_units=4, + status='delivered') + + create_or_update_monthly_billing_sms(service_id=sample_template.service_id, + billing_month=rate_2) + monthly_billing = MonthlyBilling.query.all() + assert len(monthly_billing) == 1 + assert monthly_billing[0].month == 'January' + + january = get_monthly_billing_sms(service_id=sample_template.service_id, billing_month=rate_2) + first_row = {"billing_units": 2, + "rate_multiplier": 1, + "international": False, + "rate": 0.0158, + "total_cost": 3 * 0.0158} + assert_monthly_billing(january, 2017, "January", sample_template.service_id, 2, first_row) + second_row = {"billing_units": 6, + "rate_multiplier": 1, + "international": False, + "rate": 0.0124, + "total_cost": 1 * 0.0124} + assert sorted(january.monthly_totals[1]) == sorted(second_row) + + +def test_update_monthly_billing_overwrites_old_totals(sample_template): + july = datetime(2017, 7, 1) + create_rate(july, 0.123, 'sms') + create_notification(template=sample_template, created_at=datetime(2017, 7, 2), billable_units=1, status='delivered') + + create_or_update_monthly_billing_sms(sample_template.service_id, july) + first_update = get_monthly_billing_sms(sample_template.service_id, july) + expected = {"billing_units": 1, + "rate_multiplier": 1, + "international": False, + "rate": 0.123, + "total_cost": 1 * 0.123} + assert_monthly_billing(first_update, 2017, "July", sample_template.service_id, 1, expected) + + create_notification(template=sample_template, created_at=datetime(2017, 7, 5), billable_units=2, status='delivered') + create_or_update_monthly_billing_sms(sample_template.service_id, july) + second_update = get_monthly_billing_sms(sample_template.service_id, july) + expected_update = {"billing_units": 3, + "rate_multiplier": 1, + "international": False, + "rate": 0.123, + "total_cost": 3 * 0.123} + assert_monthly_billing(second_update, 2017, "July", sample_template.service_id, 1, expected_update) + + +def assert_monthly_billing(monthly_billing, year, month, service_id, expected_len, first_row): + assert monthly_billing.year == year + assert monthly_billing.month == month + assert monthly_billing.service_id == service_id + assert len(monthly_billing.monthly_totals) == expected_len + assert sorted(monthly_billing.monthly_totals[0]) == sorted(first_row) diff --git a/tests/app/dao/test_provider_rates_dao.py b/tests/app/dao/test_provider_rates_dao.py index 417612781..7edb7de43 100644 --- a/tests/app/dao/test_provider_rates_dao.py +++ b/tests/app/dao/test_provider_rates_dao.py @@ -1,7 +1,8 @@ +import uuid from datetime import datetime from decimal import Decimal -from app.dao.provider_rates_dao import create_provider_rates -from app.models import ProviderRates, ProviderDetails +from app.dao.provider_rates_dao import create_provider_rates, create_sms_rate +from app.models import ProviderRates, ProviderDetails, Rate def test_create_provider_rates(notify_db, notify_db_session, mmg_provider): @@ -15,3 +16,11 @@ def test_create_provider_rates(notify_db, notify_db_session, mmg_provider): assert ProviderRates.query.first().rate == rate assert ProviderRates.query.first().valid_from == now assert ProviderRates.query.first().provider_id == provider.id + + +def test_create_sms_rate(): + rate = Rate(id=uuid.uuid4(), valid_from=datetime.now(), rate=0.014, notification_type='sms') + create_sms_rate(rate) + rates = Rate.query.all() + assert len(rates) == 1 + assert rates[0] == rate diff --git a/tests/app/db.py b/tests/app/db.py index ea9e4cbdd..669803d92 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1,8 +1,8 @@ from datetime import datetime import uuid - from app.dao.jobs_dao import dao_create_job +from app.dao.provider_rates_dao import create_sms_rate from app.dao.service_inbound_api_dao import save_service_inbound_api from app.models import ( Service, @@ -11,6 +11,7 @@ from app.models import ( Notification, ScheduledNotification, ServicePermission, + Rate, Job, InboundSms, Organisation, @@ -239,3 +240,9 @@ def create_organisation(colour='blue', logo='test_x2.png', name='test_org_1'): dao_create_organisation(organisation) return organisation + + +def create_rate(start_date, value, notification_type): + rate = Rate(id=uuid.uuid4(), valid_from=start_date, rate=value, notification_type=notification_type) + create_sms_rate(rate) + return rate From 793248a74f0da249bbe9e23582b00dff953919aa Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 19 Jul 2017 15:47:12 +0100 Subject: [PATCH 04/34] Fix data migration merge conflict --- .../{0109_monthly_billing.py => 0110_monthly_billing.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0109_monthly_billing.py => 0110_monthly_billing.py} (89%) diff --git a/migrations/versions/0109_monthly_billing.py b/migrations/versions/0110_monthly_billing.py similarity index 89% rename from migrations/versions/0109_monthly_billing.py rename to migrations/versions/0110_monthly_billing.py index 124a509cf..19b1ce4fc 100644 --- a/migrations/versions/0109_monthly_billing.py +++ b/migrations/versions/0110_monthly_billing.py @@ -1,14 +1,14 @@ """empty message -Revision ID: 0109_monthly_billing -Revises: 0108_change_logo_not_nullable +Revision ID: 0110_monthly_billing +Revises: 0109_rem_old_noti_status Create Date: 2017-07-13 14:35:03.183659 """ # revision identifiers, used by Alembic. -revision = '0109_monthly_billing' -down_revision = '0108_change_logo_not_nullable' +revision = '0110_monthly_billing' +down_revision = '0109_rem_old_noti_status' from alembic import op import sqlalchemy as sa From 7f4eec79e421bc7de97ac60e28d3461094b5495c Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 7 Jul 2017 10:46:26 +0100 Subject: [PATCH 05/34] add POST letter schema similar to sms/email, however, for consistency with response values and internal storage, rather than supplying an "email_address" field or a "phone_number" field, supply an "address_line_1" and "postcode" field within the personalisation object. --- app/schema_validation/definitions.py | 3 + app/v2/notifications/notification_schemas.py | 109 ++++++++---- app/v2/notifications/post_notifications.py | 7 +- .../test_post_letter_notifications.py | 158 ++++++++++++++++++ .../notifications/test_post_notifications.py | 4 +- 5 files changed, 243 insertions(+), 38 deletions(-) create mode 100644 tests/app/v2/notifications/test_post_letter_notifications.py diff --git a/app/schema_validation/definitions.py b/app/schema_validation/definitions.py index f0fdc9356..e779c5794 100644 --- a/app/schema_validation/definitions.py +++ b/app/schema_validation/definitions.py @@ -20,6 +20,9 @@ personalisation = { } +letter_personalisation = dict(personalisation, required=["address_line_1", "postcode"]) + + https_url = { "type": "string", "format": "uri", diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 69372c4e1..0f7081e00 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -1,5 +1,5 @@ from app.models import NOTIFICATION_STATUS_TYPES, TEMPLATE_TYPES -from app.schema_validation.definitions import (uuid, personalisation) +from app.schema_validation.definitions import (uuid, personalisation, letter_personalisation) template = { @@ -192,40 +192,89 @@ post_email_response = { } -def create_post_sms_response_from_notification(notification, body, from_number, url_root, service_id, scheduled_for): - return {"id": notification.id, - "reference": notification.client_reference, - "content": {'body': body, - 'from_number': from_number}, - "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)), - "template": __create_template_from_notification(notification=notification, - url_root=url_root, - service_id=service_id), - "scheduled_for": scheduled_for if scheduled_for else None - } +post_letter_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST letter notification schema", + "type": "object", + "title": "POST v2/notifications/letter", + "properties": { + "reference": {"type": "string"}, + "template_id": uuid, + "personalisation": letter_personalisation + }, + "required": ["template_id", "personalisation"] +} + +letter_content = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "Letter content for POST letter notification", + "type": "object", + "title": "notification letter content", + "properties": { + "body": {"type": "string"}, + "subject": {"type": "string"} + }, + "required": ["body", "subject"] +} + +post_letter_response = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST sms notification response schema", + "type": "object", + "title": "response v2/notifications/letter", + "properties": { + "id": uuid, + "reference": {"type": ["string", "null"]}, + "content": letter_content, + "uri": {"type": "string", "format": "uri"}, + "template": template, + "scheduled_for": {"type": ["string", "null"]} + }, + "required": ["id", "content", "uri", "template"] +} -def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, service_id, - scheduled_for): +def create_post_sms_response_from_notification(notification, body, from_number, url_root, scheduled_for): + noti = __create_notification_response(notification, url_root, scheduled_for) + noti['content'] = { + 'from_number': from_number, + 'body': body + } + return noti + + +def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, scheduled_for): + noti = __create_notification_response(notification, url_root, scheduled_for) + noti['content'] = { + "from_email": email_from, + "body": content, + "subject": subject + } + return noti + + +def create_post_letter_response_from_notification(notification, content, subject, url_root, scheduled_for): + noti = __create_notification_response(notification, url_root, scheduled_for) + noti['content'] = { + "body": content, + "subject": subject + } + return noti + + +def __create_notification_response(notification, url_root, scheduled_for): return { "id": notification.id, "reference": notification.client_reference, - "content": { - "from_email": email_from, - "body": content, - "subject": subject - }, "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)), - "template": __create_template_from_notification(notification=notification, - url_root=url_root, - service_id=service_id), + 'template': { + "id": notification.template_id, + "version": notification.template_version, + "uri": "{}services/{}/templates/{}".format( + url_root, + str(notification.service_id), + str(notification.template_id) + ) + }, "scheduled_for": scheduled_for if scheduled_for else None } - - -def __create_template_from_notification(notification, url_root, service_id): - return { - "id": notification.template_id, - "version": notification.template_version, - "uri": "{}services/{}/templates/{}".format(url_root, str(service_id), str(notification.template_id)) - } diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 274005386..9ccbdb70a 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -2,7 +2,7 @@ from flask import request, jsonify, current_app from app import api_user, authenticated_service from app.config import QueueNames -from app.models import SMS_TYPE, EMAIL_TYPE, PRIORITY, SCHEDULE_NOTIFICATIONS +from app.models import SMS_TYPE, EMAIL_TYPE, PRIORITY from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, @@ -11,20 +11,17 @@ from app.notifications.process_notifications import ( from app.notifications.validators import ( validate_and_format_recipient, check_rate_limiting, - service_has_permission, check_service_can_schedule_notification, check_service_has_permission, validate_template ) from app.schema_validation import validate -from app.utils import get_public_notify_type_text from app.v2.notifications import v2_notification_blueprint from app.v2.notifications.notification_schemas import ( post_sms_request, create_post_sms_response_from_notification, post_email_request, create_post_email_response_from_notification) -from app.v2.errors import BadRequestError @v2_notification_blueprint.route('/', methods=['POST']) @@ -87,7 +84,6 @@ def post_notification(notification_type): body=str(template_with_content), from_number=sms_sender, url_root=request.url_root, - service_id=authenticated_service.id, scheduled_for=scheduled_for) else: resp = create_post_email_response_from_notification(notification=notification, @@ -95,6 +91,5 @@ def post_notification(notification_type): subject=template_with_content.subject, email_from=authenticated_service.email_from, url_root=request.url_root, - service_id=authenticated_service.id, scheduled_for=scheduled_for) return jsonify(resp), 201 diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py new file mode 100644 index 000000000..394c6bc92 --- /dev/null +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -0,0 +1,158 @@ +import uuid + +from flask import url_for, json +import pytest + +from app.models import Job, Notification, SMS_TYPE, EMAIL_TYPE, LETTER_TYPE +from app.v2.errors import RateLimitError + +from tests import create_authorization_header +from tests.app.db import create_service, create_template + + +def letter_request(client, data, service_id, _expected_status=201): + resp = client.post( + url_for('v2_notifications.post_notification', notification_type='letter'), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service_id)] + ) + assert resp.status_code == _expected_status + json_resp = json.loads(resp.get_data(as_text=True)) + return json_resp + + +@pytest.mark.parametrize('reference', [None, 'reference_from_client']) +def test_post_letter_notification_returns_201(client, sample_letter_template, mocker, reference): + mocked = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') + data = { + 'template_id': str(sample_letter_template.id), + 'personalisation': { + 'address_line_1': 'Her Royal Highness Queen Elizabeth II', + 'address_line_2': 'Buckingham Palace', + 'address_line_3': 'London', + 'postcode': 'SW1 1AA', + 'name': 'Lizzie' + } + } + + if reference: + data.update({'reference': reference}) + + resp_json = letter_request(client, data, service_id=sample_letter_template.service_id) + + job = Job.query.one() + notification = Notification.query.all() + notification_id = notification.id + assert resp_json['id'] == str(notification_id) + assert resp_json['reference'] == reference + assert resp_json['content']['subject'] == sample_letter_template.subject + assert resp_json['content']['body'] == sample_letter_template.content + assert 'v2/notifications/{}'.format(notification_id) in resp_json['uri'] + assert resp_json['template']['id'] == str(sample_letter_template.id) + assert resp_json['template']['version'] == sample_letter_template.version + assert ( + 'services/{}/templates/{}'.format( + sample_letter_template.service_id, + sample_letter_template.id + ) in resp_json['template']['uri'] + ) + assert not resp_json['scheduled_for'] + + mocked.assert_called_once_with((str(job.id), ), queue='job-tasks') + + +def test_post_letter_notification_returns_400_and_missing_template( + client, + sample_service +): + data = { + 'template_id': str(uuid.uuid4()), + 'personalisation': {'address_line_1': '', 'postcode': ''} + } + + error_json = letter_request(client, data, service_id=sample_service.id, _expected_status=400) + + assert error_json['status_code'] == 400 + assert error_json['errors'] == [{'error': 'BadRequestError', 'message': 'Template not found'}] + + + +def test_post_notification_returns_403_and_well_formed_auth_error( + client, + sample_letter_template +): + data = { + 'template_id': str(sample_letter_template.id), + 'personalisation': {'address_line_1': '', 'postcode': ''} + } + + error_json = letter_request(client, data, service_id=sample_letter_template.service_id, _expected_status=401) + + assert error_json['status_code'] == 401 + assert error_json['errors'] == [{ + 'error': 'AuthError', + 'message': 'Unauthorized, authentication token must be provided' + }] + + +def test_notification_returns_400_for_schema_problems( + client, + sample_service +): + data = { + 'personalisation': {'address_line_1': '', 'postcode': ''} + } + + error_json = letter_request(client, data, service_id=sample_service.id, _expected_status=400) + + assert error_json['status_code'] == 400 + assert error_json['errors'] == [{ + 'error': 'ValidationError', + 'message': 'template_id is a required property' + }] + + +def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded( + client, + sample_letter_template, + mocker +): + persist_mock = mocker.patch('app.v2.notifications.post_notifications.persist_notification') + mocker.patch( + 'app.v2.notifications.post_notifications.check_rate_limiting', + side_effect=RateLimitError('LIMIT', 'INTERVAL', 'TYPE') + ) + + data = { + 'template_id': str(sample_letter_template.id), + 'personalisation': {'address_line_1': '', 'postcode': ''} + } + + error_json = letter_request(client, data, service_id=sample_letter_template.service_id, _expected_status=429) + + assert error_json['status_code'] == 429 + assert error_json['errors'] == [{ + 'error': 'RateLimitError', + 'message': 'Exceeded rate limit for key type TYPE of LIMIT requests per INTERVAL seconds' + }] + + assert not persist_mock.called + + +def test_post_letter_notification_returns_400_if_not_allowed_to_send_notification( + client, + notify_db_session +): + service = create_service(service_permissions=[EMAIL_TYPE, SMS_TYPE]) + template = create_template(service, template_type=LETTER_TYPE) + + data = { + 'template_id': str(template.id), + 'personalisation': {'address_line_1': '', 'postcode': ''} + } + + error_json = letter_request(client, data, service_id=service.id, _expected_status=400) + assert error_json['status_code'] == 400 + assert error_json['errors'] == [ + {'error': 'BadRequestError', 'message': 'Cannot send text letters'} + ] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index aea003b7e..8a180adff 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -58,8 +58,8 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol @pytest.mark.parametrize("notification_type, key_send_to, send_to", [("sms", "phone_number", "+447700900855"), ("email", "email_address", "sample@email.com")]) -def test_post_sms_notification_returns_400_and_missing_template(client, sample_service, - notification_type, key_send_to, send_to): +def test_post_notification_returns_400_and_missing_template(client, sample_service, + notification_type, key_send_to, send_to): data = { key_send_to: send_to, 'template_id': str(uuid.uuid4()) From 2be194d9cee1061b9604d8141f2611e8facb8fc2 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 7 Jul 2017 17:10:16 +0100 Subject: [PATCH 06/34] refactor post_notification to separate sms/email and letter flows --- app/v2/notifications/notification_schemas.py | 4 +- app/v2/notifications/post_notifications.py | 125 +++++++++++++------ 2 files changed, 91 insertions(+), 38 deletions(-) diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 0f7081e00..fbd8fba53 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -234,11 +234,11 @@ post_letter_response = { } -def create_post_sms_response_from_notification(notification, body, from_number, url_root, scheduled_for): +def create_post_sms_response_from_notification(notification, content, from_number, url_root, scheduled_for): noti = __create_notification_response(notification, url_root, scheduled_for) noti['content'] = { 'from_number': from_number, - 'body': body + 'body': content } return noti diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 9ccbdb70a..ea02f3c7f 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -1,8 +1,10 @@ +import functools + from flask import request, jsonify, current_app from app import api_user, authenticated_service from app.config import QueueNames -from app.models import SMS_TYPE, EMAIL_TYPE, PRIORITY +from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, PRIORITY from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, @@ -16,20 +18,28 @@ from app.notifications.validators import ( validate_template ) from app.schema_validation import validate +from app.v2.errors import BadRequestError from app.v2.notifications import v2_notification_blueprint from app.v2.notifications.notification_schemas import ( post_sms_request, - create_post_sms_response_from_notification, post_email_request, - create_post_email_response_from_notification) + post_letter_request, + create_post_sms_response_from_notification, + create_post_email_response_from_notification, + create_post_letter_response_from_notification +) @v2_notification_blueprint.route('/', methods=['POST']) def post_notification(notification_type): if notification_type == EMAIL_TYPE: form = validate(request.get_json(), post_email_request) - else: + elif notification_type == SMS_TYPE: form = validate(request.get_json(), post_sms_request) + elif notification_type == LETTER_TYPE: + form = validate(request.get_json(), post_letter_request) + else: + raise BadRequestError(message='Unknown notification type {}'.format(notification_type)) check_service_has_permission(notification_type, authenticated_service.permissions) @@ -38,12 +48,6 @@ def post_notification(notification_type): check_rate_limiting(authenticated_service, api_user) - form_send_to = form['phone_number'] if notification_type == SMS_TYPE else form['email_address'] - send_to = validate_and_format_recipient(send_to=form_send_to, - key_type=api_user.key_type, - service=authenticated_service, - notification_type=notification_type) - template, template_with_content = validate_template( form['template_id'], form.get('personalisation', {}), @@ -51,20 +55,74 @@ def post_notification(notification_type): notification_type, ) + if notification_type == LETTER_TYPE: + notification = process_letter_notification( + form=form, + api_key=api_user, + template=template, + service=authenticated_service, + ) + else: + notification = process_sms_or_email_notification( + form=form, + notification_type=notification_type, + api_key=api_user, + template=template, + service=authenticated_service + ) + + if notification_type == SMS_TYPE: + sms_sender = authenticated_service.sms_sender or current_app.config.get('FROM_NUMBER') + create_resp_partial = functools.partial( + create_post_sms_response_from_notification, + from_number=sms_sender + ) + elif notification_type == EMAIL_TYPE: + create_resp_partial = functools.partial( + create_post_email_response_from_notification, + subject=template_with_content.subject, + email_from=authenticated_service.email_from + ) + elif notification_type == LETTER_TYPE: + create_resp_partial = functools.partial( + create_post_letter_response_from_notification, + subject=template_with_content.subject, + ) + + resp = create_resp_partial( + notification=notification, + content=str(template_with_content), + url_root=request.url_root, + scheduled_for=scheduled_for + ) + return jsonify(resp), 201 + + +def process_sms_or_email_notification(*, form, notification_type, api_key, template, service): + form_send_to = form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number'] + + send_to = validate_and_format_recipient(send_to=form_send_to, + key_type=api_key.key_type, + service=service, + notification_type=notification_type) + # Do not persist or send notification to the queue if it is a simulated recipient simulated = simulated_recipient(send_to, notification_type) - notification = persist_notification(template_id=template.id, - template_version=template.version, - recipient=form_send_to, - service=authenticated_service, - personalisation=form.get('personalisation', None), - notification_type=notification_type, - api_key_id=api_user.id, - key_type=api_user.key_type, - client_reference=form.get('reference', None), - simulated=simulated) + notification = persist_notification( + template_id=template.id, + template_version=template.version, + recipient=form_send_to, + service=service, + personalisation=form.get('personalisation', None), + notification_type=notification_type, + api_key_id=api_key.id, + key_type=api_key.key_type, + client_reference=form.get('reference', None), + simulated=simulated + ) + scheduled_for = form.get("scheduled_for", None) if scheduled_for: persist_scheduled_notification(notification.id, form["scheduled_for"]) else: @@ -72,24 +130,19 @@ def post_notification(notification_type): queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None send_notification_to_queue( notification=notification, - research_mode=authenticated_service.research_mode, + research_mode=service.research_mode, queue=queue_name ) else: current_app.logger.info("POST simulated notification for id: {}".format(notification.id)) - if notification_type == SMS_TYPE: - sms_sender = authenticated_service.sms_sender or current_app.config.get('FROM_NUMBER') - resp = create_post_sms_response_from_notification(notification=notification, - body=str(template_with_content), - from_number=sms_sender, - url_root=request.url_root, - scheduled_for=scheduled_for) - else: - resp = create_post_email_response_from_notification(notification=notification, - content=str(template_with_content), - subject=template_with_content.subject, - email_from=authenticated_service.email_from, - url_root=request.url_root, - scheduled_for=scheduled_for) - return jsonify(resp), 201 + return notification + + +def process_letter_notification(*, form, api_key, template, service): + # create job + + # create notification + + # trigger build_dvla_file task + raise NotImplementedError From 9caf45451e388e254b74777044676d9d3c4b24b6 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 7 Jul 2017 17:10:25 +0100 Subject: [PATCH 07/34] make persist_notification require kwargs when functions get as big as that, it's confusing to try and work out what things are what. By including a * as the first arg, we require that anyone calling the function has to use kwargs to reference the parameters --- app/notifications/process_notifications.py | 1 + app/notifications/validators.py | 4 +- .../test_process_notification.py | 59 +++++++++++++------ .../test_post_letter_notifications.py | 17 ++++-- .../notifications/test_post_notifications.py | 14 +++++ 5 files changed, 70 insertions(+), 25 deletions(-) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index ca3ae0b53..ee03b537e 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -35,6 +35,7 @@ def check_placeholders(template_object): def persist_notification( + *, template_id, template_version, recipient, diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 3aa86c963..83b620506 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -9,7 +9,7 @@ from notifications_utils.clients.redis import rate_limit_cache_key, daily_limit_ from app.dao import services_dao, templates_dao from app.models import ( - INTERNATIONAL_SMS_TYPE, SMS_TYPE, + INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE, KEY_TYPE_TEST, KEY_TYPE_TEAM, SCHEDULE_NOTIFICATIONS ) from app.service.utils import service_allowed_to_send_to @@ -104,7 +104,7 @@ def validate_and_format_recipient(send_to, key_type, service, notification_type) number=send_to, international=international_phone_info.international ) - else: + elif notification_type == EMAIL_TYPE: return validate_and_format_email_address(email_address=send_to) diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index ac73b36da..407ad2093 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -51,10 +51,18 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api assert Notification.query.count() == 0 assert NotificationHistory.query.count() == 0 - notification = persist_notification(sample_template.id, sample_template.version, '+447111111111', - sample_template.service, {}, 'sms', sample_api_key.id, - sample_api_key.key_type, job_id=sample_job.id, - job_row_number=100, reference="ref") + notification = persist_notification( + template_id=sample_template.id, + template_version=sample_template.version, + recipient='+447111111111', + service=sample_template.service, + personalisation={}, + notification_type='sms', + api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type, + job_id=sample_job.id, + job_row_number=100, + reference="ref") assert Notification.query.get(notification.id) is not None assert NotificationHistory.query.get(notification.id) is not None @@ -127,14 +135,14 @@ def test_persist_notification_does_not_increment_cache_if_test_key( assert Notification.query.count() == 0 assert NotificationHistory.query.count() == 0 persist_notification( - sample_template.id, - sample_template.version, - '+447111111111', - sample_template.service, - {}, - 'sms', - api_key.id, - api_key.key_type, + template_id=sample_template.id, + template_version=sample_template.version, + recipient='+447111111111', + service=sample_template.service, + personalisation={}, + notification_type='sms', + api_key_id=api_key.id, + key_type=api_key.key_type, job_id=sample_job.id, job_row_number=100, reference="ref", @@ -193,18 +201,33 @@ def test_persist_notification_increments_cache_if_key_exists(sample_template, sa mock_incr = mocker.patch('app.notifications.process_notifications.redis_store.incr') mock_incr_hash_value = mocker.patch('app.notifications.process_notifications.redis_store.increment_hash_value') - persist_notification(sample_template.id, sample_template.version, '+447111111111', - sample_template.service, {}, 'sms', sample_api_key.id, - sample_api_key.key_type, reference="ref") + persist_notification( + template_id=sample_template.id, + template_version=sample_template.version, + recipient='+447111111111', + service=sample_template.service, + personalisation={}, + notification_type='sms', + api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type, + reference="ref" + ) mock_incr.assert_not_called() mock_incr_hash_value.assert_not_called() mocker.patch('app.notifications.process_notifications.redis_store.get', return_value=1) mocker.patch('app.notifications.process_notifications.redis_store.get_all_from_hash', return_value={sample_template.id, 1}) - persist_notification(sample_template.id, sample_template.version, '+447111111122', - sample_template.service, {}, 'sms', sample_api_key.id, - sample_api_key.key_type, reference="ref2") + persist_notification( + template_id=sample_template.id, + template_version=sample_template.version, + recipient='+447111111122', + service=sample_template.service, + personalisation={}, + notification_type='sms', + api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type, + reference="ref2") mock_incr.assert_called_once_with(str(sample_template.service_id) + "-2016-01-01-count", ) mock_incr_hash_value.assert_called_once_with(cache_key_for_service_template_counter(sample_template.service_id), sample_template.id) diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 394c6bc92..7c8f2b34d 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -10,14 +10,17 @@ from tests import create_authorization_header from tests.app.db import create_service, create_template +pytestmark = pytest.mark.skip('Leters not currently implemented') + + def letter_request(client, data, service_id, _expected_status=201): resp = client.post( url_for('v2_notifications.post_notification', notification_type='letter'), data=json.dumps(data), headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service_id)] ) - assert resp.status_code == _expected_status json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == _expected_status, json_resp return json_resp @@ -76,7 +79,6 @@ def test_post_letter_notification_returns_400_and_missing_template( assert error_json['errors'] == [{'error': 'BadRequestError', 'message': 'Template not found'}] - def test_post_notification_returns_403_and_well_formed_auth_error( client, sample_letter_template @@ -139,11 +141,16 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded( assert not persist_mock.called +@pytest.mark.parametrize('service_args', [ + {'service_permissions': [EMAIL_TYPE, SMS_TYPE]}, + {'restricted': True} +]) def test_post_letter_notification_returns_400_if_not_allowed_to_send_notification( client, - notify_db_session + notify_db_session, + service_args ): - service = create_service(service_permissions=[EMAIL_TYPE, SMS_TYPE]) + service = create_service(**service_args) template = create_template(service, template_type=LETTER_TYPE) data = { @@ -154,5 +161,5 @@ def test_post_letter_notification_returns_400_if_not_allowed_to_send_notificatio error_json = letter_request(client, data, service_id=service.id, _expected_status=400) assert error_json['status_code'] == 400 assert error_json['errors'] == [ - {'error': 'BadRequestError', 'message': 'Cannot send text letters'} + {'error': 'BadRequestError', 'message': 'Cannot send letters'} ] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 8a180adff..597cdf1b7 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -433,3 +433,17 @@ def test_post_notification_raises_bad_request_if_service_not_invited_to_schedule error_json = json.loads(response.get_data(as_text=True)) assert error_json['errors'] == [ {"error": "BadRequestError", "message": 'Cannot schedule notifications (this feature is invite-only)'}] + + +def test_post_notification_raises_bad_request_if_not_valid_notification_type(client, sample_service): + auth_header = create_authorization_header(service_id=sample_service.id) + response = client.post( + '/v2/notifications/foo', + data='{}', + headers=[('Content-Type', 'application/json'), auth_header] + ) + assert response.status_code == 400 + error_json = json.loads(response.get_data(as_text=True)) + assert error_json['errors'] == [ + {'error': 'BadRequestError', 'message': 'Unknown notification type foo'} + ] From 79e33073c9bf90bf2dbd89ade8a037673bf5920a Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 20 Jul 2017 15:23:46 +0100 Subject: [PATCH 08/34] raise 404 when unknown url consistent with other endpoints. also refactor of notification_schema to separate some fns to a different file --- app/v2/notifications/create_response.py | 45 ++++++++++++++++++ app/v2/notifications/notification_schemas.py | 46 ------------------- app/v2/notifications/post_notifications.py | 9 ++-- .../test_post_letter_notifications.py | 5 +- .../notifications/test_post_notifications.py | 6 +-- 5 files changed, 55 insertions(+), 56 deletions(-) create mode 100644 app/v2/notifications/create_response.py diff --git a/app/v2/notifications/create_response.py b/app/v2/notifications/create_response.py new file mode 100644 index 000000000..7eecfb1b9 --- /dev/null +++ b/app/v2/notifications/create_response.py @@ -0,0 +1,45 @@ + +def create_post_sms_response_from_notification(notification, content, from_number, url_root, scheduled_for): + noti = __create_notification_response(notification, url_root, scheduled_for) + noti['content'] = { + 'from_number': from_number, + 'body': content + } + return noti + + +def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, scheduled_for): + noti = __create_notification_response(notification, url_root, scheduled_for) + noti['content'] = { + "from_email": email_from, + "body": content, + "subject": subject + } + return noti + + +def create_post_letter_response_from_notification(notification, content, subject, url_root, scheduled_for): + noti = __create_notification_response(notification, url_root, scheduled_for) + noti['content'] = { + "body": content, + "subject": subject + } + return noti + + +def __create_notification_response(notification, url_root, scheduled_for): + return { + "id": notification.id, + "reference": notification.client_reference, + "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)), + 'template': { + "id": notification.template_id, + "version": notification.template_version, + "uri": "{}services/{}/templates/{}".format( + url_root, + str(notification.service_id), + str(notification.template_id) + ) + }, + "scheduled_for": scheduled_for if scheduled_for else None + } diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index fbd8fba53..7f61f0ee0 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -232,49 +232,3 @@ post_letter_response = { }, "required": ["id", "content", "uri", "template"] } - - -def create_post_sms_response_from_notification(notification, content, from_number, url_root, scheduled_for): - noti = __create_notification_response(notification, url_root, scheduled_for) - noti['content'] = { - 'from_number': from_number, - 'body': content - } - return noti - - -def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, scheduled_for): - noti = __create_notification_response(notification, url_root, scheduled_for) - noti['content'] = { - "from_email": email_from, - "body": content, - "subject": subject - } - return noti - - -def create_post_letter_response_from_notification(notification, content, subject, url_root, scheduled_for): - noti = __create_notification_response(notification, url_root, scheduled_for) - noti['content'] = { - "body": content, - "subject": subject - } - return noti - - -def __create_notification_response(notification, url_root, scheduled_for): - return { - "id": notification.id, - "reference": notification.client_reference, - "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)), - 'template': { - "id": notification.template_id, - "version": notification.template_version, - "uri": "{}services/{}/templates/{}".format( - url_root, - str(notification.service_id), - str(notification.template_id) - ) - }, - "scheduled_for": scheduled_for if scheduled_for else None - } diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index ea02f3c7f..f39f54345 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -1,6 +1,6 @@ import functools -from flask import request, jsonify, current_app +from flask import request, jsonify, current_app, abort from app import api_user, authenticated_service from app.config import QueueNames @@ -18,12 +18,13 @@ from app.notifications.validators import ( validate_template ) from app.schema_validation import validate -from app.v2.errors import BadRequestError from app.v2.notifications import v2_notification_blueprint from app.v2.notifications.notification_schemas import ( post_sms_request, post_email_request, - post_letter_request, + post_letter_request +) +from app.v2.notifications.create_response import ( create_post_sms_response_from_notification, create_post_email_response_from_notification, create_post_letter_response_from_notification @@ -39,7 +40,7 @@ def post_notification(notification_type): elif notification_type == LETTER_TYPE: form = validate(request.get_json(), post_letter_request) else: - raise BadRequestError(message='Unknown notification type {}'.format(notification_type)) + abort(404) check_service_has_permission(notification_type, authenticated_service.permissions) diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 7c8f2b34d..d65c6c5d8 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -1,3 +1,4 @@ + import uuid from flask import url_for, json @@ -145,7 +146,7 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded( {'service_permissions': [EMAIL_TYPE, SMS_TYPE]}, {'restricted': True} ]) -def test_post_letter_notification_returns_400_if_not_allowed_to_send_notification( +def test_post_letter_notification_returns_403_if_not_allowed_to_send_notification( client, notify_db_session, service_args @@ -159,7 +160,7 @@ def test_post_letter_notification_returns_400_if_not_allowed_to_send_notificatio } error_json = letter_request(client, data, service_id=service.id, _expected_status=400) - assert error_json['status_code'] == 400 + assert error_json['status_code'] == 403 assert error_json['errors'] == [ {'error': 'BadRequestError', 'message': 'Cannot send letters'} ] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 597cdf1b7..40475e030 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -442,8 +442,6 @@ def test_post_notification_raises_bad_request_if_not_valid_notification_type(cli data='{}', headers=[('Content-Type', 'application/json'), auth_header] ) - assert response.status_code == 400 + assert response.status_code == 404 error_json = json.loads(response.get_data(as_text=True)) - assert error_json['errors'] == [ - {'error': 'BadRequestError', 'message': 'Unknown notification type foo'} - ] + assert 'The requested URL was not found on the server.' in error_json['message'] From 6da3d3ed0b0447285e3aa8d3256ed80d0d0b6da1 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 21 Jul 2017 14:26:59 +0100 Subject: [PATCH 09/34] Remove wheels-ing on deployment --- .gitignore | 2 -- Makefile | 5 ++--- scripts/aws_install_dependencies.sh | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 12f77b15c..ffd54699f 100644 --- a/.gitignore +++ b/.gitignore @@ -69,7 +69,5 @@ environment.sh celerybeat-schedule -wheelhouse/ - # CloudFoundry .cf diff --git a/Makefile b/Makefile index 4142722ff..d8c650d5f 100644 --- a/Makefile +++ b/Makefile @@ -80,8 +80,7 @@ generate-version-file: ## Generates the app version file .PHONY: build build: dependencies generate-version-file ## Build project - rm -rf wheelhouse - . venv/bin/activate && PIP_ACCEL_CACHE=${PIP_ACCEL_CACHE} pip-accel wheel --wheel-dir=wheelhouse -r requirements.txt + . venv/bin/activate && PIP_ACCEL_CACHE=${PIP_ACCEL_CACHE} pip-accel install -r requirements.txt .PHONY: cf-build cf-build: dependencies generate-version-file ## Build project for PAAS @@ -260,7 +259,7 @@ clean-docker-containers: ## Clean up any remaining docker containers .PHONY: clean clean: - rm -rf node_modules cache target venv .coverage build tests/.cache wheelhouse + rm -rf node_modules cache target venv .coverage build tests/.cache .PHONY: cf-login cf-login: ## Log in to Cloud Foundry diff --git a/scripts/aws_install_dependencies.sh b/scripts/aws_install_dependencies.sh index 03daadb21..b82881d45 100755 --- a/scripts/aws_install_dependencies.sh +++ b/scripts/aws_install_dependencies.sh @@ -5,4 +5,4 @@ set -eo pipefail echo "Install dependencies" cd /home/notify-app/notifications-api; -pip3 install --find-links=wheelhouse -r /home/notify-app/notifications-api/requirements.txt +pip3 install -r /home/notify-app/notifications-api/requirements.txt From 3e2b8190b9262734f0849e33ff778f69be034e6e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 24 Jul 2017 15:13:18 +0100 Subject: [PATCH 10/34] - Added a scheduled task to create or update billing for the month, yesterday is used to calculate the start and end date for the month. - The new task has not been added to the beat application yet. - Added an updated_at column to the monthly billing table, we may want to only calculate from the last updated date rather than the entire month. --- app/celery/scheduled_tasks.py | 21 ++++++++++- app/dao/monthly_billing_dao.py | 17 +++++++-- app/dao/notification_usage_dao.py | 3 +- app/models.py | 1 + migrations/versions/0110_monthly_billing.py | 1 + tests/app/celery/test_scheduled_tasks.py | 39 ++++++++++++++++++--- tests/app/dao/test_monthly_billing.py | 23 ++++++++++-- 7 files changed, 93 insertions(+), 12 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index b14f7a5be..ceb813e25 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -9,9 +9,17 @@ from sqlalchemy.exc import SQLAlchemyError from app.aws import s3 from app import notify_celery from app import performance_platform_client +from app.dao.date_util import get_month_start_end_date from app.dao.inbound_sms_dao import delete_inbound_sms_created_more_than_a_week_ago from app.dao.invited_user_dao import delete_invitations_created_more_than_two_days_ago -from app.dao.jobs_dao import dao_set_scheduled_jobs_to_pending, dao_get_jobs_older_than_limited_by +from app.dao.jobs_dao import ( + dao_set_scheduled_jobs_to_pending, + dao_get_jobs_older_than_limited_by +) +from app.dao.monthly_billing_dao import ( + get_service_ids_that_need_sms_billing_populated, + create_or_update_monthly_billing_sms +) from app.dao.notifications_dao import ( dao_timeout_notifications, is_delivery_slow_for_provider, @@ -281,3 +289,14 @@ def delete_dvla_response_files_older_than_seven_days(): except SQLAlchemyError as e: current_app.logger.exception("Failed to delete dvla response files") raise + + +@notify_celery.task(name="populate_monthly_billing") +@statsd(namespace="tasks") +def populate_monthly_billing(): + # for every service with billable units this month update billing totals for yesterday + # this will overwrite the existing amount. + yesterday = datetime.utcnow() - timedelta(days=1) + start_date, end_date = get_month_start_end_date(yesterday) + services = get_service_ids_that_need_sms_billing_populated(start_date, end_date=end_date) + [create_or_update_monthly_billing_sms(service_id=s.service_id, billing_month=start_date) for s in services] diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index 8e3204584..4dede3f2d 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -1,12 +1,25 @@ from datetime import datetime from app import db +from app.dao.date_util import get_month_start_end_date from app.dao.notification_usage_dao import get_billing_data_for_month -from app.models import MonthlyBilling, SMS_TYPE +from app.models import MonthlyBilling, SMS_TYPE, NotificationHistory + + +def get_service_ids_that_need_sms_billing_populated(start_date, end_date): + return db.session.query( + NotificationHistory.service_id + ).filter( + NotificationHistory.created_at >= start_date, + NotificationHistory.created_at <= end_date, + NotificationHistory.notification_type == SMS_TYPE, + NotificationHistory.billable_units != 0 + ).distinct().all() def create_or_update_monthly_billing_sms(service_id, billing_month): - monthly = get_billing_data_for_month(service_id=service_id, billing_month=billing_month) + start_date, end_date = get_month_start_end_date(billing_month) + monthly = get_billing_data_for_month(service_id=service_id, start_date=start_date, end_date=end_date) # update monthly monthly_totals = _monthly_billing_data_to_json(monthly) row = MonthlyBilling.query.filter_by(year=billing_month.year, diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 289a5a03d..13b8944c6 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -36,8 +36,7 @@ def get_yearly_billing_data(service_id, year): @statsd(namespace="dao") -def get_billing_data_for_month(service_id, billing_month): - start_date, end_date = get_month_start_end_date(billing_month) +def get_billing_data_for_month(service_id, start_date, end_date): rates = get_rates_for_year(start_date, end_date, SMS_TYPE) result = [] # so the start end date in the query are the valid from the rate, not the month - this is going to take some thought diff --git a/app/models.py b/app/models.py index e035acccb..b58eaf348 100644 --- a/app/models.py +++ b/app/models.py @@ -1257,6 +1257,7 @@ class MonthlyBilling(db.Model): year = db.Column(db.Float(asdecimal=False), nullable=False) notification_type = db.Column(notification_types, nullable=False) monthly_totals = db.Column(JSON, nullable=False) + updated_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) __table_args__ = ( UniqueConstraint('service_id', 'month', 'year', 'notification_type', name='uix_monthly_billing'), diff --git a/migrations/versions/0110_monthly_billing.py b/migrations/versions/0110_monthly_billing.py index 19b1ce4fc..19fa1dbdd 100644 --- a/migrations/versions/0110_monthly_billing.py +++ b/migrations/versions/0110_monthly_billing.py @@ -26,6 +26,7 @@ def upgrade(): postgresql.ENUM('email', 'sms', 'letter', name='notification_type', create_type=False), nullable=False), sa.Column('monthly_totals', postgresql.JSON(), nullable=False), + sa.Column('updated_at', sa.DateTime, nullable=False), sa.ForeignKeyConstraint(['service_id'], ['services.id'], ), sa.PrimaryKeyConstraint('id') ) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index b4ad7e255..0f54f9dde 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -25,8 +25,8 @@ from app.celery.scheduled_tasks import ( send_scheduled_notifications, switch_current_sms_provider_on_slow_delivery, timeout_job_statistics, - timeout_notifications -) + timeout_notifications, + populate_monthly_billing) from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient from app.dao.jobs_dao import dao_get_job_by_id from app.dao.notifications_dao import dao_get_scheduled_notifications @@ -36,10 +36,10 @@ from app.dao.provider_details_dao import ( ) from app.models import ( Service, Template, - SMS_TYPE, LETTER_TYPE -) + SMS_TYPE, LETTER_TYPE, + MonthlyBilling) from app.utils import get_london_midnight_in_utc -from tests.app.db import create_notification, create_service, create_template, create_job +from tests.app.db import create_notification, create_service, create_template, create_job, create_rate from tests.app.conftest import ( sample_job as create_sample_job, sample_notification_history as create_notification_history, @@ -98,6 +98,8 @@ def test_should_have_decorated_tasks_functions(): 'remove_transformed_dvla_files' assert delete_dvla_response_files_older_than_seven_days.__wrapped__.__name__ == \ 'delete_dvla_response_files_older_than_seven_days' + assert populate_monthly_billing.__wrapped__.__name__ == \ + 'populate_monthly_billing' @pytest.fixture(scope='function') @@ -607,3 +609,30 @@ def test_delete_dvla_response_files_older_than_seven_days_does_not_remove_files( delete_dvla_response_files_older_than_seven_days() remove_s3_mock.assert_not_called() + + +@freeze_time("2017-07-12 02:00:00") +def test_populate_monthly_billing(sample_template): + yesterday = datetime(2017, 7, 11, 13, 30) + create_rate(datetime(2016, 1, 1), 0.0123, 'sms') + create_notification(template=sample_template, status='delivered', created_at=yesterday) + create_notification(template=sample_template, status='delivered', created_at=yesterday - timedelta(days=1)) + create_notification(template=sample_template, status='delivered', created_at=yesterday + timedelta(days=1)) + # not included in billing + create_notification(template=sample_template, status='delivered', created_at=yesterday - timedelta(days=30)) + + assert len(MonthlyBilling.query.all()) == 0 + populate_monthly_billing() + + monthly_billing = MonthlyBilling.query.all() + assert len(monthly_billing) == 1 + assert monthly_billing[0].service_id == sample_template.service_id + assert monthly_billing[0].year == 2017 + assert monthly_billing[0].month == 'July' + assert monthly_billing[0].notification_type == 'sms' + assert len(monthly_billing[0].monthly_totals) == 1 + assert sorted(monthly_billing[0].monthly_totals[0]) == sorted({'international': False, + 'rate_multiplier': 1, + 'billing_units': 3, + 'rate': 0.0123, + 'total_cost': 0.0369}) diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index 6a0399d67..2ed13aaa2 100644 --- a/tests/app/dao/test_monthly_billing.py +++ b/tests/app/dao/test_monthly_billing.py @@ -1,8 +1,12 @@ from datetime import datetime -from app.dao.monthly_billing_dao import create_or_update_monthly_billing_sms, get_monthly_billing_sms +from app.dao.monthly_billing_dao import ( + create_or_update_monthly_billing_sms, + get_monthly_billing_sms, + get_service_ids_that_need_sms_billing_populated +) from app.models import MonthlyBilling -from tests.app.db import create_notification, create_rate +from tests.app.db import create_notification, create_rate, create_service, create_template def test_add_monthly_billing(sample_template): @@ -105,3 +109,18 @@ def assert_monthly_billing(monthly_billing, year, month, service_id, expected_le assert monthly_billing.service_id == service_id assert len(monthly_billing.monthly_totals) == expected_len assert sorted(monthly_billing.monthly_totals[0]) == sorted(first_row) + + +def test_get_service_id(): + service_1 = create_service(service_name="Service One") + template_1 = create_template(service=service_1) + service_2 = create_service(service_name="Service Two") + template_2 = create_template(service=service_2) + create_notification(template=template_1, created_at=datetime(2017, 6, 30, 13, 30), status='delivered') + create_notification(template=template_1, created_at=datetime(2017, 7, 1, 14, 30), status='delivered') + create_notification(template=template_2, created_at=datetime(2017, 7, 15, 13, 30)) + create_notification(template=template_2, created_at=datetime(2017, 7, 31, 13, 30)) + services = get_service_ids_that_need_sms_billing_populated(start_date=datetime(2017, 7, 1), + end_date=datetime(2017, 7, 16)) + expected_services = [service_1.id, service_2.id] + assert sorted([x.service_id for x in services]) == sorted(expected_services) From cc32cff32a8e2da10cf6e2d9338d7133165cb637 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 25 Jul 2017 11:40:12 +0100 Subject: [PATCH 11/34] bump test requirements also ignore celery improvements --- requirements.txt | 2 +- requirements_for_test.txt | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/requirements.txt b/requirements.txt index 875e27a94..0c9b3f1e0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -13,7 +13,7 @@ flask-marshmallow==0.6.2 Flask-Bcrypt==0.6.2 credstash==1.8.0 boto3==1.4.4 -celery==3.1.25 +celery==3.1.25 # pyup: <4 monotonic==1.2 statsd==3.2.1 jsonschema==2.5.1 diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 62e3e4049..78d2d58ae 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -1,11 +1,11 @@ -r requirements.txt pycodestyle==2.3.1 -pytest==3.0.1 -pytest-mock==1.2 -pytest-cov==2.3.1 +pytest==3.1.3 +pytest-mock==1.6.2 +pytest-cov==2.5.1 coveralls==1.1 -moto==0.4.25 -flex==5.8.0 -freezegun==0.3.7 -requests-mock==1.0.0 +moto==1.0.1 +flex==6.11.0 +freezegun==0.3.9 +requests-mock==1.3.0 strict-rfc3339==0.7 From eaf5cbb86876c417e6dec4d6b76e24f94cbe71bd Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Jul 2017 11:43:41 +0100 Subject: [PATCH 12/34] Add labels to query so that the named tuples can be referenced later. Remove unnecessary function --- app/dao/monthly_billing_dao.py | 10 +++++----- app/dao/notification_usage_dao.py | 8 ++++---- app/dao/provider_rates_dao.py | 5 ----- tests/app/dao/test_monthly_billing.py | 2 +- tests/app/dao/test_provider_rates_dao.py | 12 ++---------- tests/app/db.py | 5 +++-- 6 files changed, 15 insertions(+), 27 deletions(-) diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index 4dede3f2d..6cc499001 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -50,8 +50,8 @@ def _monthly_billing_data_to_json(monthly): # (month, billing_units, rate_multiplier, international, notification_type, rate) # total cost must take into account the free allowance. # might be a good idea to capture free allowance in this table - return [{"billing_units": x[1], - "rate_multiplier": x[2], - "international": x[3], - "rate": x[5], - "total_cost": (x[1] * x[2]) * x[5]} for x in monthly] + return [{"billing_units": x.billing_units, + "rate_multiplier": x.rate_multiplier, + "international": x.international, + "rate": x.rate, + "total_cost": (x.billing_units * x.rate_multiplier) * x.rate} for x in monthly] diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 13b8944c6..43fb6a6cd 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -142,12 +142,12 @@ def is_between(date, start_date, end_date): def sms_billing_data_per_month_query(rate, service_id, start_date, end_date): month = get_london_month_from_utc_column(NotificationHistory.created_at) result = db.session.query( - month, - func.sum(NotificationHistory.billable_units), - rate_multiplier(), + month.label('month'), + func.sum(NotificationHistory.billable_units).label('billing_units'), + rate_multiplier().label('rate_multiplier'), NotificationHistory.international, NotificationHistory.notification_type, - cast(rate, Float()) + cast(rate, Float()).label('rate') ).filter( *billing_data_filter(SMS_TYPE, start_date, end_date, service_id) ).group_by( diff --git a/app/dao/provider_rates_dao.py b/app/dao/provider_rates_dao.py index 443543f86..145bd431e 100644 --- a/app/dao/provider_rates_dao.py +++ b/app/dao/provider_rates_dao.py @@ -9,8 +9,3 @@ def create_provider_rates(provider_identifier, valid_from, rate): provider_rates = ProviderRates(provider_id=provider.id, valid_from=valid_from, rate=rate) db.session.add(provider_rates) - - -@transactional -def create_sms_rate(rate): - db.session.add(rate) diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index 2ed13aaa2..9027ff870 100644 --- a/tests/app/dao/test_monthly_billing.py +++ b/tests/app/dao/test_monthly_billing.py @@ -111,7 +111,7 @@ def assert_monthly_billing(monthly_billing, year, month, service_id, expected_le assert sorted(monthly_billing.monthly_totals[0]) == sorted(first_row) -def test_get_service_id(): +def test_get_service_id(notify_db_session): service_1 = create_service(service_name="Service One") template_1 = create_template(service=service_1) service_2 = create_service(service_name="Service Two") diff --git a/tests/app/dao/test_provider_rates_dao.py b/tests/app/dao/test_provider_rates_dao.py index 7edb7de43..c78290a90 100644 --- a/tests/app/dao/test_provider_rates_dao.py +++ b/tests/app/dao/test_provider_rates_dao.py @@ -1,8 +1,8 @@ import uuid from datetime import datetime from decimal import Decimal -from app.dao.provider_rates_dao import create_provider_rates, create_sms_rate -from app.models import ProviderRates, ProviderDetails, Rate +from app.dao.provider_rates_dao import create_provider_rates +from app.models import ProviderRates, ProviderDetails def test_create_provider_rates(notify_db, notify_db_session, mmg_provider): @@ -16,11 +16,3 @@ def test_create_provider_rates(notify_db, notify_db_session, mmg_provider): assert ProviderRates.query.first().rate == rate assert ProviderRates.query.first().valid_from == now assert ProviderRates.query.first().provider_id == provider.id - - -def test_create_sms_rate(): - rate = Rate(id=uuid.uuid4(), valid_from=datetime.now(), rate=0.014, notification_type='sms') - create_sms_rate(rate) - rates = Rate.query.all() - assert len(rates) == 1 - assert rates[0] == rate diff --git a/tests/app/db.py b/tests/app/db.py index 669803d92..ace7b7c1c 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1,8 +1,8 @@ from datetime import datetime import uuid +from app import db from app.dao.jobs_dao import dao_create_job -from app.dao.provider_rates_dao import create_sms_rate from app.dao.service_inbound_api_dao import save_service_inbound_api from app.models import ( Service, @@ -244,5 +244,6 @@ def create_organisation(colour='blue', logo='test_x2.png', name='test_org_1'): def create_rate(start_date, value, notification_type): rate = Rate(id=uuid.uuid4(), valid_from=start_date, rate=value, notification_type=notification_type) - create_sms_rate(rate) + db.session.add(rate) + db.session.commit() return rate From d2a1da9ea6cf134f51ef73280a0e09af9033e8f2 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Jul 2017 11:44:39 +0100 Subject: [PATCH 13/34] Removed comment --- app/dao/monthly_billing_dao.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index 6cc499001..cd51c19d3 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -46,8 +46,6 @@ def get_monthly_billing_sms(service_id, billing_month): def _monthly_billing_data_to_json(monthly): - # ('April', 6, 1, False, 'sms', 0.014) - # (month, billing_units, rate_multiplier, international, notification_type, rate) # total cost must take into account the free allowance. # might be a good idea to capture free allowance in this table return [{"billing_units": x.billing_units, From 5612ca023e094c8eecb9e42761d3c289ca5d85e7 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Jul 2017 14:26:42 +0100 Subject: [PATCH 14/34] - Add transactional - Rename function for clarity --- app/dao/monthly_billing_dao.py | 5 ++++- app/dao/notification_usage_dao.py | 10 +++++----- tests/app/dao/test_notification_usage_dao.py | 16 ++++++++-------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index cd51c19d3..49a571e5c 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -1,9 +1,11 @@ from datetime import datetime from app import db +from app.dao.dao_utils import transactional from app.dao.date_util import get_month_start_end_date from app.dao.notification_usage_dao import get_billing_data_for_month from app.models import MonthlyBilling, SMS_TYPE, NotificationHistory +from app.statsd_decorators import statsd def get_service_ids_that_need_sms_billing_populated(start_date, end_date): @@ -17,6 +19,7 @@ def get_service_ids_that_need_sms_billing_populated(start_date, end_date): ).distinct().all() +@transactional def create_or_update_monthly_billing_sms(service_id, billing_month): start_date, end_date = get_month_start_end_date(billing_month) monthly = get_billing_data_for_month(service_id=service_id, start_date=start_date, end_date=end_date) @@ -34,9 +37,9 @@ def create_or_update_monthly_billing_sms(service_id, billing_month): month=datetime.strftime(billing_month, "%B"), monthly_totals=monthly_totals) db.session.add(row) - db.session.commit() +@statsd(namespace="dao") def get_monthly_billing_sms(service_id, billing_month): monthly = MonthlyBilling.query.filter_by(service_id=service_id, year=billing_month.year, diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 43fb6a6cd..7cd6517e9 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -20,7 +20,7 @@ from app.utils import get_london_month_from_utc_column @statsd(namespace="dao") def get_yearly_billing_data(service_id, year): start_date, end_date = get_financial_year(year) - rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + rates = get_rates_for_daterange(start_date, end_date, SMS_TYPE) def get_valid_from(valid_from): return start_date if valid_from < start_date else valid_from @@ -37,7 +37,7 @@ def get_yearly_billing_data(service_id, year): @statsd(namespace="dao") def get_billing_data_for_month(service_id, start_date, end_date): - rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + rates = get_rates_for_daterange(start_date, end_date, SMS_TYPE) result = [] # so the start end date in the query are the valid from the rate, not the month - this is going to take some thought for r, n in zip(rates, rates[1:]): @@ -52,7 +52,7 @@ def get_billing_data_for_month(service_id, start_date, end_date): @statsd(namespace="dao") def get_monthly_billing_data(service_id, year): start_date, end_date = get_financial_year(year) - rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + rates = get_rates_for_daterange(start_date, end_date, SMS_TYPE) result = [] for r, n in zip(rates, rates[1:]): @@ -117,7 +117,7 @@ def sms_yearly_billing_data_query(rate, service_id, start_date, end_date): return result -def get_rates_for_year(start_date, end_date, notification_type): +def get_rates_for_daterange(start_date, end_date, notification_type): rates = Rate.query.filter(Rate.notification_type == notification_type).order_by(Rate.valid_from).all() results = [] for current_rate, current_rate_expiry_date in zip(rates, rates[1:]): @@ -207,7 +207,7 @@ def get_total_billable_units_for_sent_sms_notifications_in_date_range(start_date def discover_rate_bounds_for_billing_query(start_date, end_date): bounds = [] - rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + rates = get_rates_for_daterange(start_date, end_date, SMS_TYPE) def current_valid_from(index): return rates[index].valid_from diff --git a/tests/app/dao/test_notification_usage_dao.py b/tests/app/dao/test_notification_usage_dao.py index 83bfa264c..1555bad77 100644 --- a/tests/app/dao/test_notification_usage_dao.py +++ b/tests/app/dao/test_notification_usage_dao.py @@ -6,7 +6,7 @@ from flask import current_app from app.dao.date_util import get_financial_year from app.dao.notification_usage_dao import ( - get_rates_for_year, + get_rates_for_daterange, get_yearly_billing_data, get_monthly_billing_data, get_total_billable_units_for_sent_sms_notifications_in_date_range, @@ -28,7 +28,7 @@ def test_get_rates_for_year(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 5, 18), 0.016) set_up_rate(notify_db, datetime(2017, 3, 31, 23), 0.0158) start_date, end_date = get_financial_year(2017) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert len(rates) == 1 assert datetime.strftime(rates[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2017-03-31 23:00:00" assert rates[0].rate == 0.0158 @@ -39,7 +39,7 @@ def test_get_rates_for_year_multiple_result_per_year(notify_db, notify_db_sessio set_up_rate(notify_db, datetime(2016, 5, 18), 0.016) set_up_rate(notify_db, datetime(2017, 4, 1), 0.0158) start_date, end_date = get_financial_year(2016) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert len(rates) == 2 assert datetime.strftime(rates[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2016-04-01 00:00:00" assert rates[0].rate == 0.015 @@ -52,7 +52,7 @@ def test_get_rates_for_year_returns_correct_rates(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 9, 1), 0.016) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2017) - rates_2017 = get_rates_for_year(start_date, end_date, 'sms') + rates_2017 = get_rates_for_daterange(start_date, end_date, 'sms') assert len(rates_2017) == 2 assert datetime.strftime(rates_2017[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2016-09-01 00:00:00" assert rates_2017[0].rate == 0.016 @@ -64,7 +64,7 @@ def test_get_rates_for_year_in_the_future(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2018) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert datetime.strftime(rates[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2017-06-01 00:00:00" assert rates[0].rate == 0.0175 @@ -73,7 +73,7 @@ def test_get_rates_for_year_returns_empty_list_if_year_is_before_earliest_rate(n set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2015) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert rates == [] @@ -83,7 +83,7 @@ def test_get_rates_for_year_early_rate(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 9, 1), 0.016) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2016) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert len(rates) == 3 @@ -91,7 +91,7 @@ def test_get_rates_for_year_edge_case(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 3, 31, 23, 00), 0.015) set_up_rate(notify_db, datetime(2017, 3, 31, 23, 00), 0.0175) start_date, end_date = get_financial_year(2016) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert len(rates) == 1 assert datetime.strftime(rates[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2016-03-31 23:00:00" assert rates[0].rate == 0.015 From 7db1bfbb774ca222855cbaf47366477d7134a675 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 25 Jul 2017 14:45:59 +0100 Subject: [PATCH 15/34] remove credstash --- aws_run_celery.py | 8 +------- db.py | 8 ++------ requirements.txt | 1 - server_commands.py | 5 ----- wsgi.py | 7 ------- 5 files changed, 3 insertions(+), 26 deletions(-) diff --git a/aws_run_celery.py b/aws_run_celery.py index 36a1f480a..8194528db 100644 --- a/aws_run_celery.py +++ b/aws_run_celery.py @@ -1,11 +1,5 @@ #!/usr/bin/env python -from app import notify_celery, create_app -from credstash import getAllSecrets -import os - -# On AWS get secrets and export to env, skip this on Cloud Foundry -if os.getenv('VCAP_SERVICES') is None: - os.environ.update(getAllSecrets(region="eu-west-1")) +from app import create_app application = create_app("delivery") application.app_context().push() diff --git a/db.py b/db.py index 3a8da01d4..bc558cbd2 100644 --- a/db.py +++ b/db.py @@ -1,12 +1,8 @@ from flask.ext.script import Manager, Server from flask_migrate import Migrate, MigrateCommand -from app import create_app, db -from credstash import getAllSecrets -import os -# On AWS get secrets and export to env, skip this on Cloud Foundry -if os.getenv('VCAP_SERVICES') is None: - os.environ.update(getAllSecrets(region="eu-west-1")) +from app import create_app, db + application = create_app() diff --git a/requirements.txt b/requirements.txt index 0c9b3f1e0..61390b6c4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,6 @@ marshmallow==2.4.2 marshmallow-sqlalchemy==0.8.0 flask-marshmallow==0.6.2 Flask-Bcrypt==0.6.2 -credstash==1.8.0 boto3==1.4.4 celery==3.1.25 # pyup: <4 monotonic==1.2 diff --git a/server_commands.py b/server_commands.py index 85bf13137..af071db47 100644 --- a/server_commands.py +++ b/server_commands.py @@ -1,7 +1,6 @@ from flask.ext.script import Manager, Server from flask_migrate import Migrate, MigrateCommand from app import (create_app, db, commands) -from credstash import getAllSecrets import os default_env_file = '/home/ubuntu/environment' @@ -11,10 +10,6 @@ if os.path.isfile(default_env_file): with open(default_env_file, 'r') as environment_file: environment = environment_file.readline().strip() -# On AWS get secrets and export to env, skip this on Cloud Foundry -if os.getenv('VCAP_SERVICES') is None: - os.environ.update(getAllSecrets(region="eu-west-1")) - from app.config import configs os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment] diff --git a/wsgi.py b/wsgi.py index 2df2c3976..9fbeb28ac 100644 --- a/wsgi.py +++ b/wsgi.py @@ -1,13 +1,6 @@ -import os - from app import create_app -from credstash import getAllSecrets -# On AWS get secrets and export to env, skip this on Cloud Foundry -if os.getenv('VCAP_SERVICES') is None: - os.environ.update(getAllSecrets(region="eu-west-1")) - application = create_app() if __name__ == "__main__": From e23d38de26132240e36516f7b5d66fe78679b608 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Jul 2017 15:50:14 +0100 Subject: [PATCH 16/34] Fix bug in get rates function. --- app/dao/notification_usage_dao.py | 6 ++-- tests/app/dao/test_monthly_billing.py | 1 + tests/app/dao/test_notification_usage_dao.py | 30 ++++++++++++++------ 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 7cd6517e9..79f786dd0 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -129,8 +129,10 @@ def get_rates_for_daterange(start_date, end_date, notification_type): results.append(rates[-1]) if not results: - if start_date >= rates[-1].valid_from: - results.append(rates[-1]) + for x in reversed(rates): + if start_date >= x.valid_from: + results.append(x) + break return results diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index 9027ff870..538f19c51 100644 --- a/tests/app/dao/test_monthly_billing.py +++ b/tests/app/dao/test_monthly_billing.py @@ -13,6 +13,7 @@ def test_add_monthly_billing(sample_template): jan = datetime(2017, 1, 1) feb = datetime(2017, 2, 15) create_rate(start_date=jan, value=0.0158, notification_type='sms') + create_rate(start_date=datetime(2017, 3, 31, 23, 00, 00), value=0.123, notification_type='sms') create_notification(template=sample_template, created_at=jan, billable_units=1, status='delivered') create_notification(template=sample_template, created_at=feb, billable_units=2, status='delivered') diff --git a/tests/app/dao/test_notification_usage_dao.py b/tests/app/dao/test_notification_usage_dao.py index 1555bad77..9eab4f089 100644 --- a/tests/app/dao/test_notification_usage_dao.py +++ b/tests/app/dao/test_notification_usage_dao.py @@ -24,7 +24,7 @@ from freezegun import freeze_time from tests.conftest import set_config -def test_get_rates_for_year(notify_db, notify_db_session): +def test_get_rates_for_daterange(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 5, 18), 0.016) set_up_rate(notify_db, datetime(2017, 3, 31, 23), 0.0158) start_date, end_date = get_financial_year(2017) @@ -34,7 +34,7 @@ def test_get_rates_for_year(notify_db, notify_db_session): assert rates[0].rate == 0.0158 -def test_get_rates_for_year_multiple_result_per_year(notify_db, notify_db_session): +def test_get_rates_for_daterange_multiple_result_per_year(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2016, 5, 18), 0.016) set_up_rate(notify_db, datetime(2017, 4, 1), 0.0158) @@ -47,7 +47,7 @@ def test_get_rates_for_year_multiple_result_per_year(notify_db, notify_db_sessio assert rates[1].rate == 0.016 -def test_get_rates_for_year_returns_correct_rates(notify_db, notify_db_session): +def test_get_rates_for_daterange_returns_correct_rates(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2016, 9, 1), 0.016) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) @@ -60,7 +60,7 @@ def test_get_rates_for_year_returns_correct_rates(notify_db, notify_db_session): assert rates_2017[1].rate == 0.0175 -def test_get_rates_for_year_in_the_future(notify_db, notify_db_session): +def test_get_rates_for_daterange_in_the_future(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2018) @@ -69,7 +69,7 @@ def test_get_rates_for_year_in_the_future(notify_db, notify_db_session): assert rates[0].rate == 0.0175 -def test_get_rates_for_year_returns_empty_list_if_year_is_before_earliest_rate(notify_db, notify_db_session): +def test_get_rates_for_daterange_returns_empty_list_if_year_is_before_earliest_rate(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2015) @@ -77,7 +77,7 @@ def test_get_rates_for_year_returns_empty_list_if_year_is_before_earliest_rate(n assert rates == [] -def test_get_rates_for_year_early_rate(notify_db, notify_db_session): +def test_get_rates_for_daterange_early_rate(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2015, 6, 1), 0.014) set_up_rate(notify_db, datetime(2016, 6, 1), 0.015) set_up_rate(notify_db, datetime(2016, 9, 1), 0.016) @@ -87,7 +87,7 @@ def test_get_rates_for_year_early_rate(notify_db, notify_db_session): assert len(rates) == 3 -def test_get_rates_for_year_edge_case(notify_db, notify_db_session): +def test_get_rates_for_daterange_edge_case(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 3, 31, 23, 00), 0.015) set_up_rate(notify_db, datetime(2017, 3, 31, 23, 00), 0.0175) start_date, end_date = get_financial_year(2016) @@ -97,6 +97,19 @@ def test_get_rates_for_year_edge_case(notify_db, notify_db_session): assert rates[0].rate == 0.015 +def test_get_rates_for_daterange_where_daterange_is_one_month_that_falls_between_rate_valid_from( + notify_db, notify_db_session +): + set_up_rate(notify_db, datetime(2017, 1, 1), 0.175) + set_up_rate(notify_db, datetime(2017, 3, 31), 0.123) + start_date = datetime(2017, 2, 1, 00, 00, 00) + end_date = datetime(2017, 2, 28, 23, 59, 59, 99999) + rates = get_rates_for_daterange(start_date, end_date, 'sms') + assert len(rates) == 1 + assert datetime.strftime(rates[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2017-01-01 00:00:00" + assert rates[0].rate == 0.175 + + def test_get_yearly_billing_data(notify_db, notify_db_session, sample_template, sample_email_template): set_up_rate(notify_db, datetime(2016, 4, 1), 0.014) set_up_rate(notify_db, datetime(2016, 6, 1), 0.0158) @@ -254,8 +267,7 @@ def test_get_monthly_billing_data_with_multiple_rates(notify_db, notify_db_sessi assert results[3] == ('June', 4, 1, False, 'sms', 0.0175) -def test_get_monthly_billing_data_with_no_notifications_for_year(notify_db, notify_db_session, sample_template, - sample_email_template): +def test_get_monthly_billing_data_with_no_notifications_for_daterange(notify_db, notify_db_session, sample_template): set_up_rate(notify_db, datetime(2016, 4, 1), 0.014) results = get_monthly_billing_data(sample_template.service_id, 2016) assert len(results) == 0 From 9da5682c7022c5b298a828befb837e29bed25a0f Mon Sep 17 00:00:00 2001 From: venusbb Date: Tue, 25 Jul 2017 17:17:06 +0100 Subject: [PATCH 17/34] Experiment with logging a custom request header --- app/authentication/auth.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 9c42f2431..d10a98bb0 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -52,14 +52,18 @@ def restrict_ip_sms(): ip_list = ip_route.split(',') if len(ip_list) >= 3: ip = ip_list[len(ip_list) - 3] - current_app.logger.info("Inbound sms ip route list {} OS environ{}" - .format(ip_route, current_app.config.get('SMS_INBOUND_WHITELIST'))) + current_app.logger.info("Inbound sms ip route list {}" + .format(ip_route)) + + # Temporary custom header for route security - to experiment if the header passes through + if request.headers.get("X-Custom-forwarder"): + current_app.logger.info("X-Custom-forwarder {}".format(request.headers.get("X-Custom-forwarder"))) if ip in current_app.config.get('SMS_INBOUND_WHITELIST'): current_app.logger.info("Inbound sms ip addresses {} passed ".format(ip)) return else: - current_app.logger.info("Inbound sms ip addresses {} blocked ".format(ip)) + current_app.logger.info("Inbound sms ip addresses blocked {}".format(ip)) return # raise AuthError('Unknown source IP address from the SMS provider', 403) From beca03a39c647cbbe0ae7c388614fe655384f774 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Wed, 12 Jul 2017 13:10:44 +0100 Subject: [PATCH 18/34] Add migration script to drop service flags --- .../versions/0109_drop_old_service_flags.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 migrations/versions/0109_drop_old_service_flags.py diff --git a/migrations/versions/0109_drop_old_service_flags.py b/migrations/versions/0109_drop_old_service_flags.py new file mode 100644 index 000000000..41c8c5550 --- /dev/null +++ b/migrations/versions/0109_drop_old_service_flags.py @@ -0,0 +1,28 @@ +"""empty message + +Revision ID: 0109_drop_old_service_flags +Revises: 0108_change_logo_not_nullable +Create Date: 2017-07-12 13:35:45.636618 + +""" + +# revision identifiers, used by Alembic. +revision = '0109_drop_old_service_flags' +down_revision = '0108_change_logo_not_nullable' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +def upgrade(): + op.drop_column('services', 'can_send_letters') + op.drop_column('services', 'can_send_international_sms') + op.drop_column('services_history', 'can_send_letters') + op.drop_column('services_history', 'can_send_international_sms') + + +def downgrade(): + op.add_column('services_history', sa.Column('can_send_international_sms', sa.BOOLEAN(), server_default=sa.text('false'), autoincrement=False, nullable=False)) + op.add_column('services_history', sa.Column('can_send_letters', sa.BOOLEAN(), server_default=sa.text('false'), autoincrement=False, nullable=False)) + op.add_column('services', sa.Column('can_send_international_sms', sa.BOOLEAN(), server_default=sa.text('false'), autoincrement=False, nullable=False)) + op.add_column('services', sa.Column('can_send_letters', sa.BOOLEAN(), server_default=sa.text('false'), autoincrement=False, nullable=False)) From 277f5b9053e418fc031296654cf5f50875f6e69c Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Wed, 19 Jul 2017 14:27:41 +0100 Subject: [PATCH 19/34] Renamed script --- ...ld_service_flags.py => 0110_drop_old_service_flags.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0109_drop_old_service_flags.py => 0110_drop_old_service_flags.py} (86%) diff --git a/migrations/versions/0109_drop_old_service_flags.py b/migrations/versions/0110_drop_old_service_flags.py similarity index 86% rename from migrations/versions/0109_drop_old_service_flags.py rename to migrations/versions/0110_drop_old_service_flags.py index 41c8c5550..38fe72cd7 100644 --- a/migrations/versions/0109_drop_old_service_flags.py +++ b/migrations/versions/0110_drop_old_service_flags.py @@ -1,14 +1,14 @@ """empty message -Revision ID: 0109_drop_old_service_flags -Revises: 0108_change_logo_not_nullable +Revision ID: 0110_drop_old_service_flags +Revises: 0109_rem_old_noti_status Create Date: 2017-07-12 13:35:45.636618 """ # revision identifiers, used by Alembic. -revision = '0109_drop_old_service_flags' -down_revision = '0108_change_logo_not_nullable' +revision = '0110_drop_old_service_flags' +down_revision = '0109_rem_old_noti_status' from alembic import op import sqlalchemy as sa From 4989493bdf554a26517f4c33db3a1c1df3f84d86 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 25 Jul 2017 17:23:30 +0100 Subject: [PATCH 20/34] Renamed migration script --- ...ld_service_flags.py => 0111_drop_old_service_flags.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0110_drop_old_service_flags.py => 0111_drop_old_service_flags.py} (87%) diff --git a/migrations/versions/0110_drop_old_service_flags.py b/migrations/versions/0111_drop_old_service_flags.py similarity index 87% rename from migrations/versions/0110_drop_old_service_flags.py rename to migrations/versions/0111_drop_old_service_flags.py index 38fe72cd7..fd9bc5487 100644 --- a/migrations/versions/0110_drop_old_service_flags.py +++ b/migrations/versions/0111_drop_old_service_flags.py @@ -1,14 +1,14 @@ """empty message -Revision ID: 0110_drop_old_service_flags -Revises: 0109_rem_old_noti_status +Revision ID: 0111_drop_old_service_flags +Revises: 0110_monthly_billing Create Date: 2017-07-12 13:35:45.636618 """ # revision identifiers, used by Alembic. -revision = '0110_drop_old_service_flags' -down_revision = '0109_rem_old_noti_status' +revision = '0111_drop_old_service_flags' +down_revision = '0110_monthly_billing' from alembic import op import sqlalchemy as sa From b62ee8380c81a37d5cc6fa0234140cbaf0ce00d3 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Jul 2017 17:38:53 +0100 Subject: [PATCH 21/34] Use BST to calculate monthly billing --- app/dao/date_util.py | 4 +++- tests/app/dao/test_date_utils.py | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/app/dao/date_util.py b/app/dao/date_util.py index 41ad9dc72..8019fdecc 100644 --- a/app/dao/date_util.py +++ b/app/dao/date_util.py @@ -2,6 +2,8 @@ from datetime import datetime, timedelta import pytz +from app.utils import convert_bst_to_utc + def get_financial_year(year): return get_april_fools(year), get_april_fools(year + 1) - timedelta(microseconds=1) @@ -28,4 +30,4 @@ def get_month_start_end_date(month_year): _, num_days = calendar.monthrange(month_year.year, month_year.month) first_day = datetime(month_year.year, month_year.month, 1, 0, 0, 0) last_day = datetime(month_year.year, month_year.month, num_days, 23, 59, 59, 99999) - return first_day, last_day + return convert_bst_to_utc(first_day), convert_bst_to_utc(last_day) diff --git a/tests/app/dao/test_date_utils.py b/tests/app/dao/test_date_utils.py index 4ede18853..5267f4a06 100644 --- a/tests/app/dao/test_date_utils.py +++ b/tests/app/dao/test_date_utils.py @@ -17,14 +17,15 @@ def test_get_april_fools(): assert april_fools.tzinfo is None -@pytest.mark.parametrize("month, year, expected_end", - [(7, 2017, 31), - (2, 2016, 29), - (2, 2017, 28), - (9, 2018, 30), - (12, 2019, 31)]) -def test_get_month_start_end_date(month, year, expected_end): +@pytest.mark.parametrize("month, year, expected_start, expected_end", + [ + (7, 2017, datetime(2017, 6, 30, 23, 00, 00), datetime(2017, 7, 31, 22, 59, 59, 99999)), + (2, 2016, datetime(2016, 2, 1, 00, 00, 00), datetime(2016, 2, 29, 23, 59, 59, 99999)), + (2, 2017, datetime(2017, 2, 1, 00, 00, 00), datetime(2017, 2, 28, 23, 59, 59, 99999)), + (9, 2018, datetime(2018, 8, 31, 23, 00, 00), datetime(2018, 9, 30, 22, 59, 59, 99999)), + (12, 2019, datetime(2019, 12, 1, 00, 00, 00), datetime(2019, 12, 31, 23, 59, 59, 99999))]) +def test_get_month_start_end_date(month, year, expected_start, expected_end): month_year = datetime(year, month, 10, 13, 30, 00) result = get_month_start_end_date(month_year) - assert result[0] == datetime(year, month, 1, 0, 0, 0, 0) - assert result[1] == datetime(year, month, expected_end, 23, 59, 59, 99999) + assert result[0] == expected_start + assert result[1] == expected_end From f73b5140eddc63f689a3d2f18ea17ad411db96d3 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Jul 2017 17:47:08 +0100 Subject: [PATCH 22/34] Bah! style check --- tests/app/dao/test_date_utils.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/app/dao/test_date_utils.py b/tests/app/dao/test_date_utils.py index 5267f4a06..1533b4834 100644 --- a/tests/app/dao/test_date_utils.py +++ b/tests/app/dao/test_date_utils.py @@ -19,11 +19,11 @@ def test_get_april_fools(): @pytest.mark.parametrize("month, year, expected_start, expected_end", [ - (7, 2017, datetime(2017, 6, 30, 23, 00, 00), datetime(2017, 7, 31, 22, 59, 59, 99999)), - (2, 2016, datetime(2016, 2, 1, 00, 00, 00), datetime(2016, 2, 29, 23, 59, 59, 99999)), - (2, 2017, datetime(2017, 2, 1, 00, 00, 00), datetime(2017, 2, 28, 23, 59, 59, 99999)), - (9, 2018, datetime(2018, 8, 31, 23, 00, 00), datetime(2018, 9, 30, 22, 59, 59, 99999)), - (12, 2019, datetime(2019, 12, 1, 00, 00, 00), datetime(2019, 12, 31, 23, 59, 59, 99999))]) + (7, 2017, datetime(2017, 6, 30, 23, 00, 00), datetime(2017, 7, 31, 22, 59, 59, 99999)), + (2, 2016, datetime(2016, 2, 1, 00, 00, 00), datetime(2016, 2, 29, 23, 59, 59, 99999)), + (2, 2017, datetime(2017, 2, 1, 00, 00, 00), datetime(2017, 2, 28, 23, 59, 59, 99999)), + (9, 2018, datetime(2018, 8, 31, 23, 00, 00), datetime(2018, 9, 30, 22, 59, 59, 99999)), + (12, 2019, datetime(2019, 12, 1, 00, 00, 00), datetime(2019, 12, 31, 23, 59, 59, 99999))]) def test_get_month_start_end_date(month, year, expected_start, expected_end): month_year = datetime(year, month, 10, 13, 30, 00) result = get_month_start_end_date(month_year) From 9c55fa7f342f983f16198a44fb3f002de827983e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Jul 2017 18:08:46 +0100 Subject: [PATCH 23/34] Use the end date for the month, when we are in BST the first day of the month is an hour behind and in the previous month. --- app/dao/monthly_billing_dao.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index 49a571e5c..d2eabb9db 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -26,7 +26,7 @@ def create_or_update_monthly_billing_sms(service_id, billing_month): # update monthly monthly_totals = _monthly_billing_data_to_json(monthly) row = MonthlyBilling.query.filter_by(year=billing_month.year, - month=datetime.strftime(billing_month, "%B"), + month=datetime.strftime(end_date, "%B"), notification_type='sms').first() if row: row.monthly_totals = monthly_totals @@ -34,7 +34,7 @@ def create_or_update_monthly_billing_sms(service_id, billing_month): row = MonthlyBilling(service_id=service_id, notification_type=SMS_TYPE, year=billing_month.year, - month=datetime.strftime(billing_month, "%B"), + month=datetime.strftime(end_date, "%B"), monthly_totals=monthly_totals) db.session.add(row) From 0a7890f069faad3330258c9a5bc7dbd784ddf85d Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 26 Jul 2017 09:43:25 +0100 Subject: [PATCH 24/34] Use the right date for the billing month. --- app/celery/scheduled_tasks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 08433e017..f7649d098 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -298,5 +298,5 @@ def populate_monthly_billing(): # this will overwrite the existing amount. yesterday = datetime.utcnow() - timedelta(days=1) start_date, end_date = get_month_start_end_date(yesterday) - services = get_service_ids_that_need_sms_billing_populated(start_date, end_date=end_date) - [create_or_update_monthly_billing_sms(service_id=s.service_id, billing_month=start_date) for s in services] + services = get_service_ids_that_need_sms_billing_populated(start_date=start_date, end_date=end_date) + [create_or_update_monthly_billing_sms(service_id=s.service_id, billing_month=end_date) for s in services] From 92c441656b7efa3ec4cb931c05c2b7e36be192f3 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 26 Jul 2017 09:44:20 +0100 Subject: [PATCH 25/34] Use the right date for the method --- app/celery/scheduled_tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index f7649d098..17533ac08 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -299,4 +299,4 @@ def populate_monthly_billing(): yesterday = datetime.utcnow() - timedelta(days=1) start_date, end_date = get_month_start_end_date(yesterday) services = get_service_ids_that_need_sms_billing_populated(start_date=start_date, end_date=end_date) - [create_or_update_monthly_billing_sms(service_id=s.service_id, billing_month=end_date) for s in services] + [create_or_update_monthly_billing_sms(service_id=s.service_id, billing_month=yesterday) for s in services] From 3e6e75998b8e2f28fb7cfeaf9477a22f21d1555d Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 21 Jul 2017 16:06:12 +0100 Subject: [PATCH 26/34] Update config to use logo cdn --- app/config.py | 1 - app/delivery/send_to_providers.py | 29 ++++++-------------- tests/app/delivery/test_send_to_providers.py | 23 ++++++---------- 3 files changed, 16 insertions(+), 37 deletions(-) diff --git a/app/config.py b/app/config.py index 99b77cc8d..5d71c0f41 100644 --- a/app/config.py +++ b/app/config.py @@ -115,7 +115,6 @@ class Config(object): PAGE_SIZE = 50 API_PAGE_SIZE = 250 SMS_CHAR_COUNT_LIMIT = 495 - BRANDING_PATH = '/images/email-template/crests/' TEST_MESSAGE_FILENAME = 'Test message' ONE_OFF_MESSAGE_FILENAME = 'Report' MAX_VERIFY_CODE_COUNT = 10 diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 5e33e2113..be7dbed88 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -145,34 +145,22 @@ def provider_to_use(notification_type, notification_id, international=False): return clients.get_client_by_name_and_type(active_providers_in_order[0].identifier, notification_type) -def get_logo_url(base_url, branding_path, logo_file): - """ - Get the complete URL for a given logo. - - We have to convert the base_url into a static url. Our hosted environments all have their own cloudfront instances, - found at the static subdomain (eg https://static.notifications.service.gov.uk). - - If running locally (dev environment), don't try and use cloudfront - just stick to the actual underlying source - ({URL}/static/{PATH}) - """ +def get_logo_url(base_url, logo_file): base_url = parse.urlparse(base_url) netloc = base_url.netloc - # covers both preview and staging - if base_url.netloc.startswith('localhost') or 'notify.works' in base_url.netloc: - path = '/static' + branding_path + logo_file - else: - if base_url.netloc.startswith('www'): - # strip "www." - netloc = base_url.netloc[4:] + if base_url.netloc.startswith('localhost'): + netloc = 'notify.tools' + elif base_url.netloc.startswith('www'): + # strip "www." + netloc = base_url.netloc[4:] - netloc = 'static.' + netloc - path = branding_path + logo_file + netloc = 'static-logos.' + netloc logo_url = parse.ParseResult( scheme=base_url.scheme, netloc=netloc, - path=path, + path=logo_file, params=base_url.params, query=base_url.query, fragment=base_url.fragment @@ -185,7 +173,6 @@ def get_html_email_options(service): if service.organisation: logo_url = get_logo_url( current_app.config['ADMIN_BASE_URL'], - current_app.config['BRANDING_PATH'], service.organisation.logo ) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index e1a4ec4a2..d14fee6d8 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -435,29 +435,22 @@ def test_get_html_email_renderer_prepends_logo_path(notify_api): renderer = send_to_providers.get_html_email_options(service) - assert renderer['brand_logo'] == 'http://localhost:6012/static/images/email-template/crests/justice-league.png' + assert renderer['brand_logo'] == 'http://static-logos.notify.tools/justice-league.png' @pytest.mark.parametrize('base_url, expected_url', [ # don't change localhost to prevent errors when testing locally - ('http://localhost:6012', 'http://localhost:6012/static/sub-path/filename.png'), - # on other environments, replace www with staging - ('https://www.notifications.service.gov.uk', 'https://static.notifications.service.gov.uk/sub-path/filename.png'), - - # staging and preview do not have cloudfront running, so should act as localhost - pytest.mark.xfail(('https://www.notify.works', 'https://static.notify.works/sub-path/filename.png')), - pytest.mark.xfail(('https://www.staging-notify.works', 'https://static.notify.works/sub-path/filename.png')), - pytest.mark.xfail(('https://notify.works', 'https://static.notify.works/sub-path/filename.png')), - pytest.mark.xfail(('https://staging-notify.works', 'https://static.notify.works/sub-path/filename.png')), - # these tests should be removed when cloudfront works on staging/preview - ('https://www.notify.works', 'https://www.notify.works/static/sub-path/filename.png'), - ('https://www.staging-notify.works', 'https://www.staging-notify.works/static/sub-path/filename.png'), + ('http://localhost:6012', 'http://static-logos.notify.tools/filename.png'), + ('https://www.notifications.service.gov.uk', 'https://static-logos.notifications.service.gov.uk/filename.png'), + ('https://notify.works', 'https://static-logos.notify.works/filename.png'), + ('https://staging-notify.works', 'https://static-logos.staging-notify.works/filename.png'), + ('https://www.notify.works', 'https://static-logos.notify.works/filename.png'), + ('https://www.staging-notify.works', 'https://static-logos.staging-notify.works/filename.png'), ]) def test_get_logo_url_works_for_different_environments(base_url, expected_url): - branding_path = '/sub-path/' logo_file = 'filename.png' - logo_url = send_to_providers.get_logo_url(base_url, branding_path, logo_file) + logo_url = send_to_providers.get_logo_url(base_url, logo_file) assert logo_url == expected_url From 0daf88aff7f91b6ddf4d6766c9c00b17a8dc0c62 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 21 Jul 2017 16:37:26 +0100 Subject: [PATCH 27/34] Refactored code --- app/delivery/send_to_providers.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index be7dbed88..fcf4348d8 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -155,11 +155,9 @@ def get_logo_url(base_url, logo_file): # strip "www." netloc = base_url.netloc[4:] - netloc = 'static-logos.' + netloc - logo_url = parse.ParseResult( scheme=base_url.scheme, - netloc=netloc, + netloc='static-logos.' + netloc, path=logo_file, params=base_url.params, query=base_url.query, From c1f2634c902dfe5ab6ed887094fb78fb40305176 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 26 Jul 2017 13:19:17 +0100 Subject: [PATCH 28/34] Removed month and year and replaced it with start_date and end_date. This will allow us to sort the data properly. --- app/commands.py | 1 + app/dao/monthly_billing_dao.py | 14 ++--- app/models.py | 10 ++-- .../versions/0112_add_start_end_dates.py | 54 +++++++++++++++++++ tests/app/celery/test_scheduled_tasks.py | 4 +- tests/app/dao/test_monthly_billing.py | 41 ++++++++------ 6 files changed, 95 insertions(+), 29 deletions(-) create mode 100644 migrations/versions/0112_add_start_end_dates.py diff --git a/app/commands.py b/app/commands.py index c7ec0567b..14e26b887 100644 --- a/app/commands.py +++ b/app/commands.py @@ -158,6 +158,7 @@ class PopulateMonthlyBilling(Command): ) def run(self, service_id, month, year): + print('Starting populating monthly billing') create_or_update_monthly_billing_sms(service_id, datetime(int(year), int(month), 1)) results = get_monthly_billing_sms(service_id, datetime(int(year), int(month), 1)) print("Finished populating data for {} for service id {}".format(month, service_id)) diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index d2eabb9db..4f4e254c8 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -25,25 +25,25 @@ def create_or_update_monthly_billing_sms(service_id, billing_month): monthly = get_billing_data_for_month(service_id=service_id, start_date=start_date, end_date=end_date) # update monthly monthly_totals = _monthly_billing_data_to_json(monthly) - row = MonthlyBilling.query.filter_by(year=billing_month.year, - month=datetime.strftime(end_date, "%B"), + row = MonthlyBilling.query.filter_by(start_date=start_date, notification_type='sms').first() if row: row.monthly_totals = monthly_totals + row.updated_at = datetime.utcnow() else: row = MonthlyBilling(service_id=service_id, notification_type=SMS_TYPE, - year=billing_month.year, - month=datetime.strftime(end_date, "%B"), - monthly_totals=monthly_totals) + monthly_totals=monthly_totals, + start_date=start_date, + end_date=end_date) db.session.add(row) @statsd(namespace="dao") def get_monthly_billing_sms(service_id, billing_month): + start_date, end_date = get_month_start_end_date(billing_month) monthly = MonthlyBilling.query.filter_by(service_id=service_id, - year=billing_month.year, - month=datetime.strftime(billing_month, "%B"), + start_date=start_date, notification_type=SMS_TYPE).first() return monthly diff --git a/app/models.py b/app/models.py index b58eaf348..5ab742b4f 100644 --- a/app/models.py +++ b/app/models.py @@ -1253,20 +1253,20 @@ class MonthlyBilling(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, nullable=False) service = db.relationship('Service', backref='monthly_billing') - month = db.Column(db.String, nullable=False) - year = db.Column(db.Float(asdecimal=False), nullable=False) + start_date = db.Column(db.DateTime, nullable=False) + end_date = db.Column(db.DateTime, nullable=False) notification_type = db.Column(notification_types, nullable=False) monthly_totals = db.Column(JSON, nullable=False) updated_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) __table_args__ = ( - UniqueConstraint('service_id', 'month', 'year', 'notification_type', name='uix_monthly_billing'), + UniqueConstraint('service_id', 'start_date', 'notification_type', name='uix_monthly_billing'), ) def serialized(self): return { - "month": self.month, - "year": self.year, + "start_date": self.start_date, + "end_date": self.end_date, "service_id": str(self.service_id), "notification_type": self.notification_type, "monthly_totals": self.monthly_totals diff --git a/migrations/versions/0112_add_start_end_dates.py b/migrations/versions/0112_add_start_end_dates.py new file mode 100644 index 000000000..658c573ae --- /dev/null +++ b/migrations/versions/0112_add_start_end_dates.py @@ -0,0 +1,54 @@ +"""empty message + +Revision ID: 0112_add_start_end_dates +Revises: 0111_drop_old_service_flags +Create Date: 2017-07-12 13:35:45.636618 + +""" +from datetime import datetime +from alembic import op +import sqlalchemy as sa +from app.dao.date_util import get_month_start_end_date + +down_revision = '0111_drop_old_service_flags' +revision = '0112_add_start_end_dates' + + +def upgrade(): + op.drop_index('uix_monthly_billing', 'monthly_billing') + op.add_column('monthly_billing', sa.Column('start_date', sa.DateTime)) + op.add_column('monthly_billing', sa.Column('end_date', sa.DateTime)) + conn = op.get_bind() + results = conn.execute("Select id, month, year from monthly_billing") + res = results.fetchall() + for x in res: + start_date, end_date = get_month_start_end_date( + datetime(int(x.year), datetime.strptime(x.month, '%B').month, 1)) + conn.execute("update monthly_billing set start_date = '{}', end_date = '{}' where id = '{}'".format(start_date, + end_date, + x.id)) + op.alter_column('monthly_billing', 'start_date', nullable=False) + op.alter_column('monthly_billing', 'end_date', nullable=False) + op.drop_column('monthly_billing', 'month') + op.drop_column('monthly_billing', 'year') + + op.create_index(op.f('uix_monthly_billing'), 'monthly_billing', ['service_id', 'start_date', 'notification_type'], + unique=True) + + +def downgrade(): + op.add_column('monthly_billing', sa.Column('month', sa.String(), nullable=True)) + op.add_column('monthly_billing', sa.Column('year', sa.Float(), nullable=True)) + conn = op.get_bind() + results = conn.execute("Select id, start_date, end_date from monthly_billing") + res = results.fetchall() + for x in res: + year = datetime.strftime(x.end_date, '%Y') + month = datetime.strftime(x.end_date, '%B') + conn.execute("update monthly_billing set month = '{}', year = {} where id = '{}'".format(month, year, x.id)) + + op.drop_column('monthly_billing', 'start_date') + op.drop_column('monthly_billing', 'end_date') + + op.create_index(op.f('uix_monthly_billing'), 'monthly_billing', + ['service_id', 'month', 'year', 'notification_type'], unique=True) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 6ea42844d..8ce518dfb 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -627,8 +627,8 @@ def test_populate_monthly_billing(sample_template): monthly_billing = MonthlyBilling.query.all() assert len(monthly_billing) == 1 assert monthly_billing[0].service_id == sample_template.service_id - assert monthly_billing[0].year == 2017 - assert monthly_billing[0].month == 'July' + assert monthly_billing[0].start_date == datetime(2017, 6, 30, 23) + assert monthly_billing[0].end_date == datetime(2017, 7, 31, 22, 59, 59, 99999) assert monthly_billing[0].notification_type == 'sms' assert len(monthly_billing[0].monthly_totals) == 1 assert sorted(monthly_billing[0].monthly_totals[0]) == sorted({'international': False, diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index 538f19c51..e07588374 100644 --- a/tests/app/dao/test_monthly_billing.py +++ b/tests/app/dao/test_monthly_billing.py @@ -1,5 +1,8 @@ from datetime import datetime +from freezegun import freeze_time +from freezegun.api import FakeDatetime + from app.dao.monthly_billing_dao import ( create_or_update_monthly_billing_sms, get_monthly_billing_sms, @@ -23,8 +26,8 @@ def test_add_monthly_billing(sample_template): billing_month=feb) monthly_billing = MonthlyBilling.query.all() assert len(monthly_billing) == 2 - assert monthly_billing[0].month == 'January' - assert monthly_billing[1].month == 'February' + assert monthly_billing[0].start_date == datetime(2017, 1, 1) + assert monthly_billing[1].start_date == datetime(2017, 2, 1) january = get_monthly_billing_sms(service_id=sample_template.service_id, billing_month=jan) expected_jan = {"billing_units": 1, @@ -32,7 +35,8 @@ def test_add_monthly_billing(sample_template): "international": False, "rate": 0.0158, "total_cost": 1 * 0.0158} - assert_monthly_billing(january, 2017, "January", sample_template.service_id, 1, expected_jan) + assert_monthly_billing(january, sample_template.service_id, 1, expected_jan, + start_date=datetime(2017, 1, 1), end_date=datetime(2017, 1, 31)) february = get_monthly_billing_sms(service_id=sample_template.service_id, billing_month=feb) expected_feb = {"billing_units": 2, @@ -40,7 +44,8 @@ def test_add_monthly_billing(sample_template): "international": False, "rate": 0.0158, "total_cost": 2 * 0.0158} - assert_monthly_billing(february, 2017, "February", sample_template.service_id, 1, expected_feb) + assert_monthly_billing(february, sample_template.service_id, 1, expected_feb, + start_date=datetime(2017, 2, 1), end_date=datetime(2017, 2, 28)) def test_add_monthly_billing_multiple_rates_in_a_month(sample_template): @@ -62,7 +67,7 @@ def test_add_monthly_billing_multiple_rates_in_a_month(sample_template): billing_month=rate_2) monthly_billing = MonthlyBilling.query.all() assert len(monthly_billing) == 1 - assert monthly_billing[0].month == 'January' + assert monthly_billing[0].start_date == datetime(2017, 1, 1) january = get_monthly_billing_sms(service_id=sample_template.service_id, billing_month=rate_2) first_row = {"billing_units": 2, @@ -70,7 +75,8 @@ def test_add_monthly_billing_multiple_rates_in_a_month(sample_template): "international": False, "rate": 0.0158, "total_cost": 3 * 0.0158} - assert_monthly_billing(january, 2017, "January", sample_template.service_id, 2, first_row) + assert_monthly_billing(january, sample_template.service_id, 2, first_row, + start_date=datetime(2017, 1, 1), end_date=datetime(2017, 1, 1)) second_row = {"billing_units": 6, "rate_multiplier": 1, "international": False, @@ -83,30 +89,35 @@ def test_update_monthly_billing_overwrites_old_totals(sample_template): july = datetime(2017, 7, 1) create_rate(july, 0.123, 'sms') create_notification(template=sample_template, created_at=datetime(2017, 7, 2), billable_units=1, status='delivered') - - create_or_update_monthly_billing_sms(sample_template.service_id, july) + with freeze_time('2017-07-20 02:30:00'): + create_or_update_monthly_billing_sms(sample_template.service_id, july) first_update = get_monthly_billing_sms(sample_template.service_id, july) expected = {"billing_units": 1, "rate_multiplier": 1, "international": False, "rate": 0.123, "total_cost": 1 * 0.123} - assert_monthly_billing(first_update, 2017, "July", sample_template.service_id, 1, expected) + assert_monthly_billing(first_update, sample_template.service_id, 1, expected, + start_date=datetime(2017, 6, 30, 23), end_date=datetime(2017, 7, 31, 23, 59, 59, 99999)) + first_updated_at = first_update.updated_at + with freeze_time('2017-07-20 03:30:00'): + create_notification(template=sample_template, created_at=datetime(2017, 7, 5), billable_units=2, + status='delivered') - create_notification(template=sample_template, created_at=datetime(2017, 7, 5), billable_units=2, status='delivered') - create_or_update_monthly_billing_sms(sample_template.service_id, july) + create_or_update_monthly_billing_sms(sample_template.service_id, july) second_update = get_monthly_billing_sms(sample_template.service_id, july) expected_update = {"billing_units": 3, "rate_multiplier": 1, "international": False, "rate": 0.123, "total_cost": 3 * 0.123} - assert_monthly_billing(second_update, 2017, "July", sample_template.service_id, 1, expected_update) + assert_monthly_billing(second_update, sample_template.service_id, 1, expected_update, + start_date=datetime(2017, 6, 30, 23), end_date=datetime(2017, 7, 31, 23, 59, 59, 99999)) + assert second_update.updated_at == FakeDatetime(2017, 7, 20, 3, 30) + assert first_updated_at != second_update.updated_at -def assert_monthly_billing(monthly_billing, year, month, service_id, expected_len, first_row): - assert monthly_billing.year == year - assert monthly_billing.month == month +def assert_monthly_billing(monthly_billing, service_id, expected_len, first_row, start_date, end_date): assert monthly_billing.service_id == service_id assert len(monthly_billing.monthly_totals) == expected_len assert sorted(monthly_billing.monthly_totals[0]) == sorted(first_row) From 5669d0475f9706a44e7c1615e52ee04460fd5bd1 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 26 Jul 2017 14:46:40 +0100 Subject: [PATCH 29/34] Don't drop the columns yet --- migrations/versions/0112_add_start_end_dates.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/migrations/versions/0112_add_start_end_dates.py b/migrations/versions/0112_add_start_end_dates.py index 658c573ae..dc98d7fa3 100644 --- a/migrations/versions/0112_add_start_end_dates.py +++ b/migrations/versions/0112_add_start_end_dates.py @@ -29,24 +29,11 @@ def upgrade(): x.id)) op.alter_column('monthly_billing', 'start_date', nullable=False) op.alter_column('monthly_billing', 'end_date', nullable=False) - op.drop_column('monthly_billing', 'month') - op.drop_column('monthly_billing', 'year') - op.create_index(op.f('uix_monthly_billing'), 'monthly_billing', ['service_id', 'start_date', 'notification_type'], unique=True) def downgrade(): - op.add_column('monthly_billing', sa.Column('month', sa.String(), nullable=True)) - op.add_column('monthly_billing', sa.Column('year', sa.Float(), nullable=True)) - conn = op.get_bind() - results = conn.execute("Select id, start_date, end_date from monthly_billing") - res = results.fetchall() - for x in res: - year = datetime.strftime(x.end_date, '%Y') - month = datetime.strftime(x.end_date, '%B') - conn.execute("update monthly_billing set month = '{}', year = {} where id = '{}'".format(month, year, x.id)) - op.drop_column('monthly_billing', 'start_date') op.drop_column('monthly_billing', 'end_date') From 8b6be67bbdad829d2816001d9fff96c556e3dd1f Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 26 Jul 2017 15:06:09 +0100 Subject: [PATCH 30/34] make columns nullable --- migrations/versions/0112_add_start_end_dates.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/migrations/versions/0112_add_start_end_dates.py b/migrations/versions/0112_add_start_end_dates.py index dc98d7fa3..cbb96b64c 100644 --- a/migrations/versions/0112_add_start_end_dates.py +++ b/migrations/versions/0112_add_start_end_dates.py @@ -16,6 +16,8 @@ revision = '0112_add_start_end_dates' def upgrade(): op.drop_index('uix_monthly_billing', 'monthly_billing') + op.alter_column('monthly_billing', 'month', nullable=True) + op.alter_column('monthly_billing', 'year', nullable=True) op.add_column('monthly_billing', sa.Column('start_date', sa.DateTime)) op.add_column('monthly_billing', sa.Column('end_date', sa.DateTime)) conn = op.get_bind() From de41fde0d37e3f52b316bcda74445c07d04eb28b Mon Sep 17 00:00:00 2001 From: Athanasios Voutsadakis Date: Fri, 28 Jul 2017 16:09:49 +0100 Subject: [PATCH 31/34] Revert "remove credstash" This reverts commit 7db1bfbb774ca222855cbaf47366477d7134a675. --- aws_run_celery.py | 8 +++++++- db.py | 6 +++++- requirements.txt | 1 + server_commands.py | 5 +++++ wsgi.py | 9 ++++++++- 5 files changed, 26 insertions(+), 3 deletions(-) diff --git a/aws_run_celery.py b/aws_run_celery.py index 8194528db..36a1f480a 100644 --- a/aws_run_celery.py +++ b/aws_run_celery.py @@ -1,5 +1,11 @@ #!/usr/bin/env python -from app import create_app +from app import notify_celery, create_app +from credstash import getAllSecrets +import os + +# On AWS get secrets and export to env, skip this on Cloud Foundry +if os.getenv('VCAP_SERVICES') is None: + os.environ.update(getAllSecrets(region="eu-west-1")) application = create_app("delivery") application.app_context().push() diff --git a/db.py b/db.py index bc558cbd2..3a8da01d4 100644 --- a/db.py +++ b/db.py @@ -1,8 +1,12 @@ from flask.ext.script import Manager, Server from flask_migrate import Migrate, MigrateCommand - from app import create_app, db +from credstash import getAllSecrets +import os +# On AWS get secrets and export to env, skip this on Cloud Foundry +if os.getenv('VCAP_SERVICES') is None: + os.environ.update(getAllSecrets(region="eu-west-1")) application = create_app() diff --git a/requirements.txt b/requirements.txt index 61390b6c4..0c9b3f1e0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,6 +11,7 @@ marshmallow==2.4.2 marshmallow-sqlalchemy==0.8.0 flask-marshmallow==0.6.2 Flask-Bcrypt==0.6.2 +credstash==1.8.0 boto3==1.4.4 celery==3.1.25 # pyup: <4 monotonic==1.2 diff --git a/server_commands.py b/server_commands.py index af071db47..85bf13137 100644 --- a/server_commands.py +++ b/server_commands.py @@ -1,6 +1,7 @@ from flask.ext.script import Manager, Server from flask_migrate import Migrate, MigrateCommand from app import (create_app, db, commands) +from credstash import getAllSecrets import os default_env_file = '/home/ubuntu/environment' @@ -10,6 +11,10 @@ if os.path.isfile(default_env_file): with open(default_env_file, 'r') as environment_file: environment = environment_file.readline().strip() +# On AWS get secrets and export to env, skip this on Cloud Foundry +if os.getenv('VCAP_SERVICES') is None: + os.environ.update(getAllSecrets(region="eu-west-1")) + from app.config import configs os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment] diff --git a/wsgi.py b/wsgi.py index 9fbeb28ac..2df2c3976 100644 --- a/wsgi.py +++ b/wsgi.py @@ -1,5 +1,12 @@ -from app import create_app +import os +from app import create_app +from credstash import getAllSecrets + + +# On AWS get secrets and export to env, skip this on Cloud Foundry +if os.getenv('VCAP_SERVICES') is None: + os.environ.update(getAllSecrets(region="eu-west-1")) application = create_app() From 9c212e78af2b0e2c17b4375581226660b93f9837 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 31 Jul 2017 11:11:00 +0100 Subject: [PATCH 32/34] Revert "Revert "remove credstash"" This reverts commit de41fde0d37e3f52b316bcda74445c07d04eb28b. --- aws_run_celery.py | 8 +------- db.py | 8 ++------ requirements.txt | 1 - server_commands.py | 5 ----- wsgi.py | 7 ------- 5 files changed, 3 insertions(+), 26 deletions(-) diff --git a/aws_run_celery.py b/aws_run_celery.py index 36a1f480a..8194528db 100644 --- a/aws_run_celery.py +++ b/aws_run_celery.py @@ -1,11 +1,5 @@ #!/usr/bin/env python -from app import notify_celery, create_app -from credstash import getAllSecrets -import os - -# On AWS get secrets and export to env, skip this on Cloud Foundry -if os.getenv('VCAP_SERVICES') is None: - os.environ.update(getAllSecrets(region="eu-west-1")) +from app import create_app application = create_app("delivery") application.app_context().push() diff --git a/db.py b/db.py index 3a8da01d4..bc558cbd2 100644 --- a/db.py +++ b/db.py @@ -1,12 +1,8 @@ from flask.ext.script import Manager, Server from flask_migrate import Migrate, MigrateCommand -from app import create_app, db -from credstash import getAllSecrets -import os -# On AWS get secrets and export to env, skip this on Cloud Foundry -if os.getenv('VCAP_SERVICES') is None: - os.environ.update(getAllSecrets(region="eu-west-1")) +from app import create_app, db + application = create_app() diff --git a/requirements.txt b/requirements.txt index 0c9b3f1e0..61390b6c4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,6 @@ marshmallow==2.4.2 marshmallow-sqlalchemy==0.8.0 flask-marshmallow==0.6.2 Flask-Bcrypt==0.6.2 -credstash==1.8.0 boto3==1.4.4 celery==3.1.25 # pyup: <4 monotonic==1.2 diff --git a/server_commands.py b/server_commands.py index 85bf13137..af071db47 100644 --- a/server_commands.py +++ b/server_commands.py @@ -1,7 +1,6 @@ from flask.ext.script import Manager, Server from flask_migrate import Migrate, MigrateCommand from app import (create_app, db, commands) -from credstash import getAllSecrets import os default_env_file = '/home/ubuntu/environment' @@ -11,10 +10,6 @@ if os.path.isfile(default_env_file): with open(default_env_file, 'r') as environment_file: environment = environment_file.readline().strip() -# On AWS get secrets and export to env, skip this on Cloud Foundry -if os.getenv('VCAP_SERVICES') is None: - os.environ.update(getAllSecrets(region="eu-west-1")) - from app.config import configs os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment] diff --git a/wsgi.py b/wsgi.py index 2df2c3976..9fbeb28ac 100644 --- a/wsgi.py +++ b/wsgi.py @@ -1,13 +1,6 @@ -import os - from app import create_app -from credstash import getAllSecrets -# On AWS get secrets and export to env, skip this on Cloud Foundry -if os.getenv('VCAP_SERVICES') is None: - os.environ.update(getAllSecrets(region="eu-west-1")) - application = create_app() if __name__ == "__main__": From b5dc7642aa574b60e172142ad88fd0e42d56528d Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 31 Jul 2017 11:12:43 +0100 Subject: [PATCH 33/34] remove aws_run_celery file it's no longer relevant since the switch to PaaS --- aws_run_celery.py | 5 ----- manifest-delivery-base.yml | 14 +++++++------- run_celery.py | 1 + 3 files changed, 8 insertions(+), 12 deletions(-) delete mode 100644 aws_run_celery.py diff --git a/aws_run_celery.py b/aws_run_celery.py deleted file mode 100644 index 8194528db..000000000 --- a/aws_run_celery.py +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env python -from app import create_app - -application = create_app("delivery") -application.app_context().push() diff --git a/manifest-delivery-base.yml b/manifest-delivery-base.yml index 597df9f3f..6fba774ea 100644 --- a/manifest-delivery-base.yml +++ b/manifest-delivery-base.yml @@ -16,39 +16,39 @@ memory: 1G applications: - name: notify-delivery-celery-beat - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery beat --loglevel=INFO + command: scripts/run_app_paas.sh celery -A run_celery.notify_celery beat --loglevel=INFO instances: 1 memory: 128M env: NOTIFY_APP_NAME: delivery-celery-beat - name: notify-delivery-worker-database - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q database-tasks + command: scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q database-tasks env: NOTIFY_APP_NAME: delivery-worker-database - name: notify-delivery-worker-research - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=5 -Q research-mode-tasks + command: scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=5 -Q research-mode-tasks env: NOTIFY_APP_NAME: delivery-worker-research - name: notify-delivery-worker-sender - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q send-tasks,send-sms-tasks,send-email-tasks + command: scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q send-tasks,send-sms-tasks,send-email-tasks env: NOTIFY_APP_NAME: delivery-worker-sender - name: notify-delivery-worker-periodic - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=2 -Q periodic-tasks,statistics-tasks + command: scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=2 -Q periodic-tasks,statistics-tasks instances: 1 env: NOTIFY_APP_NAME: delivery-worker-periodic - name: notify-delivery-worker-priority - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=5 -Q priority-tasks + command: scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=5 -Q priority-tasks env: NOTIFY_APP_NAME: delivery-worker-priority - name: notify-delivery-worker - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q job-tasks,retry-tasks,notify-internal-tasks + command: scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q job-tasks,retry-tasks,notify-internal-tasks env: NOTIFY_APP_NAME: delivery-worker diff --git a/run_celery.py b/run_celery.py index 066735e63..013499615 100644 --- a/run_celery.py +++ b/run_celery.py @@ -1,4 +1,5 @@ #!/usr/bin/env python +# notify_celery is referenced from manifest_delivery_base.yml, and cannot be removed from app import notify_celery, create_app application = create_app('delivery') From 3cb3cf438ec905d7050572d8804310b6b67db3f9 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 31 Jul 2017 13:29:30 +0100 Subject: [PATCH 34/34] remove SEND_COMBINED --- app/config.py | 2 -- manifest-delivery-base.yml | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/config.py b/app/config.py index 5d71c0f41..cfd37fa33 100644 --- a/app/config.py +++ b/app/config.py @@ -22,7 +22,6 @@ class QueueNames(object): PERIODIC = 'periodic-tasks' PRIORITY = 'priority-tasks' DATABASE = 'database-tasks' - SEND_COMBINED = 'send-tasks' SEND_SMS = 'send-sms-tasks' SEND_EMAIL = 'send-email-tasks' RESEARCH_MODE = 'research-mode-tasks' @@ -38,7 +37,6 @@ class QueueNames(object): QueueNames.PRIORITY, QueueNames.PERIODIC, QueueNames.DATABASE, - QueueNames.SEND_COMBINED, QueueNames.SEND_SMS, QueueNames.SEND_EMAIL, QueueNames.RESEARCH_MODE, diff --git a/manifest-delivery-base.yml b/manifest-delivery-base.yml index 6fba774ea..78053a729 100644 --- a/manifest-delivery-base.yml +++ b/manifest-delivery-base.yml @@ -33,7 +33,7 @@ applications: NOTIFY_APP_NAME: delivery-worker-research - name: notify-delivery-worker-sender - command: scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q send-tasks,send-sms-tasks,send-email-tasks + command: scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q send-sms-tasks,send-email-tasks env: NOTIFY_APP_NAME: delivery-worker-sender