From 0f6a090470530fc7c1504cc6160ff77535c82fbc Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 23 Aug 2016 10:15:22 +0100
Subject: [PATCH 1/2] Fix admin app putting service into research mode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We changed the `update_service` method to only update indivdual
attributes of a service, and only allow it to update specified
attributes: https://github.com/alphagov/notifications-admin/commit/0cfe10639a0018553080885569b4ba125341122d

We neglected to specify `research_mode` as one of the allowed
attributes.

This broke the app’s ability to put a service in or out of research
mode.

This commit:
- makes sure the tests cover this eventuality
- fixes the bug by specifying `research_mode` as one of the allowed
  attributes
---
 app/notify_client/service_api_client.py       |  1 +
 tests/app/main/views/test_service_settings.py | 10 +++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py
index 0e67f28ca..db837e53c 100644
--- a/app/notify_client/service_api_client.py
+++ b/app/notify_client/service_api_client.py
@@ -89,6 +89,7 @@ class ServiceAPIClient(NotificationsAPIClient):
             'restricted',
             'email_from',
             'reply_to_email_address',
+            'research_mode',
             'sms_sender',
             'created_by',
             'branding',
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 8b214b647..6b54934ed 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -602,14 +602,18 @@ def test_switch_service_to_research_mode(
         mocker):
     with app_.test_request_context():
         with app_.test_client() as client:
-            mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one)
+            mocker.patch('app.service_api_client.post', return_value=service_one)
 
             client.login(active_user_with_permissions)
             response = client.get(url_for('main.service_switch_research_mode', service_id=service_one['id']))
             assert response.status_code == 302
             assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True)
-            app.service_api_client.update_service_with_properties.assert_called_with(
-                service_one['id'], {"research_mode": True}
+            app.service_api_client.post.assert_called_with(
+                '/service/{}'.format(service_one['id']),
+                {
+                    'research_mode': True,
+                    'created_by': active_user_with_permissions.id
+                }
             )
 
 

From f8db73ecc7cc09e69d3d11a7c2c4a30fa91ca89f Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 23 Aug 2016 10:16:05 +0100
Subject: [PATCH 2/2] Reduce uneccessary indentation

---
 tests/app/main/views/test_service_settings.py | 44 +++++++++----------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 6b54934ed..77817e033 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -592,29 +592,29 @@ def test_if_reply_to_email_address_set_then_form_populated(app_,
 
 
 def test_switch_service_to_research_mode(
-        app_,
-        service_one,
-        mock_login,
-        mock_get_user,
-        active_user_with_permissions,
-        mock_get_service,
-        mock_has_permissions,
-        mocker):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            mocker.patch('app.service_api_client.post', return_value=service_one)
+    app_,
+    service_one,
+    mock_login,
+    mock_get_user,
+    active_user_with_permissions,
+    mock_get_service,
+    mock_has_permissions,
+    mocker
+):
+    with app_.test_request_context(), app_.test_client() as client:
+        mocker.patch('app.service_api_client.post', return_value=service_one)
 
-            client.login(active_user_with_permissions)
-            response = client.get(url_for('main.service_switch_research_mode', service_id=service_one['id']))
-            assert response.status_code == 302
-            assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True)
-            app.service_api_client.post.assert_called_with(
-                '/service/{}'.format(service_one['id']),
-                {
-                    'research_mode': True,
-                    'created_by': active_user_with_permissions.id
-                }
-            )
+        client.login(active_user_with_permissions)
+        response = client.get(url_for('main.service_switch_research_mode', service_id=service_one['id']))
+        assert response.status_code == 302
+        assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True)
+        app.service_api_client.post.assert_called_with(
+            '/service/{}'.format(service_one['id']),
+            {
+                'research_mode': True,
+                'created_by': active_user_with_permissions.id
+            }
+        )
 
 
 def test_switch_service_from_research_mode_to_normal(