Bug 14620 - Contact information validations
Summary: Contact information validations
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lari Taskula
QA Contact: Testopia
URL:
Keywords:
Depends on: 5685
Blocks: 14590 17433
  Show dependency treegraph
 
Reported: 2015-07-30 11:24 UTC by Lari Taskula
Modified: 2023-08-26 17:29 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Large patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 14620 - Contact information validations (64.59 KB, patch)
2015-08-12 12:17 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620 - Whitespace fix for validations (7.94 KB, patch)
2015-08-13 11:44 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620 - Whitespace fix for validations (6.58 KB, patch)
2015-08-13 12:26 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620 - Whitespace fix for validations (6.51 KB, patch)
2015-08-13 12:29 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620 - Whitespace fix for validations (6.51 KB, patch)
2015-08-13 12:55 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 14620 - Contact information validations (64.56 KB, patch)
2015-08-14 14:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620 - Whitespace fix for validations (7.61 KB, patch)
2015-08-17 10:17 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620 - Contact information validations (68.35 KB, patch)
2015-08-28 14:00 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 14620 - Contact information validations (68.35 KB, patch)
2015-08-31 10:08 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620 - Contact information validations (68.83 KB, patch)
2015-09-08 13:32 UTC, Lari Taskula
Details | Diff | Splinter Review
[Signed-off] Bug 14620 - Contact information validations (69.02 KB, patch)
2015-09-09 12:40 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14620 - Contact information validations (68.35 KB, patch)
2015-11-10 14:11 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620 - Contact information validations (68.22 KB, patch)
2015-11-10 14:39 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620: Contact information validations (26.52 KB, patch)
2017-04-04 14:34 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620: Contact information validations (26.46 KB, patch)
2017-10-07 21:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14620: (follow-up) Link URL in system preference description (1.15 KB, patch)
2017-10-07 21:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14620: Contact information validations (26.16 KB, patch)
2019-10-09 15:53 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14620: Patron contact validation script (7.38 KB, patch)
2019-10-09 15:53 UTC, Lari Taskula
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Taskula 2015-07-30 11:24:18 UTC
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.
Comment 1 Lari Taskula 2015-08-12 12:17:40 UTC Comment hidden (obsolete)
Comment 2 Lari Taskula 2015-08-12 13:08:14 UTC
(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.
Comment 3 Joonas Kylmälä 2015-08-13 09:00:31 UTC
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!
Comment 4 Lari Taskula 2015-08-13 11:44:08 UTC Comment hidden (obsolete)
Comment 5 Lari Taskula 2015-08-13 12:26:54 UTC Comment hidden (obsolete)
Comment 6 Lari Taskula 2015-08-13 12:29:22 UTC Comment hidden (obsolete)
Comment 7 Joonas Kylmälä 2015-08-13 12:55:35 UTC Comment hidden (obsolete)
Comment 8 Lari Taskula 2015-08-14 14:31:37 UTC Comment hidden (obsolete)
Comment 9 Lari Taskula 2015-08-17 10:17:26 UTC Comment hidden (obsolete)
Comment 10 Joonas Kylmälä 2015-08-28 14:00:12 UTC Comment hidden (obsolete)
Comment 11 Lari Taskula 2015-08-31 10:08:53 UTC Comment hidden (obsolete)
Comment 12 Zeno Tajoli 2015-09-01 08:14:21 UTC
Patch complexity is 'Large' because this change has many architectural connections
Comment 13 Lari Taskula 2015-09-08 13:32:47 UTC Comment hidden (obsolete)
Comment 14 Marc Véron 2015-09-09 12:40:46 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2015-09-15 08:23:03 UTC
Blocked by bug 7174.
Comment 16 Lari Taskula 2015-09-25 10:40:17 UTC
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.
Comment 17 Lari Taskula 2015-11-10 14:11:28 UTC Comment hidden (obsolete)
Comment 18 Lari Taskula 2015-11-10 14:39:58 UTC Comment hidden (obsolete)
Comment 19 Lari Taskula 2017-04-04 14:34:26 UTC Comment hidden (obsolete)
Comment 20 Lari Taskula 2017-04-04 14:37:45 UTC
Rebased on top of master, created unit tests for Koha::Validation, removed dependency to Bug 7174.
Comment 21 Marc Véron 2017-04-05 21:18:58 UTC
Hi Lari

Validation of phone number works fine with the exception of 
- Fax number
- Phone number in Alternative contact 
(both Staff client and OPAC)
Comment 22 Katrin Fischer 2017-10-07 21:19:37 UTC
Had a conflict, think I fixed it.
Comment 23 Katrin Fischer 2017-10-07 21:39:53 UTC
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.
Comment 24 Katrin Fischer 2017-10-07 21:43:53 UTC Comment hidden (obsolete)
Comment 25 Katrin Fischer 2017-10-07 21:43:57 UTC Comment hidden (obsolete)
Comment 26 Katrin Fischer 2017-10-07 21:47:26 UTC
Apart from the qa script this worked nicely, please fix!
Comment 27 Lari Taskula 2019-10-09 15:53:00 UTC
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
Comment 28 Lari Taskula 2019-10-09 15:53:21 UTC
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
===============     ===============     ========================================
Comment 29 David Nind 2020-02-08 14:46:49 UTC
Patch no longer applies 8-(