Merge pull request #1420 from alphagov/template-order-by-name

List templates in alphabetical order

app/dao/templates_dao.py

@@ -1,7 +1,7 @@
 from datetime import datetime
 import uuid
 
-from sqlalchemy import desc
+from sqlalchemy import asc, desc
 from sqlalchemy.sql.expression import bindparam
 
 from app import db
@@ -65,14 +65,16 @@ def dao_get_all_templates_for_service(service_id, template_type=None):
             template_type=template_type,
             archived=False
         ).order_by(
-            desc(Template.created_at)
+            asc(Template.name),
+            asc(Template.template_type),
         ).all()
 
     return Template.query.filter_by(
         service_id=service_id,
         archived=False
     ).order_by(
-        desc(Template.created_at)
+        asc(Template.name),
+        asc(Template.template_type),
     ).all()
 
 

tests/app/dao/test_templates_dao.py

@@ -163,7 +163,7 @@ def test_get_all_templates_for_service(notify_db, notify_db_session, service_fac
     assert len(dao_get_all_templates_for_service(service_2.id)) == 2
 
 
-def test_get_all_templates_for_service_shows_newest_created_first(notify_db, notify_db_session, sample_service):
+def test_get_all_templates_for_service_is_alphabetised(notify_db, notify_db_session, sample_service):
     template_1 = create_sample_template(
         notify_db,
         notify_db_session,
@@ -190,14 +190,14 @@ def test_get_all_templates_for_service_shows_newest_created_first(notify_db, not
     )
 
     assert Template.query.count() == 3
-    assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template 3'
+    assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template 1'
     assert dao_get_all_templates_for_service(sample_service.id)[1].name == 'Sample Template 2'
-    assert dao_get_all_templates_for_service(sample_service.id)[2].name == 'Sample Template 1'
+    assert dao_get_all_templates_for_service(sample_service.id)[2].name == 'Sample Template 3'
 
-    template_2.name = 'Sample Template 2 (updated)'
+    template_2.name = 'AAAAA Sample Template 2'
     dao_update_template(template_2)
-    assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template 3'
+    assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'AAAAA Sample Template 2'
-    assert dao_get_all_templates_for_service(sample_service.id)[1].name == 'Sample Template 2 (updated)'
+    assert dao_get_all_templates_for_service(sample_service.id)[1].name == 'Sample Template 1'
 
 
 def test_get_all_returns_empty_list_if_no_templates(sample_service):

tests/app/db.py

@@ -120,12 +120,13 @@ def create_service_with_defined_sms_sender(
 def create_template(
     service,
     template_type=SMS_TYPE,
+    template_name=None,
     subject='Template subject',
     content='Dear Sir/Madam, Hello. Yours Truly, The Government.',
     template_id=None
 ):
     data = {
-        'name': '{} Template Name'.format(template_type),
+        'name': template_name or '{} Template Name'.format(template_type),
         'template_type': template_type,
         'content': content,
         'service': service,

tests/app/template/test_rest.py

@@ -301,10 +301,10 @@ def test_should_be_able_to_get_all_templates_for_a_service(client, sample_user,
 
     assert response.status_code == 200
     update_json_resp = json.loads(response.get_data(as_text=True))
-    assert update_json_resp['data'][0]['name'] == 'my template 2'
+    assert update_json_resp['data'][0]['name'] == 'my template 1'
     assert update_json_resp['data'][0]['version'] == 1
     assert update_json_resp['data'][0]['created_at']
-    assert update_json_resp['data'][1]['name'] == 'my template 1'
+    assert update_json_resp['data'][1]['name'] == 'my template 2'
     assert update_json_resp['data'][1]['version'] == 1
     assert update_json_resp['data'][1]['created_at']
 

tests/app/v2/templates/test_get_templates.py

@@ -1,6 +1,7 @@
 import pytest
 
 from flask import json
+from itertools import product
 
 from app.models import TEMPLATE_TYPES, EMAIL_TYPE
 from tests import create_authorization_header
@@ -8,12 +9,15 @@ from tests.app.db import create_template
 
 
 def test_get_all_templates_returns_200(client, sample_service):
-    num_templates = 3
+    templates = [
-    templates = []
+        create_template(
-    for i in range(num_templates):
+            sample_service,
-        for tmp_type in TEMPLATE_TYPES:
+            template_type=tmp_type,
-            subject = 'subject_{}'.format(i) if tmp_type == EMAIL_TYPE else ''
+            subject='subject_{}'.format(name) if tmp_type == EMAIL_TYPE else '',
-            templates.append(create_template(sample_service, template_type=tmp_type, subject=subject))
+            template_name=name,
+        )
+        for name, tmp_type in product(('A', 'B', 'C'), TEMPLATE_TYPES)
+    ]
 
     auth_header = create_authorization_header(service_id=sample_service.id)
 
@@ -25,25 +29,27 @@ def test_get_all_templates_returns_200(client, sample_service):
 
     json_response = json.loads(response.get_data(as_text=True))
 
-    assert len(json_response['templates']) == num_templates * len(TEMPLATE_TYPES)
+    assert len(json_response['templates']) == len(templates)
 
-    # need to reverse index as get all templates returns list sorted by descending date
+    for index, template in enumerate(json_response['templates']):
-    for i in range(len(json_response['templates'])):
+        assert template['id'] == str(templates[index].id)
-        reverse_index = len(json_response['templates']) - 1 - i
+        assert template['body'] == templates[index].content
-        assert json_response['templates'][reverse_index]['id'] == str(templates[i].id)
+        assert template['type'] == templates[index].template_type
-        assert json_response['templates'][reverse_index]['body'] == templates[i].content
+        if templates[index].template_type == EMAIL_TYPE:
-        assert json_response['templates'][reverse_index]['type'] == templates[i].template_type
+            assert template['subject'] == templates[index].subject
-        if templates[i].template_type == EMAIL_TYPE:
-            assert json_response['templates'][reverse_index]['subject'] == templates[i].subject
 
 
 @pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES)
 def test_get_all_templates_for_valid_type_returns_200(client, sample_service, tmp_type):
-    num_templates = 3
+    templates = [
-    templates = []
+        create_template(
-    for i in range(num_templates):
+            sample_service,
-        subject = 'subject_{}'.format(i) if tmp_type == EMAIL_TYPE else ''
+            template_type=tmp_type,
-        templates.append(create_template(sample_service, template_type=tmp_type, subject=subject))
+            template_name='Template {}'.format(i),
+            subject='subject_{}'.format(i) if tmp_type == EMAIL_TYPE else ''
+        )
+        for i in range(3)
+    ]
 
     auth_header = create_authorization_header(service_id=sample_service.id)
 
@@ -55,16 +61,14 @@ def test_get_all_templates_for_valid_type_returns_200(client, sample_service, tm
 
     json_response = json.loads(response.get_data(as_text=True))
 
-    assert len(json_response['templates']) == num_templates
+    assert len(json_response['templates']) == len(templates)
 
-    # need to reverse index as get all templates returns list sorted by descending date
+    for index, template in enumerate(json_response['templates']):
-    for i in range(len(json_response['templates'])):
+        assert template['id'] == str(templates[index].id)
-        reverse_index = len(json_response['templates']) - 1 - i
+        assert template['body'] == templates[index].content
-        assert json_response['templates'][reverse_index]['id'] == str(templates[i].id)
+        assert template['type'] == tmp_type
-        assert json_response['templates'][reverse_index]['body'] == templates[i].content
+        if templates[index].template_type == EMAIL_TYPE:
-        assert json_response['templates'][reverse_index]['type'] == tmp_type
+            assert template['subject'] == templates[index].subject
-        if templates[i].template_type == EMAIL_TYPE:
-            assert json_response['templates'][reverse_index]['subject'] == templates[i].subject
 
 
 @pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES)