Skip to content

(fix) O3-5408 : Form Builder crashes on creation of some forms#1082

Open
RajPrakash681 wants to merge 4 commits intoopenmrs:mainfrom
RajPrakash681:fix/person-attribute-clean
Open

(fix) O3-5408 : Form Builder crashes on creation of some forms#1082
RajPrakash681 wants to merge 4 commits intoopenmrs:mainfrom
RajPrakash681:fix/person-attribute-clean

Conversation

@RajPrakash681
Copy link
Contributor

@RajPrakash681 RajPrakash681 commented Feb 8, 2026

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

This PR implements support for the personAttribute question type in the Form Builder and fixes a crash that occurred when selecting it. Previously, selecting this question type caused an application crash, and there was no UI to specific which person attribute type to use, leading to ambiguous "name name" errors.

video

Screen.Recording.2026-02-08.200601.mp4

Related Issue

https://issues.openmrs.org/browse/O3-5408

Other

NA

Fixes crash and adds UI for person attribute selection.
@RajPrakash681
Copy link
Contributor Author

@denniskigen please have a look at this

testOrder: TestOrderTypeQuestion,
};

const PersonAttributeTypeQuestion: React.FC = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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


return (
<div>
<FormLabel style={{ display: 'block', marginBottom: '0.5rem' }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please avoid inline styling

const handlePersonAttributeValidation = async (question, errors) => {
if (question.type === 'personAttribute' && !question.questionOptions.attributeType) {
errors.push({
errorMessage: `❓ Person attribute type missing in schema`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

All these error messages should be translated

Copy link
Collaborator

@NethmiRodrigo NethmiRodrigo left a comment

Choose a reason for hiding this comment

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

So a couple of things:

  1. In the recording in your PR description, when you select a person attribute type from the dropdown, the entire app refreshed and that definitely should not happen
  2. There’s something very wrong and I’m not sure what that is, but if you create a personAttribute type question, it doesn’t render at all in the preview. Here is a sample question you can try. Maybe we need to fix this in the form engine itself? I’m not sure, but can you talk about this in #react-form-engine on Slack?
  3. When you edit a personAttribute type question, you’d see that the fields that you selected like the attributeType aren’t populated.
  4. There are some missing translation keys that haven’t been added to en.json

@RajPrakash681
Copy link
Contributor Author

RajPrakash681 commented Feb 11, 2026

So a couple of things:

  1. In the recording in your PR description, when you select a person attribute type from the dropdown, the entire app refreshed and that definitely should not happen
  2. There’s something very wrong and I’m not sure what that is, but if you create a personAttribute type question, it doesn’t render at all in the preview. Here is a sample question you can try. Maybe we need to fix this in the form engine itself? I’m not sure, but can you talk about this in #react-form-engine on Slack?
  3. When you edit a personAttribute type question, you’d see that the fields that you selected like the attributeType aren’t populated.
  4. There are some missing translation keys that haven’t been added to en.json

ok so the app refreshing issue i have fixed will proceed accordingly!

This commit fixes two critical issues with person attribute type questions:

1. App refresh on selection: Removed conflicting initialSelectedItem prop
   and added useEffect to properly sync selectedPersonAttributeType state
   when personAttributeTypes data loads asynchronously.

2. Fields not populated on edit: State now properly initializes to null
   and syncs via useEffect when data becomes available, ensuring the
   ComboBox displays the correct selected value when editing existing
   questions.

Changes:
- Added useEffect hook to sync state with async data
- Removed initialSelectedItem from ComboBox (controlled component pattern)
- Changed initial state from undefined to null for proper initialization
- Added missing translation keys to en.json

This addresses maintainer feedback about app refreshing and fields not
populating when editing person attribute questions.
@RajPrakash681 RajPrakash681 force-pushed the fix/person-attribute-clean branch from 853a7ca to 00c053a Compare February 11, 2026 14:19
This commit fixes two critical issues with person attribute type questions:

1. App refresh on selection: Removed conflicting initialSelectedItem prop
   and added useEffect to properly sync selectedPersonAttributeType state
   when personAttributeTypes data loads asynchronously.

2. Fields not populated on edit: State now properly initializes to null
   and syncs via useEffect when data becomes available, ensuring the
   ComboBox displays the correct selected value when editing existing
   questions.

Changes:
- Added useEffect hook to sync state with async data
- Removed initialSelectedItem from ComboBox (controlled component pattern)
- Changed initial state from undefined to null for proper initialization
- Added missing translation keys to en.json
@RajPrakash681
Copy link
Contributor Author

Screen.Recording.2026-02-11.194257.mp4

and about the rendering issue yes that's a problem i will ask that on slack and i think we need a separate ticket for that

@RajPrakash681
Copy link
Contributor Author

@NethmiRodrigo i asked ian about adding PersonAttributeAdapter support and he said we should have it so yeah i will create a ticket accordingly

@RajPrakash681
Copy link
Contributor Author

@NethmiRodrigo i asked ian about adding PersonAttributeAdapter support and he said we should have it so yeah i will create a ticket accordingly

is it good to go as per the changes i made in order to fix the refreshing issue ?

@NethmiRodrigo
Copy link
Collaborator

is it good to go as per the changes i made in order to fix the refreshing issue ?

I think we should merge this in after your fix to the form-engine, the way I see it, there’s no point in the form builder supporting person attribute types if the form engine doesn’t. For now, what we should do is update the form builder so that it doesn’t crash for unsupported question types and just display a notification in the question modal saying that the form builder doesn’t support creating questions of this type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants