Bug 37197

Summary: Batch patron modification from reports fails by using GET instead of POST
Product: Koha Reporter: Phil Ringnalda <phil>
Component: ReportsAssignee: Phil Ringnalda <phil>
Status: Failed QA --- QA Contact: Matt Blenkinsop <matt.blenkinsop>
Severity: major    
Priority: P5 - low CC: jonathan.druart, matt.blenkinsop
Version: MainKeywords: regression
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 36192    
Bug Blocks: 37188    
Attachments: Bug 37197: Reports option to send to batch patron modification needs to use POST
Bug 37197: Reports option to send to batch patron modification needs to use POST
Bug 37197: Reports option to send to batch patron modification needs to use POST

Description Phil Ringnalda 2024-06-26 14:21:24 UTC
When you SELECT cardnumber FROM borrowers you get a Batch operations menuitem to send the results to Patron batch modification, but because of what looks like it might be an accidental change in https://git.koha-community.org/Koha-community/Koha/commit/8a20e0cb6a0efd6b993e361d439dbb4486ba15d1 they are sent in a GET with the op show (which no longer exists) rather than a POST with cud-show.
Comment 1 Phil Ringnalda 2024-06-27 03:33:38 UTC
Created attachment 168187 [details] [review]
Bug 37197: Reports option to send to batch patron modification needs to use POST

When you have an SQL report that selects cardnumber from borrowers, you
get a menuitem to send the results to Batch patron modification. Currently
that fails with a message about no card numbers or borrowernumbers given,
because it tries to do a GET with the op show, when modborrowers.pl is
expecting a POST with the op cud-show.

Test plan:
1. Without the patch, Reports - Create from SQL - name it and paste

select cardnumber from borrowers

   in the SQL textarea and save, then Run report.
2. Above the results is a "Batch operations with 20 visible records" menu,
   choose Batch patron modification
3. Note that instead of modifying the records, you're stuck with a message
   saying "No patron card numbers or borrowernumbers given." even though
   there's a whole string of them in the URL.
4. Apply patch, refresh the page with the results (oddly, no apparent need
   to even restart_all), and choose the menuitem again, but this time with
   a successful result.
Comment 2 David Nind 2024-06-28 18:40:36 UTC
Created attachment 168287 [details] [review]
Bug 37197: Reports option to send to batch patron modification needs to use POST

When you have an SQL report that selects cardnumber from borrowers, you
get a menuitem to send the results to Batch patron modification. Currently
that fails with a message about no card numbers or borrowernumbers given,
because it tries to do a GET with the op show, when modborrowers.pl is
expecting a POST with the op cud-show.

Test plan:
1. Without the patch, Reports - Create from SQL - name it and paste

select cardnumber from borrowers

   in the SQL textarea and save, then Run report.
2. Above the results is a "Batch operations with 20 visible records" menu,
   choose Batch patron modification
3. Note that instead of modifying the records, you're stuck with a message
   saying "No patron card numbers or borrowernumbers given." even though
   there's a whole string of them in the URL.
4. Apply patch, refresh the page with the results (oddly, no apparent need
   to even restart_all), and choose the menuitem again, but this time with
   a successful result.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Matt Blenkinsop 2024-07-01 15:38:25 UTC
Created attachment 168322 [details] [review]
Bug 37197: Reports option to send to batch patron modification needs to use POST

When you have an SQL report that selects cardnumber from borrowers, you
get a menuitem to send the results to Batch patron modification. Currently
that fails with a message about no card numbers or borrowernumbers given,
because it tries to do a GET with the op show, when modborrowers.pl is
expecting a POST with the op cud-show.

Test plan:
1. Without the patch, Reports - Create from SQL - name it and paste

select cardnumber from borrowers

   in the SQL textarea and save, then Run report.
2. Above the results is a "Batch operations with 20 visible records" menu,
   choose Batch patron modification
3. Note that instead of modifying the records, you're stuck with a message
   saying "No patron card numbers or borrowernumbers given." even though
   there's a whole string of them in the URL.
4. Apply patch, refresh the page with the results (oddly, no apparent need
   to even restart_all), and choose the menuitem again, but this time with
   a successful result.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 4 Katrin Fischer 2024-07-01 16:11:02 UTC
I am wondering if POST is the correct one here as we are not changing data but sending them to display in another tool. Why not GET and op=show?
Comment 5 Phil Ringnalda 2024-07-01 17:04:11 UTC
My bib numbers are seven digits, so &b=nnnnnnn is ten digits, and a report showing 1000 rows per page would be a query string of 10,000 characters. Well, 9,999 since the first one wouldn't have an ampersand.

I don't have any real feeling about what a true current answer to "what is the safe maximum size of a URL" would actually be, but there certainly are a lot of answers a lot smaller than that.