Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(feat) O3-3113: Add disallowDecimals validator #220

Merged
merged 9 commits into from
May 15, 2024

Conversation

jabahum
Copy link
Contributor

@jabahum jabahum commented Apr 25, 2024

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 a validator in the form engine that validates whether decimal values should be allowed in numeric obs fields. This resembles the Angular form engine's similarly named disallowDecimals validator.

The Carbon NumberInput doesn't provide any special handling for decimal values, so this validator can step in if you want to mandate that a field should not support decimals, for example. A scenario where you'd want to use this is when representing a numeric field that's backed by a concept that has allowDecimal set to false, such as Platelets below:

platelets

{
  "name": "Test Form 1",
  "description": "Test Form 1 Description",
  "version": "1",
  "published": true,
  "retired": false,
  "encounter": "Consultation",
  "pages": [
    {
      "label": "Page 1",
      "sections": [
        {
          "label": "Section 1",
          "isExpanded": "true",
          "questions": [
            {
              "label": "CD4 count :",
              "type": "obs",
              "id": "cd4count",
              "questionOptions": {
                "concept": "161011AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
                "rendering": "number",
                "disallowDecimals": "true"
              },
              "validators": []
            }
          ]
        }
      ]
    }
  ],
  "processor": "EncounterFormProcessor",
  "referencedForms": [],
  "encounterType": "67a71486-1a54-468f-ac3e-7091a9a79584"
}

Screenshots

Related Issue

Other

@jabahum jabahum marked this pull request as ready for review April 25, 2024 09:18
@denniskigen denniskigen changed the title (feat) O3-3113 : Validation of numeric fields and decimal field values in the Laboratory Test Results Form (feat) O3-3113: Add disallowDecimals validator Apr 25, 2024
@denniskigen
Copy link
Member

denniskigen commented Apr 25, 2024

@jabahum I've updated the PR description with extra context. It'd be cool if you could add a sample schema that shows how to leverage this validator, as well as a short video of it in action.

@jabahum
Copy link
Contributor Author

jabahum commented Apr 25, 2024

@jabahum I've updated the PR description with extra context. It'd be cool if you could add a sample schema that shows how to leverage this validator, as well as a short video of it in action.

This has been added @denniskigen

src/validators/form-validator.ts Outdated Show resolved Hide resolved
src/validators/form-validator.ts Outdated Show resolved Hide resolved
src/validators/form-validator.ts Outdated Show resolved Hide resolved
@jabahum jabahum force-pushed the O3-3113 branch 2 times, most recently from 6df2d0f to 28c5ceb Compare April 26, 2024 12:16
src/validators/form-validator.ts Outdated Show resolved Hide resolved
src/validators/form-validator.ts Outdated Show resolved Hide resolved
src/validators/form-validator.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@CynthiaKamau CynthiaKamau left a comment

Choose a reason for hiding this comment

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

Thanks @jabahum , please address the conflicts so that we can close this

package.json Outdated Show resolved Hide resolved
src/validators/form-validator.ts Outdated Show resolved Hide resolved
@denniskigen
Copy link
Member

@jabahum I've tried filling in the Laboratory Test Results form and I don't see the validation kick in when entering decimal values for the Platelets and MCV fields, which don't allow decimal values.

CleanShot 2024-05-08 at 11  57 50@2x

src/validators/form-validator.ts Outdated Show resolved Hide resolved
src/validators/form-validator.ts Outdated Show resolved Hide resolved
src/validators/form-validator.ts Outdated Show resolved Hide resolved
@denniskigen
Copy link
Member

@jabahum is the Lab Test Results form now getting validated correctly?

@hadijahkyampeire
Copy link
Contributor

@denniskigen your issue has been fixed, it was because the question was lacking min and max, so I have set a default since all the validation bases on the question having min and max
Screenshot 2024-05-14 at 21 19 48

@denniskigen
Copy link
Member

denniskigen commented May 14, 2024

@hadijahkyampeire the validation should be able to work regardless of whether min and max values are set. The basic premise should be that you're rendering a numeric field that doesn't allow decimals, so you add this validator to ensure that decimal values get flagged when entered into the input, min and max values notwithstanding.

@hadijahkyampeire
Copy link
Contributor

okay @denniskigen I can make it that way, I had got some information that we want to validate if the number is within the range of the min and max, and honestly it wasn't sounding correct to me as well. Thanks I will make that change.

Copy link
Member

@samuelmale samuelmale left a comment

Choose a reason for hiding this comment

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

Just a few comments

translations/en.json Outdated Show resolved Hide resolved
src/validators/form-validator.ts Outdated Show resolved Hide resolved
translations/en.json Outdated Show resolved Hide resolved
@hadijahkyampeire hadijahkyampeire dismissed pirupius’s stale review May 15, 2024 06:17

All issues addressed

@hadijahkyampeire hadijahkyampeire merged commit 42d73f3 into openmrs:main May 15, 2024
4 checks passed
vasharma05 pushed a commit that referenced this pull request May 27, 2024
* add disallowDecimals attribute

* fix comments

* no need to translate error

* do not translate error type

* fix tests and merge conflicts

* set default min and max

* fix undefined submission

* separate decimal validation from min/max validation

* update translations

---------

Co-authored-by: hadijahkyampeire <hadijah315@gmail.com>
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.

7 participants