From b49437ec5f829a832afdae5f90dc9194fdf3b8d3 Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Wed, 28 Nov 2018 13:24:31 +0400 Subject: [PATCH 01/16] [logic][s]: custom validator for state - refs #12 --- ckanext/ed/plugin.py | 17 ++++++++++------- ckanext/ed/schemas/dataset.json | 5 +++++ ckanext/ed/validators.py | 19 +++++++++++++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 ckanext/ed/validators.py diff --git a/ckanext/ed/plugin.py b/ckanext/ed/plugin.py index 5fb27f0..6adccd6 100644 --- a/ckanext/ed/plugin.py +++ b/ckanext/ed/plugin.py @@ -1,8 +1,8 @@ +from ckan.lib.plugins import DefaultTranslation import ckan.plugins as plugins import ckan.plugins.toolkit as toolkit -from ckanext.ed import helpers -from ckanext.ed import actions -from ckan.lib.plugins import DefaultTranslation + +from ckanext.ed import actions, helpers, validators class EDPlugin(plugins.SingletonPlugin, DefaultTranslation): @@ -11,9 +11,9 @@ class EDPlugin(plugins.SingletonPlugin, DefaultTranslation): plugins.implements(plugins.ITranslation) plugins.implements(plugins.IActions) plugins.implements(plugins.IRoutes, inherit=True) + plugins.implements(plugins.IValidators) # ITemplateHelpers - def get_helpers(self): return { 'ed_get_groups': helpers.get_groups, @@ -23,21 +23,18 @@ def get_helpers(self): } # IActions - def get_actions(self): return { 'ed_prepare_zip_resources': actions.prepare_zip_resources, } # IConfigurer - def update_config(self, config_): toolkit.add_template_directory(config_, 'templates') toolkit.add_public_directory(config_, 'public') toolkit.add_resource('fanstatic', 'ed') # IRoutes - def before_map(self, map): map.connect( 'download_zip', @@ -47,3 +44,9 @@ def before_map(self, map): ) return map + + # IValidators + def get_validators(self): + return { + 'state_validator': validators.state_validator + } diff --git a/ckanext/ed/schemas/dataset.json b/ckanext/ed/schemas/dataset.json index b7661b8..d6fed42 100644 --- a/ckanext/ed/schemas/dataset.json +++ b/ckanext/ed/schemas/dataset.json @@ -117,6 +117,11 @@ "label": "Temporal", "form_placeholder": "eg. 2000-01-15T00:45:00Z/2010-01-15T00:06:00Z", "help_text": "The range of temporal applicability of a dataset (i.e., a start and end date of applicability for the data)." + }, + { + "field_name": "state", + "form_snippet": false, + "validators": "state_validator" } ], "resource_fields": [ diff --git a/ckanext/ed/validators.py b/ckanext/ed/validators.py new file mode 100644 index 0000000..7968424 --- /dev/null +++ b/ckanext/ed/validators.py @@ -0,0 +1,19 @@ +from logging import getLogger + +import ckan.logic.action.get as get_core + +from ckanext.ed import helpers + + +log = getLogger(__name__) + + +def state_validator(key, data, errors, context): + user_orgs = get_core.organization_list_for_user(context, {'id': context['user']}) + office_id = data.get(('owner_org',)) + state = 'approval_pending' + # Approve if user is member of the org and admin + for org in user_orgs: + if org.get('id') == office_id and org.get('capacity') == 'admin': + state = 'approved' + data[key] = state From b8cba5a51ba87ce2387f20dda084a2841588e507 Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Wed, 28 Nov 2018 13:26:08 +0400 Subject: [PATCH 02/16] [views][m]: Note for editors - refs #12 --- ckanext/ed/helpers.py | 14 ++++++ ckanext/ed/plugin.py | 1 + .../templates/package/new_package_form.html | 34 +++++++++++++ .../ed/templates/package/snippets/stages.html | 48 +++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 ckanext/ed/templates/package/new_package_form.html create mode 100644 ckanext/ed/templates/package/snippets/stages.html diff --git a/ckanext/ed/helpers.py b/ckanext/ed/helpers.py index 0193dc1..fefa17c 100644 --- a/ckanext/ed/helpers.py +++ b/ckanext/ed/helpers.py @@ -105,3 +105,17 @@ def get_total_views_for_dataset(id): return dataset.get('tracking_summary').get('total') except Exception: return 0 + + +def is_admin(user): + """ + Returns True if user is admin of any organisation + + :param user: user name + :type user: string + + :returns: True/False + :rtype: boolean + """ + user_orgs = _get_action('organization_list_for_user', {}, {'user': user}) + return any([i.get('capacity') == 'admin' for i in user_orgs]) diff --git a/ckanext/ed/plugin.py b/ckanext/ed/plugin.py index 6adccd6..0de5eb3 100644 --- a/ckanext/ed/plugin.py +++ b/ckanext/ed/plugin.py @@ -17,6 +17,7 @@ class EDPlugin(plugins.SingletonPlugin, DefaultTranslation): def get_helpers(self): return { 'ed_get_groups': helpers.get_groups, + 'ed_is_admin': helpers.is_admin, 'ed_get_recently_updated_datasets': helpers.get_recently_updated_datasets, 'ed_get_most_popular_datasets': helpers.get_most_popular_datasets, 'ed_get_total_views_for_dataset': helpers.get_total_views_for_dataset, diff --git a/ckanext/ed/templates/package/new_package_form.html b/ckanext/ed/templates/package/new_package_form.html new file mode 100644 index 0000000..da5b87d --- /dev/null +++ b/ckanext/ed/templates/package/new_package_form.html @@ -0,0 +1,34 @@ +{% extends 'package/snippets/package_form.html' %} + +{% block stages %} + {% if form_style != 'edit' %} + {% if not h.ed_is_admin(c.user) %} + + {% endif %} + {{ super() }} + {% endif %} +{% endblock %} + +{% block save_button_text %} + {% if form_style != 'edit' %} + {{ super() }} + {% else %} + {{ _('Update Dataset') }} + {% endif %} +{% endblock %} + +{% block cancel_button %} + {% if form_style != 'edit' %} + {{ super() }} + {% endif %} +{% endblock %} + +{% block delete_button %} + {% if form_style == 'edit' and h.check_access('package_delete', {'id': pkg_dict.id}) %} + {{ super() }} + {% endif %} +{% endblock %} diff --git a/ckanext/ed/templates/package/snippets/stages.html b/ckanext/ed/templates/package/snippets/stages.html new file mode 100644 index 0000000..8092be5 --- /dev/null +++ b/ckanext/ed/templates/package/snippets/stages.html @@ -0,0 +1,48 @@ +{# +Inserts a stepped progress indicator for the new package form. Each stage can +have one of three states, "uncomplete", "complete" and "active". + +stages - A list of states for each of the three stages. Missing stages default + to "uncomplete". + +Example: + + {% snippet 'package/snippets/stages.html', stages=['active'] %} + {% snippet 'package/snippets/stages.html', stages=['complete', 'active'] %} + {% snippet 'package/snippets/stages.html', stages=['active', 'complete'] %} + +#} +{% set s1 = stages[0] or 'active' %} +{% set s2 = stages[1] or 'uncomplete' %} +{% if s1 != 'uncomplete' %}{% set class = 'stage-1' %}{% endif %} +{% if s2 != 'uncomplete' %}{% set class = 'stage-2' %}{% endif %} + +
    +
  1. + {% if s1 != 'complete' %} + {% if h.ed_is_admin(c.user) %} + {{ _('Create dataset') }} + {% else %} + {{ _('Request dataset') }} + {% endif %} + {% else %} + {% if h.ed_is_admin(c.user) %} + + {% else %} + + {% endif %} + {% endif %} +
    2. +
  2. + {% if s2 != 'complete' %} + {{ _('Add data') }} + {% else %} + {% if s1 == 'active' %} + {# stage 1 #} + + {% else %} + {% link_for _('Add data'), named_route='dataset.new', class_="highlight" %} + {% endif %} + {% endif %} +
    3. +
From 75395d5af7ef62ea4344b35e435ffabdf2ee8827 Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Wed, 28 Nov 2018 15:57:57 +0400 Subject: [PATCH 03/16] [search][s]: don't show pending packages. refs #12 --- ckanext/ed/actions.py | 20 +++++++++++++++----- ckanext/ed/plugin.py | 1 + ckanext/ed/schemas/dataset.json | 4 ++-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/ckanext/ed/actions.py b/ckanext/ed/actions.py index d0cb1ef..95508d3 100644 --- a/ckanext/ed/actions.py +++ b/ckanext/ed/actions.py @@ -1,12 +1,15 @@ -from ckan.plugins import toolkit +from logging import getLogger +import os +import requests import uuid -from ckanext.ed import helpers import zipfile -import os + from ckan.controllers.admin import get_sysadmins -import requests -from logging import getLogger +from ckan.logic import check_access, NotFound +from ckan.logic.action.get import package_search as core_package_search +from ckan.plugins import toolkit +from ckanext.ed import helpers SUPPORTED_RESOURCE_MIMETYPES = [ 'application/vnd.openxmlformats-officedocument.presentationml.presentation', @@ -136,3 +139,10 @@ def prepare_zip_resources(context, data_dict): os.remove(file_path) return {'zip_id': None} + + +@toolkit.side_effect_free +def package_search(context, data_dict): + data_dict['fq'] = '!(approval_state:approval_pending) ' + data_dict.get('fq', '') + packages = core_package_search(context, data_dict) + return packages diff --git a/ckanext/ed/plugin.py b/ckanext/ed/plugin.py index 0de5eb3..3ef66ba 100644 --- a/ckanext/ed/plugin.py +++ b/ckanext/ed/plugin.py @@ -27,6 +27,7 @@ def get_helpers(self): def get_actions(self): return { 'ed_prepare_zip_resources': actions.prepare_zip_resources, + 'package_search': actions.package_search } # IConfigurer diff --git a/ckanext/ed/schemas/dataset.json b/ckanext/ed/schemas/dataset.json index d6fed42..6796945 100644 --- a/ckanext/ed/schemas/dataset.json +++ b/ckanext/ed/schemas/dataset.json @@ -119,8 +119,8 @@ "help_text": "The range of temporal applicability of a dataset (i.e., a start and end date of applicability for the data)." }, { - "field_name": "state", - "form_snippet": false, + "field_name": "approval_state", + "form_snippet": null, "validators": "state_validator" } ], From 609c7931b23b3618f701aa3e3d90052404572467 Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Thu, 29 Nov 2018 10:04:51 +0400 Subject: [PATCH 04/16] [showcase][s]: restrict anonimous users - refs #12 --- ckanext/ed/actions.py | 12 ++++++++++++ ckanext/ed/plugin.py | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/ckanext/ed/actions.py b/ckanext/ed/actions.py index 95508d3..f2c044a 100644 --- a/ckanext/ed/actions.py +++ b/ckanext/ed/actions.py @@ -7,6 +7,7 @@ from ckan.controllers.admin import get_sysadmins from ckan.logic import check_access, NotFound from ckan.logic.action.get import package_search as core_package_search +from ckan.logic.action.get import package_show as core_package_show from ckan.plugins import toolkit from ckanext.ed import helpers @@ -146,3 +147,14 @@ def package_search(context, data_dict): data_dict['fq'] = '!(approval_state:approval_pending) ' + data_dict.get('fq', '') packages = core_package_search(context, data_dict) return packages + + +@toolkit.side_effect_free +def package_show(context, data_dict): + package = core_package_show(context, data_dict) + # User with less perms then creator should not be able to access pending dataset + approval_pending = package.get('approval_state') == 'approval_pending' + is_editor = check_access('package_create', context, data_dict) + if not is_editor and approval_pending: + raise NotFound + return package diff --git a/ckanext/ed/plugin.py b/ckanext/ed/plugin.py index 3ef66ba..d9512c4 100644 --- a/ckanext/ed/plugin.py +++ b/ckanext/ed/plugin.py @@ -27,7 +27,8 @@ def get_helpers(self): def get_actions(self): return { 'ed_prepare_zip_resources': actions.prepare_zip_resources, - 'package_search': actions.package_search + 'package_search': actions.package_search, + 'package_show': actions.package_show } # IConfigurer From daadd48606e23475be62f047d77447f8598f669f Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Thu, 29 Nov 2018 17:50:55 +0400 Subject: [PATCH 05/16] [tests][m]: fix broken tests - refs #12 - pass the context to get popular and recently viewed datasets - update test to pass user name to context - update layout1 to pass username to helper - new test case to cehck only approved datasets are listed --- ckanext/ed/helpers.py | 29 +++++++++++++++-------- ckanext/ed/templates/home/layout1.html | 4 ++-- ckanext/ed/tests/test_helpers.py | 32 ++++++++++++++++++++------ 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/ckanext/ed/helpers.py b/ckanext/ed/helpers.py index fefa17c..d895285 100644 --- a/ckanext/ed/helpers.py +++ b/ckanext/ed/helpers.py @@ -24,26 +24,31 @@ def get_groups(): return groups -def get_recently_updated_datasets(limit=5): +def get_recently_updated_datasets(limit=5, user=None): ''' Returns recent created or updated datasets. :param limit: Limit of the datasets to be returned. Default is 5. :type limit: integer + :param user: user name + :type user: string + :returns: a list of recently created or updated datasets :rtype: list ''' try: - pkg_search_results = toolkit.get_action('package_search')(data_dict={ - 'sort': 'metadata_modified desc', - 'rows': limit, - })['results'] + pkg_search_results = toolkit.get_action('package_search')( + context={'user': user}, + data_dict={ + 'sort': 'metadata_modified desc', + 'rows': limit + })['results'] except toolkit.ValidationError, search.SearchError: return [] else: pkgs = [] for pkg in pkg_search_results: - package = toolkit.get_action('package_show')( + package = toolkit.get_action('package_show')(context={'user': user}, data_dict={'id': pkg['id']}) modified = datetime.strptime( package['metadata_modified'].split('T')[0], '%Y-%m-%d') @@ -52,15 +57,20 @@ def get_recently_updated_datasets(limit=5): return pkgs -def get_most_popular_datasets(limit=5): +def get_most_popular_datasets(limit=5, user=None): ''' Returns most popular datasets based on total views. :param limit: Limit of the datasets to be returned. Default is 5. :type limit: integer + :param user: user name + :type user: string + :returns: a list of most popular datasets :rtype: list ''' - data = pkg_search_results = toolkit.get_action('package_search')(data_dict={ + data = pkg_search_results = toolkit.get_action('package_search')( + context={'user': user}, + data_dict={ 'sort': 'views_total desc', 'rows': limit, })['results'] @@ -117,5 +127,6 @@ def is_admin(user): :returns: True/False :rtype: boolean """ - user_orgs = _get_action('organization_list_for_user', {}, {'user': user}) + user_orgs = _get_action('organization_list_for_user', {'user': user}, {'user': user}) + assert False, user_orgs return any([i.get('capacity') == 'admin' for i in user_orgs]) diff --git a/ckanext/ed/templates/home/layout1.html b/ckanext/ed/templates/home/layout1.html index 72b1fa1..4a99322 100644 --- a/ckanext/ed/templates/home/layout1.html +++ b/ckanext/ed/templates/home/layout1.html @@ -38,7 +38,7 @@

{{ _('Categories') }}

- {% set recent_datasets = h.ed_get_recently_updated_datasets(limit=5) %} + {% set recent_datasets = h.ed_get_recently_updated_datasets(limit=5, user=c.user) %}

{{_('Recent Datasets')}}

{% if recent_datasets %}
    @@ -52,7 +52,7 @@

    {{_('Recent Datasets')}}

    {% endif %}
- {% set most_popular_datasets = h.ed_get_most_popular_datasets(limit=5) %} + {% set most_popular_datasets = h.ed_get_most_popular_datasets(limit=5, user=c.user) %}

{{_('Most Popular Datasets')}}

{% if most_popular_datasets %}
    diff --git a/ckanext/ed/tests/test_helpers.py b/ckanext/ed/tests/test_helpers.py index 40cccd4..6fab9e1 100644 --- a/ckanext/ed/tests/test_helpers.py +++ b/ckanext/ed/tests/test_helpers.py @@ -7,19 +7,37 @@ class TestHelpers(test_helpers.FunctionalTestBase): def test_get_recently_updated_datasets(self): - factories.Dataset() - factories.Dataset() - factories.Dataset() - dataset = factories.Dataset() + user = core_factories.User() + org = core_factories.Organization( + users=[{'name': user['name'], 'capacity': 'admin'}] + ) + factories.Dataset(owner_org=org['id']) + factories.Dataset(owner_org=org['id']) + factories.Dataset(owner_org=org['id']) + dataset = factories.Dataset(owner_org=org['id']) - result = helpers.get_recently_updated_datasets() - assert len(result) == 4 + result = helpers.get_recently_updated_datasets(user=user['name']) + assert len(result) == 4, 'Epextec 4 but got %s' % len(result) assert result[0]['id'] == dataset['id'] - result = helpers.get_recently_updated_datasets(limit=2) + result = helpers.get_recently_updated_datasets(limit=2, user=user['name']) assert len(result) == 2 assert result[0]['id'] == dataset['id'] + def test_get_recently_updated_datasets_lists_only_approved(self): + user = core_factories.User() + org = core_factories.Organization( + users=[{'name': user['name'], 'capacity': 'admin'}] + ) + factories.Dataset() + factories.Dataset() + factories.Dataset(owner_org=org['id']) + dataset = factories.Dataset(owner_org=org['id']) + + result = helpers.get_recently_updated_datasets(user=user['name']) + assert len(result) == 2, 'Epextec 2 but got %s' % len(result) + assert result[0]['id'] == dataset['id'] + def test_get_groups(self): group1 = core_factories.Group() From 8e63b693f6aed014af18b95eff80a6bffe098a21 Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Fri, 30 Nov 2018 09:44:04 +0400 Subject: [PATCH 06/16] [tests][s]: test get_admin --- ckanext/ed/helpers.py | 1 - ckanext/ed/tests/test_helpers.py | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/ckanext/ed/helpers.py b/ckanext/ed/helpers.py index d895285..75968e7 100644 --- a/ckanext/ed/helpers.py +++ b/ckanext/ed/helpers.py @@ -128,5 +128,4 @@ def is_admin(user): :rtype: boolean """ user_orgs = _get_action('organization_list_for_user', {'user': user}, {'user': user}) - assert False, user_orgs return any([i.get('capacity') == 'admin' for i in user_orgs]) diff --git a/ckanext/ed/tests/test_helpers.py b/ckanext/ed/tests/test_helpers.py index 6fab9e1..a4d2916 100644 --- a/ckanext/ed/tests/test_helpers.py +++ b/ckanext/ed/tests/test_helpers.py @@ -49,3 +49,24 @@ def test_get_groups(self): result = helpers.get_groups() assert result[0]['id'] == group1['id'] assert len(result) == 4 + + def test_is_admin(self): + core_factories.User(name='george') + core_factories.User(name='john') + core_factories.User(name='paul') + core_factories.Organization( + users=[ + {'name': 'george', 'capacity': 'admin'}, + {'name': 'john', 'capacity': 'editor'}, + {'name': 'paul', 'capacity': 'member'} + ] + ) + + result = helpers.is_admin('george') + assert result, '%s is not True' % result + result = helpers.is_admin('john') + assert not result, '%s is not False' % result + result = helpers.is_admin('paul') + assert not result, '%s is not False' % result + result = helpers.is_admin('ringo') + assert not result, '%s is not False' % result From 5e8417e43f7ba307224358e42672b24936a2531f Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Fri, 30 Nov 2018 10:31:21 +0400 Subject: [PATCH 07/16] [tests][s]: test validators --- ckanext/ed/tests/test_validators.py | 62 +++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 ckanext/ed/tests/test_validators.py diff --git a/ckanext/ed/tests/test_validators.py b/ckanext/ed/tests/test_validators.py new file mode 100644 index 0000000..683e155 --- /dev/null +++ b/ckanext/ed/tests/test_validators.py @@ -0,0 +1,62 @@ +from nose.tools import assert_equals + +from ckan import model +from ckan.tests import factories as core_factories +from ckan.tests.helpers import call_action, FunctionalTestBase + + +class TestValidators(FunctionalTestBase): + def test_dataset_by_sysadmin_and_admin_is_approved(self): + core_factories.User(name='george') + core_factories.Organization( + users=[{'name': 'george', 'capacity': 'admin'}], + name='us-ed-1', + id='us-ed-1' + ) + + sysadmin = core_factories.Sysadmin() + context = _create_context(sysadmin) + data_dict = _create_dataset_dict('test-dataset-1', 'us-ed-1') + call_action('package_create', context, **data_dict) + dataset = call_action('package_show', context, id='test-dataset-1') + assert_equals(dataset['approval_state'], 'approved') + + context = _create_context({'name': 'george'}) + data_dict = _create_dataset_dict('test-dataset-2', 'us-ed-1') + call_action('package_create', context, **data_dict) + dataset = call_action('package_show', context, id='test-dataset-2') + assert_equals(dataset['approval_state'], 'approved') + + + def test_dataset_by_editor_is_approval_pending(self): + core_factories.User(name='john') + core_factories.Organization( + users=[{'name': 'john', 'capacity': 'editor'}], + name='us-ed-2', + id='us-ed-2' + ) + + context = _create_context({'name': 'john'}) + data_dict = _create_dataset_dict('test-dataset', 'us-ed-2') + call_action('package_create', context, **data_dict) + dataset = call_action('package_show', context, id='test-dataset') + assert_equals(dataset['approval_state'], 'approval_pending') + + +def _create_context(user): + return {'model': model, 'user': user['name']} + + +def _create_dataset_dict(package_name, office_name='us-ed'): + return { + 'name': package_name, + 'contact_name': 'Stu Shepard', + 'program_code': '321', + 'access_level': 'public', + 'bureau_code': '123', + 'contact_email': '%s@email.com' % package_name, + 'notes': 'notes', + 'owner_org': office_name, + 'title': 'Title', + 'identifier': 'identifier' + } From 4ea13ffec79ca9d216b43ac725992a788752406e Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Fri, 30 Nov 2018 14:16:06 +0400 Subject: [PATCH 08/16] [tests][s]: test package_show --- ckanext/ed/actions.py | 2 +- ckanext/ed/tests/test_plugins.py | 177 +++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 ckanext/ed/tests/test_plugins.py diff --git a/ckanext/ed/actions.py b/ckanext/ed/actions.py index f2c044a..7e8f881 100644 --- a/ckanext/ed/actions.py +++ b/ckanext/ed/actions.py @@ -154,7 +154,7 @@ def package_show(context, data_dict): package = core_package_show(context, data_dict) # User with less perms then creator should not be able to access pending dataset approval_pending = package.get('approval_state') == 'approval_pending' - is_editor = check_access('package_create', context, data_dict) + is_editor = check_access('package_update', context, data_dict) if not is_editor and approval_pending: raise NotFound return package diff --git a/ckanext/ed/tests/test_plugins.py b/ckanext/ed/tests/test_plugins.py new file mode 100644 index 0000000..52462b4 --- /dev/null +++ b/ckanext/ed/tests/test_plugins.py @@ -0,0 +1,177 @@ +from nose.tools import assert_raises, assert_equals +from paste.registry import Registry +import mock +import pylons + +from ckan import model +from ckan.tests import factories as core_factories +from ckan.tests.helpers import call_action, call_auth, FunctionalTestBase +import ckan.plugins.toolkit as toolkit + +from ckanext.ed.tests import factories + + +class TestPlugins(FunctionalTestBase): + def test_dataset_appears_in_search_if_approved(self): + core_factories.User(name='george') + core_factories.User(name='john') + core_factories.User(name='paul') + core_factories.Organization( + users=[ + {'name': 'george', 'capacity': 'admin'}, + {'name': 'john', 'capacity': 'editor'}, + {'name': 'paul', 'capacity': 'reader'} + ], + name='us-ed-1', + id='us-ed-1' + ) + dataset = factories.Dataset(approval_state='approved', owner_org='us-ed-1') + + # Admin + context = _create_context({'name': 'george'}) + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + # Editor + context = _create_context({'name': 'john'}) + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + # Member + context = _create_context({'name': 'paul'}) + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + # Anonimous + context = _create_context({'name': 'ringo'}) + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + + def test_dataset_not_appears_in_search_if_not_approved(self): + core_factories.User(name='george') + core_factories.User(name='john') + core_factories.User(name='paul') + core_factories.Organization( + users=[ + {'name': 'george', 'capacity': 'admin'}, + {'name': 'john', 'capacity': 'editor'}, + {'name': 'paul', 'capacity': 'member'} + ], + name='us-ed-2', + id='us-ed-2' + ) + + # Dataset created by factories seem to use sysadmin so approval_state + # forced to be "approved". Creating packages this way to avoid that + context = _create_context({'name': 'john'}) + data_dict = _create_dataset_dict('test-dataset-1', 'us-ed-2') + call_action('package_create', context, **data_dict) + + context = _create_context({'name': 'john'}) + data_dict = _create_dataset_dict('test-dataset-2', 'us-ed-2') + call_action('package_create', context, **data_dict) + + # 2 datasets above "approveal_pending" and 1 below "approved" + context = _create_context({'name': 'george'}) + data_dict = _create_dataset_dict('test-dataset-3', 'us-ed-2') + call_action('package_create', context, **data_dict) + + # Admin + context = _create_context({'name': 'george'}) + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + # Editor + context = _create_context({'name': 'john'}) + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + # Member + context = _create_context({'name': 'paul'}) + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + # Anonimous + context = _create_context({'name': 'ringo'}) + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + + def test_package_show(self): + core_factories.User(name='george') + core_factories.User(name='john') + core_factories.User(name='paul') + core_factories.Organization( + users=[ + {'name': 'george', 'capacity': 'admin'}, + {'name': 'john', 'capacity': 'editor'}, + {'name': 'paul', 'capacity': 'reader'} + ], + name='us-ed-3', + id='us-ed-3' + ) + + pkg_1 = 'test-dataset-1' + pkg_2 = 'test-dataset-2' + + # Dataset created by factories seem to use sysadmin so approval_state + # forced to be "approved". Creating packages this way to avoid that + context = _create_context({'name': 'john'}) + data_dict = _create_dataset_dict(pkg_1, 'us-ed-3') + call_action('package_create', context, **data_dict) + + # 1 datasets above "approveal_pending" and 1 below "approved" + context = _create_context({'name': 'george'}) + data_dict = _create_dataset_dict(pkg_2, 'us-ed-3') + call_action('package_create', context, **data_dict) + + # Admin + context = _create_context({'name': 'george'}) + packages = call_action('package_show', context, **{'id': pkg_1}) + assert_equals(packages['name'], pkg_1) + packages = call_action('package_show', context, **{'id': pkg_2}) + assert_equals(packages['name'], pkg_2) + + # Editor + context = _create_context({'name': 'john'}) + packages = call_action('package_show', context, **{'id': pkg_1}) + assert_equals(packages['name'], pkg_1) + packages = call_action('package_show', context, **{'id': pkg_2}) + assert_equals(packages['name'], pkg_2) + + # 2 checks bellow not passing due to context issue + # # Member + # context = _create_context({'name': 'paul'}) + # packages = call_action('package_show', context, **{'id': pkg_1}) + # assert False, packages + # assert_equals(packages, {}) + # packages = call_action('package_show', context, **{'id': pkg_2}) + # assert_equals(packages['name'], pkg_2) + # + # # Anonimous + # context = _create_context({'name': 'ringo'}) + # packages = call_action('package_show', context, **{'id': pkg_1}) + # assert_equals(packages, {}) + # packages = call_action('package_show', context, **{'id': pkg_2}) + # assert_equals(packages['name'], pkg_2) + +# Helpers + +def _create_context(user): + return {'model': model, 'user': user['name']} + + +def _create_dataset_dict(package_name, office_name='us-ed'): + return { + 'name': package_name, + 'contact_name': 'Stu Shepard', + 'program_code': '321', + 'access_level': 'public', + 'bureau_code': '123', + 'contact_email': '%s@email.com' % package_name, + 'notes': 'notes', + 'owner_org': office_name, + 'title': 'Title', + 'identifier': 'identifier' + } From a7de217fc2254389683e70a35e567b2d6f6e35a4 Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Fri, 30 Nov 2018 14:30:40 +0400 Subject: [PATCH 09/16] [tidy][m]: tidy logic and code - get rid of approved - correct role for readers --- ckanext/ed/tests/test_helpers.py | 2 +- ckanext/ed/tests/test_validators.py | 6 +++--- ckanext/ed/validators.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ckanext/ed/tests/test_helpers.py b/ckanext/ed/tests/test_helpers.py index a4d2916..ba6069d 100644 --- a/ckanext/ed/tests/test_helpers.py +++ b/ckanext/ed/tests/test_helpers.py @@ -58,7 +58,7 @@ def test_is_admin(self): users=[ {'name': 'george', 'capacity': 'admin'}, {'name': 'john', 'capacity': 'editor'}, - {'name': 'paul', 'capacity': 'member'} + {'name': 'paul', 'capacity': 'reader'} ] ) diff --git a/ckanext/ed/tests/test_validators.py b/ckanext/ed/tests/test_validators.py index 683e155..25f51ab 100644 --- a/ckanext/ed/tests/test_validators.py +++ b/ckanext/ed/tests/test_validators.py @@ -6,7 +6,7 @@ class TestValidators(FunctionalTestBase): - def test_dataset_by_sysadmin_and_admin_is_approved(self): + def test_dataset_by_sysadmin_and_admin_is_not_approval_pending(self): core_factories.User(name='george') core_factories.Organization( users=[{'name': 'george', 'capacity': 'admin'}], @@ -19,13 +19,13 @@ def test_dataset_by_sysadmin_and_admin_is_approved(self): data_dict = _create_dataset_dict('test-dataset-1', 'us-ed-1') call_action('package_create', context, **data_dict) dataset = call_action('package_show', context, id='test-dataset-1') - assert_equals(dataset['approval_state'], 'approved') + assert_equals(dataset.get('approval_state'), None) context = _create_context({'name': 'george'}) data_dict = _create_dataset_dict('test-dataset-2', 'us-ed-1') call_action('package_create', context, **data_dict) dataset = call_action('package_show', context, id='test-dataset-2') - assert_equals(dataset['approval_state'], 'approved') + assert_equals(dataset.get('approval_state'), None) def test_dataset_by_editor_is_approval_pending(self): diff --git a/ckanext/ed/validators.py b/ckanext/ed/validators.py index 7968424..1d07e05 100644 --- a/ckanext/ed/validators.py +++ b/ckanext/ed/validators.py @@ -12,8 +12,8 @@ def state_validator(key, data, errors, context): user_orgs = get_core.organization_list_for_user(context, {'id': context['user']}) office_id = data.get(('owner_org',)) state = 'approval_pending' - # Approve if user is member of the org and admin + # user is member of the org and admin remove pending for org in user_orgs: if org.get('id') == office_id and org.get('capacity') == 'admin': - state = 'approved' + state = None data[key] = state From 5e1c335a9eafaf110aa7b711b34d4a7e52ea0f69 Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 30 Nov 2018 13:29:32 +0100 Subject: [PATCH 10/16] Make validator logic more robust --- ckanext/ed/validators.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/ckanext/ed/validators.py b/ckanext/ed/validators.py index 1d07e05..40fd034 100644 --- a/ckanext/ed/validators.py +++ b/ckanext/ed/validators.py @@ -1,19 +1,22 @@ -from logging import getLogger - -import ckan.logic.action.get as get_core - -from ckanext.ed import helpers - - -log = getLogger(__name__) +from ckan.plugins import toolkit def state_validator(key, data, errors, context): - user_orgs = get_core.organization_list_for_user(context, {'id': context['user']}) + user_orgs = toolkit.get_action('organization_list_for_user')( + context, {'id': context['user']}) office_id = data.get(('owner_org',)) - state = 'approval_pending' - # user is member of the org and admin remove pending + + state = data.pop(key, None) + + # If the user is member of the organization but not an admin, force the + # state to be pending for org in user_orgs: - if org.get('id') == office_id and org.get('capacity') == 'admin': - state = None + if org.get('id') == office_id: + if org.get('capacity') == 'admin': + # If no state provided and user is an admin, default to active + state = state or 'active' + else: + # If not admin, create as pending + state = 'approval_pending' + data[key] = state From 1460cb63336b80f0416667447c0e637b8dab1fd6 Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 30 Nov 2018 13:30:19 +0100 Subject: [PATCH 11/16] Fix auth check in package_show --- ckanext/ed/actions.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/ckanext/ed/actions.py b/ckanext/ed/actions.py index 7e8f881..f7a2766 100644 --- a/ckanext/ed/actions.py +++ b/ckanext/ed/actions.py @@ -5,7 +5,6 @@ import zipfile from ckan.controllers.admin import get_sysadmins -from ckan.logic import check_access, NotFound from ckan.logic.action.get import package_search as core_package_search from ckan.logic.action.get import package_show as core_package_show from ckan.plugins import toolkit @@ -154,7 +153,11 @@ def package_show(context, data_dict): package = core_package_show(context, data_dict) # User with less perms then creator should not be able to access pending dataset approval_pending = package.get('approval_state') == 'approval_pending' - is_editor = check_access('package_update', context, data_dict) - if not is_editor and approval_pending: - raise NotFound + try: + toolkit.check_access('package_update', context, data_dict) + can_edit = True + except toolkit.NotAuthorized: + can_edit = False + if not can_edit and approval_pending: + raise toolkit.ObjectNotFound return package From 1ac9211057f0226452c07817d71c38efea2acbad Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 30 Nov 2018 13:30:41 +0100 Subject: [PATCH 12/16] Fix tests call_action sets ignore_auth=True by default, which basically means that everybody can `package_update`, so the the check on `package_show` doesn't work. Also simplified the tests when possible. --- ckanext/ed/tests/test_plugins.py | 74 ++++++++++++++------------------ 1 file changed, 32 insertions(+), 42 deletions(-) diff --git a/ckanext/ed/tests/test_plugins.py b/ckanext/ed/tests/test_plugins.py index 52462b4..42ecb46 100644 --- a/ckanext/ed/tests/test_plugins.py +++ b/ckanext/ed/tests/test_plugins.py @@ -1,11 +1,7 @@ from nose.tools import assert_raises, assert_equals -from paste.registry import Registry -import mock -import pylons -from ckan import model from ckan.tests import factories as core_factories -from ckan.tests.helpers import call_action, call_auth, FunctionalTestBase +from ckan.tests.helpers import call_action, FunctionalTestBase import ckan.plugins.toolkit as toolkit from ckanext.ed.tests import factories @@ -20,34 +16,33 @@ def test_dataset_appears_in_search_if_approved(self): users=[ {'name': 'george', 'capacity': 'admin'}, {'name': 'john', 'capacity': 'editor'}, - {'name': 'paul', 'capacity': 'reader'} + {'name': 'paul', 'capacity': 'member'} ], name='us-ed-1', id='us-ed-1' ) - dataset = factories.Dataset(approval_state='approved', owner_org='us-ed-1') + factories.Dataset(approval_state='approved', owner_org='us-ed-1') # Admin - context = _create_context({'name': 'george'}) + context = {'user': 'george'} packages = call_action('package_search', context, **{}) assert_equals(packages['count'], 1) # Editor - context = _create_context({'name': 'john'}) + context = {'user': 'john'} packages = call_action('package_search', context, **{}) assert_equals(packages['count'], 1) # Member - context = _create_context({'name': 'paul'}) + context = {'user': 'paul'} packages = call_action('package_search', context, **{}) assert_equals(packages['count'], 1) # Anonimous - context = _create_context({'name': 'ringo'}) + context = {'user': 'ringo'} packages = call_action('package_search', context, **{}) assert_equals(packages['count'], 1) - def test_dataset_not_appears_in_search_if_not_approved(self): core_factories.User(name='george') core_factories.User(name='john') @@ -64,40 +59,39 @@ def test_dataset_not_appears_in_search_if_not_approved(self): # Dataset created by factories seem to use sysadmin so approval_state # forced to be "approved". Creating packages this way to avoid that - context = _create_context({'name': 'john'}) + context = {'user': 'john'} data_dict = _create_dataset_dict('test-dataset-1', 'us-ed-2') call_action('package_create', context, **data_dict) - context = _create_context({'name': 'john'}) + context = {'user': 'john'} data_dict = _create_dataset_dict('test-dataset-2', 'us-ed-2') call_action('package_create', context, **data_dict) # 2 datasets above "approveal_pending" and 1 below "approved" - context = _create_context({'name': 'george'}) + context = {'user': 'george'} data_dict = _create_dataset_dict('test-dataset-3', 'us-ed-2') call_action('package_create', context, **data_dict) # Admin - context = _create_context({'name': 'george'}) + context = {'user': 'george'} packages = call_action('package_search', context, **{}) assert_equals(packages['count'], 1) # Editor - context = _create_context({'name': 'john'}) + context = {'user': 'john'} packages = call_action('package_search', context, **{}) assert_equals(packages['count'], 1) # Member - context = _create_context({'name': 'paul'}) + context = {'user': 'paul'} packages = call_action('package_search', context, **{}) assert_equals(packages['count'], 1) # Anonimous - context = _create_context({'name': 'ringo'}) + context = {'user': 'ringo'} packages = call_action('package_search', context, **{}) assert_equals(packages['count'], 1) - def test_package_show(self): core_factories.User(name='george') core_factories.User(name='john') @@ -106,7 +100,7 @@ def test_package_show(self): users=[ {'name': 'george', 'capacity': 'admin'}, {'name': 'john', 'capacity': 'editor'}, - {'name': 'paul', 'capacity': 'reader'} + {'name': 'paul', 'capacity': 'member'} ], name='us-ed-3', id='us-ed-3' @@ -117,49 +111,45 @@ def test_package_show(self): # Dataset created by factories seem to use sysadmin so approval_state # forced to be "approved". Creating packages this way to avoid that - context = _create_context({'name': 'john'}) + context = {'user': 'john'} data_dict = _create_dataset_dict(pkg_1, 'us-ed-3') call_action('package_create', context, **data_dict) # 1 datasets above "approveal_pending" and 1 below "approved" - context = _create_context({'name': 'george'}) + context = {'user': 'george'} data_dict = _create_dataset_dict(pkg_2, 'us-ed-3') call_action('package_create', context, **data_dict) # Admin - context = _create_context({'name': 'george'}) + context = {'user': 'george'} packages = call_action('package_show', context, **{'id': pkg_1}) assert_equals(packages['name'], pkg_1) packages = call_action('package_show', context, **{'id': pkg_2}) assert_equals(packages['name'], pkg_2) # Editor - context = _create_context({'name': 'john'}) + context = {'user': 'john'} packages = call_action('package_show', context, **{'id': pkg_1}) assert_equals(packages['name'], pkg_1) packages = call_action('package_show', context, **{'id': pkg_2}) assert_equals(packages['name'], pkg_2) # 2 checks bellow not passing due to context issue - # # Member - # context = _create_context({'name': 'paul'}) - # packages = call_action('package_show', context, **{'id': pkg_1}) - # assert False, packages - # assert_equals(packages, {}) - # packages = call_action('package_show', context, **{'id': pkg_2}) - # assert_equals(packages['name'], pkg_2) - # - # # Anonimous - # context = _create_context({'name': 'ringo'}) - # packages = call_action('package_show', context, **{'id': pkg_1}) - # assert_equals(packages, {}) - # packages = call_action('package_show', context, **{'id': pkg_2}) - # assert_equals(packages['name'], pkg_2) + # Member + context = {'user': 'paul', 'ignore_auth': False} + assert_raises(toolkit.ObjectNotFound, + call_action, 'package_show', context, id=pkg_1) + packages = call_action('package_show', context, **{'id': pkg_2}) + assert_equals(packages['name'], pkg_2) -# Helpers + # Anonymous + context = {'user': 'ringo', 'ignore_auth': False} + assert_raises(toolkit.ObjectNotFound, + call_action, 'package_show', context, id=pkg_1) + packages = call_action('package_show', context, **{'id': pkg_2}) + assert_equals(packages['name'], pkg_2) -def _create_context(user): - return {'model': model, 'user': user['name']} +# Helpers def _create_dataset_dict(package_name, office_name='us-ed'): From 3f8d2250271cecda0f97e40c5238369e7d7073c0 Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Fri, 30 Nov 2018 17:35:20 +0400 Subject: [PATCH 13/16] [tests][m]: tests for each role --- ckanext/ed/tests/test_plugins.py | 167 ---------------------------- ckanext/ed/tests/test_validators.py | 4 +- ckanext/ed/tests/test_workflow.py | 114 +++++++++++++++++++ 3 files changed, 116 insertions(+), 169 deletions(-) delete mode 100644 ckanext/ed/tests/test_plugins.py create mode 100644 ckanext/ed/tests/test_workflow.py diff --git a/ckanext/ed/tests/test_plugins.py b/ckanext/ed/tests/test_plugins.py deleted file mode 100644 index 42ecb46..0000000 --- a/ckanext/ed/tests/test_plugins.py +++ /dev/null @@ -1,167 +0,0 @@ -from nose.tools import assert_raises, assert_equals - -from ckan.tests import factories as core_factories -from ckan.tests.helpers import call_action, FunctionalTestBase -import ckan.plugins.toolkit as toolkit - -from ckanext.ed.tests import factories - - -class TestPlugins(FunctionalTestBase): - def test_dataset_appears_in_search_if_approved(self): - core_factories.User(name='george') - core_factories.User(name='john') - core_factories.User(name='paul') - core_factories.Organization( - users=[ - {'name': 'george', 'capacity': 'admin'}, - {'name': 'john', 'capacity': 'editor'}, - {'name': 'paul', 'capacity': 'member'} - ], - name='us-ed-1', - id='us-ed-1' - ) - factories.Dataset(approval_state='approved', owner_org='us-ed-1') - - # Admin - context = {'user': 'george'} - packages = call_action('package_search', context, **{}) - assert_equals(packages['count'], 1) - - # Editor - context = {'user': 'john'} - packages = call_action('package_search', context, **{}) - assert_equals(packages['count'], 1) - - # Member - context = {'user': 'paul'} - packages = call_action('package_search', context, **{}) - assert_equals(packages['count'], 1) - - # Anonimous - context = {'user': 'ringo'} - packages = call_action('package_search', context, **{}) - assert_equals(packages['count'], 1) - - def test_dataset_not_appears_in_search_if_not_approved(self): - core_factories.User(name='george') - core_factories.User(name='john') - core_factories.User(name='paul') - core_factories.Organization( - users=[ - {'name': 'george', 'capacity': 'admin'}, - {'name': 'john', 'capacity': 'editor'}, - {'name': 'paul', 'capacity': 'member'} - ], - name='us-ed-2', - id='us-ed-2' - ) - - # Dataset created by factories seem to use sysadmin so approval_state - # forced to be "approved". Creating packages this way to avoid that - context = {'user': 'john'} - data_dict = _create_dataset_dict('test-dataset-1', 'us-ed-2') - call_action('package_create', context, **data_dict) - - context = {'user': 'john'} - data_dict = _create_dataset_dict('test-dataset-2', 'us-ed-2') - call_action('package_create', context, **data_dict) - - # 2 datasets above "approveal_pending" and 1 below "approved" - context = {'user': 'george'} - data_dict = _create_dataset_dict('test-dataset-3', 'us-ed-2') - call_action('package_create', context, **data_dict) - - # Admin - context = {'user': 'george'} - packages = call_action('package_search', context, **{}) - assert_equals(packages['count'], 1) - - # Editor - context = {'user': 'john'} - packages = call_action('package_search', context, **{}) - assert_equals(packages['count'], 1) - - # Member - context = {'user': 'paul'} - packages = call_action('package_search', context, **{}) - assert_equals(packages['count'], 1) - - # Anonimous - context = {'user': 'ringo'} - packages = call_action('package_search', context, **{}) - assert_equals(packages['count'], 1) - - def test_package_show(self): - core_factories.User(name='george') - core_factories.User(name='john') - core_factories.User(name='paul') - core_factories.Organization( - users=[ - {'name': 'george', 'capacity': 'admin'}, - {'name': 'john', 'capacity': 'editor'}, - {'name': 'paul', 'capacity': 'member'} - ], - name='us-ed-3', - id='us-ed-3' - ) - - pkg_1 = 'test-dataset-1' - pkg_2 = 'test-dataset-2' - - # Dataset created by factories seem to use sysadmin so approval_state - # forced to be "approved". Creating packages this way to avoid that - context = {'user': 'john'} - data_dict = _create_dataset_dict(pkg_1, 'us-ed-3') - call_action('package_create', context, **data_dict) - - # 1 datasets above "approveal_pending" and 1 below "approved" - context = {'user': 'george'} - data_dict = _create_dataset_dict(pkg_2, 'us-ed-3') - call_action('package_create', context, **data_dict) - - # Admin - context = {'user': 'george'} - packages = call_action('package_show', context, **{'id': pkg_1}) - assert_equals(packages['name'], pkg_1) - packages = call_action('package_show', context, **{'id': pkg_2}) - assert_equals(packages['name'], pkg_2) - - # Editor - context = {'user': 'john'} - packages = call_action('package_show', context, **{'id': pkg_1}) - assert_equals(packages['name'], pkg_1) - packages = call_action('package_show', context, **{'id': pkg_2}) - assert_equals(packages['name'], pkg_2) - - # 2 checks bellow not passing due to context issue - # Member - context = {'user': 'paul', 'ignore_auth': False} - assert_raises(toolkit.ObjectNotFound, - call_action, 'package_show', context, id=pkg_1) - packages = call_action('package_show', context, **{'id': pkg_2}) - assert_equals(packages['name'], pkg_2) - - # Anonymous - context = {'user': 'ringo', 'ignore_auth': False} - assert_raises(toolkit.ObjectNotFound, - call_action, 'package_show', context, id=pkg_1) - packages = call_action('package_show', context, **{'id': pkg_2}) - assert_equals(packages['name'], pkg_2) - -# Helpers - - -def _create_dataset_dict(package_name, office_name='us-ed'): - return { - 'name': package_name, - 'contact_name': 'Stu Shepard', - 'program_code': '321', - 'access_level': 'public', - 'bureau_code': '123', - 'contact_email': '%s@email.com' % package_name, - 'notes': 'notes', - 'owner_org': office_name, - 'title': 'Title', - 'identifier': 'identifier' - } diff --git a/ckanext/ed/tests/test_validators.py b/ckanext/ed/tests/test_validators.py index 25f51ab..70efc7e 100644 --- a/ckanext/ed/tests/test_validators.py +++ b/ckanext/ed/tests/test_validators.py @@ -19,13 +19,13 @@ def test_dataset_by_sysadmin_and_admin_is_not_approval_pending(self): data_dict = _create_dataset_dict('test-dataset-1', 'us-ed-1') call_action('package_create', context, **data_dict) dataset = call_action('package_show', context, id='test-dataset-1') - assert_equals(dataset.get('approval_state'), None) + assert_equals(dataset.get('approval_state'), 'active') context = _create_context({'name': 'george'}) data_dict = _create_dataset_dict('test-dataset-2', 'us-ed-1') call_action('package_create', context, **data_dict) dataset = call_action('package_show', context, id='test-dataset-2') - assert_equals(dataset.get('approval_state'), None) + assert_equals(dataset.get('approval_state'), 'active') def test_dataset_by_editor_is_approval_pending(self): diff --git a/ckanext/ed/tests/test_workflow.py b/ckanext/ed/tests/test_workflow.py new file mode 100644 index 0000000..cdb5d34 --- /dev/null +++ b/ckanext/ed/tests/test_workflow.py @@ -0,0 +1,114 @@ +from nose.tools import assert_raises, assert_equals + +from ckan.lib.search import rebuild +from ckan.tests import factories as core_factories +from ckan.tests import helpers as test_helpers +from ckan.tests.helpers import call_action, FunctionalTestBase +import ckan.plugins.toolkit as toolkit + +from ckanext.ed.tests import factories + + +class TestWorFlows(FunctionalTestBase): + # For some reasons @classmethod does not work + def setup(self): + self.pkg_1 = 'test-dataset-1' + self.pkg_2 = 'test-dataset-2' + test_helpers.reset_db() + rebuild() + core_factories.User(name='george') + core_factories.User(name='john') + core_factories.User(name='paul') + core_factories.Organization( + users=[ + {'name': 'george', 'capacity': 'admin'}, + {'name': 'john', 'capacity': 'editor'}, + {'name': 'paul', 'capacity': 'reader'} + ], + name='us-ed-1', + id='us-ed-1' + ) + # Dataset created by factories seem to use sysadmin so approval_state + # forced to be "approved". Creating packages this way to avoid that + context = {'user': 'john'} + data_dict = _create_dataset_dict(self.pkg_1, 'us-ed-1') + call_action('package_create', context, **data_dict) + + # 1 datasets above "approveal_pending" and 1 below "approved" + context = {'user': 'george'} + data_dict = _create_dataset_dict(self.pkg_2, 'us-ed-1') + call_action('package_create', context, **data_dict) + + + @classmethod + def tearDownClass(self): + pass + + + def test_dataset_not_appears_in_search_if_not_approved_admin(self): + context = {'user': 'george'} + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + def test_dataset_not_appears_in_search_if_not_approved_editor(self): + context = {'user': 'john'} + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + def test_dataset_not_appears_in_search_if_not_approved_reder(self): + context = {'user': 'paul'} + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + def test_dataset_not_appears_in_search_if_not_approved_aninimous(self): + context = {'user': 'ringo'} + packages = call_action('package_search', context, **{}) + assert_equals(packages['count'], 1) + + + + def test_package_show_admin_can_see_both(self): + context = {'user': 'george'} + packages = call_action('package_show', context, **{'id': self.pkg_1}) + assert_equals(packages['name'], self.pkg_1) + packages = call_action('package_show', context, **{'id': self.pkg_2}) + assert_equals(packages['name'], self.pkg_2) + + def test_package_show_editor_can_see_both(self): + context = {'user': 'john'} + packages = call_action('package_show', context, **{'id': self.pkg_1}) + assert_equals(packages['name'], self.pkg_1) + packages = call_action('package_show', context, **{'id': self.pkg_2}) + assert_equals(packages['name'], self.pkg_2) + + def test_package_show_member_can_not_see_pending(self): + context = {'user': 'paul', 'ignore_auth': False} + assert_raises(toolkit.ObjectNotFound, + call_action, 'package_show', context, id=self.pkg_1) + packages = call_action('package_show', context, **{'id': self.pkg_2}) + assert_equals(packages['name'], self.pkg_2) + + def test_package_show_anonimous_can_not_see_pending(self): + context = {'user': 'ringo', 'ignore_auth': False} + assert_raises(toolkit.ObjectNotFound, + call_action, 'package_show', context, id=self.pkg_1) + packages = call_action('package_show', context, **{'id': self.pkg_2}) + assert_equals(packages['name'], self.pkg_2) + + +# Helpers + + +def _create_dataset_dict(package_name, office_name='us-ed'): + return { + 'name': package_name, + 'contact_name': 'Stu Shepard', + 'program_code': '321', + 'access_level': 'public', + 'bureau_code': '123', + 'contact_email': '%s@email.com' % package_name, + 'notes': 'notes', + 'owner_org': office_name, + 'title': 'Title', + 'identifier': 'identifier' + } From 5d296b00a80d52a2dedec5bf7ae3ea999c192e46 Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Mon, 3 Dec 2018 09:57:07 +0400 Subject: [PATCH 14/16] [tests][s]: state pending for updated packages --- ckanext/ed/tests/test_helpers.py | 4 ++-- ckanext/ed/tests/test_workflow.py | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/ckanext/ed/tests/test_helpers.py b/ckanext/ed/tests/test_helpers.py index ba6069d..1525ddb 100644 --- a/ckanext/ed/tests/test_helpers.py +++ b/ckanext/ed/tests/test_helpers.py @@ -29,8 +29,8 @@ def test_get_recently_updated_datasets_lists_only_approved(self): org = core_factories.Organization( users=[{'name': user['name'], 'capacity': 'admin'}] ) - factories.Dataset() - factories.Dataset() + factories.Dataset(approval_state='approval_pending') + factories.Dataset(approval_state='approval_pending') factories.Dataset(owner_org=org['id']) dataset = factories.Dataset(owner_org=org['id']) diff --git a/ckanext/ed/tests/test_workflow.py b/ckanext/ed/tests/test_workflow.py index cdb5d34..d187d01 100644 --- a/ckanext/ed/tests/test_workflow.py +++ b/ckanext/ed/tests/test_workflow.py @@ -65,8 +65,6 @@ def test_dataset_not_appears_in_search_if_not_approved_aninimous(self): packages = call_action('package_search', context, **{}) assert_equals(packages['count'], 1) - - def test_package_show_admin_can_see_both(self): context = {'user': 'george'} packages = call_action('package_show', context, **{'id': self.pkg_1}) From 8edd1adc04553d3223f3721d1ef9753b15429614 Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Tue, 4 Dec 2018 09:25:19 +0400 Subject: [PATCH 15/16] [search][s]: use IPackageController.before_search --- ckanext/ed/actions.py | 8 -------- ckanext/ed/plugin.py | 9 ++++++++- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/ckanext/ed/actions.py b/ckanext/ed/actions.py index f7a2766..4cb3964 100644 --- a/ckanext/ed/actions.py +++ b/ckanext/ed/actions.py @@ -5,7 +5,6 @@ import zipfile from ckan.controllers.admin import get_sysadmins -from ckan.logic.action.get import package_search as core_package_search from ckan.logic.action.get import package_show as core_package_show from ckan.plugins import toolkit @@ -141,13 +140,6 @@ def prepare_zip_resources(context, data_dict): return {'zip_id': None} -@toolkit.side_effect_free -def package_search(context, data_dict): - data_dict['fq'] = '!(approval_state:approval_pending) ' + data_dict.get('fq', '') - packages = core_package_search(context, data_dict) - return packages - - @toolkit.side_effect_free def package_show(context, data_dict): package = core_package_show(context, data_dict) diff --git a/ckanext/ed/plugin.py b/ckanext/ed/plugin.py index d9512c4..d5c8ba1 100644 --- a/ckanext/ed/plugin.py +++ b/ckanext/ed/plugin.py @@ -12,6 +12,7 @@ class EDPlugin(plugins.SingletonPlugin, DefaultTranslation): plugins.implements(plugins.IActions) plugins.implements(plugins.IRoutes, inherit=True) plugins.implements(plugins.IValidators) + plugins.implements(plugins.IPackageController, inherit=True) # ITemplateHelpers def get_helpers(self): @@ -27,10 +28,16 @@ def get_helpers(self): def get_actions(self): return { 'ed_prepare_zip_resources': actions.prepare_zip_resources, - 'package_search': actions.package_search, 'package_show': actions.package_show } + # IPackageController + def before_search(self, search_params): + search_params.update({ + 'fq': '!(approval_state:approval_pending) ' + search_params.get('fq', '') + }) + return search_params + # IConfigurer def update_config(self, config_): toolkit.add_template_directory(config_, 'templates') From 9f0d9999978b1e4d5a454ed1f83da6cd40eecce9 Mon Sep 17 00:00:00 2001 From: Irakli Mchedlishvili Date: Tue, 4 Dec 2018 09:46:22 +0400 Subject: [PATCH 16/16] [updated datasets][s]: get rid of package_show --- ckanext/ed/helpers.py | 21 ++++++--------------- ckanext/ed/templates/home/layout1.html | 4 ++-- ckanext/ed/tests/test_helpers.py | 10 +++++----- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/ckanext/ed/helpers.py b/ckanext/ed/helpers.py index 75968e7..d6bffdb 100644 --- a/ckanext/ed/helpers.py +++ b/ckanext/ed/helpers.py @@ -24,7 +24,7 @@ def get_groups(): return groups -def get_recently_updated_datasets(limit=5, user=None): +def get_recently_updated_datasets(limit=5): ''' Returns recent created or updated datasets. :param limit: Limit of the datasets to be returned. Default is 5. @@ -37,27 +37,19 @@ def get_recently_updated_datasets(limit=5, user=None): ''' try: pkg_search_results = toolkit.get_action('package_search')( - context={'user': user}, data_dict={ 'sort': 'metadata_modified desc', 'rows': limit })['results'] + return pkg_search_results except toolkit.ValidationError, search.SearchError: return [] else: - pkgs = [] - for pkg in pkg_search_results: - package = toolkit.get_action('package_show')(context={'user': user}, - data_dict={'id': pkg['id']}) - modified = datetime.strptime( - package['metadata_modified'].split('T')[0], '%Y-%m-%d') - package['days_ago_modified'] = ((datetime.now() - modified).days) - pkgs.append(package) - return pkgs - - -def get_most_popular_datasets(limit=5, user=None): + log.warning('Unexpected Error occured while searching') + return [] + +def get_most_popular_datasets(limit=5): ''' Returns most popular datasets based on total views. :param limit: Limit of the datasets to be returned. Default is 5. @@ -69,7 +61,6 @@ def get_most_popular_datasets(limit=5, user=None): :rtype: list ''' data = pkg_search_results = toolkit.get_action('package_search')( - context={'user': user}, data_dict={ 'sort': 'views_total desc', 'rows': limit, diff --git a/ckanext/ed/templates/home/layout1.html b/ckanext/ed/templates/home/layout1.html index 4a99322..72b1fa1 100644 --- a/ckanext/ed/templates/home/layout1.html +++ b/ckanext/ed/templates/home/layout1.html @@ -38,7 +38,7 @@

    {{ _('Categories') }}

    - {% set recent_datasets = h.ed_get_recently_updated_datasets(limit=5, user=c.user) %} + {% set recent_datasets = h.ed_get_recently_updated_datasets(limit=5) %}

    {{_('Recent Datasets')}}

    {% if recent_datasets %}
      @@ -52,7 +52,7 @@

      {{_('Recent Datasets')}}

      {% endif %}
    - {% set most_popular_datasets = h.ed_get_most_popular_datasets(limit=5, user=c.user) %} + {% set most_popular_datasets = h.ed_get_most_popular_datasets(limit=5) %}

    {{_('Most Popular Datasets')}}

    {% if most_popular_datasets %}
      diff --git a/ckanext/ed/tests/test_helpers.py b/ckanext/ed/tests/test_helpers.py index 1525ddb..fb826d8 100644 --- a/ckanext/ed/tests/test_helpers.py +++ b/ckanext/ed/tests/test_helpers.py @@ -16,11 +16,11 @@ def test_get_recently_updated_datasets(self): factories.Dataset(owner_org=org['id']) dataset = factories.Dataset(owner_org=org['id']) - result = helpers.get_recently_updated_datasets(user=user['name']) + result = helpers.get_recently_updated_datasets() assert len(result) == 4, 'Epextec 4 but got %s' % len(result) assert result[0]['id'] == dataset['id'] - result = helpers.get_recently_updated_datasets(limit=2, user=user['name']) + result = helpers.get_recently_updated_datasets(limit=2) assert len(result) == 2 assert result[0]['id'] == dataset['id'] @@ -29,12 +29,12 @@ def test_get_recently_updated_datasets_lists_only_approved(self): org = core_factories.Organization( users=[{'name': user['name'], 'capacity': 'admin'}] ) - factories.Dataset(approval_state='approval_pending') - factories.Dataset(approval_state='approval_pending') + factories.Dataset(owner_org=org['id'], approval_state='approval_pending') + factories.Dataset(owner_org=org['id'], approval_state='approval_pending') factories.Dataset(owner_org=org['id']) dataset = factories.Dataset(owner_org=org['id']) - result = helpers.get_recently_updated_datasets(user=user['name']) + result = helpers.get_recently_updated_datasets() assert len(result) == 2, 'Epextec 2 but got %s' % len(result) assert result[0]['id'] == dataset['id']