diff --git a/app/billing/billing_schemas.py b/app/billing/billing_schemas.py index acc06b5ab..4dd4a273e 100644 --- a/app/billing/billing_schemas.py +++ b/app/billing/billing_schemas.py @@ -20,7 +20,6 @@ def serialize_ft_billing_remove_emails(rows): "chargeable_units": row.chargeable_units, "notifications_sent": row.notifications_sent, "rate": float(row.rate), - "postage": row.postage, "cost": float(row.cost), "free_allowance_used": row.free_allowance_used, "charged_units": row.charged_units, diff --git a/app/commands.py b/app/commands.py index b8fd51cc9..1e22f098a 100644 --- a/app/commands.py +++ b/app/commands.py @@ -244,52 +244,6 @@ def bulk_invite_user_to_service(file_name, service_id, user_id, auth_type, permi file.close() -@notify_command(name='populate-notification-postage') -@click.option( - '-s', - '--start_date', - default=datetime(2017, 2, 1), - help="start date inclusive", - type=click_dt(format='%Y-%m-%d') -) -@statsd(namespace="tasks") -def populate_notification_postage(start_date): - current_app.logger.info('populating historical notification postage') - - total_updated = 0 - - while start_date < datetime.utcnow(): - # process in ten day chunks - end_date = start_date + timedelta(days=10) - - sql = \ - """ - UPDATE {} - SET postage = 'second' - WHERE notification_type = 'letter' AND - postage IS NULL AND - created_at BETWEEN :start AND :end - """ - - execution_start = datetime.utcnow() - - if end_date > datetime.utcnow() - timedelta(days=8): - print('Updating notifications table as well') - db.session.execute(sql.format('notifications'), {'start': start_date, 'end': end_date}) - - result = db.session.execute(sql.format('notification_history'), {'start': start_date, 'end': end_date}) - db.session.commit() - - current_app.logger.info('notification postage took {}ms. Migrated {} rows for {} to {}'.format( - datetime.utcnow() - execution_start, result.rowcount, start_date, end_date)) - - start_date += timedelta(days=10) - - total_updated += result.rowcount - - current_app.logger.info('Total inserted/updated records = {}'.format(total_updated)) - - @notify_command(name='archive-jobs-created-between-dates') @click.option('-s', '--start_date', required=True, help="start date inclusive", type=click_dt(format='%Y-%m-%d')) @click.option('-e', '--end_date', required=True, help="end date inclusive", type=click_dt(format='%Y-%m-%d')) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 3175665e1..a813f4f4a 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -14,7 +14,6 @@ from app.dao.date_util import ( from app.dao.organisation_dao import dao_get_organisation_live_services from app.models import ( EMAIL_TYPE, - INTERNATIONAL_POSTAGE_TYPES, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, LETTER_TYPE, @@ -153,24 +152,12 @@ def fetch_letter_costs_and_totals_for_all_services(start_date, end_date): def fetch_letter_line_items_for_all_services(start_date, end_date): - formatted_postage = case( - [(FactBilling.postage.in_(INTERNATIONAL_POSTAGE_TYPES), "international")], else_=FactBilling.postage - ).label("postage") - - postage_order = case( - (formatted_postage == "second", 1), - (formatted_postage == "first", 2), - (formatted_postage == "international", 3), - else_=0 # assumes never get 0 as a result - ) - query = db.session.query( Organisation.name.label("organisation_name"), Organisation.id.label("organisation_id"), Service.name.label("service_name"), Service.id.label("service_id"), FactBilling.rate.label("letter_rate"), - formatted_postage, func.sum(FactBilling.notifications_sent).label("letters_sent"), ).select_from( Service @@ -188,11 +175,9 @@ def fetch_letter_line_items_for_all_services(start_date, end_date): Service.id, Service.name, FactBilling.rate, - formatted_postage ).order_by( Organisation.name, Service.name, - postage_order, FactBilling.rate, ) return query.all() @@ -244,20 +229,19 @@ def fetch_billing_totals_for_year(service_id, year): def fetch_monthly_billing_for_year(service_id, year): """ - Returns a row for each distinct rate, notification_type, postage and month + Returns a row for each distinct rate, notification_type, and month from ft_billing over the specified financial year e.g. ( rate=0.0165, notification_type=sms, - postage=none, month=2022-04-01 00:00:00, notifications_sent=123, ... ) - The "postage" field is "none" except for letters. Each subquery takes care - of anything specific to the notification type e.g. rate multipliers for SMS. + Each subquery takes care of anything specific to the notification type e.g. + rate multipliers for SMS. Since the data in ft_billing is only refreshed once a day for all services, we also update the table on-the-fly if we need accurate data for this year. @@ -276,7 +260,6 @@ def fetch_monthly_billing_for_year(service_id, year): db.session.query( query.c.rate.label("rate"), query.c.notification_type.label("notification_type"), - query.c.postage.label("postage"), func.date_trunc('month', query.c.local_date).cast(Date).label("month"), func.sum(query.c.notifications_sent).label("notifications_sent"), @@ -287,7 +270,6 @@ def fetch_monthly_billing_for_year(service_id, year): ).group_by( query.c.rate, query.c.notification_type, - query.c.postage, 'month', ) for query in [ @@ -308,7 +290,6 @@ def query_service_email_usage_for_year(service_id, year): return db.session.query( FactBilling.local_date, - FactBilling.postage, # should always be "none" FactBilling.notifications_sent, FactBilling.billable_units.label("chargeable_units"), FactBilling.rate, @@ -329,7 +310,6 @@ def query_service_letter_usage_for_year(service_id, year): return db.session.query( FactBilling.local_date, - FactBilling.postage, FactBilling.notifications_sent, # We can't use billable_units here as it represents the # sheet count for letters, which is already accounted for @@ -410,7 +390,6 @@ def query_service_sms_usage_for_year(service_id, year): return db.session.query( FactBilling.local_date, - FactBilling.postage, # should always be "none" FactBilling.notifications_sent, this_rows_chargeable_units.label("chargeable_units"), FactBilling.rate, @@ -477,7 +456,6 @@ def _query_for_billing_data(notification_type, start_date, end_date, service): literal(0).label('rate_multiplier'), literal(False).label('international'), literal(None).label('letter_page_count'), - literal('none').label('postage'), literal(0).label('billable_units'), func.count().label('notifications_sent'), ).filter( @@ -504,7 +482,6 @@ def _query_for_billing_data(notification_type, start_date, end_date, service): rate_multiplier.label('rate_multiplier'), international.label('international'), literal(None).label('letter_page_count'), - literal('none').label('postage'), func.sum(NotificationAllTimeView.billable_units).label('billable_units'), func.count().label('notifications_sent'), ).filter( @@ -523,7 +500,6 @@ def _query_for_billing_data(notification_type, start_date, end_date, service): def _letter_query(): rate_multiplier = func.coalesce(NotificationAllTimeView.rate_multiplier, 1).cast(Integer) - postage = func.coalesce(NotificationAllTimeView.postage, 'none') return db.session.query( NotificationAllTimeView.template_id, literal(service.crown).label('crown'), @@ -533,7 +509,6 @@ def _query_for_billing_data(notification_type, start_date, end_date, service): rate_multiplier.label('rate_multiplier'), NotificationAllTimeView.international, NotificationAllTimeView.billable_units.label('letter_page_count'), - postage.label('postage'), func.sum(NotificationAllTimeView.billable_units).label('billable_units'), func.count().label('notifications_sent'), ).filter( @@ -547,7 +522,6 @@ def _query_for_billing_data(notification_type, start_date, end_date, service): NotificationAllTimeView.template_id, rate_multiplier, NotificationAllTimeView.billable_units, - postage, NotificationAllTimeView.international ) @@ -616,8 +590,7 @@ def update_fact_billing(data, process_day): data.notification_type, process_day, data.crown, - data.letter_page_count, - data.postage) + data.letter_page_count) billing_record = create_billing_record(data, rate, process_day) table = FactBilling.__table__ @@ -638,7 +611,6 @@ def update_fact_billing(data, process_day): billable_units=billing_record.billable_units, notifications_sent=billing_record.notifications_sent, rate=billing_record.rate, - postage=billing_record.postage, ) stmt = stmt.on_conflict_do_update( @@ -664,7 +636,6 @@ def create_billing_record(data, rate, process_day): billable_units=data.billable_units, notifications_sent=data.notifications_sent, rate=rate, - postage=data.postage, ) return billing_record diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index f04ba73ce..cf75ba54b 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -298,7 +298,7 @@ def insert_notification_history_delete_notifications( SELECT id, job_id, job_row_number, service_id, template_id, template_version, api_key_id, key_type, notification_type, created_at, sent_at, sent_by, updated_at, reference, billable_units, client_reference, international, phone_prefix, rate_multiplier, notification_status, - created_by_id, postage, document_download_count + created_by_id, document_download_count FROM notifications WHERE service_id = :service_id AND notification_type = :notification_type @@ -311,7 +311,7 @@ def insert_notification_history_delete_notifications( SELECT id, job_id, job_row_number, service_id, template_id, template_version, api_key_id, key_type, notification_type, created_at, sent_at, sent_by, updated_at, reference, billable_units, client_reference, international, phone_prefix, rate_multiplier, notification_status, - created_by_id, postage, document_download_count + created_by_id, document_download_count FROM notifications WHERE service_id = :service_id AND notification_type = :notification_type @@ -569,25 +569,6 @@ def notifications_not_yet_sent(should_be_sending_after_seconds, notification_typ return notifications -def dao_get_letters_and_sheets_volume_by_postage(print_run_deadline): - notifications = db.session.query( - func.count(Notification.id).label('letters_count'), - func.sum(Notification.billable_units).label('sheets_count'), - Notification.postage - ).filter( - Notification.created_at < convert_local_timezone_to_utc(print_run_deadline), - Notification.notification_type == LETTER_TYPE, - Notification.status == NOTIFICATION_CREATED, - Notification.key_type == KEY_TYPE_NORMAL, - Notification.billable_units > 0 - ).group_by( - Notification.postage - ).order_by( - Notification.postage - ).all() - return notifications - - def _duplicate_update_warning(notification, status): current_app.logger.info( ( diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index 1b6d346d3..f6d5da06a 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -58,7 +58,6 @@ def dao_update_template_reply_to(template_id, reply_to): "content": template.content, "service_id": template.service_id, "subject": template.subject, - "postage": template.postage, "created_by_id": template.created_by_id, "version": template.version, "archived": template.archived, diff --git a/app/models.py b/app/models.py index bfd6339a2..5a3caee33 100644 --- a/app/models.py +++ b/app/models.py @@ -906,7 +906,6 @@ class TemplateBase(db.Model): archived = db.Column(db.Boolean, nullable=False, default=False) hidden = db.Column(db.Boolean, nullable=False, default=False) subject = db.Column(db.Text) - postage = db.Column(db.String, nullable=True) @declared_attr def service_id(cls): @@ -1007,7 +1006,6 @@ class TemplateBase(db.Model): } for key in self._as_utils_template().placeholders }, - "postage": self.postage, "letter_contact_block": self.service_letter_contact.contact_block if self.service_letter_contact else None, } @@ -1338,19 +1336,11 @@ NOTIFICATION_STATUS_TYPES_ENUM = db.Enum(*NOTIFICATION_STATUS_TYPES, name='notif NOTIFICATION_STATUS_LETTER_ACCEPTED = 'accepted' NOTIFICATION_STATUS_LETTER_RECEIVED = 'received' +# TODO: delete these keywords for postage FIRST_CLASS = 'first' SECOND_CLASS = 'second' EUROPE = 'europe' REST_OF_WORLD = 'rest-of-world' -POSTAGE_TYPES = [FIRST_CLASS, SECOND_CLASS, EUROPE, REST_OF_WORLD] -UK_POSTAGE_TYPES = [FIRST_CLASS, SECOND_CLASS] -INTERNATIONAL_POSTAGE_TYPES = [EUROPE, REST_OF_WORLD] -RESOLVE_POSTAGE_FOR_FILE_NAME = { - FIRST_CLASS: 1, - SECOND_CLASS: 2, - EUROPE: 'E', - REST_OF_WORLD: 'N', -} class NotificationStatusTypes(db.Model): @@ -1367,6 +1357,10 @@ class NotificationAllTimeView(db.Model): """ __tablename__ = 'notifications_all_time_view' + # Tell alembic not to create this as a table. We have a migration where we manually set this up as a view. + # This is custom logic we apply - not built-in logic. See `migrations/env.py` + __table_args__ = {"info": {"managed_by_alembic": False}} + id = db.Column(UUID(as_uuid=True), primary_key=True) job_id = db.Column(UUID(as_uuid=True)) job_row_number = db.Column(db.Integer) @@ -1388,7 +1382,6 @@ class NotificationAllTimeView(db.Model): phone_prefix = db.Column(db.String) rate_multiplier = db.Column(db.Numeric(asdecimal=False)) created_by_id = db.Column(UUID(as_uuid=True)) - postage = db.Column(db.String) document_download_count = db.Column(db.Integer) @@ -1451,7 +1444,6 @@ class Notification(db.Model): document_download_count = db.Column(db.Integer, nullable=True) - postage = db.Column(db.String, nullable=True) provider_response = db.Column(db.Text, nullable=True) # queue_name = db.Column(db.Text, nullable=True) @@ -1668,29 +1660,8 @@ class Notification(db.Model): "sent_at": get_dt_string_or_none(self.sent_at), "completed_at": self.completed_at(), "scheduled_for": None, - "postage": self.postage } - if self.notification_type == LETTER_TYPE: - personalisation = InsensitiveDict(self.personalisation) - - ( - serialized['line_1'], - serialized['line_2'], - serialized['line_3'], - serialized['line_4'], - serialized['line_5'], - serialized['line_6'], - serialized['postcode'], - ) = ( - personalisation.get(line) for line in address_lines_1_to_6_and_postcode_keys - ) - - serialized['estimated_delivery'] = \ - get_letter_timings(serialized['created_at'], postage=self.postage)\ - .earliest_delivery\ - .strftime(DATETIME_FORMAT) - return serialized @@ -1731,8 +1702,6 @@ class NotificationHistory(db.Model, HistoryModel): created_by_id = db.Column(UUID(as_uuid=True), nullable=True) - postage = db.Column(db.String, nullable=True) - document_download_count = db.Column(db.Integer, nullable=True) __table_args__ = ( @@ -2055,7 +2024,6 @@ class FactBilling(db.Model): rate_multiplier = db.Column(db.Integer(), nullable=False, primary_key=True) international = db.Column(db.Boolean, nullable=False, primary_key=True) rate = db.Column(db.Numeric(), nullable=False, primary_key=True) - postage = db.Column(db.String, nullable=False, primary_key=True) billable_units = db.Column(db.Integer(), nullable=True) notifications_sent = db.Column(db.Integer(), nullable=True) created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) diff --git a/app/notifications/process_letter_notifications.py b/app/notifications/process_letter_notifications.py index 3fe7e7cbf..ba99b4805 100644 --- a/app/notifications/process_letter_notifications.py +++ b/app/notifications/process_letter_notifications.py @@ -14,7 +14,6 @@ def create_letter_notification( reply_to_text=None, billable_units=None, updated_at=None, - postage=None ): notification = persist_notification( template_id=template.id, @@ -33,9 +32,6 @@ def create_letter_notification( status=status, reply_to_text=reply_to_text, billable_units=billable_units, - # letter_data.get('postage') is only set for precompiled letters - # letters from a template will pass in 'europe' or 'rest-of-world' if None then use postage from template - postage=postage or letter_data.get('postage') or template.postage, updated_at=updated_at ) return notification diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 0e120ed21..1afd7b90b 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -24,7 +24,6 @@ from app.dao.notifications_dao import ( ) from app.models import ( EMAIL_TYPE, - INTERNATIONAL_POSTAGE_TYPES, KEY_TYPE_TEST, LETTER_TYPE, NOTIFICATION_CREATED, @@ -100,7 +99,6 @@ def persist_notification( status=NOTIFICATION_CREATED, reply_to_text=None, billable_units=None, - postage=None, document_download_count=None, updated_at=None ): @@ -148,10 +146,6 @@ def persist_notification( current_app.logger.info('Persisting notification with type: {}'.format(EMAIL_TYPE)) notification.normalised_to = format_email_address(notification.to) current_app.logger.info('Persisting notification to formatted email: {}'.format(notification.normalised_to)) - elif notification_type == LETTER_TYPE: - notification.postage = postage - notification.international = postage in INTERNATIONAL_POSTAGE_TYPES - notification.normalised_to = ''.join(notification.to.split()).lower() # if simulated create a Notification model to return but do not persist the Notification to the dB if not simulated: diff --git a/app/notifications/rest.py b/app/notifications/rest.py index d90ef425d..2dc42da50 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -111,7 +111,6 @@ def send_notification(notification_type): simulated = simulated_recipient(notification_form['to'], notification_type) notification_model = persist_notification(template_id=template.id, template_version=template.version, - postage=template.postage, recipient=request.get_json()['to'], service=authenticated_service, personalisation=notification_form.get('personalisation', None), diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 7a079df66..62f3247d2 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -240,34 +240,3 @@ def check_service_letter_contact_id(service_id, letter_contact_id, notification_ message = 'letter_contact_id {} does not exist in database for service id {}' \ .format(letter_contact_id, service_id) raise BadRequestError(message=message) - - -def validate_address(service, letter_data): - address = PostalAddress.from_personalisation( - letter_data, - allow_international_letters=(INTERNATIONAL_LETTERS in str(service.permissions)), - ) - if not address.has_enough_lines: - raise ValidationError( - message=f'Address must be at least {PostalAddress.MIN_LINES} lines' - ) - if address.has_too_many_lines: - raise ValidationError( - message=f'Address must be no more than {PostalAddress.MAX_LINES} lines' - ) - if not address.has_valid_last_line: - if address.allow_international_letters: - raise ValidationError( - message='Last line of address must be a real UK postcode or another country' - ) - raise ValidationError( - message='Must be a real UK postcode' - ) - if address.has_invalid_characters: - raise ValidationError( - message='Address lines must not start with any of the following characters: @ ( ) = [ ] ” \\ / , < >' - ) - if address.international: - return address.postage - else: - return None diff --git a/app/platform_stats/rest.py b/app/platform_stats/rest.py index ff5a44b28..6a4cea871 100644 --- a/app/platform_stats/rest.py +++ b/app/platform_stats/rest.py @@ -16,7 +16,6 @@ from app.dao.fact_notification_status_dao import ( fetch_notification_status_totals_for_all_services, ) from app.errors import InvalidRequest, register_errors -from app.models import UK_POSTAGE_TYPES from app.platform_stats.platform_stats_schema import platform_stats_request from app.schema_validation import validate from app.service.statistics import format_admin_stats @@ -76,13 +75,8 @@ def get_data_for_billing_report(): sms_costs = fetch_sms_billing_for_all_services(start_date, end_date) letter_overview = fetch_letter_costs_and_totals_for_all_services(start_date, end_date) - letter_breakdown = fetch_letter_line_items_for_all_services(start_date, end_date) - lb_by_service = [ - (lb.service_id, - f"{lb.letters_sent} {postage_description(lb.postage)} letters at {format_letter_rate(lb.letter_rate)}") - for lb in letter_breakdown - ] + combined = {} for s in sms_costs: if float(s.sms_cost) > 0: @@ -95,7 +89,6 @@ def get_data_for_billing_report(): "sms_chargeable_units": s.chargeable_billable_sms, "total_letters": 0, "letter_cost": 0, - "letter_breakdown": "" } combined[s.service_id] = entry @@ -115,12 +108,9 @@ def get_data_for_billing_report(): "sms_chargeable_units": 0, "total_letters": data.total_letters, "letter_cost": float(data.letter_cost), - "letter_breakdown": "" } combined[data.service_id] = letter_entry - for service_id, breakdown in lb_by_service: - combined[service_id]['letter_breakdown'] += (breakdown + '\n') - + billing_details = fetch_billing_details_for_all_services() for service in billing_details: if service.service_id in combined: @@ -209,13 +199,6 @@ def volumes_by_service_report(): return jsonify(report) -def postage_description(postage): - if postage in UK_POSTAGE_TYPES: - return f'{postage} class' - else: - return 'international' - - def format_letter_rate(number): if number >= 1: return f"£{number:,.2f}" diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 7e813f4d0..86af11404 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -35,14 +35,6 @@ def validate_schema_email_address(instance): return True -@format_checker.checks('postage', raises=ValidationError) -def validate_schema_postage(instance): - if isinstance(instance, str): - if instance not in ["first", "second", "europe", "rest-of-world"]: - raise ValidationError("invalid. It must be first, second, europe or rest-of-world.") - return True - - @format_checker.checks('datetime_within_next_day', raises=ValidationError) def validate_schema_date_with_hour(instance): if isinstance(instance, str): diff --git a/app/schemas.py b/app/schemas.py index dad38f84b..65de68deb 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -400,7 +400,6 @@ class TemplateSchemaNoDetail(TemplateSchema): 'created_by', 'created_by_id', 'hidden', - 'postage', 'process_type', 'redact_personalisation', 'reply_to', diff --git a/app/serialised_models.py b/app/serialised_models.py index 583c13591..628a12b2d 100644 --- a/app/serialised_models.py +++ b/app/serialised_models.py @@ -41,7 +41,6 @@ class SerialisedTemplate(SerialisedModel): 'archived', 'content', 'id', - 'postage', 'process_type', 'reply_to_text', 'subject', diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 3884f9978..de9ca9645 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -22,7 +22,6 @@ from app.notifications.process_notifications import ( ) from app.notifications.validators import ( check_service_over_daily_message_limit, - validate_address, validate_and_format_recipient, validate_template, ) @@ -65,16 +64,7 @@ def send_one_off_notification(service_id, post_data): notification_type=template.template_type, allow_guest_list_recipients=False, ) - postage = None client_reference = None - if template.template_type == LETTER_TYPE: - # Validate address and set postage to europe|rest-of-world if international letter, - # otherwise persist_notification with use template postage - postage = validate_address(service, personalisation) - if not postage: - postage = template.postage - from app.utils import get_reference_from_personalisation - client_reference = get_reference_from_personalisation(personalisation) validate_created_by(service, post_data['created_by']) @@ -97,7 +87,6 @@ def send_one_off_notification(service_id, post_data): created_by_id=post_data['created_by'], reply_to_text=reply_to, reference=create_one_off_reference(template.template_type), - postage=postage, client_reference=client_reference ) diff --git a/app/template/rest.py b/app/template/rest.py index 9280e7dc9..47e6e38c0 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -199,7 +199,7 @@ def get_template_versions(service_id, template_id): def _template_has_not_changed(current_data, updated_template): return all( current_data[key] == updated_template[key] - for key in ('name', 'content', 'subject', 'archived', 'process_type', 'postage') + for key in ('name', 'content', 'subject', 'archived', 'process_type') ) diff --git a/app/template/template_schemas.py b/app/template/template_schemas.py index 72edb1cfa..932728b18 100644 --- a/app/template/template_schemas.py +++ b/app/template/template_schemas.py @@ -15,7 +15,6 @@ post_create_template_schema = { "subject": {"type": "string"}, "created_by": uuid, "parent_folder_id": uuid, - "postage": {"type": "string", "format": "postage"}, }, "if": { "properties": { @@ -39,7 +38,6 @@ post_update_template_schema = { "process_type": {"enum": TEMPLATE_PROCESS_TYPE}, "content": {"type": "string"}, "subject": {"type": "string"}, - "postage": {"type": "string", "format": "postage"}, "reply_to": nullable_uuid, "created_by": uuid, "archived": {"type": "boolean"}, diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 5d1d923b4..f66b9fd08 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -239,7 +239,6 @@ post_precompiled_letter_request = { "properties": { "reference": {"type": "string"}, "content": {"type": "string"}, - "postage": {"type": "string", "format": "postage"} }, "required": ["reference", "content"], "additionalProperties": False diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 1ade33819..fed503912 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -43,7 +43,6 @@ from app.notifications.validators import ( check_service_email_reply_to_id, check_service_has_permission, check_service_sms_sender_id, - validate_address, validate_and_format_recipient, validate_template, ) @@ -326,8 +325,6 @@ def process_letter_notification( if not service.research_mode and service.restricted and api_key.key_type != KEY_TYPE_TEST: raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403) - postage = validate_address(service, letter_data['personalisation']) - test_key = api_key.key_type == KEY_TYPE_TEST status = NOTIFICATION_CREATED @@ -347,8 +344,7 @@ def process_letter_notification( api_key=api_key, status=status, reply_to_text=reply_to_text, - updated_at=updated_at, - postage=postage + updated_at=updated_at ) resp = create_response_for_post_notification( diff --git a/migrations/versions/0384_remove_letter_branding_.py b/migrations/versions/0384_remove_letter_branding_.py index 688224222..fd390dfc6 100644 --- a/migrations/versions/0384_remove_letter_branding_.py +++ b/migrations/versions/0384_remove_letter_branding_.py @@ -1,6 +1,6 @@ """ -Revision ID: 1358d15190ba +Revision ID: 0384_remove_letter_branding_ Revises: 0383_update_default_templates.py Create Date: 2023-02-09 22:24:07.187569 @@ -9,7 +9,7 @@ from alembic import op import sqlalchemy as sa from sqlalchemy.dialects import postgresql -revision = '1358d15190ba' +revision = '0384_remove_letter_branding_' down_revision = '0383_update_default_templates.py' diff --git a/migrations/versions/0385_remove postage_.py b/migrations/versions/0385_remove postage_.py new file mode 100644 index 000000000..6c1adcb53 --- /dev/null +++ b/migrations/versions/0385_remove postage_.py @@ -0,0 +1,150 @@ +""" + +Revision ID: 0385_remove postage_.py +Revises: 0384_remove_letter_branding_ +Create Date: 2023-02-10 12:20:39.411493 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0385_remove postage_.py' +down_revision = '0384_remove_letter_branding_' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + # we need to replace the entire notifications_all_time_view in order to update it + op.execute("DROP VIEW notifications_all_time_view;") + op.execute(""" + CREATE VIEW notifications_all_time_view AS + ( + SELECT + id, + job_id, + job_row_number, + service_id, + template_id, + template_version, + api_key_id, + key_type, + billable_units, + notification_type, + created_at, + sent_at, + sent_by, + updated_at, + notification_status, + reference, + client_reference, + international, + phone_prefix, + rate_multiplier, + created_by_id, + document_download_count + FROM notifications + ) UNION + ( + SELECT + id, + job_id, + job_row_number, + service_id, + template_id, + template_version, + api_key_id, + key_type, + billable_units, + notification_type, + created_at, + sent_at, + sent_by, + updated_at, + notification_status, + reference, + client_reference, + international, + phone_prefix, + rate_multiplier, + created_by_id, + document_download_count + FROM notification_history + ) + """) + + op.drop_column('notification_history', 'postage') + op.drop_column('notifications', 'postage') + op.drop_column('templates', 'postage') + op.drop_column('templates_history', 'postage') + op.drop_column('ft_billing', 'postage') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('ft_billing', sa.Column('postage', sa.VARCHAR(), autoincrement=False, nullable=True)) + op.add_column('templates_history', sa.Column('postage', sa.VARCHAR(), autoincrement=False, nullable=True)) + op.add_column('templates', sa.Column('postage', sa.VARCHAR(), autoincrement=False, nullable=True)) + op.add_column('notifications', sa.Column('postage', sa.VARCHAR(), autoincrement=False, nullable=True)) + op.add_column('notification_history', sa.Column('postage', sa.VARCHAR(), autoincrement=False, nullable=True)) + + op.execute("DROP VIEW notifications_all_time_view;") + op.execute(""" + CREATE VIEW notifications_all_time_view AS + ( + SELECT + id, + job_id, + job_row_number, + service_id, + template_id, + template_version, + api_key_id, + key_type, + billable_units, + notification_type, + created_at, + sent_at, + sent_by, + updated_at, + notification_status, + reference, + client_reference, + international, + phone_prefix, + rate_multiplier, + postage, + created_by_id, + document_download_count + FROM notifications + ) UNION + ( + SELECT + id, + job_id, + job_row_number, + service_id, + template_id, + template_version, + api_key_id, + key_type, + billable_units, + notification_type, + created_at, + sent_at, + sent_by, + updated_at, + notification_status, + reference, + client_reference, + international, + phone_prefix, + rate_multiplier, + postage, + created_by_id, + document_download_count + FROM notification_history + ) + """) + # ### end Alembic commands ### diff --git a/tests/app/billing/test_rest.py b/tests/app/billing/test_rest.py index b299ff8df..06ae617d9 100644 --- a/tests/app/billing/test_rest.py +++ b/tests/app/billing/test_rest.py @@ -132,7 +132,6 @@ def test_get_yearly_usage_by_monthly_from_ft_billing(admin_request, notify_db_se for dt in (date(2016, 4, 28), date(2016, 11, 10), date(2017, 2, 26)): create_ft_billing(local_date=dt, template=sms_template, rate=0.0162) create_ft_billing(local_date=dt, template=email_template, billable_unit=0, rate=0) - create_ft_billing(local_date=dt, template=letter_template, rate=0.33, postage='second') json_response = admin_request.get( 'billing.get_yearly_usage_by_monthly_from_ft_billing', @@ -153,7 +152,6 @@ def test_get_yearly_usage_by_monthly_from_ft_billing(admin_request, notify_db_se assert letter_row["chargeable_units"] == 1 assert letter_row["notifications_sent"] == 1 assert letter_row["rate"] == 0.33 - assert letter_row["postage"] == "second" assert letter_row["cost"] == 0.33 assert letter_row["free_allowance_used"] == 0 assert letter_row["charged_units"] == 1 @@ -163,7 +161,6 @@ def test_get_yearly_usage_by_monthly_from_ft_billing(admin_request, notify_db_se assert sms_row["chargeable_units"] == 1 assert sms_row["notifications_sent"] == 1 assert sms_row["rate"] == 0.0162 - assert sms_row["postage"] == "none" # free allowance is 1 assert sms_row["cost"] == 0 assert sms_row["free_allowance_used"] == 1 @@ -203,7 +200,6 @@ def test_get_yearly_billing_usage_summary_from_ft_billing(admin_request, notify_ for dt in (date(2016, 4, 28), date(2016, 11, 10), date(2017, 2, 26)): create_ft_billing(local_date=dt, template=sms_template, rate=0.0162) create_ft_billing(local_date=dt, template=email_template, billable_unit=0, rate=0) - create_ft_billing(local_date=dt, template=letter_template, rate=0.33, postage='second') json_response = admin_request.get( 'billing.get_yearly_billing_usage_summary_from_ft_billing', diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index 5843132bb..8e7c5c2cc 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -289,116 +289,6 @@ def test_create_nightly_billing_for_day_different_sent_by( assert record.rate_multiplier == 1.0 -@pytest.mark.skip(reason="Needs updating for TTS: Remove mail") -def test_create_nightly_billing_for_day_different_letter_postage( - notify_db_session, - sample_letter_template, - mocker -): - yesterday = datetime.now() - timedelta(days=1) - mocker.patch('app.dao.fact_billing_dao.get_rate', side_effect=mocker_get_rate) - - for _ in range(2): - create_notification( - created_at=yesterday, - template=sample_letter_template, - status='delivered', - sent_by='dvla', - billable_units=2, - postage='first' - ) - create_notification( - created_at=yesterday, - template=sample_letter_template, - status='delivered', - sent_by='dvla', - billable_units=2, - postage='second' - ) - create_notification( - created_at=yesterday, - template=sample_letter_template, - status='delivered', - sent_by='dvla', - billable_units=1, - postage='europe' - ) - create_notification( - created_at=yesterday, - template=sample_letter_template, - status='delivered', - sent_by='dvla', - billable_units=3, - postage='rest-of-world' - ) - - records = FactBilling.query.all() - assert len(records) == 0 - create_nightly_billing_for_day(str(yesterday.date())) - - records = FactBilling.query.order_by('postage').all() - assert len(records) == 4 - - assert records[0].notification_type == LETTER_TYPE - assert records[0].local_date == datetime.date(yesterday) - assert records[0].postage == 'europe' - assert records[0].notifications_sent == 1 - assert records[0].billable_units == 1 - - assert records[1].notification_type == LETTER_TYPE - assert records[1].local_date == datetime.date(yesterday) - assert records[1].postage == 'first' - assert records[1].notifications_sent == 2 - assert records[1].billable_units == 4 - - assert records[2].notification_type == LETTER_TYPE - assert records[2].local_date == datetime.date(yesterday) - assert records[2].postage == 'rest-of-world' - assert records[2].notifications_sent == 1 - assert records[2].billable_units == 3 - - assert records[3].notification_type == LETTER_TYPE - assert records[3].local_date == datetime.date(yesterday) - assert records[3].postage == 'second' - assert records[3].notifications_sent == 1 - assert records[3].billable_units == 2 - - -@pytest.mark.skip(reason="Needs updating for TTS: Timezone handling") -def test_create_nightly_billing_for_day_letter( - sample_service, - sample_letter_template, - mocker -): - yesterday = datetime.now() - timedelta(days=1) - - mocker.patch('app.dao.fact_billing_dao.get_rate', side_effect=mocker_get_rate) - - create_notification( - created_at=yesterday, - template=sample_letter_template, - status='delivered', - sent_by='dvla', - international=False, - rate_multiplier=2.0, - billable_units=2, - ) - - records = FactBilling.query.all() - assert len(records) == 0 - create_nightly_billing_for_day(str(yesterday.date())) - - records = FactBilling.query.order_by('rate_multiplier').all() - assert len(records) == 1 - - record = records[0] - assert record.notification_type == LETTER_TYPE - assert record.local_date == datetime.date(yesterday) - assert record.rate == Decimal(2.1) - assert record.billable_units == 2 - assert record.rate_multiplier == 2.0 - - @pytest.mark.skip(reason="Needs updating for TTS: Timezone handling") def test_create_nightly_billing_for_day_null_sent_by_sms( sample_service, diff --git a/tests/app/conftest.py b/tests/app/conftest.py index ba4a6419b..9f9ad6bd7 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -86,7 +86,6 @@ def create_sample_notification( rate_multiplier=1.0, scheduled_for=None, normalised_to=None, - postage=None, ): if created_at is None: created_at = datetime.utcnow() @@ -133,7 +132,6 @@ def create_sample_notification( "client_reference": client_reference, "rate_multiplier": rate_multiplier, "normalised_to": normalised_to, - "postage": postage, } if job_row_number is not None: data["job_row_number"] = job_row_number @@ -308,7 +306,7 @@ def sample_template_without_email_permission(notify_db_session): @pytest.fixture def sample_letter_template(sample_service_full_permissions): - return create_template(sample_service_full_permissions, template_type=LETTER_TYPE, postage='second') + return create_template(sample_service_full_permissions, template_type=LETTER_TYPE) @pytest.fixture @@ -486,7 +484,6 @@ def sample_notification(notify_db_session): 'client_reference': None, 'rate_multiplier': 1.0, 'normalised_to': None, - 'postage': None, } notification = Notification(**data) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 4a6a1324f..729ab25f7 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -11,7 +11,6 @@ from app.dao.notifications_dao import ( dao_create_notification, dao_delete_notifications_by_id, dao_get_last_notification_added_for_job_id, - dao_get_letters_and_sheets_volume_by_postage, dao_get_notification_by_reference, dao_get_notification_count_for_job_id, dao_get_notification_history_by_reference, @@ -111,25 +110,6 @@ def test_should_update_status_by_id_if_created(sample_template, sample_notificat assert updated.status == 'failed' -def test_should_update_status_by_id_if_pending_virus_check(sample_letter_template): - notification = create_notification(template=sample_letter_template, status='pending-virus-check') - assert Notification.query.get(notification.id).status == 'pending-virus-check' - updated = update_notification_status_by_id(notification.id, 'cancelled') - assert Notification.query.get(notification.id).status == 'cancelled' - assert updated.status == 'cancelled' - - -def test_should_update_status_of_international_letter_to_cancelled(sample_letter_template): - notification = create_notification( - template=sample_letter_template, - international=True, - postage='europe', - ) - assert Notification.query.get(notification.id).international is True - update_notification_status_by_id(notification.id, 'cancelled') - assert Notification.query.get(notification.id).status == 'cancelled' - - def test_should_update_status_by_id_and_set_sent_by(sample_template): notification = create_notification(template=sample_template, status='sending') @@ -1562,38 +1542,6 @@ def test_notifications_not_yet_sent_return_no_rows(sample_service, notification_ assert len(results) == 0 -def test_dao_get_letters_and_sheets_volume_by_postage(notify_db_session): - first_service = create_service(service_name='first service', service_id='3a5cea08-29fd-4bb9-b582-8dedd928b149') - second_service = create_service(service_name='second service', service_id='642bf33b-54b5-45f2-8c13-942a46616704') - first_template = create_template(service=first_service, template_type='letter', postage='second') - second_template = create_template(service=second_service, template_type='letter', postage='second') - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 9, 30), postage='first') - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 12, 30), postage='europe') - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 13, 30), postage='rest-of-world') - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 14, 30), billable_units=3) - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 14, 30), billable_units=0) - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 15, 30)) - create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 30), postage='first') - create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 31), postage='first') - create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 32)) - create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 33)) - create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 34)) - - results = dao_get_letters_and_sheets_volume_by_postage(print_run_deadline=datetime(2020, 12, 1, 17, 30)) - - assert len(results) == 4 - - expected_results = [ - {'letters_count': 1, 'sheets_count': 1, 'postage': 'europe'}, - {'letters_count': 3, 'sheets_count': 3, 'postage': 'first'}, - {'letters_count': 1, 'sheets_count': 1, 'postage': 'rest-of-world'}, - {'letters_count': 5, 'sheets_count': 7, 'postage': 'second'} - ] - - for result in results: - assert result._asdict() in expected_results - - @pytest.mark.parametrize('created_at_utc,date_to_check,expected_count', [ # Clocks change on the 27th of March 2022, so the query needs to look at the # time range 00:00 - 23:00 (UTC) thereafter. diff --git a/tests/app/dao/test_fact_billing_dao.py b/tests/app/dao/test_fact_billing_dao.py index f9383cd39..984bd7cc7 100644 --- a/tests/app/dao/test_fact_billing_dao.py +++ b/tests/app/dao/test_fact_billing_dao.py @@ -44,21 +44,18 @@ def set_up_yearly_data(): service = create_service() sms_template = create_template(service=service, template_type="sms") email_template = create_template(service=service, template_type="email") - letter_template = create_template(service=service, template_type="letter") # use different rates for adjacent financial years to make sure the query # doesn't accidentally bleed over into them for dt in (date(2016, 3, 31), date(2017, 4, 1)): create_ft_billing(local_date=dt, template=sms_template, rate=0.163) create_ft_billing(local_date=dt, template=email_template, rate=0, billable_unit=0) - create_ft_billing(local_date=dt, template=letter_template, rate=0.31, postage='second') # a selection of dates that represent the extreme ends of the financial year # and some arbitrary dates in between for dt in (date(2016, 4, 1), date(2016, 4, 29), date(2017, 2, 6), date(2017, 3, 31)): create_ft_billing(local_date=dt, template=sms_template, rate=0.162) create_ft_billing(local_date=dt, template=email_template, rate=0, billable_unit=0) - create_ft_billing(local_date=dt, template=letter_template, rate=0.30, postage='second') return service @@ -66,20 +63,17 @@ def set_up_yearly_data(): def set_up_yearly_data_variable_rates(): service = create_service() sms_template = create_template(service=service, template_type="sms") - letter_template = create_template(service=service, template_type="letter") create_ft_billing(local_date='2018-05-16', template=sms_template, rate=0.162) create_ft_billing(local_date='2018-05-17', template=sms_template, rate_multiplier=2, rate=0.0150, billable_unit=2) create_ft_billing(local_date='2018-05-16', template=sms_template, rate_multiplier=2, rate=0.162, billable_unit=2) - create_ft_billing(local_date='2018-05-16', template=letter_template, rate=0.33, postage='second') create_ft_billing( local_date='2018-05-17', - template=letter_template, + template=sms_template, rate=0.36, notifications_sent=2, billable_unit=4, # 2 pages each - postage='second' ) return service @@ -231,70 +225,13 @@ def test_fetch_billing_data_for_day_is_grouped_by_notification_type(notify_db_se assert len(notification_types) == 3 -def test_fetch_billing_data_for_day_groups_by_postage(notify_db_session): - service = create_service() - letter_template = create_template(service=service, template_type='letter') - email_template = create_template(service=service, template_type='email') - create_notification(template=letter_template, status='delivered', postage='first') - create_notification(template=letter_template, status='delivered', postage='first') - create_notification(template=letter_template, status='delivered', postage='second') - create_notification(template=letter_template, status='delivered', postage='europe') - create_notification(template=letter_template, status='delivered', postage='rest-of-world') - create_notification(template=email_template, status='delivered') - - today = convert_utc_to_local_timezone(datetime.utcnow()) - results = fetch_billing_data_for_day(today.date()) - assert len(results) == 5 - - -def test_fetch_billing_data_for_day_groups_by_sent_by(notify_db_session): - service = create_service() - letter_template = create_template(service=service, template_type='letter') - email_template = create_template(service=service, template_type='email') - create_notification(template=letter_template, status='delivered', postage='second', sent_by='dvla') - create_notification(template=letter_template, status='delivered', postage='second', sent_by='dvla') - create_notification(template=letter_template, status='delivered', postage='second', sent_by=None) - create_notification(template=email_template, status='delivered') - - today = convert_utc_to_local_timezone(datetime.utcnow()) - results = fetch_billing_data_for_day(today.date()) - assert len(results) == 2 - - -def test_fetch_billing_data_for_day_groups_by_page_count(notify_db_session): - service = create_service() - letter_template = create_template(service=service, template_type='letter') - email_template = create_template(service=service, template_type='email') - create_notification(template=letter_template, status='delivered', postage='second', billable_units=1) - create_notification(template=letter_template, status='delivered', postage='second', billable_units=1) - create_notification(template=letter_template, status='delivered', postage='second', billable_units=2) - create_notification(template=email_template, status='delivered') - - today = convert_utc_to_local_timezone(datetime.utcnow()) - results = fetch_billing_data_for_day(today.date()) - assert len(results) == 3 - - -def test_fetch_billing_data_for_day_sets_postage_for_emails_and_sms_to_none(notify_db_session): - service = create_service() - sms_template = create_template(service=service, template_type='sms') - email_template = create_template(service=service, template_type='email') - create_notification(template=sms_template, status='delivered') - create_notification(template=email_template, status='delivered') - - today = convert_utc_to_local_timezone(datetime.utcnow()) - results = fetch_billing_data_for_day(today.date()) - assert len(results) == 2 - assert results[0].postage == 'none' - assert results[1].postage == 'none' - - def test_fetch_billing_data_for_day_returns_empty_list(notify_db_session): today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today.date()) assert results == [] +# TODO: ready for reactivation? @pytest.mark.skip(reason="Needs updating for TTS: Timezone handling") def test_fetch_billing_data_for_day_uses_correct_table(notify_db_session): service = create_service() diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index a02fee373..4b8265f3e 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -30,8 +30,6 @@ def test_create_template(sample_service, sample_user, template_type, subject): 'service': sample_service, 'created_by': sample_user } - if template_type == 'letter': - data['postage'] = 'second' if subject: data.update({'subject': subject}) template = Template(**data) @@ -64,7 +62,6 @@ def test_create_template_with_reply_to(sample_service, sample_user): 'service': sample_service, 'created_by': sample_user, 'reply_to': letter_contact.id, - 'postage': 'second' } template = Template(**data) dao_create_template(template) @@ -99,7 +96,6 @@ def test_dao_update_template_reply_to_none_to_some(sample_service, sample_user): 'content': "Template content", 'service': sample_service, 'created_by': sample_user, - 'postage': 'second' } template = Template(**data) dao_create_template(template) @@ -131,7 +127,6 @@ def test_dao_update_template_reply_to_some_to_some(sample_service, sample_user): 'service': sample_service, 'created_by': sample_user, 'service_letter_contact_id': letter_contact.id, - 'postage': 'second', } template = Template(**data) dao_create_template(template) @@ -156,7 +151,6 @@ def test_dao_update_template_reply_to_some_to_none(sample_service, sample_user): 'service': sample_service, 'created_by': sample_user, 'service_letter_contact_id': letter_contact.id, - 'postage': 'second' } template = Template(**data) dao_create_template(template) diff --git a/tests/app/dao/test_uploads_dao.py b/tests/app/dao/test_uploads_dao.py index 97823dd7c..80ba2f3e1 100644 --- a/tests/app/dao/test_uploads_dao.py +++ b/tests/app/dao/test_uploads_dao.py @@ -38,7 +38,6 @@ def create_uploaded_template(service): subject='Pre-compiled PDF', content="", hidden=True, - postage="second", ) diff --git a/tests/app/db.py b/tests/app/db.py index 02207edb9..5f649af00 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -201,7 +201,6 @@ def create_template( hidden=False, archived=False, folder=None, - postage=None, process_type='normal', contact_block_id=None ): @@ -216,10 +215,6 @@ def create_template( 'folder': folder, 'process_type': process_type, } - if template_type == LETTER_TYPE: - data["postage"] = postage or "second" - if contact_block_id: - data['service_letter_contact_id'] = contact_block_id if template_type != SMS_TYPE: data['subject'] = subject template = Template(**data) @@ -255,7 +250,6 @@ def create_notification( one_off=False, reply_to_text=None, created_by_id=None, - postage=None, document_download_count=None, ): assert job or template @@ -278,9 +272,6 @@ def create_notification( if not api_key: api_key = create_api_key(template.service, key_type=key_type) - if template.template_type == 'letter' and postage is None: - postage = 'second' - data = { 'id': uuid.uuid4(), 'to': to_field, @@ -310,7 +301,6 @@ def create_notification( 'normalised_to': normalised_to, 'reply_to_text': reply_to_text, 'created_by_id': created_by_id, - 'postage': postage, 'document_download_count': document_download_count, } notification = Notification(**data) @@ -337,7 +327,6 @@ def create_notification_history( international=False, phone_prefix=None, created_by_id=None, - postage=None, id=None ): assert job or template @@ -351,9 +340,6 @@ def create_notification_history( sent_at = sent_at or datetime.utcnow() updated_at = updated_at or datetime.utcnow() - if template.template_type == 'letter' and postage is None: - postage = 'second' - data = { 'id': id or uuid.uuid4(), 'job_id': job and job.id, @@ -379,7 +365,6 @@ def create_notification_history( 'international': international, 'phone_prefix': phone_prefix, 'created_by_id': created_by_id, - 'postage': postage } notification_history = NotificationHistory(**data) db.session.add(notification_history) @@ -724,7 +709,6 @@ def create_ft_billing(local_date, rate=0, billable_unit=1, notifications_sent=1, - postage='none' ): data = FactBilling(local_date=local_date, service_id=template.service_id, @@ -735,8 +719,7 @@ def create_ft_billing(local_date, international=international, rate=rate, billable_units=billable_unit, - notifications_sent=notifications_sent, - postage=postage) + notifications_sent=notifications_sent,) db.session.add(data) db.session.commit() return data @@ -967,11 +950,11 @@ def set_up_usage_data(start_date): create_ft_billing(local_date=two_days_later, template=sms_template_1, billable_unit=1, rate=0.11) create_ft_billing(local_date=one_week_later, template=letter_template_1, - notifications_sent=2, billable_unit=2, rate=.35, postage='first') + notifications_sent=2, billable_unit=2, rate=.35) create_ft_billing(local_date=one_month_later, template=letter_template_1, - notifications_sent=4, billable_unit=8, rate=.45, postage='second') + notifications_sent=4, billable_unit=8, rate=.45) create_ft_billing(local_date=one_week_later, template=letter_template_1, - notifications_sent=2, billable_unit=4, rate=.45, postage='second') + notifications_sent=2, billable_unit=4, rate=.45) # service with emails only: service_with_emails = create_service(service_name='b - emails') @@ -998,11 +981,11 @@ def set_up_usage_data(start_date): create_annual_billing(service_id=service_with_letters.id, free_sms_fragment_limit=0, financial_year_start=year) create_ft_billing(local_date=start_date, template=letter_template_3, - notifications_sent=2, billable_unit=3, rate=.50, postage='first') + notifications_sent=2, billable_unit=3, rate=.50) create_ft_billing(local_date=one_week_later, template=letter_template_3, - notifications_sent=8, billable_unit=5, rate=.65, postage='second') + notifications_sent=8, billable_unit=5, rate=.65) create_ft_billing(local_date=one_month_later, template=letter_template_3, - notifications_sent=12, billable_unit=5, rate=.65, postage='second') + notifications_sent=12, billable_unit=5, rate=.65) # service with letters, without an organisation: service_with_letters_without_org = create_service(service_name='d - service without org') @@ -1014,13 +997,13 @@ def set_up_usage_data(start_date): ) create_ft_billing(local_date=two_days_later, template=letter_template_4, - notifications_sent=7, billable_unit=4, rate=1.55, postage='rest-of-world') + notifications_sent=7, billable_unit=4, rate=1.55) create_ft_billing(local_date=two_days_later, template=letter_template_4, - notifications_sent=8, billable_unit=4, rate=1.55, postage='europe') + notifications_sent=8, billable_unit=4, rate=1.55) create_ft_billing(local_date=two_days_later, template=letter_template_4, - notifications_sent=2, billable_unit=1, rate=.35, postage='second') + notifications_sent=2, billable_unit=1, rate=.35) create_ft_billing(local_date=two_days_later, template=letter_template_4, - notifications_sent=1, billable_unit=1, rate=.50, postage='first') + notifications_sent=1, billable_unit=1, rate=.50) # service with chargeable SMS, without an organisation service_with_sms_without_org = create_service( diff --git a/tests/app/notifications/test_process_letter_notifications.py b/tests/app/notifications/test_process_letter_notifications.py deleted file mode 100644 index 9bebba06a..000000000 --- a/tests/app/notifications/test_process_letter_notifications.py +++ /dev/null @@ -1,89 +0,0 @@ -from app.models import LETTER_TYPE, NOTIFICATION_CREATED, Notification -from app.notifications.process_letter_notifications import ( - create_letter_notification, -) -from app.serialised_models import SerialisedTemplate - - -def test_create_letter_notification_creates_notification(sample_letter_template, sample_api_key): - data = { - 'personalisation': { - 'address_line_1': 'The Queen', - 'address_line_2': 'Buckingham Palace', - 'postcode': 'SW1 1AA', - } - } - - template = SerialisedTemplate.from_id_and_service_id( - sample_letter_template.id, sample_letter_template.service_id - ) - - notification = create_letter_notification( - data, - template, - sample_letter_template.service, - sample_api_key, - NOTIFICATION_CREATED, - ) - - assert notification == Notification.query.one() - assert notification.job is None - assert notification.status == NOTIFICATION_CREATED - assert notification.template_id == sample_letter_template.id - assert notification.template_version == sample_letter_template.version - assert notification.api_key == sample_api_key - assert notification.notification_type == LETTER_TYPE - assert notification.key_type == sample_api_key.key_type - assert notification.reference is not None - assert notification.client_reference is None - assert notification.postage == 'second' - - -def test_create_letter_notification_sets_reference(sample_letter_template, sample_api_key): - data = { - 'personalisation': { - 'address_line_1': 'The Queen', - 'address_line_2': 'Buckingham Palace', - 'postcode': 'SW1 1AA', - }, - 'reference': 'foo' - } - - template = SerialisedTemplate.from_id_and_service_id( - sample_letter_template.id, sample_letter_template.service_id - ) - - notification = create_letter_notification( - data, - template, - sample_letter_template.service, - sample_api_key, - NOTIFICATION_CREATED, - ) - - assert notification.client_reference == 'foo' - - -def test_create_letter_notification_sets_billable_units(sample_letter_template, sample_api_key): - data = { - 'personalisation': { - 'address_line_1': 'The Queen', - 'address_line_2': 'Buckingham Palace', - 'postcode': 'SW1 1AA', - }, - } - - template = SerialisedTemplate.from_id_and_service_id( - sample_letter_template.id, sample_letter_template.service_id - ) - - notification = create_letter_notification( - data, - template, - sample_letter_template.service, - sample_api_key, - NOTIFICATION_CREATED, - billable_units=3, - ) - - assert notification.billable_units == 3 diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index c1057f2b5..ae6297221 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -450,40 +450,6 @@ def test_persist_email_notification_stores_normalised_email( assert persisted_notification.normalised_to == expected_recipient_normalised -@pytest.mark.parametrize( - "postage_argument, template_postage, expected_postage", - [ - ("second", "first", "second"), - ("first", "first", "first"), - ("first", "second", "first") - ] -) -def test_persist_letter_notification_finds_correct_postage( - mocker, - postage_argument, - template_postage, - expected_postage, - sample_service_full_permissions, - sample_api_key, -): - template = create_template(sample_service_full_permissions, template_type=LETTER_TYPE, postage=template_postage) - mocker.patch('app.dao.templates_dao.dao_get_template_by_id', return_value=template) - persist_notification( - template_id=template.id, - template_version=template.version, - recipient="Jane Doe, 10 Downing Street, London", - service=sample_service_full_permissions, - personalisation=None, - notification_type=LETTER_TYPE, - api_key_id=sample_api_key.id, - key_type=sample_api_key.key_type, - postage=postage_argument - ) - persisted_notification = Notification.query.all()[0] - - assert persisted_notification.postage == expected_postage - - def test_persist_notification_with_billable_units_stores_correct_info( mocker ): @@ -504,22 +470,3 @@ def test_persist_notification_with_billable_units_stores_correct_info( persisted_notification = Notification.query.all()[0] assert persisted_notification.billable_units == 3 - - -@pytest.mark.parametrize('postage', ['europe', 'rest-of-world']) -def test_persist_notification_for_international_letter(sample_letter_template, postage): - notification = persist_notification( - template_id=sample_letter_template.id, - template_version=sample_letter_template.version, - recipient="123 Main Street", - service=sample_letter_template.service, - personalisation=None, - notification_type=sample_letter_template.template_type, - api_key_id=None, - key_type="normal", - billable_units=3, - postage=postage, - ) - persisted_notification = Notification.query.get(notification.id) - assert persisted_notification.postage == postage - assert persisted_notification.international diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index f354d184e..d18726350 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -25,7 +25,6 @@ from app.notifications.validators import ( check_template_is_active, check_template_is_for_notification_type, service_can_send_to_recipient, - validate_address, validate_and_format_recipient, validate_template, ) @@ -623,19 +622,3 @@ def test_check_if_service_can_send_files_by_email_passes_if_contact_link_set(sam service_contact_link=sample_service.contact_link, service_id=sample_service.id ) - - -@pytest.mark.parametrize('key, address_line_3, expected_postage', - [('address_line_3', 'SW1 1AA', None), - ('address_line_5', 'CANADA', 'rest-of-world'), - ('address_line_3', 'GERMANY', 'europe') - ]) -def test_validate_address(notify_db_session, key, address_line_3, expected_postage): - service = create_service(service_permissions=[LETTER_TYPE, INTERNATIONAL_LETTERS]) - data = { - 'address_line_1': 'Prince Harry', - 'address_line_2': 'Toronto', - key: address_line_3, - } - postage = validate_address(service, data) - assert postage == expected_postage diff --git a/tests/app/platform_stats/test_rest.py b/tests/app/platform_stats/test_rest.py index 2bd3847c0..f84af730e 100644 --- a/tests/app/platform_stats/test_rest.py +++ b/tests/app/platform_stats/test_rest.py @@ -143,7 +143,6 @@ def test_get_data_for_billing_report(notify_db_session, admin_request): assert response[0]["sms_chargeable_units"] == 0 assert response[0]["total_letters"] == 8 assert response[0]["letter_cost"] == 3.40 - assert response[0]["letter_breakdown"] == "6 second class letters at 45p\n2 first class letters at 35p\n" assert response[0]["purchase_order_number"] == "service purchase order number" assert response[0]["contact_names"] == "service billing contact names" assert response[0]["contact_email_addresses"] == "service@billing.contact email@addresses.gov.uk" @@ -155,7 +154,6 @@ def test_get_data_for_billing_report(notify_db_session, admin_request): assert response[1]["sms_chargeable_units"] == 0 assert response[1]["total_letters"] == 22 assert response[1]["letter_cost"] == 14 - assert response[1]["letter_breakdown"] == "20 second class letters at 65p\n2 first class letters at 50p\n" assert response[1]["purchase_order_number"] == "org3 purchase order number" assert response[1]["contact_names"] == "org3 billing contact names" assert response[1]["contact_email_addresses"] == "org3@billing.contact email@addresses.gov.uk" @@ -167,7 +165,6 @@ def test_get_data_for_billing_report(notify_db_session, admin_request): assert response[2]["sms_chargeable_units"] == 3 assert response[2]["total_letters"] == 0 assert response[2]["letter_cost"] == 0 - assert response[2]["letter_breakdown"] == "" assert response[2]["purchase_order_number"] == "sms purchase order number" assert response[2]["contact_names"] == "sms billing contact names" assert response[2]["contact_email_addresses"] == "sms@billing.contact email@addresses.gov.uk" @@ -179,9 +176,6 @@ def test_get_data_for_billing_report(notify_db_session, admin_request): assert response[3]["sms_chargeable_units"] == 0 assert response[3]["total_letters"] == 18 assert response[3]["letter_cost"] == 24.45 - assert response[3]["letter_breakdown"] == ( - "2 second class letters at 35p\n1 first class letters at 50p\n15 international letters at £1.55\n" - ) assert response[3]["purchase_order_number"] is None diff --git a/tests/app/service/send_notification/test_send_one_off_notification.py b/tests/app/service/send_notification/test_send_one_off_notification.py index d0065d0be..3055387d4 100644 --- a/tests/app/service/send_notification/test_send_one_off_notification.py +++ b/tests/app/service/send_notification/test_send_one_off_notification.py @@ -100,7 +100,6 @@ def test_send_one_off_notification_calls_persist_correctly_for_sms( created_by_id=str(service.created_by_id), reply_to_text='testing', reference=None, - postage=None, client_reference=None ) @@ -162,57 +161,6 @@ def test_send_one_off_notification_calls_persist_correctly_for_email( created_by_id=str(service.created_by_id), reply_to_text=None, reference=None, - postage=None, - client_reference=None - ) - - -def test_send_one_off_notification_calls_persist_correctly_for_letter( - mocker, - persist_mock, - celery_mock, - notify_db_session -): - mocker.patch( - 'app.service.send_notification.create_random_identifier', - return_value='this-is-random-in-real-life', - ) - service = create_service() - template = create_template( - service=service, - template_type=LETTER_TYPE, - postage='first', - subject="Test subject", - content="Hello (( Name))\nYour thing is due soon", - ) - - post_data = { - 'template_id': str(template.id), - 'to': 'First Last', - 'personalisation': { - 'name': 'foo', - 'address_line_1': 'First Last', - 'address_line_2': '1 Example Street', - 'postcode': 'SW1A 1AA', - }, - 'created_by': str(service.created_by_id) - } - - send_one_off_notification(service.id, post_data) - - persist_mock.assert_called_once_with( - template_id=template.id, - template_version=template.version, - recipient=post_data['to'], - service=template.service, - personalisation=post_data['personalisation'], - notification_type=LETTER_TYPE, - api_key_id=None, - key_type=KEY_TYPE_NORMAL, - created_by_id=str(service.created_by_id), - reply_to_text=None, - reference='this-is-random-in-real-life', - postage='first', client_reference=None ) diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index f320afd5b..fd2c29337 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -43,8 +43,6 @@ def test_should_create_a_new_template_for_a_service( } if subject: data.update({'subject': subject}) - if template_type == LETTER_TYPE: - data.update({'postage': 'first'}) data = json.dumps(data) auth_header = create_admin_authorization_header() @@ -68,11 +66,6 @@ def test_should_create_a_new_template_for_a_service( else: assert not json_resp['data']['subject'] - if template_type == LETTER_TYPE: - assert json_resp['data']['postage'] == 'first' - else: - assert not json_resp['data']['postage'] - template = Template.query.get(json_resp['data']['id']) from app.schemas import template_schema assert sorted(json_resp['data']) == sorted(template_schema.dump(template)) @@ -307,8 +300,8 @@ def test_must_have_a_subject_on_an_email_template(client, sample_user, sample_se def test_update_should_update_a_template(client, sample_user): - service = create_service(service_permissions=[LETTER_TYPE]) - template = create_template(service, template_type="letter", postage="second") + service = create_service() + template = create_template(service, template_type="sms") assert template.created_by == service.created_by assert template.created_by != sample_user @@ -316,7 +309,6 @@ def test_update_should_update_a_template(client, sample_user): data = { 'content': 'my template has new content, swell!', 'created_by': str(sample_user.id), - 'postage': 'first' } data = json.dumps(data) auth_header = create_admin_authorization_header() @@ -332,7 +324,6 @@ def test_update_should_update_a_template(client, sample_user): assert update_json_resp['data']['content'] == ( 'my template has new content, swell!' ) - assert update_json_resp['data']['postage'] == 'first' assert update_json_resp['data']['name'] == template.name assert update_json_resp['data']['template_type'] == template.template_type assert update_json_resp['data']['version'] == 2 @@ -477,7 +468,6 @@ def test_should_get_return_all_fields_by_default( 'hidden', 'id', 'name', - 'postage', 'process_type', 'redact_personalisation', 'reply_to', @@ -828,15 +818,7 @@ def test_create_a_template_with_foreign_service_reply_to(admin_request, sample_u {"error": "ValidationError", "message": "service is a required property"}, {"error": "ValidationError", "message": "created_by is a required property"}, ] - ), - ( - {"name": "my template", "template_type": "sms", "content": "hi", "postage": "third", - "service": "1af43c02-b5a8-4923-ad7f-5279b75ff2d0", "created_by": "30587644-9083-44d8-a114-98887f07f1e3"}, - [ - {"error": "ValidationError", - "message": "postage invalid. It must be first, second, europe or rest-of-world."}, - ] - ), + ) ]) def test_create_template_validates_against_json_schema( admin_request, @@ -924,19 +906,6 @@ def test_update_template_reply_to_set_to_blank(client, notify_db_session): assert th.service_letter_contact_id is None -def test_update_template_validates_postage(admin_request, sample_service_full_permissions): - template = create_template(service=sample_service_full_permissions, template_type='letter') - - response = admin_request.post( - 'template.update_template', - service_id=sample_service_full_permissions.id, - template_id=template.id, - _data={"postage": "third"}, - _expected_status=400 - ) - assert 'postage invalid' in response['errors'][0]['message'] - - def test_update_template_with_foreign_service_reply_to(client, sample_letter_template): auth_header = create_admin_authorization_header() diff --git a/tests/app/upload/test_rest.py b/tests/app/upload/test_rest.py index 53195e151..92c98de98 100644 --- a/tests/app/upload/test_rest.py +++ b/tests/app/upload/test_rest.py @@ -33,7 +33,6 @@ def create_precompiled_template(service): subject='Pre-compiled PDF', content="", hidden=True, - postage="second", ) diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 7823563d6..03c16b6b6 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -67,7 +67,6 @@ def test_get_notification_by_id_returns_200( 'sent_at': sample_notification.sent_at, 'completed_at': sample_notification.completed_at(), 'scheduled_for': None, - 'postage': None, 'provider_response': None } @@ -120,7 +119,6 @@ def test_get_notification_by_id_with_placeholders_returns_200( 'sent_at': sample_notification.sent_at, 'completed_at': sample_notification.completed_at(), 'scheduled_for': None, - 'postage': None, 'provider_response': None } @@ -216,35 +214,6 @@ def test_get_notification_by_id_invalid_id(client, sample_notification, id): "status_code": 400} -@pytest.mark.parametrize('created_at_month, postage, estimated_delivery', [ - (12, 'second', '2000-12-06T16:00:00.000000Z'), # 4pm GMT in winter - (6, 'second', '2000-06-05T15:00:00.000000Z'), # 4pm BST in summer - - (12, 'first', '2000-12-05T16:00:00.000000Z'), # 4pm GMT in winter - (6, 'first', '2000-06-03T15:00:00.000000Z'), # 4pm BST in summer (two days before 2nd class due to weekends) -]) -def test_get_notification_adds_delivery_estimate_for_letters( - client, - sample_letter_notification, - created_at_month, - postage, - estimated_delivery, -): - sample_letter_notification.created_at = datetime.date(2000, created_at_month, 1) - sample_letter_notification.postage = postage - - auth_header = create_service_authorization_header(service_id=sample_letter_notification.service_id) - response = client.get( - path='/v2/notifications/{}'.format(sample_letter_notification.id), - headers=[('Content-Type', 'application/json'), auth_header] - ) - - json_response = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert json_response['postage'] == postage - assert json_response['estimated_delivery'] == estimated_delivery - - @pytest.mark.parametrize('template_type', ['sms', 'email']) def test_get_notification_doesnt_have_delivery_estimate_for_non_letters(client, sample_service, template_type): template = create_template(service=sample_service, template_type=template_type) diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 83c8fc13e..7d7be8450 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -57,7 +57,6 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol assert len(notifications) == 1 assert notifications[0].status == NOTIFICATION_CREATED notification_id = notifications[0].id - assert notifications[0].postage is None assert notifications[0].document_download_count is None assert resp_json['id'] == str(notification_id) assert resp_json['reference'] == reference @@ -429,7 +428,6 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_ assert validate(resp_json, post_email_response) == resp_json notification = Notification.query.one() assert notification.status == NOTIFICATION_CREATED - assert notification.postage is None assert resp_json['id'] == str(notification.id) assert resp_json['reference'] == reference assert notification.reference is None diff --git a/tests/app/v2/template/test_get_template.py b/tests/app/v2/template/test_get_template.py index 4dbee4e81..51445e96f 100644 --- a/tests/app/v2/template/test_get_template.py +++ b/tests/app/v2/template/test_get_template.py @@ -9,19 +9,16 @@ from tests.app.db import create_letter_contact, create_template valid_version_params = [None, 1] -@pytest.mark.parametrize("tmp_type, expected_name, expected_subject,postage", [ - (SMS_TYPE, 'sms Template Name', None, None), - (EMAIL_TYPE, 'email Template Name', 'Template subject', None), - (LETTER_TYPE, 'letter Template Name', 'Template subject', "second") +@pytest.mark.parametrize("tmp_type, expected_name, expected_subject", [ + (SMS_TYPE, 'sms Template Name', None), + (EMAIL_TYPE, 'email Template Name', 'Template subject'), + (LETTER_TYPE, 'letter Template Name', 'Template subject') ]) @pytest.mark.parametrize("version", valid_version_params) def test_get_template_by_id_returns_200( - client, sample_service, tmp_type, expected_name, expected_subject, version, postage + client, sample_service, tmp_type, expected_name, expected_subject, version ): letter_contact_block_id = None - if tmp_type == 'letter': - letter_contact_block = create_letter_contact(sample_service, "Buckingham Palace, London, SW1A 1AA") - letter_contact_block_id = letter_contact_block.id template = create_template(sample_service, template_type=tmp_type, contact_block_id=(letter_contact_block_id)) auth_header = create_service_authorization_header(service_id=sample_service.id) @@ -47,7 +44,6 @@ def test_get_template_by_id_returns_200( "subject": expected_subject, 'name': expected_name, 'personalisation': {}, - 'postage': postage, 'letter_contact_block': letter_contact_block.contact_block if letter_contact_block_id else None, } diff --git a/tests/app/v2/template/test_template_schemas.py b/tests/app/v2/template/test_template_schemas.py index f5b804cb6..36ac2926b 100644 --- a/tests/app/v2/template/test_template_schemas.py +++ b/tests/app/v2/template/test_template_schemas.py @@ -34,7 +34,6 @@ valid_json_get_response_with_optionals = { 'body': 'some body', 'subject': "some subject", 'name': 'some name', - 'postage': 'first', } valid_request_args = [{"id": str(uuid.uuid4()), "version": 1}, {"id": str(uuid.uuid4())}] @@ -80,7 +79,6 @@ valid_json_post_response_with_optionals = { 'version': 1, 'body': "some body", 'subject': 'some subject', - 'postage': 'second', 'html': '
some body
', }