From 9df8a1396ac123f3dd2702c707da1ac82614fece Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 6 Dec 2021 12:53:37 +1000 Subject: [PATCH 1/7] [QOL-7902] clean up test workflow and docs --- .github/workflows/test.yml | 14 +++++++------- README.md | 29 +++++++++++++++-------------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 214ff8c..c384d26 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,7 +3,7 @@ name: Tests on: [push, pull_request] jobs: lint: - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 @@ -32,7 +32,7 @@ jobs: fail-fast: false name: CKAN ${{ matrix.ckan-version }} - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest container: image: openknowledge/ckan-dev:${{ matrix.ckan-version }} services: @@ -70,6 +70,7 @@ jobs: - name: Install requirements run: | + pip install -r requirements.txt pip install -r requirements-dev.txt pip install -e . # Replace default path to CKAN core config file with the one on the container @@ -85,9 +86,8 @@ jobs: if: ${{ matrix.ckan-version == '2.7' || matrix.ckan-version == '2.8' }} run: | paster --plugin=ckan db init -c test.ini - paster report initdb -c test.ini - paster report generate tagless-datasets -c test.ini + paster --plugin=ckanext-report report initdb -c test.ini + paster --plugin=ckanext-report report generate tagless-datasets -c test.ini - - name: Run all tests - run: | - pytest --ckan-ini=test.ini --cov=ckanext.report ckanext/report/tests + - name: Run tests + run: pytest --ckan-ini=test.ini --cov=ckanext.report --disable-warnings ckanext/report/tests diff --git a/README.md b/README.md index 02b7155..b095018 100644 --- a/README.md +++ b/README.md @@ -14,34 +14,32 @@ Example report: ![Demo report image](report-demo.png) -A number of extensions currently offer reports that rely on this extension, e.g. [ckanext-archiver](https://github.com/datagovuk/ckanext-archiver/blob/master/ckanext/archiver/reports.py), [ckanext-qa](https://github.com/datagovuk/ckanext-qa/blob/master/ckanext/qa/reports.py), [ckanext-dgu](https://github.com/datagovuk/ckanext-dgu/blob/master/ckanext/dgu/lib/reports.py). +A number of extensions currently offer reports that rely on this extension, e.g. [ckanext-archiver](https://github.com/ckan/ckanext-archiver/blob/master/ckanext/archiver/reports.py), [ckanext-qa](https://github.com/ckan/ckanext-qa/blob/master/ckanext/qa/reports.py), [ckanext-dgu](https://github.com/datagovuk/ckanext-dgu/blob/master/ckanext/dgu/lib/reports.py). TODO: * Stop a report from being generated multiple times in parallel (unnecessary waste) - use a queue? * Stop more than one report being generated in parallel (high load for the server) - maybe use a queue. -Compatibility: Requires CKAN version 2.1 or later (but can be easily adapted for older versions). +## Compatibility: +| CKAN version | Compatibility | +| --------------- | ------------------- | +| 2.6 and earlier | yes | +| 2.7 | yes | +| 2.8 | yes | +| 2.9 | yes | -| CKAN version | Compatibility | -| --------------- | ------------- | -| 2.6 and earlier | yes | -| 2.7 | yes | -| 2.8 | yes | -| 2.9 | yes | +Status: was in production at data.gov.uk around 2014-2016, but since that uses its own CSS rather than core CKAN's, for others to use it CSS needs adding. For an example, see this branch: see https://github.com/GSA/ckanext-report/tree/geoversion - -Status: in production at data.gov.uk but since that uses its own CSS rather than core CKAN's, for others to use it CSS needs adding. For an example, see this branch: see https://github.com/yaditi/ckanext-report/tree/geoversion - -Author(s): David Read +Author(s): David Read and contributors ## Install & setup Install ckanext-report into your CKAN virtual environment in the usual way: - (pyenv) $ pip install -e git+https://github.com/datagovuk/ckanext-report.git#egg=ckanext-report + (pyenv) $ pip install -e git+https://github.com/ckan/ckanext-report.git#egg=ckanext-report Initialize the database tables needed by ckanext-report: @@ -54,7 +52,7 @@ Enable the plugin. In your config (e.g. development.ini or production.ini) add ` ## Command-line interface -The following operations can be run from the command line using the ``paster --plugin=ckanext-report report`` command: +The following operations can be run from the command line using the ``paster --plugin=ckanext-report report`` or ``ckan report`` commands: ``` report list @@ -67,14 +65,17 @@ The following operations can be run from the command line using the ``paster --p Get the list of reports: (pyenv) $ paster --plugin=ckanext-report report list --config=mysite.ini + (pyenv) $ ckan --config=mysite.ini report list Generate all reports: (pyenv) $ paster --plugin=ckanext-report report generate --config=mysite.ini + (pyenv) $ ckan --config=mysite.ini report generate Generate a single report: (pyenv) $ paster --plugin=ckanext-report report generate --config=mysite.ini + (pyenv) $ ckan --config=mysite.ini report generate ## Demo report - Tagless Datasets From 59156aa45b05555dbfc7c0da706cd93cb5d7375b Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 6 Dec 2021 12:55:19 +1000 Subject: [PATCH 2/7] [QOL-7902] fix Flask routes --- README.md | 5 ++++- ckanext/report/templates/report/view.html | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index b095018..cd935cb 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,9 @@ Report (snippet) table - main data, as a list of rows, each row is a dict data - other data values, as a dict #} + +{% set ckan_29_or_higher = h.ckan_version().split('.')[1] | int >= 9 %} +{% set dataset_read_route = 'dataset.read' if ckan_29_or_higher else 'dataset_read' %}
  • Datasets without tags: {{ table|length }} / {{ data['num_packages'] }} ({{ data['packages_without_tags_percent'] }})
  • Average tags per package: {{ data['average_tags_per_package'] }} tags
    • @@ -161,7 +164,7 @@ data - other data values, as a dict {% for row in table %} - + {{ row.title }} diff --git a/ckanext/report/templates/report/view.html b/ckanext/report/templates/report/view.html index 5334fcf..e81fdfc 100644 --- a/ckanext/report/templates/report/view.html +++ b/ckanext/report/templates/report/view.html @@ -43,8 +43,8 @@

    {{ _('Options') }}

    {% if are_some_results %}
    {{ _('Download') }}: - CSV - JSON + CSV + JSON
    {% endif %}

    {{ _('Results') }}

    From addf1158fcb3127c412c971c871d00636e602146 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 6 Dec 2021 12:57:18 +1000 Subject: [PATCH 3/7] [QOL-7902] fix Flask headers - generate headers in shared function, apply separately for Pylons and Flask. Pylons sets them on the toolkit response object, Flask has to create a Response on the fly. --- ckanext/report/controllers/__init__.py | 13 ++++++------ ckanext/report/controllers/blueprints.py | 21 +++++++++++++++---- .../report/controllers/pylons_controllers.py | 8 ++++++- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/ckanext/report/controllers/__init__.py b/ckanext/report/controllers/__init__.py index 162a8e5..d2a95bd 100644 --- a/ckanext/report/controllers/__init__.py +++ b/ckanext/report/controllers/__init__.py @@ -121,13 +121,14 @@ def report_view(report_name, organization=None, refresh=False): except t.NotAuthorized: t.abort(401) filename = 'report_%s.csv' % key - t.response.headers['Content-Type'] = 'application/csv' - t.response.headers['Content-Disposition'] = six.text_type('attachment; filename=%s' % (filename)) - return make_csv_from_dicts(data['table']) + response_headers = { + 'Content-Type': 'application/csv', + 'Content-Disposition': six.text_type('attachment; filename=%s' % (filename)) + } + return make_csv_from_dicts(data['table']), response_headers elif format == 'json': - t.response.headers['Content-Type'] = 'application/json' data['generated_at'] = report_date - return json.dumps(data) + return json.dumps(data), {'Content-Type': 'application/json'} else: t.abort(400, 'Format not known - try html, json or csv') @@ -141,4 +142,4 @@ def report_view(report_name, organization=None, refresh=False): 'report_date': report_date, 'options': options, 'options_html': options_html, 'report_template': report['template'], - 'are_some_results': are_some_results}) + 'are_some_results': are_some_results}), {} diff --git a/ckanext/report/controllers/blueprints.py b/ckanext/report/controllers/blueprints.py index 5b48753..3131e9a 100644 --- a/ckanext/report/controllers/blueprints.py +++ b/ckanext/report/controllers/blueprints.py @@ -1,9 +1,11 @@ # encoding: utf-8 -from flask import Blueprint +import six + +from flask import Blueprint, Response -from ckanext.report.controllers import report_index, report_view from ckan.plugins import toolkit +from . import report_index, report_view reporting = Blueprint( @@ -16,6 +18,17 @@ def redirect_to_index(): return toolkit.redirect_to('/report') +def view(report_name, organization=None, refresh=False): + body, headers = report_view(report_name, organization, refresh) + if headers: + response = Response(body) + for key, value in six.iteritems(headers): + response.headers[key] = value + return response + else: + return body + + reporting.add_url_rule( u'/report', 'index', view_func=report_index ) @@ -23,10 +36,10 @@ def redirect_to_index(): u'/reports', 'reports', view_func=redirect_to_index ) reporting.add_url_rule( - u'/report/', 'view', view_func=report_view + u'/report/', 'view', view_func=view, methods=('GET', 'POST',) ) reporting.add_url_rule( - u'/report//', 'org', view_func=report_view + u'/report//', 'org', view_func=view ) diff --git a/ckanext/report/controllers/pylons_controllers.py b/ckanext/report/controllers/pylons_controllers.py index 93b5947..82517a6 100644 --- a/ckanext/report/controllers/pylons_controllers.py +++ b/ckanext/report/controllers/pylons_controllers.py @@ -1,5 +1,7 @@ # encoding: utf-8 +import six + import ckan.plugins.toolkit as t from ckanext.report.controllers import report_index, report_view @@ -10,4 +12,8 @@ def index(self): return report_index() def view(self, report_name, organization=None, refresh=False): - return report_view(report_name, organization, refresh) + body, headers = report_view(report_name, organization, refresh) + if headers: + for key, value in six.iteritems(headers): + t.response.headers[key] = value + return body From f092946fcc00e95b81421f6b4f244cdffcaa4aed Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 6 Dec 2021 12:58:21 +1000 Subject: [PATCH 4/7] [QOL-7902] cleanup - fix indentation - drop width restriction on report page - handle exceptions more gracefully when report not found --- ckanext/report/controllers/__init__.py | 3 +++ ckanext/report/helpers.py | 10 +++++----- ckanext/report/templates/report/view.html | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/ckanext/report/controllers/__init__.py b/ckanext/report/controllers/__init__.py index d2a95bd..c9bfb74 100644 --- a/ckanext/report/controllers/__init__.py +++ b/ckanext/report/controllers/__init__.py @@ -38,6 +38,9 @@ def report_view(report_name, organization=None, refresh=False): t.abort(401) except t.ObjectNotFound: t.abort(404) + except Exception as e: + log.error("Failed to get report: %s", e) + raise # ensure correct url is being used if 'organization' in _get_routing_rule()\ diff --git a/ckanext/report/helpers.py b/ckanext/report/helpers.py index 9d46141..dca49f8 100644 --- a/ckanext/report/helpers.py +++ b/ckanext/report/helpers.py @@ -35,11 +35,11 @@ def relative_url_for(**kwargs): args = dict(list(tk.request.environ['pylons.routes_dict'].items()) + user_specified_params + list(kwargs.items())) - # remove blanks - for k, v in list(args.items()): - if not v: - del args[k] - return tk.url_for(**args) + # remove blanks + for k, v in list(args.items()): + if not v: + del args[k] + return tk.url_for(**args) def chunks(list_, size): diff --git a/ckanext/report/templates/report/view.html b/ckanext/report/templates/report/view.html index e81fdfc..f3fbdfb 100644 --- a/ckanext/report/templates/report/view.html +++ b/ckanext/report/templates/report/view.html @@ -14,7 +14,7 @@

    {{ report.title }}

    {{ _('Generated') }}: {{ h.report__render_datetime(report_date, '%d/%m/%Y %H:%M') }}

    {% if c.userobj.sysadmin %} -
    +
    {% trans %}Refresh report{% endtrans %}
    From 1f9cbeb8a2b7548d94b457701d15f37355960e2c Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 6 Dec 2021 13:08:16 +1000 Subject: [PATCH 5/7] [QOL-7902] oops drop nonexistent requirements file --- .github/workflows/test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c384d26..43efa8a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -70,7 +70,6 @@ jobs: - name: Install requirements run: | - pip install -r requirements.txt pip install -r requirements-dev.txt pip install -e . # Replace default path to CKAN core config file with the one on the container From 8471e9e64c9b304e86dfd05e5cf583b47ab82122 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 6 Dec 2021 14:40:24 +1000 Subject: [PATCH 6/7] [QOL-7906] pass report name properly to routing - moving away from the relative_url_for function and using named routes instead, as it doesn't properly handle placeholders in Flask URL rules. --- ckanext/report/controllers/__init__.py | 3 ++- ckanext/report/templates/report/option_organization.html | 2 +- ckanext/report/templates/report/view.html | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ckanext/report/controllers/__init__.py b/ckanext/report/controllers/__init__.py index c9bfb74..af7d342 100644 --- a/ckanext/report/controllers/__init__.py +++ b/ckanext/report/controllers/__init__.py @@ -73,7 +73,8 @@ def report_view(report_name, organization=None, refresh=False): log.warn('Not displaying report option HTML for param %s as option not recognized') continue option_display_params = {'value': options[option], - 'default': report['option_defaults'][option]} + 'default': report['option_defaults'][option], + 'report_name': report_name} try: options_html[option] = \ t.render_snippet('report/option_%s.html' % option, diff --git a/ckanext/report/templates/report/option_organization.html b/ckanext/report/templates/report/option_organization.html index 4cb8bac..57df931 100644 --- a/ckanext/report/templates/report/option_organization.html +++ b/ckanext/report/templates/report/option_organization.html @@ -9,7 +9,7 @@ {% set offer_organization_index = (default == None) %} {% if offer_organization_index and value != None %} - {% trans %}Index of all organizations{% endtrans %} + {% trans %}Index of all organizations{% endtrans %} {% endif %}