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

[TTAHUB-925]remove-erroneous-records #972

Merged
merged 4 commits into from Aug 4, 2022

Conversation

GarrettEHill
Copy link
Collaborator

@GarrettEHill GarrettEHill commented Jul 25, 2022

Description of change

A bas assumption was made during the migration that caused addition erroneous records to be created. This will remove these records.

How to test

Issue(s)

Checklists

Every PR

  • Meets issue criteria
  • JIRA ticket status updated
  • Code is meaningfully tested
  • Meets accessibility standards (WCAG 2.1 Levels A, AA)
  • API Documentation updated
  • Boundary diagram updated
  • Logical Data Model updated
  • Architectural Decision Records written for major infrastructure decisions

Production Deploy

  • Staging smoke test completed

After merge/deploy

  • Update JIRA ticket status

hardwarehuman
hardwarehuman previously approved these changes Jul 26, 2022
Copy link
Collaborator

@hardwarehuman hardwarehuman left a comment

Choose a reason for hiding this comment

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

This looks right to me, though I'm wondering if we might want to leave the erroneous records as full permanent tables for now just in case we make a mistake here and want to restore certain records. We can drop the tables later.

src/migrations/20220725000000-remove-erroneous-records.js Outdated Show resolved Hide resolved
AND args."grantId" = r."grantId"
WHERE r.id is null;
------------------------------------------------------------------------------------
CREATE TEMP TABLE "erroneousObjectives" AS
Copy link
Collaborator

Choose a reason for hiding this comment

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

A more standard and scalable way to do something like this is:

CREATE TEMP TABLE err_obj AS
WITH correct_aros AS (
SELECT DISTINCT
  o.id "objectiveId",
  aro.id "aroId"
FROM "Objectives" o
JOIN "ActivityReportObjectives" aro
  ON o.id = aro."objectiveId"
EXCEPT
SELECT DISTINCT
  "objectiveId",
  "aroId"
FROM "erroneousAROs"
),
correct_objectives AS (SELECT DISTINCT "objectiveId" FROM correct_aros)
SELECT
  o.id "objectiveId",
  o."goalId"
FROM "Objectives" o
LEFT JOIN correct_aros c
  ON c."objectiveId" = o.id
WHERE c."objectiveId" IS NULL

src/migrations/20220725000000-remove-erroneous-records.js Outdated Show resolved Hide resolved
@GarrettEHill GarrettEHill dismissed hardwarehuman’s stale review August 2, 2022 21:44

added dedupe of objectives on csv file

Copy link
Collaborator

@thewatermethod thewatermethod left a comment

Choose a reason for hiding this comment

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

Works great! Should we add a test case for the objectives roll up?

@GarrettEHill
Copy link
Collaborator Author

Works great! Should we add a test case for the objectives roll up?

I already modified the current tests to include a roll up objective.

@GarrettEHill GarrettEHill changed the title remove-erroneous-records [TTAHUB-925]remove-erroneous-records Aug 4, 2022
@AdamAdHocTeam
Copy link
Collaborator

This looks right to me, though I'm wondering if we might want to leave the erroneous records as full permanent tables for now just in case we make a mistake here and want to restore certain records. We can drop the tables later.

+1

@GarrettEHill GarrettEHill merged commit 5901fa6 into main Aug 4, 2022
@GarrettEHill GarrettEHill deleted the TTAHUB-925/remove-erroneous-records branch August 4, 2022 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants