Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

fix(patient): Validation input for email and phone number #1983

Closed
wants to merge 34 commits into from

Conversation

martimfj
Copy link
Contributor

@martimfj martimfj commented Apr 13, 2020

Fixes parts of #1976

Changes proposed in this pull request:

  • Input validation of email and phone number (On Create and Edit)
  • Remove error feedback as user edits field

@vercel
Copy link

vercel bot commented Apr 13, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hospitalrun/hospitalrun-frontend/yapgry5vv
✅ Preview: https://hospitalrun-frontend-git-fork-martimfj-validatetelemail.hospitalrun.now.sh

expect(wrapper.find(GeneralInformation).prop('feedbackFields').phoneNumber).toMatch(
'patient.errors.patientPhoneNumberFeedback',
)
expect(wrapper.update.isInvalid === true)
Copy link
Member

Choose a reason for hiding this comment

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

this can be written as expect(wrapper.prop('isInvalid)).toBeTruthy() to more closely align to how other tests have been written

expect(wrapper.find(GeneralInformation).prop('feedbackFields').email).toMatch(
'patient.errors.patientEmailFeedback',
)
expect(wrapper.update.isInvalid === true)
Copy link
Member

Choose a reason for hiding this comment

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

same as other comment

expect(wrapper.find(GeneralInformation).prop('feedbackFields').phoneNumber).toMatch(
'patient.errors.patientPhoneNumberFeedback',
)
expect(wrapper.update.isInvalid === true)
Copy link
Member

Choose a reason for hiding this comment

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

same as other comment

expect(wrapper.find(GeneralInformation).prop('feedbackFields').email).toMatch(
'patient.errors.patientEmailFeedback',
)
expect(wrapper.update.isInvalid === true)
Copy link
Member

Choose a reason for hiding this comment

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

same as other comment

Comment on lines +111 to +136
if (patient.phoneNumber) {
if (!patient.phoneNumber.match(/^\d+/)) {
inputIsValid = false
setErrorMessage(t('patient.errors.updatePatientError'))
setInvalidFields({
...invalidFields,
phoneNumber: true,
})
setFeedbackFields({
...feedbackFields,
phoneNumber: t('patient.errors.patientPhoneNumberFeedback'),
})
}
}
if (patient.email) {
if (!patient.email.match(/^\w+([.-]?\w+)*@\w+([.-]?\w+)*(\.\w{2,3})+$/)) {
inputIsValid = false
setErrorMessage(t('patient.errors.updatePatientError'))
setInvalidFields({
...invalidFields,
email: true,
})
setFeedbackFields({
...feedbackFields,
email: t('patient.errors.patientEmailFeedback'),
})
Copy link
Member

Choose a reason for hiding this comment

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

I think we may want to use https://www.npmjs.com/package/validator (or equivalent library if there is a different one).

I fear that there are edge cases that we have not considered. For example, phone numbers can have (, ), -, / in some cases.

This library seems comprehensive in the validations it performs.

Copy link
Member

Choose a reason for hiding this comment

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

@fox1t thoughts here?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we absolutely need to relay on some external library to do this properly. However I don't have any preference.

@jackcmeyer jackcmeyer requested a review from fox1t April 14, 2020 13:23
@jackcmeyer jackcmeyer added patients issue/pull request that interacts with patients module 🚀enhancement an issue/pull request that adds a feature to the application labels Apr 14, 2020
@jackcmeyer jackcmeyer added this to the v2.0 milestone Apr 14, 2020
@vercel vercel bot requested a deployment to Preview April 16, 2020 15:08 Abandoned
@vercel
Copy link

vercel bot commented Apr 16, 2020

Deployment failed with the following error:

request to https://api.zeit.co/v12/now/deployments?teamId=team_KiGwX2JSRgAI3krc36DK1uGg&skipAutoDetectionConfirmation=1 failed, reason: socket hang up

@vercel vercel bot temporarily deployed to Preview April 16, 2020 18:34 Inactive
@fox1t
Copy link
Member

fox1t commented Apr 23, 2020

Any news about this?

@vercel vercel bot temporarily deployed to Preview April 24, 2020 16:38 Inactive
@gitpod-io
Copy link

gitpod-io bot commented Apr 24, 2020

@vercel vercel bot temporarily deployed to Preview April 24, 2020 16:55 Inactive
@vercel vercel bot temporarily deployed to Preview April 25, 2020 08:54 Inactive
@jackcmeyer
Copy link
Member

closing in favor of #2030

@jackcmeyer jackcmeyer closed this May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🚀enhancement an issue/pull request that adds a feature to the application patients issue/pull request that interacts with patients module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants