Bug 5072 - Filling an order page with no budget defined causes a 500 error
Summary: Filling an order page with no budget defined causes a 500 error
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: PATCH-Sent (DO NOT USE) normal (vote)
Target Milestone: ---
Assignee: Robin Sheat
QA Contact: Bugs List
URL:
Whiteboard:
Keywords:
: 4111 5061 (view as bug list)
Depends on: 6994
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-29 02:48 UTC by Robin Sheat
Modified: 2013-12-05 20:04 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
proposed patch (4.52 KB, patch)
2010-07-29 04:17 UTC, Robin Sheat
Details | Diff | Splinter Review
Bug 5072 - prevent creating new orders if there are no valid budgets (2.84 KB, patch)
2011-08-01 21:43 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 5072 : Template changes (1.39 KB, patch)
2011-08-01 21:50 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 5072 : Template changes (1.50 KB, patch)
2011-08-01 22:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 5072 - prevent creating new orders if there are no valid budgets (2.95 KB, patch)
2011-08-01 22:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
proposed patch (passed QA) (1.50 KB, patch)
2011-08-03 12:06 UTC, Paul Poulain
Details | Diff | Splinter Review
proposed patch (passed QA) (2.96 KB, patch)
2011-08-03 12:08 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Robin Sheat 2010-07-29 02:48:57 UTC
To reproduce:
1) In acquisitions, add a new order to a basket
2) Select "From an external source"
3) Add some search terms, and hit search
4) On one of the results, hit 'order'
5) Fill out the 'new order' page
6) Watch the system fail and say "Mandatory parameter budget_id missing at /usr/share/koha/lib/C4/Acquisition.pm line 903."
Note: this only happens if you have no budgets defined.

Expected result:
You can't get past step 2, because koha pipes up and says "hey, add some budgets first."
Comment 1 Robin Sheat 2010-07-29 04:17:38 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2010-07-29 05:16:54 UTC
*** Bug 5061 has been marked as a duplicate of this bug. ***
Comment 3 Katrin Fischer 2010-07-29 05:19:07 UTC
Colin commented on bug 5061, that this bug is also related to 3616.

Colin: 'The bug is related to 3616 but needs a separate fix. In Acquisitions.pm the error message is issued via a die. die is not a user friendly way to handle
error conditions.'
Comment 4 Robin Sheat 2010-07-29 05:24:22 UTC
While I haven't tested it, I suspect that #3616 is solved by this as well, as this corrects the workflow to prevent the user getting to the point of trying to enter a new order if it would fail further down the track.
Comment 5 Katrin Fischer 2011-03-13 20:25:16 UTC
Hi Robin, 
your patch contains changes to a .gitignore file. Can you please resubmit without it?
Comment 6 Chris Cormack 2011-08-01 21:43:23 UTC Comment hidden (obsolete)
Comment 7 Chris Cormack 2011-08-01 21:50:27 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2011-08-01 22:04:30 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2011-08-01 22:04:50 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2011-08-01 22:05:04 UTC
Retested this on current master. If you try to save a order in acquisitions you get an alert now that you have to choose a fund first.
If you have no funds defined, you can't save, but you don't get a perl error message any longer.

With patch you can not add order line to a basket before you have created a fund and a budget.

The patch does not check if you are allowed to spend on that budget. But it's an improvement over current behaviour.
Comment 11 Paul Poulain 2011-08-03 12:06:52 UTC
Created attachment 4818 [details] [review]
proposed patch (passed QA)
Comment 12 Paul Poulain 2011-08-03 12:08:33 UTC
Created attachment 4819 [details] [review]
proposed patch (passed QA)

QA comment:

* patch addresses the problem
* The 2nd patch also contains an undocumented fix in POD doc of the .pm. Thanks Katrin for fixing, shame for not saying it.

Outside from this -very minor- comment, passed QA (+no DB change)
Comment 13 Chris Cormack 2011-08-04 01:55:29 UTC
Pushed please test
Comment 14 Katrin Fischer 2011-08-15 20:03:59 UTC
*** Bug 4111 has been marked as a duplicate of this bug. ***
Comment 15 Katrin Fischer 2011-10-26 15:59:05 UTC
This works, but there is a small follow up to prevent the message from showing up on the 'order from suggestion' page (bug 6994)
Comment 16 Jared Camins-Esakov 2012-12-31 00:08:16 UTC
There have been no further reports of problems so I am marking this bug resolved.