Bug 38413 - Batch operations from item search results fail when "select visible rows" and many items are selected
Summary: Batch operations from item search results fail when "select visible rows" and...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P3 major
Assignee: Phil Ringnalda
QA Contact: Emily Lamancusa (emlam)
URL:
Keywords:
Depends on: 36630
Blocks:
  Show dependency treegraph
 
Reported: 2024-11-08 19:19 UTC by Kristi Krueger
Modified: 2024-12-13 21:24 UTC (History)
6 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes an Apache web server error ("Request-URI Too Long - The requested URL's length exceeds the capacity limit for this server.") when using item search and batch modification to edit many items (500+).
Version(s) released in:
24.11.00,24.05.06
Circulation function:


Attachments
Bug 38413: Batch operations from item search need to POST to not exceed URL length limits (3.14 KB, patch)
2024-11-09 01:07 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 38413: Batch operations from item search need to POST to not exceed URL length limits (3.18 KB, patch)
2024-11-10 18:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 38413: Batch operations from item search need to POST to not exceed URL length limits (3.25 KB, patch)
2024-11-12 19:14 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kristi Krueger 2024-11-08 19:19:11 UTC
To replicate: 
1. Go to item search. 
2. Enter parameters that retrieve more than 500 items.
3. From the item search results page, show All entries, then "Select visible rows"
4. Select either of the batch operations (Batch item modification or batch item deletion)
5. A resulting error message of either "This site can't be reached. The connection was reset" or a URL with all the item numbers inserted appears.
Comment 1 Phil Ringnalda 2024-11-09 00:36:16 UTC
That's because bug 36630 changed it from a POST to a GET, which runs the risk of exceeding the maximum URL length.

It was fooled into making the wrong change in the wrong place by a dummy op. The way both batch modification and batch deletion work in item search is by completely emptying out the content of the form with id="batch_item_operations", sticking in an op and inputs for every selected item, and submitting that. The bulk CSRF-proctection changes were fooled by the dummy initial content of the form, so they changed the op from show to cud-show and added a CSRF token, but the JavaScript that actually prepares and submits the form just went right along deleting all of that, then adding back op="show" and not adding a CSRF token.

The fix is to reverse the changes bug 36630 made so the form goes back to a POST, stick in an HTML comment above the dummy contents saying that they are dummy contents that will be replaced by JavaScript, and then change the op in https://git.koha-community.org/Koha-community/Koha/src/commit/c10c244a289b2c1968b52d8158cedff4feae5e0e/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt#L785 to cud-show and add a CSRF token input.
Comment 2 Phil Ringnalda 2024-11-09 01:07:51 UTC
Created attachment 174311 [details] [review]
Bug 38413: Batch operations from item search need to POST to not exceed URL length limits

Batch item modification and deletion accepts either op="show" or op="cud-show"
because sometimes you just have one item and it's saner to GET with one, but
sometimes you've selected 961 items from itemsearch and you need to POST to
not get hit by Apache's "too long" error.

Bug 36630 switched item search from using POST to GET because item search is
confusing. It uses a form with a dummy op and then empties out the form and
adds back contents, and the CSRF protection changed the dummy op to "cud-show"
so it could POST, but didn't notice that the op would be replaced by one that
was still "show." The form needs to go back to POST, the dummy contents need a
comment saying they are just dummies so nobody else gets fooled, and the JS
that replaces the form contents needs to insert a CSRF token and the correct op.

Test plan:
 1. Without the patch, Search - Item search, search with no criteria so you
    get hundreds of items
 2. Change to Show All entries, then Select visible rows
 3. Batch operations - Batch item modification
 4. Apache should give you an error about your overly long URL
 5. Apply patch and repeat steps 1-3
 6. After a long pause while Batch item modification swallows the POST, you
    should get the batch modification form with all your items.

Sponsored-by: Chetco Community Public Library
Comment 3 David Nind 2024-11-10 18:35:22 UTC
Created attachment 174315 [details] [review]
Bug 38413: Batch operations from item search need to POST to not exceed URL length limits

Batch item modification and deletion accepts either op="show" or op="cud-show"
because sometimes you just have one item and it's saner to GET with one, but
sometimes you've selected 961 items from itemsearch and you need to POST to
not get hit by Apache's "too long" error.

Bug 36630 switched item search from using POST to GET because item search is
confusing. It uses a form with a dummy op and then empties out the form and
adds back contents, and the CSRF protection changed the dummy op to "cud-show"
so it could POST, but didn't notice that the op would be replaced by one that
was still "show." The form needs to go back to POST, the dummy contents need a
comment saying they are just dummies so nobody else gets fooled, and the JS
that replaces the form contents needs to insert a CSRF token and the correct op.

Test plan:
 1. Without the patch, Search - Item search, search with no criteria so you
    get hundreds of items
 2. Change to Show All entries, then Select visible rows
 3. Batch operations - Batch item modification
 4. Apache should give you an error about your overly long URL
 5. Apply patch and repeat steps 1-3
 6. After a long pause while Batch item modification swallows the POST, you
    should get the batch modification form with all your items.

Sponsored-by: Chetco Community Public Library
Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Emily Lamancusa (emlam) 2024-11-12 19:14:20 UTC
Created attachment 174437 [details] [review]
Bug 38413: Batch operations from item search need to POST to not exceed URL length limits

Batch item modification and deletion accepts either op="show" or op="cud-show"
because sometimes you just have one item and it's saner to GET with one, but
sometimes you've selected 961 items from itemsearch and you need to POST to
not get hit by Apache's "too long" error.

Bug 36630 switched item search from using POST to GET because item search is
confusing. It uses a form with a dummy op and then empties out the form and
adds back contents, and the CSRF protection changed the dummy op to "cud-show"
so it could POST, but didn't notice that the op would be replaced by one that
was still "show." The form needs to go back to POST, the dummy contents need a
comment saying they are just dummies so nobody else gets fooled, and the JS
that replaces the form contents needs to insert a CSRF token and the correct op.

Test plan:
 1. Without the patch, Search - Item search, search with no criteria so you
    get hundreds of items
 2. Change to Show All entries, then Select visible rows
 3. Batch operations - Batch item modification
 4. Apache should give you an error about your overly long URL
 5. Apply patch and repeat steps 1-3
 6. After a long pause while Batch item modification swallows the POST, you
    should get the batch modification form with all your items.

Sponsored-by: Chetco Community Public Library
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 5 Katrin Fischer 2024-11-18 16:08:47 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 6 Lucas Gass (lukeg) 2024-11-19 16:39:22 UTC
Backported to 24.05.x for upcoming 24.05.06
Comment 7 Fridolin Somers 2024-12-13 14:24:22 UTC
Depends on Bug 36630 not in 23.11.x
Comment 8 David Nind 2024-12-13 21:24:42 UTC
This is a bug fix and no UI changes, so no changes to the documentation required.