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

MPDX-8383 - Complete Task Modal - preselect result #1148

Merged
merged 5 commits into from
Oct 25, 2024

Conversation

wjames111
Copy link
Contributor

Description

  • Jira Ticket 8383
  • Description: In the complete task modal, if there is only one task result option, we can preselect it to speed up completing tasks for users.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

@wjames111 wjames111 added On Staging Will be merged to the staging branch by Github Actions Preview Environment Add this label to create an Amplify Preview labels Oct 22, 2024
@wjames111 wjames111 self-assigned this Oct 22, 2024
@wjames111 wjames111 requested a review from dr-bizz October 22, 2024 20:42
Copy link
Contributor

Copy link
Contributor

github-actions bot commented Oct 22, 2024

Bundle sizes [mpdx-react]

Compared against 0162a33

No significant changes found

Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

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

This is great! Thank you for fixing it so fast!

Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

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

I don't think this is quite right. It shows the correct value in the UI but doesn't seem to actually select it because displayResult is being sent as null.

It also isn't showing the Next Action autocomplete.

@wjames111
Copy link
Contributor Author

@canac good catch, what about something like this. I'm sure there's a better way but I can't think of anything at the moment.
useEffect(() => { if (availableResults.length === 1) { handleResultChange({ result: availableResults[0], setFieldValue, setResultSelected, phaseData, }); } }, [availableResults]);

@canac
Copy link
Contributor

canac commented Oct 23, 2024

@wjames111 Yeah, I think that's exactly what you'll want to do.

Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

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

Almost there!

Comment on lines 91 to 92
expect(getByRole('combobox', { name: 'Result' })).toBeInTheDocument();
expect(getByRole('combobox', { name: 'Result' })).toHaveValue('Attempted');
Copy link
Contributor

Choose a reason for hiding this comment

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

The getByRole for the value check is enough to also assert that it is in the document.

Suggested change
expect(getByRole('combobox', { name: 'Result' })).toBeInTheDocument();
expect(getByRole('combobox', { name: 'Result' })).toHaveValue('Attempted');
expect(getByRole('combobox', { name: 'Result' })).toHaveValue('Attempted');

@@ -63,7 +63,7 @@ export const handleTaskPhaseChange = ({
setFieldValue('taskPhase', phase);
setFieldValue('activityType', '');
setFieldValue('subject', '');
setFieldValue('displayResult', undefined);
setFieldValue('displayResult', '');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this change is causing an error when I try to add or edit a task:

Variable $attributes of type TaskUpdateInput! was provided invalid value for displayResult (Expected "" to be one of: INITIATION_RESULT_NO_RESPONSE, INITIATION_RESULT_CIRCLE_BACK, INITIATION_RESULT_APPOINTMENT_SCHEDULED, INITIATION_RESULT_NOT_INTERESTED, FOLLOW_UP_RESULT_NO_RESPONSE, FOLLOW_UP_RESULT_PARTNER_FINANCIAL, FOLLOW_UP_RESULT_PARTNER_SPECIAL, FOLLOW_UP_RESULT_PARTNER_PRAY, FOLLOW_UP_RESULT_NOT_INTERESTED, APPOINTMENT_RESULT_CANCELLED, APPOINTMENT_RESULT_FOLLOW_UP, APPOINTMENT_RESULT_PARTNER_FINANCIAL, APPOINTMENT_RESULT_PARTNER_SPECIAL, APPOINTMENT_RESULT_PARTNER_PRAY, APPOINTMENT_RESULT_NOT_INTERESTED, PARTNER_CARE_COMPLETED)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So for some reason leaving it as undefined breaks it and I can't quite figure out why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifically on Log Task.

@wjames111 wjames111 force-pushed the MPDX-8383-preselect-single-task branch from c4d6631 to 3dff50e Compare October 24, 2024 19:55
@canac
Copy link
Contributor

canac commented Oct 24, 2024

@wjames111 I think we can fix the bug where the result is populated before the user picks an action by passing an empty array to availableResults when the activityType is missing.

Sorry, I thought you had requested another review. I'm sure you were going to get to that bug, but I'm just mentioning what I found since I already spent the time looking at the code.

The rest looks great once you get all the tests passing again!

diff --git a/src/components/Task/Modal/Form/Complete/TaskModalCompleteForm.tsx b/src/components/Task/Modal/Form/Complete/TaskModalCompleteForm.tsx
index 7b38566fc..d572c847a 100644
--- a/src/components/Task/Modal/Form/Complete/TaskModalCompleteForm.tsx
+++ b/src/components/Task/Modal/Form/Complete/TaskModalCompleteForm.tsx
@@ -350,7 +350,7 @@ const TaskModalCompleteForm = ({
               </Grid>
 
               <ResultAutocomplete
-                availableResults={availableResults}
+                availableResults={activityType ? availableResults : []}
                 result={displayResult}
                 setFieldValue={setFieldValue}
                 setResultSelected={setResultSelected}
diff --git a/src/components/Task/Modal/Form/LogForm/TaskModalLogForm.tsx b/src/components/Task/Modal/Form/LogForm/TaskModalLogForm.tsx
index 98babc1e2..fcf96c4e4 100644
--- a/src/components/Task/Modal/Form/LogForm/TaskModalLogForm.tsx
+++ b/src/components/Task/Modal/Form/LogForm/TaskModalLogForm.tsx
@@ -421,7 +421,7 @@ const TaskModalLogForm = ({
               )}
 
               <ResultAutocomplete
-                availableResults={availableResults}
+                availableResults={activityType ? availableResults : []}
                 result={displayResult}
                 setFieldValue={setFieldValue}
                 setResultSelected={setResultSelected}
diff --git a/src/components/Task/Modal/Form/TaskModalForm.tsx b/src/components/Task/Modal/Form/TaskModalForm.tsx
index fbf817d9b..aa4920862 100644
--- a/src/components/Task/Modal/Form/TaskModalForm.tsx
+++ b/src/components/Task/Modal/Form/TaskModalForm.tsx
@@ -583,7 +583,7 @@ const TaskModalForm = ({
               </Grid>
               {initialTask.completedAt && (
                 <ResultAutocomplete
-                  availableResults={availableResults}
+                  availableResults={activityType ? availableResults : []}
                   result={displayResult}
                   setFieldValue={setFieldValue}
                   setResultSelected={setResultSelected}

@wjames111
Copy link
Contributor Author

@canac sorry didn't mean to request your review yet. But thank you, that is a much easier solution then I was thinking.

@wjames111
Copy link
Contributor Author

@canac I did notice that the Result autocomplete is hidden while availableResults is empty. Not sure how often users would need to skip down the form but if it's an issue I could just check for the activityType in the useEffect.

@wjames111 wjames111 requested a review from canac October 24, 2024 21:22
Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

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

This is great! As a user, I'm looking forward to saving some clicks when I complete tasks!

@wjames111 wjames111 merged commit 9f1e8f2 into main Oct 25, 2024
17 of 18 checks passed
@wjames111 wjames111 deleted the MPDX-8383-preselect-single-task branch October 25, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On Staging Will be merged to the staging branch by Github Actions Preview Environment Add this label to create an Amplify Preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants