From b49437ec5f829a832afdae5f90dc9194fdf3b8d3 Mon Sep 17 00:00:00 2001
From: Irakli Mchedlishvili <irakli.mchedlishvili@datopian.com>
Date: Wed, 28 Nov 2018 13:24:31 +0400
Subject: [PATCH 01/18] [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 <irakli.mchedlishvili@datopian.com>
Date: Wed, 28 Nov 2018 13:26:08 +0400
Subject: [PATCH 02/18] [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) %}
+      <div class="alert alert-warning" role="alert">
+        <p>
+          {% trans %}<strong>Note:</strong> This dataset will be submited for an administrator approval!{% endtrans %}
+        </p>
+      </div>
+      {% 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 %}
+
+<ol class="stages {{ class }}">
+  <li class="first {{ s1 }}">
+    {% if s1 != 'complete' %}
+      {% if h.ed_is_admin(c.user) %}
+        <span class="highlight">{{ _('Create dataset') }}</span>
+      {% else %}
+        <span class="highlight">{{ _('Request dataset') }}</span>
+      {% endif %}
+    {% else %}
+      {% if h.ed_is_admin(c.user) %}
+        <button class="highlight" name="save" value="go-dataset" type="submit">{{ _('Create dataset') }}</button>
+      {% else %}
+        <button class="highlight" name="save" value="go-dataset" type="submit">{{ _('Request dataset') }}</button>
+      {% endif %}
+    {% endif %}
+  </li>
+  <li class="last {{ s2 }}">
+    {% if s2 != 'complete' %}
+      <span class="highlight">{{ _('Add data') }}</span>
+    {% else %}
+      {% if s1 == 'active' %}
+        {# stage 1 #}
+        <button class="highlight" name="save" value="go-resources" type="submit">{{ _('Add data') }}</button>
+      {% else %}
+        {% link_for _('Add data'), named_route='dataset.new', class_="highlight" %}
+      {% endif %}
+    {% endif %}
+  </li>
+</ol>

From 75395d5af7ef62ea4344b35e435ffabdf2ee8827 Mon Sep 17 00:00:00 2001
From: Irakli Mchedlishvili <irakli.mchedlishvili@datopian.com>
Date: Wed, 28 Nov 2018 15:57:57 +0400
Subject: [PATCH 03/18] [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 <irakli.mchedlishvili@datopian.com>
Date: Thu, 29 Nov 2018 10:04:51 +0400
Subject: [PATCH 04/18] [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 <irakli.mchedlishvili@datopian.com>
Date: Thu, 29 Nov 2018 17:50:55 +0400
Subject: [PATCH 05/18] [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 @@ <h2 class="title">{{ _('Categories') }}</h2>
   <div class="container">
     <div class="row">
       <div class="module-datasets col-lg-6 col-md-12 col-sm-12 col-xs-12">
-        {% 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) %}
         <h3 class="title"><i class="fa fa-clock-o"></i> {{_('Recent Datasets')}}</h3>
           {% if recent_datasets %}
           <ul class="module-recent list-unstyled">
@@ -52,7 +52,7 @@ <h3 class="title"><i class="fa fa-clock-o"></i> {{_('Recent Datasets')}}</h3>
           {% endif %}
       </div>
       <div class="module-datasets col-lg-6 col-md-12 col-sm-12 col-xs-12">
-        {% 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) %}
         <h3 class="title"><i class="fa fa-line-chart"></i> {{_('Most Popular Datasets')}}</h3>
           {% if most_popular_datasets %}
           <ul class="module-recent list-unstyled">
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 <irakli.mchedlishvili@datopian.com>
Date: Fri, 30 Nov 2018 09:44:04 +0400
Subject: [PATCH 06/18] [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 <irakli.mchedlishvili@datopian.com>
Date: Fri, 30 Nov 2018 10:31:21 +0400
Subject: [PATCH 07/18] [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 <irakli.mchedlishvili@datopian.com>
Date: Fri, 30 Nov 2018 14:16:06 +0400
Subject: [PATCH 08/18] [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 <irakli.mchedlishvili@datopian.com>
Date: Fri, 30 Nov 2018 14:30:40 +0400
Subject: [PATCH 09/18] [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 <amercadero@gmail.com>
Date: Fri, 30 Nov 2018 13:29:32 +0100
Subject: [PATCH 10/18] 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 <amercadero@gmail.com>
Date: Fri, 30 Nov 2018 13:30:19 +0100
Subject: [PATCH 11/18] 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 <amercadero@gmail.com>
Date: Fri, 30 Nov 2018 13:30:41 +0100
Subject: [PATCH 12/18] 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 <irakli.mchedlishvili@datopian.com>
Date: Fri, 30 Nov 2018 17:35:20 +0400
Subject: [PATCH 13/18] [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 <irakli.mchedlishvili@datopian.com>
Date: Mon, 3 Dec 2018 09:57:07 +0400
Subject: [PATCH 14/18] [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 <irakli.mchedlishvili@datopian.com>
Date: Tue, 4 Dec 2018 09:25:19 +0400
Subject: [PATCH 15/18] [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 <irakli.mchedlishvili@datopian.com>
Date: Tue, 4 Dec 2018 09:46:22 +0400
Subject: [PATCH 16/18] [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 @@ <h2 class="title">{{ _('Categories') }}</h2>
   <div class="container">
     <div class="row">
       <div class="module-datasets col-lg-6 col-md-12 col-sm-12 col-xs-12">
-        {% 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) %}
         <h3 class="title"><i class="fa fa-clock-o"></i> {{_('Recent Datasets')}}</h3>
           {% if recent_datasets %}
           <ul class="module-recent list-unstyled">
@@ -52,7 +52,7 @@ <h3 class="title"><i class="fa fa-clock-o"></i> {{_('Recent Datasets')}}</h3>
           {% endif %}
       </div>
       <div class="module-datasets col-lg-6 col-md-12 col-sm-12 col-xs-12">
-        {% 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) %}
         <h3 class="title"><i class="fa fa-line-chart"></i> {{_('Most Popular Datasets')}}</h3>
           {% if most_popular_datasets %}
           <ul class="module-recent list-unstyled">
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']
 

From e38408427264ebbeb8c441ee9dbb2c76faae6870 Mon Sep 17 00:00:00 2001
From: Irakli Mchedlishvili <irakli.mchedlishvili@datopian.com>
Date: Tue, 4 Dec 2018 10:00:10 +0400
Subject: [PATCH 17/18] [emails][m]: email admins re pending pkgs refs #13

- override package_create to send email if approval_pending
- Email template including info about package, user and admin
- tests
---
 ckanext/ed/actions.py                         | 16 ++++++
 ckanext/ed/mailer.py                          | 42 +++++++++++++++
 ckanext/ed/plugin.py                          |  1 +
 .../emails/package_publish_request.txt        | 18 +++++++
 ckanext/ed/tests/test_mailer.py               | 51 +++++++++++++++++++
 5 files changed, 128 insertions(+)
 create mode 100644 ckanext/ed/mailer.py
 create mode 100644 ckanext/ed/templates/emails/package_publish_request.txt
 create mode 100644 ckanext/ed/tests/test_mailer.py

diff --git a/ckanext/ed/actions.py b/ckanext/ed/actions.py
index 4cb3964..add50a7 100644
--- a/ckanext/ed/actions.py
+++ b/ckanext/ed/actions.py
@@ -5,10 +5,14 @@
 import zipfile
 
 from ckan.controllers.admin import get_sysadmins
+from ckan.lib.mailer import MailerException
+from ckan.logic.action.create import package_create as core_package_create
 from ckan.logic.action.get import package_show as core_package_show
 from ckan.plugins import toolkit
 
 from ckanext.ed import helpers
+from ckanext.ed.mailer import mail_package_publish_request_to_admins
+
 
 SUPPORTED_RESOURCE_MIMETYPES = [
     'application/vnd.openxmlformats-officedocument.presentationml.presentation',
@@ -153,3 +157,15 @@ def package_show(context, data_dict):
     if not can_edit and approval_pending:
         raise toolkit.ObjectNotFound
     return package
+
+
+@toolkit.side_effect_free
+def package_create(context, data_dict):
+    dataset_dict = core_package_create(context, data_dict)
+    if dataset_dict.get('approval_state') == 'approval_pending':
+        try:
+            mail_package_publish_request_to_admins(context, dataset_dict)
+        except MailerException:
+            message = '[email] Package Publishing request is not sent: {0}'
+            log.critical(message.format(data_dict.get('title')))
+    return dataset_dict
diff --git a/ckanext/ed/mailer.py b/ckanext/ed/mailer.py
new file mode 100644
index 0000000..c60e719
--- /dev/null
+++ b/ckanext/ed/mailer.py
@@ -0,0 +1,42 @@
+import logging
+
+from ckan import model
+from ckan.common import config
+from ckan.plugins import toolkit
+from ckan.lib.mailer import mail_user
+from ckan.lib.base import render_jinja2
+from ckan.logic.action.get import member_list as core_member_list
+
+log = logging.getLogger(__name__)
+
+
+def mail_package_publish_request_to_admins(context, data_dict):
+    members = core_member_list(
+        context=context,
+        data_dict={'id': data_dict.get('owner_org')}
+    )
+    admin_ids = [i[0] for i in members if i[2] == 'Admin']
+    for admin_id in admin_ids:
+        user = model.User.get(admin_id)
+        if user.email:
+            subj = _compose_email_subj(data_dict, event='request')
+            body = _compose_email_body(data_dict, user, event='request')
+            mail_user(user, subj, body)
+            log.debug('[email] Pakcage publishing request email sent to {0}'.format(user.name))
+
+
+def _compose_email_subj(data_dict, event='request'):
+    return '[US ED] Package Publishing {0}: {1}'.format(event.capitalize(), data_dict.get('title'))
+
+
+def _compose_email_body(data_dict, user, event='request'):
+    pkg_link = toolkit.url_for('dataset_read', id=data_dict['name'], qualified=True)
+    return render_jinja2('emails/package_publish_{0}.txt'.format(event), {
+        'admin_name': user.fullname or user.name,
+        'site_title': config.get('ckan.site_title'),
+        'site_url': config.get('ckan.site_url'),
+        'package_title': data_dict.get('title'),
+        'package_description': data_dict.get('notes', ''),
+        'package_url': pkg_link,
+        'publisher_name': data_dict.get('contact_name')
+    })
diff --git a/ckanext/ed/plugin.py b/ckanext/ed/plugin.py
index d5c8ba1..ad4b642 100644
--- a/ckanext/ed/plugin.py
+++ b/ckanext/ed/plugin.py
@@ -28,6 +28,7 @@ def get_helpers(self):
     def get_actions(self):
         return {
             'ed_prepare_zip_resources': actions.prepare_zip_resources,
+            'package_create': actions.package_create,
             'package_show': actions.package_show
         }
 
diff --git a/ckanext/ed/templates/emails/package_publish_request.txt b/ckanext/ed/templates/emails/package_publish_request.txt
new file mode 100644
index 0000000..82dd5ac
--- /dev/null
+++ b/ckanext/ed/templates/emails/package_publish_request.txt
@@ -0,0 +1,18 @@
+Dear {{ admin_name }},
+
+A new package has been requested for publishing by {{ publisher_name }}:
+
+    {{ package_title }}
+
+    {{ package_description}}
+
+To approve or reject the request, please visit the following page (logged in as a office administrator):
+
+    {{ package_url }}
+
+Have a nice day.
+
+
+--
+Message sent by {{ site_title }} ({{ site_url }})
+This is an automated message, please don't respond to this address.
diff --git a/ckanext/ed/tests/test_mailer.py b/ckanext/ed/tests/test_mailer.py
new file mode 100644
index 0000000..5847a96
--- /dev/null
+++ b/ckanext/ed/tests/test_mailer.py
@@ -0,0 +1,51 @@
+import mock
+from nose.tools import assert_raises, assert_equals
+
+from ckan import model
+from ckan.tests import factories as core_factories
+from ckan.tests import helpers as test_helpers
+from ckanext.ed.tests import factories
+
+from ckanext.ed.mailer import mail_package_publish_request_to_admins
+
+
+# Without jinja2 mocking it can't find a template in the test mode
+class TestHelpers(test_helpers.FunctionalTestBase):
+    context = {'model': model, 'ignore_auth': True}
+
+    @mock.patch('ckanext.ed.mailer.mail_user')
+    @mock.patch('ckanext.ed.mailer.render_jinja2')
+    def test_mail_package_publish_request_to_admins(self, mock_render_jinja2, mock_mail_user):
+        admin_1 = core_factories.User()
+        admin_2 = core_factories.User()
+        editor = core_factories.User()
+        office = core_factories.Organization(
+            users=[
+                {'name': admin_1['name'], 'capacity': 'admin'},
+                {'name': admin_2['name'], 'capacity': 'admin'},
+                {'name': editor['name'], 'capacity': 'editor'},
+            ],
+            name='us-ed',
+            id='us-ed'
+        )
+        mail_package_publish_request_to_admins(
+            self.context,
+            _create_dataset_dict('test', office['name'])
+        )
+        assert_equals(mock_mail_user.call_count, 2)
+        assert_equals(mock_render_jinja2.call_args[0][0], 'emails/package_publish_request.txt')
+
+
+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 4a830fc2f1a09f113a0b508b4237a061c03f4510 Mon Sep 17 00:00:00 2001
From: Irakli Mchedlishvili <irakli.mchedlishvili@datopian.com>
Date: Tue, 4 Dec 2018 15:12:53 +0400
Subject: [PATCH 18/18] [actions][m]: approve/reject for admin

- update is_admin helper to filter by owner_org (used show buttons)
- approve/reject buttons at the bottom of the package
  - discus comment section for feedback (available only if pending/rejected)
- notify editor package has been rejected
- controllers to approve/reject
  - check logged in user has perms to delete -> approve/reject
---
 ckanext/ed/controller.py               | 46 +++++++++++++++++-
 ckanext/ed/helpers.py                  | 13 +++--
 ckanext/ed/plugin.py                   |  7 +++
 ckanext/ed/schemas/dataset.json        |  3 +-
 ckanext/ed/templates/package/read.html | 67 ++++++++++++++++++++++++++
 5 files changed, 130 insertions(+), 6 deletions(-)
 create mode 100644 ckanext/ed/templates/package/read.html

diff --git a/ckanext/ed/controller.py b/ckanext/ed/controller.py
index b31ac4c..50b19ea 100644
--- a/ckanext/ed/controller.py
+++ b/ckanext/ed/controller.py
@@ -1,8 +1,14 @@
 import os
-from ckan.plugins import toolkit
-from ckanext.ed.helpers import get_storage_path_for
+import logging
+
+from ckan import model
 from ckan.common import response
 from ckan.lib import base
+from ckan.plugins import toolkit
+
+from ckanext.ed.helpers import get_storage_path_for
+
+log = logging.getLogger()
 
 
 class DownloadController(base.BaseController):
@@ -25,3 +31,39 @@ def download_zip(self, zip_id):
         response.headers['Content-Type'] = 'application/octet-stream'
         response.content_disposition = 'attachment; filename=' + package_name
         os.remove(file_path)
+
+
+class ApproveRejectControler(base.BaseController):
+    def approve(self, id):
+        _make_action(id, 'approve')
+
+    def reject(self, id):
+        _make_action(id, 'reject')
+
+
+def _raise_not_authz_or_not_pending(id):
+    toolkit.check_access(
+        'package_delete', {'model': model, 'user': toolkit.c.user}, {'id': id})
+    # check approval_state is pending
+    data_dict = toolkit.get_action('package_show')({}, {'id': id})
+    if data_dict.get('approval_state') != 'approval_pending':
+        raise toolkit.ObjectNotFound('Dataset "{}" not found'.format(id))
+
+
+def _make_action(package_id, action='reject'):
+    states = {
+        'reject': 'rejected',
+        'approve': 'approved'
+    }
+    # check access and state
+    _raise_not_authz_or_not_pending(package_id)
+    data_dict = toolkit.get_action('package_patch')(
+        {'model': model, 'user': toolkit.c.user},
+        {'id': package_id, 'approval_state': states[action]}
+    )
+    msg = 'Dataset "{0}" {1}'.format(data_dict['title'], states[action])
+    if action == 'approve':
+        toolkit.h.flash_success(msg)
+    else:
+        toolkit.h.flash_error(msg)
+    toolkit.redirect_to(controller='package', action='read', id=data_dict['name'])
diff --git a/ckanext/ed/helpers.py b/ckanext/ed/helpers.py
index d6bffdb..e2ccfb5 100644
--- a/ckanext/ed/helpers.py
+++ b/ckanext/ed/helpers.py
@@ -108,15 +108,22 @@ def get_total_views_for_dataset(id):
         return 0
 
 
-def is_admin(user):
+def is_admin(user, office=None):
     """
-    Returns True if user is admin of any organisation
+    Returns True if user is admin of given organisation.
+    If office param is not provided checks if user is admin of any organisation
 
     :param user: user name
     :type user: string
+    :param office: office id
+    :type user: string
 
     :returns: True/False
     :rtype: boolean
     """
-    user_orgs = _get_action('organization_list_for_user', {'user': user}, {'user': user})
+    user_orgs = _get_action(
+                'organization_list_for_user', {'user': user}, {'user': user})
+    if office is not None:
+        return any([i.get('capacity') == 'admin' \
+                and i.get('id') == office for i in user_orgs])
     return any([i.get('capacity') == 'admin' for i in user_orgs])
diff --git a/ckanext/ed/plugin.py b/ckanext/ed/plugin.py
index ad4b642..ea62e66 100644
--- a/ckanext/ed/plugin.py
+++ b/ckanext/ed/plugin.py
@@ -47,6 +47,13 @@ def update_config(self, config_):
 
     # IRoutes
     def before_map(self, map):
+        publish_controller = 'ckanext.ed.controller:ApproveRejectControler'
+        map.connect('/dataset-publish/{id}/approve',
+                    controller=publish_controller,
+                    action='approve')
+        map.connect('/dataset-publish/{id}/reject',
+                    controller=publish_controller,
+                    action='reject')
         map.connect(
             'download_zip',
             '/download/zip/{zip_id}',
diff --git a/ckanext/ed/schemas/dataset.json b/ckanext/ed/schemas/dataset.json
index 6796945..6d47154 100644
--- a/ckanext/ed/schemas/dataset.json
+++ b/ckanext/ed/schemas/dataset.json
@@ -121,7 +121,8 @@
     {
       "field_name": "approval_state",
       "form_snippet": null,
-      "validators": "state_validator"
+      "validators": "state_validator",
+      "label": "Approved"
     }
   ],
   "resource_fields": [
diff --git a/ckanext/ed/templates/package/read.html b/ckanext/ed/templates/package/read.html
new file mode 100644
index 0000000..9dc87ef
--- /dev/null
+++ b/ckanext/ed/templates/package/read.html
@@ -0,0 +1,67 @@
+{% extends "package/read_base.html" %}
+
+{% block primary_content_inner %}
+  {{ super() }}
+  {% if c.pkg_dict.approval_state  == 'approval_pending' %}
+    <div class="alert alert-warning" role="alert">
+      <p>{% trans %}This dataset is waiting for an administrator approval{% endtrans %}</p>
+    </div>
+  {% elif c.pkg_dict.approval_state  == 'rejected' %}
+    {% if not h.ed_is_admin(c.user)%}
+    <div class="alert alert-warning" role="alert">
+      <p>{% trans %}Dataset was rejected by administrator please see comment{% endtrans %}</p>
+    </div>
+    {% endif %}
+  {% endif %}
+    {% block package_description %}
+      {% if pkg.private %}
+        <span class="dataset-private label label-inverse pull-right">
+          <i class="fa fa-lock"></i>
+          {{ _('Private') }}
+        </span>
+      {% endif %}
+      <h1>
+        {% block page_heading %}
+          {{ h.dataset_display_name(pkg) }}
+          {% if pkg.state.startswith('draft') %}
+            [{{ _('Draft') }}]
+          {% endif %}
+          {% if pkg.state == 'deleted' %}
+            [{{ _('Deleted') }}]
+          {% endif %}
+        {% endblock %}
+      </h1>
+      {% block package_notes %}
+        {% if pkg.notes %}
+          <div class="notes embedded-content">
+            {{ h.render_markdown(h.get_translated(pkg, 'notes')) }}
+          </div>
+        {% endif %}
+      {% endblock %}
+      {# FIXME why is this here? seems wrong #}
+      <span class="insert-comment-thread"></span>
+    {% endblock %}
+    {% block package_resources %}
+      {% snippet "package/snippets/resources_list.html", pkg=pkg, resources=pkg.resources %}
+    {% endblock %}
+
+    {% block package_tags %}
+      {% snippet "package/snippets/tags.html", tags=pkg.tags %}
+    {% endblock %}
+
+    {% block package_additional_info %}
+      {% snippet "package/snippets/additional_info.html", pkg_dict=pkg %}
+    {% endblock %}
+    {% if c.pkg_dict.approval_state  == 'approval_pending' or c.pkg_dict.approval_state  == 'rejected' %}
+      {% if h.ed_is_admin(c.user, c.pkg_dict.owner_org) %}
+        <a href="/dataset-publish/{{ c.pkg_dict.id }}/approve" class="btn btn-primary">
+          {% trans %}Approve Dataset{% endtrans %}
+        </a>
+        <a href="/dataset-publish/{{ c.pkg_dict.id }}/reject" class="btn btn-danger">
+          {% trans %}Reject Dataset{% endtrans %}
+        </a>
+        <br>
+      {% endif %}
+      {{h.disqus_comments()}}
+    {% endif %}
+{% endblock %}