Merge pull request #4010 from alphagov/refactor-duplicate-method

Refactor `get_simple_polygons` method for reuse

app/models/broadcast_message.py

@@ -137,26 +137,15 @@ class BroadcastMessage(JSONModel):
 
     @cached_property
     def polygons(self):
-        return Polygons(
-            list(itertools.chain(*(
-                area.polygons for area in self.areas
-            )))
-        )
+        return self.get_polygons_from_areas(area_attribute='polygons')
 
     @cached_property
     def simple_polygons(self):
-        return self.get_simple_polygons(areas=self.areas)
+        return self.get_polygons_from_areas(area_attribute='simple_polygons')
 
     @cached_property
     def simple_polygons_with_bleed(self):
-        polygons = Polygons(
-            list(itertools.chain(*(
-                area.simple_polygons_with_bleed 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
-        return polygons.smooth.simplify if len(self.areas) > 1 else polygons
+        return self.get_polygons_from_areas(area_attribute='simple_polygons_with_bleed')
 
     @property
     def reference(self):
@@ -228,15 +217,18 @@ class BroadcastMessage(JSONModel):
             area_ids
         )
 
-    def get_simple_polygons(self, areas):
+    def get_polygons_from_areas(self, area_attribute):
         polygons = Polygons(
             list(itertools.chain(*(
-                area.simple_polygons for area in areas
+                getattr(area, area_attribute) 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
-        return polygons.smooth.simplify if len(areas) > 1 else polygons
+        if area_attribute != 'polygons' and len(self.areas) > 1:
+            # We’re combining simplified polygons from multiple areas so we
+            # need to re-simplify the combined polygons to keep the point
+            # count down
+            return polygons.smooth.simplify
+        return polygons
 
     def add_areas(self, *new_area_ids):
         self.area_ids = list(OrderedSet(self.area_ids + list(new_area_ids)))

tests/app/main/views/test_broadcast.py

@@ -1383,7 +1383,10 @@ def test_add_broadcast_area(
     polygon_class = namedtuple("polygon_class", ["as_coordinate_pairs_lat_long"])
     coordinates = [[50.1, 0.1], [50.2, 0.2], [50.3, 0.2]]
     polygons = polygon_class(as_coordinate_pairs_lat_long=coordinates)
-    mocker.patch('app.models.broadcast_message.BroadcastMessage.get_simple_polygons', return_value=polygons)
+    mock_get_polygons_from_areas = mocker.patch(
+        'app.models.broadcast_message.BroadcastMessage.get_polygons_from_areas',
+        return_value=polygons,
+    )
 
     client_request.login(active_user_create_broadcasts_permission)
     client_request.post(
@@ -1395,6 +1398,7 @@ def test_add_broadcast_area(
             'areas': ['ctry19-E92000001', 'ctry19-W92000004']
         }
     )
+    mock_get_polygons_from_areas.assert_called_once_with(area_attribute='simple_polygons')
     mock_update_broadcast_message.assert_called_once_with(
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,
@@ -1447,7 +1451,10 @@ def test_add_broadcast_sub_area_district_view(
     polygon_class = namedtuple("polygon_class", ["as_coordinate_pairs_lat_long"])
     coordinates = [[50.1, 0.1], [50.2, 0.2], [50.3, 0.2]]
     polygons = polygon_class(as_coordinate_pairs_lat_long=coordinates)
-    mocker.patch('app.models.broadcast_message.BroadcastMessage.get_simple_polygons', return_value=polygons)
+    mock_get_polygons_from_areas = mocker.patch(
+        'app.models.broadcast_message.BroadcastMessage.get_polygons_from_areas',
+        return_value=polygons,
+    )
 
     client_request.login(active_user_create_broadcasts_permission)
     client_request.post(
@@ -1464,6 +1471,7 @@ def test_add_broadcast_sub_area_district_view(
     expected_data['names'] = ['England', 'Scotland'] + expected_data['names']
     expected_data['aggregate_names'] = sorted(['England', 'Scotland'] + expected_data['aggregate_names'])
 
+    mock_get_polygons_from_areas.assert_called_once_with(area_attribute='simple_polygons')
     mock_update_broadcast_message.assert_called_once_with(
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,
@@ -1489,7 +1497,10 @@ def test_add_broadcast_sub_area_county_view(
     polygon_class = namedtuple("polygon_class", ["as_coordinate_pairs_lat_long"])
     coordinates = [[50.1, 0.1], [50.2, 0.2], [50.3, 0.2]]
     polygons = polygon_class(as_coordinate_pairs_lat_long=coordinates)
-    mocker.patch('app.models.broadcast_message.BroadcastMessage.get_simple_polygons', return_value=polygons)
+    mock_get_polygons_from_areas = mocker.patch(
+        'app.models.broadcast_message.BroadcastMessage.get_polygons_from_areas',
+        return_value=polygons,
+    )
 
     client_request.login(active_user_create_broadcasts_permission)
     client_request.post(
@@ -1500,6 +1511,7 @@ def test_add_broadcast_sub_area_county_view(
         area_slug='ctyua19-E10000016',  # Kent
         _data={'select_all': 'y'},
     )
+    mock_get_polygons_from_areas.assert_called_once_with(area_attribute='simple_polygons')
     mock_update_broadcast_message.assert_called_once_with(
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,
@@ -1533,7 +1545,10 @@ def test_remove_broadcast_area_page(
     polygon_class = namedtuple("polygon_class", ["as_coordinate_pairs_lat_long"])
     coordinates = [[50.1, 0.1], [50.2, 0.2], [50.3, 0.2]]
     polygons = polygon_class(as_coordinate_pairs_lat_long=coordinates)
-    mocker.patch('app.models.broadcast_message.BroadcastMessage.get_simple_polygons', return_value=polygons)
+    mock_get_polygons_from_areas = mocker.patch(
+        'app.models.broadcast_message.BroadcastMessage.get_polygons_from_areas',
+        return_value=polygons,
+    )
 
     client_request.login(active_user_create_broadcasts_permission)
     client_request.get(
@@ -1548,6 +1563,7 @@ def test_remove_broadcast_area_page(
             _external=True,
         ),
     )
+    mock_get_polygons_from_areas.assert_called_once_with(area_attribute='simple_polygons')
     mock_update_broadcast_message.assert_called_once_with(
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,