feat: MI Assign home screen (M2-7405) #530
Conversation
In prep for adjusting `ActivityGroupsBuildManager` to accommodate assignments, refactor logic slightly to eliminate multiple calls to `filter`. Also rename identifiers to be make logic easier to follow and stay consistent with use of `EventEntity` type elsewhere in app. Also remove debug statement in `useFeatureFlags`.
Update logic to take into account both auto-assigned and manually assigned activities and flows when generating list of activities/flows available on the applet's home screen. Falls back to existing approach if `enableActivityAssign` feature flag is disabled. Refactor activity/flow sorting into a single array `sort` call using a custom comparer, instead of the less efficient and harder to read approach involving multiple `filter` calls, a custom `sort`, followed by recominbation. This change doesn't account for tracking distinct in-progress states for each assignment of the same activity, or assignment-specific one-time completion status; that will be done in an upcoming commit.
Make shared `TargetSubjectLabel` component which will also be used in the activity screens. Also fix missing translations and missing/incorrect use of i18next plural forms in several `ActivityLabel` subcomponents.
Made widespread change to incorporate `targetSubjectId` into activity progres tracking, when assignments feature is enabled. Anytime `eventId` is used, now `targetSubjectid` also needs to be passed to identify which activity assignment is the active assessment. This value is also passed to the `answers` POST endpoint to ensure the submission is also properly associated with the `targetSubjectId`. For self-reports, and when assignments feature is disabled, `targetSubjectId` is set to `null`, effectively inheriting existing functionality. For `PublicSurvey` and `PublicAutoCompletion`, `targetSubjectId` stays `null` since public assessments do not support activity assignments.
farmerpaul
force-pushed
the
feature/M2-7405-mi-assign-home-screen
branch
from
2c818e6 to
60f9071
Compare
farmerpaul
requested review from
sultanofcardio and
qiushihe
and removed request for
sultanofcardio
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
farmerpaul
force-pushed
the
feature/M2-7405-mi-assign-home-screen
branch
from
ebbd36b to
8f14baf
Compare
There was a problem hiding this comment.
Seems to be mostly adding an extra parameter/attribute to all the places, plus some renaming/refactoring of variables and types.
But the code looks good nonetheless 👍
@farmerpaul please let me know when you're done with the tests, and I'll give this another go.
farmerpaul
force-pushed
the
feature/M2-7405-mi-assign-home-screen
branch
from
951533e to
909efa3
Compare
Also fix tsconfig.node.json to exclude node_modules from tsc.
farmerpaul
force-pushed
the
feature/M2-7405-mi-assign-home-screen
branch
from
909efa3 to
a3cf7d9
Compare
There was a problem hiding this comment.
Amazing work @farmerpaul! Have not reviewed the code but performed a round of peer testing which was mostly successful. All passing with followup fix:
✅ Auto-assigned activities & flows display
✅ Manually assigned activities & flows don't display when not assigned
✅ Manually assigned activities & flows display when assigned
✅ Manually assigned activities & flows inherit the existing global schedule
✅ Manually assigned activities & flows inherit the existing individual schedule
✅ Assigned activity (non self-report) sees a badge with question counter
Expected "13 Questions" as specified in Figma. Actual "13 questions" (lower case 'q'). Is Figma incorrect?
✅ Assigned flow (non self-report) sees a badge with activity counter
Expected same as above, "2 Activities" (couldn't find example in Figma but inferring from activities). Actual "2 activities" (lower case 'a'). Is Figma incorrect?
✅ Multiple assignments for the same activity / flow displayed for each subject
✅ Sorting available->scheduled, activity/flow order in admin panel, self report -> additional subjects
✅ Each assignment preserves its own unique in-progress state
Works for self-reports, but could not test with MI assigned activities. Starting activity results in BE returning 403 on call to/subjects/{_id}endpoint. Is this to be implemented in M2-7475 to skip calling this endpoint in favor of getting subject info from assignment?
✅ One time completion, single assignment activity disappears after being completed
Works for self-reports, but could not test with MI assigned activities (same issue as above)
✅ One time completion, multiple assignment activity disappears after being completed (per assignment)
Works for self-reports, but could not test with MI assigned activities (same issue as above)
There was a problem hiding this comment.
Will leave to @qiushihe to complete their review of the code. Peer tested successfully 🎉 🎉 🎉
📝 Description
🔗 Jira Ticket M2-7405
Adds support for displaying assigned activities and flows in the Web App, including:
TargetSubjectLabelchip component on multi-informant assignmentsActivityCardfor the same activity/flow when user has multiple assignments for that activity/flowautoAssign: trueas well as have manual assignments, and the web app respects this possibility. In a future Admin App ticket, UI restrictions will be imposed to make auto-assign and manual assignments mutually exclusive.targetSubjectIdreturned by the/answers/applet/{applet_id}/completionsendpoint to compare assignment has already been completed that dayAlso fixes a couple missing translations and incorrect usage of i18next plural forms.
Tip
Although many files were touched in this PR, many of those constituted tying
targetSubjectIdto activity progress in this commit, and on the whole were somewhat repetitive. Strongly typing thetargetSubjectandtargetSubjectIdprops to be required prop (cannot beundefined) made it easier to make that change in a comprehensive way. I attempted to group other related changes into their own commits and it may be easiest to review commit-by-commit.📸 Screenshots
CleanShot.2024-08-30.at.11.37.22.mp4
🪤 Peer Testing
Note
This feature depends on the
enableActivityAssignfeature flag. This should already be enabled in dev/local environments. It would also be worth testing this with that feature flag disabled to confirm that existing behaviour is preserved with the MI Assign feature disabled, by addingfeatures.enableActivityAssign = falseprior to thereturnstatement here inuseFeatureFlags.Please refer to ticket's AC for the main peer testing steps.
In addition, after completing a multi-informant assigned activity, confirm that the POST to the
answersendpoint includes the appropriatetargetSubjectId. Also confirm that for self-reports, that property is not included.