In the FA framework, the system chooses the value of the parameter fed to it, but it is possible to get to that point without any parameter being set, so the permanent and current location branchcode pulldowns do not get set. It would be useful to have it fall back to the user's branch, if the CGI param is not set. (Patch on the way!)
Created attachment 6835 [details] [review] Proposed patch
Created attachment 6847 [details] [review] Bug 7364: Possible to get to FA item add without current branch being defined. Some paths to the FA item-add screen didn't have a branch defined--if the param is set, use it, otherwise, use the user's logged in branch. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Much cleaner way of assigning the branchcode that works in all cases. Marking as Passed QA
I think I understand what this patch does, but there is no clear test case that I can use to test myself, so please provide one (and switch back to passed QA)
Paul, Is it really necessary to do a second QA on a 5 line patch that is clearly coded, and provides a more solid failover? The code is currently assuming that if the biblio's framework is FA, then a 'branch' param is passed in the URL. This isn't the case if you come from somewhere other than the Fast Add during Checkout. Test plan: 1. Set your library to one that's NOT the first alphabetically in the branch list 2. From Circulation home page, click "Fast Cataloging" 3. Create a fast biblio, and save. 4. Now, on the items screen, you won't see your logged in branch, but rather the first in the list. Apply patch, and step 4 now shows the currently logged in branch as the default. Resetting to Passed QA.
(In reply to comment #5) > Is it really necessary to do a second QA on a 5 line patch that is clearly > coded, and provides a more solid failover? it was not really a QA. It's just that I test all the patches before pushing them, and I couldn't understand how to test this one. Thanks for the > 1. Set your library to one that's NOT the first alphabetically in the branch list that was what was missing !
YIKES: after applying this page, on my test server, I get a nasty: syntax error at /home/paul/koha.dev/koha-community/cataloguing/additem.pl line 668, near "->param{" Execution of /home/paul/koha.dev/koha-community/cataloguing/additem.pl aborted due to compilation errors. Reverting this patch solves the issue. I'm logged in as a superlibrarian, (but not the mysql user), nothing more specific that could explain the problem. i'm reverting master until we understand where this comes from !
Created attachment 7176 [details] [review] Syntax-corrected patch Fixed use of {} in patch; should be (). cataloging/additem.pl loads without incident or error now