Merge pull request #2771 from specify/code-quality-2

Improve code quality checks
specify · Feb 5, 2023 · d16a33c · d16a33c
2 parents 5acc2e0 + 13fd898
commit d16a33c
Show file tree

Hide file tree

Showing 413 changed files with 8,254 additions and 15,310 deletions.
diff --git a/.github/workflows/README.md b/.github/workflows/README.md
@@ -0,0 +1,73 @@
+# GitHub Secrets
+
+Documentation for used GitHub secrets.
+
+**On any changes to used secrets/tokens, update this file**
+
+<table>
+    <thead>
+        <tr>
+            <th>Name</th>
+            <th>Active</th>
+            <th>Description</th>
+            <th>Used by</th>
+        </tr>
+    </thead>
+    <tbody>
+        <tr>
+            <td>QODANA_TOKEN</td>
+            <td>No</td>
+            <td>
+                Token for JetBrain's Qodana service
+            </td>
+            <td>
+                <a href="https://github.com/specify/specify7/pull/2710">
+                    #2710
+                </a>
+            </td>
+        </tr>
+        <tr>
+            <td>WEBLATE_API_TOKEN</td>
+            <td>Yes</td>
+            <td>
+                Regular weblate organization token. Used for API calls
+            </td>
+            <td>
+                <a href="https://github.com/specify/specify7/blob/production/.github/workflows/test.yml">
+                    test.yml
+                </a>,
+                <a href="https://github.com/specify/specify7/blob/weblate-localization/.github/workflows/push.yml">
+                    push.yml
+                </a>
+            </td>
+        </tr>
+        <tr>
+            <td>WEBLATE_PUSH_TO_GITHUB</td>
+            <td>Yes</td>
+            <td>
+                Personal GitHub token (from @maxxxxxdlp account). Personal token
+                is used to bypass branch protection rules (to allow Weblate to
+                push directly to production branch)
+            </td>
+            <td>
+                <a href="https://github.com/specify/specify7/blob/weblate-localization/.github/workflows/push.yml">
+                    push.yml
+                </a>
+            </td>
+        </tr>
+        <tr>
+            <td>TESTS_PUSH_TO_GITHUB</td>
+            <td>Yes</td>
+            <td>
+                Personal GitHub token (from @maxxxxxdlp account). Personal token
+                is used to bypass branch protection rules (to allow Weblate to
+                push directly to production branch)
+            </td>
+            <td>
+                <a href="https://github.com/specify/specify7/blob/production/.github/workflows/test.yml">
+                    test.yml
+                </a>
+            </td>
+        </tr>
+    </tbody>
+</table>
diff --git a/.github/workflows/sync-pre-commit.yml b/.github/workflows/sync-pre-commit.yml
@@ -24,6 +24,7 @@ jobs:
         with:
           node-version: 18
           cache: npm
+          cache-dependency-path: specifyweb/frontend/js_src/package-lock.json
 
       - name: Install dependencies
         working-directory: ./pre-commit-tools

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -5,10 +5,14 @@ on: [push]
 jobs:
   changes:
     name: Check if back-end or front-end files changed
+    # Don't run this for pushes generated by push.yml on weblate-localization
+    # branch → prevents an infinite loop of actions triggering actions
+    if: github.event.head_commit.committer.name != 'Hosted Weblate' && github.event.head_commit.committer.name != 'github-actions'
     runs-on: ubuntu-latest
     outputs:
       backend_changed: ${{ steps.back-end-check.outputs.changed }}
       frontend: ${{ steps.filter.outputs.frontend }}
+      frontend_changes: ${{ steps.filter.outputs.frontend_files }}
 
     steps:
       - uses: actions/checkout@v3
@@ -30,7 +34,7 @@ jobs:
         run: |
           changed_files=`echo "${{steps.filter.outputs.backend_files}}" | tr " " "\n" | grep -v 'specifyweb/frontend/' || echo ""`
           echo "Changed back-end files: ${changed_files}"
-          echo "changed=$([[ -z "${changed_files}" ]] && echo "0" || echo "1")" >> $GITHUB_OUTPUT
+          echo "changed=$([[ -z "${changed_files}" ]] && echo "" || echo "1")" >> $GITHUB_OUTPUT
 
   test-back-end:
     name: Run back-end tests
@@ -116,6 +120,13 @@ jobs:
 
     steps:
       - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+          # Using a Personal Access Token from an admin account to bypass branch
+          # protection rules and allow direct pushes to production. Inspirited
+          # by: https://github.com/community/community/discussions/13836
+          token: ${{ secrets.TESTS_PUSH_TO_GITHUB }}
+
       - uses: actions/setup-node@v3
         with:
           node-version: 18
@@ -160,12 +171,12 @@ jobs:
           if git diff-index --quiet HEAD --; then
             echo "Localization strings are unchanged"
           else
-            # Set the committer and author to the name and email of the person
-            # who triggered the action
-            git config --local user.name "${{ github.event.head_commit.author.name }}"
-            git config --local user.email "${{ github.event.head_commit.author.email }}"
+            git config --local user.name "github-actions"
+            git config --local user.email "41898282+github-actions[bot]@users.noreply.github.com"
+            # Set the committer to the name and email of the person who
+            # triggered the action
             git commit \
-              --author 'github-actions <41898282+github-actions[bot]@users.noreply.github.com>' \
+              --author '${{ github.event.head_commit.author.name }} <${{ github.event.head_commit.author.email }}>' \
               -m 'Sync localization strings with Weblate' \
               -m "Triggered by ${{ github.sha }} on branch ${{ github.ref }}"
             git push
@@ -178,3 +189,77 @@ jobs:
         env:
           WEBLATE_API_TOKEN: ${{ secrets.WEBLATE_API_TOKEN }}
         run: npm run validateWeblate
+
+      - name: Cleanup localization outfiles
+        working-directory: specifyweb/frontend/js_src
+        run: rm -rf ./localization-strings
+
+      - name: Get relative path of all changed files
+        working-directory: specifyweb/frontend/js_src
+        id: changed
+        run: |
+          # Strip specifyweb/frontend/js_src/ from every file name
+          changed_files=`sed 's/specifyweb\/frontend\/js_src\///g' <<< "${{ needs.changes.outputs.frontend_changes }}"`
+
+          # Convert to array
+          IFS=' ' read -r -a split_files <<< "$changed_files"
+          
+          # Exclude files that were removed
+          for file in "${split_files[@]}"; do
+              if [ ! -e "$file" ]; then
+                  echo "File was removed: $file"
+                  split_files=("${split_files[@]/$file}")
+              fi
+          done
+
+          # Convert back to string
+          changed="${files[*]}"
+          
+          echo "Changed front-end files: ${changed}"
+          echo "changed=${changed}" >> $GITHUB_OUTPUT
+
+      - name: Run ESLint auto fixes
+        # Don't run this for production branch. Reasons:
+        #  - ESLint auto fixes may break code. Don't want it to break production
+        #    code
+        #  - This would have already run on the feature branch by the time code
+        #    is pushed into production.
+        #  - There shouldn't be many direct pushes to production anyway
+        if: github.ref != format('refs/heads/{0}', github.event.repository.default_branch)
+        working-directory: specifyweb/frontend/js_src
+        run: |
+          # TODO: once most errors are fixed remove "set +e" to not ignore errors
+          set +e
+          npx eslint --fix --color `echo "${{steps.changed.outputs.changed}}" | tr " " "\n"`
+          set -e
+
+      - name: Reformat all code with Prettier
+        working-directory: specifyweb/frontend/js_src
+        run: |
+          npx prettier --write --minify `echo "${{steps.changed.outputs.changed}}" | tr " " "\n"`
+
+      - name: Commit linted files (if made any changes)
+        working-directory: specifyweb/frontend/js_src
+        # Creates a new commit. Amending an existing commit is a bad idea
+        # because:
+        # - Would require original committer to force pull. May cause merge
+        #   conflicts
+        # - Changes that require linting might have been made over several
+        #   commits. If you amend the last commit, than the fixes for all
+        #   commits would be in the last commit.
+        run: |
+          git add .
+
+          if git diff-index --quiet HEAD --; then
+            echo "Linters did not detect any issues. Good job!"
+          else
+            git config --local user.name "github-actions"
+            git config --local user.email "41898282+github-actions[bot]@users.noreply.github.com"
+            # Set the committer to the name and email of the person who
+            # triggered the action
+            git commit \
+              --author '${{ github.event.head_commit.author.name }} <${{ github.event.head_commit.author.email }}>' \
+              -m 'Lint code with ESLint and Prettier' \
+              -m "Triggered by ${{ github.sha }} on branch ${{ github.ref }}"
+            git push
+          fi
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -13,7 +13,7 @@ repos:
   - repo: local
     hooks:
       - id: frontend-tests
-        name: TypeScript and QUnit
+        name: TypeScript, Jest and localization tests
         description: Run TypeScript typechecker and automated tests on .ts files
         entry: /bin/bash -c "cd specifyweb/frontend/js_src && npm run test"
         language: script
@@ -39,6 +39,7 @@ repos:
 
   #- repo: https://github.com/pre-commit/mirrors-mypy
   #- repo: https://github.com/pre-commit/mirrors-eslint
+  #- repo: https://github.com/awebdeveloper/pre-commit-stylelint
 
   # global hooks:
   - repo: https://github.com/pre-commit/pre-commit-hooks
@@ -120,16 +121,6 @@ repos:
           - prettier-plugin-tailwind-css@^1.5.0
           - prettier-plugin-firebase-database@^1.0.1
 
-  - repo: https://github.com/awebdeveloper/pre-commit-stylelint
-    rev: 0.0.2
-    hooks:
-      - id: stylelint
-        files: \.(css|scss|sass|js|jsx|ts|tsx)$
-        additional_dependencies:
-          - stylelint@^14.1.0
-          - stylelint-config-prettier@^9.0.3
-          - stylelint-config-standard@^24.0.0
-
   - repo: https://github.com/Lucas-C/pre-commit-hooks-safety
     rev: v1.2.2
     hooks:

diff --git a/specifyweb/frontend/js_src/.eslintrc.js b/specifyweb/frontend/js_src/.eslintrc.js
diff --git a/specifyweb/frontend/js_src/.prettierignore b/specifyweb/frontend/js_src/.prettierignore
@@ -0,0 +1,4 @@
+lib/tests/fixtures/
+lib/tests/ajax/static
+lib/components/DataModel/*.js
+*.snap
diff --git a/specifyweb/frontend/js_src/README.md b/specifyweb/frontend/js_src/README.md
@@ -81,8 +81,8 @@ development. Many IDEs have plugins for closer integration.
 
   Highly customizable code linter
 
-  Config file is located in [`./.eslintrc`](./.eslintrc.js). The config file
-  get's most rules and plugins from
+  Config file is located in [`./eslint.config.js`](./eslint.config.js). The
+  config file get's most rules and plugins from
   [@maxxxxxdlp/eslint-config](https://www.npmjs.com/package/@maxxxxxdlp/eslint-config)
 
   Configure your IDE to run ESLint on the following files:
@@ -91,6 +91,12 @@ development. Many IDEs have plugins for closer integration.
   {**/*,*}.{ts,tsx,jsx,xml,json,md,css,html,yaml,yml}
   ```
 
+  > **NOTE** We are using a FlatConfig for ESLint. It is not yet supported by
+  > WebStorm ( and by extension PyCharm), but will be in the next release. Until
+  > then, a compatability layer was provided. You will need to explicitly tell
+  > your IDE to use the ./lib/scripts/eslint-compat ESLint package, rather than
+  > the one in node_modules
+
 - TypeScript
 
   Type checking

diff --git a/specifyweb/frontend/js_src/babel.config.js b/specifyweb/frontend/js_src/babel.config.js
@@ -4,20 +4,17 @@
  * Webpack uses Babel too, but that config is provided inside of webpack.config.js
  */
 
-"use strict";
+'use strict';
 
 module.exports = {
   env: {
     test: {
       presets: [
-        [
-          '@babel/preset-env',
-          {targets: {node: 'current'}},
-        ],
+        ['@babel/preset-env', { targets: { node: 'current' } }],
         ['@babel/preset-react'],
         ['@babel/preset-typescript'],
       ],
-      "plugins": ["@babel/plugin-transform-modules-commonjs"],
-    }
-  }
-}
+      plugins: ['@babel/plugin-transform-modules-commonjs'],
+    },
+  },
+};