-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: Configure Report To Select Scores or Raw Scores (CAMHI) (M2-7738) #1935
Conversation
This allows callers who pass a custom onChange function to this component to receive and optionally invoke the original react-hook-form onChange. This removes the need to invoke `setValue` (or equivalent) manually, which the docs say you should avoid
…scales configuration screen
…tion screen This block of code now also manually removes the linked subscale name if it can't be used to find a valid subscale
This pull request is automatically being deployed by Amplify Hosting (learn more). |
I wrapped this component and forgot to move the key up a level
Apparently MUI doesn't like a default value of `undefined` on the select, so the empty string works while still showing the placeholder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @sultanofcardio, I've run through the peer testing so far. All tests pass according to the expectations you've outlined in each of the scenarios, however I did discover a client-side error happening after each of two of these scenarios:
- Remove lookup table
- Delete selected subscale
In each of these cases, although I'm correctly led to fix the validation error on the Scores & Reports tab, once I fix it (by selecting a different eligible subscale, for instance), I'm unable to save changes. Clicking Save & Publish after I've addressed this validation error results in this client-side error, and trying to save from then on doesn't seem work until I reload the browser:
If there are other activities in the applet with existing report scores that don't have a scoringType, they will prevent any updates to the applet. So we must make this field nullable
Subscales can contain other subscales, but we shouldn't include them in the report
Thanks for finding this @farmerpaul 🙏 It should be fixed with c30a70c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That error is happening less now! However, I can still cause it to happen, but thankfully, it's not related to your work – it's a pre-existing bug reproducible in develop
. It seems that when you have a subscale that references another subscale, and you delete the referenced subscale (but not the one that was referencing it) and try to Save & Publish, you get that same TypeError: Cannot read properties of undefined (reading 'name')
and you can no longer save your changes. If you think it's too much effort to come up with a fix for this, I'll just log this as a bug in Jira. But if it's a quick fix, would be nice to take care of it.
I had a look through the code and nothing seems off to me, so happy to approve!
The lookup table parsing allows ranges in the score column, but this is invalid as scores can only be numbers. So I'm updating this code to ignore those until we can update the parsing code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Peer tested this alongside peer testing M2-8097.
Code LGTM. Great job @sultanofcardio
… (#1935) * Add support for a tooltip icon with a tooltip in the RadioGroupController * Add radio buttons with hardcoded text * Update ScoreReport type to include scoreType and linkedSubscaleName * WIP update to score type selection logic * Set selected items based on subscale * Update score range calculation to account for linked subscale * Exclude subscales with no activity items * Pass the `onChange` to the `onCustomChange` in the InputController This allows callers who pass a custom onChange function to this component to receive and optionally invoke the original react-hook-form onChange. This removes the need to invoke `setValue` (or equivalent) manually, which the docs say you should avoid * Update report score linked subscale name when it is edited on the subscales configuration screen * Fix type name * Require linked subscale when report score type is score * Update the logic that accounts for updates on the subscales configuration screen This block of code now also manually removes the linked subscale name if it can't be used to find a valid subscale * Remove the link to a report score if a subscale or its lookup table is deleted * Translate strings * Fix broken tests * Move key prop to parent I wrapped this component and forgot to move the key up a level * Fix MUI select warning Apparently MUI doesn't like a default value of `undefined` on the select, so the empty string works while still showing the placeholder * Set score type back to raw score if there are no eligible subscales * Hide the radio group if there are no eligible subscales * Fix broken test * Abstract field names * Rename scoreType to scoringType * Rename linkedSubscaleName to subscaleName * Change `rawScore` to `raw_score` * Make the `scoringType` field nullable If there are other activities in the applet with existing report scores that don't have a scoringType, they will prevent any updates to the applet. So we must make this field nullable * Remove the noisy MUI controlled field warning * Handle the case where the score is undefined 🤷 * Filter out ineligible subscale items Subscales can contain other subscales, but we shouldn't include them in the report * Account for existing applets with reports but no subscales * Account for reports that were saved without an explicit scoring type * Reset report scores to raw_score if there are no more eligible subscales * Remove linked subscales from reports when they no longer contain non-subscale items * Fix score range calculation * Ignore score ranges The lookup table parsing allows ranges in the score column, but this is invalid as scores can only be numbers. So I'm updating this code to ignore those until we can update the parsing code
📝 Description
Note
It may be helpful to look at TL;DR on Subscales, T Scores, and Lookup Tables before reviewing this PR if you're unfamiliar with subscales
🔗 Jira Ticket M2-7738
This PR adds support for linking subscales to report scores, allowing them to use T-Scores from a subscale's lookup table for their calculations.
Note
Only subscales that contain both a lookup table and at least one non-subscale item can be linked to a report score. It is valid for subscales to contain only nested subscales, but those are not valid for linking
To link a subscale to a report score, the admin must first define an eligible subscale, and then indicate the type of score to be used on the report
📸 Screenshots
In addition to the screenshots above, here are some other cases to consider
No eligible subscales
When there are no eligible subscales, the radio group does not appear and the score type is fixed to raw score
Raw score by default
When there are eligible subscales, raw score is selected by default and the Score Calculation Type and the Items used to calculate score remain editable
Tooltips
Tooltips are included to tell the user about each score type
🪤 Peer Testing
Note
Some scenarios may require the API modifications from ChildMindInstitute/mindlogger-backend-refactor#1616 to be tested
Create an activity and use it to test the following conditions
Hidden by default
Tooltip copy
Default score type
scoringType
field in the request body israw_score
Only eligible subscales
Link to subscales page
Note
The selected subscale can't be directly navigated to, so the one that appears after navigation may be a different one
Score range and items
Linked subscale is saved
subscaleName
field in the request body contains the name of the selected subscaleWarning
If ChildMindInstitute/mindlogger-backend-refactor#1616 is not yet complete, then the data may not be persisted
Only eligible items
itemsScore
field in the request body contains only the name of the non-subscale item(s)Warning
If ChildMindInstitute/mindlogger-backend-refactor#1616 is not yet complete, then the data may not be persisted
Change propagation
Name change
subscaleName
field in the request body contains the name of the selected subscaleWarning
If ChildMindInstitute/mindlogger-backend-refactor#1616 is not yet complete, then the data may not be persisted
Remove lookup table
Delete selected subscale
✏️ Notes
onChange
prop on theRadioGroupController
and theonCustomChange
prop in theInputController
to accept the originalonChange
function fromreact-hook-form
as a parameter, so that it may optionally be called by the provider