As discussed in bug 10262, code related to the CircControl should be unified under C4::Circulation::GetCircControlBranch.
Created attachment 18462 [details] [review] Bug 10374 - Clean up and unify CircControl related code As discussed in bug 10262, code related to the CircControl should be unified under C4::Circulation::GetCircControlBranch. Test Plan: 1) Apply this patch 2) Find some overdue items 2) Test the different settings for CircControl setting the system preference, returning an item, and verifying the correct rule was used.
Created attachment 18463 [details] [review] Bug 10374 - Clean up and unify CircControl related code As discussed in bug 10262, code related to the CircControl should be unified under C4::Circulation::GetCircControlBranch. Test Plan: 1) Apply this patch 2) Run perl t/db_dependent/Circulation.t 3) All tests should pass
Created attachment 18467 [details] [review] Bug 10374 - Clean up and unify CircControl related code - Followup - Clean up AddReturn This patch tidys up C4::Circulation::AddReturn. It rearranges some comments to make them more sensible, and addresses the FIXME for an unitelligable comment. This patch makes no functional changes to the code.
Created attachment 18468 [details] [review] Bug 10374 - Clean up and unify CircControl related code - Followup - Clean up AddReturn This patch tidys up C4::Circulation::AddReturn. It rearranges some comments to make them more sensible, and addresses the FIXME for an unitelligable comment. This patch makes no functional changes to the code.
Created attachment 18469 [details] [review] Bug 10374 - Clean up and unify CircControl related code - Followup - Clean up AddReturn This patch tidys up C4::Circulation::AddReturn. It rearranges some comments to make them more sensible, and addresses the FIXME for an unitelligable comment. This patch makes no functional changes to the code.
Created attachment 18470 [details] [review] Bug 10374 - Clean up and unify CircControl related code - Followup - Clean up AddReturn This patch tidys up C4::Circulation::AddReturn. It rearranges some comments to make them more sensible, and addresses the FIXME for an unitelligable comment. This patch makes no functional changes to the code.
Created attachment 18471 [details] [review] Bug 10374 - Clean up and unify CircControl related code - Followup - Clean up AddReturn This patch tidys up C4::Circulation::AddReturn. It rearranges some comments to make them more sensible, and addresses the FIXME for an unitelligable comment. This patch makes no functional changes to the code.
Applying: Bug 10374 - Clean up and unify CircControl related code Using index info to reconstruct a base tree... M C4/Circulation.pm M C4/Reserves.pm M reserve/request.pl M t/db_dependent/Circulation.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Circulation.t CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t Auto-merging reserve/request.pl Auto-merging C4/Reserves.pm CONFLICT (content): Merge conflict in C4/Reserves.pm Auto-merging C4/Circulation.pm Patch failed at 0001 Bug 10374 - Clean up and unify CircControl related code The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
Comment on attachment 18463 [details] [review] Bug 10374 - Clean up and unify CircControl related code Review of attachment 18463 [details] [review]: ----------------------------------------------------------------- This was the oldest bug that was "Patch does not apply", but since there are minor logic issues. I figure this commentary on them might spur a fix. ::: C4/Circulation.pm @@ +2249,5 @@ > + if ( $circcontrol eq 'PickupLibrary' ) { > + if ( C4::Context->userenv && C4::Context->userenv->{branch} ) > + { > + $branch = C4::Context->userenv->{branch}; > + } else { If one is tweaking the if/else's, why not use a consistent style? Here it is "} else {" a few lines later "}" with "else {" on the next line. @@ +2265,5 @@ > + else { > + $branch = $item->{ C4::Context->preference('HomeOrHoldingBranch') } > + || $item->{homebranch}; > + } > + } What happened to the else case? What if $circcontrol is not 'PickupLibrary', 'PatronLibrary', or 'ItemHomeLibrary'?