Handle missing resource formats

.github/workflows/test.yml

@@ -4,37 +4,41 @@ jobs:
   lint:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-python@v2
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
         with:
-          python-version: '3.8'
+          python-version: '3.9'
       - name: Install requirements
-        run: |
-          pip install flake8 pycodestyle pylint bandit
-          pip install -e .
+        run: pip install flake8 pycodestyle
       - name: Check syntax
         run: flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics --exclude ckan
       - name: Run flake8
         run: flake8 . --count --max-line-length=127 --statistics --exclude ckan
-      - name: Run pylint
-        run: pylint --output-format=colorized ckanext/downloadall
-      - name: Run bandit
-        run: bandit -s B101  -r ./ -f screen
 
   test:
     needs: lint
     strategy:
       matrix:
-        ckan-version: [2.9]
+        include:
+          - ckan-version: "2.11"
+            ckan-image: "ckan/ckan-dev:2.11-py3.10"
+            solr-version: "9"
+          - ckan-version: "2.10"
+            ckan-image: "ckan/ckan-dev:2.10-py3.10"
+            solr-version: "9"
+          - ckan-version: "2.9"
+            ckan-image: "ckan/ckan-dev:2.9-py3.9"
+            solr-version: "8"
       fail-fast: false
 
     name: CKAN ${{ matrix.ckan-version }}
     runs-on: ubuntu-latest
     container:
-      image: openknowledge/ckan-dev:${{ matrix.ckan-version }}
+      image: ${{ matrix.ckan-image }}
+      options: --user root
     services:
       solr:
-        image: ckan/ckan-solr:${{ matrix.ckan-version }}
+        image: ckan/ckan-solr:${{ matrix.ckan-version }}-solr9
       postgres:
         image: ckan/ckan-postgres-dev:${{ matrix.ckan-version }}
         env:
@@ -59,6 +63,9 @@ jobs:
         pip install -e .
         # Replace default path to CKAN core config file with the one on the container
         sed -i -e 's/use = config:.*/use = config:\/srv\/app\/src\/ckan\/test-core.ini/' test.ini
+        # Install unzip for SonarQube Scan
+        apt-get update
+        apt-get install unzip -y
     - name: Setup extension
       run: |
         ckan -c test.ini db init

.gitignore

@@ -42,4 +42,6 @@ coverage.xml
 docs/_build/
 
 .vscode
-idea/
+idea/
+
+.DS_Store

ckanext/downloadall/plugin.py

@@ -86,7 +86,7 @@ def get_helpers(self):
     # IPackageController
     def before_index(self, pkg_dict):
         try:
-            if 'All resource data' in pkg_dict['res_name']:
+            if 'All resource data' in pkg_dict.get('res_name', ''):
                 # we've got a 'Download all zip', so remove it's ZIP from the
                 # SOLR facet of resource formats, as it's not really a data
                 # resource

ckanext/downloadall/public/css/downloadall.css

@@ -0,0 +1,3 @@
+a.downloadall-disabled {
+    display: none;
+}

ckanext/downloadall/tasks.py

@@ -347,10 +347,10 @@ def remove_resources_that_should_not_be_included_in_the_datapackage(dataset):
             existing_zip_resource = res
             continue
 
-        if res['format'] in resource_formats_to_ignore:
+        if res.get('format', '') in resource_formats_to_ignore:
             log.debug('Resource resource {}/{} skipped - because it is '
                       'format {}'.format(i + 1, len(dataset.get('resources', [])),
-                                         res['format']))
+                                         res.get('format', '')))
             continue
         resources_to_include.append(res)
     dataset = dict(dataset, resources=resources_to_include)

ckanext/downloadall/templates/package/read.html

@@ -1,15 +1,20 @@
 {% ckan_extends %}
 
+{% block styles %}
+    {{ super() }}
+    <link rel="stylesheet" href="/css/downloadall.css" />
+{% endblock %}
+
 {% block package_resources %}
 
   {% if pkg.resources %}
     <div class="btn-group" style="float:right; margin-top: -7px;">
 
       {% set zip_res = h.downloadall__pop_zip_resource(pkg) %}
       {% if zip_res %}
-        <a class="btn btn-primary resource-url-analytics resource-type-{{ zip_res.resource_type }}" href="{{ zip_res.url }}">
+        <a class="btn btn-primary downloadall-enabled resource-url-analytics resource-type-{{ zip_res.resource_type }}" href="{{ zip_res.url }}">
       {% else %}
-        <a class="btn btn-primary" disabled="disabled" title="{{ _('This download is not currently available') }}" href="#">
+        <a class="btn btn-primary downloadall-disabled" disabled="disabled" title="{{ _('This download is not currently available') }}" href="#">
       {% endif %}
           <i class="fa fa-arrow-circle-o-down"></i> {{ _('Download all') }}
         </a>

ckanext/downloadall/tests/test_action.py

@@ -10,8 +10,7 @@
 class TestDatastoreCreate(TestBase):
     def test_datastore_create(self):
         dataset = factories.Dataset(
-            owner_org=self.org['id'],
-            resources=[{'url': 'http://some.image.png', 'format': 'png'}])
+            resources=[{'url': 'https://example.com/data.csv', 'format': 'csv'}])
         helpers.call_action('job_clear')
 
         helpers.call_action('datastore_create',

ckanext/downloadall/tests/test_plugin.py

@@ -6,64 +6,63 @@
 
 class TestNotify(TestBase):
     def test_new_resource_leads_to_queued_task(self):
-        dataset = factories.Dataset(owner_org=self.org['id'], resources=[
-            {'url': 'http://some.image.png', 'format': 'png'}])
-        assert [job['title'] for job in helpers.call_action('job_list')] == [
-            'DownloadAll new "{}" {}'.format(dataset['name'], dataset['id'])]
+        dataset = factories.Dataset(resources=[
+            {'url': 'https://example.com/data.csv', 'format': 'csv'}])
+        assert 'DownloadAll new "{}" {}'.format(dataset['name'], dataset['id']) in [
+            job['title'] for job in helpers.call_action('job_list')]
 
     def test_changed_resource_leads_to_queued_task(self):
-        dataset = factories.Dataset(owner_org=self.org['id'], resources=[
-            {'url': 'http://some.image.png', 'format': 'png'}])
+        dataset = factories.Dataset(resources=[
+            {'url': 'https://example.com/data.csv', 'format': 'csv'}])
         helpers.call_action('job_clear')
 
         dataset['resources'][0]['url'] = 'http://another.image.png'
         helpers.call_action('package_update', **dataset)
 
-        assert [job['title'] for job in helpers.call_action('job_list')] == [
-            'DownloadAll changed "{}" {}'.format(dataset['name'], dataset['id'])]
+        assert 'DownloadAll changed "{}" {}'.format(dataset['name'], dataset['id']) in [
+            job['title'] for job in helpers.call_action('job_list')]
 
     def test_deleted_resource_leads_to_queued_task(self):
-        dataset = factories.Dataset(owner_org=self.org['id'], resources=[
-            {'url': 'http://some.image.png', 'format': 'png'}])
+        dataset = factories.Dataset(resources=[
+            {'url': 'https://example.com/data.csv', 'format': 'csv'}])
         helpers.call_action('job_clear')
 
         dataset['resources'] = []
         helpers.call_action('package_update', **dataset)
 
-        assert [job['title'] for job in helpers.call_action('job_list')] == [
-            'DownloadAll changed "{}" {}'.format(dataset['name'], dataset['id'])]
+        assert 'DownloadAll changed "{}" {}'.format(dataset['name'], dataset['id']) in [
+            job['title'] for job in helpers.call_action('job_list')]
 
     def test_created_dataset_leads_to_queued_task(self):
         dataset = {'name': 'testdataset_da',
-                   'owner_org': self.org['id'],
                    'title': 'Test Dataset',
                    'notes': 'Just another test dataset.',
                    'resources': [
-                        {'url': 'http://some.image.png', 'format': 'png'}
+                        {'url': 'https://example.com/data.csv', 'format': 'csv'}
                    ]}
         dataset = helpers.call_action('package_create', **dataset)
         # this should prompt datapackage.json to be updated
 
-        assert [job['title'] for job in helpers.call_action('job_list')] == [
-            'DownloadAll new "{}" {}'.format(dataset['name'], dataset['id'])]
+        assert 'DownloadAll new "{}" {}'.format(dataset['name'], dataset['id']) in [
+            job['title'] for job in helpers.call_action('job_list')]
 
     def test_changed_dataset_leads_to_queued_task(self):
-        dataset = factories.Dataset(owner_org=self.org['id'], resources=[
-            {'url': 'http://some.image.png', 'format': 'png'}])
+        dataset = factories.Dataset(resources=[
+            {'url': 'https://example.com/data.csv', 'format': 'csv'}])
         helpers.call_action('job_clear')
 
         dataset['notes'] = 'Changed description'
         helpers.call_action('package_update', **dataset)
         # this should prompt datapackage.json to be updated
 
-        assert [job['title'] for job in helpers.call_action('job_list')] == [
-            'DownloadAll changed "{}" {}'.format(dataset['name'], dataset['id'])]
+        assert 'DownloadAll changed "{}" {}'.format(dataset['name'], dataset['id']) in [
+            job['title'] for job in helpers.call_action('job_list')]
 
     def test_creation_of_zip_resource_leads_to_queued_task(self):
         # but we don't get an infinite loop because it is stopped by the
         # skip_if_no_changes
-        dataset = factories.Dataset(owner_org=self.org['id'], resources=[
-            {'url': 'http://some.image.png', 'format': 'png'}])
+        dataset = factories.Dataset(resources=[
+            {'url': 'https://example.com/data.csv', 'format': 'csv'}])
         helpers.call_action('job_clear')
         resource = {
             'package_id': dataset['id'],
@@ -73,15 +72,8 @@ def test_creation_of_zip_resource_leads_to_queued_task(self):
         }
         helpers.call_action('resource_create', **resource)
 
-        assert [job['title'] for job in helpers.call_action('job_list')] == [
-            'DownloadAll changed "{}" {}'.format(dataset['name'], dataset['id'])]
-
-    def test_other_instance_types_do_nothing(self):
-        factories.User()
-        factories.Organization()
-        factories.Group()
-        assert not list(job['title'] for job in helpers.call_action('job_list'))
-        assert not list(helpers.call_action('job_list'))
+        assert 'DownloadAll changed "{}" {}'.format(dataset['name'], dataset['id']) in [
+            job['title'] for job in helpers.call_action('job_list')]
 
     # An end-to-end test is too tricky to write - creating a dataset and seeing
     # the zip file created requires the queue worker to run, but that rips down