I'm thinking we should centralize all patron's contact information validators into one module. Currently we only have validations for e-mail addresses, using the Email::Valid module. In our local library we also want to validate phone numbers with a regex that only recognizes phone numbers from my country. So I suggest we create a phone number validation as well, with an option to easily create custom regex for your local use. If a library does not wish to use validations, we should give them an option to turn them off. If more local validators are implemented, we should also give the library an option to select which validator they wish to use. With this method we could implement phone number validators from around the world and add them into our module. Then we would make the validations simply by calling our validator. The validator would know by system preference which one of the implemented phone number validators it should use. Validations should be both client and server side.
Created attachment 41461 [details] [review] Bug 14620 - Contact information validations I'm thinking we should centralize all patron's contact information validators into one module. Currently we only have validations for e-mail addresses, using the Email::Valid module. In our local library we also want to validate phone numbers with a regex that only recognizes phone numbers from my country. So I suggest we create a phone number validation as well, with an option to easily create custom regex for your local use. If a library does not wish to use validations, we should give them an option to turn them off. If more local validators are implemented, we should also give the library an option to select which validator they wish to use. With this method we could implement phone number validators from around the world and add them into our module. Then we would make the validations simply by calling our validator. The validator would know by system preference which one of the implemented phone number validators it should use. Validations should be both client and server side.
(In reply to Lari Taskula from comment #1) > Created attachment 41461 [details] [review] [review] > Bug 14620 - Contact information validations > Centralizes email and phone number validations into Koha::Validation. Adds two new system preferences, 'ValidateEmailAddress' and 'ValidatePhoneNumber', which determine whether to use the validations or not. Email address validation uses Email::Valid module introduced in Bug 5685. Email can be verified via Koha::Validation::validate_email("email@address.com"). Phone number validation uses regex to validate phone numbers. The system preference, ValidatePhoneNumber, let's you define which regex you want to use. In the patch there is two different phone number validators implemented, (international and Finnish). Anyone can easily create their own regex for his local use. Validations are both client- and server-side. The following test plan applies to memberentry.pl, opac-memberentry.pl and opac-messaging.pl. Test plan: 1. Apply the patch (and run updatedatabase.pl). 2. Set system preferences ValidateEmailAddress to "Enable" and ValidatePhoneNumber to "International Phone Numbers". 3. Navigate to edit user contact informations in Staff client and OPAC. 4. Make sure JavaScript is enabled. 5. Insert invalid email (e.g. "invalid") and invalid phone number ("+123invalid") and send the form. 6. Confirm that form will not be submitted and errors will be given. 7. Disable JavaScript. 8. Try sending the form again. 9. Confirm that form will be submitted but errors will be given on top of the form. 10. Observe that none of the changes are accepted. 11. Set system preferences ValidateEmailAddress to "Disable" and ValidatePhoneNumber to "none". 12. Repeat step 3-9 but observe that errors will not be given and changes will be submitted and saved.
Phone number validation still accepts trailing (and I think also leading) whitespaces. Didn't figure out any fix to this yet, so if anyone has ideas of how to fix this, they're welcome!
Created attachment 41469 [details] [review] Bug 14620 - Whitespace fix for validations This fix trims emails and phone numbers just before form is sent.
Created attachment 41470 [details] [review] Bug 14620 - Whitespace fix for validations This fix trims emails and phone numbers just before form is sent.
Created attachment 41471 [details] [review] Bug 14620 - Whitespace fix for validations This fix trims emails and phone numbers just before form is sent.
Created attachment 41472 [details] [review] Bug 14620 - Whitespace fix for validations This fix trims emails and phone numbers just before form is sent. Fixes the invalid syntax used in JQuery's .on() functions' argument.
Created attachment 41502 [details] [review] Bug 14620 - Contact information validations I'm thinking we should centralize all patron's contact information validators into one module. Currently we only have validations for e-mail addresses, using the Email::Valid module. In our local library we also want to validate phone numbers with a regex that only recognizes phone numbers from my country. So I suggest we create a phone number validation as well, with an option to easily create custom regex for your local use. If a library does not wish to use validations, we should give them an option to turn them off. If more local validators are implemented, we should also give the library an option to select which validator they wish to use. With this method we could implement phone number validators from around the world and add them into our module. Then we would make the validations simply by calling our validator. The validator would know by system preference which one of the implemented phone number validators it should use. Validations should be both client and server side.
Created attachment 41527 [details] [review] Bug 14620 - Whitespace fix for validations This fix trims emails and phone numbers just before form is sent. Fixes the invalid syntax used in JQuery's .on() functions' argument. Removes the trimming for click events as change event should be enough. Test plan: 1. Apply the patches (and run updatedatabase.pl). 2. Set system preferences ValidateEmailAddress to "Enable" and ValidatePhoneNumber to "International Phone Numbers". 3. Make sure JavaScript is enabled. 4. Navigate to edit user contact informations in Staff client and OPAC. 5. Clear email and phone fields. 6. Insert whitespaces to email and phone fields and submit the form. 7. Edit patron. 8. Observe that there are no longer whitespaces in email and phone fields. 9. Insert invalid email (e.g. "invalid") and invalid phone number ("+123invalid") and send the form. 10. Confirm that form will not be submitted and errors will be given. 11. Disable JavaScript. 12. Insert invalid email and invalid phone number 13. Try sending the form again. 14. Confirm that form will be submitted but errors will be given on top of the form. 15. Observe that none of the changes are accepted. 16. Set system preferences ValidateEmailAddress to "Disable" and ValidatePhoneNumber to "none". 17. Repeat step 3-9 but observe that errors will not be given and changes will be submitted and saved.
Created attachment 42080 [details] [review] Bug 14620 - Contact information validations I'm thinking we should centralize all patron's contact information validators into one module. Currently we only have validations for e-mail addresses, using the Email::Valid module. In our local library we also want to validate phone numbers with a regex that only recognizes phone numbers from my country. So I suggest we create a phone number validation as well, with an option to easily create custom regex for your local use. If a library does not wish to use validations, we should give them an option to turn them off. If more local validators are implemented, we should also give the library an option to select which validator they wish to use. With this method we could implement phone number validators from around the world and add them into our module. Then we would make the validations simply by calling our validator. The validator would know by system preference which one of the implemented phone number validators it should use. Validations should be both client and server side. Test plan: 1. Apply the patches (and run updatedatabase.pl). 2. Set system preferences ValidateEmailAddress to "Enable" and ValidatePhoneNumber to "International Phone Numbers". 3. Make sure JavaScript is enabled. 4. Navigate to edit user contact informations in Staff client and OPAC. 5. Clear email and phone fields. 6. Insert whitespaces to email and phone fields and submit the form. 7. Edit patron. 8. Observe that there are no longer whitespaces in email and phone fields. 9. Insert invalid email (e.g. "invalid") and invalid phone number ("+123invalid") and send the form. 10. Confirm that form will not be submitted and errors will be given. 11. Disable JavaScript. 12. Insert invalid email and invalid phone number 13. Try sending the form again. 14. Confirm that form will be submitted but errors will be given on top of the form. 15. Observe that none of the changes are accepted. 16. Set system preferences ValidateEmailAddress to "Disable" and ValidatePhoneNumber to "none". 17. Repeat step 3-9 but observe that errors will not be given and changes will be submitted and saved.
Created attachment 42107 [details] [review] Bug 14620 - Contact information validations
Patch complexity is 'Large' because this change has many architectural connections
Created attachment 42456 [details] [review] Bug 14620 - Contact information validations
Created attachment 42462 [details] [review] [Signed-off] Bug 14620 - Contact information validations I'm thinking we should centralize all patron's contact information validators into one module. Currently we only have validations for e-mail addresses, using the Email::Valid module. In our local library we also want to validate phone numbers with a regex that only recognizes phone numbers from my country. So I suggest we create a phone number validation as well, with an option to easily create custom regex for your local use. If a library does not wish to use validations, we should give them an option to turn them off. If more local validators are implemented, we should also give the library an option to select which validator they wish to use. With this method we could implement phone number validators from around the world and add them into our module. Then we would make the validations simply by calling our validator. The validator would know by system preference which one of the implemented phone number validators it should use. Validations should be both client and server side. Test plan: 1. Apply the patches (and run updatedatabase.pl). 2. Set system preferences ValidateEmailAddress to "Enable" and ValidatePhoneNumber to "International Phone Numbers". 3. Make sure JavaScript is enabled. 4. Navigate to edit user contact informations in Staff client and OPAC. 5. Clear email and phone fields. 6. Insert whitespaces to email and phone fields and submit the form. 7. Edit patron. 8. Observe that there are no longer whitespaces in email and phone fields. 9. Insert invalid email (e.g. "invalid") and invalid phone number ("+123invalid") and send the form. 10. Confirm that form will not be submitted and errors will be given. 11. Disable JavaScript. 12. Insert invalid email and invalid phone number 13. Try sending the form again. 14. Confirm that form will be submitted but errors will be given on top of the form. 15. Observe that none of the changes are accepted. 16. Set system preferences ValidateEmailAddress to "Disable" and ValidatePhoneNumber to "none". 17. Repeat step 3-9 but observe that errors will not be given and changes will be submitted and saved. Followed test plan. Works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Blocked by bug 7174.
Email::Valid has some issues validating umlauts. While client-side (validate plugin) works fine, server side (Email::Valid) does not allow umlauts (e.g. äöå). Noting this down for future fix.
Created attachment 44703 [details] [review] Bug 14620 - Contact information validations
Created attachment 44705 [details] [review] Bug 14620 - Contact information validations I'm thinking we should centralize all patron's contact information validators into one module. Currently we only have validations for e-mail addresses, using the Email::Valid module. In our local library we also want to validate phone numbers with a regex that only recognizes phone numbers from my country. So I suggest we create a phone number validation as well, with an option to easily create custom regex for your local use. If a library does not wish to use validations, we should give them an option to turn them off. If more local validators are implemented, we should also give the library an option to select which validator they wish to use. With this method we could implement phone number validators from around the world and add them into our module. Then we would make the validations simply by calling our validator. The validator would know by system preference which one of the implemented phone number validators it should use. Validations should be both client and server side. Test plan: 1. Apply the patches (and run updatedatabase.pl). 2. Set system preferences ValidateEmailAddress to "Enable" and ValidatePhoneNumber to "International Phone Numbers". 3. Make sure JavaScript is enabled. 4. Navigate to edit user contact informations in Staff client and OPAC. 5. Clear email and phone fields. 6. Insert whitespaces to email and phone fields and submit the form. 7. Edit patron. 8. Observe that there are no longer whitespaces in email and phone fields. 9. Insert invalid email (e.g. "invalid") and invalid phone number ("+123invalid") and send the form. 10. Confirm that form will not be submitted and errors will be given. 11. Disable JavaScript. 12. Insert invalid email and invalid phone number 13. Try sending the form again. 14. Confirm that form will be submitted but errors will be given on top of the form. 15. Observe that none of the changes are accepted. 16. Set system preferences ValidateEmailAddress to "Disable" and ValidatePhoneNumber to "none". 17. Repeat step 3-9 but observe that errors will not be given and changes will be submitted and saved.
Created attachment 61843 [details] [review] Bug 14620: Contact information validations This patch adds a phone number validation by regex and centralizes different validation methods into Koha::Validation class. Introduces a new system preference, ValidatePhoneNumber, that takes a regular expression and uses it to validate phone numbers both client and server side. Unit tests to test: 1. prove t/db_dependent/Koha/Validation.t To test: 1. Apply the patches and run updatedatabase.pl to install the new system preference 2. Set system preference ValidatePhoneNumber to any regex you like (there is an example in the description of the preference) 3. Navigate to edit user contact informations in Staff client and OPAC. 4. Insert invalid email (e.g. "invalid") and invalid phone number ("+123invalid") and send the form. 5. Confirm that form will not be submitted and errors will be given. 6. Disable JavaScript and test that these errors will also be provided by the server.
Rebased on top of master, created unit tests for Koha::Validation, removed dependency to Bug 7174.
Hi Lari Validation of phone number works fine with the exception of - Fax number - Phone number in Alternative contact (both Staff client and OPAC)
Had a conflict, think I fixed it.
There are some complaints from the QA tool: FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt FAIL forbidden patterns forbidden pattern: opac-tmpl should certainly replaced with [% interface %] (line 233) OK git manipulation OK spelling OK tt_valid OK valid_template FAIL Koha/Validation.pm OK critic FAIL forbidden patterns forbidden pattern: Koha is now under the GPLv3 license (line 9) OK git manipulation OK pod OK pod coverage OK spelling OK valid Tests pass, database update is ok. Reattaching the rebased patch and a tiny follow-up with a suggestion for better system preference formatting.
Created attachment 67812 [details] [review] Bug 14620: Contact information validations This patch adds a phone number validation by regex and centralizes different validation methods into Koha::Validation class. Introduces a new system preference, ValidatePhoneNumber, that takes a regular expression and uses it to validate phone numbers both client and server side. Unit tests to test: 1. prove t/db_dependent/Koha/Validation.t To test: 1. Apply the patches and run updatedatabase.pl to install the new system preference 2. Set system preference ValidatePhoneNumber to any regex you like (there is an example in the description of the preference) 3. Navigate to edit user contact informations in Staff client and OPAC. 4. Insert invalid email (e.g. "invalid") and invalid phone number ("+123invalid") and send the form. 5. Confirm that form will not be submitted and errors will be given. 6. Disable JavaScript and test that these errors will also be provided by the server.
Created attachment 67813 [details] [review] Bug 14620: (follow-up) Link URL in system preference description
Apart from the qa script this worked nicely, please fix!
Created attachment 93930 [details] [review] Bug 14620: Contact information validations This patch adds a phone number validation by regex and centralizes different validation methods into Koha::Validation class. Introduces a new system preference, ValidatePhoneNumber, that takes a regular expression and uses it to validate phone numbers both client and server side. Unit tests to test: 1. prove t/db_dependent/Koha/Validation.t To test: 1. Apply the patches and run updatedatabase.pl to install the new system preference 2. Set system preference ValidatePhoneNumber to any regex you like (there is an example in the description of the preference) 3. Navigate to edit user contact informations in Staff client and OPAC. 4. Insert invalid email (e.g. "invalid") and invalid phone number ("+123invalid") and send the form. 5. Confirm that form will not be submitted and errors will be given. 6. Disable JavaScript and test that these errors will also be provided by the server. Sponsored-by: Koha-Suomi Oy
Created attachment 93931 [details] [review] Bug 14620: Patron contact validation script Goes through patron database and validates their email and phonenumber fields. Only reports issues, does not modify anything. Usage: ./misc/maintenance/validate_patron_contact_data.pl --help Example output: =============== =============== ======================================== Borrowernumber Field Value =============== =============== ======================================== 29 phone 1NV4L1D 30 smsalertnumber lol 30 phone 1NV4L1D 39 smsalertnumber lol 39 phone 1NV4L1D 40 smsalertnumber lol 40 phone 1NV4L1D 41 smsalertnumber lol 41 phone 1NV4L1D 42 smsalertnumber lol =============== =============== ======================================== Borrowernumber Field Value =============== =============== ========================================
Patch no longer applies 8-(