Bug 7364 - Fast-Add framework doesn't always get the branch for item creation
Summary: Fast-Add framework doesn't always get the branch for item creation
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: 3.6
Hardware: All All
: P3 enhancement (vote)
Assignee: Ian Walls
QA Contact: Ian Walls
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-16 15:38 UTC by D Ruth Holloway
Modified: 2013-12-05 19:57 UTC (History)
3 users (show)

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


Attachments
Proposed patch (1.24 KB, patch)
2011-12-16 15:42 UTC, D Ruth Holloway
Details | Diff | Splinter Review
Bug 7364: Possible to get to FA item add without current branch being defined. (1.29 KB, patch)
2011-12-18 07:20 UTC, Chris Cormack
Details | Diff | Splinter Review
Syntax-corrected patch (1.42 KB, patch)
2012-01-16 18:07 UTC, Ian Walls
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description D Ruth Holloway 2011-12-16 15:38:44 UTC
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!)
Comment 1 D Ruth Holloway 2011-12-16 15:42:52 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2011-12-18 07:20:36 UTC Comment hidden (obsolete)
Comment 3 Ian Walls 2012-01-09 20:33:40 UTC
Much cleaner way of assigning the branchcode that works in all cases.  Marking as Passed QA
Comment 4 Paul Poulain 2012-01-13 10:34:59 UTC
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)
Comment 5 Ian Walls 2012-01-13 18:04:50 UTC
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.
Comment 6 Paul Poulain 2012-01-16 14:29:38 UTC
(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 !
Comment 7 Paul Poulain 2012-01-16 17:57:26 UTC
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 !
Comment 8 Ian Walls 2012-01-16 18:07:36 UTC
Created attachment 7176 [details] [review]
Syntax-corrected patch

Fixed use of {} in patch; should be ().  cataloging/additem.pl loads without incident or error now