From 60aa2d2b4262e2a343f4f44aef9de4954c5552ac Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 26 Jan 2021 10:14:04 +0000 Subject: [PATCH 1/3] =?UTF-8?q?Display=20areas=20that=20aren=E2=80=99t=20i?= =?UTF-8?q?n=20the=20library?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/broadcast_areas/__init__.py | 35 +++++++++++++++++++ app/models/broadcast_message.py | 9 +++-- tests/__init__.py | 2 ++ tests/app/main/views/test_broadcast.py | 47 ++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 2 deletions(-) diff --git a/app/broadcast_areas/__init__.py b/app/broadcast_areas/__init__.py index 5c10f6228..0631f37cd 100644 --- a/app/broadcast_areas/__init__.py +++ b/app/broadcast_areas/__init__.py @@ -89,6 +89,41 @@ class BroadcastArea(SortableMixin): id = parent_broadcast_area.id +class CustomBroadcastArea: + + # We don’t yet have a way to estimate the number of phones in a + # user-defined polygon + count_of_phones = 0 + + def __init__(self, *, name, polygons=None): + self.name = name + self._polygons = polygons or [] + + @property + def polygons(self): + return Polygons( + # Polygons in the DB are stored with the coordinate pair + # order flipped – this flips them back again + Polygons(self._polygons).as_coordinate_pairs_lat_long + ) + + simple_polygons = polygons + + +class CustomBroadcastAreas(SerialisedModelCollection): + model = CustomBroadcastArea + + def __init__(self, *, areas, polygons): + self.items = areas + self._polygons = polygons + + def __getitem__(self, index): + return self.model( + name=self.items[index], + polygons=self._polygons if index == 0 else None, + ) + + class BroadcastAreaLibrary(SerialisedModelCollection, SortableMixin, GetItemByIdMixin): model = BroadcastArea diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 569bb36bb..8dbbc492f 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -5,7 +5,7 @@ from notifications_utils.template import BroadcastPreviewTemplate from orderedset import OrderedSet from werkzeug.utils import cached_property -from app.broadcast_areas import broadcast_area_libraries +from app.broadcast_areas import CustomBroadcastAreas, broadcast_area_libraries from app.broadcast_areas.polygons import Polygons from app.formatters import round_to_significant_figures from app.models import JSONModel, ModelList @@ -74,7 +74,12 @@ class BroadcastMessage(JSONModel): @property def areas(self): - return self.get_areas(areas=self._dict['areas']) + return self.get_areas( + areas=self._dict['areas'] + ) or CustomBroadcastAreas( + areas=self._dict['areas'], + polygons=self._dict['simple_polygons'], + ) @property def parent_areas(self): diff --git a/tests/__init__.py b/tests/__init__.py index 0164f9e3d..f30c4c251 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -657,6 +657,7 @@ def broadcast_message_json( approved_by_id=None, cancelled_by_id=None, areas=None, + simple_polygons=None, content=None, reference=None, template_name='Example template', @@ -676,6 +677,7 @@ def broadcast_message_json( 'areas': areas or [ 'ctry19-E92000001', 'ctry19-S92000003', ], + 'simple_polygons': simple_polygons or [], 'status': status, diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 4a5f8512c..6d731be0a 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -684,6 +684,53 @@ def test_preview_broadcast_areas_page( ] == estimates +def test_preview_broadcast_areas_page_with_custom_polygons( + mocker, + client_request, + service_one, + fake_uuid, +): + service_one['permissions'] += ['broadcast'] + mocker.patch( + 'app.broadcast_message_api_client.get_broadcast_message', + return_value=broadcast_message_json( + id_=fake_uuid, + template_id=fake_uuid, + created_by_id=fake_uuid, + service_id=SERVICE_ONE_ID, + status='draft', + areas=['Area one', 'Area two', 'Area three'], + simple_polygons=[ + [[1, 2], [3, 4], [5, 6]], + [[7, 8], [9, 10], [11, 12]], + ], + ), + ) + page = client_request.get( + '.preview_broadcast_areas', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + ) + + assert [ + normalize_spaces(item.text) + for item in page.select('ul.area-list li.area-list-item') + ] == [ + 'Area one remove', 'Area two remove', 'Area three remove', + ] + + assert len(page.select('#area-list-map')) == 1 + + assert [ + normalize_spaces(item.text) + for item in page.select('ul li.area-list-key') + ] == [ + 'An area of 722.3 square miles Will get the alert', + 'An extra area of 1,402.5 square miles is Likely to get the alert', + '0 phones estimated', + ] + + @pytest.mark.parametrize('areas, expected_list', ( ([], [ 'Countries', From 5ae53b625baddc65c0ef985683042c750c334765 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 27 Jan 2021 11:24:22 +0000 Subject: [PATCH 2/3] Show broadcasts created by the API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Broadcasts created by the API are different in that: - they aren’t created by any user, so don’t have a `created_by_id` - they are created instantly, not in steps, so don’t have an `updated_at` time This commit alters the views to account for when these pieces of information aren’t present. --- app/models/broadcast_message.py | 6 +- .../views/broadcast/macros/area-map.html | 4 +- .../views/broadcast/view-message.html | 26 +++++-- tests/app/main/views/test_broadcast.py | 74 +++++++++++++++++-- 4 files changed, 95 insertions(+), 15 deletions(-) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 8dbbc492f..fc31ec1bf 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -45,6 +45,10 @@ class BroadcastMessage(JSONModel): return True if not self.starts_at and other.starts_at: return False + if self.updated_at and not other.updated_at: + return self.updated_at < other.created_at + if not self.updated_at and other.updated_at: + return self.created_at < other.updated_at return self.updated_at < other.updated_at @classmethod @@ -129,7 +133,7 @@ class BroadcastMessage(JSONModel): @cached_property def created_by(self): - return User.from_id(self.created_by_id) + return User.from_id(self.created_by_id) if self.created_by_id else None @cached_property def approved_by(self): diff --git a/app/templates/views/broadcast/macros/area-map.html b/app/templates/views/broadcast/macros/area-map.html index 196f4c447..737cd24c0 100644 --- a/app/templates/views/broadcast/macros/area-map.html +++ b/app/templates/views/broadcast/macros/area-map.html @@ -20,7 +20,9 @@ alert
  • - {% if broadcast_message.count_of_phones == broadcast_message.count_of_phones_likely %} + {% if broadcast_message.count_of_phones == 0 %} + Unknown number of phones + {% elif broadcast_message.count_of_phones == broadcast_message.count_of_phones_likely %} {{ broadcast_message.count_of_phones|format_thousands }} phones estimated {% else %} {{ broadcast_message.count_of_phones|format_thousands }} to {{ broadcast_message.count_of_phones_likely|format_thousands }} phones diff --git a/app/templates/views/broadcast/view-message.html b/app/templates/views/broadcast/view-message.html index 2004729d0..08be7e083 100644 --- a/app/templates/views/broadcast/view-message.html +++ b/app/templates/views/broadcast/view-message.html @@ -19,10 +19,15 @@ {% block service_page_title %} {% if broadcast_message.status == 'pending-approval' %} - {% if broadcast_message.created_by == current_user and current_user.has_permissions('send_messages') %} + {% if broadcast_message.created_by and broadcast_message.created_by == current_user and current_user.has_permissions('send_messages') %} {{ broadcast_message.template.name }} is waiting for approval {% elif current_user.has_permissions('send_messages') %} - {{ broadcast_message.created_by.name }} wants to broadcast + {% if broadcast_message.created_by %} + {{ broadcast_message.created_by.name }} + {% else %} + An API call + {% endif %} + wants to broadcast {{ broadcast_message.template.name }} {% else %} This alert is waiting for approval @@ -37,7 +42,7 @@ {{ govukBackLink({ "href": back_link }) }} {% if broadcast_message.status == 'pending-approval' %} - {% if broadcast_message.created_by == current_user and current_user.has_permissions('send_messages') %} + {% if broadcast_message.created_by and broadcast_message.created_by == current_user and current_user.has_permissions('send_messages') %}