From 99c0f33b7feb33c8c4d6e3ed40a26dc84a7f3e51 Mon Sep 17 00:00:00 2001
From: amercader <amercadero@gmail.com>
Date: Mon, 13 Jun 2016 10:21:53 +0100
Subject: [PATCH] [#3108] Raise validation errors on group/org_member_create

Add not_missing to the schema and raise the errors on the action. Adds
some tests as well
---
 ckan/logic/action/create.py            |  3 ++
 ckan/logic/schema.py                   |  6 ++--
 ckan/tests/logic/action/test_create.py | 43 +++++++++++++++++++++++++-
 3 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py
index 264fe0ca2ee..0a2a175309d 100644
--- a/ckan/logic/action/create.py
+++ b/ckan/logic/action/create.py
@@ -1372,6 +1372,9 @@ def _group_or_org_member_create(context, data_dict, is_org=False):
 
     schema = ckan.logic.schema.member_schema()
     data, errors = _validate(data_dict, schema, context)
+    if errors:
+        model.Session.rollback()
+        raise ValidationError(errors)
 
     username = _get_or_bust(data_dict, 'username')
     role = data_dict.get('role')
diff --git a/ckan/logic/schema.py b/ckan/logic/schema.py
index b16c914f28a..ad1f3d8cebe 100644
--- a/ckan/logic/schema.py
+++ b/ckan/logic/schema.py
@@ -532,9 +532,9 @@ def default_follow_dataset_schema():
 
 def member_schema():
     schema = {
-        'id': [group_id_exists, unicode],
-        'username': [user_name_exists, unicode],
-        'role': [role_exists, unicode],
+        'id': [not_missing, not_empty, group_id_exists, unicode],
+        'username': [not_missing, user_name_exists, unicode],
+        'role': [not_missing, role_exists, unicode],
     }
     return schema
 
diff --git a/ckan/tests/logic/action/test_create.py b/ckan/tests/logic/action/test_create.py
index b48f65c5f9d..e38ab5656fd 100644
--- a/ckan/tests/logic/action/test_create.py
+++ b/ckan/tests/logic/action/test_create.py
@@ -151,7 +151,6 @@ def setup_class(cls):
 
     @classmethod
     def teardown_class(cls):
-        p.unload('image_view')
 
         helpers.reset_db()
 
@@ -485,6 +484,48 @@ def test_organization_member_creation(self):
         assert_equals(new_membership['table_id'], user['id'])
         assert_equals(new_membership['capacity'], 'member')
 
+    def test_group_member_creation_raises_validation_error_if_id_missing(self):
+
+        assert_raises(logic.ValidationError,
+                      helpers.call_action, 'group_member_create',
+                      username='someuser',
+                      role='member',)
+
+    def test_group_member_creation_raises_validation_error_if_username_missing(self):
+
+        assert_raises(logic.ValidationError,
+                      helpers.call_action, 'group_member_create',
+                      id='someid',
+                      role='member',)
+
+    def test_group_member_creation_raises_validation_error_if_role_missing(self):
+
+        assert_raises(logic.ValidationError,
+                      helpers.call_action, 'group_member_create',
+                      id='someid',
+                      username='someuser',)
+
+    def test_org_member_creation_raises_validation_error_if_id_missing(self):
+
+        assert_raises(logic.ValidationError,
+                      helpers.call_action, 'organization_member_create',
+                      username='someuser',
+                      role='member',)
+
+    def test_org_member_creation_raises_validation_error_if_username_missing(self):
+
+        assert_raises(logic.ValidationError,
+                      helpers.call_action, 'organization_member_create',
+                      id='someid',
+                      role='member',)
+
+    def test_org_member_creation_raises_validation_error_if_role_missing(self):
+
+        assert_raises(logic.ValidationError,
+                      helpers.call_action, 'organization_member_create',
+                      id='someid',
+                      username='someuser',)
+
 
 class TestDatasetCreate(helpers.FunctionalTestBase):