diff --git a/app/broadcast_areas/__init__.py b/app/broadcast_areas/__init__.py index f7488d0dc..b9edb42d5 100644 --- a/app/broadcast_areas/__init__.py +++ b/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): diff --git a/app/broadcast_areas/repo.py b/app/broadcast_areas/repo.py index b0e76d9d7..840d495df 100644 --- a/app/broadcast_areas/repo.py +++ b/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 diff --git a/tests/app/broadcast_areas/test_broadcast_area.py b/tests/app/broadcast_areas/test_broadcast_area.py index 01dfd2dd7..4c00c4137 100644 --- a/tests/app/broadcast_areas/test_broadcast_area.py +++ b/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', ( diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 71aa2a43b..caad2f2bc 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/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(