From 74be99e7c71796c954b181037af53b3800dc528f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Oct 2016 15:38:36 +0100 Subject: [PATCH 1/3] Let team key send to whitelist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is an overlap between team key/trial mode/whitelist. But it’s not a complete overlap. So it’s hard to understand all the different permutations of which key lets you send to which people when. This commit tries to reduce the differences between these concepts. So for a user of the API **In trial mode** - You can send to anyone in your team or whitelist, using the team key - You can simulate sending to anyone, using the simulate key **When you’re live** - You can send to anyone in your team or whitelist, using the team key - You can simulate sending to anyone, using the simulate key - You can send to anyone with the live key So doing a `git diff` on that list, the only difference between being in trial mode and live mode is now: `+` You can send to anyone with the live key **(How trial mode used to work)** - You can send to anyone in your team or whitelist, using the normal key - You can simulate sending to anyone, using the simulate key - You can send to _just_ people in your team using the team key --- app/service/utils.py | 18 ++++----- .../rest/test_send_notification.py | 39 ++++++++++++------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/app/service/utils.py b/app/service/utils.py index 17ce89071..2c4ccbf04 100644 --- a/app/service/utils.py +++ b/app/service/utils.py @@ -34,16 +34,16 @@ def service_allowed_to_send_to(recipient, service, key_type): return True team_members = itertools.chain.from_iterable( - [user.mobile_number, user.email_address] for user in service.users) + [user.mobile_number, user.email_address] for user in service.users + ) + whitelist_members = [ + member.recipient for member in service.whitelist + ] - if key_type == KEY_TYPE_TEAM: - return allowed_to_send_to( - recipient, - team_members - ) - - if key_type == KEY_TYPE_NORMAL and service.restricted: - whitelist_members = [member.recipient for member in service.whitelist] + if ( + (key_type == KEY_TYPE_NORMAL and service.restricted) or + (key_type == KEY_TYPE_TEAM) + ): return allowed_to_send_to( recipient, itertools.chain( diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 9fc0661ba..923d81df5 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -819,23 +819,34 @@ def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode(c apply_async.assert_not_called() -@pytest.mark.parametrize('notification_type,to', [ - ('sms', '07123123123'), - ('email', 'whitelist_recipient@mail.com')]) -def test_should_send_notification_to_whitelist_recipient_in_trial_mode_with_live_key(client, - notify_db, - notify_db_session, - notification_type, - to, - mocker): - service = create_sample_service(notify_db, notify_db_session, limit=2, restricted=True) +@pytest.mark.parametrize('service_restricted', [ + True, False +]) +@pytest.mark.parametrize('key_type', [ + KEY_TYPE_NORMAL, KEY_TYPE_TEAM +]) +@pytest.mark.parametrize('notification_type, to, _create_sample_template', [ + ('sms', '07123123123', create_sample_template), + ('email', 'whitelist_recipient@mail.com', create_sample_email_template)] +) +def test_should_send_notification_to_whitelist_recipient( + client, + notify_db, + notify_db_session, + notification_type, + to, + _create_sample_template, + key_type, + service_restricted, + mocker +): + service = create_sample_service(notify_db, notify_db_session, limit=2, restricted=service_restricted) apply_async = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type)) + template = _create_sample_template(notify_db, notify_db_session, service=service) if notification_type == 'sms': - template = create_sample_template(notify_db, notify_db_session, service=service) service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session, service=service, mobile_number=to) elif notification_type == 'email': - template = create_sample_email_template(notify_db, notify_db_session, service=service) service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session, service=service, email_address=to) @@ -849,8 +860,8 @@ def test_should_send_notification_to_whitelist_recipient_in_trial_mode_with_live 'template': str(template.id) } - sample_live_key = create_sample_api_key(notify_db, notify_db_session, service) - auth_header = create_jwt_token(secret=sample_live_key.unsigned_secret, client_id=str(sample_live_key.service_id)) + sample_key = create_sample_api_key(notify_db, notify_db_session, service, key_type=key_type) + auth_header = create_jwt_token(secret=sample_key.unsigned_secret, client_id=str(sample_key.service_id)) response = client.post( path='/notifications/{}'.format(notification_type), From ed203e33ea41513938c23787c847fd28fa3ea85e Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Mon, 10 Oct 2016 10:24:33 +0100 Subject: [PATCH 2/3] Refactor test for sending to non-whitelist member --- .../rest/test_send_notification.py | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 923d81df5..ec3d228e9 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -768,27 +768,27 @@ def test_should_not_persist_notification_or_send_sms_if_simulated_number( assert Notification.query.count() == 0 -@pytest.mark.parametrize('notification_type,to, key_type', [ - ('sms', '07827992635', KEY_TYPE_NORMAL), - ('email', 'non_whitelist_recipient@mail.com', KEY_TYPE_NORMAL), - ('sms', '07827992635', KEY_TYPE_TEAM), - ('email', 'non_whitelist_recipient@mail.com', KEY_TYPE_TEAM)]) -def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode(client, - notify_db, - notify_db_session, - notification_type, - to, - key_type, - mocker): +@pytest.mark.parametrize('key_type', [ + KEY_TYPE_NORMAL, KEY_TYPE_TEAM +]) +@pytest.mark.parametrize('notification_type, to, _create_sample_template', [ + ('sms', '07827992635', create_sample_template), + ('email', 'non_whitelist_recipient@mail.com', create_sample_email_template)] +) +def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode( + client, + notify_db, + notify_db_session, + notification_type, + to, + _create_sample_template, + key_type, + mocker): service = create_sample_service(notify_db, notify_db_session, limit=2, restricted=True) service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session, service=service) apply_async = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type)) - if notification_type == 'sms': - template = create_sample_template(notify_db, notify_db_session, service=service) - elif notification_type == 'email': - template = create_sample_email_template(notify_db, notify_db_session, service=service) - + template = _create_sample_template(notify_db, notify_db_session, service=service) assert service_whitelist.service_id == service.id assert to not in [member.recipient for member in service.whitelist] From 9bbd4713e4875965069ec1db04e94b0edba79e76 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Mon, 10 Oct 2016 10:27:57 +0100 Subject: [PATCH 3/3] Fix PEP issueFix PEP issueFix PEP issueFix PEP issueFix PEP issueFix PEP issueFix PEP issueFix PEP issue --- tests/app/notifications/rest/test_send_notification.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index ec3d228e9..a45ff4daf 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -783,7 +783,8 @@ def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode( to, _create_sample_template, key_type, - mocker): + mocker +): service = create_sample_service(notify_db, notify_db_session, limit=2, restricted=True) service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session, service=service)