Refactor coordinate processing into Polygons class

We have a bunch of stuff for doing lat/long transformation in the
`BroadcastMessage` class. This is not a good separation of concerns, now
that we have a separate class for dealing with polygons and coordinates.

app/broadcast_areas/__init__.py

@@ -1,8 +1,7 @@
-import itertools
-
 from notifications_utils.serialised_model import SerialisedModelCollection
 from werkzeug.utils import cached_property
 
+from .polygons import Polygons
 from .repo import BroadcastAreasRepository
 
 
@@ -35,11 +34,15 @@ class BroadcastArea(SortableMixin):
 
     @cached_property
     def polygons(self):
-        return BroadcastAreasRepository().get_polygons_for_area(self.id)
+        return Polygons(
+            BroadcastAreasRepository().get_polygons_for_area(self.id)
+        )
 
     @cached_property
     def simple_polygons(self):
-        return BroadcastAreasRepository().get_simple_polygons_for_area(self.id)
+        return Polygons(
+            BroadcastAreasRepository().get_simple_polygons_for_area(self.id)
+        )
 
     @property
     def sub_areas(self):
@@ -80,29 +83,5 @@ class BroadcastAreaLibraries(SerialisedModelCollection, GetItemByIdMixin):
         areas = BroadcastAreasRepository().get_areas(area_ids)
         return [BroadcastArea(area) for area in areas]
 
-    def get_polygons_for_areas_long_lat(self, *area_ids):
-        return list(itertools.chain(*(
-            area.polygons
-            for area in self.get_areas(*area_ids)
-        )))
-
-    def get_polygons_for_areas_lat_long(self, *area_ids):
-        return [
-            [[long, lat] for lat, long in polygon]
-            for polygon in self.get_polygons_for_areas_long_lat(*area_ids)
-        ]
-
-    def get_simple_polygons_for_areas_long_lat(self, *area_ids):
-        return list(itertools.chain(*(
-            area.simple_polygons
-            for area in self.get_areas(*area_ids)
-        )))
-
-    def get_simple_polygons_for_areas_lat_long(self, *area_ids):
-        return [
-            [[long, lat] for lat, long in polygon]
-            for polygon in self.get_simple_polygons_for_areas_long_lat(*area_ids)
-        ]
-
 
 broadcast_area_libraries = BroadcastAreaLibraries()

app/broadcast_areas/create-broadcast-areas-db.py

@@ -43,7 +43,10 @@ def polygons_and_simplified_polygons(feature):
             'Polygons.perimeter_to_buffer_ratio)'
         )
 
-    return full_resolution.as_coordinate_pairs, simplified.as_coordinate_pairs
+    return (
+        full_resolution.as_coordinate_pairs_long_lat,
+        simplified.as_coordinate_pairs_long_lat,
+    )
 
 
 repo = BroadcastAreasRepository()

app/broadcast_areas/polygons.py

@@ -53,6 +53,9 @@ class Polygons():
     def __getitem__(self, index):
         return self.polygons[index]
 
+    def __len__(self):
+        return len(self.polygons)
+
     @cached_property
     def perimeter_length(self):
         return sum(
@@ -141,25 +144,22 @@ class Polygons():
         ])
 
     @cached_property
-    def as_coordinate_pairs(self):
+    def as_coordinate_pairs_long_lat(self):
         return [
-            [
+            [[x, y] for x, y in polygon.exterior.coords]
-                [x, y] for x, y in p.exterior.coords
+            for polygon in self
-            ]
-            for p in self
         ]
 
     @cached_property
-    def as_unenclosed_coordinate_pairs(self):
+    def as_coordinate_pairs_lat_long(self):
-        # Some mapping tools require shapes to be unenclosed, i.e. the
-        # last point joins the first point implicitly
         return [
-            coordinates[:-1] for coordinates in self.as_coordinate_pairs
+            [[y, x] for x, y in coordinate_pairs]
+            for coordinate_pairs in self.as_coordinate_pairs_long_lat
         ]
 
     @cached_property
     def point_count(self):
-        return len(list(itertools.chain(*self.as_coordinate_pairs)))
+        return len(list(itertools.chain(*self.as_coordinate_pairs_long_lat)))
 
 
 def flatten_polygons(polygons):

app/models/broadcast_message.py

@@ -1,3 +1,4 @@
+import itertools
 from datetime import datetime, timedelta
 
 from notifications_utils.template import BroadcastPreviewTemplate
@@ -74,17 +75,17 @@ class BroadcastMessage(JSONModel):
     @cached_property
     def polygons(self):
         return Polygons(
-            broadcast_area_libraries.get_polygons_for_areas_lat_long(
+            list(itertools.chain(*(
-                *self._dict['areas']
+                area.polygons for area in self.areas
-            )
+            )))
         )
 
     @cached_property
     def simple_polygons(self):
         polygons = Polygons(
-            broadcast_area_libraries.get_simple_polygons_for_areas_lat_long(
+            list(itertools.chain(*(
-                *self._dict['areas']
+                area.simple_polygons for area in self.areas
-            )
+            )))
         )
         # If we’ve added multiple areas then we need to re-simplify the
         # combined shapes to keep the point count down

app/templates/views/broadcast/preview-areas.html

@@ -36,7 +36,7 @@
       attribution: '&copy; <a href="https://www.openstreetmap.org/copyright">OpenStreetMap</a> contributors'
     }).addTo(mymap);
 
-    {% for polygon in broadcast_message.simple_polygons.bleed.as_coordinate_pairs %}
+    {% for polygon in broadcast_message.simple_polygons.bleed.as_coordinate_pairs_lat_long %}
       polygons.push(
         L.polygon({{polygon}}, {
           opacity: 0.5,
@@ -48,7 +48,7 @@
       );
     {% endfor %}
 
-    {% for polygon in broadcast_message.simple_polygons.as_coordinate_pairs %}
+    {% for polygon in broadcast_message.simple_polygons.as_coordinate_pairs_lat_long %}
       polygons.push(
         L.polygon({{polygon}}, {
           opacity: 0.1,
@@ -60,7 +60,7 @@
       );
     {% endfor %}
 
-    {% for polygon in broadcast_message.polygons.as_coordinate_pairs %}
+    {% for polygon in broadcast_message.polygons.as_coordinate_pairs_lat_long %}
       polygons.push(
         L.polygon({{polygon}}, {
           color: '#0b0b0c', // $black

tests/app/broadcast_areas/test_broadcast_area.py

@@ -87,52 +87,32 @@ def test_get_areas_accepts_lists():
 
 def test_has_polygons():
 
-    assert len(
+    england = broadcast_area_libraries.get_areas('ctry19-E92000001')[0]
-        broadcast_area_libraries.get_polygons_for_areas_long_lat('ctry19-E92000001')
+    scotland = broadcast_area_libraries.get_areas('ctry19-S92000003')[0]
-    ) == 35
 
-    assert len(
+    assert len(england.polygons) == 35
-        broadcast_area_libraries.get_polygons_for_areas_long_lat('ctry19-S92000003')
+    assert len(scotland.polygons) == 195
-    ) == 195
 
-    assert len(
+    assert england.polygons.as_coordinate_pairs_lat_long[0][0] == [
-        broadcast_area_libraries.get_polygons_for_areas_long_lat(
-            'ctry19-E92000001',
-            'ctry19-S92000003',
-        )
-    ) == 35 + 195 == 230
-
-    assert len(
-        broadcast_area_libraries.get_polygons_for_areas_lat_long(
-            'ctry19-E92000001',
-            'ctry19-S92000003',
-        )
-    ) == 35 + 195 == 230
-
-    assert broadcast_area_libraries.get_polygons_for_areas_lat_long('ctry19-E92000001')[0][0] == [
         55.811085, -2.034358  # https://goo.gl/maps/wsf2LUWzYinwydMk8
     ]
 
 
-def test_polygons_are_enclosed_unless_asked_not_to_be():
+def test_polygons_are_enclosed():
     england = broadcast_area_libraries.get('ctry19').get('ctry19-E92000001')
 
-    assert len(england.polygons) == len(england.polygons.as_unenclosed_coordinate_pairs)
+    first_polygon = england.polygons.as_coordinate_pairs_lat_long[0]
-
-    first_polygon = england.polygons[0].as_coordinate_pairs
     assert first_polygon[0] != first_polygon[1] != first_polygon[2]
     assert first_polygon[0] == first_polygon[-1]
 
-    first_polygon_unenclosed = england.polygons[0].as_unenclosed_coordinate_pairs
-    assert first_polygon_unenclosed[0] == first_polygon[0]
-    assert first_polygon_unenclosed[-1] != first_polygon[-1]
-    assert first_polygon_unenclosed[-1] == first_polygon[-2]
-
 
 def test_lat_long_order():
 
-    lat_long = broadcast_area_libraries.get_polygons_for_areas_lat_long('ctry19-E92000001')
+    england = broadcast_area_libraries.get_areas('ctry19-E92000001')[0]
-    long_lat = broadcast_area_libraries.get_polygons_for_areas_long_lat('ctry19-E92000001')
+
+    lat_long = england.polygons.as_coordinate_pairs_lat_long
+    long_lat = england.polygons.as_coordinate_pairs_long_lat
+
     assert len(lat_long[0]) == len(long_lat[0]) == 2082  # Coordinates in polygon
     assert len(lat_long[0][0]) == len(long_lat[0][0]) == 2  # Axes in coordinates
     assert lat_long[0][0] == list(reversed(long_lat[0][0]))

tests/app/models/test_broadcast_message.py

@@ -18,13 +18,19 @@ def test_simple_polygons(fake_uuid):
     ))
 
     assert [
-        [len(polygon) for polygon in broadcast_message.polygons],
+        [
-        [len(polygon) for polygon in broadcast_message.simple_polygons],
+            len(polygon)
+            for polygon in broadcast_message.polygons.as_coordinate_pairs_lat_long
+        ],
+        [
+            len(polygon)
+            for polygon in broadcast_message.simple_polygons.as_coordinate_pairs_lat_long
+        ],
     ] == [
         # One polygon for each area
         [27, 31],
         # Because the areas are close to each other, the simplification
         # and unioning process results in a single polygon with fewer
         # total coordinates
-        [34],
+        [55],
     ]