Bug 17309 - Renewing and HomeOrHoldingBranch syspref
Summary: Renewing and HomeOrHoldingBranch syspref
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: David Kuhn
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-16 20:41 UTC by Ed Veal
Modified: 2017-05-03 04:09 UTC (History)
10 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
Bug 17309 - Renewing and HomeOrHoldingBranch syspref (2.22 KB, patch)
2016-10-25 19:58 UTC, David Kuhn
Details | Diff | Splinter Review
Bug 17309 - Renewing and HomeOrHoldingBranch syspref (2.23 KB, patch)
2016-12-15 00:09 UTC, David Kuhn
Details | Diff | Splinter Review
Bug 17309 - Renewing and HomeOrHoldingBranch syspref (2.43 KB, patch)
2017-03-21 15:01 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 17309 - Renewing and HomeOrHoldingBranch syspref (2.44 KB, patch)
2017-03-21 16:47 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Veal 2016-09-16 20:41:14 UTC
When renewing an item the HomeOrHoldingBranch syspref is not taken into consideration and the issuing branch rules are applied. This can be a problem when the rules differ. 

Ed
Comment 1 David Kuhn 2016-10-25 19:58:34 UTC
Created attachment 56846 [details] [review]
Bug 17309 - Renewing and HomeOrHoldingBranch syspref

The AddRenewal subroutine currently uses the circulation rules for the branch
stored in the Issues table (which is the holding branch) when calculating the new due date.

This patch replaces using the branch from the Issues table with the branch
specified by the HomeOrHoldingBranch syspref.

To test:
1. Set up 2 branches, Branch1 and Branch2
2. Set up a loan rule in Branch1 for DVDs with a 21 day loan period and a
   21 day renewal period.
3. Set up a loan rule in Branch2 for DVDs with a 14 day loan period and a
   14 day renewal period.
4. Checkout a DVD belonging to Branch1 while logged into Branch2. It will
   receive the correct 21 day loan period.
5. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 14 day due date, rather than 21 days.
6. Checkout a DVD belonging to Branch2 while logged into Branch1. It will
   receive the correct 14 day loan period.
7. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 21 day due date, rather than 14 days.
8. Apply the patch and repeat steps 4-7. The correct due date should be given
   when the item is renewed, regardless of where it is checked out or renewed.
Comment 2 David Kuhn 2016-12-15 00:09:37 UTC
Created attachment 58195 [details] [review]
Bug 17309 - Renewing and HomeOrHoldingBranch syspref

The AddRenewal subroutine currently uses the circulation rules for the branch
stored in the Issues table (which is the holding branch) when calculating the new due date.

This patch replaces using the branch from the Issues table with the branch
specified by the HomeOrHoldingBranch syspref.

To test:
1. Set up 2 branches, Branch1 and Branch2
2. Set up a loan rule in Branch1 for DVDs with a 21 day loan period and a
   21 day renewal period.
3. Set up a loan rule in Branch2 for DVDs with a 14 day loan period and a
   14 day renewal period.
4. Checkout a DVD belonging to Branch1 while logged into Branch2. It will
   receive the correct 21 day loan period.
5. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 14 day due date, rather than 21 days.
6. Checkout a DVD belonging to Branch2 while logged into Branch1. It will
   receive the correct 14 day loan period.
7. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 21 day due date, rather than 14 days.
8. Apply the patch and repeat steps 4-7. The correct due date should be given
   when the item is renewed, regardless of where it is checked out or renewed.

This update removes reassignment of $branch variable.
Comment 3 sandboxes@biblibre.com 2017-03-21 15:01:12 UTC
Patch tested with a sandbox, by Cédric Vita <cedric.vita@dracenie.com>
Comment 4 sandboxes@biblibre.com 2017-03-21 15:01:33 UTC
Created attachment 61399 [details] [review]
Bug 17309 - Renewing and HomeOrHoldingBranch syspref

The AddRenewal subroutine currently uses the circulation rules for the branch
stored in the Issues table (which is the holding branch) when calculating the new due date.

This patch replaces using the branch from the Issues table with the branch
specified by the HomeOrHoldingBranch syspref.

To test:
1. Set up 2 branches, Branch1 and Branch2
2. Set up a loan rule in Branch1 for DVDs with a 21 day loan period and a
   21 day renewal period.
3. Set up a loan rule in Branch2 for DVDs with a 14 day loan period and a
   14 day renewal period.
4. Checkout a DVD belonging to Branch1 while logged into Branch2. It will
   receive the correct 21 day loan period.
5. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 14 day due date, rather than 21 days.
6. Checkout a DVD belonging to Branch2 while logged into Branch1. It will
   receive the correct 14 day loan period.
7. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 21 day due date, rather than 14 days.
8. Apply the patch and repeat steps 4-7. The correct due date should be given
   when the item is renewed, regardless of where it is checked out or renewed.

This update removes reassignment of $branch variable.

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 5 Jonathan Druart 2017-03-21 16:47:13 UTC
Created attachment 61404 [details] [review]
Bug 17309 - Renewing and HomeOrHoldingBranch syspref

The AddRenewal subroutine currently uses the circulation rules for the branch
stored in the Issues table (which is the holding branch) when calculating the new due date.

This patch replaces using the branch from the Issues table with the branch
specified by the HomeOrHoldingBranch syspref.

To test:
1. Set up 2 branches, Branch1 and Branch2
2. Set up a loan rule in Branch1 for DVDs with a 21 day loan period and a
   21 day renewal period.
3. Set up a loan rule in Branch2 for DVDs with a 14 day loan period and a
   14 day renewal period.
4. Checkout a DVD belonging to Branch1 while logged into Branch2. It will
   receive the correct 21 day loan period.
5. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 14 day due date, rather than 21 days.
6. Checkout a DVD belonging to Branch2 while logged into Branch1. It will
   receive the correct 14 day loan period.
7. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 21 day due date, rather than 14 days.
8. Apply the patch and repeat steps 4-7. The correct due date should be given
   when the item is renewed, regardless of where it is checked out or renewed.

This update removes reassignment of $branch variable.

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Marcel de Rooy 2017-04-03 07:53:18 UTC
I guess the Version (== 3.22) left this patch in the queue last friday..
It seems to be directed for master.
Changing the version.
Comment 7 Kyle M Hall 2017-04-03 10:39:36 UTC
Pushed to master for 17.05, thanks David!
Comment 8 Katrin Fischer 2017-04-10 21:24:21 UTC
Enhancement or bug?
Comment 9 David Kuhn 2017-04-11 16:49:07 UTC
(In reply to Katrin Fischer from comment #8)
> Enhancement or bug?

This patch is to fix a bug.
Comment 10 Katrin Fischer 2017-04-11 17:24:09 UTC
I am asking, because it is marked as enh - please switch to the appropriate severity! (Second input after Importance) Enh are usually not backported to stable releases, so it's important to set the right status :)
Comment 11 Katrin Fischer 2017-04-11 21:53:17 UTC
Thx!
Comment 12 Katrin Fischer 2017-04-16 11:19:10 UTC
This patch has been pushed to 16.11.x and will be in 16.11.07.
Comment 13 Julian Maurice 2017-04-21 10:50:31 UTC
Pushed to 3.22.x for 3.22.20
Comment 14 Mason James 2017-05-03 04:09:47 UTC
Pushed to 16.05.x, for 16.05.12 release