Enable user to Join Households #1022

martha · 2025-01-13T18:43:06Z

Description

GH issue: https://github.com/open-path/Green-River/issues/5752

Depends on hmis-warehouse PR: greenriver/hmis-warehouse#5047

Summary of changes:

Implement frontend workflow for joining household (Figma)
Add StepDialog component. For the time being, this just uses MUI's out-of-the-box tabber, not the newly designed one.
Add ability to use GenericTable's selection status as a controlled prop.
Add option to use useClientAlerts hook with an existing household from memory.
Add ability for the DynamicForm caller to both
- filter out certain errors, in order to takeover rendering separately outside of the ErrorAlert
- and listen on error change, in order to respond appropriately
Add a "disabled reason" on the FormDialogActionContent component, which is displayed as a tooltip when the button is disabled

Type of change

Bug fix
New feature (adds functionality)
Code clean-up / housekeeping
Dependency update

Checklist before requesting review

I have performed a self-review of my code
I have run the code that is being changed under ideal conditions, and it doesn't fail
My code includes comments and/or descriptive variable names to help other engineers understand the intent (or not applicable)
My code follows the style guidelines of this project (eslint)
I have updated the documentation (or not applicable)
If it's not obvious how to test this change, I have provided testing instructions in this PR or the related issue

martha · 2025-01-13T18:52:30Z

src/modules/household/components/elements/AddToHouseholdButton.tsx

@@ -56,6 +66,11 @@ const AddToHouseholdButton = ({
      inputVariables: { projectId, clientId },
      pickListArgs: { projectId, householdId },
      localConstants: { householdId, projectCocCount: cocCount },
+      onFieldChange: (linkId?: string, value?: any) => {


@gigxz as you can see, this is still a draft with many todos, but this is the key part I'd like early feedback about!

Thanks @martha I will take a closer look. I agree with you that RHF may make this a bit different, for example maybe we could hook into the RHF watch functionality.

I'll explain the backend alternative I was thinking of, too, just to give some more detail. This is an approach where the backend would be solely responsible for determining whether there are conflicts:

Similarly to how the Delete Client returns some structured JSON data related to a warning, the SubmitForm mutation could return structured data attached to the validation error Client has another enrollment in this project that conflicts with this entry date. that it already returns. That could be implemented here by adding something like data : { conflicting_enrollment_id: 5 } to the validation error.

If -- like with DeleteClient -- we included ALL the relevant information about conflicting enrollment(s), the validation error itself would have sufficient data on it for us to show the Conflicting Enrollment alert.

With this approach,

There wouldn't be a need to do additional background queries based on the currently selected Entry Date

The user wouldn't be made aware of the conflict (and the option to "join") until they have already tried to submit the form

The logic for whether enrollments conflict or not would remain in one place (the Enrollment Validator here, which relies on the somewhat complicated with_conflicting_dates scope.) Since the logic is only in one place, we don't have to worry about making sure that the Backend and Frontend equivalents of "with conflicting dates" have perfectly matched logic. (Mis-matched logic would result in the backend error being surfaced but the frontend not showing the workflow option, or vice versa).

Once the Conflicting Enrollment alert is shown, the user would have the option to initiate the join, or change the Entry Date to something valid and re-submit.

Thank you for the feedback @gigxz. I've made a start on your idea -- it's a bit rough, but I think something like this will be necessary to selectively override the FormDialog's default rendering of its error messages.

I also have a question about this part:

If -- like with DeleteClient -- we included ALL the relevant information about conflicting enrollment(s), the validation error itself would have sufficient data on it for us to show the Conflicting Enrollment alert.

Am I understanding the following correctly?

the data attr of an error returned from the backend is just json, but can't be easily typed using HmisSchema objects

you're suggesting the error's data attr should return everything needed to render the ConflictingClientAlert, but not return all the data needed for the whole JoinHouseholds workflow, right?

If I got that right, then, I think I'd propose removing (descoping) the sentence "There [is/are] also [n] other household member[s] associated: [Additional Clients]" from the mocks here. It seems preferable to me to rely on our existing logic that lives in the schema objects for determining whether the current user has access to those clients, and/or access to view their names. The user will see those clients' names on the next step anyway, if they initiate the join workflow, so including them here seems more complicated than it's worth to me. (But maybe I am missing an easy way to do it.)

If I got that wrong, and you really did intend for the conflicting enrollment error message to include all the data necessary for the JoinHouseholds workflow -- including client alerts of the associated household members, nested data that is expected in a certain format/type in the common column definitions we are using, etc. -- then I'll have some followup questions about how the typing should work (hand-pick the data we need in the enrollment validator, then cast on the frontend to the relevant gql typescript types? is there a better way?)

Thanks for your help!

@martha good question, I didn't mean typing the data field but rather just including enough information for the explanatory alert. If it makes sense to descope some content from the alert, I think that's OK. And it seems fine to me that the "join household workflow" itself would need to fetch additional data.

It is a bit brittle to have this un-typed contract between the backend and the frontend regarding what the supplemental error data looks like. The frontend should be as gracious as possible of course, so if the data is missing or malformed it just shows the default message.

src/api/operations/client.queries.graphql

martha · 2025-01-17T17:22:49Z

src/modules/household/components/elements/JoinHouseholdDialog.tsx

+          receivingHouseholdId: receivingHousehold.id,
+          joiningEnrollmentInputs: joiningClients.map((hc) => {
+            return {
+              // todo @martha - discuss, should we use enrollmentLockVersion?


Let's discuss whether to use enrollmentLockVersion here. I think I would propose de-scoping it and maybe ticketing it for a future improvement.

martha · 2025-01-17T17:23:06Z

src/modules/household/components/elements/JoinHouseholdAddRelationships.tsx

+      <Typography variant='body1'>
+        Update joining clients' relationships{' '}
+        {receivingHohName && <>to {receivingHohName}</>}
+        {/* todo @martha - add warning here about entry dates, pending conversation with design */}


Pending https://www.figma.com/design/kqZiUPjXvqSvJVslJCnlTB?node-id=9475-76147#1096321325

martha · 2025-01-17T17:28:02Z

src/components/elements/DateWithRelativeTooltip.tsx

@@ -54,7 +54,10 @@ const DateWithRelativeTooltip = ({
      >
        {formattedDate}
        {/* Include the tooltip text as visually hidden for accessibility */}
-        <Box sx={visuallyHidden}>, {formattedDateRelative}</Box>
+        {/* Add position: fixed to address visual bug when visuallyHidden is used inside dialog */}


Seems like when visuallyHidden is used inside a dialog, it causes some surprising scroll behavior. I wasn't able to repro it on stackblitz using the latest mui version. I would propose cutting a separate ticket to track this down later (and see if it impacts anywhere else in our app, since we use visuallyHidden all over the place)

Edit: Created https://github.com/open-path/Green-River/issues/7233

martha · 2025-01-17T17:28:21Z

src/modules/household/components/elements/AddToHouseholdButton.tsx

-        preFormComponent: clientAlerts.length > 0 && (
-          <ClientAlertStack clientAlerts={clientAlerts} />
-        ),
+        // This wrapper is necessary around the preFormComponent Stack,


I can create a separate ticket to improve this, as well

Edit: created https://github.com/open-path/Green-River/issues/7234

martha · 2025-01-17T17:29:58Z

src/modules/household/components/elements/AddToHouseholdButton.tsx

@@ -1,38 +1,46 @@
-import { Button } from '@mui/material';


Question about our directory structure here -- what is src/modules/household/components/elements/? Should components/elements be flattened into components?

martha · 2025-01-17T17:31:24Z

@gigxz ok! There is still some work to be done on the backend draft PR (including a system test), but I think this frontend work is now ready for your review. Thank you 🙏

martha · 2025-01-24T20:20:41Z

@gigxz I'm converting this back to draft for now. As I've been working more on the Split workflow, I've been realizing how much code the 2 workflows share, and thinking I can do a better job of factoring this into reusable components to reduce code duplication. 🤔 I want to take a proper stab at that with a fresh brain Monday morning.

Take a first stab at household join

986a820

martha commented Jan 13, 2025

View reviewed changes

gigxz reviewed Jan 13, 2025

View reviewed changes

src/api/operations/client.queries.graphql Outdated Show resolved Hide resolved

martha added 12 commits January 15, 2025 12:19

Determine on server side whether there is a conflicting enrollment

1d3bb68

Move query for conflicting enrollment into join households dialog

5ff4c3f

Merge branch 'deploy/6761-table-actions' into martha/5752-join-hh

cb86d9b

Progress the ux and resolve some todos (still wip)

5233e35

Resolve some todos (still wip)

caff6f9

Add client alerts to join workflow

39def83

Fix some bugs and todos

11b23f2

Set button to Added

d2968b0

Fix some todos, wip

daabab9

Re-add explanatory text

03c17ff

Hide workflow if no permission

2feaf4f

Update comment

348531b

martha commented Jan 17, 2025

View reviewed changes

martha marked this pull request as ready for review January 17, 2025 17:23

martha commented Jan 17, 2025

View reviewed changes

Revert unused schema updates

c9f2503

martha mentioned this pull request Jan 17, 2025

Implement Join Household mutation greenriver/hmis-warehouse#5047

Open

11 tasks

Update to match mocks

e354d9d

martha changed the title ~~Take a first stab at household join~~ Enable user to Join Households Jan 20, 2025

martha added 4 commits January 20, 2025 15:54

Add aria labels for a11y and system test

1d12df3

Fix aria label location

95059cc

Make GenericTable more friendly to table inputs

dfd7469

Correct mistake with aria labels

7c8465c

martha added 5 commits January 21, 2025 11:39

Add household sorting

d94bd31

Update to match mocks

c2d8bf4

Merge branch 'deploy/6761-table-actions' into martha/5752-join-hh

138a3e1

Renames

1ba0159

Fix typescript from merge

01dd94c

martha marked this pull request as draft January 24, 2025 20:19

Refactor for reusability

0c799c7

martha marked this pull request as ready for review January 27, 2025 14:43

Merge branch 'deploy/6761-table-actions' into martha/5752-join-hh

9895852

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable user to Join Households #1022

Enable user to Join Households #1022

martha commented Jan 13, 2025 •

edited

Loading

martha Jan 13, 2025

gigxz Jan 13, 2025

martha Jan 15, 2025

gigxz Jan 16, 2025 •

edited

Loading

martha Jan 17, 2025

martha Jan 17, 2025

martha Jan 17, 2025 •

edited

Loading

martha Jan 17, 2025 •

edited

Loading

martha Jan 17, 2025

martha commented Jan 17, 2025

martha commented Jan 24, 2025

Enable user to Join Households #1022

Are you sure you want to change the base?

Enable user to Join Households #1022

Conversation

martha commented Jan 13, 2025 • edited Loading

Description

Type of change

Checklist before requesting review

martha Jan 13, 2025

Choose a reason for hiding this comment

gigxz Jan 13, 2025

Choose a reason for hiding this comment

martha Jan 15, 2025

Choose a reason for hiding this comment

gigxz Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

martha Jan 17, 2025

Choose a reason for hiding this comment

martha Jan 17, 2025

Choose a reason for hiding this comment

martha Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

martha Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

martha Jan 17, 2025

Choose a reason for hiding this comment

martha commented Jan 17, 2025

martha commented Jan 24, 2025

martha commented Jan 13, 2025 •

edited

Loading

gigxz Jan 16, 2025 •

edited

Loading

martha Jan 17, 2025 •

edited

Loading

martha Jan 17, 2025 •

edited

Loading