React autosave: patient dashboard form (bugs fixed 🙂 ) #3337

mercedesb · 2025-01-10T21:54:58Z

I rule and have completed some work on Case Manager that's ready for review!

This PR rewrites the patient dashboard form in React with safe autosave. This is mostly the same as #3216 but with the identified bugs fixed. Fixes #3240

Let me know if anything else pops up. I'm sorry for the long delay on this 🫣

For reviewer:

Adjust the title to explain what it does for the notification email to the listserv.
Tag this PR:
- feature if it contains a feature, fix, or similar. This is anything that contains a user-facing fix in some way, such as frontend changes, alterations to backend behavior, or bug fixes.
- dependencies if it contains library upgrades or similar. This is anything that upgrades any dependency, such as a Gemfile update or npm package upgrade.
If it contains neither, no need to tag this PR.

lomky · 2025-01-16T23:12:15Z

I will get this into sandbox and tested shortly!

Fixes #3240

colinxfleming

this is looking pretty good to me. I want to have @lomky take a look at it real quick but then I'm comfortable rolling with it. One minor regression noted in a comment but (as noted) I'm good to roll with this.

Sorry again for the review delay!

colinxfleming · 2025-02-25T03:13:31Z

test/system/update_patient_info_test.rb

    describe 'updating name' do
      before do
        fill_in 'First and last name', with: 'Susie Everyteen 2'
-        click_away_from_field


honestly dope

colinxfleming · 2025-02-25T03:18:23Z

app/javascript/src/components/PatientDashboardForm.jsx

-    return debounce(autosave, 300)
-  }, []);
+  const debouncedAutosave = (params) => {
+    return debounce(autosave(params), 300)


this works like a charm for me fysa

colinxfleming · 2025-02-25T03:29:57Z

app/views/patients/_patient_dashboard.html.erb

+    weeksOptions: weeks_options.map { |opt| { label: opt[0], value: opt[1] } },
+    daysOptions: days_options.map { |opt| { label: opt[0], value: opt[1] } },
+    initialCallDate: patient.initial_call_date.strftime("%m/%d/%Y"),
+    statusHelpText: status_help_text(patient),


at some point we'll need to find a way to re-rack this from changes (easiest example on main: setting an appointment date makes the status Fundraising, removing appointment dates makes the status Needs Appointment. That said I'm perfectly fine letting this regress a bit and merging this, and tackling this later, because I'd like to start running this react code in prod again. (Please make an issue if we do that so we don't forget.)

I'm not totally following your comment but I think that's b/c I'm not as familiar with all the parts of the app as you are. Can you clarify by what you mean by "this"? The status help text? Or the status input itself?

yeah, the status help text -- the content changes based on the state of the patient (and I believe that's a model field right now). On prod now it refreshes after a change without a page reload.

cool cool, I went ahead and fixed that in this branch 🤗 hopefully no regressions now

colinxfleming · 2025-02-25T03:35:40Z

app/javascript/src/components/PatientDashboardForm.jsx


  const autosave = async (updatedData) => {
    const updatedPatientData = { ...patientData, ...updatedData }
-    setPatientData(updatedPatientData)


this is the main culprit right?

actually the change below with the debounce was the main culprit. a combo of the useMemo and not passing the params through to the autosave function

ah, good to know! makes sense

I really need to learn to just stop reaching for useMemo. I mess it up nearly every time and it has cost me hours of my life 😅

mercedesb · 2025-02-26T03:21:51Z

Looks like I'm running into test failures due to the debounced save and ajax stuff with Capybara. I'll continue looking into this this week.

lomky · 2025-02-26T16:26:05Z

I'm seeing a small bug when I try to set "0 days" in the weeks along settings, it blanks it out. I suspect its something to do with the javascript -> ruby translation with "whats a false", but I haven't dug in more

mercedesb · 2025-02-26T16:30:43Z

I'm seeing a small bug when I try to set "0 days" in the weeks along settings, it blanks it out. I suspect its something to do with the javascript -> ruby translation with "whats a false", but I haven't dug in more

Great, I'll add that to my list when I'm resolving the specs issue

mercedesb · 2025-02-27T01:54:14Z

😮‍💨 ok I think we're good! finally. sorry for the avalanche of notifications. I've addressed all feedback

dynamic status text regression
blank '0 days' regression
spec failures
a host of mini bugs that we're not gonna talk about

have a great week!

lomky · 2025-03-13T00:50:39Z

All the functionality is looking great! I think we have one last visual to fix where the Days Along field is shifting up, but otherwise it seems to be working great for me and i'm so excited to get it merged!

in the old component, it had an invisible label pushing it down to be level with Weeks Along.

main:

branch:

mercedesb · 2025-03-16T17:25:00Z

Visual regression fixed.

I've rebased this branch on latest main but am getting a security vulnerability that needs to be fixed (the json gem) as well as specs related to auth which shouldn't be from anything this branch touches.

I'm going to let this be and hope that those issues are resolved in the base branch. Let me know if you need me to rebase or retrigger workflows this week.

colinxfleming · 2025-03-17T00:03:38Z

Popping in here to say re: build, there's currently SOMETHING up with github actions and capybara, this results in about 10 system test failures and we can ignore those for now. I also bumped the json gem on main, so if the regression is solved for I think we can smash merge even if build's not passing.

lomky · 2025-03-17T23:13:47Z

I'm sorry for more back and forth, but somethings just going wrong intermittently in my testing and I can't pin it down.

here's the two scenarios I've run into:

Days Along off-by-one

I created a new caller and filled in their info, including an initial weeks & days, and saved it. I went into their patient record and for some reason, the weeks/days are saved properly, but displaying wrong?

I created two more patients, but I cannot reproduce this bug. in the meantime, the bug persists on the first patient.

Field Changes sometimes 'saved' but then lost

I created a new patient, went into their record and updated the phone number. The green banner appeared.
I went back to the home page, the phone number wasn't changed. I went into the patient record, the phone number wasn't changed.
I changed the number again, and it gave a green banner. This time it did save and displayed properly on renavigating.
I cannot reproduce this error again on purpose, but saw it once another time on the Name field.

sheldon-b · 2025-03-19T23:17:23Z

app/javascript/src/components/Input.jsx

+  // if we don't have an onChange handler, this component is an uncontrolled component
+  // if we have an onChange handler, we'll manage state like a traditional controlled component
+  let value, setValue;
+  if (!onChange) {
+    value = initialValue
+  } else {
+    ([value, setValue] = useState(initialValue || ""))
+  }


Note after talking with Kat: I'd consider always calling the useState hook instead of conditionally, even if you're not using the value. This violates the "Rule of Hooks"

🔴 Do not call Hooks inside conditions or loops.

It may be fine since there's only ever one hook. But it may not be. I haven't actually tested it

Suggested change

// if we don't have an onChange handler, this component is an uncontrolled component

// if we have an onChange handler, we'll manage state like a traditional controlled component

let value, setValue;

if (!onChange) {

value = initialValue

} else {

([value, setValue] = useState(initialValue || ""))

}

// if we don't have an onChange handler, this component is an uncontrolled component

// if we have an onChange handler, we'll manage state like a traditional controlled component

let [value] = useState(initialValue || "")

if (!onChange) {

value = initialValue

}

This is a good point and something I went back and forth on a lot. I don't want to mutate the state variable directly either since that also breaks React conventions.

I've pushed up a more complex but more complete solution in today's commits.

mercedesb · 2025-03-23T18:33:19Z

@lomky

Re: "Field Changes sometimes 'saved' but then lost", I currently have a 300 ms debounce on the input. I'm not able to reproduce what you've described. But I can sometimes force a page reload faster than the 300 ms if I'm realllllllyyyyy trying. Could that be something you're running into? Seeing the green banner from a previous save and reloading the page before the new change can be committed? Want me to decrease the 300 ms debounce on the autosave?

Re: Days Along off-by-one
Can you clarify your screenshot? I'm trying to guess what it's supposed to say based on the timestamp of when you left your comment but I still don't know which part of the screenshot is correct and which isn't. Could you let me know what you expect to see in which box and where it's wrong? That would save me a lot of time tracking down whatever could be happening.

lomky · 2025-03-24T22:45:32Z

Re: "Field Changes" - I'm not trying especially hard, just typing and tabbing through fields! but that is pretty fast.

Re: Days Along off-by-one - The value selected is 2, but the display (see the weeks along below) says 3. That's true of the other places the value showed, too. But when I checked the console, it was saved properly as 2. If I changed the value, the display also changed, but still off by one. it was a weird bug and I couldn't reproduce it on another record, but it persisted on that record.

For overall context, I spent about 5-10 minutes just naviating around, typing, and changing things, and saw the off-by-one once and field change loss two or three times. So it wasn't even the most common occurence, but too often for my comfort, if that makes sense?

mercedesb · 2025-06-22T20:32:58Z

I think bugs are fixed?

I'm getting Selenium::WebDriver::Error::SessionNotCreatedError errors in the specs which seem to be affecting all the recent PRs so those failures aren't introduced by these changes.

mercedesb force-pushed the fix-patient-dashboard-react-bugs branch 3 times, most recently from c649765 to 5db331b Compare January 10, 2025 23:51

colinxfleming requested a review from lomky February 25, 2025 03:05

colinxfleming reviewed Feb 25, 2025

View reviewed changes

mercedesb force-pushed the fix-patient-dashboard-react-bugs branch 3 times, most recently from 8387079 to 8120369 Compare February 26, 2025 02:40

mercedesb force-pushed the fix-patient-dashboard-react-bugs branch from cd785c7 to 9134b28 Compare March 16, 2025 17:13

sheldon-b reviewed Mar 19, 2025

View reviewed changes

mercedesb force-pushed the fix-patient-dashboard-react-bugs branch from 9134b28 to 3d360f7 Compare March 23, 2025 18:33

lomky added this to the React Autosave milestone Mar 26, 2025

mercedesb added 7 commits June 22, 2025 14:51

Re-Reactify Patient Dashboard

eb72db1

fix identified bugs :)

234b1aa

fix styling of status

6a44583

remove click_away_from_field to get spec to pass

49b71fd

add initial_call_date_display to patient json

4578729

use initial_call_date_display from json instead of prop

a7799e0

allow Input to behave either like a controlled or uncontrolled component

fbcf4bb

mercedesb added 8 commits June 22, 2025 14:51

add status_help_text to update patient response

a3fecd8

simplify state mgmt in Input even more

b3a35a5

bugfix: when selected value is 0, dont set to empty string

3313fbd

fix debounce so it doesnt make a new timeout on every rerender

b6b5016

make integration spec helper to wait for debounce autosave to run

1f6aef5

use correct class name to offset select with sr-only label

fcd88a5

refactor Input to handle controlled/uncontrolled w/o conditional hooks

94c78ab

remove useCallback

5c3ea3b

mercedesb force-pushed the fix-patient-dashboard-react-bugs branch from 3d360f7 to 5c3ea3b Compare June 22, 2025 20:20

React autosave: patient dashboard form (bugs fixed 🙂 ) #3337

Are you sure you want to change the base?

React autosave: patient dashboard form (bugs fixed 🙂 ) #3337

Uh oh!

Conversation

mercedesb commented Jan 10, 2025 • edited by lomky Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lomky commented Jan 16, 2025

Uh oh!

colinxfleming left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mercedesb commented Feb 26, 2025

Uh oh!

lomky commented Feb 26, 2025

Uh oh!

mercedesb commented Feb 26, 2025

Uh oh!

mercedesb commented Feb 27, 2025

Uh oh!

lomky commented Mar 13, 2025

Uh oh!

mercedesb commented Mar 16, 2025

Uh oh!

colinxfleming commented Mar 17, 2025

Uh oh!

lomky commented Mar 17, 2025

Days Along off-by-one

Field Changes sometimes 'saved' but then lost

Uh oh!

sheldon-b Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mercedesb commented Mar 23, 2025

Uh oh!

lomky commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mercedesb commented Jun 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mercedesb commented Jan 10, 2025 •

edited by lomky

Loading

sheldon-b Mar 19, 2025 •

edited

Loading

lomky commented Mar 24, 2025 •

edited

Loading