Merge pull request #3618 from alphagov/fix-broadcast-area-count

generate library summary in python

app/broadcast_areas/__init__.py

@@ -1,3 +1,4 @@
+from notifications_utils.formatters import formatted_list
 from notifications_utils.serialised_model import SerialisedModelCollection
 from werkzeug.utils import cached_property
 
@@ -65,7 +66,15 @@ class BroadcastAreaLibrary(SerialisedModelCollection, SortableMixin, GetItemById
         self.items = BroadcastAreasRepository().get_all_areas_for_library(self.id)
 
     def get_examples(self):
-        return BroadcastAreasRepository().get_library_description(self.id)
+        # we show up to four things. three areas, then either a fourth area if there are exactly four, or "and X more".
+        areas_to_show = sorted(area.name for area in self)[:4]
+
+        count_of_areas_not_named = len(self.items) - 3
+        # if there's exactly one area not named, there are exactly four - we should just show all four.
+        if count_of_areas_not_named > 1:
+            areas_to_show = areas_to_show[:3] + [f'{count_of_areas_not_named} more…']
+
+        return formatted_list(areas_to_show, before_each='', after_each='')
 
 
 class BroadcastAreaLibraries(SerialisedModelCollection, GetItemByIdMixin):

app/broadcast_areas/repo.py

@@ -119,37 +119,6 @@ class BroadcastAreasRepository(object):
         libraries = [(row[0], row[1], row[2], row[3]) for row in results]
         return sorted(libraries)
 
-    def get_library_description(self, library_id):
-        # TODO: this count is wrong, and we shouldn't be building up user strings in sql. Replace this with python code.
-        q = """
-        WITH
-        areas AS (
-            SELECT * FROM broadcast_areas
-            WHERE broadcast_area_library_id = ?
-            AND broadcast_area_library_group_id IS NULL
-        ),
-        area_count AS (SELECT COUNT(*) AS c FROM areas),
-        subset_area_count AS (SELECT c - 4 FROM area_count),
-        description_area_names  AS (SELECT name FROM areas ORDER BY name ASC LIMIT 3),
-        description_areas_joined AS (
-            SELECT GROUP_CONCAT(name, ", ") FROM description_area_names
-        )
-        SELECT
-        CASE (SELECT * FROM subset_area_count)
-        WHEN 0 THEN
-            (SELECT * FROM description_areas_joined)
-            || ", and "
-            || (SELECT name from areas ORDER BY name DESC limit 1)
-        ELSE
-            (SELECT * FROM description_areas_joined)
-            || ", and "
-            || (SELECT * FROM subset_area_count)
-            || " more…"
-        END
-        """
-        description = self.query(q, library_id)[0][0]
-        return description
-
     def get_areas(self, area_ids):
         q = """
         SELECT id, name

tests/app/broadcast_areas/test_broadcast_area.py

@@ -38,10 +38,10 @@ def test_loads_areas_from_library():
 
 def test_examples():
     countries = broadcast_area_libraries.get('ctry19').get_examples()
-    assert countries == 'England, Northern Ireland, Scotland, and Wales'
+    assert countries == 'England, Northern Ireland, Scotland and Wales'
 
     wards = broadcast_area_libraries.get('wd20-lad20-ctyua19').get_examples()
-    assert wards == 'Aberdeen City, Aberdeenshire, Angus, and 213 more…'
+    assert wards == 'Aberdeen City, Aberdeenshire, Adur and 391 more…'
 
 
 @pytest.mark.parametrize('id', (

tests/app/main/views/test_broadcast.py

@@ -378,7 +378,7 @@ def test_choose_broadcast_library_page(
     ]
 
     assert normalize_spaces(page.select('.file-list-hint-large')[0].text) == (
-        'England, Northern Ireland, Scotland, and Wales'
+        'England, Northern Ireland, Scotland and Wales'
     )
 
     assert page.select_one('a.file-list-filename-large.govuk-link')['href'] == url_for(