Bug 38155 - Can't close invoices using checkboxes from invoices.pl
Summary: Can't close invoices using checkboxes from invoices.pl
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Brendan Lawlor
QA Contact: Emily Lamancusa (emlam)
URL:
Keywords: regression
Depends on: 36246
Blocks: 38766 36192
  Show dependency treegraph
 
Reported: 2024-10-11 18:38 UTC by Sara Brown
Modified: 2025-02-07 18:08 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes closing and reopening of invoices (Acquisitions > [Vendor] > Invoices). Previously, the invoices you selected weren't closed or reopened when clicking on the "Close/Reopen selected invoices" button - all that happened was that one of the selected invoices was displayed instead. (This is related to the CSRF changes added in Koha 24.05 to improve form security.)
Version(s) released in:
25.05.00,24.11.02
Circulation function:


Attachments
Bug 38155: Fix Acquistions close selected invoices button (3.60 KB, patch)
2024-12-18 16:25 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 38155: Fix Acquistions close selected invoices button (5.64 KB, patch)
2024-12-18 21:38 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 38155: Fix Acquisitions Reopen selected invoices button (4.89 KB, patch)
2024-12-18 21:38 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 38155: Fix Acquistions close selected invoices button (5.71 KB, patch)
2024-12-19 15:28 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 38155: Fix Acquisitions Reopen selected invoices button (4.96 KB, patch)
2024-12-19 15:28 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 38155: Fix Acquistions close selected invoices button (5.78 KB, patch)
2024-12-20 15:04 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 38155: Fix Acquisitions Reopen selected invoices button (5.03 KB, patch)
2024-12-20 15:04 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 38155: (QA follow-up) Fix TT filters and scalar context (5.95 KB, patch)
2024-12-20 15:04 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 Sara Brown 2024-10-11 18:38:39 UTC
Closing one or more invoices by using checkboxes at /acqui/invoices.pl isn't working on 24.05.03. To replicate:

-In Acquisitions > Invoices, search for invoices using available filters or go go to a vendor's record and select Invoices
-Select checkboxes for one or more open invoices, and click the 'Close selected invoices' button
-Instead of closing the invoice(s), you're brought to one of the invoices you selected (not closed, and 'Close' checkbox isn't selected)
Comment 1 Joe Askins 2024-10-15 20:45:51 UTC
Fayetteville Public Library noticed this issue shortly after our most recent upgrade. We would very much like this button's old functionality to return, as it's nice to be able to bulk-close invoices. We look forward to seeing this addressed!
Comment 2 Brendan Lawlor 2024-12-18 16:25:53 UTC
Created attachment 175658 [details] [review]
Bug 38155: Fix Acquistions close selected invoices button

This patch updates the data-op property of the button to be prefixed with cud-

To test:
1. Add some invoices for a vendor
2. Got to Acquisitions->Invoices
3. Select multiple invoices using the checkbox
4. Click the 'Close selected invoices' button
5. Confirm it worked
Comment 4 Brendan Lawlor 2024-12-18 17:13:01 UTC
(In reply to David Nind from comment #3)
> Hi Brandan.
> 
> I'm getting this error after the patch when I go to close the invoices:
> 
> An error has occurred!
> Error 403
> Programming error - op 'cud-close' must not start with 'cud-' for GET


This is hard for me to test in ktd, do you have the steps to create invoices handy?

I think for the link to work it needs some more attributes, probably at least data-method="post"
Comment 5 David Nind 2024-12-18 17:46:03 UTC
(In reply to Brendan Lawlor from comment #4)
> (In reply to David Nind from comment #3)
> > Hi Brandan.
> > 
> > I'm getting this error after the patch when I go to close the invoices:
> > 
> > An error has occurred!
> > Error 403
> > Programming error - op 'cud-close' must not start with 'cud-' for GET
> 
> 
> This is hard for me to test in ktd, do you have the steps to create invoices
> handy?
> 
> I think for the link to work it needs some more attributes, probably at
> least data-method="post"

I followed the test plan 8-)

Steps to reproduce:
1. Apply the patch, restart everything (restart_all), clear your browser cache.
2. Acquisitions > go to a vendor page (click Search in the search vendors section).
3. Receive shipments
4. Create several invoices from here.
   (Even though the invoices have no data, I did check whether that was the issue by adding orders to a basket, closing the basket, and then receiving them properly, but I still got the same error.)
5. Acquisitions > Invoices > Search (no filter information entered).
6. Select a couple of the open invoices (tick boxes).
7. Click "Close selected invoices".
8. Error!
Comment 6 Brendan Lawlor 2024-12-18 21:38:34 UTC
Created attachment 175682 [details] [review]
Bug 38155: Fix Acquistions close selected invoices button

This patch updates the closed selected invoices link button to use
the form-submit.js method from bug 36246. The link needs to have a single
data-invoiceid parameter, so the js on invoices.tt creates a comma
separated list from the checkboxes.

The list of invoiceid is split into an array in invoice.pl

To test:
1. Add some invoices for a vendor using the 'receive shipments' button
2. Go to Acquisitions->Invoices
3. Select multiple invoices using the checkbox
4. Click the 'Close selected invoices' button
5. Confirm it works
6. Confirm closing individual invoices from the Actions button still works
Comment 7 Brendan Lawlor 2024-12-18 21:38:36 UTC
Created attachment 175683 [details] [review]
Bug 38155: Fix Acquisitions Reopen selected invoices button

This patch updates the reopen selected invoices link button to use
the form-submit.js method from bug 36246. The link needs to have a single
data-invoiceid parameter, so the js on invoices.tt creates a comma
separated list from the checkboxes.

The list of invoiceid is split into an array in invoice.pl

To test:
1. Add some invoices for a vendor using the 'receive shipments' button
2. Go to Acquisitions->Invoices
3. Select multiple invoices using the checkboxes
4. Click the 'Close selected invoices' button
5. Confirm it works
6. Go to the Closed invoices tab
7. Select multiple invoices using the checkboxes
8. Click the 'Reopen selected invoices' button
9. Confirm it works
6. Confirm reopening individual invoices from the Actions button still works
Comment 8 Andrew Fuerste-Henry 2024-12-19 15:28:10 UTC
Created attachment 175777 [details] [review]
Bug 38155: Fix Acquistions close selected invoices button

This patch updates the closed selected invoices link button to use
the form-submit.js method from bug 36246. The link needs to have a single
data-invoiceid parameter, so the js on invoices.tt creates a comma
separated list from the checkboxes.

The list of invoiceid is split into an array in invoice.pl

To test:
1. Add some invoices for a vendor using the 'receive shipments' button
2. Go to Acquisitions->Invoices
3. Select multiple invoices using the checkbox
4. Click the 'Close selected invoices' button
5. Confirm it works
6. Confirm closing individual invoices from the Actions button still works

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Comment 9 Andrew Fuerste-Henry 2024-12-19 15:28:12 UTC
Created attachment 175778 [details] [review]
Bug 38155: Fix Acquisitions Reopen selected invoices button

This patch updates the reopen selected invoices link button to use
the form-submit.js method from bug 36246. The link needs to have a single
data-invoiceid parameter, so the js on invoices.tt creates a comma
separated list from the checkboxes.

The list of invoiceid is split into an array in invoice.pl

To test:
1. Add some invoices for a vendor using the 'receive shipments' button
2. Go to Acquisitions->Invoices
3. Select multiple invoices using the checkboxes
4. Click the 'Close selected invoices' button
5. Confirm it works
6. Go to the Closed invoices tab
7. Select multiple invoices using the checkboxes
8. Click the 'Reopen selected invoices' button
9. Confirm it works
6. Confirm reopening individual invoices from the Actions button still works

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Comment 10 Emily Lamancusa (emlam) 2024-12-20 15:04:28 UTC
Created attachment 175846 [details] [review]
Bug 38155: Fix Acquistions close selected invoices button

This patch updates the closed selected invoices link button to use
the form-submit.js method from bug 36246. The link needs to have a single
data-invoiceid parameter, so the js on invoices.tt creates a comma
separated list from the checkboxes.

The list of invoiceid is split into an array in invoice.pl

To test:
1. Add some invoices for a vendor using the 'receive shipments' button
2. Go to Acquisitions->Invoices
3. Select multiple invoices using the checkbox
4. Click the 'Close selected invoices' button
5. Confirm it works
6. Confirm closing individual invoices from the Actions button still works

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 11 Emily Lamancusa (emlam) 2024-12-20 15:04:31 UTC
Created attachment 175847 [details] [review]
Bug 38155: Fix Acquisitions Reopen selected invoices button

This patch updates the reopen selected invoices link button to use
the form-submit.js method from bug 36246. The link needs to have a single
data-invoiceid parameter, so the js on invoices.tt creates a comma
separated list from the checkboxes.

The list of invoiceid is split into an array in invoice.pl

To test:
1. Add some invoices for a vendor using the 'receive shipments' button
2. Go to Acquisitions->Invoices
3. Select multiple invoices using the checkboxes
4. Click the 'Close selected invoices' button
5. Confirm it works
6. Go to the Closed invoices tab
7. Select multiple invoices using the checkboxes
8. Click the 'Reopen selected invoices' button
9. Confirm it works
6. Confirm reopening individual invoices from the Actions button still works

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 12 Emily Lamancusa (emlam) 2024-12-20 15:04:33 UTC
Created attachment 175848 [details] [review]
Bug 38155: (QA follow-up) Fix TT filters and scalar context

data-referer is the url for the current search page, to redirect
back after performing the operation. Since it is a url, variables within
this link all need to use the uri filter, not html or KohaDates.

Also specify scalar context when splitting the input parameter to
prevent param in list context warnings.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 13 Emily Lamancusa (emlam) 2024-12-20 15:07:23 UTC
Cleaned up a few things in a follow-up, but otherwise it looks good! Thanks, Brendan! Passing QA
Comment 14 Brendan Lawlor 2024-12-20 16:08:40 UTC
(In reply to Emily Lamancusa (emlam) from comment #13)
> Cleaned up a few things in a follow-up, but otherwise it looks good! Thanks,
> Brendan! Passing QA

> variables within this link all need to use the uri filter, not html or KohaDates.

Thanks for this info. I got this from copying the link formatting from the Actions drop down link (line 109) for close because the data-referer wasn't working because it was url encoded. It needed to be encoded before because it used window.location.href for the redirect.

Should I file a new bug to change the filters in those Actions links to use uri instead of html?

> Also specify scalar context when splitting the input parameter 
I need to read up on this and learn more Perl :)

Thanks, Emily!
Comment 15 Emily Lamancusa (emlam) 2024-12-20 16:47:38 UTC
(In reply to Brendan Lawlor from comment #14)
> (In reply to Emily Lamancusa (emlam) from comment #13)

> > variables within this link all need to use the uri filter, not html or KohaDates.
> 
> Thanks for this info. I got this from copying the link formatting from the
> Actions drop down link (line 109) for close because the data-referer wasn't
> working because it was url encoded. It needed to be encoded before because
> it used window.location.href for the redirect.
>
> Should I file a new bug to change the filters in those Actions links to use
> uri instead of html?

Ah...yes, those are wrong too, though I think | html and | uri give the same output if the data doesn't have any special characters? More importantly, use uri instead of $KohaDates in the referer link - otherwise, the redirect after performing the operation will break if the original search included dates. For example, you'll get an internal server error if you do the following without my follow-up:
1. Add some vendor invoices
2. Go to Acquisitions > Invoices
3. Specify search filters for shipment to/from dates (that match at least some of your invoices) and search for invoices
4. Select some invoices and click "Close selected invoices"
--> Internal server error! "The given date <date> does not match the date format (iso)"

There seem to be a lot of places in invoices.tt that are passing locally-formatted dates as parameters, but they've been that way for years...I wonder if the processing changed at some point? Or have the internal server errors just gone quietly unnoticed until now? =/

> > Also specify scalar context when splitting the input parameter 
> I need to read up on this and learn more Perl :)
> 
> Thanks, Emily!

:) Thank you too!
Comment 16 Emily Lamancusa (emlam) 2024-12-20 16:49:39 UTC
Also full disclosure, it helps to have a general understanding of what scalar context vs list context means in Perl...but mostly I caught that last bit because I found bug 29771 while checking if we had precedent for sending multiple values as a single comma-separated parameter.
Comment 17 Katrin Fischer 2024-12-23 09:37:18 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 18 mteal 2025-01-14 15:08:36 UTC
It would be great if this could be backported to 24.11. Closing invoices individually takes significantly longer.
Comment 19 Joe Askins 2025-01-14 15:24:44 UTC
Seconding Greenville's desire to see this backported. This loss of functionality negatively affected our workflow, so we'd love to get it back ASAP. Thanks!
Comment 20 Paul Derscheid 2025-02-04 21:54:36 UTC
Nice work everyone!

Pushed to 24.11.x for 24.11.02