Bug 29822 - Use table column selection modal for DefaultPatronSearchFields preference
Summary: Use table column selection modal for DefaultPatronSearchFields preference
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
: 30608 (view as bug list)
Depends on: 34256
Blocks: 33428
  Show dependency treegraph
 
Reported: 2022-01-07 17:45 UTC by Kelly McElligott
Modified: 2024-02-26 13:39 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact: Rasa Satinskiene
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/838
Text to go in the release notes:
This enhancement changes the `DefaultPatronSearchFields` system preference from an input field where field values separated by a comma are entered, to a modal window listing all patron fields (non-selectable fields are greyed out). This makes it much easier to set the standard fields for patron search, and helps avoid breaking the patron search when incorrect field values are entered.
Version(s) released in:
23.11.00


Attachments
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.17 KB, patch)
2022-12-23 11:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.22 KB, patch)
2022-12-23 20:46 UTC, David Nind
Details | Diff | Splinter Review
Modal highlighting why I think it's a bad idea (242.21 KB, image/png)
2023-02-09 14:30 UTC, Martin Renvoize
Details
Bug 29822: Use the modal (2.77 KB, patch)
2023-02-09 17:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.21 KB, patch)
2023-02-10 16:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Use the modal (2.61 KB, patch)
2023-02-10 16:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update code to reference by pipe (6.22 KB, patch)
2023-02-10 16:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Database and Preference description updates (4.85 KB, patch)
2023-02-10 16:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update unit tests (3.54 KB, patch)
2023-02-10 16:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.21 KB, patch)
2023-02-27 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Use the modal (2.61 KB, patch)
2023-02-27 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update code to reference by pipe (4.68 KB, patch)
2023-02-27 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Database and Preference description updates (4.85 KB, patch)
2023-02-27 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update unit tests (3.54 KB, patch)
2023-02-27 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.21 KB, patch)
2023-04-19 13:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Use the modal (2.61 KB, patch)
2023-04-19 13:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update code to reference by pipe (4.68 KB, patch)
2023-04-19 13:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Database and Preference description updates (4.85 KB, patch)
2023-04-19 13:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update unit tests (3.54 KB, patch)
2023-04-19 13:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.21 KB, patch)
2023-05-11 08:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Use the modal (2.61 KB, patch)
2023-05-11 08:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update code to reference by pipe (4.69 KB, patch)
2023-05-11 08:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Database and Preference description updates (4.85 KB, patch)
2023-05-11 08:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update unit tests (3.54 KB, patch)
2023-05-11 08:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Escape backslash on split (6.41 KB, patch)
2023-05-11 09:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.25 KB, patch)
2023-05-16 01:38 UTC, David Nind
Details | Diff | Splinter Review
Bug 29822: Use the modal (2.66 KB, patch)
2023-05-16 01:38 UTC, David Nind
Details | Diff | Splinter Review
Bug 29822: Update code to reference by pipe (4.73 KB, patch)
2023-05-16 01:38 UTC, David Nind
Details | Diff | Splinter Review
Bug 29822: Database and Preference description updates (4.89 KB, patch)
2023-05-16 01:38 UTC, David Nind
Details | Diff | Splinter Review
Bug 29822: Update unit tests (3.59 KB, patch)
2023-05-16 01:38 UTC, David Nind
Details | Diff | Splinter Review
Bug 29822: Escape backslash on split (6.45 KB, patch)
2023-05-16 01:38 UTC, David Nind
Details | Diff | Splinter Review
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.24 KB, patch)
2023-07-12 16:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Use the modal (2.65 KB, patch)
2023-07-12 16:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update code to reference by pipe (3.79 KB, patch)
2023-07-12 16:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Database and Preference description updates (4.94 KB, patch)
2023-07-12 16:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update unit tests (3.65 KB, patch)
2023-07-12 16:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Escape backslash on split (6.49 KB, patch)
2023-07-12 16:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.24 KB, patch)
2023-07-12 17:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Use the modal (2.65 KB, patch)
2023-07-12 17:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update code to reference by pipe (3.78 KB, patch)
2023-07-12 17:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Database and Preference description updates (4.95 KB, patch)
2023-07-12 17:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update unit tests (3.65 KB, patch)
2023-07-12 17:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Escape backslash on split (6.52 KB, patch)
2023-07-12 17:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.24 KB, patch)
2023-07-14 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Use the modal (2.65 KB, patch)
2023-07-14 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update code to reference by pipe (3.73 KB, patch)
2023-07-14 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Database and Preference description updates (4.95 KB, patch)
2023-07-14 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update unit tests (3.65 KB, patch)
2023-07-14 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Escape backslash on split (6.47 KB, patch)
2023-07-14 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29822: Update 'DefaultPatronSearchFields' to a select list (2.29 KB, patch)
2023-08-11 12:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29822: Use the modal (2.70 KB, patch)
2023-08-11 12:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29822: Update code to reference by pipe (3.79 KB, patch)
2023-08-11 12:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29822: Database and Preference description updates (5.01 KB, patch)
2023-08-11 12:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29822: Update unit tests (3.71 KB, patch)
2023-08-11 12:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29822: (QA follow-up) Use pipe for all field separators (2.72 KB, patch)
2023-08-11 12:16 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kelly McElligott 2022-01-07 17:45:23 UTC
As Koha did with system preferences such as borrower mandatory fields and borrower unwanted fields, it would be helpful and avoid searching errors if the system preference, DefaultPatronSearchFields would also use checkboxes.  These checkboxes will allow users to easily choose which fields Koha should use for searching. Currently, if a user puts a value in this field that is not valid, searching for patrons will be broken.
Comment 1 Martin Renvoize 2022-12-22 15:12:56 UTC
*** Bug 30608 has been marked as a duplicate of this bug. ***
Comment 2 Martin Renvoize 2022-12-23 11:07:14 UTC Comment hidden (obsolete)
Comment 3 David Nind 2022-12-23 20:46:06 UTC Comment hidden (obsolete)
Comment 4 David Nind 2022-12-23 21:22:45 UTC
The test plan I used (using KTD):

1. Check system preference DefaultPatronSearchFields and note that it is defined by entering a comma separated list.
2. Go to the patrons module and view the HTML source for the page:
   . search the source for 'standard' - this should have two results
   . note that the option value for the Standard input field are "firstname,middle_name,surname,othernames,cardnumber,userid"
3. Apply the patch.
4. Reload or search again for the DefaultPatronSearchFields system preference.
5. Note that this is now a dropdown list with selectable values, rather than needing to enter a comma separated list.
6. Add a field to the list, such as 'Primary email'.
7. Repeat step 2 - the standard input field should now include the field you added.
8. Make sure some patrons have a primary email address, such as firstname@gmail.com
9. Do a standard search for gmail.com
   ==> the results should include all the patrons you added a gmail address to
10. Do a specific search on the 'Primary email' field 
   ==> the search results should be the same as for step 9

BONUS: Perform the same search using the header search bar and the search options drop down (note: you need to change the 'Search type' from 'Starts with' to 'Contains').




(entered in the 'Other phone' field
Comment 5 David Nind 2022-12-23 21:31:03 UTC
Ignore the last bit of random text!

I was going to comment on the fact that you enter the mobile number in the 'Other number' field for this to work for patron search. 

This is different from entering the SMS number when the SMSSendDriver system preference is enabled.

It took me a while to figure this out.

Should I create a bug to sort out this confusion? I found a couple of bugs related to this, but Bug 14872 (specifically comment no. 6) would seem to be a better approach.
Comment 6 Martin Renvoize 2022-12-24 06:45:26 UTC
Thanks for testing David, always great to get your feedback.

I should have said somewhere I. The bug, the list of fields I picked is entirely arbitrarily picked at the moment.. I picked what made most sense to me.. so do feel free to help refine that list.

Regards SMS alert number, I found that one rather confusing too, though I wasn't aware of the bug you linked.  I like the idea of dropping the filed from borrowers and instead having a preference to define which field should contain this notification number.

I'll go comment there.

Many thanks for testing again.
Comment 7 Katrin Fischer 2023-01-30 23:07:49 UTC

1) I believe the list is now too limited. This is also likely to cause issues for libraries that have added fields not in your list. I think we should offer the full list, like BorrowerUnwantedField does - it would also simplify your patch :)

2) I believe we had discussed not changing this one, because of something with the extended patron attributes, but I cannot remember the exact issue now :(
Comment 8 Martin Renvoize 2023-02-09 14:10:19 UTC
(In reply to Katrin Fischer from comment #7)
> 
> 1) I believe the list is now too limited. This is also likely to cause
> issues for libraries that have added fields not in your list. I think we
> should offer the full list, like BorrowerUnwantedField does - it would also
> simplify your patch :)

Honestly, I'm a bit surprised you want every field listed and I think it increases the code burden personally.

> 2) I believe we had discussed not changing this one, because of something
> with the extended patron attributes, but I cannot remember the exact issue
> now :(

I can definitively say we do not support patron attributes as options here:

* The code limits the options to an allow list that can be seen in koha-tmpl/intranet-tmpl/prog/en/includes/patronfields.inc for the field selection in various patron search forms.
* The autocomplete javascript does NOT limit the fields in the same way, but is hard coded to use `me.` as a table identifier.. adding `extended_attributes.field_name` is very unlikely to work, but I have not tested.. what I have found though is that if you add a random "field" name, then autocomplete just completely breaks.
Comment 9 Martin Renvoize 2023-02-09 14:30:04 UTC
Created attachment 146430 [details]
Modal highlighting why I think it's a bad idea

I've attached a mock of the modal.. all the red dots are the fields I feel sohuld be actively disabled for this preference.. many of the remaining one's I also feel don't make much sense / would lead to unusable results - searching for 'mr' in the title field for example.. how is that remotely helpful?

Also, to convert this to a Modal we would need to do a database update to convert the existing preference from csv to pipe delimited string, update the preference descriptions and catch all the cases where the preference is currently in use and convert them from expecting a CSV to expecting a Pipe delimited string.

I'm happy to expand the select list.. but I really don't think it's appropriate to display a modal filled with options greyed out.
Comment 10 Katrin Fischer 2023-02-09 14:44:34 UTC
You can still use the modal and exclude those fields easily in the yaml file. That way we still have a consistent display/one place of code to change things and it's reduced to the more helfpul fields.
Comment 11 Martin Renvoize 2023-02-09 14:45:20 UTC
The excludes just get greyed out rather than removed as far as I can tell.
Comment 12 Katrin Fischer 2023-02-09 14:47:25 UTC
I'd include the note fields OPAC and circulation.

Wondering: how do we handle additional patron attributes here? Are they always searchable automatically or not searchable at all?
Comment 13 Katrin Fischer 2023-02-09 14:47:52 UTC
(In reply to Martin Renvoize from comment #11)
> The excludes just get greyed out rather than removed as far as I can tell.

Maybe we could rethink that? I still think having more ways to do a similar thing is not good.
Comment 14 Martin Renvoize 2023-02-09 14:52:22 UTC
Can we deal with patron attributes as it's own bug please.. this is rapidly going from a small improvement to a large and difficult development.
Comment 15 Katrin Fischer 2023-02-09 14:58:25 UTC
(In reply to Martin Renvoize from comment #14)
> Can we deal with patron attributes as it's own bug please.. this is rapidly
> going from a small improvement to a large and difficult development.

I don't want to change behaviour, was just wondering about existing behaviour and that is about this bug.

For BorrowerMandatoryFields you could add the patron attribute names to the system preference and it worked. If a similar feature existed for this, it could block switching it to a modal or at least we should verify first.
Comment 16 Martin Renvoize 2023-02-09 15:21:50 UTC
This preference does not affect the attributes in any way.. I've varified that in the code.

Attributes are handled as their own case in the main patron search page.. always searched.
Comment 17 Martin Renvoize 2023-02-09 17:37:25 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize 2023-02-09 17:39:31 UTC
(In reply to Katrin Fischer from comment #7)
> it would also simplify your patch :)

Not in the slightest.. it makes it significantly more involved and has a much higher risk of regression.
Comment 19 Martin Renvoize 2023-02-10 16:08:38 UTC Comment hidden (obsolete)
Comment 20 Martin Renvoize 2023-02-10 16:08:41 UTC Comment hidden (obsolete)
Comment 21 Martin Renvoize 2023-02-10 16:08:44 UTC Comment hidden (obsolete)
Comment 22 Martin Renvoize 2023-02-10 16:08:47 UTC Comment hidden (obsolete)
Comment 23 Martin Renvoize 2023-02-10 16:08:51 UTC Comment hidden (obsolete)
Comment 24 Martin Renvoize 2023-02-10 16:09:58 UTC
1 patch turns into 5 to convert it to the Modal approach.. and the unit tests are failing.. I can't see why the split doesn't appear to work with pipe instead of comma as the separation character.
Comment 25 Martin Renvoize 2023-02-27 12:11:15 UTC Comment hidden (obsolete)
Comment 26 Martin Renvoize 2023-02-27 12:11:18 UTC Comment hidden (obsolete)
Comment 27 Martin Renvoize 2023-02-27 12:11:20 UTC Comment hidden (obsolete)
Comment 28 Martin Renvoize 2023-02-27 12:11:23 UTC Comment hidden (obsolete)
Comment 29 Martin Renvoize 2023-02-27 12:11:26 UTC Comment hidden (obsolete)
Comment 30 Martin Renvoize 2023-02-27 12:11:40 UTC
Rebased
Comment 31 Sally 2023-03-17 10:25:18 UTC
Sorry, I get a template error:

Template process failed: file error - parse error - patronfields.inc line 90-91: unexpected token ([)
  [% standard.replace('|', ',')
        [% SET full_address = 'streetnumber,streettype,address,address2,city,state,zipcode,country' %] at /kohadevbox/koha/C4/Templates.pm line 127

(I also tried on PTFS' and Bywater's sandboxes, one with the patch applied, and one with the patch applied after initialisation - just to make sure it wasn't a sandbox fault.)
Comment 32 Martin Renvoize 2023-04-19 13:18:25 UTC Comment hidden (obsolete)
Comment 33 Martin Renvoize 2023-04-19 13:18:28 UTC Comment hidden (obsolete)
Comment 34 Martin Renvoize 2023-04-19 13:18:31 UTC Comment hidden (obsolete)
Comment 35 Martin Renvoize 2023-04-19 13:18:34 UTC Comment hidden (obsolete)
Comment 36 Martin Renvoize 2023-04-19 13:18:38 UTC Comment hidden (obsolete)
Comment 37 Martin Renvoize 2023-04-19 13:22:48 UTC
We should be back on again now.. master is moving so fast at the moment :)
Comment 38 Jonathan Druart 2023-05-11 07:38:58 UTC
 FAIL   installer/data/mysql/atomicupdate/bug_29822.pl
   FAIL   file permissions
                File must have the exec flag

 OK     installer/data/mysql/mandatory/sysprefs.sql
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/js_includes.inc
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc
 FAIL   koha-tmpl/intranet-tmpl/prog/en/includes/patronfields.inc
   FAIL   forbidden patterns
                forbidden pattern: Do not use line breaks inside template tags (bug 18675) (line 90)
   FAIL   valid_template
                parse error - /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/patronfields.inc line 90-91: unexpected token ([)
  [% standard.replace('|', ',')
        [% SET full_address = 'streetnumber,streettype,address,address2,city,state,zipcode,country' %]

 OK     koha-tmpl/intranet-tmpl/prog/js/patron-autocomplete.js
 OK     t/db_dependent/selenium/patrons_search.t

Processing additional checks OK!

Running tests (1)
        * Proving /kohadevbox/koha/t/db_dependent/selenium/patrons_search.t KO!
    #   Failed test 'All standard fields are searchable if DefaultPatronSearchFields not set. middle_name is there.'    #   at /kohadevbox/koha/t/db_dependent/selenium/patrons_search.t line 163.
     #          got: '0'    #     expected: '14'
     # Looks like you planned 25 tests but ran 1.     # Looks like you failed 1 test of 1 run.
 #   Failed test 'Search patrons'
#   at /kohadevbox/koha/t/db_dependent/selenium/patrons_search.t line 341.
 Can't call method "get_value" on an undefined value at /kohadevbox/koha/t/db_dependent/selenium/patrons_search.t line 164.
 # Looks like your test exited with 255 just after 1.
 /kohadevbox/koha/t/db_dependent/selenium/patrons_search.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 1/1 subtests  
Test Summary Report-------------------
 /kohadevbox/koha/t/db_dependent/selenium/patrons_search.t (Wstat: 65280 Tests: 1 Failed: 1)
Comment 39 Martin Renvoize 2023-05-11 08:42:45 UTC Comment hidden (obsolete)
Comment 40 Martin Renvoize 2023-05-11 08:42:48 UTC Comment hidden (obsolete)
Comment 41 Martin Renvoize 2023-05-11 08:42:51 UTC Comment hidden (obsolete)
Comment 42 Martin Renvoize 2023-05-11 08:42:54 UTC Comment hidden (obsolete)
Comment 43 Martin Renvoize 2023-05-11 08:42:57 UTC Comment hidden (obsolete)
Comment 44 Martin Renvoize 2023-05-11 08:44:31 UTC
Rebased.. seems we lost a closing brace.. should be working now
Comment 45 Jonathan Druart 2023-05-11 09:04:41 UTC
The tests are still failing for me.
Comment 46 Jonathan Druart 2023-05-11 09:41:38 UTC Comment hidden (obsolete)
Comment 47 David Nind 2023-05-16 01:38:08 UTC Comment hidden (obsolete)
Comment 48 David Nind 2023-05-16 01:38:12 UTC Comment hidden (obsolete)
Comment 49 David Nind 2023-05-16 01:38:15 UTC Comment hidden (obsolete)
Comment 50 David Nind 2023-05-16 01:38:18 UTC Comment hidden (obsolete)
Comment 51 David Nind 2023-05-16 01:38:21 UTC Comment hidden (obsolete)
Comment 52 David Nind 2023-05-16 01:38:24 UTC Comment hidden (obsolete)
Comment 53 David Nind 2023-05-16 01:42:38 UTC
Testing notes using KTD:

1. Check system preference DefaultPatronSearchFields and note that it is defined by entering a comma separated list.
2. Go to the patrons module and view the HTML source for the page:
   . search the source for 'standard' - this should have two results
   . note that the option value for the Standard input field are "firstname,middle_name,surname,othernames,cardnumber,userid"
3. Break the patron search:
   . for the DefaultPatronSearchFields system preference, add a field that doesn't exist, for example: someunknownfield
   . for patron search, enter any value in the search field and make sure standard is selected for the search field drop down list
   . you get a pop-up error message:
      "Something went wrong when loading the table.
      500: Internal Server Error."
4. Apply the patch.
5. Update the database: databaseupdate
6. Restart everything: flushmemcached, restart_all
7. Reload or search again for the DefaultPatronSearchFields system preference.
8. Note that this is now a modal window with all the patron fields that are selectable shown (greyed out items are not selectable), rather than needing to enter a comma separated list.
9. Select some fields, such as 'First name', 'Surname', 'Primary email'.
10. Repeat step 2 - the standard input field now includes the fields you selected.
11. Make sure some patrons have a primary email address, such as firstname@gmail.com
12. Do a standard search for gmail.com
   ==> the results should include all the patrons you added a gmail address to
13. Do a specific search on the 'Primary email' field 
   ==> the search results should be the same as for step 12
14. Run the tests, these should pass (note - KTD needs to be started with selenium: ktd --selenium up):
    . prove t/db_dependent/selenium/patrons_search.t

BONUS: Perform the same search using the header search bar and the search options drop down (note: you need to change the 'Search type' from 'Starts with' to 'Contains').
Comment 54 Jonathan Druart 2023-07-04 12:34:25 UTC
Sorry Martin, this needs another rebase (bug 34092). Does not seem hard but not trivial.
Comment 55 Martin Renvoize 2023-07-12 16:17:58 UTC
Bugger.. I thought this was part of that dependency tree already :(.

Looking at the rebase now.
Comment 56 Martin Renvoize 2023-07-12 16:47:15 UTC Comment hidden (obsolete)
Comment 57 Martin Renvoize 2023-07-12 16:47:21 UTC Comment hidden (obsolete)
Comment 58 Martin Renvoize 2023-07-12 16:47:24 UTC Comment hidden (obsolete)
Comment 59 Martin Renvoize 2023-07-12 16:47:27 UTC Comment hidden (obsolete)
Comment 60 Martin Renvoize 2023-07-12 16:47:30 UTC Comment hidden (obsolete)
Comment 61 Martin Renvoize 2023-07-12 16:47:34 UTC Comment hidden (obsolete)
Comment 62 Martin Renvoize 2023-07-12 16:48:31 UTC
OK.. rebased.. but waiting for my ktd to startup so I can re-run the tests.  Patches uploaded just in case someone is already available ;P
Comment 63 Martin Renvoize 2023-07-12 17:00:58 UTC
Unit tests failing with rebase.. resetting to Failed whilst I work on that
Comment 64 Martin Renvoize 2023-07-12 17:19:32 UTC
Created attachment 153379 [details] [review]
Bug 29822: Update 'DefaultPatronSearchFields' to a select list

This patch updates the `DefaultPatronSearchFields` preference from
freetext expecting a comma delimited list of borrower fields to a
multi-select with a sensible list of fields available.

I opted to not use the Modal approach with all borrower fields listed as
I felt it made more sense to only expose a more limited subset of the
fields available to us.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 65 Martin Renvoize 2023-07-12 17:19:35 UTC
Created attachment 153380 [details] [review]
Bug 29822: Use the modal

As requested by Katrin.. switch to the modal and expose all fields from
the borrowers database table

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 66 Martin Renvoize 2023-07-12 17:19:38 UTC
Created attachment 153381 [details] [review]
Bug 29822: Update code to reference by pipe

Moving to modalselect also has the effect of moving from comma delimited
to pipe delimitation for the preference contents

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 67 Martin Renvoize 2023-07-12 17:19:41 UTC
Created attachment 153382 [details] [review]
Bug 29822: Database and Preference description updates

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 68 Martin Renvoize 2023-07-12 17:19:45 UTC
Created attachment 153383 [details] [review]
Bug 29822: Update unit tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 69 Martin Renvoize 2023-07-12 17:19:48 UTC
Created attachment 153384 [details] [review]
Bug 29822: Escape backslash on split

+ effectively replace 'standard', don't display it.
+ avoid mixing | and ,

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 70 Martin Renvoize 2023-07-14 08:55:08 UTC
Created attachment 153442 [details] [review]
Bug 29822: Update 'DefaultPatronSearchFields' to a select list

This patch updates the `DefaultPatronSearchFields` preference from
freetext expecting a comma delimited list of borrower fields to a
multi-select with a sensible list of fields available.

I opted to not use the Modal approach with all borrower fields listed as
I felt it made more sense to only expose a more limited subset of the
fields available to us.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 71 Martin Renvoize 2023-07-14 08:55:11 UTC
Created attachment 153443 [details] [review]
Bug 29822: Use the modal

As requested by Katrin.. switch to the modal and expose all fields from
the borrowers database table

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 72 Martin Renvoize 2023-07-14 08:55:14 UTC
Created attachment 153444 [details] [review]
Bug 29822: Update code to reference by pipe

Moving to modalselect also has the effect of moving from comma delimited
to pipe delimitation for the preference contents

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 73 Martin Renvoize 2023-07-14 08:55:18 UTC
Created attachment 153445 [details] [review]
Bug 29822: Database and Preference description updates

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 74 Martin Renvoize 2023-07-14 08:55:21 UTC
Created attachment 153446 [details] [review]
Bug 29822: Update unit tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 75 Martin Renvoize 2023-07-14 08:55:25 UTC
Created attachment 153447 [details] [review]
Bug 29822: Escape backslash on split

+ effectively replace 'standard', don't display it.
+ avoid mixing | and ,

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 76 Nick Clemens 2023-08-11 11:19:48 UTC
This and 33428 are going to clash, happy to rebase mine on top of this

Would it be better to leave the fields from the pref as pipe, and just move the other fields 'full_address', 'all_emails' etc to use the pipe too? Then we don't have to replace in the one instance for splitting.

I don't understand the comment:
+ effectively replace 'standard', don't display it.
Comment 77 Nick Clemens 2023-08-11 12:16:04 UTC
Created attachment 154361 [details] [review]
Bug 29822: Update 'DefaultPatronSearchFields' to a select list

This patch updates the `DefaultPatronSearchFields` preference from
freetext expecting a comma delimited list of borrower fields to a
multi-select with a sensible list of fields available.

I opted to not use the Modal approach with all borrower fields listed as
I felt it made more sense to only expose a more limited subset of the
fields available to us.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 78 Nick Clemens 2023-08-11 12:16:07 UTC
Created attachment 154362 [details] [review]
Bug 29822: Use the modal

As requested by Katrin.. switch to the modal and expose all fields from
the borrowers database table

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 79 Nick Clemens 2023-08-11 12:16:10 UTC
Created attachment 154363 [details] [review]
Bug 29822: Update code to reference by pipe

Moving to modalselect also has the effect of moving from comma delimited
to pipe delimitation for the preference contents

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 80 Nick Clemens 2023-08-11 12:16:13 UTC
Created attachment 154364 [details] [review]
Bug 29822: Database and Preference description updates

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 81 Nick Clemens 2023-08-11 12:16:16 UTC
Created attachment 154365 [details] [review]
Bug 29822: Update unit tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 82 Nick Clemens 2023-08-11 12:16:19 UTC
Created attachment 154366 [details] [review]
Bug 29822: (QA follow-up) Use pipe for all field separators

- Tidy atomic update

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 83 Tomás Cohen Arazi 2023-09-25 13:49:23 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 84 Fridolin Somers 2023-09-25 19:08:39 UTC
Enhancement not pushed to 23.05.x
Comment 85 Rasa Satinskiene 2024-02-26 13:39:19 UTC
Manual updated