diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index cb6b2c84b..a87db3fcd 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,79 +1,22 @@ -*A note to PR reviewers: it may be helpful to review our -[code review documentation](https://github.com/GSA/notifications-api/blob/main/docs/all.md#code-reviews) -to know what to keep in mind while reviewing pull requests.* +*A note to PR reviewers: it may be helpful to review our [code review documentation](https://github.com/GSA/notifications-api/blob/main/docs/all.md#code-reviews) to know what to keep in mind while reviewing pull requests.* ## Description -Please enter a clear description about your proposed changes and what the -expected outcome(s) is/are from there. If there are complex implementation -details within the changes, this is a great place to explain those details using -plain language. - -This should include: - -- Links to issues that this PR addresses -- Screenshots or screen captures of any visible changes, especially for UI work -- Dependency changes - -If there are any caveats, known issues, follow-up items, etc., make a quick note -of them here as well, though more details are probably warranted in the issue -itself in this case. +Please enter a detailed description here. ## TODO (optional) -If you're opening a draft PR, it might be helpful to list any outstanding work, -especially if you're asking folks to take a look before it's ready for full -review. In this case, create a small checklist with the outstanding items: - -- [ ] TODO item 1 -- [ ] TODO item 2 -- [ ] TODO item ... +* [ ] TODO item 1 +* [ ] TODO item 2 +* [ ] TODO item ... ## Security Considerations -Please think about the security compliance aspect of your changes and what the -potential impacts might be. - -**NOTE: Please be mindful of sharing sensitive information here! If you're not -sure of what to write, please ask the team first before writing anything here.** - -Relevant details could include (and are not limited to) the following: - -- Handling secrets/credential management (or specifically calling out that there - is nothing to handle) -- Any adjustments to the flow of data in and out the system, or even within it -- Connecting or disconnecting any external services to the application -- Handling of any sensitive information, such as PII -- Handling of information within log statements or other application monitoring - services/hooks -- The inclusion of a new external dependency or the removal of an existing one -- ... (anything else relevant from a security compliance perspective) - -There are some cases where there are no security considerations to be had, e.g., -updating our documentation with publicly available information. In those cases -it is fine to simply put something like this: - -- None; this is a documentation update with publicly available information. +* Consideration 1 +* Consideration 2 +* Consideration ... diff --git a/README.md b/README.md index 8547e540a..0ae5d67c6 100644 --- a/README.md +++ b/README.md @@ -493,6 +493,14 @@ instructions above for more details. - [Celery scheduled tasks](./docs/all.md#celery-scheduled-tasks) - [Notify.gov](./docs/all.md#notifygov) - [System Description](./docs/all.md#system-description) +- [Pull Requests](.docs/all.md#pull-requests) + - [Getting Started](.docs/all.md#getting-started) + - [Description](.docs/all.md#description) + - [TODO (optional)](.docs/all.md#todo-(optional)) + - [Security Considerations](.docs/all.md#security-considerations) +- [Code Reviews](.docs/all.md#code-reviews) + - [For the reviewer](.docs/all.md#for-the-reviewer) + - [For the author](.docs/all.md#for-the-author) - [Run Book](./docs/all.md#run-book) - [ Alerts, Notifications, Monitoring](./docs/all.md#-alerts-notifications-monitoring) - [ Restaging Apps](./docs/all.md#-restaging-apps) diff --git a/app/commands.py b/app/commands.py index 826c2013b..c97a2f774 100644 --- a/app/commands.py +++ b/app/commands.py @@ -589,6 +589,17 @@ def process_row_from_job(job_id, job_row_number): ) +@notify_command(name="download-csv-file-by-name") +@click.option("-f", "--csv_filename", required=True, help="csv file name") +def download_csv_file_by_name(csv_filename): + + bucket_name = current_app.config["CSV_UPLOAD_BUCKET"]["bucket"] + access_key = current_app.config["CSV_UPLOAD_BUCKET"]["access_key_id"] + secret = current_app.config["CSV_UPLOAD_BUCKET"]["secret_access_key"] + region = current_app.config["CSV_UPLOAD_BUCKET"]["region"] + print(s3.get_s3_file(bucket_name, csv_filename, access_key, secret, region)) + + @notify_command(name="populate-annual-billing-with-the-previous-years-allowance") @click.option( "-y", @@ -854,13 +865,12 @@ def promote_user_to_platform_admin(user_email_address): @notify_command(name="purge-csv-bucket") def purge_csv_bucket(): - bucket_name = getenv("CSV_BUCKET_NAME") - access_key = getenv("CSV_AWS_ACCESS_KEY_ID") - secret = getenv("CSV_AWS_SECRET_ACCESS_KEY") - region = getenv("CSV_AWS_REGION") - print("ABOUT TO RUN PURGE CSV BUCKET") + bucket_name = current_app.config["CSV_UPLOAD_BUCKET"]["bucket"] + access_key = current_app.config["CSV_UPLOAD_BUCKET"]["access_key_id"] + secret = current_app.config["CSV_UPLOAD_BUCKET"]["secret_access_key"] + region = current_app.config["CSV_UPLOAD_BUCKET"]["region"] + s3.purge_bucket(bucket_name, access_key, secret, region) - print("RAN PURGE CSV BUCKET") """ diff --git a/app/dao/date_util.py b/app/dao/date_util.py index 98c0372c5..d93986b45 100644 --- a/app/dao/date_util.py +++ b/app/dao/date_util.py @@ -1,3 +1,4 @@ +import calendar from datetime import date, datetime, time, timedelta from app.utils import utc_now @@ -66,3 +67,29 @@ def get_calendar_year_for_datetime(start_date): return year - 1 else: return year + + +def get_number_of_days_for_month(year, month): + return calendar.monthrange(year, month)[1] + + +def generate_date_range(start_date, end_date=None, days=0): + if end_date: + current_date = start_date + while current_date <= end_date: + try: + yield current_date.date() + except ValueError: + pass + current_date += timedelta(days=1) + elif days > 0: + end_date = start_date + timedelta(days=days) + current_date = start_date + while current_date < end_date: + try: + yield current_date.date() + except ValueError: + pass + current_date += timedelta(days=1) + else: + return "An end_date or number of days must be specified" diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 22c87fe83..a810ff0db 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -84,21 +84,21 @@ def update_fact_notification_status(process_day, notification_type, service_id): def fetch_notification_status_for_service_by_month(start_date, end_date, service_id): return ( db.session.query( - func.date_trunc("month", FactNotificationStatus.local_date).label("month"), - FactNotificationStatus.notification_type, - FactNotificationStatus.notification_status, - func.sum(FactNotificationStatus.notification_count).label("count"), + func.date_trunc("month", NotificationAllTimeView.created_at).label("month"), + NotificationAllTimeView.notification_type, + NotificationAllTimeView.status.label("notification_status"), + func.count(NotificationAllTimeView.id).label("count"), ) .filter( - FactNotificationStatus.service_id == service_id, - FactNotificationStatus.local_date >= start_date, - FactNotificationStatus.local_date < end_date, - FactNotificationStatus.key_type != KeyType.TEST, + NotificationAllTimeView.service_id == service_id, + NotificationAllTimeView.created_at >= start_date, + NotificationAllTimeView.created_at < end_date, + NotificationAllTimeView.key_type != KeyType.TEST, ) .group_by( - func.date_trunc("month", FactNotificationStatus.local_date).label("month"), - FactNotificationStatus.notification_type, - FactNotificationStatus.notification_status, + func.date_trunc("month", NotificationAllTimeView.created_at).label("month"), + NotificationAllTimeView.notification_type, + NotificationAllTimeView.status, ) .all() ) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 7ddc456e7..6fa663341 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -8,7 +8,7 @@ from sqlalchemy.sql.expression import and_, asc, case, func from app import db from app.dao.dao_utils import VersionOptions, autocommit, version_class -from app.dao.date_util import get_current_calendar_year +from app.dao.date_util import generate_date_range, get_current_calendar_year from app.dao.organization_dao import dao_get_organization_by_email_address from app.dao.service_sms_sender_dao import insert_service_sms_sender from app.dao.service_user_dao import dao_get_service_user @@ -27,6 +27,7 @@ from app.models import ( InvitedUser, Job, Notification, + NotificationAllTimeView, NotificationHistory, Organization, Permission, @@ -40,6 +41,7 @@ from app.models import ( User, VerifyCode, ) +from app.service import statistics from app.utils import ( escape_special_characters, get_archived_db_column_value, @@ -426,6 +428,61 @@ def dao_fetch_todays_stats_for_service(service_id): ) +def dao_fetch_stats_for_service_from_days(service_id, start_date, end_date): + start_date = get_midnight_in_utc(start_date) + end_date = get_midnight_in_utc(end_date + timedelta(days=1)) + + return ( + db.session.query( + NotificationAllTimeView.notification_type, + NotificationAllTimeView.status, + func.date_trunc("day", NotificationAllTimeView.created_at).label("day"), + func.count(NotificationAllTimeView.id).label("count"), + ) + .filter( + NotificationAllTimeView.service_id == service_id, + NotificationAllTimeView.key_type != KeyType.TEST, + NotificationAllTimeView.created_at >= start_date, + NotificationAllTimeView.created_at < end_date, + ) + .group_by( + NotificationAllTimeView.notification_type, + NotificationAllTimeView.status, + func.date_trunc("day", NotificationAllTimeView.created_at), + ) + .all() + ) + + +def dao_fetch_stats_for_service_from_days_for_user( + service_id, start_date, end_date, user_id +): + start_date = get_midnight_in_utc(start_date) + end_date = get_midnight_in_utc(end_date + timedelta(days=1)) + + return ( + db.session.query( + NotificationAllTimeView.notification_type, + NotificationAllTimeView.status, + func.date_trunc("day", NotificationAllTimeView.created_at).label("day"), + func.count(NotificationAllTimeView.id).label("count"), + ) + .filter( + NotificationAllTimeView.service_id == service_id, + NotificationAllTimeView.key_type != KeyType.TEST, + NotificationAllTimeView.created_at >= start_date, + NotificationAllTimeView.created_at < end_date, + NotificationAllTimeView.created_by_id == user_id, + ) + .group_by( + NotificationAllTimeView.notification_type, + NotificationAllTimeView.status, + func.date_trunc("day", NotificationAllTimeView.created_at), + ) + .all() + ) + + def dao_fetch_todays_stats_for_all_services( include_from_test_key=True, only_active=True ): @@ -607,3 +664,52 @@ def get_live_services_with_organization(): ) return query.all() + + +def fetch_notification_stats_for_service_by_month_by_user( + start_date, end_date, service_id, user_id +): + return ( + db.session.query( + func.date_trunc("month", NotificationAllTimeView.created_at).label("month"), + NotificationAllTimeView.notification_type, + (NotificationAllTimeView.status).label("notification_status"), + func.count(NotificationAllTimeView.id).label("count"), + ) + .filter( + NotificationAllTimeView.service_id == service_id, + NotificationAllTimeView.created_at >= start_date, + NotificationAllTimeView.created_at < end_date, + NotificationAllTimeView.key_type != KeyType.TEST, + NotificationAllTimeView.created_by_id == user_id, + ) + .group_by( + func.date_trunc("month", NotificationAllTimeView.created_at).label("month"), + NotificationAllTimeView.notification_type, + NotificationAllTimeView.status, + ) + .all() + ) + + +def get_specific_days_stats(results, start_date, days=None, end_date=None): + if days is not None and end_date is not None: + raise ValueError("Only set days OR set end_date, not both.") + elif days is not None: + gen_range = generate_date_range(start_date, days=days) + elif end_date is not None: + gen_range = generate_date_range(start_date, end_date) + else: + raise ValueError("Either days or end_date must be set.") + + grouped_results = {date: [] for date in gen_range} | { + day.date(): [notification_type, status, day, count] + for notification_type, status, day, count in results + } + + stats = { + day.strftime("%Y-%m-%d"): statistics.format_statistics(rows) + for day, rows in grouped_results.items() + } + + return stats diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index d7291b35c..e541f4052 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -4,7 +4,7 @@ from secrets import randbelow import sqlalchemy from flask import current_app -from sqlalchemy import func +from sqlalchemy import func, text from sqlalchemy.orm import joinedload from app import db @@ -244,3 +244,15 @@ def user_can_be_archived(user): return False return True + + +def dao_report_users(): + sql = """ + select users.name, users.email_address, users.mobile_number, services.name as service_name + from users + inner join user_to_service on users.id=user_to_service.user_id + inner join services on services.id=user_to_service.service_id + where services.name not like '_archived%' + order by services.name asc, users.name asc + """ + return db.session.execute(text(sql)) diff --git a/app/service/rest.py b/app/service/rest.py index 0503c71d5..71faab7a1 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -1,5 +1,5 @@ import itertools -from datetime import datetime +from datetime import datetime, timedelta from flask import Blueprint, current_app, jsonify, request from sqlalchemy.exc import IntegrityError @@ -17,7 +17,7 @@ from app.dao.api_key_dao import ( save_model_api_key, ) from app.dao.dao_utils import dao_rollback, transaction -from app.dao.date_util import get_calendar_year +from app.dao.date_util import get_calendar_year, get_month_start_and_end_date_in_utc from app.dao.fact_notification_status_dao import ( fetch_monthly_template_usage_for_service, fetch_notification_status_for_service_by_month, @@ -63,13 +63,17 @@ from app.dao.services_dao import ( dao_fetch_all_services_by_user, dao_fetch_live_services_data, dao_fetch_service_by_id, + dao_fetch_stats_for_service_from_days, + dao_fetch_stats_for_service_from_days_for_user, dao_fetch_todays_stats_for_all_services, dao_fetch_todays_stats_for_service, dao_remove_user_from_service, dao_resume_service, dao_suspend_service, dao_update_service, + fetch_notification_stats_for_service_by_month_by_user, get_services_by_partial_name, + get_specific_days_stats, ) from app.dao.templates_dao import dao_get_template_by_id from app.dao.users_dao import get_user_by_id @@ -210,6 +214,58 @@ def get_service_notification_statistics(service_id): ) +@service_blueprint.route("//statistics//") +def get_service_notification_statistics_by_day(service_id, start, days): + return jsonify( + data=get_service_statistics_for_specific_days(service_id, start, int(days)) + ) + + +def get_service_statistics_for_specific_days(service_id, start, days=1): + # start and end dates needs to be reversed because + # the end date is today and the start is x days in the past + # a day needs to be substracted to allow for today + end_date = datetime.strptime(start, "%Y-%m-%d") + start_date = end_date - timedelta(days=days - 1) + + results = dao_fetch_stats_for_service_from_days(service_id, start_date, end_date) + + stats = get_specific_days_stats(results, start_date, days=days) + + return stats + + +@service_blueprint.route( + "//statistics/user///" +) +def get_service_notification_statistics_by_day_by_user( + service_id, user_id, start, days +): + return jsonify( + data=get_service_statistics_for_specific_days_by_user( + service_id, user_id, start, int(days) + ) + ) + + +def get_service_statistics_for_specific_days_by_user( + service_id, user_id, start, days=1 +): + # start and end dates needs to be reversed because + # the end date is today and the start is x days in the past + # a day needs to be substracted to allow for today + end_date = datetime.strptime(start, "%Y-%m-%d") + start_date = end_date - timedelta(days=days - 1) + + results = dao_fetch_stats_for_service_from_days_for_user( + service_id, start_date, end_date, user_id + ) + + stats = get_specific_days_stats(results, start_date, days=days) + + return stats + + @service_blueprint.route("", methods=["POST"]) def create_service(): data = request.get_json() @@ -592,6 +648,7 @@ def get_monthly_notification_stats(service_id): stats = fetch_notification_status_for_service_by_month( start_date, end_date, service_id ) + statistics.add_monthly_notification_status_stats(data, stats) now = utc_now() @@ -604,6 +661,87 @@ def get_monthly_notification_stats(service_id): return jsonify(data=data) +@service_blueprint.route( + "//notifications//monthly", methods=["GET"] +) +def get_monthly_notification_stats_by_user(service_id, user_id): + # check service_id validity + dao_fetch_service_by_id(service_id) + # user = get_user_by_id(user_id=user_id) + + try: + year = int(request.args.get("year", "NaN")) + except ValueError: + raise InvalidRequest("Year must be a number", status_code=400) + + start_date, end_date = get_calendar_year(year) + + data = statistics.create_empty_monthly_notification_status_stats_dict(year) + + stats = fetch_notification_stats_for_service_by_month_by_user( + start_date, end_date, service_id, user_id + ) + + statistics.add_monthly_notification_status_stats(data, stats) + + now = utc_now() + if end_date > now: + todays_deltas = fetch_notification_status_for_service_for_day( + now, service_id=service_id + ) + statistics.add_monthly_notification_status_stats(data, todays_deltas) + + return jsonify(data=data) + + +@service_blueprint.route( + "//notifications//month", methods=["GET"] +) +def get_single_month_notification_stats_by_user(service_id, user_id): + # check service_id validity + dao_fetch_service_by_id(service_id) + + try: + month = int(request.args.get("month", "NaN")) + year = int(request.args.get("year", "NaN")) + except ValueError: + raise InvalidRequest( + "Both a month and year are required as numbers", status_code=400 + ) + + month_year = datetime(year, month, 10, 00, 00, 00) + start_date, end_date = get_month_start_and_end_date_in_utc(month_year) + + results = dao_fetch_stats_for_service_from_days_for_user( + service_id, start_date, end_date, user_id + ) + + stats = get_specific_days_stats(results, start_date, end_date=end_date) + return jsonify(stats) + + +@service_blueprint.route("//notifications/month", methods=["GET"]) +def get_single_month_notification_stats_for_service(service_id): + # check service_id validity + dao_fetch_service_by_id(service_id) + + try: + month = int(request.args.get("month", "NaN")) + year = int(request.args.get("year", "NaN")) + except ValueError: + raise InvalidRequest( + "Both a month and year are required as numbers", status_code=400 + ) + + month_year = datetime(year, month, 10, 00, 00, 00) + start_date, end_date = get_month_start_and_end_date_in_utc(month_year) + + results = dao_fetch_stats_for_service_from_days(service_id, start_date, end_date) + + stats = get_specific_days_stats(results, start_date, end_date=end_date) + return jsonify(stats) + + def get_detailed_service(service_id, today_only=False): service = dao_fetch_service_by_id(service_id) diff --git a/app/service/statistics.py b/app/service/statistics.py index 90b933960..a6b58e067 100644 --- a/app/service/statistics.py +++ b/app/service/statistics.py @@ -113,7 +113,6 @@ def create_empty_monthly_notification_status_stats_dict(year): def add_monthly_notification_status_stats(data, stats): for row in stats: month = row.month.strftime("%Y-%m") - data[month][row.notification_type][row.notification_status] += row.count - + data[month][row.notification_type][StatisticsType.REQUESTED] += row.count return data diff --git a/app/user/rest.py b/app/user/rest.py index 049549f2c..dd714ce57 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -18,6 +18,7 @@ from app.dao.users_dao import ( create_secret_code, create_user_code, dao_archive_user, + dao_report_users, get_login_gov_user, get_user_and_accounts, get_user_by_email, @@ -667,6 +668,12 @@ def update_password(user_id): return jsonify(data=user.serialize()), 200 +@user_blueprint.route("/report-all-users", methods=["GET"]) +def report_all_users(): + users = dao_report_users() + return jsonify(data=users.serialize()), 200 + + @user_blueprint.route("//organizations-and-services", methods=["GET"]) def get_organizations_and_services_for_user(user_id): user = get_user_and_accounts(user_id) diff --git a/docs/all.md b/docs/all.md index 0ad78ae2b..d98f0d336 100644 --- a/docs/all.md +++ b/docs/all.md @@ -38,6 +38,11 @@ - [Celery scheduled tasks](#celery-scheduled-tasks) - [Notify.gov](#notifygov) - [System Description](#system-description) +- [Pull Requests](#pull-requests) + - [Getting Started](#getting-started) + - [Description](#description) + - [TODO (optional)](#todo-(optional)) + - [Security Considerations](#security-considerations) - [Code Reviews](#code-reviews) - [For the reviewer](#for-the-reviewer) - [For the author](#for-the-author) @@ -55,6 +60,9 @@ - [Data Storage Policies \& Procedures](#data-storage-policies--procedures) - [Potential PII Locations](#potential-pii-locations) - [Data Retention Policy](#data-retention-policy) +- [Debug messages not being sent](#debug-messages-not-being-sent) + - [Getting the file location and tracing what happens](#getting-the-file-location-and-tracing-what-happens) + - [Viewing the csv file](#viewing-the-csv-file) # Infrastructure overview @@ -497,7 +505,7 @@ flask command purge_functional_test_data -u Running on cloud.gov: ``` -cf run-task notify-api "flask command purge_functional_test_data -u " +cf run-task notify-api --command "flask command purge_functional_test_data -u " ``` @@ -817,6 +825,97 @@ Notify.gov also provisions and uses two AWS services via a [supplemental service For further details of the system and how it connects to supporting services, see the [application boundary diagram](https://github.com/GSA/us-notify-compliance/blob/main/diagrams/rendered/apps/application.boundary.png) +Pull Requests +============= + +Changes are made to our applications via pull requests, which show a diff +(the before and after state of all proposed changes in the code) of of the work +done for that particular branch. We use pull requests as the basis for working +on Notify.gov and modifying the application over time for improvements, bug +fixes, new features, and more. + +There are several things that make for a good and complete pull request: + +* An appropriate and descriptive title +* A detailed description of what's being changed, including any outstanding work + (TODOs) +* A list of security considerations, which contains information about anything + we need to be mindful of from a security compliance perspective +* The proper labels, assignee, code reviewer, and other project metadata set + + +### Getting Started + +When you first open a pull request, start off by making sure the metadata for it +is in place: + +* Provide an appropriate and descriptive title for the pull request +* Link the pull request to its corresponding issue (must be done after creating + the pull request itself) +* Assign yourself as the author +* Attach the appropriate labels to it +* Set it to be on the Notify.gov project board +* Select one or more reviewers from the team or mark the pull request as a draft + depending on its current state + * If the pull request is a draft, please be sure to add reviewers once it is + ready for review and mark it ready for review + +### Description + +Please enter a clear description about your proposed changes and what the +expected outcome(s) is/are from there. If there are complex implementation +details within the changes, this is a great place to explain those details using +plain language. + +This should include: + +* Links to issues that this PR addresses (especially if more than one) +* Screenshots or screen captures of any visible changes, especially for UI work +* Dependency changes + +If there are any caveats, known issues, follow-up items, etc., make a quick note +of them here as well, though more details are probably warranted in the issue +itself in this case. + +### TODO (optional) + +If you're opening a draft PR, it might be helpful to list any outstanding work, +especially if you're asking folks to take a look before it's ready for full +review. In this case, create a small checklist with the outstanding items: + +* [ ] TODO item 1 +* [ ] TODO item 2 +* [ ] TODO item ... + +### Security Considerations + +Please think about the security compliance aspect of your changes and what the +potential impacts might be. + +**NOTE: Please be mindful of sharing sensitive information here! If you're not sure of what to write, please ask the team first before writing anything here.** + +Relevant details could include (and are not limited to) the following: + +* Handling secrets/credential management (or specifically calling out that there + is nothing to handle) +* Any adjustments to the flow of data in and out the system, or even within it +* Connecting or disconnecting any external services to the application +* Handling of any sensitive information, such as PII +* Handling of information within log statements or other application monitoring + services/hooks +* The inclusion of a new external dependency or the removal of an existing one +* ... (anything else relevant from a security compliance perspective) + +There are some cases where there are no security considerations to be had, e.g., +updating our documentation with publicly available information. In those cases +it is fine to simply put something like this: + +* None; this is a documentation update with publicly available information. + +This way it shows that we still gave this section consideration and that nothing +happens to apply in this scenario. + + Code Reviews ============ @@ -856,19 +955,19 @@ behavior and lack of professionalism is not acceptable or tolerated.** When performing a code review, it is helpful to keep the following guidelines in mind: -- Be on the lookout for any sensitive information and/or leaked credentials, +* Be on the lookout for any sensitive information and/or leaked credentials, secrets, PII, etc. -- Ask and call out things that aren't clear to you; it never hurts to double +* Ask and call out things that aren't clear to you; it never hurts to double check your understanding of something! -- Check that things are named descriptively and appropriately and call out +* Check that things are named descriptively and appropriately and call out anything that is not. -- Check that comments are present for complex areas when needed. -- Make sure the pull request itself is properly prepared - it has a clear +* Check that comments are present for complex areas when needed. +* Make sure the pull request itself is properly prepared - it has a clear description, calls out security concerns, and has the necessary labels, flags, issue link, etc., set on it. -- Do not be shy about using the suggested changes feature in GitHub pull request +* Do not be shy about using the suggested changes feature in GitHub pull request comments; this can help save a lot of time! -- Do not be shy about marking a review with the `Request Changes` status - yes, +* Do not be shy about marking a review with the `Request Changes` status - yes, it looks big and red when it shows up, but this is completely fine and not to be taken as a personal mark against the author(s) of the pull request! @@ -896,14 +995,14 @@ behavior and lack of professionalism is not acceptable or tolerated.** When going over a review, it may be helpful to keep these perspectives in mind: -- Approach the review with an open mind, curiosity, and appreciation. -- If anything the reviewer(s) mentions is unclear to you, please ask for +* Approach the review with an open mind, curiosity, and appreciation. +* If anything the reviewer(s) mentions is unclear to you, please ask for clarification and engage them in further dialogue! -- If you disagree with a suggestion or request, please say so and engage in an +* If you disagree with a suggestion or request, please say so and engage in an open and respecful dialogue to come to a mutual understanding of what the appropriate next step(S) should be - accept the change, reject the change, take a different path entirely, etc. -- If there are no issues with any suggested edits or requested changes, make +* If there are no issues with any suggested edits or requested changes, make the necessary adjustments and let the reviewer(s) know when the work is ready for review again. @@ -1224,3 +1323,40 @@ Data Retention Policy Seven (7) days by default. Each service can be set with a custom policy via `ServiceDataRetention` by a Platform Admin. The `ServiceDataRetention` setting applies per-service and per-message type and controls both entries in the `notifications` table as well as `csv` contact files uploaded to s3 Data cleanup is controlled by several tasks in the `nightly_tasks.py` file, kicked off by Celery Beat. + + +# Debug messages not being sent + + +## Getting the file location and tracing what happens + + +Ask the user to provide the csv file name. Either the csv file they uploaded, or the one that is autogenerated when they do a one-off send and is visible in the UI + +Starting with the admin logs, search for this file name. When you find it, the log line should have the file name linked to the job_id and the csv file location. Save both of these. + +In the api logs, search by job_id. Either you will see evidence of the job failing and retrying over and over (in which case search for a stack trace using timestamp), or you will ultimately get to a log line that links the job_id to a message_id. In this case, now search by message_id. You should be able to find the actual result from AWS, either success or failure, with hopefully some helpful info. + +## Viewing the csv file + +If you need to view the questionable csv file on production, run the following command: + + +``` +cf run-task notify-api-production --command "flask command download-csv-file-by-name -f " +``` + +locally, just do: + +``` +poetry run flask command download-csv-file-by-name -f +``` + +## Debug steps + +1. Either send a message and capture the csv file name, or get a csv file name from a user +2. Using the log tool at logs.fr.cloud.gov, use filters to limit what you're searching on (cf.app is 'notify-admin-production' for example) and then search with the csv file name in double quotes over the relevant time period (last 5 minutes if you just sent a message, or else whatever time the user sent at) +3. When you find the log line, you should also find the job_id and the s3 file location. Save these somewhere. +4. To get the csv file contents, you can run the command above. This command currently prints to the notify-api log, so after you run the command, +you need to search in notify-api-production for the last 5 minutes with the logs sorted by timestamp. The contents of the csv file unfortunately appear on separate lines so it's very important to sort by time. +5. If you want to see where the message actually failed, search with cf.app is notify-api-production using the job_id that you saved in step #3. If you get far enough, you might see one of the log lines has a message_id. If you see it, you can switch and search on that, which should tell you what happened in AWS (success or failure). diff --git a/migrations/versions/0025_notify_service_data.py b/migrations/versions/0025_notify_service_data.py index 0683e7dd2..e90d01aad 100644 --- a/migrations/versions/0025_notify_service_data.py +++ b/migrations/versions/0025_notify_service_data.py @@ -15,6 +15,7 @@ from alembic import op from sqlalchemy import text from app.hashing import hashpw +from app.utils import utc_now revision = "0025_notify_service_data" down_revision = "0024_add_research_mode_defaults" @@ -32,7 +33,7 @@ def upgrade(): """ conn.execute( text(user_insert), - {"user_id": user_id, "time_now": datetime.utcnow(), "password": password}, + {"user_id": user_id, "time_now": utc_now(), "password": password}, ) service_history_insert = """INSERT INTO services_history (id, name, created_at, active, message_limit, restricted, research_mode, email_from, created_by_id, reply_to_email_address, version) VALUES (:service_id, 'Notify service', :time_now, True, 1000, False, False, 'testsender@dispostable.com', @@ -41,7 +42,7 @@ def upgrade(): """ conn.execute( text(service_history_insert), - {"service_id": service_id, "time_now": datetime.utcnow(), "user_id": user_id}, + {"service_id": service_id, "time_now": utc_now(), "user_id": user_id}, ) service_insert = """INSERT INTO services (id, name, created_at, active, message_limit, restricted, research_mode, email_from, created_by_id, reply_to_email_address, version) VALUES (:service_id, 'Notify service', :time_now, True, 1000, False, False, 'testsender@dispostable.com', @@ -49,7 +50,7 @@ def upgrade(): """ conn.execute( text(service_insert), - {"service_id": service_id, "time_now": datetime.utcnow(), "user_id": user_id}, + {"service_id": service_id, "time_now": utc_now(), "user_id": user_id}, ) user_to_service_insert = """INSERT INTO user_to_service (user_id, service_id) VALUES (:user_id, :service_id)""" conn.execute( @@ -74,7 +75,7 @@ def upgrade(): "template_id": uuid.uuid4(), "template_name": "Notify email verification code", "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": email_verification_content, "service_id": service_id, "subject": "Confirm GOV.UK Notify registration", @@ -87,7 +88,7 @@ def upgrade(): "template_id": "ece42649-22a8-4d06-b87f-d52d5d3f0a27", "template_name": "Notify email verification code", "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": email_verification_content, "service_id": service_id, "subject": "Confirm GOV.UK Notify registration", @@ -107,7 +108,7 @@ def upgrade(): "template_id": "4f46df42-f795-4cc4-83bb-65ca312f49cc", "template_name": "Notify invitation email", "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": invitation_content, "service_id": service_id, "subject": invitation_subject, @@ -120,7 +121,7 @@ def upgrade(): "template_id": "4f46df42-f795-4cc4-83bb-65ca312f49cc", "template_name": "Notify invitation email", "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": invitation_content, "service_id": service_id, "subject": invitation_subject, @@ -135,7 +136,7 @@ def upgrade(): "template_id": "36fb0730-6259-4da1-8a80-c8de22ad4246", "template_name": "Notify SMS verify code", "template_type": "sms", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": sms_code_content, "service_id": service_id, "subject": None, @@ -149,7 +150,7 @@ def upgrade(): "template_id": "36fb0730-6259-4da1-8a80-c8de22ad4246", "template_name": "Notify SMS verify code", "template_type": "sms", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": sms_code_content, "service_id": service_id, "subject": None, @@ -172,7 +173,7 @@ def upgrade(): "template_id": "474e9242-823b-4f99-813d-ed392e7f1201", "template_name": "Notify password reset email", "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": password_reset_content, "service_id": service_id, "subject": "Reset your GOV.UK Notify password", @@ -186,7 +187,7 @@ def upgrade(): "template_id": "474e9242-823b-4f99-813d-ed392e7f1201", "template_name": "Notify password reset email", "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": password_reset_content, "service_id": service_id, "subject": "Reset your GOV.UK Notify password", diff --git a/migrations/versions/0028_fix_reg_template_history.py b/migrations/versions/0028_fix_reg_template_history.py index 8bf11fe59..fcbffc51a 100644 --- a/migrations/versions/0028_fix_reg_template_history.py +++ b/migrations/versions/0028_fix_reg_template_history.py @@ -11,6 +11,8 @@ from datetime import datetime from sqlalchemy import text +from app.utils import utc_now + revision = "0028_fix_reg_template_history" down_revision = "0026_rename_notify_service" @@ -38,7 +40,7 @@ def upgrade(): "id": "ece42649-22a8-4d06-b87f-d52d5d3f0a27", "name": "Notify email verification code", "type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": email_verification_content, "service_id": service_id, "subject": "Confirm GOV.UK Notify registration", diff --git a/migrations/versions/0082_add_golive_template.py b/migrations/versions/0082_add_golive_template.py index 55cdc3e04..97ff5b97e 100644 --- a/migrations/versions/0082_add_golive_template.py +++ b/migrations/versions/0082_add_golive_template.py @@ -14,6 +14,8 @@ from alembic import op from flask import current_app from sqlalchemy import text +from app.utils import utc_now + revision = "0082_add_go_live_template" down_revision = "0081_noti_status_as_enum" @@ -89,7 +91,7 @@ GOV.UK Notify team "template_id": template_id, "template_name": template_name, "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": template_content, "notify_service_id": current_app.config["NOTIFY_SERVICE_ID"], "subject": template_subject, diff --git a/migrations/versions/0117_international_sms_notify.py b/migrations/versions/0117_international_sms_notify.py index ebdbbddef..c33750af4 100644 --- a/migrations/versions/0117_international_sms_notify.py +++ b/migrations/versions/0117_international_sms_notify.py @@ -9,6 +9,8 @@ Create Date: 2017-08-29 14:09:41.042061 # revision identifiers, used by Alembic. from sqlalchemy import text +from app.utils import utc_now + revision = "0117_international_sms_notify" down_revision = "0115_add_inbound_numbers" @@ -22,7 +24,7 @@ NOTIFY_SERVICE_ID = "d6aa2c68-a2d9-4437-ab19-3ae8eb202553" def upgrade(): input_params = { "notify_service_id": NOTIFY_SERVICE_ID, - "datetime_now": datetime.utcnow(), + "datetime_now": utc_now(), } conn = op.get_bind() conn.execute( diff --git a/migrations/versions/0134_add_email_2fa_template_.py b/migrations/versions/0134_add_email_2fa_template_.py index 492281175..57809e1bc 100644 --- a/migrations/versions/0134_add_email_2fa_template_.py +++ b/migrations/versions/0134_add_email_2fa_template_.py @@ -12,6 +12,8 @@ from alembic import op from flask import current_app from sqlalchemy import text +from app.utils import utc_now + revision = "0134_add_email_2fa_template" down_revision = "0133_set_services_sms_prefix" @@ -44,7 +46,7 @@ def upgrade(): "template_id": template_id, "template_name": template_name, "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": template_content, "notify_service_id": current_app.config["NOTIFY_SERVICE_ID"], "subject": template_subject, diff --git a/migrations/versions/0139_migrate_sms_allowance_data.py b/migrations/versions/0139_migrate_sms_allowance_data.py index 8e7536bfb..0203f5563 100644 --- a/migrations/versions/0139_migrate_sms_allowance_data.py +++ b/migrations/versions/0139_migrate_sms_allowance_data.py @@ -13,6 +13,7 @@ from alembic import op from sqlalchemy import text from app.dao.date_util import get_current_calendar_year_start_year +from app.utils import utc_now revision = "0139_migrate_sms_allowance_data" down_revision = "0138_sms_sender_nullable" @@ -34,7 +35,7 @@ def upgrade(): input_params = { "current_year": current_year, "default_limit": default_limit, - "time_now": datetime.utcnow(), + "time_now": utc_now(), } insert_row_if_not_exist = """ INSERT INTO annual_billing diff --git a/migrations/versions/0171_add_org_invite_template.py b/migrations/versions/0171_add_org_invite_template.py index 5ec1925da..7c0b9df09 100644 --- a/migrations/versions/0171_add_org_invite_template.py +++ b/migrations/versions/0171_add_org_invite_template.py @@ -12,6 +12,8 @@ from alembic import op from flask import current_app from sqlalchemy import text +from app.utils import utc_now + revision = "0171_add_org_invite_template" down_revision = "0170_hidden_non_nullable" @@ -53,7 +55,7 @@ def upgrade(): "template_id": template_id, "template_name": template_name, "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": template_content, "notify_service_id": current_app.config["NOTIFY_SERVICE_ID"], "subject": template_subject, diff --git a/migrations/versions/0265_add_confirm_edit_templates.py b/migrations/versions/0265_add_confirm_edit_templates.py index 378891313..ad8d2470e 100644 --- a/migrations/versions/0265_add_confirm_edit_templates.py +++ b/migrations/versions/0265_add_confirm_edit_templates.py @@ -12,6 +12,8 @@ from alembic import op from flask import current_app from sqlalchemy import text +from app.utils import utc_now + revision = "0265_add_confirm_edit_templates" down_revision = "0264_add_folder_permissions_perm" @@ -57,7 +59,7 @@ def upgrade(): "template_id": email_template_id, "template_name": email_template_name, "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": email_template_content, "notify_service_id": current_app.config["NOTIFY_SERVICE_ID"], "subject": email_template_subject, @@ -78,7 +80,7 @@ def upgrade(): "template_id": mobile_template_id, "template_name": mobile_template_name, "template_type": "sms", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": mobile_template_content, "notify_service_id": current_app.config["NOTIFY_SERVICE_ID"], "subject": None, diff --git a/migrations/versions/0294_add_verify_reply_to_.py b/migrations/versions/0294_add_verify_reply_to_.py index 305c52997..d37f75ea0 100644 --- a/migrations/versions/0294_add_verify_reply_to_.py +++ b/migrations/versions/0294_add_verify_reply_to_.py @@ -12,6 +12,8 @@ from alembic import op from flask import current_app from sqlalchemy import text +from app.utils import utc_now + revision = "0294_add_verify_reply_to" down_revision = "0293_drop_complaint_fk" @@ -58,7 +60,7 @@ def upgrade(): "template_id": email_template_id, "template_name": email_template_name, "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": email_template_content, "notify_service_id": current_app.config["NOTIFY_SERVICE_ID"], "subject": email_template_subject, diff --git a/migrations/versions/0330_broadcast_invite_email.py b/migrations/versions/0330_broadcast_invite_email.py index 24dc60c68..bcd865d46 100644 --- a/migrations/versions/0330_broadcast_invite_email.py +++ b/migrations/versions/0330_broadcast_invite_email.py @@ -12,6 +12,8 @@ from datetime import datetime from alembic import op from sqlalchemy import text +from app.utils import utc_now + revision = "0330_broadcast_invite_email" down_revision = "0329_purge_broadcast_data" @@ -60,7 +62,7 @@ def upgrade(): input_params = { "template_id": template_id, "template_name": broadcast_invitation_template_name, - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": broadcast_invitation_content, "service_id": service_id, "subject": broadcast_invitation_subject, diff --git a/migrations/versions/0347_add_dvla_volumes_template.py b/migrations/versions/0347_add_dvla_volumes_template.py index 6b610c16c..f32821b79 100644 --- a/migrations/versions/0347_add_dvla_volumes_template.py +++ b/migrations/versions/0347_add_dvla_volumes_template.py @@ -13,6 +13,8 @@ from alembic import op from flask import current_app from sqlalchemy import text +from app.utils import utc_now + revision = "0347_add_dvla_volumes_template" down_revision = "0346_notify_number_sms_sender" @@ -57,7 +59,7 @@ def upgrade(): "template_id": email_template_id, "template_name": email_template_name, "template_type": "email", - "time_now": datetime.utcnow(), + "time_now": utc_now(), "content": email_template_content, "notify_service_id": current_app.config["NOTIFY_SERVICE_ID"], "subject": email_template_subject, diff --git a/migrations/versions/0401_add_e2e_test_user.py b/migrations/versions/0401_add_e2e_test_user.py index d3d83afad..e99a6af8a 100644 --- a/migrations/versions/0401_add_e2e_test_user.py +++ b/migrations/versions/0401_add_e2e_test_user.py @@ -16,6 +16,7 @@ from alembic import op from app import db from app.dao.users_dao import get_user_by_email from app.models import User +from app.utils import utc_now revision = "0401_add_e2e_test_user" down_revision = "0400_add_total_message_limit" @@ -32,11 +33,11 @@ def upgrade(): "password": password, "mobile_number": "+12025555555", "state": "active", - "created_at": datetime.datetime.utcnow(), - "password_changed_at": datetime.datetime.utcnow(), + "created_at": utc_now(), + "password_changed_at": utc_now(), "failed_login_count": 0, "platform_admin": "f", - "email_access_validated_at": datetime.datetime.utcnow(), + "email_access_validated_at": utc_now(), } conn = op.get_bind() insert_sql = """ diff --git a/notifications_utils/recipients.py b/notifications_utils/recipients.py index 68e2cb101..34cd2def2 100644 --- a/notifications_utils/recipients.py +++ b/notifications_utils/recipients.py @@ -602,6 +602,26 @@ def validate_us_phone_number(number): raise InvalidPhoneError(exc._msg) from exc +def show_mangled_number_clues(number): + + translator = { + "1": "X", + "2": "X", + "3": "X", + "4": "X", + "5": "X", + "6": "X", + "7": "X", + "8": "X", + "9": "X", + "0": "X", + } + for key in translator: + number = number.replace(key, translator[key]) + + return number + + def validate_phone_number(number, international=False): if (not international) or is_us_phone_number(number): return validate_us_phone_number(number) @@ -619,7 +639,11 @@ def validate_phone_number(number, international=False): except NumberParseException as exc: if exc._msg == "Could not interpret numbers after plus-sign.": raise InvalidPhoneError("Not a valid country prefix") from exc - raise InvalidPhoneError(exc._msg) from exc + if not isinstance(number, str): + raise InvalidPhoneError(f"Number must be string, not type {type(number)}") + raise InvalidPhoneError( + f"Invalid phone number looks like {show_mangled_number_clues(number)} {exc._msg}" + ) validate_and_format_phone_number = validate_phone_number diff --git a/poetry.lock b/poetry.lock index 3283f820d..1e10ead84 100644 --- a/poetry.lock +++ b/poetry.lock @@ -111,13 +111,13 @@ frozenlist = ">=1.1.0" [[package]] name = "alembic" -version = "1.13.1" +version = "1.13.2" description = "A database migration tool for SQLAlchemy." optional = false python-versions = ">=3.8" files = [ - {file = "alembic-1.13.1-py3-none-any.whl", hash = "sha256:2edcc97bed0bd3272611ce3a98d98279e9c209e7186e43e75bbb1b2bdfdbcc43"}, - {file = "alembic-1.13.1.tar.gz", hash = "sha256:4932c8558bf68f2ee92b9bbcb8218671c627064d5b08939437af6d77dc05e595"}, + {file = "alembic-1.13.2-py3-none-any.whl", hash = "sha256:6b8733129a6224a9a711e17c99b08462dbf7cc9670ba8f2e2ae9af860ceb1953"}, + {file = "alembic-1.13.2.tar.gz", hash = "sha256:1ff0ae32975f4fd96028c39ed9bb3c867fe3af956bd7bb37343b54c9fe7445ef"}, ] [package.dependencies] @@ -204,17 +204,17 @@ tests-no-zope = ["attrs[tests-mypy]", "cloudpickle", "hypothesis", "pympler", "p [[package]] name = "awscli" -version = "1.33.15" +version = "1.33.21" description = "Universal Command Line Environment for AWS." optional = false python-versions = ">=3.8" files = [ - {file = "awscli-1.33.15-py3-none-any.whl", hash = "sha256:5a8d7e68a4cf68afc9d9ba4bef511526eb71027360f95a1080d39158bc930083"}, - {file = "awscli-1.33.15.tar.gz", hash = "sha256:54a8089edb6756da46addcfcd56fdca21307a121216a81ef542e17b284cbe9c9"}, + {file = "awscli-1.33.21-py3-none-any.whl", hash = "sha256:92e5f8a0f5e3497459f1711ef4044ac1e60ff8d2058e889c63aa6929d5c67459"}, + {file = "awscli-1.33.21.tar.gz", hash = "sha256:d0a7209e323c85b28d85cffa9470fff664d5f861bb3b5bda843329e0836f5760"}, ] [package.dependencies] -botocore = "1.34.133" +botocore = "1.34.139" colorama = ">=0.2.5,<0.4.7" docutils = ">=0.10,<0.17" PyYAML = ">=3.10,<6.1" @@ -403,17 +403,17 @@ files = [ [[package]] name = "boto3" -version = "1.34.131" +version = "1.34.138" description = "The AWS SDK for Python" optional = false python-versions = ">=3.8" files = [ - {file = "boto3-1.34.131-py3-none-any.whl", hash = "sha256:05e388cb937e82be70bfd7eb0c84cf8011ff35cf582a593873ac21675268683b"}, - {file = "boto3-1.34.131.tar.gz", hash = "sha256:dab8f72a6c4e62b4fd70da09e08a6b2a65ea2115b27dd63737142005776ef216"}, + {file = "boto3-1.34.138-py3-none-any.whl", hash = "sha256:81518aa95fad71279411fb5c94da4b4a554a5d53fc876faca62b7b5c8737f1cb"}, + {file = "boto3-1.34.138.tar.gz", hash = "sha256:f79c15e33eb7706f197d98d828b193cf0891966682ad3ec5e900f6f9e7362e35"}, ] [package.dependencies] -botocore = ">=1.34.131,<1.35.0" +botocore = ">=1.34.138,<1.35.0" jmespath = ">=0.7.1,<2.0.0" s3transfer = ">=0.10.0,<0.11.0" @@ -422,13 +422,13 @@ crt = ["botocore[crt] (>=1.21.0,<2.0a0)"] [[package]] name = "botocore" -version = "1.34.133" +version = "1.34.139" description = "Low-level, data-driven core of boto 3." optional = false python-versions = ">=3.8" files = [ - {file = "botocore-1.34.133-py3-none-any.whl", hash = "sha256:f269dad8e17432d2527b97ed9f1fd30ec8dc705f8b818957170d1af484680ef2"}, - {file = "botocore-1.34.133.tar.gz", hash = "sha256:5ea609aa4831a6589e32eef052a359ad8d7311733b4d86a9d35dab4bd3ec80ff"}, + {file = "botocore-1.34.139-py3-none-any.whl", hash = "sha256:dd1e085d4caa2a4c1b7d83e3bc51416111c8238a35d498e9d3b04f3b63b086ba"}, + {file = "botocore-1.34.139.tar.gz", hash = "sha256:df023d8cf8999d574214dad4645cb90f9d2ccd1494f6ee2b57b1ab7522f6be77"}, ] [package.dependencies] @@ -553,13 +553,13 @@ zstd = ["zstandard (==0.22.0)"] [[package]] name = "certifi" -version = "2024.6.2" +version = "2024.7.4" description = "Python package for providing Mozilla's CA Bundle." optional = false python-versions = ">=3.6" files = [ - {file = "certifi-2024.6.2-py3-none-any.whl", hash = "sha256:ddc6c8ce995e6987e7faf5e3f1b02b302836a0e5d98ece18392cb1a36c72ad56"}, - {file = "certifi-2024.6.2.tar.gz", hash = "sha256:3cd43f1c6fa7dedc5899d69d3ad0398fd018ad1a17fba83ddaf78aa46c747516"}, + {file = "certifi-2024.7.4-py3-none-any.whl", hash = "sha256:c198e21b1289c2ab85ee4e67bb4b4ef3ead0892059901a8d5b622f24a1101e90"}, + {file = "certifi-2024.7.4.tar.gz", hash = "sha256:5a1e7645bc0ec61a09e26c36f6106dd4cf40c6db3a1fb6352b0244e7fb057c7b"}, ] [[package]] @@ -1261,13 +1261,13 @@ tests = ["coverage", "coveralls", "dill", "mock", "nose"] [[package]] name = "faker" -version = "25.8.0" +version = "26.0.0" description = "Faker is a Python package that generates fake data for you." optional = false python-versions = ">=3.8" files = [ - {file = "Faker-25.8.0-py3-none-any.whl", hash = "sha256:4c40b34a9c569018d4f9d6366d71a4da8a883d5ddf2b23197be5370f29b7e1b6"}, - {file = "Faker-25.8.0.tar.gz", hash = "sha256:bdec5f2fb057d244ebef6e0ed318fea4dcbdf32c3a1a010766fc45f5d68fc68d"}, + {file = "Faker-26.0.0-py3-none-any.whl", hash = "sha256:886ee28219be96949cd21ecc96c4c742ee1680e77f687b095202c8def1a08f06"}, + {file = "Faker-26.0.0.tar.gz", hash = "sha256:0f60978314973de02c00474c2ae899785a42b2cf4f41b7987e93c132a2b8a4a9"}, ] [package.dependencies] @@ -1305,18 +1305,18 @@ typing = ["typing-extensions (>=4.8)"] [[package]] name = "flake8" -version = "7.0.0" +version = "7.1.0" description = "the modular source code checker: pep8 pyflakes and co" optional = false python-versions = ">=3.8.1" files = [ - {file = "flake8-7.0.0-py2.py3-none-any.whl", hash = "sha256:a6dfbb75e03252917f2473ea9653f7cd799c3064e54d4c8140044c5c065f53c3"}, - {file = "flake8-7.0.0.tar.gz", hash = "sha256:33f96621059e65eec474169085dc92bf26e7b2d47366b70be2f67ab80dc25132"}, + {file = "flake8-7.1.0-py2.py3-none-any.whl", hash = "sha256:2e416edcc62471a64cea09353f4e7bdba32aeb079b6e360554c659a122b1bc6a"}, + {file = "flake8-7.1.0.tar.gz", hash = "sha256:48a07b626b55236e0fb4784ee69a465fbf59d79eec1f5b4785c3d3bc57d17aa5"}, ] [package.dependencies] mccabe = ">=0.7.0,<0.8.0" -pycodestyle = ">=2.11.0,<2.12.0" +pycodestyle = ">=2.12.0,<2.13.0" pyflakes = ">=3.2.0,<3.3.0" [[package]] @@ -2389,13 +2389,13 @@ files = [ [[package]] name = "moto" -version = "5.0.9" +version = "5.0.10" description = "" optional = false python-versions = ">=3.8" files = [ - {file = "moto-5.0.9-py2.py3-none-any.whl", hash = "sha256:21a13e02f83d6a18cfcd99949c96abb2e889f4bd51c4c6a3ecc8b78765cb854e"}, - {file = "moto-5.0.9.tar.gz", hash = "sha256:eb71f1cba01c70fff1f16086acb24d6d9aeb32830d646d8989f98a29aeae24ba"}, + {file = "moto-5.0.10-py2.py3-none-any.whl", hash = "sha256:9ffae2f64cc8fe95b9a12d63ae7268a7d6bea9993b922905b5abd8197d852cd0"}, + {file = "moto-5.0.10.tar.gz", hash = "sha256:eff37363221c93ea44f95721ae0ddb56f977fe70437a041b6cc641ee90266279"}, ] [package.dependencies] @@ -2826,13 +2826,13 @@ ptyprocess = ">=0.5" [[package]] name = "phonenumbers" -version = "8.13.39" +version = "8.13.40" description = "Python version of Google's common library for parsing, formatting, storing and validating international phone numbers." optional = false python-versions = "*" files = [ - {file = "phonenumbers-8.13.39-py2.py3-none-any.whl", hash = "sha256:3ad2d086fa71e7eef409001b9195ac54bebb0c6e3e752209b558ca192c9229a0"}, - {file = "phonenumbers-8.13.39.tar.gz", hash = "sha256:db7ca4970d206b2056231105300753b1a5b229f43416f8c2b3010e63fbb68d77"}, + {file = "phonenumbers-8.13.40-py2.py3-none-any.whl", hash = "sha256:9582752c20a1da5ec4449f7f97542bf8a793c8e2fec0ab57f767177bb8fc0b1d"}, + {file = "phonenumbers-8.13.40.tar.gz", hash = "sha256:f137c2848b8e83dd064b71881b65680584417efa202177fd330e2f7ff6c68113"}, ] [[package]] @@ -3210,13 +3210,13 @@ files = [ [[package]] name = "pycodestyle" -version = "2.11.1" +version = "2.12.0" description = "Python style guide checker" optional = false python-versions = ">=3.8" files = [ - {file = "pycodestyle-2.11.1-py2.py3-none-any.whl", hash = "sha256:44fe31000b2d866f2e41841b18528a505fbd7fef9017b04eff4e2648a0fadc67"}, - {file = "pycodestyle-2.11.1.tar.gz", hash = "sha256:41ba0e7afc9752dfb53ced5489e89f8186be00e599e712660695b7a75ff2663f"}, + {file = "pycodestyle-2.12.0-py2.py3-none-any.whl", hash = "sha256:949a39f6b86c3e1515ba1787c2022131d165a8ad271b11370a8819aa070269e4"}, + {file = "pycodestyle-2.12.0.tar.gz", hash = "sha256:442f950141b4f43df752dd303511ffded3a04c2b6fb7f65980574f0c31e6e79c"}, ] [[package]] @@ -3634,13 +3634,13 @@ full = ["numpy"] [[package]] name = "redis" -version = "5.0.6" +version = "5.0.7" description = "Python client for Redis database and key-value store" optional = false python-versions = ">=3.7" files = [ - {file = "redis-5.0.6-py3-none-any.whl", hash = "sha256:c0d6d990850c627bbf7be01c5c4cbaadf67b48593e913bb71c9819c30df37eee"}, - {file = "redis-5.0.6.tar.gz", hash = "sha256:38473cd7c6389ad3e44a91f4c3eaf6bcb8a9f746007f29bf4fb20824ff0b2197"}, + {file = "redis-5.0.7-py3-none-any.whl", hash = "sha256:0e479e24da960c690be5d9b96d21f7b918a98c0cf49af3b6fafaa0753f93a0db"}, + {file = "redis-5.0.7.tar.gz", hash = "sha256:8f611490b93c8109b50adc317b31bfd84fff31def3475b92e7e80bf39f48175b"}, ] [package.extras] @@ -3988,13 +3988,13 @@ pyasn1 = ">=0.1.3" [[package]] name = "s3transfer" -version = "0.10.1" +version = "0.10.2" description = "An Amazon S3 Transfer Manager" optional = false -python-versions = ">= 3.8" +python-versions = ">=3.8" files = [ - {file = "s3transfer-0.10.1-py3-none-any.whl", hash = "sha256:ceb252b11bcf87080fb7850a224fb6e05c8a776bab8f2b64b7f25b969464839d"}, - {file = "s3transfer-0.10.1.tar.gz", hash = "sha256:5683916b4c724f799e600f41dd9e10a9ff19871bf87623cc8f491cb4f5fa0a19"}, + {file = "s3transfer-0.10.2-py3-none-any.whl", hash = "sha256:eca1c20de70a39daee580aef4986996620f365c4e0fda6a86100231d62f1bf69"}, + {file = "s3transfer-0.10.2.tar.gz", hash = "sha256:0711534e9356d3cc692fdde846b4a1e4b0cb6519971860796e6bc4c7aea00ef6"}, ] [package.dependencies] @@ -4020,18 +4020,18 @@ jeepney = ">=0.6" [[package]] name = "setuptools" -version = "70.1.1" +version = "70.2.0" description = "Easily download, build, install, upgrade, and uninstall Python packages" optional = false python-versions = ">=3.8" files = [ - {file = "setuptools-70.1.1-py3-none-any.whl", hash = "sha256:a58a8fde0541dab0419750bcc521fbdf8585f6e5cb41909df3a472ef7b81ca95"}, - {file = "setuptools-70.1.1.tar.gz", hash = "sha256:937a48c7cdb7a21eb53cd7f9b59e525503aa8abaf3584c730dc5f7a5bec3a650"}, + {file = "setuptools-70.2.0-py3-none-any.whl", hash = "sha256:b8b8060bb426838fbe942479c90296ce976249451118ef566a5a0b7d8b78fb05"}, + {file = "setuptools-70.2.0.tar.gz", hash = "sha256:bd63e505105011b25c3c11f753f7e3b8465ea739efddaccef8f0efac2137bac1"}, ] [package.extras] -docs = ["furo", "jaraco.packaging (>=9.3)", "jaraco.tidelift (>=1.4)", "pygments-github-lexers (==0.0.5)", "pyproject-hooks (!=1.1)", "rst.linker (>=1.9)", "sphinx (>=3.5)", "sphinx-favicon", "sphinx-inline-tabs", "sphinx-lint", "sphinx-notfound-page (>=1,<2)", "sphinx-reredirects", "sphinxcontrib-towncrier"] -testing = ["build[virtualenv] (>=1.0.3)", "filelock (>=3.4.0)", "importlib-metadata", "ini2toml[lite] (>=0.14)", "jaraco.develop (>=7.21)", "jaraco.envs (>=2.2)", "jaraco.path (>=3.2.0)", "jaraco.test", "mypy (==1.10.0)", "packaging (>=23.2)", "pip (>=19.1)", "pyproject-hooks (!=1.1)", "pytest (>=6,!=8.1.1)", "pytest-checkdocs (>=2.4)", "pytest-cov", "pytest-enabler (>=2.2)", "pytest-home (>=0.5)", "pytest-mypy", "pytest-perf", "pytest-ruff (>=0.3.2)", "pytest-subprocess", "pytest-timeout", "pytest-xdist (>=3)", "tomli", "tomli-w (>=1.0.0)", "virtualenv (>=13.0.0)", "wheel"] +doc = ["furo", "jaraco.packaging (>=9.3)", "jaraco.tidelift (>=1.4)", "pygments-github-lexers (==0.0.5)", "pyproject-hooks (!=1.1)", "rst.linker (>=1.9)", "sphinx (>=3.5)", "sphinx-favicon", "sphinx-inline-tabs", "sphinx-lint", "sphinx-notfound-page (>=1,<2)", "sphinx-reredirects", "sphinxcontrib-towncrier"] +test = ["build[virtualenv] (>=1.0.3)", "filelock (>=3.4.0)", "importlib-metadata", "ini2toml[lite] (>=0.14)", "jaraco.develop (>=7.21)", "jaraco.envs (>=2.2)", "jaraco.path (>=3.2.0)", "jaraco.test", "mypy (==1.10.0)", "packaging (>=23.2)", "pip (>=19.1)", "pyproject-hooks (!=1.1)", "pytest (>=6,!=8.1.*)", "pytest-checkdocs (>=2.4)", "pytest-cov", "pytest-enabler (>=2.2)", "pytest-home (>=0.5)", "pytest-mypy", "pytest-perf", "pytest-ruff (>=0.3.2)", "pytest-subprocess", "pytest-timeout", "pytest-xdist (>=3)", "tomli", "tomli-w (>=1.0.0)", "virtualenv (>=13.0.0)", "wheel"] [[package]] name = "shapely" @@ -4751,4 +4751,4 @@ multidict = ">=4.0" [metadata] lock-version = "2.0" python-versions = "^3.12.2" -content-hash = "b1b4bfbfdc1f5cc9ae9d090f35b235a62e9dbabc683a5b5a1d0d414605219b48" +content-hash = "74d41976bb5028dce7b953ee4c2f6108a46215c8e48d2a4b2152d0277a63b395" diff --git a/pyproject.toml b/pyproject.toml index 2ea004520..627c97d67 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,11 +8,11 @@ readme = "README.md" [tool.poetry.dependencies] python = "^3.12.2" -alembic = "==1.13.1" +alembic = "==1.13.2" amqp = "==5.2.0" beautifulsoup4 = "==4.12.3" -boto3 = "^1.34.131" -botocore = "^1.34.133" +boto3 = "^1.34.138" +botocore = "^1.34.139" cachetools = "==5.3.3" celery = {version = "==5.4.0", extras = ["redis"]} certifi = ">=2022.12.7" @@ -48,14 +48,14 @@ pyjwt = "==2.8.0" python-dotenv = "==1.0.1" sqlalchemy = "==2.0.31" werkzeug = "^3.0.3" -faker = "^25.8.0" +faker = "^26.0.0" async-timeout = "^4.0.3" bleach = "^6.1.0" geojson = "^3.1.0" govuk-bank-holidays = "^0.14" numpy = "^1.26.4" ordered-set = "^4.1.0" -phonenumbers = "^8.13.39" +phonenumbers = "^8.13.40" python-json-logger = "^2.0.7" pytz = "^2024.1" regex = "^2024.5.15" @@ -70,13 +70,13 @@ markupsafe = "^2.1.5" pycparser = "^2.22" python-dateutil = "^2.9.0.post0" pyyaml = "^6.0.1" -s3transfer = "^0.10.1" +s3transfer = "^0.10.2" six = "^1.16.0" urllib3 = "^2.2.2" webencodings = "^0.5.1" itsdangerous = "^2.2.0" jinja2 = "^3.1.4" -redis = "^5.0.6" +redis = "^5.0.7" requests = "^2.32.3" @@ -86,13 +86,13 @@ bandit = "*" black = "^24.3.0" cloudfoundry-client = "*" exceptiongroup = "==1.2.1" -flake8 = "^7.0.0" +flake8 = "^7.1.0" flake8-bugbear = "^24.1.17" freezegun = "^1.5.1" honcho = "*" isort = "^5.13.2" jinja2-cli = {version = "==0.8.2", extras = ["yaml"]} -moto = "==5.0.9" +moto = "==5.0.10" pip-audit = "*" pre-commit = "^3.7.1" pytest = "^8.2.2" @@ -102,7 +102,7 @@ pytest-cov = "^5.0.0" pytest-xdist = "^3.5.0" radon = "^6.0.1" requests-mock = "^1.11.0" -setuptools = "^70.1.1" +setuptools = "^70.2.0" sqlalchemy-utils = "^0.41.2" vulture = "^2.10" detect-secrets = "^1.5.0" diff --git a/terraform/demo/main.tf b/terraform/demo/main.tf index def3903a0..ea0f259e4 100644 --- a/terraform/demo/main.tf +++ b/terraform/demo/main.tf @@ -21,15 +21,6 @@ module "database" { rds_plan_name = "micro-psql" } -module "redis" { # default v6.2; delete after v7.0 resource is bound - source = "github.com/GSA-TTS/terraform-cloudgov//redis?ref=v1.0.0" - - cf_org_name = local.cf_org_name - cf_space_name = local.cf_space_name - name = "${local.app_name}-redis-${local.env}" - redis_plan_name = "redis-dev" -} - module "redis-v70" { source = "github.com/GSA-TTS/terraform-cloudgov//redis?ref=v1.0.0" diff --git a/terraform/production/main.tf b/terraform/production/main.tf index 28b2bf2d6..45cf7a5b8 100644 --- a/terraform/production/main.tf +++ b/terraform/production/main.tf @@ -21,15 +21,6 @@ module "database" { rds_plan_name = "small-psql-redundant" } -module "redis" { # default v6.2; delete after v7.0 resource is bound - source = "github.com/GSA-TTS/terraform-cloudgov//redis?ref=v1.0.0" - - cf_org_name = local.cf_org_name - cf_space_name = local.cf_space_name - name = "${local.app_name}-redis-${local.env}" - redis_plan_name = "redis-3node-large" -} - module "redis-v70" { source = "github.com/GSA-TTS/terraform-cloudgov//redis?ref=v1.0.0" diff --git a/terraform/staging/main.tf b/terraform/staging/main.tf index 3cc0358a5..4fdbf9e38 100644 --- a/terraform/staging/main.tf +++ b/terraform/staging/main.tf @@ -21,15 +21,6 @@ module "database" { rds_plan_name = "micro-psql" } -module "redis" { # default v6.2; delete after v7.0 resource is bound - source = "github.com/GSA-TTS/terraform-cloudgov//redis?ref=v1.0.0" - - cf_org_name = local.cf_org_name - cf_space_name = local.cf_space_name - name = "${local.app_name}-redis-${local.env}" - redis_plan_name = "redis-dev" -} - module "redis-v70" { source = "github.com/GSA-TTS/terraform-cloudgov//redis?ref=v1.0.0" diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 4c7030b2e..dc46de45d 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -33,31 +33,44 @@ def test_fetch_notification_status_for_service_by_month(notify_db_session): service_1 = create_service(service_name="service_1") service_2 = create_service(service_name="service_2") - create_ft_notification_status( - date(2018, 1, 1), NotificationType.SMS, service_1, count=4 - ) - create_ft_notification_status( - date(2018, 1, 2), NotificationType.SMS, service_1, count=10 - ) - create_ft_notification_status( - date(2018, 1, 2), - NotificationType.SMS, - service_1, - notification_status=NotificationStatus.CREATED, - ) - create_ft_notification_status(date(2018, 1, 3), NotificationType.EMAIL, service_1) + create_template(service=service_1) + create_template(service=service_1, template_type=TemplateType.EMAIL) + # not the service being tested + create_template(service=service_2) - create_ft_notification_status(date(2018, 2, 2), NotificationType.SMS, service_1) + # loop messages for the month + for x in range(0, 14): + create_notification( + service_1.templates[0], + created_at=datetime(2018, 1, 1, 1, x, 0), + status=NotificationStatus.DELIVERED, + ) + create_notification( + service_1.templates[0], created_at=datetime(2018, 1, 1, 1, 1, 0) + ) + create_notification( + service_1.templates[1], + created_at=datetime(2018, 1, 1, 1, 1, 0), + status=NotificationStatus.DELIVERED, + ) + create_notification( + service_1.templates[0], + created_at=datetime(2018, 2, 1, 1, 1, 0), + status=NotificationStatus.DELIVERED, + ) - # not included - too early - create_ft_notification_status(date(2017, 12, 31), NotificationType.SMS, service_1) - # not included - too late - create_ft_notification_status(date(2017, 3, 1), NotificationType.SMS, service_1) - # not included - wrong service - create_ft_notification_status(date(2018, 1, 3), NotificationType.SMS, service_2) - # not included - test keys - create_ft_notification_status( - date(2018, 1, 3), NotificationType.SMS, service_1, key_type=KeyType.TEST + # not the right month + create_notification( + service_1.templates[0], + created_at=datetime(2018, 4, 1, 1, 1, 0), + status=NotificationStatus.DELIVERED, + ) + + # not the right service + create_notification( + service_2.templates[0], + created_at=datetime(2018, 2, 1, 1, 1, 0), + status=NotificationStatus.DELIVERED, ) results = sorted( diff --git a/tests/app/service/send_notification/test_send_notification.py b/tests/app/service/send_notification/test_send_notification.py index 9b833bfd0..036c5bac8 100644 --- a/tests/app/service/send_notification/test_send_notification.py +++ b/tests/app/service/send_notification/test_send_notification.py @@ -79,7 +79,7 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker): assert json_resp["result"] == "error" assert len(json_resp["message"].keys()) == 1 assert ( - "Invalid phone number: The string supplied did not seem to be a phone number." + "Invalid phone number: Invalid phone number looks like invalid The string supplied did not seem to be a phone number." # noqa in json_resp["message"]["to"] ) assert response.status_code == 400 diff --git a/tests/app/service/test_statistics.py b/tests/app/service/test_statistics.py index 28484a8d6..c760d01b8 100644 --- a/tests/app/service/test_statistics.py +++ b/tests/app/service/test_statistics.py @@ -301,12 +301,15 @@ def test_add_monthly_notification_status_stats(): data = create_empty_monthly_notification_status_stats_dict(2018) # this data won't be affected data["2018-05"][NotificationType.EMAIL][NotificationStatus.SENDING] = 32 + data["2018-05"][NotificationType.EMAIL][StatisticsType.REQUESTED] = 32 # this data will get combined with the 8 from row_data data["2018-05"][NotificationType.SMS][NotificationStatus.SENDING] = 16 + data["2018-05"][NotificationType.SMS][StatisticsType.REQUESTED] = 16 add_monthly_notification_status_stats(data, rows) # first 3 months are empty + assert data == { "2018-01": {NotificationType.SMS: {}, NotificationType.EMAIL: {}}, "2018-02": {NotificationType.SMS: {}, NotificationType.EMAIL: {}}, @@ -315,12 +318,22 @@ def test_add_monthly_notification_status_stats(): NotificationType.SMS: { NotificationStatus.SENDING: 1, NotificationStatus.DELIVERED: 2, + StatisticsType.REQUESTED: 3, + }, + NotificationType.EMAIL: { + NotificationStatus.SENDING: 4, + StatisticsType.REQUESTED: 4, }, - NotificationType.EMAIL: {NotificationStatus.SENDING: 4}, }, "2018-05": { - NotificationType.SMS: {NotificationStatus.SENDING: 24}, - NotificationType.EMAIL: {NotificationStatus.SENDING: 32}, + NotificationType.SMS: { + NotificationStatus.SENDING: 24, + StatisticsType.REQUESTED: 24, + }, + NotificationType.EMAIL: { + NotificationStatus.SENDING: 32, + StatisticsType.REQUESTED: 32, + }, }, "2018-06": {NotificationType.SMS: {}, NotificationType.EMAIL: {}}, } diff --git a/tests/app/service/test_statistics_rest.py b/tests/app/service/test_statistics_rest.py index 591e5ca8e..6d20cacc3 100644 --- a/tests/app/service/test_statistics_rest.py +++ b/tests/app/service/test_statistics_rest.py @@ -234,17 +234,36 @@ def test_get_monthly_notification_stats_returns_stats(admin_request, sample_serv sms_t2 = create_template(sample_service) email_template = create_template(sample_service, template_type=TemplateType.EMAIL) - create_ft_notification_status(datetime(2016, 6, 1), template=sms_t1) - create_ft_notification_status(datetime(2016, 6, 2), template=sms_t1) - - create_ft_notification_status(datetime(2016, 7, 1), template=sms_t1) - create_ft_notification_status(datetime(2016, 7, 1), template=sms_t2) - create_ft_notification_status( - datetime(2016, 7, 1), - template=sms_t1, - notification_status=NotificationStatus.CREATED, + create_notification( + sms_t1, + created_at=datetime(2016, 6, 1, 1, 1, 0), + status=NotificationStatus.DELIVERED, + ) + create_notification( + sms_t1, + created_at=datetime(2016, 6, 2, 1, 1, 0), + status=NotificationStatus.DELIVERED, + ) + create_notification( + sms_t1, + created_at=datetime(2016, 7, 1, 1, 1, 0), + status=NotificationStatus.DELIVERED, + ) + create_notification( + sms_t2, + created_at=datetime(2016, 7, 1, 1, 1, 0), + status=NotificationStatus.DELIVERED, + ) + create_notification( + sms_t1, + created_at=datetime(2016, 7, 1, 1, 1, 0), + status=NotificationStatus.CREATED, + ) + create_notification( + email_template, + created_at=datetime(2016, 7, 1, 1, 1, 0), + status=NotificationStatus.DELIVERED, ) - create_ft_notification_status(datetime(2016, 7, 1), template=email_template) response = admin_request.get( "service.get_monthly_notification_stats", @@ -256,7 +275,8 @@ def test_get_monthly_notification_stats_returns_stats(admin_request, sample_serv assert response["data"]["2016-06"] == { NotificationType.SMS: { # it combines the two days - NotificationStatus.DELIVERED: 2 + NotificationStatus.DELIVERED: 2, + StatisticsType.REQUESTED: 2, }, NotificationType.EMAIL: {}, } @@ -265,86 +285,43 @@ def test_get_monthly_notification_stats_returns_stats(admin_request, sample_serv NotificationType.SMS: { NotificationStatus.CREATED: 1, NotificationStatus.DELIVERED: 2, + StatisticsType.REQUESTED: 3, }, - NotificationType.EMAIL: {StatisticsType.DELIVERED: 1}, - } - - -@freeze_time("2016-06-05 12:00:00") -def test_get_monthly_notification_stats_combines_todays_data_and_historic_stats( - admin_request, sample_template -): - create_ft_notification_status( - datetime(2016, 5, 1, 12), - template=sample_template, - count=1, - ) - create_ft_notification_status( - datetime(2016, 6, 1, 12), - template=sample_template, - notification_status=NotificationStatus.CREATED, - count=2, - ) # noqa - - create_notification( - sample_template, - created_at=datetime(2016, 6, 5, 12), - status=NotificationStatus.CREATED, - ) - create_notification( - sample_template, - created_at=datetime(2016, 6, 5, 12), - status=NotificationStatus.DELIVERED, - ) - - # this doesn't get returned in the stats because it is old - it should be in ft_notification_status by now - create_notification( - sample_template, - created_at=datetime(2016, 6, 4, 12), - status=NotificationStatus.SENDING, - ) - - response = admin_request.get( - "service.get_monthly_notification_stats", - service_id=sample_template.service_id, - year=2016, - ) - - assert len(response["data"]) == 6 # January to June - assert response["data"]["2016-05"] == { - NotificationType.SMS: {NotificationStatus.DELIVERED: 1}, - NotificationType.EMAIL: {}, - } - assert response["data"]["2016-06"] == { - NotificationType.SMS: { - # combines the stats from the historic ft_notification_status and the current notifications - NotificationStatus.CREATED: 3, - NotificationStatus.DELIVERED: 1, + NotificationType.EMAIL: { + StatisticsType.DELIVERED: 1, + StatisticsType.REQUESTED: 1, }, - NotificationType.EMAIL: {}, } def test_get_monthly_notification_stats_ignores_test_keys( admin_request, sample_service ): - create_ft_notification_status( - datetime(2016, 6, 1), - service=sample_service, + create_template(service=sample_service) + + create_notification( + sample_service.templates[0], + created_at=datetime(2016, 6, 1, 1, 1, 0), key_type=KeyType.NORMAL, - count=1, + status=NotificationStatus.DELIVERED, ) - create_ft_notification_status( - datetime(2016, 6, 1), - service=sample_service, + create_notification( + sample_service.templates[0], + created_at=datetime(2016, 6, 2, 1, 1, 0), + key_type=KeyType.NORMAL, + status=NotificationStatus.DELIVERED, + ) + create_notification( + sample_service.templates[0], + created_at=datetime(2016, 6, 1, 1, 1, 0), key_type=KeyType.TEAM, - count=2, + status=NotificationStatus.DELIVERED, ) - create_ft_notification_status( - datetime(2016, 6, 1), - service=sample_service, + create_notification( + sample_service.templates[0], + created_at=datetime(2016, 6, 1, 1, 1, 0), key_type=KeyType.TEST, - count=4, + status=NotificationStatus.DELIVERED, ) response = admin_request.get( @@ -355,26 +332,27 @@ def test_get_monthly_notification_stats_ignores_test_keys( assert response["data"]["2016-06"][NotificationType.SMS] == { NotificationStatus.DELIVERED: 3, + StatisticsType.REQUESTED: 3, } def test_get_monthly_notification_stats_checks_dates(admin_request, sample_service): t = create_template(sample_service) - # create_ft_notification_status(datetime(2016, 3, 31), template=t, notification_status='created') - create_ft_notification_status( - datetime(2016, 4, 2), - template=t, - notification_status=NotificationStatus.SENDING, + + create_notification( + t, + created_at=datetime(2016, 4, 2), + status=NotificationStatus.SENDING, ) - create_ft_notification_status( - datetime(2017, 3, 31), - template=t, - notification_status=NotificationStatus.DELIVERED, + create_notification( + t, + created_at=datetime(2017, 3, 31), + status=NotificationStatus.DELIVERED, ) - create_ft_notification_status( - datetime(2017, 4, 11), - template=t, - notification_status=NotificationStatus.PERMANENT_FAILURE, + create_notification( + t, + created_at=datetime(2017, 4, 11), + status=NotificationStatus.PERMANENT_FAILURE, ) response = admin_request.get( @@ -386,9 +364,11 @@ def test_get_monthly_notification_stats_checks_dates(admin_request, sample_servi assert "2017-04" not in response["data"] assert response["data"]["2016-04"][NotificationType.SMS] == { NotificationStatus.SENDING: 1, + StatisticsType.REQUESTED: 1, } assert response["data"]["2016-04"][NotificationType.SMS] == { NotificationStatus.SENDING: 1, + StatisticsType.REQUESTED: 1, } @@ -399,15 +379,15 @@ def test_get_monthly_notification_stats_only_gets_for_one_service( templates = [create_template(services[0]), create_template(services[1])] - create_ft_notification_status( - datetime(2016, 6, 1), - template=templates[0], - notification_status=NotificationStatus.CREATED, + create_notification( + templates[0], + created_at=datetime(2016, 6, 1), + status=NotificationStatus.CREATED, ) - create_ft_notification_status( - datetime(2016, 6, 1), - template=templates[1], - notification_status=NotificationStatus.DELIVERED, + create_notification( + templates[1], + created_at=datetime(2016, 6, 1), + status=NotificationStatus.DELIVERED, ) response = admin_request.get( @@ -417,6 +397,9 @@ def test_get_monthly_notification_stats_only_gets_for_one_service( ) assert response["data"]["2016-06"] == { - NotificationType.SMS: {NotificationStatus.CREATED: 1}, + NotificationType.SMS: { + NotificationStatus.CREATED: 1, + StatisticsType.REQUESTED: 1, + }, NotificationType.EMAIL: {}, } diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index a388d264e..dc7b78e82 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -226,7 +226,7 @@ def test_cannot_create_user_with_empty_strings(admin_request, notify_db_session) assert resp["message"] == { "email_address": ["Not a valid email address"], "mobile_number": [ - "Invalid phone number: The string supplied did not seem to be a phone number." + "Invalid phone number: Invalid phone number looks like The string supplied did not seem to be a phone number." # noqa ], "name": ["Invalid name"], } @@ -949,7 +949,7 @@ def test_cannot_update_user_with_mobile_number_as_empty_string( _expected_status=400, ) assert resp["message"]["mobile_number"] == [ - "Invalid phone number: The string supplied did not seem to be a phone number." + "Invalid phone number: Invalid phone number looks like The string supplied did not seem to be a phone number." # noqa ] diff --git a/tests/notifications_utils/test_recipient_validation.py b/tests/notifications_utils/test_recipient_validation.py index ff48df775..cc6c2a676 100644 --- a/tests/notifications_utils/test_recipient_validation.py +++ b/tests/notifications_utils/test_recipient_validation.py @@ -9,6 +9,7 @@ from notifications_utils.recipients import ( get_international_phone_info, international_phone_info, is_us_phone_number, + show_mangled_number_clues, try_validate_and_format_phone_number, validate_and_format_phone_number, validate_email_address, @@ -324,6 +325,11 @@ def test_phone_number_rejects_invalid_international_values(phone_number, error_m assert error_message == str(e.value) +def test_show_mangled_number_clues(): + x = show_mangled_number_clues("848!!-202?-2020$$") + assert x == "XXX!!-XXX?-XXXX$$" + + @pytest.mark.parametrize("email_address", valid_email_addresses) def test_validate_email_address_accepts_valid(email_address): try: