Moved schema-validator logic to form engine #107

arodidev · 2023-08-17T05:13:15Z

Moves the form-schema validator logic to live inside the form-engine

samuelmale

Move the validator to https://github.com/openmrs/openmrs-form-engine-lib/tree/main/src/validators

src/form-validator.ts

src/components/inputs/date/ohri-date.component.tsx

samuelmale · 2023-08-22T18:42:16Z

src/schema-validator.ts

+      if (data.results.length) {
+        const [resObject] = data.results;
+
+        resObject.datatype.name === 'Boolean' &&


Should be driven by a config

@samuelmale kindly expound on this

My bad @arodidev my comment regarding the config, see.

As a good practice, I would create an enum holding all the know datatypes to prevent hard-coding these.

const ConceptDataTypes { Boolean: 'Boolean', Coded: 'Coded', Numeric: 'Numeric', Text: 'Text' // more... }; // validator.js if (concept.datatype.name === ConceptDataTypes.Boolean) {}

samuelmale · 2023-08-22T18:49:54Z

src/schema-validator.ts

+
+  if (searchRef) {
+    try {
+      const { data } = await openmrsFetch(`/ws/rest/v1/concept?references=${searchRef}&v=${conceptRepresentation}`);


I'm concerned about the fact that we have fused the validation logic with API-specific logic. I advise reusing the existing useConcepts() hook. This means the validator becomes a React hook:

// the validator runs if doValidate is set to true // isValidating shows the validation status const { errors, warnings, isValidating } = useSchemaValidationResults(doValidate, schema);

Alternatively, you can define a FormSchemaValidator interface but ensure you fetch the concepts in one API hit with proper separation of concerns.

@samuelmale I had a previous implementation of how I understood your statement here, kindly advise if this logic suffices.

samuelmale · 2023-08-23T09:46:21Z

src/validators/schema-validator.ts

+            if (
+              answer.concept !== 'cf82933b-3f3f-45e7-a5ab-5d31aaee3da3' &&
+              answer.concept !== '488b58ff-64f5-4f8a-8979-fa79940b1594'


I would do something like:

if ( ![ConceptTrue, ConceptFalse].includes(answer.concept) ) { // handle error }

The ConceptTrue and ConceptFalse are driven by a System configuration: concept.true and concept.false. We need to be picking these values from the backend. On the contrary, these are currently defined as constants within the library which isn't the conventional practice. I've always wanted to address this technical debt but never got the latitude.

Ideally, we should introduce a new hook that will read this system setting. It would be great if you offered to address this otherwise you can use the already defined constants and we address this later on.

@samuelmale this is well noted. I would also like to take on the issue of the hook reading the values from the backend if possible.

samuelmale · 2023-09-08T15:43:01Z

src/hooks/useSchemaValidator.ts

+import { ConceptTrue, ConceptFalse } from '../constants';
+import { OHRIFormField, OHRIFormSchema } from '../api/types';
+
+export function useSchemaValidator(schema: OHRIFormSchema) {


Two things here:

We need a trigger state (to trigger the schema validation)

On top of the errors and warnings, it's important to return a progress state i.e.. isValidating

samuelmale · 2023-09-08T15:43:25Z

src/hooks/useSchemaValidator.ts

+import { OHRIFormField, OHRIFormSchema } from '../api/types';
+
+export function useSchemaValidator(schema: OHRIFormSchema) {
+  //initializing state


Get rid of the comment

samuelmale · 2023-09-08T15:53:55Z

src/hooks/useSchemaValidator.ts

+  const [answerConceptSet, setAnswerConceptSet] = useState<Set<string>>(new Set());
+  const { dataTypeToRenderingMap, conceptDataTypes } = useConfig();
+
+  const unresolvedConceptsFunc = (fullArray, filteredSetArray) => {


The signature of this function can be improved, starting with the function name. Assuming it finds unresolved concepts, findUnresolvedConcepts is more descriptive.

fullArray: It's not straightforward to tell the kind of objects this variable holds same applies to the other param

samuelmale · 2023-09-08T15:54:38Z

src/hooks/useSchemaValidator.ts

+  //initializing state
+  const [errors, setErrors] = useState([]);
+  const [warnings, setWarnings] = useState([]);
+  const [fullArray, setFullArray] = useState<Array<OHRIFormField>>([]);


Can you rename this variable to clearly reflect that it's holding fields?

samuelmale · 2023-09-08T15:56:05Z

src/hooks/useDatatype.ts

+const conceptRepresentation =
+  'custom:(uuid,display,datatype,conceptMappings:(conceptReferenceTerm:(conceptSource:(name),code)))';
+
+export function useDatatype(references: Set<string>) {


Assuming this returns concepts, what stops you from reusing the useConcepts() hook?

I think we had this conversation a while back to create a new custom hook that also returns a filtered set with unresolved concepts, but if need be I can revert back to using the useConcept hook and modifying it.

samuelmale · 2023-09-08T15:58:22Z

src/api/types.ts

+export interface ConfigObject {
+  Numeric: Array<string>;
+  Coded: Array<string>;
+  Text: Array<string>;
+  Date: Array<string>;
+  Datetime: Array<string>;
+  Boolean: Array<string>;
+  Rule: Array<string>;
+}


This is possessing a generic name; one may assume it's the general config interface for the form-engine configuration yet I think it's a form schema validator configuration?

samuelmale · 2023-09-08T16:02:23Z

src/hooks/useSchemaValidator.ts

+  const [answersArray, setAnswersArray] = useState([]);
+  const [conceptSet, setConceptSet] = useState<Set<string>>(new Set());
+  const [answerConceptSet, setAnswerConceptSet] = useState<Set<string>>(new Set());
+  const { dataTypeToRenderingMap, conceptDataTypes } = useConfig();


Have you actually tested this out? I don't see where you defined the config schema plus you would have to load this config by module name:

const { dataTypeToRenderingMap, conceptDataTypes } = useConfig({ externalModuleName: "@openmrs/openmrs-form-engine-lib", });

added form-validator logic

6cb30fa

arodidev changed the title ~~Added form-validator logic to form engine~~ Moved form-validator logic to form engine Aug 17, 2023

arodidev added 2 commits August 21, 2023 11:39

Schema-validator

84121be

render control instead of "view" mode

604efb8

samuelmale requested changes Aug 22, 2023

View reviewed changes

arodidev and others added 4 commits August 23, 2023 08:55

Revert changes on the controls

e1cf8ba

Relocated schema validator to validators dir

e32fbdf

Merge branch 'main' into dev

5dc978f

renaming error and warning arrays

e9558b5

samuelmale reviewed Aug 23, 2023

View reviewed changes

arodidev changed the title ~~Moved form-validator logic to form engine~~ Moved schema-validator logic to form engine Aug 27, 2023

arodidev added 4 commits September 6, 2023 23:40

Update and overhaul

672c387

Cleanup

1c17c0d

export validator hook

b8dc67e

added config schema

e5b6a0b

samuelmale reviewed Sep 8, 2023

View reviewed changes

arodidev and others added 4 commits September 11, 2023 17:11

Test production

3dc7968

Merge branch 'openmrs:main' into dev

f7ca7b0

Linting

bbf444f

revamp schema validator

84ecf2e

samuelmale closed this Apr 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moved schema-validator logic to form engine #107

Moved schema-validator logic to form engine #107

arodidev commented Aug 17, 2023 •

edited

Loading

samuelmale left a comment

samuelmale Aug 22, 2023

arodidev Aug 23, 2023

samuelmale Aug 23, 2023

samuelmale Aug 22, 2023

arodidev Aug 27, 2023 •

edited

Loading

samuelmale Aug 23, 2023

arodidev Aug 27, 2023 •

edited

Loading

samuelmale Sep 8, 2023

arodidev Sep 11, 2023

samuelmale Sep 8, 2023

arodidev Sep 11, 2023

samuelmale Sep 8, 2023

arodidev Sep 11, 2023

samuelmale Sep 8, 2023

arodidev Sep 11, 2023

samuelmale Sep 8, 2023

arodidev Sep 11, 2023

samuelmale Sep 8, 2023

arodidev Sep 11, 2023

samuelmale Sep 8, 2023

Moved schema-validator logic to form engine #107

Moved schema-validator logic to form engine #107

Conversation

arodidev commented Aug 17, 2023 • edited Loading

samuelmale 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

arodidev Aug 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arodidev Aug 27, 2023 • 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arodidev commented Aug 17, 2023 •

edited

Loading

arodidev Aug 27, 2023 •

edited

Loading

arodidev Aug 27, 2023 •

edited

Loading