https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #3 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 161874 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161874&action=edit Bug 36025 Incorrect clause in patron search This patch removes the search clause for extended_attributes when there is no patron attribute types which are searchable by default. The clause was incorrectly added because the array extended_attribute_types was rendered as [""] in the template patron-search.inc most likely due to a bug in DBIx::Class. Test plan: * Make sure that no borrower attribute type is set to "searched by default" in Koha administration -> Patron attribute types. * Enable the network monitring in the developer tools in your web browser (Ctrl + Shift + E in firefox or Ctrl + Shift + I and select the "Network" tab). * Go to the patrons view members-home.pl (click patrons in top menu bar). * Search for a string that will match multiple borrowers (to not be automatically redirected) * Examine the ajax-request and make sure that "extended_attributes.code" is NOT part of the query. * Also, in koha-testing-docker with selenium enabled, run 'perl t/db_dependent/selenium/patrons_search.t' -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=34519 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=35361 -- 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=36025 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- 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=36025 --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 161915 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161915&action=edit Bug 36025: Incorrect clause in patron search This patch removes the search clause for extended_attributes when there is no patron attribute types which are searchable by default. The clause was incorrectly added because the array extended_attribute_types was rendered as [""] in the template patron-search.inc most likely due to a bug in DBIx::Class. Test plan: * Make sure that no borrower attribute type is set to "searched by default" in Koha administration -> Patron attribute types. * Enable the network monitring in the developer tools in your web browser (Ctrl + Shift + E in firefox or Ctrl + Shift + I and select the "Network" tab). * Go to the patrons view members-home.pl (click patrons in top menu bar). * Search for a string that will match multiple borrowers (to not be automatically redirected) * Examine the ajax-request and make sure that "extended_attributes.code" is NOT part of the query. * Also, in koha-testing-docker with selenium enabled, run 'perl t/db_dependent/selenium/patrons_search.t' Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply CC| |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=36025 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161914|0 |1 is obsolete| | Attachment #161915|0 |1 is obsolete| | --- Comment #6 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 175917 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=175917&action=edit Bug 36025: Selenium test for the query parameters The test verifies that there is no clause for extended attributes in the patron search query when no patron attribute is searchable by default. NC amended patch - tidied Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Current status: Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #8 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- I tried to rebase, it applies but tests don't pass - can you look Andreas? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #9 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 175952 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=175952&action=edit Bug 36025: Selenium test for the query parameters Fix selenium test: 1. Use classname of form instead of no longer existing id 2. Fix test count -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|23.11 |Main -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|katrin.fischer@bsz-bw.de |testopia@bugs.koha-communit | |y.org Status|Signed Off |Passed QA --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- QA by RM. The selenium tests gave me some trouble at first, but they run smoothly on a clean database. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Katrin Fischer from comment #12)

The Selenium tests are fine, but we broke some other tests:

t_db_dependent_Koha_Template_Plugin_ExtendedAttributeTypes_t.codes

This hinders pushing new features. Please help to fix. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Failed QA --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I have reverted these patches from main. There are some other concerns that were raised on Mattermost: * A change to .pm should have unit test, a selenium test doesn't cover this requirement. * Context could be forced in the .tt avoiding the change of the return signature. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |REVERTED, REMOVE FROM release notes| |RELASE NOTES -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #16 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #15)

Can you explain the selenium test please? What you are doing with this JS code there?

The selenium test is trying to prove that there are no extended attributes included in the search, I had looked but wasn't able to find a better way of doing that -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #175917|0 |1 is obsolete| | Attachment #175918|0 |1 is obsolete| | Attachment #175952|0 |1 is obsolete| | --- Comment #17 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 176747 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176747&action=edit Bug 36025: Selenium test for the query parameters The test verifies that there is no clause for extended attributes in the patron search query when no patron attribute is searchable by default. NC amended patch - tidied Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #19 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Removed module changes, updated template, please test, this is an issue for libraries with many attributes that are not searchable, performance can be highly impacted (see bug 33554) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #21 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 176824 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176824&action=edit Bug 36025: Fix failure when pref is off Prevent JS error if ExtendedPatronAttributes is off "Uncaught TypeError: options.extended_attribute_types is undefined" -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #23 from David Nind <david@davidnind.com> --- I've attempted to test - everything works except the Selenium tests. They don't pass for me.[1] [1] Results from running the Selenium tests: prove t/db_dependent/selenium/patrons_search.t t/db_dependent/selenium/patrons_search.t .. 1/2 # Failed test 'there were no extended attributes on the search clause' # at t/db_dependent/selenium/patrons_search.t line 878. # got: '1' # expected: '0' # Looks like you failed 1 test of 2. # Failed test 'no clause for extended_attributes when none are default searchable' # at t/db_dependent/selenium/patrons_search.t line 880. # Looks like you failed 1 test of 3. t/db_dependent/selenium/patrons_search.t .. 2/2 # Failed test 'Search patrons in modal' # at t/db_dependent/selenium/patrons_search.t line 883. # Looks like you failed 1 test of 2. t/db_dependent/selenium/patrons_search.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/2 subtests Test Summary Report ------------------- t/db_dependent/selenium/patrons_search.t (Wstat: 256 (exited 1) Tests: 2 Failed: 1) Failed test: 2 Non-zero exit status: 1 Files=1, Tests=2, 132 wallclock secs ( 0.03 usr 0.00 sys + 5.44 cusr 2.11 csys = 7.58 CPU) Result: FAIL [2] Testing notes (using KTD): 1. Start up KTD with Selenium: ktd --selenium up 2. Create two new patron attributes, without "Searchable" and "Search by default" set: . Administration > Patrons and circulation > Patron attribute types 3. Add some values for two new attributes to a couple of patrons, for example: . Attribute 1: ABC . Attribute 2: XYZ 4. Open the network testing tools using your browser's web developer tools. In Firefox, Menu > More tools > Web Developer Tools > Network. 5. Go to Patrons and do a search that returns multiple patrons, for example search for 'a'. 6. In the network tab, look for the <staffinterfaceurl>:8081/api/v1/patrons?_page=1.... request and note that there is a "extended_attributes.code" as part of the URL. 7. Apply the patch and restart everything (restart_all). 8. Update one of the attributes from step 3 so that "Searchable" and "Search by default" are set. 9. Repeat steps 5 to 6 and note that request now includes the "extended_attributes.code". 10. Search for "ABC" and the results should include the two patrons you set this value for. 11. Update the other attribute from step 3 so that "Searchable" and "Search by default" are set. 12. Search for "XYZ" and the results should include the two patrons you set this value for. 13. Check that the tests pass: . New Cypress tests: yarn cypress run --spec t/cypress/integration/PatronSearch_spec.ts . Selenium tests: prove t/db_dependent/selenium/patrons_search.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #25 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to David Nind from comment #23)

I've attempted to test - everything works except the Selenium tests. They don't pass for me.[1]

[1] Results from running the Selenium tests:

prove t/db_dependent/selenium/patrons_search.t t/db_dependent/selenium/patrons_search.t .. 1/2 # Failed test 'there were no extended attributes on the search clause' # at t/db_dependent/selenium/patrons_search.t line 878. # got: '1' # expected: '0' # Looks like you failed 1 test of 2.

# Failed test 'no clause for extended_attributes when none are default searchable' # at t/db_dependent/selenium/patrons_search.t line 880. # Looks like you failed 1 test of 3. t/db_dependent/selenium/patrons_search.t .. 2/2 # Failed test 'Search patrons in modal' # at t/db_dependent/selenium/patrons_search.t line 883. # Looks like you failed 1 test of 2. t/db_dependent/selenium/patrons_search.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/2 subtests

The selenium tests would fail when there are unexpected attributes defined in the DB - I obsoleted that patch in favor of Joubu's Cypress tests - if they worked for you could you add your SO? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176748|0 |1 is obsolete| | --- Comment #26 from Magnus Enger <magnus@libriotech.no> --- Created attachment 178037 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178037&action=edit Bug 36025: Incorrect clause in patron search This patch removes the search clause for extended_attributes when there is no patron attribute types which are searchable by default. The clause was incorrectly added because the array extended_attribute_types was rendered as [""] This seems to be an issue with get_columns from DBIX Class and how that is cast when forcing an array in the template. It returns an empty string for nothing, a scalar for a single value, and an array for multiple values. This patch now checks if the variable is set and has content, and wraps as an array if so. In the case of two attributes, we do double the array, but this was true before as we always cast the return as an array. Test plan: * Make sure that no borrower attribute type is set to "searched by default" in Koha administration -> Patron attribute types. * Enable the network monitring in the developer tools in your web browser (Ctrl + Shift + E in firefox or Ctrl + Shift + I and select the "Network" tab). * Go to the patrons view members-home.pl (click patrons in top menu bar). * Search for a string that will match multiple borrowers (to not be automatically redirected) * Examine the ajax-request and make sure that "extended_attributes.code" is NOT part of the query. * Also, in koha-testing-docker with selenium enabled, run 'perl t/db_dependent/selenium/patrons_search.t' * Now set one patro attribute to searchable and searched by default and add a value for a patron, confirm search works * Set a second patron attribute to searchable and searched by default and confirm searching works Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Magnus Enger <magnus@libriotech.no> Works as advertised. Did not run the selenium tests, as they have been obsoleted. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176824|0 |1 is obsolete| | --- Comment #28 from Magnus Enger <magnus@libriotech.no> --- Created attachment 178039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178039&action=edit Bug 36025: Fix failure when pref is off Prevent JS error if ExtendedPatronAttributes is off "Uncaught TypeError: options.extended_attribute_types is undefined" Signed-off-by: Magnus Enger <magnus@libriotech.no> Makes the Cypress tests pass. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178037|0 |1 is obsolete| | --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 178081 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178081&action=edit Bug 36025: Incorrect clause in patron search This patch removes the search clause for extended_attributes when there is no patron attribute types which are searchable by default. The clause was incorrectly added because the array extended_attribute_types was rendered as [""] This seems to be an issue with get_columns from DBIX Class and how that is cast when forcing an array in the template. It returns an empty string for nothing, a scalar for a single value, and an array for multiple values. This patch now checks if the variable is set and has content, and wraps as an array if so. In the case of two attributes, we do double the array, but this was true before as we always cast the return as an array. Test plan: * Make sure that no borrower attribute type is set to "searched by default" in Koha administration -> Patron attribute types. * Enable the network monitring in the developer tools in your web browser (Ctrl + Shift + E in firefox or Ctrl + Shift + I and select the "Network" tab). * Go to the patrons view members-home.pl (click patrons in top menu bar). * Search for a string that will match multiple borrowers (to not be automatically redirected) * Examine the ajax-request and make sure that "extended_attributes.code" is NOT part of the query. * Also, in koha-testing-docker with selenium enabled, run 'perl t/db_dependent/selenium/patrons_search.t' * Now set one patro attribute to searchable and searched by default and add a value for a patron, confirm search works * Set a second patron attribute to searchable and searched by default and confirm searching works Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Magnus Enger <magnus@libriotech.no> Works as advertised. Did not run the selenium tests, as they have been obsoleted. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178039|0 |1 is obsolete| | --- Comment #31 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 178083 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178083&action=edit Bug 36025: Fix failure when pref is off Prevent JS error if ExtendedPatronAttributes is off "Uncaught TypeError: options.extended_attribute_types is undefined" Signed-off-by: Magnus Enger <magnus@libriotech.no> Makes the Cypress tests pass. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #32 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- FAIL t/cypress/support/e2e.js FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl t/cypress/support/e2e.js` I ran the tidy and merged the changes into the second patch. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Steve, OSLRI, USA <sspohn@oslri.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bywater@oslri.net, | |sspohn@oslri.net -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #35 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 24.11.x for 24.11.03 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Rebecca Coert <rcoert@arlingtonva.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rcoert@arlingtonva.us -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 ayoung <ayoung@oslri.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ayoung@oslri.net --- Comment #36 from ayoung <ayoung@oslri.net> --- Looks like this was removed from 24.11.03, according to the 24.11.03 release notes (however the 25.05 release notes say that it was applied in 24.11.03). Is this in 24.11.03 or not? Thanks! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 --- Comment #38 from Paul Derscheid <paul.derscheid@lmscloud.de> --- This was applied, then reverted, then applied again. Sorry for the confusion. But I can say that it is part of 24.11. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36025 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Needs documenting CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #40 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11.x as it wasn't backported to 24.05.x (or 23.XX) -- You are receiving this mail because: You are watching all bug changes.