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

MLR E2E Updates #11389

Merged
merged 36 commits into from Aug 1, 2023
Merged

MLR E2E Updates #11389

merged 36 commits into from Aug 1, 2023

Conversation

ntsummers1
Copy link
Contributor

@ntsummers1 ntsummers1 commented Jul 21, 2023

Description

This PR does a whole bunch of little things, I'll go through the PR and call out what each chunk of code was for.

Related ticket(s)

MDCT-2661


How to test

Run the tests locally and through the git action!

Important updates

Removed Xpath and WaitUntil as dependencies in cypress


Author checklist

  • I have performed a self-review of my code
  • I have added thorough tests, if necessary
  • I have updated relevant documentation, if necessary

karla-vm
karla-vm previously approved these changes Jul 24, 2023
@ntsummers1 ntsummers1 changed the title Update Overlay page to use submit function MLR E2E Updates Jul 27, 2023
id: node_version
run: echo ::set-output name=NODE_VERSION::$(cat .nvmrc)
- uses: actions/setup-node@v1
- uses: actions/setup-node@v3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the deploy updates were an attempt to get Cypress to use the same node version we use. Unfortunately, Cypress doesn't respect the node version you give it. So setting up node doesn't matter. It seems its tied to the git actions node version. Heres a thread with all sorts of other threads to dig down the rabbit hole (cypress-io/github-action#637).

That being said, we were on v1 which was no longer supported, so swapping to v3 here is a solid upgrade.

Comment on lines +128 to +132
- name: Checkout
uses: actions/checkout@v3

- name: Run Cypress Tests
uses: cypress-io/github-action@v4.2.0
uses: cypress-io/github-action@v5
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See comment above about attempting to get cypress to use our node version

Comment on lines 53 to 102
const reportFieldDataEntities = report?.fieldData[entityType] || [];

const onSubmit = async (enteredData: AnyObject) => {
setSubmitting(true);
const filteredFormData = filterFormData(
enteredData,
form.fields.filter(isFieldElement)
);
const newEntity = {
...selectedEntity,
...filteredFormData,
};
updateEntities(newEntity);
const reportKeys = {
reportType: report?.reportType,
state: state,
id: report?.id,
};
const dataToWrite = {
metadata: {
status: ReportStatus.IN_PROGRESS,
lastAlteredBy: full_name,
},
fieldData: {
program: entities,
},
};
await updateReport(reportKeys, dataToWrite);
setSubmitting(false);
if (userIsEndUser) {
setSubmitting(true);
const reportKeys = {
reportType: report?.reportType,
state: state,
id: report?.id,
};
const currentEntities = [...(report?.fieldData[entityType] || [])];
const selectedEntityIndex = report?.fieldData[entityType].findIndex(
(entity: EntityShape) => entity.id === selectedEntity?.id
);
const filteredFormData = filterFormData(
enteredData,
form.fields.filter(isFieldElement)
);
const entriesToClear = getEntriesToClear(
enteredData,
form.fields.filter(isFieldElement)
);
const newEntity = {
...selectedEntity,
...filteredFormData,
};
let newEntities = currentEntities;
newEntities[selectedEntityIndex] = newEntity;
newEntities[selectedEntityIndex] = setClearedEntriesToDefaultValue(
newEntities[selectedEntityIndex],
entriesToClear
);
const shouldSave = entityWasUpdated(
reportFieldDataEntities[selectedEntityIndex],
newEntity
);
if (shouldSave) {
const dataToWrite = {
metadata: {
status: ReportStatus.IN_PROGRESS,
lastAlteredBy: full_name,
},
fieldData: {
[entityType]: newEntities,
},
};
await updateReport(reportKeys, dataToWrite);
}
setSubmitting(false);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alllllll this code was updated because, it turns out, the submit button wasn't hooked up to anything. The MLR Overlay was entirely reliant on autosave, and we found that if a user goes quick enough the autosave call will get cancelled when leaving the overlay page and their last item wouldn't save. This would cause a validation error and give us an error about the submit button being disabled. All this code, plus the <Button type="submit" sx={sx.saveButton} form={form.id}> code just underneath this is to make sure the save and close button saves and closes for real.

@@ -12,7 +12,6 @@ module.exports = defineConfig({
screenshotsFolder: "screenshots",
videosFolder: "videos",
downloadsFolder: "downloads",
defaultCommandTimeout: 2000000000,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the 2 billion millisecond timeout that was causing prs to spin endlessly.

Comment on lines -79 to -90
cy.waitUntil(() =>
cy
.window()
.then(
(window) =>
Object.keys(window.localStorage).filter((key) =>
key.match(
/CognitoIdentityServiceProvider.+(refresh|access|id)Token/
)
).length === 3
)
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the authentication call. It was using xpath which is deprecated, and I've removed with waitUtil and key check. Wait until wasn't in the docs from what I could find, and so I really tried to simplify the login process to be safe. That being said, calling login from the ui is an anti-pattern I guess? At least according to cypress. Some information about how we're actually supposed to call it is here but I simplified it to work with ours.

Comment on lines +21 to +24
if (inputValue == "true") {
input.check();
input.blur();
} else input.uncheck();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separating out chaining commands. You can read why from the cypress docs here: https://docs.cypress.io/guides/core-concepts/introduction-to-cypress#Chains-of-Commands

Comment on lines +20 to +22
cy.get(
'a[href="https://www.cms.gov/About-CMS/Agency-Information/Aboutwebsite/CMSNondiscriminationNotice"]'
).contains(accessibilityStatementLinkText);
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 to a slightly better way of calling and erroring if the footer link isn't found.

Comment on lines -103 to -104
cy.findByRole("button", { name: "Add managed care program" }).click();
cy.findByLabelText("Program name").type(programName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

findBy isn't actually a cypress command, so its been swapped to use .get

Comment on lines +116 to +123
cy.get("table").within(() => {
cy.get("td")
.contains(programName)
.parent()
.find('button:contains("Edit")')
.focus()
.click();
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

findByText sometimes couldn't actually find things, and isn't actually a cypress command. So its been swapped to this.

@codeclimate
Copy link

codeclimate bot commented Jul 29, 2023

Code Climate has analyzed commit 46995ad and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (90% is the threshold).

This pull request will bring the total coverage in the repository to 96.4% (0.3% change).

View more on Code Climate.

@@ -1,82 +1,153 @@
import { EntityProvider } from "components/reports/EntityProvider";
import { render, screen } from "@testing-library/react";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Total overhaul of this file. Everything in EntityDetails has been moved around/rewritten and thus needs tests to represent it!

@@ -1,176 +1,123 @@
import { useContext, useEffect, useState } from "react";
import arrowLeftBlue from "assets/icons/icon_arrow_left_blue.png";
import React, { MouseEventHandler, useContext, useEffect } from "react";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Overhaul of this file was to move a lot of the state and context out of here and into the parent, ModalOverlay. Two reasons: That simplified a lot of code and also because it expands upon an already established pattern we have in other areas like ModalDrawer.

@@ -1,270 +1,375 @@
import { act, render, screen, waitFor } from "@testing-library/react";
import { axe } from "jest-axe";
import { render, screen } from "@testing-library/react";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Total overhaul of ModalOverlay. Everything in ModalOverlay has been moved around/rewritten and thus needs tests to represent it!

Comment on lines +45 to +54
// Context Information
const { isTablet, isMobile } = useBreakpoint();
const { report, updateReport } = useContext(ReportContext);
const [isEntityDetailsOpen, setIsEntityDetailsOpen] = useState<boolean>();
const [currentEntity, setCurrentEntity] = useState<EntityShape | undefined>(
undefined
);
const [isEntityDetailsOpen, setIsEntityDetailsOpen] = useState<boolean>();
const { report } = useContext(ReportContext);
const reportType = report?.reportType;
const reportFieldDataEntities = report?.fieldData[entityType] || [];
const { userIsAdmin, userIsReadOnly } = useUser().user ?? {};
const isAdminUserType = userIsAdmin || userIsReadOnly;
const formIsDisabled = isAdminUserType && route.modalForm?.adminDisabled;
// is MLR report in a LOCKED state
const isLocked = report?.locked || formIsDisabled;
const [submitting, setSubmitting] = useState<boolean>(false);
const { userIsAdmin, userIsReadOnly, userIsEndUser, full_name, state } =
useUser().user ?? {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really just moved some code here to combine like code into 1 spot. Also added some context and state that the EntityDetailsOverlay was originally using to simplify that code and go with our pre-established pattern in Modal Drawer

Comment on lines +116 to +166
const onSubmit = async (enteredData: AnyObject) => {
if (userIsEndUser) {
setSubmitting(true);
const reportKeys = {
reportType: report?.reportType,
state: state,
id: report?.id,
};
const currentEntities = [...(report?.fieldData[entityType] || [])];
const selectedEntityIndex = report?.fieldData[entityType].findIndex(
(entity: EntityShape) => entity.id === currentEntity?.id
);
const filteredFormData = filterFormData(
enteredData,
overlayForm!.fields.filter(isFieldElement)
);
const entriesToClear = getEntriesToClear(
enteredData,
overlayForm!.fields.filter(isFieldElement)
);
const newEntity = {
...currentEntity,
...filteredFormData,
};
let newEntities = currentEntities;
newEntities[selectedEntityIndex] = newEntity;
newEntities[selectedEntityIndex] = setClearedEntriesToDefaultValue(
newEntities[selectedEntityIndex],
entriesToClear
);
const shouldSave = entityWasUpdated(
reportFieldDataEntities[selectedEntityIndex],
newEntity
);
if (shouldSave) {
const dataToWrite = {
metadata: {
status: ReportStatus.IN_PROGRESS,
lastAlteredBy: full_name,
},
fieldData: {
[entityType]: newEntities,
},
};
await updateReport(reportKeys, dataToWrite);
}
setSubmitting(false);
}
closeEntityDetailsOverlay();
setSidebarHidden(false);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New onSubmit handler that gets passed to EntityDetailsOverlay. This gets fired when the user clicks "Save & Return" at the bottom"

<Box sx={sx.dashboardBox}>
<Heading as="h3" sx={sx.dashboardTitle}>
{dashTitle}
</Heading>
{reportFieldDataEntities.length === 0 ? (
<>
<hr />
<Box sx={sx.tableSeparator} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The separator was the wrong color and didn't have enough spacing to match the mockups

@@ -238,19 +308,16 @@ const sx = {
".tablet &, .mobile &": {
border: "none",
},
"&:nth-child(1)": {
"&:nth-of-type(1)": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cleans up some react errors you might be seeing in the console

addEntityButton: {
marginTop: "1.5rem",
marginTop: "2rem",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To match design specs

const navigate = useNavigate();
const { report } = useContext(ReportContext);
const { previousRoute, nextRoute } = useFindRoute(
report?.formTemplate.flatRoutes,
report?.formTemplate.basePath
);
const hidePrevious = previousRoute === "/mcpar" || previousRoute === "/mlr";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out all of MLR was getting no Previous button. I've re-added it and made it so that the first page of the form will not have a previous button but all others will. That is based on the mockups and feedback from Megan here

@@ -166,7 +166,7 @@
"exportSectionHeader": "Program Reporting Information",
"section": "MLR Reporting",
"subsection": "Medicaid Medical Loss Ratio (MLR) & Remittances",
"spreadsheet": "Program Information",
"spreadsheet": "Program Information & MLR Reporting",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Spreadsheet icon on the overlay page was missing some verbiage based on mockups

Copy link
Contributor

@gmrabian gmrabian left a comment

Choose a reason for hiding this comment

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

i probably want these changes

Copy link
Contributor

@britt-mo britt-mo left a comment

Choose a reason for hiding this comment

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

Tested in the browser, everything looked good. There are some console errors that might be worth making other cards for. Ran tests locally as well.

@BearHanded BearHanded merged commit 5dccb72 into main Aug 1, 2023
16 checks passed
@BearHanded BearHanded deleted the overlay-submit branch August 1, 2023 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants