QA reporting enhancements

.github/workflows/publish-qa-results.yml

@@ -23,7 +23,7 @@ on:
         required: true
       pr-number:
         type: string
-        required: false
+        required: true
       write-summary:
         type: boolean
         default: true
@@ -54,6 +54,8 @@ jobs:
           cat >> $REPORT_FILE << 'ENDOFREPORT'
           ## QA Summary
 
+          _[See our documentation for tips on how to resolve failing QA checks.](${{ env.GH_SERVER}}/${{ env.GH_REPO }}/blob/main/docs/resolving-qa-failures.md)_
+
           | QA Check        | Result  |
           |:----------------|:-------:|
           | 🌐 Web Tests | ${{ (env.WEB_TEST_OUTCOME == 'success' && '✅') || (env.WEB_TEST_OUTCOME == 'skipped' && '➖') || '❌' }} |
@@ -101,19 +103,16 @@ jobs:
           echo "REPORT_CONTENT<<ENDOFREPORT" >> $GITHUB_OUTPUT
           echo "$CONTENT" >> $GITHUB_OUTPUT
           echo "ENDOFREPORT" >> $GITHUB_OUTPUT
-      - name: Warn on missing comment requirements
-        if: inputs.write-comment && inputs.pr-number == ''
-        run: "echo 'WARNING: Cannot write a comment because pr-number is not set'"
       - name: Find previous report comment
         id: find-comment
-        if: inputs.write-comment && inputs.pr-number != ''
+        if: inputs.write-comment
         uses: peter-evans/find-comment@a54c31d7fa095754bfef525c0c8e5e5674c4b4b1 # v2.4.0
         with:
           issue-number: ${{ inputs.pr-number }}
           comment-author: 'github-actions[bot]'
           body-includes: QA Summary
       - name: Create or update comment
-        if: inputs.write-comment && inputs.pr-number != ''
+        if: inputs.write-comment
         uses: peter-evans/create-or-update-comment@23ff15729ef2fc348714a3bb66d2f655ca9066f2 # v3.1.0
         with:
           comment-id: ${{ steps.find-comment.outputs.comment-id }}

.github/workflows/qa.yml

@@ -167,12 +167,10 @@ jobs:
         env:
           CI: 1
       - name: Run linter for api side
-        continue-on-error: true
-        run: |
-          yarn eslint api/src
+        run: yarn eslint api/src
       - name: Run linter for web side
-        run: |
-          yarn eslint web/src
+        if: ${{ !cancelled() }}
+        run: yarn eslint web/src
 
   tflint:
     name: Lint terraform
@@ -190,7 +188,7 @@ jobs:
       - uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84 # v3.3.2
         name: Cache plugin dir
         with:
-          path: .tflint.d/plugins
+          path: ~/.tflint.d/plugins
           key: ${{ runner.os }}-tflint-${{ hashFiles('terraform/.tflint.hcl') }}
       - uses: terraform-linters/setup-tflint@19a52fbac37dacb22a09518e4ef6ee234f2d4987 # v4.0.0
         name: Setup TFLint
@@ -205,4 +203,5 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ github.token }}
       - name: Run TFLint
+        working-directory: terraform
         run: tflint --format compact --recursive --minimum-failure-severity=error

api/src/lib/aws.ts

@@ -12,6 +12,7 @@ import {
   SQSClient,
 } from '@aws-sdk/client-sqs'
 import { getSignedUrl as awsGetSignedUrl } from '@aws-sdk/s3-request-presigner'
+import { StreamingBlobPayloadInputTypes } from '@smithy/types'
 import { QueryResolvers, CreateUploadInput } from 'types/graphql'
 
 const CPF_REPORTER_BUCKET_NAME = `cpf-reporter-${process.env.environment}`
@@ -61,7 +62,7 @@ function getS3Client() {
 export function uploadWorkbook(
   upload: CreateUploadInput,
   uploadId: number,
-  body: any
+  body: StreamingBlobPayloadInputTypes
 ) {
   const folderName = `${upload.organizationId}/${upload.agencyId}/${upload.reportingPeriodId}/uploads/${upload.expenditureCategoryId}/${uploadId}/${upload.filename}`
   return sendPutObjectToS3Bucket(CPF_REPORTER_BUCKET_NAME, folderName, body)
@@ -70,8 +71,8 @@ export function uploadWorkbook(
 async function sendPutObjectToS3Bucket(
   bucketName: string,
   key: string,
-  body: any
-) {
+  body: StreamingBlobPayloadInputTypes
+): Promise<void> {
   const s3 = getS3Client()
   const uploadParams: PutObjectCommandInput = {
     Bucket: bucketName,
@@ -115,12 +116,11 @@ export async function s3PutSignedUrl(
   })
   return url
 }
+
 /**
  *  This function is a wrapper around the getSignedUrl function from the @aws-sdk/s3-request-presigner package.
  *  Exists to organize the imports and to make it easier to mock in tests.
  */
-
-// eslint-disable-next-line @typescript-eslint/no-unused-vars
 async function getSignedUrl(bucketName: string, key: string) {
   const s3 = getS3Client()
   const baseParams = { Bucket: bucketName, Key: key }
@@ -143,8 +143,7 @@ function getSQSClient() {
   return sqs
 }
 
-// eslint-disable-next-line @typescript-eslint/no-unused-vars
-async function sendSqsMessage(queueUrl: string, messageBody: any) {
+async function sendSqsMessage(queueUrl: string, messageBody: unknown) {
   const sqs = getSQSClient()
   await sqs.send(
     new SendMessageCommand({
@@ -154,7 +153,6 @@ async function sendSqsMessage(queueUrl: string, messageBody: any) {
   )
 }
 
-// eslint-disable-next-line @typescript-eslint/no-unused-vars
 async function receiveSqsMessage(queueUrl: string) {
   const sqs = getSQSClient()
   // const receiveResp = await sqs.send(new ReceiveMessageCommand({

docs/resolving-qa-failures.md

@@ -0,0 +1,66 @@
+# Tips for Resolving QA Check Failures
+
+The QA checks that run against open pull requests exist to help ensure that changes submitted in PRs are working properly and are consistent with our adopted best practices.
+If you have encountered a QA check failure and are unsure of how to resolve it, then this guide is for you!
+
+As always, you can reach out to the Grants team for help if you are still not sure about the best way to resolve a particular problem.
+
+## Linting Failures
+
+> [!TIP]
+> All ESLint and TFLint findings will result in a check annotation on the relevent line(s) in your pull request's code.
+> You can review them from the "Files changed" view for your PR.
+
+### ESLint
+
+The [ESLint](https://eslint.org/) QA check determines whether JavaScript and/or TypeScript files beneath the `api/src` and `web/src` directories are formatted consistently with the rules configured for this project.
+Some issues that ESLint reports are just warnings; we recommend that you fix these if you can,
+but they will not prevent you from merging your pull request (and they do cause the QA check to fail).
+Other ESLint findings are considered errors and are requirements for merging your pull request.
+
+To fix issues reported by ESLint, first review the failures by running `yarn eslint api/src web/src` from the root of this repository, which will output all warnings and errors.
+Many ESLint issues can be fixed automatically by adding the `--fix` flag to this command, but you can also resolve them manually if you prefer.
+
+Sometimes, an ESLint finding is not valid for a particular reason -
+maybe you need a nonstandard import behavior, or an import is necessary for its side-effects.
+In these cases, specific findings can be ignored.
+Check the [ESLint documentation](https://eslint.org/docs/latest/use/configure/rules) for the most appropriate way to ignore the finding.
+
+### TFlint
+
+The [TFLint](https://github.com/terraform-linters/tflint) QA check determines whether Terraform files beneath the `terraform/` directory are using best practices and/or contain potential errors.
+These checks can often help address problems that can cause a failure during `terraform apply`
+(but may not be found during `terraform plan`),
+which makes it especially useful for avoiding failed deployments after a PR is merged.
+
+To run TFlint locally, you might need to [install](https://github.com/terraform-linters/tflint?tab=readme-ov-file#installation) it first.
+Once installed, navigate to the `terraform/` directory of the repository and run `tflint --init`
+to ensure that the plugins configured in the [`.tflint.hcl` file](https://github.com/usdigitalresponse/cpf-reporter/blob/main/terraform/.tflint.hcl) are up-to-date. Then, run the linter checks by running
+`tflint --recursive --minimum-failure-severity=error` to view the problems.
+Many TFLint issues can be fixed automatically by adding the `--fix` flag to this command, but you can also resolve them manually if you prefer.
+
+Sometimes, a TFLint finding is not valid for a particular reason.
+Or, despite being valid, the team has agreed that it may be bypassed after confirming that doing so will not cause an immediate problem.
+In these cases, a `tflint-ignore` pragma comment may be used as an annotation in order to avoid a specific rule violation,
+or a `rule` block may be added to `terraform/.tflint.hcl` to ignore the linting rule entirely.
+Most TFlint errors will include a reference link that provides justification for the rule's importance and ways to resolve.
+
+For example, if TFLint fails with an error like this:
+
+```
+Warning: variable "not_used" is declared but not used (terraform_unused_declarations)
+
+  on config.tf line 1:
+   1: variable "not_used" {
+
+Reference: https://github.com/terraform-linters/tflint-ruleset-terraform/blob/v0.1.0/docs/rules/terraform_unused_declarations.md
+```
+
+the `terraform_unused_declarations` rule can be disabled only for the `not_used` variable by adding a pragma comment above the variable declaration:
+```terraform
+# tflint-ignore: terraform_unused_declarations
+variable "not_used" {
+  type        = string
+  description = "This variable is never used."
+}
+```

terraform/.tflint.hcl

@@ -5,6 +5,6 @@ plugin "terraform" {
 
 plugin "aws" {
   enabled = true
-  version = "0.22.1"
+  version = "0.28.0"
   source  = "github.com/terraform-linters/tflint-ruleset-aws"
 }