(feat) O3-3712: add a concept answer to a concept in the form builder #351

Willie-theBeastMutua · 2024-09-11T21:10:38Z

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

Users are now be able to manually add specific concepts as answers to a concept of type Coded after searching of them (similar to how we add a backing concept)

Screenshots

Related Issue

https://openmrs.atlassian.net/browse/O3-3712

Other

NethmiRodrigo · 2024-09-18T06:21:05Z

Can you please add these changes to the edit-modal component as well?

Willie-theBeastMutua · 2024-09-18T06:47:00Z

Okay doing that

Willie-theBeastMutua · 2024-10-02T12:15:26Z

Hello @NethmiRodrigo , I added the changes to edit-modal component as requested

package.json

src/components/interactive-builder/add-question.modal.tsx

src/components/interactive-builder/question-modal.scss

NethmiRodrigo · 2024-10-07T09:18:46Z

@Willie-theBeastMutua can you please update your fork and fix the changes, some of the code has gotten reverted, maybe due to a bad rebase

Willie-theBeastMutua · 2024-10-07T11:27:17Z

Hello ** @NethmiRodrigo** the code has been updated and fixed kindly check

Willie-theBeastMutua · 2024-10-07T13:15:22Z

The merge you conducted reverted some of the changes in the main but i have fixed them i hope its now okay

NethmiRodrigo · 2024-10-08T05:54:32Z

Hey @Willie-theBeastMutua thank you for the progress. One minor detail, I see that we can't remove the answers once added. Can you add support to remove an added answer, and also add a check that you can't add the same concept multiple times as an answer?

Willie-theBeastMutua · 2024-10-08T08:40:15Z

@NethmiRodrigo support for deletion and checks for duplicate concept answers has been added kindly check

NethmiRodrigo · 2024-10-09T07:00:26Z

@Willie-theBeastMutua could you add like a small x icon to the tags so we could remove individual answers, instead of the trash icon removing all answers? We can remove the trash icon. And could you align the add icon button to be aligned with the search?

Also, the additional answers don't get added to the answers key in the form JSON, so when you click save, all the answers that were added gets lost

Willie-theBeastMutua · 2024-10-11T08:37:55Z

@NethmiRodrigo the changes have been made and it saves successfully

Willie-theBeastMutua · 2024-10-15T10:34:30Z

@NethmiRodrigo did you have a look

NethmiRodrigo · 2024-10-15T11:59:14Z

Sorry I couldn't re-review it, will do asap. Thanks for pinging me. Meanwhile, can you resolve the conflicts and update your branch?

Willie-theBeastMutua · 2024-10-15T12:32:11Z

@NethmiRodrigo branch has been updated

Willie-theBeastMutua · 2024-10-15T18:10:15Z

@NethmiRodrigo can we restart the test as it times out and when i test the same test from my end it should work. Or advice on what is failing....

NethmiRodrigo · 2024-10-16T07:18:05Z

@Willie-theBeastMutua I re-ran the tests but it failed again due to a metadata issue (unrelated to your changes).
One more minor thing, I can add an answer that's already added in the UI. This doesn't get added to the actual object so when I go into edit, it only shows one instance of the answer, but is there something we can do to restrict adding duplicates of the same answer

Willie-theBeastMutua · 2024-10-16T07:33:00Z

@NethmiRodrigo to get you right.. i added the check for duplicates to only save one instance. You want the same check to be available in the UI?

NethmiRodrigo · 2024-10-16T07:56:32Z

@Willie-theBeastMutua yep, exactly!

Willie-theBeastMutua · 2024-10-28T05:22:50Z

@NethmiRodrigo the checks have been added. Kindly check

NethmiRodrigo · 2024-10-29T08:13:17Z

Could you please update your branch @Willie-theBeastMutua?

NethmiRodrigo

Thanks @Willie-theBeastMutua, some very minor suggestions

src/components/interactive-builder/add-question.modal.tsx

NethmiRodrigo · 2024-10-29T08:49:22Z

src/components/interactive-builder/add-question.modal.tsx

    closeModal();
  };

-  const createQuestion = () => {
+  const createQuestion = useCallback(() => {


Just for clarification purposes, any particular reason as to why we changed this to a useCallback?

NethmiRodrigo · 2024-10-29T08:52:09Z

src/components/interactive-builder/add-question.modal.tsx

+  useEffect(() => {
+    if (isCreatingQuestion) {
+      createQuestion();
+      setIsCreatingQuestion(false);
+    }
+  }, [isCreatingQuestion, createQuestion]);


Can't we just call the handleCreateQuestion function, instead of having a useEffect to do it? See - https://react.dev/learn/you-might-not-need-an-effect

The use effect is used due to react's state management when combining the added answers and the selected answers. Saving without checking when the state is changed only saves the selected answers and not the update. I tried not using it but react's state management did not allow me. The call back is used to execute also the use effect. They are both part of the same solution

Ok I see. So instead of having a useEffect and useCallback we could change the handleSaveMoreAnswers to return the answers list, like so:

const handleSaveMoreAnswers = () => { const newAnswers = addedAnswers.filter( (newAnswer) => !selectedAnswers.some((prevAnswer) => prevAnswer.id === newAnswer.id), ); const updatedAnswers = [...selectedAnswers, ...newAnswers]; setSelectedAnswers(updatedAnswers); setaddedAnswers([]); return updatedAnswers; };

And then in the handleCreateQuestion, we pass in the list to the createQuestion function, like so:

const handleCreateQuestion = () => { const updatedAnswers = handleSaveMoreAnswers(); createQuestion(updatedAnswers); closeModal(); };

And accept change the createQuestion function to accept a parameter and then use that param for the object:

const createQuestion = (conceptAnswers) => { try { const questionIndex = schema.pages[pageIndex]?.sections?.[sectionIndex]?.questions?.length ?? 0; const computedQuestionId = `question${questionIndex + 1}Section${sectionIndex + 1}Page-${pageIndex + 1}`; const newQuestion = { label: questionLabel, ......, ...(conceptAnswers.length && { answers: conceptAnswers.map((answer) => ({ concept: answer.id, label: answer.text, })), }), }; } };

This way we could avoid the useCallback with a massive dependency list

let me try that

Co-authored-by: Nethmi Rodrigo <[email protected]>

Willie-theBeastMutua · 2024-10-29T09:07:46Z

Could you please update your branch @Willie-theBeastMutua?

Okay let me do that

Willie-theBeastMutua · 2024-10-31T09:20:59Z

@NethmiRodrigo I was able to remove the callback and useEffect according to your suggestion and it worked perfectly. Thank you so much
Kindly check the changes and advice.

src/components/interactive-builder/add-question.modal.tsx

Willie-theBeastMutua · 2024-11-01T08:52:25Z

@NethmiRodrigo the change has been made

NethmiRodrigo · 2024-11-04T05:50:31Z

src/components/interactive-builder/edit-question.modal.tsx

-                    )}
+                    ) : null}
+
+                    {(selectedConcept || questionToEdit) && questionToEdit?.questionOptions.answers?.length ? (


Because of the condition questionToEdit?.questionOptions.answers?.length, if a user when creating a question doesn't select any answer, they wouldn't be able to add any answers when editing either, and that shouldn't be the case.

@NethmiRodrigo sorry for replying late. This is what is used even in the add-question.modal. I have used it to ensure that the concept that was selected has a coded datatype. In the add-question modal if the API returns a concept without answers even if its datatype is coded then the select answers field will not be displayed.

That was my thinking based on what has been implemented. Unless you have another idea as to how we can implement it.

In the add-question-modal, this condition makes sense. In the edit version, if the user has not selected any answer during creation, they should still be able to add answers. What I meant was that we should just remove this condition and show the Add more answers if the concept is of the datatype coded

@NethmiRodrigo how do we check if the datatype is coded? Because even in add question, the check is done to see if the concept has answers there is no check for the datatype

So in the add-question-modal, the reason that there is no check for the datatype is because as soon as you search for a concept from the API and select a concept, if the concept has answers, it got added to a state, so the check for the datatype coded wasn't necessary (hacky way to do it) -

openmrs-esm-form-builder/src/components/interactive-builder/add-question.modal.tsx

Lines 160 to 170 in 5154c46

const handleConceptSelect = (concept: Concept) => {

const updatedDatePickerType = getDatePickerType(concept);

if (updatedDatePickerType) setDatePickerType(updatedDatePickerType);

setConceptToLookup('');

setSelectedConcept(concept);

setAnswers(

concept?.answers?.map((answer) => ({

concept: answer?.uuid,

label: answer?.display,

})),

);

Willie-theBeastMutua · 2024-11-15T16:44:43Z

@NethmiRodrigo I found a way to do it kindly check and advise

NethmiRodrigo · 2024-11-18T06:17:59Z

src/components/interactive-builder/edit-question.modal.tsx

@@ -875,7 +876,14 @@ const EditQuestionModal: React.FC<EditQuestionModalProps> = ({
                      </div>
                    ) : null}

-                    {(selectedConcept || questionToEdit) && questionToEdit?.questionOptions.answers?.length ? (
+                    {!hasConceptChanged && editConceptInfo?.datatype?.uuid == '8d4a48b6-c2cc-11de-8d13-0010c6dffd0f' ? (


Why do we have a hard-coded UUID?

@Willie-theBeastMutua please check my comment, just removing the conditional logic is sufficient, and we'll merge this in. Thanks!

Why do we have a hard-coded UUID?

The UUID id for the coded datatype that is the check

@Willie-theBeastMutua please check my comment, just removing the conditional logic is sufficient, and we'll merge this in. Thanks!

If I remove the logic, the more answers will display even when the concept is not of a coded datatype so I added a hook that collects data on the concept and I check for the UUID for the coded datatype. I can use the word "Coded" to check but my fear was what would happen if the language changed

Ignore my suggestion about simply removing the condition, I see why the hook is needed.
@ibacher if we do a datatype.display === "Coded", would that be language sensitive?

NethmiRodrigo self-requested a review September 12, 2024 08:31

NethmiRodrigo changed the title ~~add a concept answer to a concept in the form builder~~ (feat) O3-3712: add a concept answer to a concept in the form builder Sep 18, 2024

NethmiRodrigo reviewed Oct 7, 2024

View reviewed changes

Willie-theBeastMutua added 2 commits October 7, 2024 14:06

Add concept answer added in edit

99c0a50

All Changes done

d761d9b

Willie-theBeastMutua force-pushed the addConceptAnswer branch from d96574f to d761d9b Compare October 7, 2024 11:20

Edit Modal changed

bcfd74b

Check for duplicates & delete added

0b098c5

Add Concept Answers

75d1cfa

Upstream Merge

bfec7d3

check for duplicate added answer

e417ff7

NethmiRodrigo reviewed Oct 29, 2024

View reviewed changes

Update src/components/interactive-builder/add-question.modal.tsx

e5de0d0

Co-authored-by: Nethmi Rodrigo <[email protected]>

Willie-theBeastMutua added 2 commits October 31, 2024 10:51

Merge remote-tracking branch 'upstream/main' into addConceptAnswer

462277e

removed callback

1b94b7b

NethmiRodrigo requested changes Nov 1, 2024

View reviewed changes

src/components/interactive-builder/add-question.modal.tsx Outdated Show resolved Hide resolved

Add Concept Answer

93e4773

NethmiRodrigo reviewed Nov 4, 2024

View reviewed changes

Willie-theBeastMutua added 2 commits November 15, 2024 15:14

Merge remote-tracking branch 'upstream/main' into addConceptAnswer

f1aa909

Add answer edit empty concept

96a39f9

NethmiRodrigo reviewed Nov 18, 2024

View reviewed changes

	const handleConceptSelect = (concept: Concept) => {
	const updatedDatePickerType = getDatePickerType(concept);
	if (updatedDatePickerType) setDatePickerType(updatedDatePickerType);
	setConceptToLookup('');
	setSelectedConcept(concept);
	setAnswers(
	concept?.answers?.map((answer) => ({
	concept: answer?.uuid,
	label: answer?.display,
	})),
	);

(feat) O3-3712: add a concept answer to a concept in the form builder #351

Are you sure you want to change the base?

(feat) O3-3712: add a concept answer to a concept in the form builder #351

Conversation

Willie-theBeastMutua commented Sep 11, 2024 • edited Loading

Requirements

Summary

Screenshots

Related Issue

Other

NethmiRodrigo commented Sep 18, 2024

Willie-theBeastMutua commented Sep 18, 2024

Willie-theBeastMutua commented Oct 2, 2024

NethmiRodrigo commented Oct 7, 2024

Willie-theBeastMutua commented Oct 7, 2024 • edited Loading

Willie-theBeastMutua commented Oct 7, 2024

NethmiRodrigo commented Oct 8, 2024

Willie-theBeastMutua commented Oct 8, 2024

NethmiRodrigo commented Oct 9, 2024

Willie-theBeastMutua commented Oct 11, 2024

Willie-theBeastMutua commented Oct 15, 2024

NethmiRodrigo commented Oct 15, 2024

Willie-theBeastMutua commented Oct 15, 2024

Willie-theBeastMutua commented Oct 15, 2024

NethmiRodrigo commented Oct 16, 2024 • edited Loading

Willie-theBeastMutua commented Oct 16, 2024

NethmiRodrigo commented Oct 16, 2024

Willie-theBeastMutua commented Oct 28, 2024

NethmiRodrigo commented Oct 29, 2024

NethmiRodrigo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Willie-theBeastMutua commented Oct 29, 2024

Willie-theBeastMutua commented Oct 31, 2024

Willie-theBeastMutua commented Nov 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Willie-theBeastMutua commented Nov 15, 2024

NethmiRodrigo Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Willie-theBeastMutua commented Sep 11, 2024 •

edited

Loading

Willie-theBeastMutua commented Oct 7, 2024 •

edited

Loading

NethmiRodrigo commented Oct 16, 2024 •

edited

Loading

NethmiRodrigo Nov 18, 2024 •

edited

Loading