From 850571cb8cdd9fe601b25dde15e66dadf22f9340 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 8 Oct 2020 14:06:33 +0100 Subject: [PATCH 1/4] Recognise that broadcast messages include content The content attribute needs to be added to the model so we can use it later. --- app/models/broadcast_message.py | 1 + tests/__init__.py | 2 ++ tests/app/models/test_broadcast_message.py | 11 +++++++++++ 3 files changed, 14 insertions(+) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index c5951b152..27106be8e 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -24,6 +24,7 @@ class BroadcastMessage(JSONModel): 'template_id', 'template_name', 'template_version', + 'content', 'service_id', 'created_by', 'personalisation', diff --git a/tests/__init__.py b/tests/__init__.py index 7fe6663f1..77db0dd93 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -650,6 +650,7 @@ def broadcast_message_json( approved_by_id=None, cancelled_by_id=None, areas=None, + content=None, ): return { 'id': id_, @@ -659,6 +660,7 @@ def broadcast_message_json( 'template_id': template_id, 'template_version': 123, 'template_name': 'Example template', + 'content': 'This is a test', 'personalisation': {}, 'areas': areas or [ diff --git a/tests/app/models/test_broadcast_message.py b/tests/app/models/test_broadcast_message.py index 955fac511..1b9a46124 100644 --- a/tests/app/models/test_broadcast_message.py +++ b/tests/app/models/test_broadcast_message.py @@ -34,3 +34,14 @@ def test_simple_polygons(fake_uuid): # total coordinates [51], ] + + +def test_content_comes_from_attribute_not_template(fake_uuid): + broadcast_message = BroadcastMessage(broadcast_message_json( + id_=fake_uuid, + service_id=fake_uuid, + template_id=fake_uuid, + status='draft', + created_by_id=fake_uuid, + )) + assert broadcast_message.content == 'This is a test' From 54d4fb2a6c7248fb88923e1cc0934f6eb82e1e04 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 8 Oct 2020 14:30:09 +0100 Subject: [PATCH 2/4] Allow platform admins to clear cached broadcasts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we add a new property to the broadcast model, we need to delete any cached broadcasts from Redis that are missing the new property. So this adds an option to do this to the cache page in platform admin. I’ve also tried to make it more obvious what the magic numbers in the test fixture are doing. --- app/main/views/platform_admin.py | 3 +++ tests/app/main/views/test_platform_admin.py | 13 +++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 5f051cf10..1259c7e5c 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -432,6 +432,9 @@ def clear_cache(): 'live-service-and-organisation-counts', 'organisation-????????-????-????-????-????????????-name', ]), + ('broadcast', [ + 'service-????????-????-????-????-????????????-broadcast-message-????????-????-????-????-????????????', + ]), ]) form = ClearCacheForm() diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index c75bab7e8..de704e0a2 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -692,7 +692,7 @@ def test_clear_cache_shows_form(client_request, platform_admin_user, mocker): call('service-????????-????-????-????-????????????-templates'), call('service-????????-????-????-????-????????????-template-????????-????-????-????-????????????-version-*'), call('service-????????-????-????-????-????????????-template-????????-????-????-????-????????????-versions'), - ], 'Removed 3 template objects from redis'), + ], 'Removed 103 template objects from redis'), ('service', [ call('has_jobs-????????-????-????-????-????????????'), call('service-????????-????-????-????-????????????'), @@ -701,13 +701,16 @@ def test_clear_cache_shows_form(client_request, platform_admin_user, mocker): call('service-????????-????-????-????-????????????-template-folders'), call('service-????????-????-????-????-????????????-returned-letters-statistics'), call('service-????????-????-????-????-????????????-returned-letters-summary'), - ], 'Removed 3 service objects from redis'), + ], 'Removed 107 service objects from redis'), ('organisation', [ call('organisations'), call('domains'), call('live-service-and-organisation-counts'), call('organisation-????????-????-????-????-????????????-name'), - ], 'Removed 3 organisation objects from redis'), + ], 'Removed 104 organisation objects from redis'), + ('broadcast', [ + call('service-????????-????-????-????-????????????-broadcast-message-????????-????-????-????-????????????'), + ], 'Removed 101 broadcast objects from redis'), )) def test_clear_cache_submits_and_tells_you_how_many_things_were_deleted( client_request, @@ -718,7 +721,9 @@ def test_clear_cache_submits_and_tells_you_how_many_things_were_deleted( expected_confirmation, ): redis = mocker.patch('app.main.views.platform_admin.redis_client') - redis.delete_cache_keys_by_pattern.side_effect = [0, 3, 1, 0, 0, 0, 0, 0] + # The way this is set up means the first time `delete_cache_keys_by_pattern` + # is called it will return `101`, the second time it will return `102`, etc + redis.delete_cache_keys_by_pattern.side_effect = [101, 102, 103, 104, 105, 106, 107, 108, 109] client_request.login(platform_admin_user) page = client_request.post('main.clear_cache', _data={'model_type': model_type}, _expected_status=200) From d8b265b0ca28b2bc7ae66fc552e415dbe7ba638f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 12 Oct 2020 14:14:12 +0100 Subject: [PATCH 3/4] Add comments to make test data clearer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s not clear where 103 is coming from unless you know that the code is doing the slightly unintuitive thing of displaying the max of all return values. --- tests/app/main/views/test_platform_admin.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index de704e0a2..1f1fb9757 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -689,9 +689,13 @@ def test_clear_cache_shows_form(client_request, platform_admin_user, mocker): @pytest.mark.parametrize('model_type, expected_calls, expected_confirmation', ( ('template', [ + # Returns 101 call('service-????????-????-????-????-????????????-templates'), + # Returns 102 call('service-????????-????-????-????-????????????-template-????????-????-????-????-????????????-version-*'), + # Returns 103 call('service-????????-????-????-????-????????????-template-????????-????-????-????-????????????-versions'), + # 103 shown here because it’s the `max` of the 3 counts ], 'Removed 103 template objects from redis'), ('service', [ call('has_jobs-????????-????-????-????-????????????'), From 974fb8c6c1ac9d1fcc98e52c9f1ee45675064019 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 12 Oct 2020 14:18:33 +0100 Subject: [PATCH 4/4] Accept content as keyword argument --- tests/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/__init__.py b/tests/__init__.py index 77db0dd93..01604ce50 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -660,7 +660,7 @@ def broadcast_message_json( 'template_id': template_id, 'template_version': 123, 'template_name': 'Example template', - 'content': 'This is a test', + 'content': content or 'This is a test', 'personalisation': {}, 'areas': areas or [