Error message on duplicate phone numbers doesn't say it's a duplicate

[mrjones-plip]

Describe the bug If you try to create a contact with a phone number that is already in use by another contact, the error message doesn't tell you it's a duplicate.

To Reproduce Steps to reproduce the behavior:

1. Create a contact in the CHT app (not admin app) with the phone number +254712345678
2. Try to create a second contact with the same phone number +254712345678
3. See the input validation error that says

   Please enter a valid local number, or use the standard international format, which includes a plus sign (+) and country code. For example: +254712345678

Expected behavior The input validation error should tell you that the phone number is in use by another contact.

Logs NA

Screenshots

https://user-images.githubusercontent.com/8253488/153676369-7ec2a6c9-3bac-4ba2-b383-004ce99bab2c.mp4

Environment

• Instance: local dev, off of CHT Docker Helper off of medic-os
• Browser: latest firefox
• Client platform: Ubuntu 20.04
• App: webapp?
• Version: 3.14.0

Additional context

• I'm running the default app that ships with CHT
• @abbyad mentioned this was part of Enketo and not something in the default app, hence the bug against cht-core repo

[abbyad]

Thanks for raising @mrjones-plip! I just noticed that improving the validation message has been mentioned in this related issue, and can be done by updating the constraint message in the XForm to include something about the number needing to be "unique" as well.

[mrjones-plip]

Cools! let's use this ticket then as an call to arms to update the default app.

[abbyad]

Good idea! Putting a reminder here for whoever picks this up to update the source XLSForm files on Drive, and not the XForms directly.

(I am also looking to see how we provide notice for using those source XLSForm files so that we don't accidentally update the XML and not the XLS)

[mrjones-plip]

Good thinking! For the future person, find the IDs of the forms to edit on google drive in the forms-on-google-drive.json file (see below).

Also - what about the wording? It gets awkward in a hurry as seen here with me mocking int up.:

Finally - whoever takes this on should know that we need at least three translations of this: English, French and Nepali

cat forms-on-google-drive.json 
{
  "app/death_report.xlsx": "1azFHCMTMehxuSg_LWOgsJZxrJo0yhseLp9FK4gTT38M",
  "app/delivery.xlsx": "1KdJEp-iErhLtgILdrafIvK0vvkOsEGRKVlsWBB3ysmQ",
  "app/pregnancy.xlsx": "1EWwkyhhle05fHj5hoKlNgABNKOrgp30BFZrbpYT1AHU",
  "app/pregnancy_home_visit.xlsx": "1q5NoVHRF0CBnHiUqFa8eMIFpC8lW1Il9BsdQdL7LjzE",
  "app/pregnancy_facility_visit_reminder.xlsx": "1VC4Io2RQM7G0sNdjponGzYITJpqmQE26D2iJ9FWlniI",
  "app/pregnancy_danger_sign_follow_up.xlsx": "1n0LBbc5jtIm1f-IDgl_KmyV-C0JdVUCz_KPRuZgRYN0",
  "app/pregnancy_danger_sign.xlsx": "1NjaWx_VyJUZWYbOtM8aQQlTITGuh2zNXkf3Ba0lSfKc",
  "app/pnc_danger_sign_follow_up_mother.xlsx": "18Ba3WJFfKerA1ZkS3WsBjYPZRQgWApDvjyak_yq9XGw",
  "app/pnc_danger_sign_follow_up_baby.xlsx": "1pZGDlVI3W5aZdPyXgt5h9VqbvHrzWVwRJVpIdYLLsPE",
  "app/undo_death_report.xlsx": "16JZI2fJhUnLde-eG48pr0ULBs9C1vKhjhpQB9URWrjI",
  "contact/PLACE_TYPE-create.xlsx": "1VH0PdPNjUxSyKm7sWakEpEuyWfusKb9_L-oVjV7Jh7A",
  "contact/PLACE_TYPE-edit.xlsx": "1uYqSVOkeZUifk4R3x2MpcbReuKlKq7OPkdQvtE-DuPc",
  "contact/person-create.xlsx": "1VhOPphx4IXyZiGbcevVZwvsvap8WPKHmUjOp8NuWJW8",
  "contact/person-edit.xlsx": "1l1C0TOLfXAdLI0edcN5TbsRo2xVAIspb1SRgXC5VXDw"
}%                                                                            

[hassan254-prog]

@garethbowen Is this issue still open for assignment? I'm interested in contributing to it.

[garethbowen]

@hassan254-prog Yes it is, and we'd appreciate any help here!

I'm not sure exactly how to fix it. If I recall correctly Enketo (which is the library we use for rendering forms) only has space for one error message. The enketo-core library is also open source so you might be able to fix it there, and then when we upgrade we could use it too.

@jkuester Do you have any idea on how @hassan254-prog should go about working on this?

[jkuester]

Honestly, I do not think that an Enketo-based solution is going to be viable here. Our custom phone-widget essentially hacks into the Enekto constraint validation flow to prompt the user that the provided value is either invalid or duplicate. However, as Gareth mentioned, Enekto/ODK only supports a single constraint message to be configured for a field in a form (and honestly, I am not sure how you would even represent multiple constraint messages within the actual xlsxform).

My current thought here is that we should try to fundamentally refactor our phone-widget logic so that, instead of hacking into the Enekto constraint validation workflow, we encapsulate all of our validation logic while accepting the input data such that the phone number value is validated before it is ever set on the Enekto model (and if it fails validation, it does not get set on the model). Then, the phone-widget would also add extra HTML to the question for displaying hard-coded error messages specifying clearly if the phone number is invalid or if it is a duplicate.

Possible challenges of this approach include:

• No easy way to disable/enable validation in a form. Currently, a tel field in a form is not validated unless the constraint expression evaluates to true.
  • My guess is that this is not a huge deal since, if you want to accept a phone number with no validation, you can just use a integer field in your form.
• With this approach, you would not be able to specify custom error messages in the forms (these would be hard-coded at the server level).
• Depending on how the input logic gets triggered, running the validation at the time of input might be prohibitively expensive (e.g. iteratively evaluating the value as it is typed). If we could get it to just run the validation when the focus is removed from the question (and still save the data to the Enekto model at that time) that would be ideal....

@garethbowen / @dianabarsan how viable does this approach sound? If it seems reasonable, perhaps we could get a tech-design put together that could give @hassan254-prog a better picture of how to proceed!

[garethbowen]

So if I'm understanding correctly, with tel type it would always use libphonenumber validation, and you could opt in to duplicate validation or not? That seems sensible to me.

There is a related issue about landline vs cellphone (issue) validation which complicates things. Could we use appearance to flag what "type" of phone number it should be and then include that in the hardcoded validation?

[jkuester]

@garethbowen well technically I think we could decide to toggle whatever behavior we wanted via appearance flags. It would just be a matter of deciding which configurations would be useful!

[jkuester]

For maximum clarity, just copying the design goals from https://github.com/medic/cht-core/issues/6390

• The constraint message will default to the message provided in constraint_message
  • If no constraint_message is provided, and the phone number has invalid formatting or is a dup, will use a message from the server translations that is specific to the issue
    • This is a slight behavior change for the old type: tel fields. Previously, a constraint message would only be shown if a constraint value was set for the field. If there was no constraint, and the validation/dupe-check failed, the field would turn red and be invalidated, but no message would be shown (even if there was a value for constraint_message). Now, we will show a constraint message when appropriate for the invalid tel regardless of the value in the constraint column.