Merge pull request #134 from open-data/changes/linting

CKAN Coding Standards

Author: JVickery-TBS

.github/workflows/flake8.yml

@@ -0,0 +1,22 @@
+name: Lint
+on: [pull_request]
+
+permissions:
+  contents: read
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: '3.9'
+      - name: Install requirements
+        run: pip install flake8 pycodestyle
+      - name: Check syntax
+        # Stop the build if there are Python syntax errors or undefined names
+        run: flake8 --show-source
+
+      - name: Warnings
+        run: flake8

.github/workflows/pyright.yml

@@ -0,0 +1,46 @@
+name: Check types
+on: [pull_request]
+env:
+  NODE_VERSION: '18'
+  PYTHON_VERSION: '3.9'
+
+permissions:
+  contents: read
+
+jobs:
+  typecheck:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+        id: gitcheckout
+      - uses: actions/setup-python@v2
+        with:
+          python-version: ${{ env.PYTHON_VERSION }}
+      - uses: actions/setup-node@v3
+        with:
+          node-version: ${{ env.NODE_VERSION }}
+      - name: Install python deps
+        id: pydepends
+        if: steps.gitcheckout.outcome == 'success'
+        run: |
+            python3 -m venv /home/runner/work/app
+            mkdir -p /home/runner/work/app/src/ckanext-recombinant
+            cp -R . /home/runner/work/app/src/ckanext-recombinant
+            source /home/runner/work/app/bin/activate
+            pip install --upgrade setuptools==70.0.0
+            pip install --upgrade pip==23.2.1
+            pip install -e 'git+https://github.com/open-data/ckan@canada-v2.10#egg=ckan' -r 'https://raw.githubusercontent.com/open-data/ckan/canada-v2.10/requirement-setuptools.txt' -r 'https://raw.githubusercontent.com/open-data/ckan/canada-v2.10/requirements.txt' -r 'https://raw.githubusercontent.com/open-data/ckan/canada-v2.10/dev-requirements.txt'
+            pip install -e 'git+https://github.com/ckan/ckanapi.git#egg=ckanapi' -r 'https://raw.githubusercontent.com/ckan/ckanapi/master/requirements.txt'
+            pip install -e /home/runner/work/app/src/ckanext-recombinant/. -r /home/runner/work/app/src/ckanext-recombinant/requirements.txt -r /home/runner/work/app/src/ckanext-recombinant/test-requirements.txt
+            pip install -e 'git+https://github.com/ckan/ckantoolkit.git#egg=ckantoolkit' -r 'https://raw.githubusercontent.com/ckan/ckantoolkit/master/requirements.txt'
+            find /home/runner/work/app -name '*.pyc' -delete
+      - name: Install node deps
+        if: steps.pydepends.outcome == 'success'
+        run: |
+          cd /home/runner/work/app/src/ckanext-recombinant
+          npm ci
+      - name: Check types
+        if: steps.pydepends.outcome == 'success'
+        run: |
+          cd /home/runner/work/app/src/ckanext-recombinant
+          npx pyright

.gitignore

@@ -2,3 +2,4 @@ ckan.log
 *.pyc
 *.egg-info/
 *.mo
+node_modules/**

changes/134.changes

@@ -0,0 +1 @@
+Refactor code to meet CKAN and Python coding standards.

ckanext/recombinant/auth.py

@@ -1,46 +1,63 @@
+from typing import Optional
+from ckan.types import (
+    AuthFunction,
+    Context,
+    DataDict,
+    AuthResult
+)
+
 from ckan.plugins.toolkit import chained_auth_function, h, _
-from ckan.model.group import Group
-from ckanapi import LocalCKAN, ValidationError
 
 
 @chained_auth_function
-def package_update(up_func, context, data_dict):
+def package_update(up_func: AuthFunction,
+                   context: Context,
+                   data_dict: Optional[DataDict]) -> AuthResult:
     """
     Recombinant packages and resources should not be
     editable by users after they have been made.
 
     Affects update, patch, and delete.
     """
-    if data_dict and data_dict.get(u'type') in h.recombinant_get_types():
+    if data_dict and data_dict.get('type') in h.recombinant_get_types():
         return {'success': False,
                 'msg': _('User %s not authorized to modify Recombinant type: %s') %
-                            (str(context[u'user']), data_dict.get(u'type'))}
-    return up_func(context, data_dict)
+                        (str(context['user']), data_dict.get('type'))}
+    # type_ignore_reason: incomplete typing
+    return up_func(context, data_dict)  # type: ignore
 
 
 @chained_auth_function
-def package_create(up_func, context, data_dict):
+def package_create(up_func: AuthFunction,
+                   context: Context,
+                   data_dict: Optional[DataDict]) -> AuthResult:
     """
     If the user has permissions to create packages, they should still
     be allowed to create the Recombinant packages and resources.
 
     However, users should not be allowed to create multiple packages
     of the same type for a single organization.
     """
-    if data_dict and data_dict.get(u'type') in h.recombinant_get_types():
+    if data_dict and data_dict.get('type') in h.recombinant_get_types():
         return {'success': False,
-                'msg': _('User %s not authorized to create Recombinant packages: %s') %
-                            (str(context[u'user']), data_dict.get(u'type'))}
-    return up_func(context, data_dict)
+                'msg': _('User %s not authorized to create '
+                         'Recombinant packages: %s') %
+                        (str(context['user']), data_dict.get('type'))}
+    # type_ignore_reason: incomplete typing
+    return up_func(context, data_dict)  # type: ignore
 
 
 @chained_auth_function
-def datastore_delete(up_func, context, data_dict):
+def datastore_delete(up_func: AuthFunction,
+                     context: Context,
+                     data_dict: Optional[DataDict]) -> AuthResult:
     """
     Users should not be able to delete a Datestore table for
     Recombinant types. We only want them to be able to delete rows.
     """
-    res = context['model'].Resource.get(data_dict.get('resource_id'))
+    if not data_dict:
+        data_dict = {}
+    res = context['model'].Resource.get(data_dict.get('resource_id', ''))
     pkg = context['model'].Package.get(getattr(res, 'package_id', None))
     if not res or not pkg or pkg.type not in h.recombinant_get_types():
         return up_func(context, data_dict)
@@ -49,6 +66,5 @@ def datastore_delete(up_func, context, data_dict):
         # we do not want that to happen for Recombinant types.
         return {'success': False,
                 'msg': _("Cannot delete Datastore for type: %s. "
-                         "Use datastore_records_delete instead.")
-                         % pkg.type}
+                         "Use datastore_records_delete instead.") % pkg.type}
     return up_func(context, data_dict)