(unsigned) feat: delete overdue draft form responses and dont nag #444

daine · 2023-07-21T19:14:32Z

Summary | Résumé

Fix for cds-snc/platform-forms-client#2338

Updates the nagware to check if the detected overdue responses are for an unpublished form, and deletes them accordingly.

Test instructions | Instructions pour tester la modification

Create overdue test responses on an unpublished form. On localhost, update

// vaultProcessing.js line 21
  try {
    fileInputPaths = extractFileInputResponses(formSubmission);

    const fiftyDaysAgo = "4320000000"; // Create overdue responses
    const createdAt = Date.now() - fiftyDaysAgo; // Create overdue responses

    await copyFilesFromReliabilityToVaultStorage(fileInputPaths);
    await saveToVault(
      submissionID,
      formSubmission.responses,
      formID,
      language,
      createdAt,
      securityAttribute
    );
  } catch (error) {

Run ./invoke_nagware.js
All test responses, overdue or not, should have been deleted

Tested with:

Overdue draft form responses (deletes form responses, skips nag)
Overdue published form responses (does not delete, nags instead)

craigzour · 2023-07-24T14:09:23Z

@daine Now that you have implemented a mechanism to delete responses before publishing forms I am not quite sure why we have to handle any deletion on the Nagware infra side.
I thought the work was just about skipping form owners nagging if there was overdue responses on a form that was still in draft mode.
To me it makes more sense to do it this way but maybe I am missing something?

cc @timarney @falila

daine · 2023-07-24T15:15:19Z

@daine Now that you have implemented a mechanism to delete responses before publishing forms I am not quite sure why we have to handle any deletion on the Nagware infra side. I thought the work was just about skipping form owners nagging if there was overdue responses on a form that was still in draft mode. To me it makes more sense to do it this way but maybe I am missing something?

cc @timarney @falila

The use case here I believe is to handle people who never publish the form (as per Brian's design and response on this issue). It's the second scenario

falila · 2023-07-24T16:12:55Z

@daine Now that you have implemented a mechanism to delete responses before publishing forms I am not quite sure why we have to handle any deletion on the Nagware infra side. I thought the work was just about skipping form owners nagging if there was overdue responses on a form that was still in draft mode. To me it makes more sense to do it this way but maybe I am missing something?
cc @timarney @falila

The use case here I believe is to handle people who never publish the form (as per Brian's design and response on this issue). It's the second scenario

The propose solution appears satisfactory concerning the deletion of test responses for drafted forms. It avoids introducing any unnecessary attributes and instead utilizes the existing template property, which is more appropriate.
Nice work!

aws/app/lambda/nagware/nagware.js

daine · 2023-07-24T17:17:45Z

aws/app/lambda/nagware/lib/templates.js


  if (!process.env.AWS_SAM_LOCAL) {
    const jsonConfig = JSON.parse(firstRecord[2].stringValue.trim(1, -1)) || undefined;
    formName = firstRecord[1].stringValue !== "" ? firstRecord[1].stringValue : `${jsonConfig.titleEn} - ${jsonConfig.titleFr}`;
+    // TODO: Not sure what field the isPublished value is in, but based on the SQL query, it should be in the 4th index


@craigzour this is a guess for me right now, I could use your guidance since I'm unable to test it

firstRecord[3] sounds about right.

Yep I agree. You can delete the comment ;)

craigzour

We are almost good to go :)

aws/app/lambda/nagware/nagware.js

craigzour · 2023-07-24T19:10:52Z

aws/app/lambda/nagware/lib/templates.js


  if (!process.env.AWS_SAM_LOCAL) {
    const jsonConfig = JSON.parse(firstRecord[2].stringValue.trim(1, -1)) || undefined;
    formName = firstRecord[1].stringValue !== "" ? firstRecord[1].stringValue : `${jsonConfig.titleEn} - ${jsonConfig.titleFr}`;
+    // TODO: Not sure what field the isPublished value is in, but based on the SQL query, it should be in the 4th index


Yep I agree. You can delete the comment ;)

craigzour

LGTM :) Thank you!

falila

🌮

Signed-off-by: Daine Trinidad <[email protected]>

github-actions · 2023-07-24T22:48:31Z

⚠ Terrform update available

Terraform: 1.5.3 (using 1.4.2)
Terragrunt: 0.48.4 (using 0.46.3)

github-actions · 2023-07-24T22:53:30Z

Staging: app

✅ Terraform Init: success
✅ Terraform Validate: success
✅ Terraform Format: success
✅ Terraform Plan: success
✅ Conftest: success

⚠️ Warning: resources will be destroyed by this change!

Plan: 2 to add, 1 to change, 2 to destroy

Show summary

CHANGE	NAME
update	`aws_lambda_function.nagware`
recreate	`aws_lambda_layer_version.nagware_lib`
	`aws_lambda_layer_version.nagware_nodejs`

Show plan

Resource actions are indicated with the following symbols:
  ~ update in-place
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # aws_lambda_function.nagware will be updated in-place
  ~ resource "aws_lambda_function" "nagware" {
        id                             = "Nagware"
      ~ last_modified                  = "2023-07-21T12:42:28.000+0000" -> (known after apply)
      ~ layers                         = [
          - "arn:aws:lambda:ca-central-1:687401027353:layer:nagware_lib_packages:8",
          - "arn:aws:lambda:ca-central-1:687401027353:layer:nagware_node_packages:15",
        ] -> (known after apply)
      ~ source_code_hash               = "y34UuQHJpw45wPAaGnYetfK3Np4OClJnnkY9F5bpN7I=" -> "iEcgvvidpFH4l1y2zkp2weM9newMLuhW5SACT1opKjk="
        tags                           = {
            "CostCentre" = "forms-platform-staging"
            "Terraform"  = "true"
        }
        # (17 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

  # aws_lambda_layer_version.nagware_lib must be replaced
-/+ resource "aws_lambda_layer_version" "nagware_lib" {
      ~ arn                         = "arn:aws:lambda:ca-central-1:687401027353:layer:nagware_lib_packages:8" -> (known after apply)
      - compatible_architectures    = [] -> null
      ~ created_date                = "2023-07-21T12:42:08.652+0000" -> (known after apply)
      ~ id                          = "arn:aws:lambda:ca-central-1:687401027353:layer:nagware_lib_packages:8" -> (known after apply)
      ~ layer_arn                   = "arn:aws:lambda:ca-central-1:687401027353:layer:nagware_lib_packages" -> (known after apply)
      + signing_job_arn             = (known after apply)
      + signing_profile_version_arn = (known after apply)
      ~ source_code_hash            = "Yt+PCj+su2PKY5jmYB/8Klpw1hhZMhvzkQjhfplPFSk=" -> "aMoOyf/2lLfJkqzVR1L+oPscSFKywcFmQTPl1evU0IY=" # forces replacement
      ~ source_code_size            = 3814 -> (known after apply)
      ~ version                     = "8" -> (known after apply)
        # (4 unchanged attributes hidden)
    }

  # aws_lambda_layer_version.nagware_nodejs must be replaced
-/+ resource "aws_lambda_layer_version" "nagware_nodejs" {
      ~ arn                         = "arn:aws:lambda:ca-central-1:687401027353:layer:nagware_node_packages:15" -> (known after apply)
      - compatible_architectures    = [] -> null
      ~ created_date                = "2023-07-21T12:42:15.055+0000" -> (known after apply)
      ~ id                          = "arn:aws:lambda:ca-central-1:687401027353:layer:nagware_node_packages:15" -> (known after apply)
      ~ layer_arn                   = "arn:aws:lambda:ca-central-1:687401027353:layer:nagware_node_packages" -> (known after apply)
      + signing_job_arn             = (known after apply)
      + signing_profile_version_arn = (known after apply)
      ~ source_code_hash            = "gqUAucq28GdqL2sbVq4aA+ZEnm+/oXU6jRvkkBKRx4o=" -> "Ggli/WoTw6SVanhWXvhR4WQDQ7Mv+fS3slM292yTyXo=" # forces replacement
      ~ source_code_size            = 3676202 -> (known after apply)
      ~ version                     = "15" -> (known after apply)
        # (4 unchanged attributes hidden)
    }

Plan: 2 to add, 1 to change, 2 to destroy.

Warning: Argument is deprecated

  with aws_s3_bucket.reliability_file_storage,
  on s3.tf line 4, in resource "aws_s3_bucket" "reliability_file_storage":
   4: resource "aws_s3_bucket" "reliability_file_storage" {

Use the aws_s3_bucket_lifecycle_configuration resource instead

(and 17 more similar warnings elsewhere)

─────────────────────────────────────────────────────────────────────────────

Saved the plan to: plan.tfplan

To perform exactly these actions, run the following command to apply:
    terraform apply "plan.tfplan"
Releasing state lock. This may take a few moments...

Show Conftest results

WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_event_rule.cron_2am_every_day"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_event_rule.cron_3am_every_day"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_event_rule.cron_4am_every_day"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_event_rule.cron_5am_every_business_day"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.archive_form_templates"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.archiver"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.audit_logs"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.dead_letter_queue_consumer"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.nagware"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.reliability"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.submission"]

28 tests, 17 passed, 11 warnings, 0 failures, 0 exceptions

daine · 2023-07-26T16:38:07Z

Note: I opened a new PR (#448) instead with signed commits. I signed the commits by doing a rebase since I added my new GPG key after already doing the commit
git rebase --exec 'git commit --amend --no-edit -n -S' -i develop

daine requested review from craigzour, falila and bryan-robitaille July 21, 2023 19:47

daine marked this pull request as ready for review July 21, 2023 20:05

daine enabled auto-merge (squash) July 21, 2023 20:08

craigzour requested changes Jul 24, 2023

View reviewed changes

aws/app/lambda/nagware/nagware.js Show resolved Hide resolved

aws/app/lambda/nagware/nagware.js Show resolved Hide resolved

daine commented Jul 24, 2023

View reviewed changes

craigzour requested changes Jul 24, 2023

View reviewed changes

craigzour approved these changes Jul 24, 2023

View reviewed changes

falila approved these changes Jul 24, 2023

View reviewed changes

daine added 9 commits July 24, 2023 15:47

feat: delete overdue draft form responses and dont nag

0747104

Signed-off-by: Daine Trinidad <[email protected]>

chore: reverting some changes for lockfile

3b8cc25

Signed-off-by: Daine Trinidad <[email protected]>

chore: some cleanup

35511e2

Signed-off-by: Daine Trinidad <[email protected]>

chore: re-adding template file that got lost during merge

eaf9fb5

Signed-off-by: Daine Trinidad <[email protected]>

chore: removing file again for cleaner diff and history

a9cf71f

Signed-off-by: Daine Trinidad <[email protected]>

chore: fixed file refactor

64d3e8a

Signed-off-by: Daine Trinidad <[email protected]>

fix: refactor missed the terraform file; fixed the new name for the file

f032c1a

Signed-off-by: Daine Trinidad <[email protected]>

fix: horrible typo, missing 's'

eed47d5

Signed-off-by: Daine Trinidad <[email protected]>

fix: move var declaration inside try catch & comment cleanup

d1704c7

Signed-off-by: Daine Trinidad <[email protected]>

daine force-pushed the feat/delete-overdue-draft-form-responses branch from be381d1 to d1704c7 Compare July 24, 2023 22:48

daine mentioned this pull request Jul 26, 2023

feat: delete overdue draft form responses and dont nag #448

Closed

daine changed the title ~~feat: delete overdue draft form responses and dont nag~~ (unsigned) feat: delete overdue draft form responses and dont nag Jul 26, 2023

daine closed this Jul 26, 2023

auto-merge was automatically disabled July 26, 2023 17:02
Pull request was closed

daine mentioned this pull request Jul 26, 2023

Feat/dontnagtestresponse #449

Merged

daine deleted the feat/delete-overdue-draft-form-responses branch July 26, 2023 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(unsigned) feat: delete overdue draft form responses and dont nag #444

(unsigned) feat: delete overdue draft form responses and dont nag #444

daine commented Jul 21, 2023 •

edited

Loading

craigzour commented Jul 24, 2023

daine commented Jul 24, 2023

falila commented Jul 24, 2023

daine Jul 24, 2023

falila Jul 24, 2023

craigzour Jul 24, 2023

craigzour left a comment

craigzour Jul 24, 2023

craigzour left a comment

falila left a comment

github-actions bot commented Jul 24, 2023

github-actions bot commented Jul 24, 2023

daine commented Jul 26, 2023

(unsigned) feat: delete overdue draft form responses and dont nag #444

(unsigned) feat: delete overdue draft form responses and dont nag #444

Conversation

daine commented Jul 21, 2023 • edited Loading

Summary | Résumé

Test instructions | Instructions pour tester la modification

craigzour commented Jul 24, 2023

daine commented Jul 24, 2023

falila commented Jul 24, 2023

daine Jul 24, 2023

Choose a reason for hiding this comment

falila Jul 24, 2023

Choose a reason for hiding this comment

craigzour Jul 24, 2023

Choose a reason for hiding this comment

craigzour left a comment

Choose a reason for hiding this comment

craigzour Jul 24, 2023

Choose a reason for hiding this comment

craigzour left a comment

Choose a reason for hiding this comment

falila left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 24, 2023

⚠ Terrform update available

github-actions bot commented Jul 24, 2023

Staging: app

daine commented Jul 26, 2023

daine commented Jul 21, 2023 •

edited

Loading