Skip to content

Evaluate button - #111

Merged
lmd59 merged 6 commits into
mainfrom
evaluate-button
Jul 16, 2025
Merged

Evaluate button#111
lmd59 merged 6 commits into
mainfrom
evaluate-button

Conversation

@lmd59

@lmd59 lmd59 commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

Summary

Modal that gives options and results of sending an evaluate request to the evaluation service.

Probably not a perfect UI/UX but hopefully good enough for now.

New Behavior

Evaluate button pops a modal that gives options for report type and if subject type, also gives options to select a subject. When the user selects "Evaluate", the application sends an $evaluate request to the evaluation service with the selected parameters.

Code Changes

  • All changes are in the PopulationCalculation component. Adds evaluation functions and Modal component pieces.

Testing Guidance

@lmd59
lmd59 marked this pull request as ready for review July 15, 2025 14:17
@elsaperelli
elsaperelli self-requested a review July 15, 2025 20:59
@elsaperelli elsaperelli self-assigned this Jul 15, 2025

@elsaperelli elsaperelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested with CMS506 with abacus-demo deqm-test-server and the bellese server and it looks good to me!

Some small comments that can definitely be addressed at a later time, just wanted to make note of it now!

Comment thread components/calculation/PopulationCalculation.tsx
const trustMetaProfile = useRecoilValue(trustMetaProfileState);
const [reportTypeValue, setReportTypeValue] = useState('population');
const [subjectValue, setSubjectValue] = useState<string | null>('');
const [subjectData, setSubjectData] = useState<{ value: string; label: string }[]>([]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For future- we may want to put this in a type to pass into this (like we do with LabeledDetailedResult on line 43).

@lmd59 lmd59 Jul 16, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This should be simpler than LabeledDetailedResult since it's literally just the Select structure. In mantine v8 there's a built in type for this (ComboboxItem), but I was having trouble finding an equivalent in v6. We could do a custom type, but because it would just be for the component structure, part of me just wants to update mantine at some point instead. One can dream... heh

});
};

const sendEvaluate = async (): Promise<fhir4.Bundle | undefined> => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we may want to change these output types since I believe it can be Bundle, Parameters (technically this is the correct output LOL), or OperationOutcome

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

headsmack thank you

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will change this to bundle or parameters. Ideally in the future, it would just be Parameters. For now, it doesn't matter because we're just spitting it out to a string, but in the future, we might actually want to do something with the structure.

},
{
name: 'periodStart',
valueString: measurementPeriodFormatted?.start.split('T')[0]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This may not matter, but I have used valueDate instead of valueString

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Huh- I think I've used valueString generally for testing in the past (ala https://github.com/projecttacoma/deqm-test-server?tab=readme-ov-file#evaluate example). I agree valueDate probably makes more sense though.

if (reportTypeValue === 'subject' && subjectValue) {
parameters.parameter?.push({
name: 'subject',
valueString: `Patient/${subjectValue}`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I also think I was able to just send in the patient id on its own, not sure if having Patient/ is a problem I think its fine, just wanted to mention

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think since it's a reference, the Patient/ is more correct. Especially if there's potential for the subject to be of a different type. For deqm-test-server, our individual evaluate seems to be able to handle it either way, but our population evaluate expects any specified subject to be a group reference.

@hossenlopp
hossenlopp self-requested a review July 15, 2025 22:33

@elsaperelli elsaperelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!!!

@lmd59
lmd59 merged commit 398d769 into main Jul 16, 2025
1 check passed
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.

3 participants