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

NAAAR analysis methods statusing: more custom logic! #12018

Open
wants to merge 1 commit into
base: naaar-am-plans
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion services/ui-src/src/components/fields/DynamicField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export const DynamicField = ({ name, label, ...props }: Props) => {
// filter analysis methods to remove deleted plans
const filteredAnalysisMethods = report?.fieldData?.analysisMethods?.map(
(method: EntityShape) => {
if (method.analysis_method_applicable_plans?.length) {
if (method?.analysis_method_applicable_plans?.length) {
method.analysis_method_applicable_plans =
method.analysis_method_applicable_plans.filter(
(plan: AnyObject) => plan.key !== selectedRecord.id
Expand Down
16 changes: 14 additions & 2 deletions services/ui-src/src/components/reports/DrawerReportPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,18 @@ export const DrawerReportPage = ({ route, validateOnRender }: Props) => {
const isCustomEntity =
canAddEntities && !getDefaultAnalysisMethodIds().includes(entity.id);
const calculateEntityCompletion = () => {
// logic to ensure analysis methods always have a plan selected
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little confused about what calculateEntityCompletion is supposed to return. An array of field ids?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It returns a boolean. The final return call ends with .every(), so these shortcuts are just returning false early.

This function is called a few lines below its definition as such

const isEntityCompleted = reportingOnIlos
        ? calculateEntityCompletion() &&
          isIlosCompleted(reportingOnIlos, entity)
        : calculateEntityCompletion();

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. I would vote for changing calculateEntityCompletion() to something more boolean-sounding though isEntityCompleted is already taken...

if (
isAnalysisMethodsPage &&
!entity?.analysis_method_applicable_plans?.length
) {
if (isCustomEntity) {
return false;
gmrabian marked this conversation as resolved.
Show resolved Hide resolved
} else if (entity?.analysis_applicable?.[0]?.value === "Yes") {
return false;
}
}

let formFields = form.fields;
if (isCustomEntity) {
formFields = addEntityForm.fields;
Expand Down Expand Up @@ -305,7 +317,7 @@ export const DrawerReportPage = ({ route, validateOnRender }: Props) => {
<Text>{entity.custom_analysis_method_description}</Text>
)}
{entity.analysis_method_frequency &&
entity.analysis_method_applicable_plans && (
entity.analysis_method_applicable_plans?.length > 0 && (
<Text>
{entity.analysis_method_frequency[0].value}:&nbsp;
{entity.analysis_method_applicable_plans
Expand All @@ -321,7 +333,7 @@ export const DrawerReportPage = ({ route, validateOnRender }: Props) => {
)}
<Box sx={buttonBoxStyling(canAddEntities)}>
{enterButton(entity, isEntityCompleted)}
{canAddEntities && !entity.isRequired && (
{hasPlans && canAddEntities && !entity.isRequired && (
<Button
sx={sx.deleteButton}
data-testid="delete-entity"
Expand Down
Loading