[#1188] Fix visibility validator

To not rely on owner_org being present, when the validator gets a value
of private and there is no owner_org, we request the dataset anyway and
check if it actually has an owner_org.

Added new validation tests

ckan/logic/validators.py

@@ -692,7 +692,28 @@ def role_exists(role, context):
 
 def datasets_with_no_organization_cannot_be_private(key, data, errors,
         context):
-    if data[key] is True and data.get(('owner_org',)) is None:
+
+    dataset_id = data.get(('id',))
+    owner_org = data.get(('owner_org',))
+    private = data[key] is True
+
+    check_passed = True
+    from nose.tools import set_trace; set_trace()
+    if not dataset_id and private and owner_org is None:
+        # When creating a dataset, enforce it directly
+        check_passed = False
+    elif dataset_id and private and owner_org is None:
+        # Check if the dataset actually has an owner_org, even if not provided
+        try:
+            dataset_dict = logic.get_action('package_show')({},
+                            {'id': dataset_id})
+            if not dataset_dict.get('owner_org'):
+                check_passed = False
+
+        except logic.NotFound:
+            check_passed = False
+
+    if not check_passed:
         errors[key].append(
                 _("Datasets with no organization can't be private."))
 
@@ -723,4 +744,4 @@ def no_loops_in_hierarchy(key, data, errors, context):
                 not in allowable_parents:
             raise Invalid(_('This parent would create a loop in the '
                             'hierarchy'))
- 
+

ckan/new_tests/logic/test_validators.py

@@ -317,3 +317,74 @@ def call_validator(*args, **kwargs):
 
     # TODO: Test user_name_validator()'s behavior when there's a 'user_obj' in
     # the context dict.
+
+    def test_datasets_with_org_can_be_private_when_creating(self):
+
+        import ckan.logic.validators as validators
+
+        data = factories.validator_data_dict()
+        errors = factories.validator_errors_dict()
+
+        key = ('private',)
+        data[key] = True
+        errors[key] = []
+
+        data[('owner_org',)] = 'some_org_id'
+
+        # Mock ckan.model.
+        mock_model = mock.MagicMock()
+
+        @t.does_not_modify_errors_dict
+        @t.does_not_modify_data_dict
+        @t.returns_None
+        def call_validator(*args, **kwargs):
+            return validators.datasets_with_no_organization_cannot_be_private(*args, **kwargs)
+        call_validator(key, data, errors, context={'model': mock_model})
+
+
+    def test_datasets_with_no_org_cannot_be_private_when_creating(self):
+
+        import ckan.logic.validators as validators
+
+        data = factories.validator_data_dict()
+        errors = factories.validator_errors_dict()
+
+        key = ('private',)
+        data[key] = True
+        errors[key] = []
+
+        # Mock ckan.model.
+        mock_model = mock.MagicMock()
+
+        @t.does_not_modify_data_dict
+        @adds_message_to_errors_dict('Datasets with no organization can\'t be private.')
+        def call_validator(*args, **kwargs):
+            return validators.datasets_with_no_organization_cannot_be_private(*args, **kwargs)
+        call_validator(key, data, errors, context={'model': mock_model})
+
+    def test_datasets_with_org_can_be_private_when_updating(self):
+
+        import ckan.logic.validators as validators
+
+        data = factories.validator_data_dict()
+        errors = factories.validator_errors_dict()
+
+        key = ('private',)
+        data[key] = True
+        errors[key] = []
+
+        data[('id',)] = 'some_dataset_id'
+        data[('owner_org',)] = 'some_org_id'
+
+        # Mock ckan.model.
+        mock_model = mock.MagicMock()
+
+        @t.does_not_modify_errors_dict
+        @t.does_not_modify_data_dict
+        @t.returns_None
+        def call_validator(*args, **kwargs):
+            return validators.datasets_with_no_organization_cannot_be_private(*args, **kwargs)
+        call_validator(key, data, errors, context={'model': mock_model})
+
+    #TODO: Need to test when you are not providing owner_org and the validator
+    #      queries for the dataset with package_show