From 65cc6adabfaa6650f2c5b3cd765398672684984e Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 5 Nov 2018 13:31:02 +0000
Subject: [PATCH] Sort letter brandings alphabetically
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently the brandings have non-deterministic sorting, which means
the order changes from page load to page load. This makes it hard to
find the item you’re looking for.

This commit sorts them by the name of the branding, same as for email
brandings.
---
 app/main/forms.py                             |  6 +++++-
 tests/app/main/views/test_service_settings.py | 15 +++++++++++++++
 tests/conftest.py                             |  1 +
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/app/main/forms.py b/app/main/forms.py
index 643fdcd31..f5929ee80 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -1,6 +1,7 @@
 import weakref
 from datetime import datetime, timedelta
 from itertools import chain
+from operator import itemgetter
 
 import pytz
 from flask_wtf import FlaskForm as Form
@@ -785,7 +786,10 @@ class LetterBranding(StripWhitespaceForm):
 
     def __init__(self, choices=[], *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self.dvla_org_id.choices = choices
+        self.dvla_org_id.choices = list(sorted(
+            choices,
+            key=itemgetter(1),
+        ))
 
     dvla_org_id = RadioField(
         'Which logo should this service’s letter have?',
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 27c038082..e7f36b77f 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -2088,6 +2088,7 @@ def test_set_letter_branding_platform_admin_only(
 @pytest.mark.parametrize('current_dvla_org_id, expected_selected', [
     (None, '001'),
     ('500', '500'),
+    ('999', '999'),
 ])
 def test_set_letter_branding_prepopulates(
     logged_in_platform_admin_client,
@@ -2101,6 +2102,20 @@ def test_set_letter_branding_prepopulates(
     response = logged_in_platform_admin_client.get(url_for('main.set_letter_branding', service_id=service_one['id']))
     assert response.status_code == 200
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+
+    for element in {'label', 'input[type=radio]'}:
+        assert len(page.select(element)) == 3
+
+    assert normalize_spaces(page.select('label')[0].text) == 'Animal and Plant Health Agency'
+    assert page.select('input')[0]['value'] == '999'
+
+    assert normalize_spaces(page.select('label')[1].text) == 'HM Government'
+    assert page.select('input')[1]['value'] == '001'
+
+    assert normalize_spaces(page.select('label')[2].text) == 'Land Registry'
+    assert page.select('input')[2]['value'] == '500'
+
+    assert len(page.select('input[checked]')) == 1
     assert page.select('input[checked]')[0]['value'] == expected_selected
 
 
diff --git a/tests/conftest.py b/tests/conftest.py
index 0b7c18c3f..4517203f9 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -2492,6 +2492,7 @@ def mock_get_letter_email_branding(mocker):
         return {
             '001': 'HM Government',
             '500': 'Land Registry',
+            '999': 'Animal and Plant Health Agency',
         }
 
     return mocker.patch(