[Bug 28782] New: Get rid of custom query param list creation for request.pl
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Bug ID: 28782 Summary: Get rid of custom query param list creation for request.pl Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: joonas.kylmala@helsinki.fi QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com This bug is opened to simplify the query param handling of reserve.pl which currently supports passing the reservable biblionumber using two different query param variables: biblionumber=XXXX biblionumbers=XXXX/YYYY/ZZZZ Almost all our other scripts use the query param duplication and ->multi_param() instead of custom list creation. -- 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=28782 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch Status|NEW |Needs Signoff -- 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=28782 --- Comment #1 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 123286 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123286&action=edit Bug 28782: Use query param list instead of splitting elements using '/' This removes the need to handle single and multiple cases separately, thus removing bunch if-else cases and simplifying our code. This coding style is also in line with our other .pl scripts. To test: 1) Make sure placing a hold still works from the following pages: /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX /cgi-bin/koha/catalogue/search.pl?q=a /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron there and through the clubs.pl create a hold to a bib) -- 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=28782 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123286|0 |1 is obsolete| | --- Comment #2 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 123287 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123287&action=edit Bug 28782: Use query param list instead of splitting elements using '/' This removes the need to handle single and multiple cases separately, thus removing bunch if-else cases and simplifying our code. This coding style is also in line with our other .pl scripts. To test: 1) Make sure placing a hold still works from the following pages: /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX /cgi-bin/koha/catalogue/search.pl?q=a /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron there and through the clubs.pl create a hold to a bib) -- 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=28782 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #3 from Owen Leonard <oleonard@myacpl.org> --- Everything tests well except for the cart. I found that I can't place holds because of a JavaScript error: Uncaught TypeError: bib_params.size is not a function placeHold http://127.0.0.1:8081/intranet-tmpl/prog/js/cart_21.0600010.js:16 -- 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=28782 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- 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=28782 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123287|0 |1 is obsolete| | --- Comment #4 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 123313 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123313&action=edit Bug 28782: Use query param list instead of splitting elements using '/' This removes the need to handle single and multiple cases separately, thus removing bunch if-else cases and simplifying our code. This coding style is also in line with our other .pl scripts. To test: 1) Make sure placing a hold still works from the following pages: /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX /cgi-bin/koha/catalogue/search.pl?q=a /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron there and through the clubs.pl create a hold to a bib) -- 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=28782 --- Comment #5 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Owen Leonard from comment #3)
Everything tests well except for the cart. I found that I can't place holds because of a JavaScript error:
Uncaught TypeError: bib_params.size is not a function placeHold http://127.0.0.1:8081/intranet-tmpl/prog/js/cart_21.0600010.js:16
Thanks for spotting, I was using the jQuery size() function instead of native JS length function accidentally. That's now fixed. -- 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=28782 Hayley Pelham <hayleypelham@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |hayleypelham@catalyst.net.n | |z --- Comment #6 from Hayley Pelham <hayleypelham@catalyst.net.nz> --- Patch doesn't apply -- 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=28782 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- 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=28782 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123313|0 |1 is obsolete| | --- Comment #7 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 124510 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124510&action=edit Bug 28782: Use query param list instead of splitting elements using '/' This removes the need to handle single and multiple cases separately, thus removing bunch if-else cases and simplifying our code. This coding style is also in line with our other .pl scripts. To test: 1) Make sure placing a hold still works from the following pages: /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX /cgi-bin/koha/catalogue/search.pl?q=a /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron there and through the clubs.pl create a hold to a bib) -- 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=28782 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joonas.kylmala@iki.fi --- Comment #8 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- (In reply to Hayley Pelham from comment #6)
Patch doesn't apply
Rebased, ready for testing. -- 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=28782 Hayley Pelham <hayleypelham@catalyst.net.nz> 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=28782 Hayley Pelham <hayleypelham@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124510|0 |1 is obsolete| | --- Comment #9 from Hayley Pelham <hayleypelham@catalyst.net.nz> --- Created attachment 124521 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124521&action=edit Bug 28782: Use query param list instead of splitting elements using '/' This removes the need to handle single and multiple cases separately, thus removing bunch if-else cases and simplifying our code. This coding style is also in line with our other .pl scripts. To test: 1) Make sure placing a hold still works from the following pages: /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX /cgi-bin/koha/catalogue/search.pl?q=a /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron there and through the clubs.pl create a hold to a bib) Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz> -- 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=28782 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |joonas.kylmala@helsinki.fi |ity.org | CC| |m.de.rooy@rijksmuseum.nl --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Adjusted assignee. Good habit to fill in ;) -- 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=28782 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Should this be an enhancement? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|minor |enhancement Assignee|joonas.kylmala@helsinki.fi |joonas.kylmala@iki.fi -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29257 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I really like the idea of this, but I wonder if we should use biblionumber= for the URL parameter or something a little shorter? I seem to remember that we had issues with extra long URLs at some point, but couldn't pin it down. I looked it up and apparently the max size of an URL is 2048 characters? So I tried this: 1) On a standard Koha sample database, activate numSearchResultsDropdown 2) Search in the catalog for 'e' 3) Select them all for placing a hold 4) Look up Henry (or another patron) to place the holds for 5) Place the holds 6) You are redirected to: http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=14410755=... ... and the display will look a bit broken. I wonder how this relates to bug 29257? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #13 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 127112 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127112&action=edit Bug 28782: (follow-up) Pass real array to query_form() It excepts to receive a real array and not a list, in order to be able to generate the query parameters. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #14 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- (In reply to Katrin Fischer from comment #12)
I really like the idea of this, but I wonder if we should use biblionumber= for the URL parameter or something a little shorter?
It's what we use basically everywhere in koha for biblionumber so I think it would be preferable to keep it that way.
I seem to remember that we had issues with extra long URLs at some point, but couldn't pin it down. I looked it up and apparently the max size of an URL is 2048 characters?
Yeah, it seems to be a limitation of some browsers. If we wanted to support browsers that have an URL length limitation then we should start an effort to fix all places in Koha where we generate URLs with arbitrary length. Realistically though, here we are not gonna run into that limitation easily, you would have to make a hold for over 70 biblios (even if your bibnums were in the millions) to reach the limit.
... and the display will look a bit broken.
Thanks, fixed in the follow-up!
I wonder how this relates to bug 29257?
That would be not needed if this is pushed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124521|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 127208 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127208&action=edit Bug 28782: Use query param list instead of splitting elements using '/' This removes the need to handle single and multiple cases separately, thus removing bunch if-else cases and simplifying our code. This coding style is also in line with our other .pl scripts. To test: 1) Make sure placing a hold still works from the following pages: /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX /cgi-bin/koha/catalogue/search.pl?q=a /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron there and through the clubs.pl create a hold to a bib) Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127112|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 127209 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127209&action=edit Bug 28782: (follow-up) Pass real array to query_form() It excepts to receive a real array and not a list, in order to be able to generate the query parameters. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Signed Off |Passed QA --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice improvement, no regressions found.. QA scripts happy. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |jonathan.druart+koha@gmail. | |com --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 1. This one seems wrong: koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt: link = `request.pl?biblionumbers=[% biblionumbers | url %]&action=cancelBulk&ids=${$('.holds_table .select_hold:checked').toArray().map(el => $(el).data('id')).join(',')}`; I don't have the time to confirm now but could do later. 2. This one is wrong: koha-tmpl/intranet-tmpl/prog/en/includes/circ-patron-search-results.inc: [% SET data_url = "/cgi-bin/koha/reserve/request.pl?borrowernumber=" _ borrower.borrowernumber _ "&multi_hold=1&biblionumbers=" _ biblionumbers %] to recreate: search, select at least 2 records, place holds, search for patron (which will give you several results), select one => url is /reserve/request.pl?borrowernumber=19&multi_hold=1&biblionumbers=ARRAY(0x55def41723e8) 3. The occurrences of biblionumers in virtualshelves/addbybiblionumber.tt seem wrong but not related (?) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #19 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- (In reply to Jonathan Druart from comment #18)
1. This one seems wrong: koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt: link = `request.pl?biblionumbers=[% biblionumbers | url %]&action=cancelBulk&ids=${$('.holds_table .select_hold:checked').toArray().map(el => $(el).data('id')).join(',')}`;
I don't have the time to confirm now but could do later.
it is wrong, have to re-do the patch, there was some changes pushed to master from Bug 23678 that are not compatible with this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andriiverem@gmail.com --- Comment #20 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- *** Bug 29259 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=28782 --- Comment #21 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Joonas, do you think you will be back at this one soon? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Jonathan Druart <jonathan.druart+koha@gmail.com> 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=28782 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127208|0 |1 is obsolete| | Attachment #127209|0 |1 is obsolete| | --- Comment #22 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129939 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129939&action=edit Bug 28782: Use query param list instead of splitting elements using '/' This removes the need to handle single and multiple cases separately, thus removing bunch if-else cases and simplifying our code. This coding style is also in line with our other .pl scripts. To test: 1) Make sure placing a hold still works from the following pages: /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX /cgi-bin/koha/catalogue/search.pl?q=a /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron there and through the clubs.pl create a hold to a bib) Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #23 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129940 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129940&action=edit Bug 28782: (follow-up) Pass real array to query_form() It excepts to receive a real array and not a list, in order to be able to generate the query parameters. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #24 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129941 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129941&action=edit Bug 28782: Fix link when placing a hold for a patron from patron list -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #25 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129942 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129942&action=edit Bug 28782: Fix cancel holds in bulk See also bug 29969. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29969 Depends on| |29660 --- Comment #26 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Patches rebased on top of bug 29660. And follow-ups submitted. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29660 [Bug 29660] reserve/request.pl should not deal with biblioitem -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #27 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- *** Bug 29257 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=28782 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29136 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29136 [Bug 29136] Patron search on request.pl has performance and display issues -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Owen Leonard <oleonard@myacpl.org> 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=28782 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129941|0 |1 is obsolete| | Attachment #129942|0 |1 is obsolete| | --- Comment #28 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 130663 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130663&action=edit Bug 28782: Fix link when placing a hold for a patron from patron list Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #29 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 130664 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130664&action=edit Bug 28782: Fix cancel holds in bulk See also bug 29969. Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129939|0 |1 is obsolete| | Attachment #129940|0 |1 is obsolete| | Attachment #130663|0 |1 is obsolete| | Attachment #130664|0 |1 is obsolete| | --- Comment #30 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 130868 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130868&action=edit Bug 28782: Use query param list instead of splitting elements using '/' This removes the need to handle single and multiple cases separately, thus removing bunch if-else cases and simplifying our code. This coding style is also in line with our other .pl scripts. To test: 1) Make sure placing a hold still works from the following pages: /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX /cgi-bin/koha/catalogue/search.pl?q=a /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron there and through the clubs.pl create a hold to a bib) Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> 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=28782 --- Comment #31 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 130869 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130869&action=edit Bug 28782: (follow-up) Pass real array to query_form() It excepts to receive a real array and not a list, in order to be able to generate the query parameters. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> 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=28782 --- Comment #32 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 130870 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130870&action=edit Bug 28782: Fix link when placing a hold for a patron from patron list Signed-off-by: Owen Leonard <oleonard@myacpl.org> 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=28782 --- Comment #33 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 130871 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130871&action=edit Bug 28782: Fix cancel holds in bulk See also bug 29969. Signed-off-by: Owen Leonard <oleonard@myacpl.org> 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=28782 --- Comment #34 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 130872 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130872&action=edit Bug 28782: (QA follow-up) Bug 29844 follow-up 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=28782 --- Comment #35 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 130873 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130873&action=edit Bug 28782: (QA follow-up) Fix missing filter 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=28782 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130872|0 |1 is obsolete| | --- Comment #36 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 130894 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130894&action=edit Bug 28782: (QA follow-up) Bug 29844 follow-up 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=28782 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130873|0 |1 is obsolete| | --- Comment #37 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 130895 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130895&action=edit Bug 28782: (QA follow-up) Fix missing filter 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=28782 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130868|0 |1 is obsolete| | Attachment #130869|0 |1 is obsolete| | Attachment #130870|0 |1 is obsolete| | Attachment #130871|0 |1 is obsolete| | Attachment #130894|0 |1 is obsolete| | Attachment #130895|0 |1 is obsolete| | --- Comment #38 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 130938 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130938&action=edit Bug 28782: Use query param list instead of splitting elements using '/' This removes the need to handle single and multiple cases separately, thus removing bunch if-else cases and simplifying our code. This coding style is also in line with our other .pl scripts. To test: 1) Make sure placing a hold still works from the following pages: /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX /cgi-bin/koha/catalogue/search.pl?q=a /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron there and through the clubs.pl create a hold to a bib) Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #39 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 130939 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130939&action=edit Bug 28782: (follow-up) Pass real array to query_form() It excepts to receive a real array and not a list, in order to be able to generate the query parameters. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #40 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 130940 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130940&action=edit Bug 28782: Fix link when placing a hold for a patron from patron list -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #41 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 130941 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130941&action=edit Bug 28782: Fix cancel holds in bulk See also bug 29969. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #42 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 130942 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130942&action=edit Bug 28782: (QA follow-up) Bug 29844 follow-up 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=28782 --- Comment #43 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 130943 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130943&action=edit Bug 28782: (QA follow-up) Fix missing filter 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=28782 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |fridolin.somers@biblibre.co | |m --- Comment #44 from Fridolin Somers <fridolin.somers@biblibre.com> --- There are some problems with filters. I create holds for 2 records and cancel them immediately. Without last patch "Bug 28782: (QA follow-up) Fix missing filter" I see URL : /cgi-bin/koha/reserve/request.pl?biblionumber=359&biblionumber=369&action=cancelBulk&ids=2,1 With last patch I see URL : /cgi-bin/koha/reserve/request.pl?biblionumber%3D359%26biblionumber%3D369&action=cancelBulk&ids=2,1 Without all patches we have URL : /cgi-bin/koha/reserve/request.pl?biblionumbers=359/369&action=cancelBulk&ids=2,1 Looks it '&' works, but it should be '&'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Bug 28782 depends on bug 29660, which changed state. Bug 29660 Summary: reserve/request.pl should not deal with biblioitem https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29660 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130938|0 |1 is obsolete| | Attachment #130939|0 |1 is obsolete| | Attachment #130940|0 |1 is obsolete| | Attachment #130941|0 |1 is obsolete| | Attachment #130942|0 |1 is obsolete| | Attachment #130943|0 |1 is obsolete| | --- Comment #45 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 131349 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131349&action=edit Bug 28782: Use query param list instead of splitting elements using '/' This removes the need to handle single and multiple cases separately, thus removing bunch if-else cases and simplifying our code. This coding style is also in line with our other .pl scripts. To test: 1) Make sure placing a hold still works from the following pages: /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX /cgi-bin/koha/catalogue/search.pl?q=a /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron there and through the clubs.pl create a hold to a bib) Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #46 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 131350 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131350&action=edit Bug 28782: (follow-up) Pass real array to query_form() It excepts to receive a real array and not a list, in order to be able to generate the query parameters. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #47 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 131351 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131351&action=edit Bug 28782: Fix link when placing a hold for a patron from patron list -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #48 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 131352 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131352&action=edit Bug 28782: Fix cancel holds in bulk See also bug 29969. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #49 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 131353 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131353&action=edit Bug 28782: (QA follow-up) Bug 29844 follow-up 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=28782 --- Comment #50 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 131354 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131354&action=edit Bug 28782: (QA follow-up) Fix missing filter 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=28782 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA --- Comment #51 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Fridolin Somers from comment #44)
There are some problems with filters.
I create holds for 2 records and cancel them immediately. Without last patch "Bug 28782: (QA follow-up) Fix missing filter" I see URL : /cgi-bin/koha/reserve/request.pl?biblionumber=359&biblionumber=369& action=cancelBulk&ids=2,1 With last patch I see URL : /cgi-bin/koha/reserve/request.pl?biblionumber%3D359%26biblionumber%3D369& action=cancelBulk&ids=2,1
Without all patches we have URL : /cgi-bin/koha/reserve/request.pl?biblionumbers=359/369& action=cancelBulk&ids=2,1 Looks it '&' works, but it should be '&'.
Good catch, fixed! I amended the patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #52 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED 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=28782 --- Comment #53 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- This introduces a warning in the log Use of uninitialized value $bad_bibs_param in split at /kohadevbox/koha/reserve/placerequest.pl line 55. We could fix it with: diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 0a8d6dd64e6..e85c6ad292a 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -51,7 +51,7 @@ my $non_priority = $input->param('non_priority'); my $patron = Koha::Patrons->find( $borrowernumber ); -my $bad_bibs_param = $input->param('bad_bibs'); +my $bad_bibs_param = $input->param('bad_bibs') || ''; my @bad_bibs = split '/', $bad_bibs_param; my $holds_to_place_count = $input->param('holds_to_place_count') || 1; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 --- Comment #54 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #53)
This introduces a warning in the log
Use of uninitialized value $bad_bibs_param in split at /kohadevbox/koha/reserve/placerequest.pl line 55.
We could fix it with:
diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 0a8d6dd64e6..e85c6ad292a 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -51,7 +51,7 @@ my $non_priority = $input->param('non_priority');
my $patron = Koha::Patrons->find( $borrowernumber );
-my $bad_bibs_param = $input->param('bad_bibs'); +my $bad_bibs_param = $input->param('bad_bibs') || ''; my @bad_bibs = split '/', $bad_bibs_param; my $holds_to_place_count = $input->param('holds_to_place_count') || 1;
Alternatively, @bad_bibs could be initialised ahead of time, and the split function call could only be done if 'bad_bibs' is passed to the server. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28782 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27301 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org