[WEB-4545] editable fields in SMART-on-FHIR

[krystophv]

for WEB-4545, adjusts editable fields in SMART-on-FHIR

Summary:

This pull request updates the behavior of the PatientForm and EditPatientDialog components to support a new "smartOnFhirMode". When this mode is enabled (based on the presence of a smartCorrelationId), only certain fields are editable, and the "Save Changes" button remains enabled if the form is valid. The changes also update related unit tests to reflect this new logic.

The most important changes are:

PatientForm behavior updates:

• Added a new smartOnFhirMode prop to PatientForm, which, when true, disables editing of identity fields (Full Name, Birthdate, MRN, Email), while leaving clinic-related fields (Tags, Sites, Target, DiabetesType) enabled. [1] [2] [3] [4] [5] [6] [7]

• Updated the prop usage in EditPatientDialog to pass smartOnFhirMode instead of isReadOnly, and changed the logic so that the "Save Changes" button is only disabled if the form is invalid, not when in smartOnFhirMode. [1] [2]

Unit test updates:

• Added and updated tests in PatientForm.test.js to verify that identity fields are disabled and clinic fields are enabled when smartOnFhirMode is true.

• Updated the EditPatientDialog.test.js mock and tests to check for the new data-smart-on-fhir-mode attribute, and to ensure the "Save Changes" button is enabled when smartCorrelationId is present. [1] [2] [3]

[Copilot]

Pull request overview

Updates patient editing UX for SMART-on-FHIR contexts by introducing a dedicated smartOnFhirMode that locks identity fields while keeping clinic-related fields editable, and adjusts dialog button-disable logic + tests accordingly.

Changes:

• Added smartOnFhirMode prop to PatientForm to disable identity fields (name/DOB/MRN/email) when enabled.
• Updated EditPatientDialog to pass smartOnFhirMode and keep “Save Changes” enabled in SMART-on-FHIR mode as long as the form is valid.
• Updated unit tests for both components to reflect the new prop and editability rules.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
app/components/navpatientheader/EditPatientDialog.js	Passes smartOnFhirMode to PatientForm and adjusts Save button disable logic.
app/components/clinic/PatientForm/PatientForm.js	Introduces smartOnFhirMode and applies it to disable identity inputs while leaving clinic fields governed by isReadOnly.
tests/unit/components/navpatientheader/EditPatientDialog.test.js	Updates mocks/assertions for smartOnFhirMode and Save button behavior in SMART-on-FHIR mode.
tests/unit/components/clinic/PatientForm/PatientForm.test.js	Adds coverage asserting identity fields are disabled and clinic fields enabled when smartOnFhirMode=true.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[henry-tp]

Hmm, I don't think I'm a fan of this prop as a way to gate these fields. In the app we currently have 7 uses of <PatientForm /> but only <EditPatientDialog /> passes down the prop as a parent. It kind of begs the question - why don't the other instances get passed this prop as well, or why can't <PatientForm /> internally access the value of isSmartOnFhir from redux (which it could). The <PatientForm /> component itself also needs implicit knowledge that isSmartOnFhir means that it is being used in an "editing a patient" context.

IMO it makes more sense within the EditPatientDialog component to tie together the concepts of "If editing a patient, and the app is in smart-on-fhir mode, the certain fields should be locked". It would help keep <PatientForm /> generic and require less implicit knowledge to be utilized.

That being said, I struggle to come up with an interface that is not overly complex. Maybe something like:

// In EditPatientDialog.js
<PatientForm
  // ...
  disabledFields={{
    fullName: true,
    birthDate: true
  }}
/>

// In PatientForm.js
const { disabledFields = {} } = props; 

return (
<TextInput
  name="fullName"
  disabled={disabledFields.fullName}
/>);

If you don't agree or think this is too complicated, I won't push back and we can move forward as-is, but I did want share my opinion on this

[krystophv]

I actually like this a lot. It'd keep the implicit knowledge of what "smart-on-fhir" mode (or whatever else situation comes up) is out of the PatientForm and make it easier to reuse/customize elsewhere.

[henry-tp]

Given that there are no longer any uses of isReadOnly, can we remove it from this component entirely?

[krystophv]

Yup. 👍🏼

[henry-tp]

I had some thoughts on the interface

[henry-tp]

Looks great, thank you!