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

Error when note fields are required in form #494

Closed
Tracked by #462
jkuester opened this issue Jul 19, 2022 · 3 comments · Fixed by #496
Closed
Tracked by #462

Error when note fields are required in form #494

jkuester opened this issue Jul 19, 2022 · 3 comments · Fixed by #496
Assignees

Comments

@jkuester
Copy link
Contributor

jkuester commented Jul 19, 2022

Setting a note field to be required has always been bad data, but in previous versions of Enketo, this was just silently ignored. Now, this config will prevent the user from continuing to the next page and completing the form. cht-core has been updated, as a part of these changes: medic/cht-core#7256) to automatically remove the required designation from note fields, but best practice would still have us identify these improperly configured notes when compiling the form with cht-conf and produce an error so that the user knows to update the config. See this script as an example of regex for finding required notes and this xsl code which is how cht-core is removing the required designation.

@jkuester jkuester self-assigned this Jul 19, 2022
@jkuester jkuester changed the title E. Error when fields are required () Error when note fields are required in form Jul 19, 2022
@jkuester jkuester moved this to Ready for AT in Product Team Activities Aug 12, 2022
@tatilepizs
Copy link

Hi @jkuester, could you please help me with some steps/comments/ideas about what will be the correct way to test it?

@jkuester
Copy link
Contributor Author

jkuester commented Aug 16, 2022

@tatilepizs Oh sure! Sorry for forgetting to put something in here before. Basically this new functionality should cause a form to fail validation if there is a note field that is required.

  1. Create a form (Form A) with a note field (can be any message in the note) where the required value is true()
  2. Create form (Form B) with a note field (can be any message in the note) where the required value is false()
  3. Put these forms in ./forms/app
  4. Run cht convert-app-forms validate-app-forms To run with the branch code, you have a couple options:
    1. Checkout the branch and follow these steps
    2. Just globally install the branch directly using npm i -g medic/cht-conf#494_required_notes and then you can use the cht commands as you would normally (note this method will overwrite any version of cht-conf you have currently installed)
  5. See that the validation fails for the note in Form A, but not the note in Form B.

@tatilepizs tatilepizs moved this from Ready for AT to AT in Progress in Product Team Activities Aug 17, 2022
@tatilepizs
Copy link

Thanks @jkuesterfor all the details, it was really helpful.

These are the forms that I used -> enketo_test_formA.xlsx - enketo_test_formB.xlsx

Testing steps:

  • Used the command npm i -g medic/cht-conf#494_required_notes to install the correct branch
  • Copied the created forms in ./forms/app
  • Run the command cht convert-app-forms validate-app-forms
  • Validation shown the following message:
    image

@tatilepizs tatilepizs moved this from AT in Progress to Ready to Merge in Product Team Activities Aug 18, 2022
Repository owner moved this from Ready to Merge to Done in Product Team Activities Aug 18, 2022
@mrjones-plip mrjones-plip moved this from Done to This Week's commitments in Product Team Activities Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants