Merge pull request #5442 from hypothesis/user-api-use-update-service

Refactor `users` API view to use service for update

h/views/api/users.py

@@ -69,19 +69,13 @@ def update(user, request):
     schema = UpdateUserAPISchema()
     appstruct = schema.validate(_json_payload(request))
 
-    _update_user(user, appstruct)
+    user_update_service = request.find_service(name='user_update')
+    user = user_update_service.update(user, **appstruct)
 
     presenter = UserJSONPresenter(user)
     return presenter.asdict()
 
 
-def _update_user(user, appstruct):
-    if 'email' in appstruct:
-        user.email = appstruct['email']
-    if 'display_name' in appstruct:
-        user.display_name = appstruct['display_name']
-
-
 def _json_payload(request):
     try:
         return request.json_body

tests/functional/api/test_users.py

@@ -76,6 +76,19 @@ def test_it_returns_http_200_when_successful(self, app, auth_client_header, user
 
         assert res.status_code == 200
 
+    def test_it_ignores_unrecognized_parameters(self, app, auth_client_header, user):
+        url = "/api/users/{username}".format(username=user.username)
+        payload = {
+            'email': 'fingers@bonzo.com',
+            'authority': 'nicetry.com'
+        }
+
+        res = app.patch_json(url, payload, headers=auth_client_header)
+
+        assert res.status_code == 200
+        assert res.json_body['email'] == 'fingers@bonzo.com'
+        assert res.json_body['authority'] == 'example.com'
+
     def test_it_returns_updated_user_when_successful(self, app, auth_client_header, user, patch_user_payload):
         url = "/api/users/{username}".format(username=user.username)
 

tests/h/views/api/users_test.py

@@ -9,6 +9,7 @@
 from h.models.auth_client import GrantType
 from h.schemas import ValidationError
 from h.services.user_signup import UserSignupService
+from h.services.user_update import UserUpdateService
 from h.services.user_unique import UserUniqueService, DuplicateUserError
 from h.views.api.users import create, update
 
@@ -33,17 +34,17 @@ def test_signs_up_user(self,
             display_name='Jeremy Weyland',
             identities=[{'provider': 'provider_a', 'provider_unique_id': 'abc123'}])
 
-    def test_it_presents_user(self, pyramid_request, valid_payload, user, presenter):
+    def test_it_presents_user(self, pyramid_request, valid_payload, user, UserJSONPresenter):
         pyramid_request.json_body = valid_payload
         create(pyramid_request)
 
-        presenter.assert_called_once_with(user)
+        UserJSONPresenter.assert_called_once_with(user)
 
-    def test_it_returns_presented_user(self, pyramid_request, valid_payload, presenter):
+    def test_it_returns_presented_user(self, pyramid_request, valid_payload, UserJSONPresenter):
         pyramid_request.json_body = valid_payload
         result = create(pyramid_request)
 
-        assert result == presenter.return_value.asdict()
+        assert result == UserJSONPresenter.return_value.asdict()
 
     def test_it_validates_the_input(self, pyramid_request, valid_payload, CreateUserAPISchema):
         create_schema = CreateUserAPISchema.return_value
@@ -115,73 +116,48 @@ def valid_payload(self):
 
 @pytest.mark.usefixtures('auth_client',
                          'user_svc',
-                         'user')
+                         'user',
+                         'user_update_svc',
+                         'UpdateUserAPISchema',
+                         'UserJSONPresenter')
 class TestUpdate(object):
-    def test_it_updates_display_name(self, pyramid_request, valid_payload, user):
-        pyramid_request.json_body = valid_payload
-        update(user, pyramid_request)
-
-        assert user.display_name == 'Jeremy Weyland'
-
-    def test_it_updates_email(self, pyramid_request, valid_payload, user):
-        pyramid_request.json_body = valid_payload
-        update(user, pyramid_request)
 
-        assert user.email == 'jeremy@weylandtech.com'
-
-    def test_you_can_update_the_displayname_of_a_user_who_has_no_email(
-            self, factories, pyramid_request, user_svc, valid_payload):
-        user = factories.User(display_name='old_display_name', email=None)
-        user_svc.fetch.return_value = user
-        user_svc.fetch.side_effect = None
-        del valid_payload['email']
-        valid_payload['display_name'] = 'new_display_name'
-        pyramid_request.json_body = valid_payload
+    def test_it_validates_request_payload(self, pyramid_request, user, user_update_svc, UpdateUserAPISchema):
+        data = {
+            'display_name': 'Rudolph Blimp',
+            'email': 'fingers@perplexology.com'
+        }
+        pyramid_request.json_body = data
 
         update(user, pyramid_request)
 
-        assert user.display_name == 'new_display_name'
-        assert user.email is None
+        UpdateUserAPISchema.return_value.validate.assert_called_once_with(data)
 
-    def test_you_can_add_an_email_to_a_user_who_has_no_email(
-            self, factories, pyramid_request, user_svc, valid_payload):
-        user = factories.User(email=None)
-        user_svc.fetch.return_value = user
-        user_svc.fetch.side_effect = None
-        del valid_payload['display_name']
-        valid_payload['email'] = 'new@new.com'
-        pyramid_request.json_body = valid_payload
+    def test_it_proxies_to_user_update_svc(self, pyramid_request, user, user_update_svc, UpdateUserAPISchema):
+        appstruct = {
+            'display_name': 'Rudolph Blimp',
+            'email': 'fingers@perplexology.com'
+        }
+        UpdateUserAPISchema.return_value.validate.return_value = appstruct
+        user_update_svc.update.return_value = user
 
         update(user, pyramid_request)
 
-        assert user.email == 'new@new.com'
+        user_update_svc.update.assert_called_once_with(user, **appstruct)
 
-    def test_it_presents_user(self, pyramid_request, valid_payload, user, presenter):
-        pyramid_request.json_body = valid_payload
+    def test_it_presents_updated_user_returned_from_service(self, pyramid_request, user, UserJSONPresenter, user_update_svc):
+        user_update_svc.update.return_value = user
         update(user, pyramid_request)
 
-        presenter.assert_called_once_with(user)
+        UserJSONPresenter.assert_called_once_with(user)
 
-    def test_it_returns_presented_user(self, pyramid_request, valid_payload, presenter):
-        pyramid_request.json_body = valid_payload
+    def test_it_returns_presented_user(self, pyramid_request, valid_payload, UserJSONPresenter):
         result = update(user, pyramid_request)
 
-        assert result == presenter.return_value.asdict()
-
-    def test_it_validates_the_input(self, user, pyramid_request, valid_payload, UpdateUserAPISchema):
-        update_schema = UpdateUserAPISchema.return_value
-        update_schema.validate.return_value = valid_payload
-        pyramid_request.json_body = valid_payload
-
-        update(user, pyramid_request)
-
-        update_schema.validate.assert_called_once_with(valid_payload)
+        assert result == UserJSONPresenter.return_value.asdict()
 
     def test_raises_when_schema_validation_fails(self, user, pyramid_request, valid_payload, UpdateUserAPISchema):
-        update_schema = UpdateUserAPISchema.return_value
-        update_schema.validate.side_effect = ValidationError('validation failed')
-
-        pyramid_request.json_body = valid_payload
+        UpdateUserAPISchema.return_value.validate.side_effect = ValidationError('validation failed')
 
         with pytest.raises(ValidationError):
             update(user, pyramid_request)
@@ -194,6 +170,9 @@ def test_raises_for_invalid_json_body(self, user, pyramid_request, patch):
 
     @pytest.fixture
     def pyramid_request(self, pyramid_request, user):
+        # Add a nominal json_body so that _json_payload() parsing of
+        # it doesn't raise
+        pyramid_request.json_body = {}
         pyramid_request.matchdict['username'] = user.username
         return pyramid_request
 
@@ -243,6 +222,13 @@ def user_unique_svc(pyramid_config):
     return svc
 
 
+@pytest.fixture
+def user_update_svc(pyramid_config):
+    svc = mock.create_autospec(UserUpdateService, spec_set=True, instance=True)
+    pyramid_config.register_service(svc, name='user_update')
+    return svc
+
+
 @pytest.fixture
 def CreateUserAPISchema(patch):
     return patch('h.views.api.users.CreateUserAPISchema')
@@ -254,7 +240,7 @@ def UpdateUserAPISchema(patch):
 
 
 @pytest.fixture
-def presenter(patch):
+def UserJSONPresenter(patch):
     return patch('h.views.api.users.UserJSONPresenter')