Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[python][treasury-report] add force-regenerate parameter to allow for deletion of the existing treasury file before generation #515

Merged
merged 16 commits into from
Dec 9, 2024

Conversation

nowei
Copy link
Contributor

@nowei nowei commented Nov 21, 2024

Backend changes for adding a force-regeneration parameter to the treasury report generation lambda.

To be merged after the mypy changes in #506 to avoid potential merge conflicts

@nowei nowei requested a review from as1729 November 21, 2024 06:59
Copy link

github-actions bot commented Nov 21, 2024

QA Summary

Pusher: @nowei, Action: pull_request_target, Workflow: Continuous Integration

See our documentation for tips on how to resolve failing QA checks.

QA Check Result
🌐 Web Tests
🔗 API Tests
🐍 Python Tests
📏 ESLint
🧼 Ruff
🛁 mypy
🧹 TFLint

Test Coverage

View the workflow summary for individual coverage reports if this comment is truncated.

Coverage report for api suite
St File % Stmts % Branch % Funcs % Lines Uncovered Line #s
🟡 All files 51.21 33.91 57.23 51.68
🔴  src 0 100 0 0
🔴   server.ts 0 100 0 0 6-13
🟢  src/directives/requireAuth 100 100 100 100
🟢   requireAuth.ts 100 100 100 100
🟡  src/directives/skipAuth 50 100 0 50
🟡   skipAuth.ts 50 100 0 50 13
🔴  src/functions 0 100 0 0
🔴   graphql.ts 0 100 0 0 15-27
🔴  src/functions/processValidationJson 36.36 50 40 35.63
🟢   processValidationJson.scenarios.ts 100 100 100 100
🔴   processValidationJson.ts 35.63 50 40 34.88 59-98,118-119,157-165,177-178,193-196,201,213-214,222-343,355-358,366
🔴  src/lib 13.12 9.57 11.7 13.43
🟡   auth.ts 62.96 48.48 57.14 65.38 60-61,77-78,84-85,101-102,124,131,134,139-146,170,174
🔴   aws.ts 25.42 18.75 25 25.42 53-58,74-97,121-123,150-171,186-272
🟢   constants.ts 100 100 100 100
🔴   db.ts 45.45 50 50 45.45 15-35,41,43,50
🔴   ec-codes.ts 0 100 100 0 1
🟢   logger.ts 100 100 100 100
🔴   persist-upload.js 0 0 0 0 16-295
🔴   preconditions.ts 0 0 0 0 2-3
🔴   records.js 0 0 0 0 12-214
🔴   templateRules.ts 0 0 0 0
🔴   tracer.ts 0 100 100 0 5-14
🔴   validate-upload.js 0 0 0 0 18-790
🟢   validation-error.ts 83.33 100 50 83.33 22
🔴   validation-rules.js 0 0 0 0 6-194
🟡  src/services/agencies 67.34 50 80 67.34
🟢   agencies.scenarios.ts 100 100 100 100
🟡   agencies.ts 65.21 50 75 65.21 40-51,60-64,97-98,104,113-121
🟡  src/services/expenditureCategories 78.57 66.66 88.88 78.57
🟢   expenditureCategories.scenarios.ts 100 100 100 100
🟡   expenditureCategories.ts 77.77 66.66 88.88 77.77 30-34,49-52,60,91
🟡  src/services/inputTemplates 77.77 66.66 85.71 77.77
🟢   inputTemplates.scenarios.ts 100 100 100 100
🟡   inputTemplates.ts 76.92 66.66 85.71 76.92 25-29,39-40,50,85
🟡  src/services/organizations 75 90.9 50 75
🟢   organizations.scenarios.ts 100 100 100 100
🟡   organizations.ts 73.97 90.9 44.44 73.97 34-35,53-57,92,164-194,202,220-247
🟢  src/services/outputTemplates 82.85 66.66 85.71 82.85
🟢   outputTemplates.scenarios.ts 100 100 100 100
🟢   outputTemplates.ts 82.35 66.66 85.71 82.35 26-30,40-41,51,114
🟡  src/services/passage 74.07 62.5 100 74.07
🟡   passage.ts 74.07 62.5 100 74.07 18-19,65-76
🟡  src/services/projects 80 100 62.5 80
🟢   projects.scenarios.ts 100 100 100 100
🟡   projects.ts 78.57 100 62.5 78.57 45-51
🟢  src/services/reportingPeriodCertifications 100 100 100 100
🟢   reportingPeriodCertifications.scenarios.ts 100 100 100 100
🟢   reportingPeriodCertifications.ts 100 100 100 100
🟡  src/services/reportingPeriods 70.58 60 57.89 71.64
🟢   reportingPeriods.scenarios.ts 100 100 100 100
🟡   reportingPeriods.ts 68.75 60 50 69.84 15-27,44-48,58-59,74-77,93,116,124,165,188-209
🟢  src/services/subrecipientUploads 88.88 83.33 85.71 88.88
🟢   subrecipientUploads.scenarios.ts 100 100 100 100
🟢   subrecipientUploads.ts 86.36 83.33 80 86.36 64,94-99
🟢  src/services/subrecipients 90.19 88.88 92.3 90.19
🟢   subrecipients.scenarios.ts 100 100 100 100
🟢   subrecipients.ts 86.11 88.88 83.33 86.11 63,101-102,108-113
🟡  src/services/uploadValidations 57.14 100 14.28 57.14
🟢   uploadValidations.scenarios.ts 100 100 100 100
🟡   uploadValidations.ts 53.84 100 14.28 53.84 10,16,30,38,45-48
🟢  src/services/uploads 92.24 76.92 89.13 92.24
🟢   uploads.scenarios.ts 100 100 100 100
🟢   uploads.ts 89.88 76.92 76.19 89.88 40,108,136-150,416-417
🟢  src/services/users 85.61 82 88.88 85.61
🟢   users.scenarios.ts 100 100 100 100
🟢   users.ts 84.61 82 84.21 84.61 220,237,253,275-277,286-290,308-309,323-326,344-346,354-355,360,369-375
🟢  src/services/validationRuleses 85.71 100 71.42 85.71
🟢   validationRuleses.scenarios.ts 100 100 100 100
🟢   validationRuleses.ts 84.61 100 71.42 84.61 43-48
Coverage report for web suite
St File % Stmts % Branch % Funcs % Lines Uncovered Line #s
🔴 All files 17.43 21.91 14.85 17.04
🟢  api/src/lib 100 100 100 100
🟢   constants.ts 100 100 100 100
🔴  web/src 28.57 18.75 66.66 28.57
🔴   App.tsx 0 0 0 0 3-36
🟢   Routes.tsx 100 100 100 100
🟡   auth.ts 50 50 100 50 19-24
🔴   entry.client.tsx 0 0 100 0 10-22
🔴  web/src/auth 7.14 0 4.16 7.14
🔴   localAuth.ts 9.09 0 8.33 9.09 39-68,76-80
🔴   passageAuth.ts 5 0 0 5 22-25,31-60
🔴  web/src/components/Agency/Agencies 0 100 0 0
🔴   Agencies.tsx 0 100 0 0 9-21
🔴  web/src/components/Agency/AgenciesCell 0 100 0 0
🔴   AgenciesCell.tsx 0 100 0 0 8-39
🔴  web/src/components/Agency/Agency 0 0 0 0
🔴   Agency.tsx 0 0 0 0 10-78
🔴  web/src/components/Agency/AgencyCell 0 100 0 0
🔴   AgencyCell.tsx 0 100 0 0 7-27
🔴  web/src/components/Agency/AgencyForm 0 0 0 0
🔴   AgencyForm.tsx 0 0 0 0 25-45
🔴  web/src/components/Agency/EditAgencyCell 0 100 0 0
🔴   EditAgencyCell.tsx 0 100 0 0 10-59
🔴  web/src/components/Agency/NewAgency 0 100 0 0
🔴   NewAgency.tsx 0 100 0 0 9-35
🟢  web/src/components/Navigation 100 60 100 100
🟢   Navigation.tsx 100 60 100 100 24-68
🔴  web/src/components/Organization/EditOrganizationCell 0 100 0 0
🔴   EditOrganizationCell.tsx 0 100 0 0 13-64
🔴  web/src/components/Organization/EditOrganizationForm 0 0 0 0
🔴   EditOrganizationForm.tsx 0 0 0 0 27-41
🔴  web/src/components/Organization/NewOrganization 0 100 0 0
🔴   NewOrganization.tsx 0 100 0 0 9-37
🔴  web/src/components/Organization/NewOrganizationForm 0 0 0 0
🔴   NewOrganizationForm.tsx 0 0 0 0 25-54
🔴  web/src/components/Organization/Organization 0 0 0 0
🔴   Organization.tsx 0 0 0 0 10-70
🔴  web/src/components/Organization/OrganizationCell 0 100 0 0
🔴   OrganizationCell.tsx 0 100 0 0 7-28
🔴  web/src/components/Organization/OrganizationPickListsCell 40 0 27.27 36.36
🟡   OrganizationPickListsCell.mock.ts 50 100 0 100
🔴   OrganizationPickListsCell.stories.tsx 0 0 0 0 6-32
🟡   OrganizationPickListsCell.tsx 64.28 0 50 58.33 14-16,50-76
🔴  web/src/components/Organization/Organizations 0 100 0 0
🔴   Organizations.tsx 0 100 0 0 9-21
🔴  web/src/components/Organization/OrganizationsCell 0 100 0 0
🔴   OrganizationsCell.tsx 0 100 0 0 8-37
🔴  web/src/components/OutputTemplate/EditOutputTemplateCell 0 100 0 0
🔴   EditOutputTemplateCell.tsx 0 100 0 0 18-81
🔴  web/src/components/OutputTemplate/NewOutputTemplate 0 0 0 0
🔴   NewOutputTemplate.tsx 0 0 0 0 17-126
🔴  web/src/components/OutputTemplate/OutputTemplate 0 0 0 0
🔴   OutputTemplate.tsx 0 0 0 0 17-97
🔴  web/src/components/OutputTemplate/OutputTemplateCell 0 100 0 0
🔴   OutputTemplateCell.tsx 0 100 0 0 17-47
🔴  web/src/components/OutputTemplate/OutputTemplateForm 0 0 0 0
🔴   OutputTemplateForm.tsx 0 0 0 0 18-63
🔴  web/src/components/OutputTemplate/OutputTemplates 0 0 0 0
🔴   OutputTemplates.tsx 0 0 0 0 18-94
🔴  web/src/components/OutputTemplate/OutputTemplatesCell 0 100 0 0
🔴   OutputTemplatesCell.tsx 0 100 0 0 18-52
🔴  web/src/components/ReportingPeriod/EditReportingPeriodCell 0 100 0 0
🔴   EditReportingPeriodCell.tsx 0 100 0 0 13-74
🔴  web/src/components/ReportingPeriod/NewReportingPeriod 0 100 0 0
🔴   NewReportingPeriod.tsx 0 100 0 0 9-35
🔴  web/src/components/ReportingPeriod/ReportingPeriod 0 0 0 0
🔴   ReportingPeriod.tsx 0 0 0 0 12-101
🔴  web/src/components/ReportingPeriod/ReportingPeriodCell 0 100 0 0
🔴   ReportingPeriodCell.tsx 0 100 0 0 7-33
🔴  web/src/components/ReportingPeriod/ReportingPeriodForm 0 0 0 0
🔴   ReportingPeriodForm.tsx 0 0 0 0 18-43
🟡  web/src/components/ReportingPeriod/ReportingPeriods 71.42 38.46 55.55 71.42
🟡   ReportingPeriods.tsx 67.74 44.44 41.66 67.74 47-52,59-60,66,81,116-133
🟢   columns.tsx 81.81 25 83.33 81.81 36-40
🟡  web/src/components/ReportingPeriod/ReportingPeriodsCell 55 0 55.55 47.05
🟢   ReportingPeriodsCell.mock.ts 100 100 100 100
🔴   ReportingPeriodsCell.stories.tsx 0 0 0 0 6-32
🟢   ReportingPeriodsCell.tsx 100 100 100 100
🔴  web/src/components/Subrecipient/SubrecipientTableUploadLinksDisplay 0 0 0 0
🔴   SubrecipientTableUploadLinksDisplay.stories.tsx 0 100 100 0 5-82
🔴   SubrecipientTableUploadLinksDisplay.tsx 0 0 0 0 14-42
🔴  web/src/components/Subrecipient/Subrecipients 0 0 0 0
🔴   Subrecipients.tsx 0 100 0 0 5-8
🔴   columns.tsx 0 0 0 0 7-93
🔴  web/src/components/Subrecipient/SubrecipientsCell 0 100 0 0
🔴   SubrecipientsCell.tsx 0 100 0 0 7-60
🟢  web/src/components/TableBuilder 83.72 72 78.94 82.92
🟡   DebouncedInput.tsx 80 100 66.66 77.77 21,32
🟡   Filter.tsx 75 100 50 75 10
🟡   TableBuilder.tsx 73.33 40 80 71.42 40-42,50
🟢   TableHeader.tsx 100 91.66 100 100 13
🟢   TableRow.tsx 100 100 100 100
🟡  web/src/components/TemplateUploadReportingPeriodCell 55 0 55.55 47.05
🟢   TemplateUploadReportingPeriodCell.mock.ts 100 100 100 100
🔴   TemplateUploadReportingPeriodCell.stories.tsx 0 0 0 0 11-37
🟢   TemplateUploadReportingPeriodCell.tsx 100 100 100 100
🔴  web/src/components/TreasuryGeneration/DownloadTreasuryFiles 0 0 0 0
🔴   DownloadTreasuryFiles.tsx 0 0 0 0 7-68
🔴  web/src/components/TreasuryGeneration/NewTreasuryGeneration 0 100 0 0
🔴   NewTreasuryGeneration.tsx 0 100 0 0 8-39
🔴  web/src/components/TreasuryGeneration/NewTreasuryGenerationForm 0 0 0 0
🔴   NewTreasuryGenerationForm.tsx 0 0 0 0 20-31
🔴  web/src/components/Upload/EditUploadCell 0 100 0 0
🔴   EditUploadCell.tsx 0 100 0 0 10-66
🔴  web/src/components/Upload/NewUpload 0 100 0 0
🔴   NewUpload.tsx 0 100 0 0 7-35
🔴  web/src/components/Upload/Upload 0 0 0 0
🔴   Upload.stories.tsx 0 100 100 0 5-93
🔴   Upload.tsx 0 0 0 0 16-119
🔴  web/src/components/Upload/UploadCell 0 100 0 0
🔴   UploadCell.tsx 0 100 0 0 7-60
🔴  web/src/components/Upload/UploadForm 0 0 0 0
🔴   UploadForm.tsx 0 0 0 0 23-108
🔴  web/src/components/Upload/UploadValidationButtonGroup 0 0 0 0
🔴   UploadValidationButtonGroup.stories.tsx 0 100 0 0 [5-47](https://github.com/usdigitalresponse/cpf-reporter/blob/37457e47d31e4e9de8622f9cbb8b6efd3f2914c1/web/src/components/Upload/UploadV...*[Comment body truncated]*

@nowei
Copy link
Contributor Author

nowei commented Nov 21, 2024

@as1729 I think this was the intent of #510; but let me know if some additional files should also be deleted.

I'm not sure about how to make the FE changes to add a button or an option for this because I haven't tried launching the FE before 😅 But this should be the BE changes required (assuming I understood the issue correctly).

Copy link

github-actions bot commented Nov 21, 2024

Terraform Summary

Pusher: @nowei, Action: pull_request_target, Workflow: Continuous Integration

Step Result
🖌 Terraform Format & Style
⚙️ Terraform Initialization
🤖 Terraform Validation
📖 Terraform Plan

Hint: If "Terraform Format & Style" failed, run terraform fmt -recursive from the terraform/ directory and commit the results.

Output

Validation Output
Success! The configuration is valid.


Plan Summary
CHANGE RESOURCE
add aws_s3_object.origin_dist_artifact["static/css/app.1b695cd7.css"]
aws_s3_object.origin_dist_artifact["static/js/180.d4d84559.chunk.js"]
aws_s3_object.origin_dist_artifact["static/js/180.d4d84559.chunk.js.LICENSE.txt"]
aws_s3_object.origin_dist_artifact["static/js/UploadUploadsPage.8e6a08a2.chunk.js"]
aws_s3_object.origin_dist_artifact["static/js/UploadUploadsPage.8e6a08a2.chunk.js.LICENSE.txt"]
aws_s3_object.origin_dist_artifact["static/js/app.1c6db355.js"]
aws_s3_object.origin_dist_artifact["static/js/app.1c6db355.js.LICENSE.txt"]
aws_s3_object.origin_dist_artifact["static/js/runtime-app.99662f13.js"]
update aws_ecs_service.console
aws_s3_object.origin_dist_artifact["200.html"]
aws_s3_object.origin_dist_artifact["build-manifest.json"]
aws_s3_object.origin_dist_artifact["chunk-references.json"]
aws_s3_object.origin_dist_artifact["index.html"]
module.api_ssl_certificate.aws_route53_record.default["api.staging.cpf.usdr.dev"]
module.cloudfront_ssl_certificate.aws_route53_record.default["staging.cpf.usdr.dev"]
module.lambda_function-cpfCreateArchive.aws_lambda_function.this[0]
module.lambda_function-cpfValidation.aws_lambda_function.this[0]
module.lambda_function-email-presigned-url.aws_lambda_function.this[0]
module.lambda_function-graphql.aws_lambda_function.this[0]
module.lambda_function-processValidationJson.aws_lambda_function.this[0]
module.lambda_function-subrecipientTreasuryReportGen.aws_lambda_function.this[0]
module.lambda_function-treasuryProjectFileGeneration.aws_iam_policy.additional_inline[0]
module.lambda_function-treasuryProjectFileGeneration.aws_lambda_function.this[0]
recreate aws_ecs_task_definition.console
aws_s3_object.lambda_artifact-graphql
aws_s3_object.lambda_artifact-processValidationJson
aws_s3_object.lambda_artifact-python
module.lambda_function-cpfCreateArchive.aws_lambda_permission.current_version_triggers["StepFunctionTrigger"]
module.lambda_function-cpfValidation.aws_lambda_permission.current_version_triggers["S3BucketNotification"]
module.lambda_function-email-presigned-url.aws_lambda_permission.current_version_triggers["StepFunctionTrigger"]
module.lambda_function-graphql.aws_lambda_permission.current_version_triggers["APIGateway"]
module.lambda_function-processValidationJson.aws_lambda_permission.current_version_triggers["S3BucketNotification"]
module.lambda_function-subrecipientTreasuryReportGen.aws_lambda_permission.current_version_triggers["StepFunctionTrigger"]
module.lambda_function-treasuryProjectFileGeneration.aws_lambda_permission.current_version_triggers["S3BucketNotification"]
module.lambda_function-treasuryProjectFileGeneration.aws_lambda_permission.current_version_triggers["StepFunctionTrigger"]
delete aws_s3_object.origin_dist_artifact["static/css/app.6a6e443e.css"]
aws_s3_object.origin_dist_artifact["static/js/180.e1468c1c.chunk.js"]
aws_s3_object.origin_dist_artifact["static/js/180.e1468c1c.chunk.js.LICENSE.txt"]
aws_s3_object.origin_dist_artifact["static/js/UploadUploadsPage.f3787988.chunk.js"]
aws_s3_object.origin_dist_artifact["static/js/UploadUploadsPage.f3787988.chunk.js.LICENSE.txt"]
aws_s3_object.origin_dist_artifact["static/js/app.c35fb72c.js"]
aws_s3_object.origin_dist_artifact["static/js/app.c35fb72c.js.LICENSE.txt"]
aws_s3_object.origin_dist_artifact["static/js/runtime-app.3d283e88.js"]

Base automatically changed from nowei/mypy-checking to main November 21, 2024 16:37
Comment on lines 301 to 304
except ClientError as e:
error = e.response.get("Error") or {}
if error.get("Code") == "404":
logger.exception("Expected to find an existing treasury output report")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: no raise because the file could've already been deleted before

logger: structlog.stdlib.BoundLogger,
):
try:
delete_file_from_s3(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we also need to delete the report.zip file in the same directory in addition to this file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the list of files to delete:

        delete_file_from_s3(
            client=s3_client,
            bucket=os.environ["REPORTING_DATA_BUCKET_NAME"],
            key=get_generated_output_file_key(
                file_type=OutputFileType.XLSX,
                project=project_use_code,
                organization=organization,
            ),
        )
        delete_file_from_s3(
            client=s3_client,
            bucket=os.environ["REPORTING_DATA_BUCKET_NAME"],
            key=get_generated_output_file_key(
                file_type=OutputFileType.CSV,
                project=project_use_code,
                organization=organization,
            ),
        )
        delete_file_from_s3(
            client=s3_client,
            bucket=os.environ["REPORTING_DATA_BUCKET_NAME"],
            key=get_generated_output_file_key(
                file_type=OutputFileType.JSON,
                project=project_use_code,
                organization=organization,
            ),
        )
        report_key = f"treasuryreports/{organization.id}/{organization.preferences.current_reporting_period_id}/report.zip"
        delete_file_from_s3(
            client=s3_client,
            bucket=os.environ["REPORTING_DATA_BUCKET_NAME"],
            key=report_key,
        )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, but I'm looking into fixing the issue with the CI; (all the changed files have a python/ prepended that needs to be removed)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed here: #575 🤞🏼

@as1729
Copy link
Contributor

as1729 commented Dec 9, 2024

@nowei one more change requested. This will require adding permissions to delete s3 files in the terraform for the lambda function.

Here's where the permissions are defined currently:
https://github.com/usdigitalresponse/cpf-reporter/blob/main/terraform/treasury_generation_lambda_functions.tf#L229

The following needs to be added:

    AllowDeleteTreasuryReports = {
      effect = "Allow"
      actions = [
        "s3:DeleteObject",
      ]
      resources = [
        # These are completed XLSX version of the project (1A, 1B, or 1C) file that can be submitted to treasury.
        # Path: /treasuryreports/{organization_id}/{reporting_period_id}/{OUTPUT_TEMPLATE_FILENAME_BY_PROJECT[project]}.xlsx
        "${module.reporting_data_bucket.bucket_arn}/treasuryreports/*/*/*.xlsx",
        "${module.reporting_data_bucket.bucket_arn}/treasuryreports/*/*/*.csv",
        "${module.reporting_data_bucket.bucket_arn}/treasuryreports/*/*/*.json",
        "${module.reporting_data_bucket.bucket_arn}/treasuryreports/*/*/*.zip",
      ]
    }

@as1729 as1729 enabled auto-merge (squash) December 9, 2024 17:11
@as1729 as1729 merged commit 378d8ae into main Dec 9, 2024
25 checks passed
@as1729 as1729 deleted the nowei/delete-on-force-regenerate branch December 9, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants