Bug 34006 - [22.11] Keep the current option for funds in receiving returns an error 500 or saves wrong fund
Summary: [22.11] Keep the current option for funds in receiving returns an error 500 o...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 22.11
Hardware: All All
: P5 - low major (vote)
Assignee: Katrin Fischer
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 31459 32694
Blocks:
  Show dependency treegraph
 
Reported: 2023-06-14 09:22 UTC by Thibaut
Modified: 2023-12-28 20:47 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.07


Attachments
Bug 34006: Fix value of "Keep current" option on receive so correct fund is saved (2.70 KB, patch)
2023-06-16 13:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34006: Fix value of "Keep current" option on receive so correct fund is saved (2.77 KB, patch)
2023-06-22 15:02 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 34006: Fix value of "Keep current" option on receive so correct fund is saved (2.83 KB, patch)
2023-06-22 15:18 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Thibaut 2023-06-14 09:22:49 UTC
On v22.11.04, the 'Keep current' option brings up the budget_period_id instead of the budget_id.

Example:
<option value="120" selected="selected" data-sort1-authcat="" data-sort2-authcat="NIV_PUBLIC">
Keep current (Arts 2023)</option>
Good line in list:
<option value="515" selected="selected" data-sort1-authcat="" data-sort2-authcat="NIV_PUBLIC">
Arts 2023
</option>
Comment 1 Katrin Fischer 2023-06-14 09:47:51 UTC
I believe this had been fixed already once in the 22.11 release cycle, but coudln't locate the bug. Maybe git history will show something.
Comment 2 Katrin Fischer 2023-06-16 13:03:01 UTC
In master things seem to be ok, will try to test in 22.11.x next.
Comment 3 Katrin Fischer 2023-06-16 13:17:23 UTC
Confiremd, on saving the wrong fund is set :(
Comment 4 Katrin Fischer 2023-06-16 13:28:22 UTC
To test:

1)
* With the sample data
* Create a new budget "July 2023"
* Create a new fund "Books" under it
* Limit access to Owner and make yourself the owner

2)
* Create a basket
* Create an order line with this fund
* Close the basket

3)
* Edit or create another patron
* Set catalogue and acq permissions without budget_manage_all and order_manage_all (no access to the fund)
* Edit username and password

4)
* Log in as the second user in a separate browser
* Receive shipment
* Create invoice
* Receive 
* You should see the fund under the "Keep current" heading selected
* Add quantity received, save
* Fund has changed to Fund_1_2 => data loss :(
Comment 5 Katrin Fischer 2023-06-16 13:37:35 UTC
I think the 500 maybe occurs if there is no fund with a matching number.
Comment 6 Katrin Fischer 2023-06-16 13:43:12 UTC
Created attachment 152415 [details] [review]
Bug 34006: Fix value of "Keep current" option on receive so correct fund is saved

Under certain circumstances the "Keep current" option on receive allows
to receive order lines where the fund is otherwise not accessible to the user.
In 22.11 the wrong value is used for the option, so that instead of the fund id
the budget id is used, this can lead to an error 500 if there is no fund with
this id or to a wrong fund being saved.

To test:

1) Create budget and fund
* With the sample data
* Create a new budget "July 2023"
* Create a new fund "Books" under it
* Limit access to Owner and make yourself the owner

2) Create order
* Create a basket
* Create an order line with this access limited fund
* Close the basket

3) Create a staff user with limited permissions
* Edit or create another patron
* Set catalogue and acq permissions without budget_manage_all and order_manage_all (no access to the fund)
* Edit username and password

4) Receive, recreating the error
* Log in as the second user in a separate browser
* Receive shipment
* Create invoice
* Receive
* You should see the fund under the "Keep current" heading selected
* Add quantity received, save
* Fund has changed to Fund_1_2 => data loss :(

5) Receive, verify fix
* Create another order line with your privileged user and the "Books" fund
* Repeat 4)
* After receiving the fund should now remain unchanged
Comment 7 Sam Lau 2023-06-20 15:58:42 UTC
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 34006: Fix value of "Keep current" option on receive so correct fund is saved
Comment 8 Katrin Fischer 2023-06-22 14:18:17 UTC
(In reply to Sam Lau from comment #7)
> CONFLICT (content): Merge conflict in
> koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt
> error: Failed to merge in the changes.
> Patch failed at 0001 Bug 34006: Fix value of "Keep current" option on
> receive so correct fund is saved

Hi Sam, this needs to be tested pre-multi-receive. So in 22.11
Comment 9 Emily Lamancusa 2023-06-22 15:02:56 UTC
Created attachment 152568 [details] [review]
Bug 34006: Fix value of "Keep current" option on receive so correct fund is saved

Under certain circumstances the "Keep current" option on receive allows
to receive order lines where the fund is otherwise not accessible to the user.
In 22.11 the wrong value is used for the option, so that instead of the fund id
the budget id is used, this can lead to an error 500 if there is no fund with
this id or to a wrong fund being saved.

To test:

1) Create budget and fund
* With the sample data
* Create a new budget "July 2023"
* Create a new fund "Books" under it
* Limit access to Owner and make yourself the owner

2) Create order
* Create a basket
* Create an order line with this access limited fund
* Close the basket

3) Create a staff user with limited permissions
* Edit or create another patron
* Set catalogue and acq permissions without budget_manage_all and order_manage_all (no access to the fund)
* Edit username and password

4) Receive, recreating the error
* Log in as the second user in a separate browser
* Receive shipment
* Create invoice
* Receive
* You should see the fund under the "Keep current" heading selected
* Add quantity received, save
* Fund has changed to Fund_1_2 => data loss :(

5) Receive, verify fix
* Create another order line with your privileged user and the "Books" fund
* Repeat 4)
* After receiving the fund should now remain unchanged

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 10 Martin Renvoize 2023-06-22 15:18:58 UTC
Created attachment 152569 [details] [review]
Bug 34006: Fix value of "Keep current" option on receive so correct fund is saved

Under certain circumstances the "Keep current" option on receive allows
to receive order lines where the fund is otherwise not accessible to the user.
In 22.11 the wrong value is used for the option, so that instead of the fund id
the budget id is used, this can lead to an error 500 if there is no fund with
this id or to a wrong fund being saved.

To test:

1) Create budget and fund
* With the sample data
* Create a new budget "July 2023"
* Create a new fund "Books" under it
* Limit access to Owner and make yourself the owner

2) Create order
* Create a basket
* Create an order line with this access limited fund
* Close the basket

3) Create a staff user with limited permissions
* Edit or create another patron
* Set catalogue and acq permissions without budget_manage_all and order_manage_all (no access to the fund)
* Edit username and password

4) Receive, recreating the error
* Log in as the second user in a separate browser
* Receive shipment
* Create invoice
* Receive
* You should see the fund under the "Keep current" heading selected
* Add quantity received, save
* Fund has changed to Fund_1_2 => data loss :(

5) Receive, verify fix
* Create another order line with your privileged user and the "Books" fund
* Repeat 4)
* After receiving the fund should now remain unchanged

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2023-06-22 15:20:00 UTC
Trivial and clear fix, thanks Katrin.
Comment 12 Martin Renvoize 2023-06-22 15:22:10 UTC Comment hidden (obsolete)
Comment 13 Martin Renvoize 2023-06-22 15:22:45 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release