diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 000000000..63e2ca333 --- /dev/null +++ b/.coveragerc @@ -0,0 +1,5 @@ +[report] +exclude_lines = + pragma: no cover + ^\s*logger\. + ^\s*current_app\.logger\. diff --git a/Makefile b/Makefile index e864ba0ea..ab52f38db 100644 --- a/Makefile +++ b/Makefile @@ -104,8 +104,8 @@ test: ## Run tests and create coverage report poetry run isort ./app ./tests poetry run coverage run --omit=*/migrations/*,*/tests/* -m pytest --maxfail=10 - ## TODO set this back to 95 asap - poetry run coverage report -m --fail-under=94 + + poetry run coverage report -m --fail-under=95 poetry run coverage html -d .coverage_cache .PHONY: test-debug diff --git a/app/clients/pinpoint/aws_pinpoint.py b/app/clients/pinpoint/aws_pinpoint.py index 3b4de7682..0c4fc05d9 100644 --- a/app/clients/pinpoint/aws_pinpoint.py +++ b/app/clients/pinpoint/aws_pinpoint.py @@ -37,6 +37,7 @@ class AwsPinpointClient(Client): # TODO right now this will only print with AWS simulated numbers, # but remove this when that changes current_app.logger.info(hilite(response)) + return response except ClientError: current_app.logger.exception( "#notify-debug-validate-phone-number Could not validate with pinpoint" diff --git a/app/commands.py b/app/commands.py index 69d3b23ec..e4fdc7cb5 100644 --- a/app/commands.py +++ b/app/commands.py @@ -752,12 +752,13 @@ def create_admin_jwt(): @notify_command(name="create-user-jwt") @click.option("-t", "--token", required=True, prompt=False) def create_user_jwt(token): - if getenv("NOTIFY_ENVIRONMENT", "") != "development": + if getenv("NOTIFY_ENVIRONMENT", "") not in ["development", "test"]: current_app.logger.error("Can only be run in development") return service_id = token[-73:-37] api_key = token[-36:] - current_app.logger.info(create_jwt_token(api_key, service_id)) + token = create_jwt_token(api_key, service_id) + current_app.logger.info(token) def _update_template(id, name, template_type, content, subject): diff --git a/tests/app/aws/test_s3.py b/tests/app/aws/test_s3.py index f8cfd36af..c37789fa4 100644 --- a/tests/app/aws/test_s3.py +++ b/tests/app/aws/test_s3.py @@ -2,7 +2,7 @@ import os import time from datetime import timedelta from os import getenv -from unittest.mock import MagicMock, Mock, call +from unittest.mock import MagicMock, Mock, call, patch import botocore import pytest @@ -13,6 +13,7 @@ from app.aws import s3 from app.aws.s3 import ( cleanup_old_s3_objects, download_from_s3, + extract_phones, file_exists, get_job_and_metadata_from_s3, get_job_from_s3, @@ -650,3 +651,33 @@ def test_read_s3_file_populates_cache(monkeypatch): assert job_cache.get("66")[0].startswith("Phone number") assert job_cache.get("66_phones")[0] == {"0", "15551234"} assert job_cache.get("66_personalisation")[0] == {0: {"Name": "Alice"}} + + +@patch("app.aws.s3.current_app") +def test_valid_csv(mock_app): + csv_data = "Name,Phone Number\nAlice,+1 (555) 555-5555\nBob,555.555.1111" + result = extract_phones(csv_data, "service1", "job1") + expected = {0: "15555555555", 1: "5555551111"} + assert result == expected + mock_app.logger.error.assert_not_called() + + +@patch("app.aws.s3.current_app") +def test_missing_phone_column(mock_app): + + csv_data = "Name,Phone Number\nAlice,\nBob" + result = extract_phones(csv_data, "service1", "job1") + assert result == {0: "", 1: "Unavailable"} + mock_app.logger.error.assert_called_once() + + +@patch("app.aws.s3.current_app") +def test_test_with_bom_header(mock_app): + csv_data = "\ufeffName,Phone Number\nAlice,1-555-555-5555" + result = extract_phones(csv_data, "service2", "job2") + expected = {0: "15555555555"} + assert result == expected + + +if __name__ == "__main__": + test_valid_csv() diff --git a/tests/app/clients/test_aws_pinpoint.py b/tests/app/clients/test_aws_pinpoint.py new file mode 100644 index 000000000..a22f188a2 --- /dev/null +++ b/tests/app/clients/test_aws_pinpoint.py @@ -0,0 +1,37 @@ +from unittest.mock import MagicMock + +import pytest +from aiohttp import ClientError + +from app.clients.pinpoint.aws_pinpoint import AwsPinpointClient + + +def test_validate_phone_number_success(): + phone = "+1234567890" + mock_response = { + "NumberValidateResponse": { + "PhoneType": "MOBILE", + "CleansedPhoneNumberE164": phone, + } + } + client_instance = AwsPinpointClient() + client_instance._client = MagicMock() + client_instance._client.phone_number_validate.return_value = mock_response + + result = client_instance.validate_phone_number("US", phone) + assert result is not None + client_instance._client.phone_number_validate.assert_called_once_with( + NumberValidateRequest={"IsoCountryCode": "US", "PhoneNumber": phone} + ) + + +def test_validate_phone_number_client_error(): + client_instance = AwsPinpointClient() + client_instance._client = MagicMock() + client_instance._client.phone_number_validate.side_effect = ClientError( + {"Error": {"Code": "BadRequest1", "MEssage": "Invalid phone"}}, + "phone number validate", + ) + + with pytest.raises(ClientError): + client_instance.validate_phone_number("US", "bad-number") diff --git a/tests/app/test_commands.py b/tests/app/test_commands.py index 4f0037fb4..51e27c082 100644 --- a/tests/app/test_commands.py +++ b/tests/app/test_commands.py @@ -1,6 +1,7 @@ +import json import os from datetime import datetime, timedelta -from unittest.mock import MagicMock, mock_open +from unittest.mock import MagicMock, mock_open, patch import pytest from sqlalchemy import func, select @@ -11,13 +12,16 @@ from app.commands import ( _update_template, associate_services_to_organizations, bulk_invite_user_to_service, + clear_redis_list, create_new_service, create_test_user, + create_user_jwt, download_csv_file_by_name, dump_sms_senders, dump_user_info, fix_billable_units, generate_salt, + get_service_sender_phones, insert_inbound_numbers_from_file, populate_annual_billing_with_defaults, populate_annual_billing_with_the_previous_years_allowance, @@ -26,8 +30,10 @@ from app.commands import ( populate_organizations_from_file, process_row_from_job, promote_user_to_platform_admin, + purge_csv_bucket, purge_functional_test_data, update_jobs_archived_flag, + update_templates, ) from app.dao.inbound_numbers_dao import dao_get_available_inbound_numbers from app.dao.users_dao import get_user_by_email @@ -732,3 +738,113 @@ def test_clear_templates_from_cache(mocker): [mocker.call(p) for p in expected_patterns], any_order=True ) mock_logger.info.assert_called_once_with("Number of templates deleted from cache 9") + + +@patch("app.commands.s3.purge_bucket") +@patch("app.commands.current_app") +def test_purge_csv_bucket(mock_current_app, mock_purge_bucket, notify_api): + mock_current_app.config = { + "CSV_UPLOAD_BUCKET": { + "bucket": "test-bucket", + "access_key_id": "FAKE_ACCESS_KEY", + "secret_access_key": "FAKE_SECRET_KEY", # pragma: allowlist secret + "region": "us-north-1", + } + } + runner = notify_api.test_cli_runner() + runner.invoke(purge_csv_bucket) + + mock_purge_bucket.assert_called_once_with( + "test-bucket", "FAKE_ACCESS_KEY", "FAKE_SECRET_KEY", "us-north-1" + ) + + +@patch("app.commands.db.session.execute") +def test_get_service_sender_phones(mock_execute, notify_api): + runner = notify_api.test_cli_runner() + result = runner.invoke(get_service_sender_phones, ["-s", "service-id"]) + assert result.exit_code == 0 + mock_execute.assert_called_once() + + +@patch("app.commands.current_app.logger.info") +def test_create_user_jwt(mock_logger, notify_api, sample_api_key, sample_service): + token = f"{sample_service.id}{sample_api_key}" + runner = notify_api.test_cli_runner() + result = runner.invoke(create_user_jwt, ["-t", token]) + assert result.exit_code == 0 + mock_logger.assert_called_once() + + +def test_update_templates_calls_update_and_clear(monkeypatch, notify_api): + templates_data = [ + { + "id": 1, + "name": "Template1", + "type": "email", + "content": "Hello", + "subject": "Subject1", + }, + { + "id": 2, + "name": "Template2", + "type": "sms", + "content": "Hi", + "subject": "Subject2", + }, + ] + m = mock_open(read_data=json.dumps(templates_data)) + monkeypatch.setattr( + "app.commands.current_app", + type("obj", (), {"config": {"CONFIG_FILES": "/fake/path"}}), + ) + with patch("app.commands.open", m): + with patch("app.commands._update_template") as mock_update_template, patch( + "app.commands._clear_templates_from_cache" + ) as mock_clear_cache: + + runner = notify_api.test_cli_runner() + result = runner.invoke(update_templates) + assert result.exit_code == 0 + + expected_calls = [ + (1, "Template1", "email", "Hello", "Subject1"), + (2, "Template2", "sms", "Hi", "Subject2"), + ] + actual_calls = [call.args for call in mock_update_template.call_args_list] + assert actual_calls == expected_calls + mock_clear_cache.assert_called_once() + + +def test_clear_redis_list(monkeypatch, notify_api): + mock_redis_store = type("RedisStoreMock", (), {})() + monkeypatch.setattr("app.commands.redis_store", mock_redis_store) + mock_redis_store.llen = lambda list_name: {"before_list": 5, "after_list": 0}[ + list_name + ] + ltrim_calls = [] + + def fake_ltrim(name, start, end): + ltrim_calls.append((name, start, end)) + + mock_redis_store.ltrim = fake_ltrim + logger_info_calls = [] + mock_logger = type( + "LoggerMock", (), {"info": lambda self, msg: logger_info_calls.append(msg)} + )() + mock_app = type("FlaskAppMock", (), {"logger": mock_logger}) + monkeypatch.setattr("app.commands.current_app", mock_app) + call_count = {"count": 0} + + def llen_side_effect(name): + if call_count["count"] == 0: + call_count["count"] += 1 + return 5 + return 0 + + mock_redis_store.llen = llen_side_effect + runner = notify_api.test_cli_runner() + result = runner.invoke(clear_redis_list, ["-n", "test_list"]) + assert result.exit_code == 0 + assert ltrim_calls == [("test_list", 1, 0)] + assert logger_info_calls == ["Cleared redis list test_list. Before: 5 after 0"] diff --git a/tests/app/test_history_meta.py b/tests/app/test_history_meta.py new file mode 100644 index 000000000..d2a0db1f8 --- /dev/null +++ b/tests/app/test_history_meta.py @@ -0,0 +1,80 @@ +from unittest.mock import MagicMock + +from app.history_meta import ( + _get_bases_for_versioned_class, + _handle_single_table_inheritance, +) + + +def test_get_bases_with_super_history_and_table(): + mock_super_mapper = MagicMock() + mock_super_mapper.class_ = object + mock_super_mapper.attrs.changed.columns = ["col1", "col2"] + mock_table = MagicMock() + mock_table.c.changed = "changed_col" + + properties = {} + bases = _get_bases_for_versioned_class( + super_history_mapper=mock_super_mapper, + table=mock_table, + properties=properties, + local_mapper=None, + ) + + assert bases == (object,) + assert properties["changed"] == ("changed_col", "col1", "col2") + + +def test_get_bases_with_super_history_and_no_table(): + mock_super_mapper = MagicMock() + mock_super_mapper.class_ = int + properties = {} + bases = _get_bases_for_versioned_class( + super_history_mapper=mock_super_mapper, + table=None, + properties=properties, + local_mapper=None, + ) + assert bases == (int,) + assert "changed" not in properties + + +def test_get_bases_without_super_history(): + class Base: + pass + + class_ = type("Dummy", (Base,), {}) + local_mapper = MagicMock() + local_mapper.base_mapper.class_ = class_ + + properties = {} + + bases = _get_bases_for_versioned_class( + super_history_mapper=None, + table=None, + properties=properties, + local_mapper=local_mapper, + ) + + assert bases == (Base,) + assert "changed" not in properties + + +def test_handle_single_table_inheritance(): + col1 = MagicMock() + col1.key = "id" + col1.name = "name_col1" + col2 = MagicMock() + col2.key = "new_column" + col2.name = "name_col2" + + local_mapper = MagicMock() + local_mapper.local_table.c = [col1, col2] + + super_history_mapper = MagicMock() + super_history_mapper.local_table.c = {"id": col1} + super_history_mapper.local_table.append_column = col2 + + _handle_single_table_inheritance(local_mapper, super_history_mapper) + + super_history_mapper.local_table.append_column.assert_called_once()