[Bug 23817] New: Searching by patron phone number is contingent on how it was entered
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Bug ID: 23817 Summary: Searching by patron phone number is contingent on how it was entered Change sponsored?: --- Product: Koha Version: 18.11 Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: kelly@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com If a library chooses to enter their patron phone numbers in as xxx-xxx-xxxx when creating the patron record; then when the phone number is searched with the filters, the library must always use hyphens when searching or the search will not work. It would be nice that the number could be searched in the filters with either hyphens or no hyphens and the library would get the same results. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|18.11 |master CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #1 from David Cook <dcook@prosentient.com.au> --- I've had a few libraries talk to me about this, and I imagine that there are many many libraries around the world who find this frustrating. I have a solution that normalizes the search term as well as the database field, so it will ignore everything aside from the numbers. So searching "1-(306)-123-4567" will match "1 306 123 4567", because they'll both be normalized to be "13061234567". -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Searching by patron phone |Normalize phone number when |number is contingent on how |searching patrons |it was entered | Assignee|koha-bugs@lists.koha-commun |dcook@prosentient.com.au |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- I wasn't sure how to handle searching with or without prefixes, but I think I'll add a wildcard on the front of the number so that "123-4567" will match "1-306-123-4567", or "306-123-4567" or "123-4567". I think that makes sense... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Normalizing to remove spaces, - and other non numeric characters would probably work for how we enter phone numbers here too, but I wonder if some libraries might enter multiple phone numbers? Then we could create a non-working search string. I can certainly imagine this happening, also thinking of migrations. Maybe we need to do a search both for the value entered and a normalized version? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #3)
Normalizing to remove spaces, - and other non numeric characters would probably work for how we enter phone numbers here too, but I wonder if some libraries might enter multiple phone numbers? Then we could create a non-working search string. I can certainly imagine this happening, also thinking of migrations. Maybe we need to do a search both for the value entered and a normalized version?
In my opinion, if they're putting multiple phone numbers into one field, they're using Koha wrong, and I don't think that we should modify Koha to support that behaviour. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- If we are changing how search works, I think it's valid to think about edge cases. The field is not used for anything by Koha currently in most cases (phone notifications are not a thing at all around here). So there is nothing to stop people from entering different formats and often notes too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #5)
If we are changing how search works, I think it's valid to think about edge cases. The field is not used for anything by Koha currently in most cases (phone notifications are not a thing at all around here). So there is nothing to stop people from entering different formats and often notes too.
Indeed it looks like we've only added validation to the "smsalertnumber" and not the other phone fields. But I'd say that's a bug rather than a feature. Something to be removed rather than accommodated. The current phone search is fundamentally broken. If you've stored the number as "0412 345 678", you'll never retrieve the record if you input "0412345678" or "(04)12 345 678" or any other format. I'll upload my patch, and then folk can decide what they want to do about it. But in my opinion, every telephone database field should only contain 1 value. If people need multiple phone numbers or notes, then they need a new feature with a denormalized contact table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I never asked not to improve the search, just to think about ways we could make it flexible enough and maybe work for other cases too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- It looks like there's a new wrinkle in this one... I was patching an older Koha that still used C4::Utils::DataTables::Members, but in master it looks like we're using the REST API, which I suspect will be much harder to change. But let's see... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #8)
It looks like there's a new wrinkle in this one...
I was patching an older Koha that still used C4::Utils::DataTables::Members, but in master it looks like we're using the REST API, which I suspect will be much harder to change. But let's see...
I don't think that it's going to be possible to easily fix this with code in master. Using literal SQL, it's possible to craft a search using DBIx::Class to do the search[1], but we can't pass in that literal SQL via the API. We could potentially inject the literal SQL within the API backend but it would need to be in the Koha/REST/Plugin/Objects.pm helper... and it would involve recursively navigating the data (more than it already is...), and that would really just be a hack at that stage anyway. [1] my $test = $rs->search(\[ "regexp_replace(phone,?,?) LIKE ?",'[^0-9]','','%1234%']); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- I think the only way forward is to normalize the data in the database (ie only store [\+0-9], and normalize the search query in "koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc" to [\+0-9] as well. It would still be necessary to update all the patron data in the database. We'd need something like "touch_all_patron_records.pl", or a once off SQL update to strip out everything that isn't [\+0-9]. regexp_replace(phone,'[^0-9]','') would be useful for that. This patch doesn't take into account Katrin's edge cases of multiple phone numbers or text comments in the "phone" field. There's no way to do that unless we completely change how DataTables work with the REST API. I've taken a look at a large patron database and over 99% of phone number data is just 1 phone number. Out of 100,000+ records, I see maybe 5 that have text comments in them. I see 2 records with 2 phone numbers in them. If they need to note the phone number belongs to a particular person or is text only, I think that needs to go in the "Contact note". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- I do have one more idea that wouldn't involve changing the data... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion --- Comment #12 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #11)
I do have one more idea that wouldn't involve changing the data...
There are some ways to provide additional columns to the DBIC query, but it auto-escapes single quotes, which makes it impossible to use regexp_replace as a derived column. Plus the DBIC search only works on real columns it seems (which is a bit silly...) -- I suppose another solution would be a pure Javascript solution that strips anything but [\+0-9] out of the phone number field when saving and searching... -- Another option would be a database trigger, but Koha has avoided those to date. -- I suppose another idea would be to add a new phone number column called "phone_normalized". That seems unnecessary though. -- Now I've spent quite a bit of time on this one, and I suspect it's going to go nowhere, unfortunately. There just isn't a good way to deal with this one using DBIC and the REST API I reckon... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- Created attachment 140610 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140610&action=edit Bug 23817: Normalize phone number when storing and when searching in Patrons module This patch normalizes the phone number when stored via Koha::Patron, and normalizes the phone number when searching "Primary phone" in the Patrons module. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as 12345678901 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "Primary phone" 6. Search for '1-(234)-567-8901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #14 from David Nind <david@davidnind.com> --- I've tested this, and it works as per the test plan. While this works, and I probably don't appreciate the effort required to get searching the phone number to work -- I have some feedback. I'm not a fan of displaying the phone number as a number without any formatting, such as +, dashes, spaces, and brackets: - Usability: They are hard to read or verify by a human - They are displayed on transit slips (see note on the patron edit form), I think it makes it harder to read phone numbers if they are displayed as a string of numbers without formatting - It doesn't follow best practices for web forms Some ideas (but not how to code): 1. Validate the input of phone numbers, so they are entered without inputting +, dashes, spaces, brackets, etc - see this article: https://uxplanet.org/phone-number-field-design-best-practices-23957cbd86d5 2. Number stored as just a number without any formatting 3. Displayed for patron according to a format display setting (as an aside, this would work going forward, but not with existing numbers if they have been entered inconsistently) 4. As long as the number is stored without formatting, then any characters such as brackets, spaces, dashes, +, etc could be removed by JavaScript when inputting in the search box. I couldn't find an "authoritative" article about phone number and web form input, validation and display - but I didn't look very hard: - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/tel - Maybe there is a well supported JavaScript library for resolving this, such as https://github.com/google/libphonenumber I'm sure there is a trade-off between being able to search (how the number is stored), vs how the number is displayed. Maybe sorting out the input of phone numbers as a separate bug would then make solving the search problem easier.... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #15 from David Cook <dcook@prosentient.com.au> --- Thanks for testing and the feedback, David :) One thing to note about the phone number input is that the web form isn't the only way that phone numbers are added. Patrons can be added via the "Import patrons" tool as well, so I think the ultimate phone number validation needs to be done on the backend. I think adding Javascript validation would be helpful from the UX point of view though. I was thinking a bit about formatting and we have had some discussion on the listserv (hopefully there will be more discussion there). Internationalization does make the phone number formatting tough. For instance, in Australia I think we have 4+ different phone number formats (although in practical terms only 2 would probably be used by the library) whereas back in Canada I think we only had 1 phone number format. -- Ideally, I would rather just normalize the data only when searching (like I'm doing in Koha 21.11), but the DataTables/REST API mechanism makes that seemingly impossible. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #16 from David Cook <dcook@prosentient.com.au> --- Created attachment 140648 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140648&action=edit Bug 23817: [Alternative patch] Normalize phone number only when searching This patch rewrites the phone number DBIC query when sent from DataTables like in the Patron module. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as '1-(234)-567-8901' 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "Primary phone" 6. Search for '12345678901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #15)
Ideally, I would rather just normalize the data only when searching (like I'm doing in Koha 21.11), but the DataTables/REST API mechanism makes that seemingly impossible.
Maybe not impossible after all... just challenging haha. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #18 from David Cook <dcook@prosentient.com.au> --- Still looking for testers for this one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 solene.ngamga@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |solene.ngamga@inlibro.com Status|Needs Signoff |Patch doesn't apply --- Comment #19 from solene.ngamga@inlibro.com --- Bug 23817 - Normalize phone number when searching patrons 140610 - Bug 23817: Normalize phone number when storing and when searching in Patrons module 140648 - Bug 23817: [Alternative patch] Normalize phone number only when searching Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 23817: Normalize phone number when storing and when searching in Patrons module Applying: Bug 23817: [Alternative patch] Normalize phone number only when searching Using index info to reconstruct a base tree... M Koha/REST/Plugin/Objects.pm M koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc Auto-merging Koha/REST/Plugin/Objects.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 23817: [Alternative patch] Normalize phone number only when searching hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-23817-Alternative-patch-Normalize-phone-number-wqS2eF.patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #20 from David Cook <dcook@prosentient.com.au> --- Hi Solene, You can't apply both patches. The second one it marked as "alternative patch", so it's either/or. That said, perhaps I should just obsolete the first patch, since the first patch is destructive while the second is not... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #140610|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 solene.ngamga@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #21 from solene.ngamga@inlibro.com --- The patch works well. But I cannot put it on the master right now. I get an error like this "Failed to attach patch to bug 23817, status=200" when I run the "attach" command. I tried with "git bz attach 23817 HEAD" and "git bz attach -e 23817 HEAD" but neither of them worked. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Marius <marius.mandrescu@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #140648|0 |1 is obsolete| | --- Comment #22 from Marius <marius.mandrescu@inlibro.com> --- Created attachment 145219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145219&action=edit Bug 23817: [Alternative patch] Normalize phone number only when searching This patch rewrites the phone number DBIC query when sent from DataTables like in the Patron module. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as '1-(234)-567-8901' 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "Primary phone" 6. Search for '12345678901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower Signed-off-by: Marius Mandrescu <marius.mandrescu@inLibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 solene.ngamga@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145219|0 |1 is obsolete| | --- Comment #23 from solene.ngamga@inlibro.com --- Created attachment 145221 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145221&action=edit Bug 23817: [Alternative patch] Normalize phone number only when searching This patch rewrites the phone number DBIC query when sent from DataTables like in the Patron module. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as '1-(234)-567-8901' 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "Primary phone" 6. Search for '12345678901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower Signed-off-by: Marius Mandrescu <marius.mandrescu@inLibro.com> Signed-off-by: Solene Ngamga <solene.ngamga@inLibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |Failed QA --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- FAIL Koha/REST/V1/Patrons.pm OK critic OK forbidden patterns OK git manipulation OK pod FAIL pod coverage POD is missing for 'preprocess_dbic_for_datatables' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #25 from David Cook <dcook@prosentient.com.au> --- Created attachment 151961 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=151961&action=edit Bug 23817: Normalize phone number when searching This patch rewrites the phone number DBIC query when sent from DataTables like in the Patron module. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as '1-(234)-567-8901' 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "All phones" 6. Search for '12345678901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #26 from David Cook <dcook@prosentient.com.au> --- Created attachment 151962 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=151962&action=edit Bug 23817: Include all phone numbers -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145221|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #151961|0 |1 is obsolete| | --- Comment #27 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 152450 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152450&action=edit Bug 23817: Normalize phone number when searching This patch rewrites the phone number DBIC query when sent from DataTables like in the Patron module. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as '1-(234)-567-8901' 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "All phones" 6. Search for '12345678901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #151962|0 |1 is obsolete| | --- Comment #28 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 152451 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152451&action=edit Bug 23817: Include all phone numbers Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #29 from David Cook <dcook@prosentient.com.au> --- Going to Fail QA my own patches... I really should include a unit test for "preprocess_dbic_for_datatables" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=34257 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=34257 | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=14620 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.derscheid@lmscloud.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Katie Bliss <kebliss@dmpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kebliss@dmpl.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Patch doesn't apply --- Comment #30 from David Cook <dcook@prosentient.com.au> --- Failed QA (missing unit test) plus patch doesn't apply anymore, although it's not hard to fix the merge conflict -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=39882 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #31 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- *** Bug 39882 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Danielle M. <dmeininger591@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dmeininger591@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152450|0 |1 is obsolete| | Attachment #152451|0 |1 is obsolete| | --- Comment #32 from David Cook <dcook@prosentient.com.au> --- Created attachment 191334 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191334&action=edit Bug 23817: Normalize phone number when searching This patch rewrites the phone number DBIC query when sent from DataTables like in the Patron module. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as '1-(234)-567-8901' 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "All phones" 6. Search for '12345678901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #191334|0 |1 is obsolete| | --- Comment #33 from David Cook <dcook@prosentient.com.au> --- Created attachment 191335 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191335&action=edit Bug 23817: Normalize phone number when searching This patch rewrites the phone number DBIC query when sent from DataTables like in the Patron module. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as '1-(234)-567-8901' 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "All phones" 6. Search for '12345678901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower 10. prove t/Koha/Patrons/DataTables.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #34 from David Cook <dcook@prosentient.com.au> --- Only took two years to get back on the horse... but glad to have this one back in action. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #191335|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #35 from David Nind <david@davidnind.com> --- Created attachment 192002 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192002&action=edit Bug 23817: Normalize phone number when searching This patch rewrites the phone number DBIC query when sent from DataTables like in the Patron module. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as '1-(234)-567-8901' 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "All phones" 6. Search for '12345678901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower 10. prove t/Koha/Patrons/DataTables.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #36 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Step 2 didn't work for me. I just searched for the koha patron and edited it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|kyle@bywatersolutions.com |testopia@bugs.koha-communit | |y.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #192002|0 |1 is obsolete| | --- Comment #37 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 192025 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192025&action=edit Bug 23817: Normalize phone number when searching This patch rewrites the phone number DBIC query when sent from DataTables like in the Patron module. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as '1-(234)-567-8901' 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "All phones" 6. Search for '12345678901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower 10. prove t/Koha/Patrons/DataTables.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |39882 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39882 [Bug 39882] Add phone number masking option -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Chip Halvorsen <Chip.Halvorsen@WestlakeLibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Chip.Halvorsen@WestlakeLibr | |ary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #38 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- (In reply to Kyle M Hall (khall) from comment #37)
Created attachment 192025 [details] [review] Bug 23817: Normalize phone number when searching
This patch rewrites the phone number DBIC query when sent from DataTables like in the Patron module.
Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry. pl?op=modify&destination=circ&borrowernumber=51 2. Input '1-(234)-567-8901' into the 'Primary phone' field 3. Note on the next screen the phone number is shown as '1-(234)-567-8901' 4. Go to http://localhost:8081/cgi-bin/koha/members/members-home.pl 5. Choose "Search field" of "All phones" 6. Search for '12345678901' 7. Note you are taken to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 8. Try out different permutations like '234 567 8901' or '5678901' 9. Note that every permutation works to find the borrower
10. prove t/Koha/Patrons/DataTables.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Searching works for any variant that includes numbers and spaces, but fails once I include the dashes or parens, so the patron doesn't come up when I search for their phone number exactly as it appears in the record. Also, this normalization is not applied to the Standard search when Primary Phone is added to DefaultPatronSearchFields. Finally, the QA tools are mad about missing POD. [FAIL] Koha/Patrons/DataTables.pm FAIL pod_coverage sub _parse_dbic_query_for_dt has no pod coverage. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #39 from David Cook <dcook@prosentient.com.au> --- (In reply to Andrew Fuerste-Henry from comment #38)
Searching works for any variant that includes numbers and spaces, but fails once I include the dashes or parens, so the patron doesn't come up when I search for their phone number exactly as it appears in the record.
Hmm good to know. I thought I would've tested that, but the unit test is missing it, so I'll need to update that. Do you have some example phone numbers I could use for testing?
Also, this normalization is not applied to the Standard search when Primary Phone is added to DefaultPatronSearchFields.
Hmm ok I can look that.
Finally, the QA tools are mad about missing POD. [FAIL] Koha/Patrons/DataTables.pm FAIL pod_coverage sub _parse_dbic_query_for_dt has no pod coverage.
This one is driving me crazy. It seems like Tomas arbitrarily started requiring POD for private/internal methods when that's not a thing that's typically done for Perl modules... https://gitlab.com/koha-community/qa-test-tools/-/work_items/103#note_316781... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=43391 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #40 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
Finally, the QA tools are mad about missing POD. [FAIL] Koha/Patrons/DataTables.pm FAIL pod_coverage sub _parse_dbic_query_for_dt has no pod coverage.
This one is driving me crazy. It seems like Tomas arbitrarily started requiring POD for private/internal methods when that's not a thing that's typically done for Perl modules...
https://gitlab.com/koha-community/qa-test-tools/-/work_items/ 103#note_3167812268
This warning is not new, but you only need to fix it when you have added or updated the method. https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL13:_POD -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23817 --- Comment #41 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- (In reply to David Cook from comment #39)
(In reply to Andrew Fuerste-Henry from comment #38)
Searching works for any variant that includes numbers and spaces, but fails once I include the dashes or parens, so the patron doesn't come up when I search for their phone number exactly as it appears in the record.
Hmm good to know. I thought I would've tested that, but the unit test is missing it, so I'll need to update that. Do you have some example phone numbers I could use for testing?
Also, this normalization is not applied to the Standard search when Primary Phone is added to DefaultPatronSearchFields.
Hmm ok I can look that.
Finally, the QA tools are mad about missing POD. [FAIL] Koha/Patrons/DataTables.pm FAIL pod_coverage sub _parse_dbic_query_for_dt has no pod coverage.
This one is driving me crazy. It seems like Tomas arbitrarily started requiring POD for private/internal methods when that's not a thing that's typically done for Perl modules...
https://gitlab.com/koha-community/qa-test-tools/-/work_items/ 103#note_3167812268
I've put the question of POD for private methods on the agenda for tomorrow's dev meeting. In testing this I just used the number from the test plan, '1-(234)-567-8901' -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org