Merge pull request #24 from conwetlab/develop

Bug fixes

ckanext/privatedatasets/plugin.py

@@ -18,6 +18,7 @@
 # along with CKAN Private Dataset Extension.  If not, see <http://www.gnu.org/licenses/>.
 
 import ckan.lib.search as search
+import ckan.model as model
 import ckan.plugins as p
 import ckan.plugins.toolkit as tk
 import auth
@@ -148,6 +149,11 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm):
     ######################### IPACKAGECONTROLLER #########################
     ######################################################################
 
+    def _delete_pkg_atts(self, pkg_dict, attrs):
+        for attr in attrs:
+            if attr in pkg_dict:
+                del pkg_dict[attr]
+
     def before_index(self, pkg_dict):
 
         if 'extras_' + constants.SEARCHABLE in pkg_dict:
@@ -222,9 +228,7 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm):
         # * users allowed to update the allowed_users list via the notification API
         if pkg_dict.get('private') is False or not updating_via_api and (not user_obj or (pkg_dict['creator_user_id'] != user_obj.id and not user_obj.sysadmin)):
             attrs = [constants.ALLOWED_USERS, constants.SEARCHABLE, constants.ACQUIRE_URL]
-            for attr in attrs:
-                if attr in pkg_dict:
-                    del pkg_dict[attr]
+            self._delete_pkg_atts(pkg_dict, attrs)
 
         return pkg_dict
 
@@ -243,6 +247,32 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm):
 
         return pkg_dict
 
+    def after_search(self, search_results, search_params):
+        for result in search_results['results']:
+            # Extra fields should not be returned 
+            attrs = [constants.ALLOWED_USERS, constants.SEARCHABLE, constants.ACQUIRE_URL]
+            
+            # Additionally, resources should not be included if the user is not allowed
+            # to show the resource
+            context = {
+                'model': model,
+                'session': model.Session,
+                'user': tk.c.user,
+                'user_obj': tk.c.userobj
+            }
+
+            try:
+                tk.check_access('package_show', context, result)
+            except tk.NotAuthorized:
+                # NotAuthorized exception is risen when the user is not allowed
+                # to read the package.
+                attrs.append('resources')
+
+            # Delete 
+            self._delete_pkg_atts(result, attrs)
+
+        return search_results
+
     ######################################################################
     ######################### ITEMPLATESHELPER ###########################
     ######################################################################

ckanext/privatedatasets/tests/test_plugin.py

@@ -18,6 +18,7 @@
 # along with CKAN Private Dataset Extension.  If not, see <http://www.gnu.org/licenses/>.
 
 import unittest
+import copy
 import ckanext.privatedatasets.plugin as plugin
 
 from mock import MagicMock
@@ -30,6 +31,7 @@ class PluginTest(unittest.TestCase):
         # Create mocks
         self._tk = plugin.tk
         plugin.tk = MagicMock()
+        plugin.tk.NotAuthorized = self._tk.NotAuthorized
 
         self._db = plugin.db
         plugin.db = MagicMock()
@@ -385,3 +387,49 @@ class PluginTest(unittest.TestCase):
     ])
     def test_packagecontroller_after_update(self, new_users, current_users, users_to_add, users_to_delete):
         self._aux_test_after_create_update(self.privateDatasets.after_update, new_users, current_users, users_to_add, users_to_delete)
+
+    @parameterized.expand([
+        (1, True),
+        (1, False),
+        # Complex results
+        (3, True),
+        (3, False)
+    ])
+    def test_packagecontroller_after_search(self, num_seach_results, user_allowed):
+
+        # Create the list with the 
+        remaining_fields = ['other_id', 'name', 'author']
+        # Resources field should be in the result when the user is allowed to show the dataset
+        if user_allowed:
+            remaining_fields.append('resources')
+
+        search_results = {'facets': ['facet1', 'facet2'], 'results': [], 'elements': num_seach_results}
+        # Add resources
+        for _ in range(num_seach_results):
+            search_results['results'].append({
+                'allowed_users': ['user1', 'user2'],
+                'seearchable': True,
+                'acquire_url': 'https://upm.es',
+                'resources': ['resource1', 'resource2'],
+                remaining_fields[0]: 'value1',
+                remaining_fields[1]: 'value2',
+                remaining_fields[2]: 'value3'
+            })
+
+        # Mocking
+        plugin.tk.check_access.side_effect = None if user_allowed else plugin.tk.NotAuthorized
+
+        # Call the function
+        final_search_results =  self.privateDatasets.after_search(copy.deepcopy(search_results), None)
+
+        # Assertations
+        for result in final_search_results['results']:
+            self.assertNotIn('allowed_users', result)
+            self.assertNotIn('searchable', result)
+            self.assertNotIn('acquire_url', result)
+
+            for remaining_field in remaining_fields:
+                self.assertIn(remaining_field, result)
+
+        self.assertEquals(final_search_results['facets'], search_results['facets'])
+        self.assertEquals(final_search_results['elements'], search_results['elements'])

ckanext/privatedatasets/tests/test_selenium.py

@@ -68,7 +68,7 @@ class TestSelenium(unittest.TestCase):
         self.clearBBDD()
 
         self.driver = webdriver.Firefox()
-        self.driver.implicitly_wait(5000000)
+        self.driver.implicitly_wait(5)
         self.driver.set_window_size(1024, 768)
         self.base_url = 'http://127.0.0.1:5000/'
 
@@ -178,12 +178,10 @@ class TestSelenium(unittest.TestCase):
         driver.find_element_by_id('field-description').send_keys(resource_description)
         driver.find_element_by_id('s2id_autogen1').clear()
         driver.find_element_by_id('s2id_autogen1').send_keys(resource_format)
-        save_elements = driver.find_elements_by_name('save')
-        save_elements[len(save_elements) - 1].click()
+        driver.find_element_by_css_selector('button.btn.btn-primary').click()
 
         # THIRD PAGE: Metadata
-        save_elements = driver.find_elements_by_name('save')
-        save_elements[len(save_elements) - 1].click()
+        driver.find_element_by_css_selector('button.btn.btn-primary').click()
 
     def modify_ds(self, url, name, description, tags, private, searchable, allowed_users, acquire_url):
         driver = self.driver