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.

Sign up for GitHub

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

Pccm template backend #11490

Merged
merged 5 commits into from
Oct 18, 2023
Merged

Pccm template backend #11490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bef7179
add pccm filtering
gmrabian Oct 16, 2023
18f8002
remove unused import
gmrabian Oct 16, 2023
ee4c3d4
generate pccm template in backend
gmrabian Oct 17, 2023
d319bfc
refactor to use hash; add tests
gmrabian Oct 17, 2023
219711d
Merge branch 'add-pccm-radio-3027' into pccm-template-backend
gmrabian Oct 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions services/app-api/handlers/formTemplates/populateTemplatesTable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
import { S3 } from "aws-sdk";
import * as path from "path";
import { logger } from "../../utils/logging";
import { AttributeValue, QueryInput } from "aws-sdk/clients/dynamodb";
import { createHash } from "crypto";

type S3ObjectRequired = SomeRequired<S3.Object, "Key" | "LastModified">;
Expand All @@ -34,26 +33,6 @@ type FormTemplateMetaData = {
hash: string;
};

/**
*
* @param reportType report type
* @param hash hash to look for
* @returns
*/
export function getTemplateVersionByHash(reportType: ReportType, hash: string) {
const queryParams: QueryInput = {
TableName: process.env.FORM_TEMPLATE_TABLE_NAME!,
IndexName: "HashIndex",
KeyConditionExpression: "reportType = :reportType AND md5Hash = :md5Hash",
Limit: 1,
ExpressionAttributeValues: {
":md5Hash": hash as AttributeValue,
":reportType": reportType as unknown as AttributeValue,
},
};
return dynamodbLib.query(queryParams);
}
Comment on lines -43 to -55
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this to formTemplate.ts


/**
* Retrieve template data from S3
*
Expand Down
6 changes: 5 additions & 1 deletion services/app-api/handlers/reports/create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,14 @@ export const createReport = handler(async (event, _context) => {

let formTemplate, formTemplateVersion;

const isProgramPCCM =
unvalidatedMetadata?.programIsPCCM?.[0]?.value === "Yes";

try {
({ formTemplate, formTemplateVersion } = await getOrCreateFormTemplate(
reportBucket,
reportType
reportType,
isProgramPCCM
Comment on lines +86 to +93
Copy link
Contributor Author

Choose a reason for hiding this comment

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

report create now checks for PCCM radio and generates a separate template if necessary – see below

));
} catch (err) {
logger.error(err, "Error getting or creating template");
Expand Down
93 changes: 73 additions & 20 deletions services/app-api/utils/formTemplates/formTemplates.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
compileValidationJsonFromRoutes,
flattenReportRoutesArray,
formTemplateForReportType,
generatePCCMTemplate,
getOrCreateFormTemplate,
getValidationFromFormTemplate,
isFieldElement,
Expand All @@ -15,6 +16,11 @@ import { mockDocumentClient, mockReportJson } from "../testing/setupJest";
import s3Lib from "../s3/s3-lib";
import dynamodbLib from "../dynamo/dynamodb-lib";

const programIsPCCM = true;
const programIsNotPCCM = false;

global.structuredClone = (val: any) => JSON.parse(JSON.stringify(val));
Copy link
Contributor Author

@gmrabian gmrabian Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

test: had to mock structuredClone to its more primitive form because 🤷 I guess node environment isn't up to date enough to support it

info here

Copy link
Contributor

Choose a reason for hiding this comment

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

Well that's sad. Mocking it here for tests isn't too painful imo, but hopefully something we can remove later.


const currentMLRFormHash = createHash("md5")
.update(JSON.stringify(mlr))
.digest("hex");
Expand All @@ -23,19 +29,30 @@ const currentMCPARFormHash = createHash("md5")
.update(JSON.stringify(mcpar))
.digest("hex");

const pccmTemplate = generatePCCMTemplate(mcpar);
const currentPCCMFormHash = createHash("md5")
.update(JSON.stringify(pccmTemplate))
.digest("hex");

describe("Test getOrCreateFormTemplate MCPAR", () => {
beforeEach(() => {
jest.restoreAllMocks();
});
it("should create a new form template if none exist", async () => {
// mocked once for search by hash
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [],
});
Comment on lines +43 to +45
Copy link
Contributor Author

Choose a reason for hiding this comment

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

test: added mocks to many tests because we now have two queries: one for matching hash, and one for latest version

// mocked again for search for latest report
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [],
});
const dynamoPutSpy = jest.spyOn(dynamodbLib, "put");
const s3PutSpy = jest.spyOn(s3Lib, "put");
const result = await getOrCreateFormTemplate(
"local-mcpar-reports",
ReportType.MCPAR
ReportType.MCPAR,
programIsNotPCCM
);
expect(dynamoPutSpy).toHaveBeenCalled();
expect(s3PutSpy).toHaveBeenCalled();
Expand All @@ -47,7 +64,34 @@ describe("Test getOrCreateFormTemplate MCPAR", () => {
expect(result.formTemplateVersion?.md5Hash).toEqual(currentMCPARFormHash);
});

it("should create a new form template for PCCM if none exist", async () => {
// mocked once for search by hash
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [],
});
// mocked again for search for latest report
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [],
});
const dynamoPutSpy = jest.spyOn(dynamodbLib, "put");
const s3PutSpy = jest.spyOn(s3Lib, "put");
const result = await getOrCreateFormTemplate(
"local-mcpar-reports",
ReportType.MCPAR,
programIsPCCM
);
expect(dynamoPutSpy).toHaveBeenCalled();
expect(s3PutSpy).toHaveBeenCalled();
expect(result.formTemplate).toEqual({
...pccmTemplate,
validationJson: getValidationFromFormTemplate(pccmTemplate as ReportJson),
});
expect(result.formTemplateVersion?.versionNumber).toEqual(1);
expect(result.formTemplateVersion?.md5Hash).toEqual(currentPCCMFormHash);
});

it("should return the right form and formTemplateVersion if it matches the most recent form", async () => {
// mocked once for search by hash
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [
{
Expand All @@ -56,19 +100,14 @@ describe("Test getOrCreateFormTemplate MCPAR", () => {
md5Hash: currentMCPARFormHash,
versionNumber: 3,
},
{
formTemplateId: "foo",
id: "mockReportJson",
md5Hash: currentMCPARFormHash + "111",
versionNumber: 2,
},
],
});
const dynamoPutSpy = jest.spyOn(dynamodbLib, "put");
const s3PutSpy = jest.spyOn(s3Lib, "put");
const result = await getOrCreateFormTemplate(
"local-mcpar-reports",
ReportType.MCPAR
ReportType.MCPAR,
programIsNotPCCM
);
expect(dynamoPutSpy).not.toHaveBeenCalled();
expect(s3PutSpy).not.toHaveBeenCalled();
Expand All @@ -77,6 +116,11 @@ describe("Test getOrCreateFormTemplate MCPAR", () => {
});

it("should create a new form if it doesn't match the most recent form", async () => {
// mocked once for search by hash
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [],
});
// mocked again for search for latest report
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [
{
Expand All @@ -97,7 +141,8 @@ describe("Test getOrCreateFormTemplate MCPAR", () => {
const s3PutSpy = jest.spyOn(s3Lib, "put");
const result = await getOrCreateFormTemplate(
"local-mcpar-reports",
ReportType.MCPAR
ReportType.MCPAR,
programIsNotPCCM
);
expect(dynamoPutSpy).toHaveBeenCalled();
expect(s3PutSpy).toHaveBeenCalled();
Expand All @@ -110,14 +155,20 @@ describe("Test getOrCreateFormTemplate MLR", () => {
jest.restoreAllMocks();
});
it("should create a new form template if none exist", async () => {
// mocked once for search by hash
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [],
});
// mocked again for search for latest report
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [],
});
const dynamoPutSpy = jest.spyOn(dynamodbLib, "put");
const s3PutSpy = jest.spyOn(s3Lib, "put");
const result = await getOrCreateFormTemplate(
"local-mlr-reports",
ReportType.MLR
ReportType.MLR,
programIsNotPCCM
);
expect(dynamoPutSpy).toHaveBeenCalled();
expect(s3PutSpy).toHaveBeenCalled();
Expand All @@ -130,6 +181,7 @@ describe("Test getOrCreateFormTemplate MLR", () => {
});

it("should return the right form and formTemplateVersion if it matches the most recent form", async () => {
// mocked once for search by hash
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [
{
Expand All @@ -138,19 +190,14 @@ describe("Test getOrCreateFormTemplate MLR", () => {
md5Hash: currentMLRFormHash,
versionNumber: 3,
},
{
formTemplateId: "foo",
id: "mockReportJson",
md5Hash: currentMLRFormHash + "111",
versionNumber: 2,
},
],
});
const dynamoPutSpy = jest.spyOn(dynamodbLib, "put");
const s3PutSpy = jest.spyOn(s3Lib, "put");
const result = await getOrCreateFormTemplate(
"local-mlr-reports",
ReportType.MLR
ReportType.MLR,
programIsNotPCCM
);
expect(dynamoPutSpy).not.toHaveBeenCalled();
expect(s3PutSpy).not.toHaveBeenCalled();
Expand All @@ -159,18 +206,23 @@ describe("Test getOrCreateFormTemplate MLR", () => {
});

it("should create a new form if it doesn't match the most recent form", async () => {
// mocked once for search by hash
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [],
});
// mocked again for search for latest report
mockDocumentClient.query.promise.mockReturnValueOnce({
Items: [
{
formTemplateId: "foo",
id: "mockReportJson",
md5Hash: currentMLRFormHash + "111111",
md5Hash: currentMCPARFormHash + "111111",
versionNumber: 3,
},
{
formTemplateId: "foo",
id: "mockReportJson",
md5Hash: currentMLRFormHash + "111",
md5Hash: currentMCPARFormHash + "111",
versionNumber: 2,
},
],
Expand All @@ -179,7 +231,8 @@ describe("Test getOrCreateFormTemplate MLR", () => {
const s3PutSpy = jest.spyOn(s3Lib, "put");
const result = await getOrCreateFormTemplate(
"local-mlr-reports",
ReportType.MLR
ReportType.MLR,
programIsNotPCCM
);
expect(dynamoPutSpy).toHaveBeenCalled();
expect(s3PutSpy).toHaveBeenCalled();
Expand Down
84 changes: 75 additions & 9 deletions services/app-api/utils/formTemplates/formTemplates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,24 @@ export async function getNewestTemplateVersion(reportType: ReportType) {
return result.Items?.[0];
}

export async function getTemplateVersionByHash(
reportType: ReportType,
hash: string
) {
const queryParams: QueryInput = {
TableName: process.env.FORM_TEMPLATE_TABLE_NAME!,
IndexName: "HashIndex",
KeyConditionExpression: "reportType = :reportType AND md5Hash = :md5Hash",
Limit: 1,
ExpressionAttributeValues: {
":md5Hash": hash as AttributeValue,
":reportType": reportType as unknown as AttributeValue,
},
};
const result = await dynamodbLib.query(queryParams);
return result.Items?.[0];
}

export const formTemplateForReportType = (reportType: ReportType) => {
switch (reportType) {
case ReportType.MCPAR:
Expand All @@ -54,25 +72,31 @@ export const formTemplateForReportType = (reportType: ReportType) => {

export async function getOrCreateFormTemplate(
reportBucket: string,
reportType: ReportType
reportType: ReportType,
isProgramPCCM: boolean
) {
const currentFormTemplate = formTemplateForReportType(reportType);
let currentFormTemplate = formTemplateForReportType(reportType);
if (isProgramPCCM) {
currentFormTemplate = generatePCCMTemplate(currentFormTemplate);
}
const stringifiedTemplate = JSON.stringify(currentFormTemplate);

const currentTemplateHash = createHash("md5")
.update(stringifiedTemplate)
.digest("hex");

const mostRecentTemplateVersion = await getNewestTemplateVersion(reportType);
const mostRecentTemplateVersionHash = mostRecentTemplateVersion?.md5Hash;
const matchingTemplateMetadata = await getTemplateVersionByHash(
reportType,
currentTemplateHash
);

if (currentTemplateHash === mostRecentTemplateVersionHash) {
if (matchingTemplateMetadata) {
Comment on lines +88 to +93
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we now identify if there is a matching template by hash rather than latest. This way if we alternate creating PCCM and MCPAR forms it won't create duplicates in the table because the hash will be the same

return {
formTemplate: await getTemplate(
reportBucket,
getFormTemplateKey(mostRecentTemplateVersion?.id)
getFormTemplateKey(matchingTemplateMetadata?.id)
),
formTemplateVersion: mostRecentTemplateVersion,
formTemplateVersion: matchingTemplateMetadata,
};
} else {
const newFormTemplateId = KSUID.randomSync().string;
Expand All @@ -92,10 +116,12 @@ export async function getOrCreateFormTemplate(
throw err;
}

const newestTemplateMetadata = await getNewestTemplateVersion(reportType);

// If we didn't find any form templates, start version at 1.
const newFormTemplateVersionItem: FormTemplate = {
versionNumber: mostRecentTemplateVersion?.versionNumber
? (mostRecentTemplateVersion.versionNumber += 1)
versionNumber: newestTemplateMetadata?.versionNumber
? (newestTemplateMetadata.versionNumber += 1)
Comment on lines +123 to +124
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we still version off of the latest for simplicity. So whether you create a MCPAR or a PCCM, if it's new it will get a +1 to the latest version and save in the same table/bucket

Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear, since PCCM form template versions are stored with reportType = MCPAR, we will have MCPAR version 17 (say) be a full MCPAR, and MCPAR version 18 be PCCM, and MCPAR version 19 be a full MCPAR again (with some differences compared to v17, such that the hash no longer matches). The only way for a developer browsing the form template version table to know whether a given entry for the MCPAR reportType is PCCM will be for them to pull the form template from S3.

I don't anticipate ever needing or wanting to do so myself, so I have no problem with that.

I also am not sure that this comment adds any clarity. Um, sorry. The code is good though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying, Ben! That helps.

Yes if we needed to find a PCCM template we could always look in the report table first, search for a report that selected "Yes" to PCCM, get the templateId from there, etc.

: 1,
md5Hash: currentTemplateHash,
id: newFormTemplateId,
Expand Down Expand Up @@ -244,3 +270,43 @@ export function getValidationFromFormTemplate(reportJson: ReportJson) {
export function getPossibleFieldsFromFormTemplate(reportJson: ReportJson) {
return Object.keys(getValidationFromFormTemplate(reportJson));
}

const routesToIncludeInPCCM = {
"A: Program Information": [
"Point of Contact",
"Reporting Period",
"Add Plans",
],
"B: State-Level Indicators": ["I: Program Characteristics"],
"C: Program-Level Indicators": ["I: Program Characteristics"],
"D: Plan-Level Indicators": ["I: Program Characteristics", "VIII: Sanctions"],
"Review & Submit": [],
} as { [key: string]: string[] };

const entitiesToIncludeInPCCM = ["plans", "sanctions"];
Comment on lines +274 to +286
Copy link
Contributor Author

Choose a reason for hiding this comment

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

routes to include from ticket

Section A: Program Information
    Point of Contact
    Reporting Period
    Add Plans
Section B: State-Level Indicators
    Program Characteristics and Enrollment
Section C: Program-Level Indicators
    Program Characteristics
Section D: Plan-Level Indicators
    Program Characteristics and Enrollment
    Sanctions
Review and Submit

entities inferred from same list

Copy link
Contributor

Choose a reason for hiding this comment

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

Comment: I think that, strictly speaking, the entity inference could have been done in code rather than hardcoding plans and sanctions in this array. But that code would have been terribly complicated compared to this one-line declaration, so I completely agree with your approach.


export const generatePCCMTemplate = (originalReportTemplate: any) => {
const reportTemplate = structuredClone(originalReportTemplate);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this structuredClone let's us copy the mcpar template without modifying it in memory, so that we can create MCPARs and PCCMs and they will always be based on the full MCPAR

// remove top level sections not in include list
reportTemplate.routes = reportTemplate.routes.filter(
(route: ReportRoute) => !!routesToIncludeInPCCM[route.name]
);

// only include listed subsections
for (let route of reportTemplate.routes) {
if (route?.children) {
route.children = route.children.filter((childRoute: ReportRoute) =>
routesToIncludeInPCCM[route.name].includes(childRoute.name)
);
}
}

// Any entity not in the allow list must be removed.
for (let entityType of Object.keys(reportTemplate.entities)) {
if (!entitiesToIncludeInPCCM.includes(entityType)) {
delete reportTemplate.entities[entityType];
}
}

return reportTemplate;
};