Bug 15261 - Verify if checkout or hold request periods overlap with existing holds
Summary: Verify if checkout or hold request periods overlap with existing holds
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement with 8 votes (vote)
Assignee: Arthur Suzuki
QA Contact: Testopia
URL:
Keywords: dependency
Depends on: 20724 24410
Blocks: 23669
  Show dependency treegraph
 
Reported: 2015-11-27 15:16 UTC by Alex Arnaud
Modified: 2023-12-31 10:47 UTC (History)
22 users (show)

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


Attachments
Bug 15261: Verify if checkouts/reserves requests periods overlap... (27.15 KB, patch)
2015-11-27 15:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: remove Date::Range and Date::Simple dependencies (2.51 KB, patch)
2015-12-18 14:43 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: remove Date::Range and Date::Simple dependencies (2.42 KB, patch)
2015-12-18 15:14 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (27.14 KB, patch)
2016-01-08 14:13 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: remove Date::Range and Date::Simple dependencies (2.42 KB, patch)
2016-01-08 14:13 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (27.19 KB, patch)
2016-01-08 16:07 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 15261: remove Date::Range and Date::Simple dependencies (2.47 KB, patch)
2016-01-08 16:07 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (27.14 KB, patch)
2016-03-11 11:18 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: remove Date::Range and Date::Simple dependencies (1.80 KB, patch)
2016-03-11 11:18 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Add PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod system preferences (6.57 KB, patch)
2016-03-11 11:19 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.15 KB, patch)
2016-06-23 10:26 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.19 KB, patch)
2017-01-30 11:00 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.31 KB, patch)
2017-03-23 13:12 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.35 KB, patch)
2017-03-23 13:42 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.55 KB, patch)
2017-03-23 14:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.56 KB, patch)
2017-09-12 14:19 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.56 KB, patch)
2017-09-12 14:25 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261 - Check reserves while renewing an issue (6.32 KB, patch)
2017-09-15 12:25 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 17829: (follow-up) Move GetMember to Koha::Patron (1.10 KB, patch)
2017-10-27 08:31 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 17829: (follow-up) Move GetMember to Koha::Patron (1.10 KB, patch)
2017-10-27 08:32 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 17829: (follow-up) Move GetMember to Koha::Patron (1.10 KB, patch)
2017-10-27 08:34 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.56 KB, patch)
2017-10-27 08:41 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261 - Check reserves while renewing an issue (6.32 KB, patch)
2017-10-27 08:41 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.61 KB, patch)
2017-10-31 10:15 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261 - Check reserves while renewing an issue (6.32 KB, patch)
2017-10-31 10:15 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (83.95 KB, patch)
2018-03-15 13:47 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261 - Check reserves while renewing an issue (6.42 KB, patch)
2018-03-15 13:47 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.07 KB, patch)
2018-06-21 07:48 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261 - Check reserves while renewing an issue (6.49 KB, patch)
2018-06-21 07:48 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.59 KB, patch)
2019-04-18 12:15 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Check reserves while renewing an issue (6.02 KB, patch)
2019-04-18 12:15 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.67 KB, patch)
2019-04-18 13:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Check reserves while renewing an issue (6.02 KB, patch)
2019-04-18 13:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (32.67 KB, patch)
2019-08-20 07:13 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Check reserves while renewing an issue (6.02 KB, patch)
2019-08-20 07:13 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Verify if checkouts/reserves requests periods overlap... (34.27 KB, patch)
2019-09-04 15:51 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Check reserves while renewing an issue (6.07 KB, patch)
2019-09-04 15:51 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Fix compilation error in reserve/placerequest.pl (992 bytes, patch)
2019-09-09 11:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Check if checkout/reserve request overlaps existing reserves (36.63 KB, patch)
2019-09-24 08:13 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Check if checkout/reserve request overlaps existing reserves (36.69 KB, patch)
2019-10-30 12:15 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Check if checkout/reserve request overlaps existing reserves (36.69 KB, patch)
2020-02-04 12:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Check if checkout/reserve request overlaps existing reserves (32.76 KB, patch)
2020-02-05 07:39 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15261: Check if checkout/reserve request overlaps existing reserves (36.67 KB, patch)
2020-02-10 16:36 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 15261: Check if checkout/reserve request overlaps existing reserves (36.74 KB, patch)
2020-07-16 08:30 UTC, Arthur Suzuki
Details | Diff | Splinter Review
Bug 15261: (QA follow-up) (11.15 KB, patch)
2020-07-16 13:19 UTC, Arthur Suzuki
Details | Diff | Splinter Review
Bug 15261: Prevent checkout only if for a different borrower (2.21 KB, patch)
2021-05-11 08:05 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 15261: Always check preference PreventCheckoutOnSameReservePeriod first (5.81 KB, patch)
2021-08-03 06:45 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 15261: Check if checkout/reserve request overlaps existing reserves (38.55 KB, patch)
2021-10-06 12:32 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: (QA follow-up) (11.15 KB, patch)
2021-10-06 12:32 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Prevent checkout only if for a different borrower (2.20 KB, patch)
2021-10-06 12:32 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Always check preference PreventCheckoutOnSameReservePeriod first (5.61 KB, patch)
2021-10-06 12:32 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15261: Check if checkout/reserve request overlaps existing reserves (39.76 KB, patch)
2022-03-10 15:41 UTC, Henri-Damien LAURENT
Details | Diff | Splinter Review
Bug 15261: (QA follow-up) (11.04 KB, patch)
2022-03-10 15:41 UTC, Henri-Damien LAURENT
Details | Diff | Splinter Review
Bug 15261: Prevent checkout only if for a different borrower (2.26 KB, patch)
2022-03-10 15:41 UTC, Henri-Damien LAURENT
Details | Diff | Splinter Review
Bug 15261: Always check preference PreventCheckoutOnSameReservePeriod first (5.66 KB, patch)
2022-03-10 15:41 UTC, Henri-Damien LAURENT
Details | Diff | Splinter Review
Bug 15261: Trying to fix after rebase (10.55 KB, patch)
2022-03-10 15:41 UTC, Henri-Damien LAURENT
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Arnaud 2015-11-27 15:16:07 UTC
When checking out or placing hold, we should check if an existing reserve whose period overlap exists.

A user place an hold from opac whose requested period overlap an existing reserve period => prevent reserve,

A librarian place an hold from staff whose requested period overlap an existing reserve period => Warn librarian (Ask for confirmation),

A librarian make a checkout from staff whose requested period overlap an existing reserve period => Warn librarian (Ask for confirmation).
Comment 1 Julian Maurice 2015-11-27 15:49:41 UTC Comment hidden (obsolete)
Comment 2 Aleisha Amohia 2015-12-01 22:35:25 UTC
The patch does not apply, given these errors after applying:

Can't locate Date/Range.pm in @INC (@INC contains: /home/koha/src /etc/perl /usr/local/lib/perl/5.14.2 /usr/local/share/perl/5.14.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.14 /usr/share/perl/5.14 /usr/local/lib/site_perl .) at /home/koha/src/Koha/DateUtils.pm line 21.
BEGIN failed--compilation aborted at /home/koha/src/Koha/DateUtils.pm line 21.
Compilation failed in require at /home/koha/src/C4/Search/History.pm line 7.
BEGIN failed--compilation aborted at /home/koha/src/C4/Search/History.pm line 7.
Compilation failed in require at /home/koha/src/C4/Auth.pm line 32.
BEGIN failed--compilation aborted at /home/koha/src/C4/Auth.pm line 32.
Compilation failed in require at /home/koha/src/mainpage.pl line 25.
BEGIN failed--compilation aborted at /home/koha/src/mainpage.pl line 25.
Comment 3 Alex Arnaud 2015-12-02 08:31:05 UTC
Hello,

There is 2 new dependencies: Date::Range and Date::Simple that you have to install.
Comment 4 delaye 2015-12-15 15:35:37 UTC
Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture

Biblibre - test with http://pro.test1.biblibre.com: ok

1 (staff side):
Place a hold on title (which has only one items) level with start date
and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are warned about an existing reserve

Biblibre - test with http://pro.test1.biblibre.com: ok


2 (staff side):
Place a hold on title (which has more than one items) level with start
date and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are NOT warned about an existing reserve. Because it remains
at least one item not reserved.

Biblibre - test with http://pro.test1.biblibre.com: ok

3 (staff side):
Place a hold on item level with start date and expiration date.
Place another hold on item level with period overlaping this reserve.
Check you are warned about an existing reserve.

Biblibre - test with http://pro.test1.biblibre.com: ok


4 (opac side):
Do the same than for staff side. Instead of a warn, reserve is
prevented.

5:
Place a hold on title (which has only one items) level with start date
and expiration date.
Try to checkout the unique item from this title with period overlaping
the reserve period.
Check you are warned about an existing reserve

Biblibre - test with http://catalogue.test1.biblibre.com: ok


6:
Place a hold on title (which has more than one items) level with start
date and expiration date.
Checkout an item from this title with period overlaping the reserve
period.
Check you are NOT warned about an existing reserve.

Biblibre  - test with http://catalogue.test1.biblibre.com: ok: ok


7:
Place a hold on item level with start date and expiration date.
Checkout this item period overlaping the reserve period.
Check you are warned about an existing reserve

Biblibre  - test with http://catalogue.test1.biblibre.com: ok: ok
Comment 5 Jonathan Druart 2015-12-16 09:27:27 UTC
Claire: Who did signoff this patch?

Alex: Are you sure you cannot avoid to add the 2 deps and use DateTime to do the job?
Comment 6 Alex Arnaud 2015-12-16 09:56:02 UTC
Jonathan: I'm not sure of anything when talking about dates and programming. But Date::Range is the only tool i found that quickly checks if 2 date ranges overlap. Unfortunately, it works only with Date::Simple.
Comment 7 Jonathan Druart 2015-12-16 10:33:23 UTC
With DateTime and DateTime::Set, I am sure you can get the same result quickly.
Comment 8 Alex Arnaud 2015-12-18 14:43:48 UTC Comment hidden (obsolete)
Comment 9 Alex Arnaud 2015-12-18 15:14:18 UTC Comment hidden (obsolete)
Comment 10 Alex Arnaud 2016-01-08 14:13:13 UTC Comment hidden (obsolete)
Comment 11 Alex Arnaud 2016-01-08 14:13:19 UTC Comment hidden (obsolete)
Comment 12 Biblibre Sandboxes 2016-01-08 16:07:13 UTC
Patch tested with a sandbox, by delaye <stephane.delaye@biblibre.com>
Comment 13 Biblibre Sandboxes 2016-01-08 16:07:40 UTC Comment hidden (obsolete)
Comment 14 Biblibre Sandboxes 2016-01-08 16:07:44 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2016-01-11 10:35:47 UTC
Hi Alex and Julian,

some things about this patch: 
- Can you please add a some information about the use case of this? I am not sure about how this is used in the library.
- It seems to change the behaviour, but there are no new preferences or configuration options introduced. Bit worried about this.

As this touches the complicated matter of holds, I'd like to ask for another sign-off. Also to get another opinion about the mechanics of it.

Adding Marcel and Kyle in CC.
Comment 16 Marcel de Rooy 2016-01-11 10:37:49 UTC
Could you please clarify why an existing reserve and a new one should not be allowed to have some overlap in the period?
In our case this may happen frequently.
Comment 17 Kyle M Hall 2016-01-12 15:21:01 UTC
(In reply to Marcel de Rooy from comment #16)
> Could you please clarify why an existing reserve and a new one should not be
> allowed to have some overlap in the period?
> In our case this may happen frequently.

It seems to me that this is more like treating a hold as a true reservation than a hold. It's saying "This item will go to this patron on this date, so nobody else can have it on that date". Kind of like reserving a conference room.

Now, whether we should have that behavior or not is a the real question or not. 

This link is for another ILS and at seems to explain Holds vs Reserves vs Reservations pretty well: http://www.goalexandria.com/totw/tips/2009/100509.html
Comment 18 Kyle M Hall 2016-01-12 15:22:49 UTC
I should also note, we have Holds ( of course! ), we have Reserves ( aka Course Reserves ) but Koha doesn't really have Reservations as a first class feature.
Comment 19 Katrin Fischer 2016-01-12 15:40:25 UTC
Hi Kyle or Marcel,

can you maybe explain the usual use case for the HoldDateInFuture setting? Maybe this will help to clear things up about the differences even more. I am not quite sure about myself and would like to understand better how it's currently used.
Comment 20 Kyle M Hall 2016-01-12 15:44:47 UTC
(In reply to Katrin Fischer from comment #19)
> Hi Kyle or Marcel,
> 
> can you maybe explain the usual use case for the HoldDateInFuture setting?
> Maybe this will help to clear things up about the differences even more. I
> am not quite sure about myself and would like to understand better how it's
> currently used.

I'm not the author of HoldDateInFuture ( iirc ), so I don't really know. I think that it is basically shoehorning Reservations behavior into holds, but since I don't have a use case, I may be wrong.
Comment 21 Katrin Fischer 2016-01-12 16:16:55 UTC
Ah, I thought it was yours - Holds... must be Kyle! Sorry!
Comment 22 Kyle M Hall 2016-01-12 17:54:48 UTC
(In reply to Katrin Fischer from comment #21)
> Ah, I thought it was yours - Holds... must be Kyle! Sorry!

lol, I can understand why ; )
Comment 23 Marcel de Rooy 2016-01-13 08:20:29 UTC
(In reply to Katrin Fischer from comment #19)
> can you maybe explain the usual use case for the HoldDateInFuture setting?
> Maybe this will help to clear things up about the differences even more. I
> am not quite sure about myself and would like to understand better how it's
> currently used.

We have enabled a.o.: AllowHoldDateInFuture, ConfirmFutureHolds=2, OPACAllowHoldDateInFuture.
This offers the user to supply two additional dates with a hold request: Hold starts on date, and Hold not needed after.

What does that mean? The patron tells us here that he would like to have the book only in the indicated (future) period. So we do not put the book in waiting status for him until this period is very close (ConfirmFutureHolds). In the meantime we can issue the book to someone else or let others place holds. Similarly, another patron can also say: I want this book between A and B. No problem if this overlaps the period of patron 1. Priority is here based on [future] start date.
If the book could not be put in waiting status for patron 1 in the indicated period somehow, the expired hold will be cancelled.

This is imo just a straightforward use of the two dates as Koha offers by default. Our specific local situation is not relevant in this discussion.

Note that I am still missing any response from the authors of this patch.
Comment 24 Julian Maurice 2016-02-17 09:47:51 UTC
Why Koha should allow to place hold that will be (probably) impossible to satisfy ?

Let's take an example:
- Patron 1 place a hold that starts on 2016-03-01 and ends on 2016-03-07
- Patron 2 place a hold that starts on 2016-03-02 and ends on 2016-03-06
If patron 1 get the book, patron 2 will never have it (unless patron 1 returns the book very quickly, which is very unlikely to happen imo) and is never informed about the high probability that this will happen.

Another issue:
- Patron 1 place a hold that starts on 2016-03-02 and ends on 2016-03-08
- Patron 2 place a hold (after patron 1 has placed their hold) that starts on 2016-03-01 (we don't care about the end date here)
If priority is based on start date, then patron 2 will "steal" the book from patron 1. Patron 1 will never get the book while they have placed their hold first.
Comment 25 Katrin Fischer 2016-02-17 09:58:04 UTC
I think ideally we shoudl have a proper reservation system that maybe even works separately from the holds system. But my main concern here is that the new behavioru should be optional - so we honor the different workflows. If the books is not checked out or only used on-site, the library can coordinate overlapping holds.
Comment 26 Marcel de Rooy 2016-02-17 13:09:04 UTC
(In reply to Julian Maurice from comment #24)
Thx for your response.

> Let's take an example:
> - Patron 1 place a hold that starts on 2016-03-01 and ends on 2016-03-07
> - Patron 2 place a hold that starts on 2016-03-02 and ends on 2016-03-06
> If patron 1 get the book, patron 2 will never have it (unless patron 1
> returns the book very quickly, which is very unlikely to happen imo) and is
> never informed about the high probability that this will happen.
"Unlikely to happen imo " is the problem here. As a special library we loan the book and it will normally be returned the same day. (The book does not really leave the building.)

> Another issue:
> - Patron 1 place a hold that starts on 2016-03-02 and ends on 2016-03-08
> - Patron 2 place a hold (after patron 1 has placed their hold) that starts
> on 2016-03-01 (we don't care about the end date here)
> If priority is based on start date, then patron 2 will "steal" the book from
> patron 1. Patron 1 will never get the book while they have placed their hold
> first.
No. Patron2 may receive the book before patron1. That is no problem. Patron1 will probably have it the next day.
Comment 27 Julian Maurice 2016-02-17 13:24:05 UTC
(In reply to Marcel de Rooy from comment #26)
> No. Patron2 may receive the book before patron1. That is no problem. Patron1
> will probably have it the next day.

For a 1-day loan that is true.

Looks like we are going to need another reserves syspref :(
Comment 28 Alex Arnaud 2016-03-11 11:18:28 UTC Comment hidden (obsolete)
Comment 29 Alex Arnaud 2016-03-11 11:18:43 UTC Comment hidden (obsolete)
Comment 30 Alex Arnaud 2016-03-11 11:19:07 UTC Comment hidden (obsolete)
Comment 31 Alex Arnaud 2016-06-23 10:26:31 UTC Comment hidden (obsolete)
Comment 32 Fridolin Somers 2016-11-28 10:55:34 UTC
Change in C4/Installer/PerlDependencies.pm is useless I think
Comment 33 Alex Arnaud 2017-01-27 14:54:37 UTC
(In reply to Fridolin SOMERS from comment #32)
> Change in C4/Installer/PerlDependencies.pm is useless I think

I think you are right Frido
Comment 34 Alex Arnaud 2017-01-30 11:00:24 UTC Comment hidden (obsolete)
Comment 35 Alex Arnaud 2017-03-23 13:12:54 UTC Comment hidden (obsolete)
Comment 36 Alex Arnaud 2017-03-23 13:42:05 UTC
Created attachment 61539 [details] [review]
Bug 15261: Verify if checkouts/reserves requests periods overlap...

... with existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

A user place an hold from opac whose requested period overlap an
existing reserve period => prevent reserve,

A librarian place an hold from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation),

A librarian make a checkout from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation).

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
Place a hold on title (which has only one items) level with start date
and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are warned about an existing reserve

2 (staff side):
Place a hold on title (which has more than one items) level with start
date and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are NOT warned about an existing reserve. Because it remains
at least one item not reserved.

3 (staff side):
Place a hold on item level with start date and expiration date.
Place another hold on item level with period overlaping this reserve.
Check you are warned about an existing reserve.

4 (opac side):
Do the same than for staff side. Instead of a warn, reserve is
prevented.

5:
Place a hold on title (which has only one items) level with start date
and expiration date.
Try to checkout the unique item from this title with period overlaping
the reserve period.
Check you are warned about an existing reserve

6:
Place a hold on title (which has more than one items) level with start
date and expiration date.
Checkout an item from this title with period overlaping the reserve
period.
Check you are NOT warned about an existing reserve.

7:
Place a hold on item level with start date and expiration date.
Checkout this item period overlaping the reserve period.
Check you are warned about an existing reserve

Rabased on master
Rebased on master (2016-06-23)
Rebased on master (2017-03-23)
Comment 37 Biblibre Sandboxes 2017-03-23 14:53:15 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 38 Biblibre Sandboxes 2017-03-23 14:53:42 UTC
Created attachment 61544 [details] [review]
Bug 15261: Verify if checkouts/reserves requests periods overlap...

... with existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

A user place an hold from opac whose requested period overlap an
existing reserve period => prevent reserve,

A librarian place an hold from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation),

A librarian make a checkout from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation).

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
Place a hold on title (which has only one items) level with start date
and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are warned about an existing reserve

2 (staff side):
Place a hold on title (which has more than one items) level with start
date and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are NOT warned about an existing reserve. Because it remains
at least one item not reserved.

3 (staff side):
Place a hold on item level with start date and expiration date.
Place another hold on item level with period overlaping this reserve.
Check you are warned about an existing reserve.

4 (opac side):
Do the same than for staff side. Instead of a warn, reserve is
prevented.

5:
Place a hold on title (which has only one items) level with start date
and expiration date.
Try to checkout the unique item from this title with period overlaping
the reserve period.
Check you are warned about an existing reserve

6:
Place a hold on title (which has more than one items) level with start
date and expiration date.
Checkout an item from this title with period overlaping the reserve
period.
Check you are NOT warned about an existing reserve.

7:
Place a hold on item level with start date and expiration date.
Checkout this item period overlaping the reserve period.
Check you are warned about an existing reserve

Rabased on master
Rebased on master (2016-06-23)
Rebased on master (2017-03-23)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 39 Marcel de Rooy 2017-05-17 14:43:58 UTC
Typo: PreventChechoutOnSameReservePeriod
Comment 40 Marcel de Rooy 2017-06-02 07:02:05 UTC
No longer applies and typo mentioned in previous comment
Comment 41 Alex Arnaud 2017-09-12 14:19:03 UTC Comment hidden (obsolete)
Comment 42 Alex Arnaud 2017-09-12 14:25:22 UTC
Created attachment 67094 [details] [review]
Bug 15261: Verify if checkouts/reserves requests periods overlap...

... with existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

A user place an hold from opac whose requested period overlap an
existing reserve period => prevent reserve,

A librarian place an hold from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation),

A librarian make a checkout from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation).

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
Place a hold on title (which has only one items) level with start date
and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are warned about an existing reserve

2 (staff side):
Place a hold on title (which has more than one items) level with start
date and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are NOT warned about an existing reserve. Because it remains
at least one item not reserved.

3 (staff side):
Place a hold on item level with start date and expiration date.
Place another hold on item level with period overlaping this reserve.
Check you are warned about an existing reserve.

4 (opac side):
Do the same than for staff side. Instead of a warn, reserve is
prevented.

5:
Place a hold on title (which has only one items) level with start date
and expiration date.
Try to checkout the unique item from this title with period overlaping
the reserve period.
Check you are warned about an existing reserve

6:
Place a hold on title (which has more than one items) level with start
date and expiration date.
Checkout an item from this title with period overlaping the reserve
period.
Check you are NOT warned about an existing reserve.

7:
Place a hold on item level with start date and expiration date.
Checkout this item period overlaping the reserve period.
Check you are warned about an existing reserve

Rabased on master
Rebased on master (2016-06-23)
Rebased on master (2017-03-23)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 43 Alex Arnaud 2017-09-15 12:25:52 UTC
Created attachment 67143 [details] [review]
Bug 15261 - Check reserves while renewing an issue

Test plan:

  - Create an issue (i.e from 15/09/2017 to 04/10/2017),
  - place a reserve on the same item from 05/10/2017 to 30/10/2017),
  - enable PreventCheckoutOnSameReservePeriod,
  - Check that you are not able to renew the issue.
Comment 44 Alex Buckley 2017-10-03 23:48:13 UTC
When applying these patches this error is thrown:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15261: Verify if checkouts/reserves requests periods overlap...
Applying: Bug 15261 - Check reserves while renewing an issue
fatal: sha1 information is lacking or useless (C4/Circulation.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 15261 - Check reserves while renewing an issue
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-15261---Check-reserves-while-renewing-an-issue-b3aCiG.patch
Comment 45 Alex Arnaud 2017-10-27 08:31:37 UTC Comment hidden (obsolete)
Comment 46 Alex Arnaud 2017-10-27 08:32:42 UTC Comment hidden (obsolete)
Comment 47 Alex Arnaud 2017-10-27 08:34:13 UTC Comment hidden (obsolete)
Comment 48 Alex Arnaud 2017-10-27 08:41:14 UTC Comment hidden (obsolete)
Comment 49 Alex Arnaud 2017-10-27 08:41:37 UTC Comment hidden (obsolete)
Comment 50 Alex Arnaud 2017-10-31 10:15:14 UTC Comment hidden (obsolete)
Comment 51 Alex Arnaud 2017-10-31 10:15:22 UTC Comment hidden (obsolete)
Comment 52 Lucie Gay 2018-03-14 11:37:09 UTC Comment hidden (obsolete)
Comment 53 Alex Arnaud 2018-03-15 13:47:33 UTC Comment hidden (obsolete)
Comment 54 Alex Arnaud 2018-03-15 13:47:45 UTC Comment hidden (obsolete)
Comment 55 Fridolin Somers 2018-06-19 13:05:46 UTC
Oups, looks like patch makes a lot of unwanted changes in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Comment 56 Fridolin Somers 2018-06-20 10:37:40 UTC
Oups there is an obsolete : use C4::Branch in t/db_dependent/Circulation/CanBookBeIssued.t
Comment 57 Fridolin Somers 2018-06-21 07:42:12 UTC
(In reply to Fridolin SOMERS from comment #55)
> Oups, looks like patch makes a lot of unwanted changes in
> koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref

Oh I found it :
Tabs instead of 4 spaces in many places.
This brakes the file which is a YAML.
Comment 58 Alex Arnaud 2018-06-21 07:48:48 UTC
Created attachment 76221 [details] [review]
Bug 15261: Verify if checkouts/reserves requests periods overlap...

... with existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

A user place an hold from opac whose requested period overlap an
existing reserve period => prevent reserve,

A librarian place an hold from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation),

A librarian make a checkout from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation).

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
Place a hold on title (which has only one items) level with start date
and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are warned about an existing reserve

2 (staff side):
Place a hold on title (which has more than one items) level with start
date and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are NOT warned about an existing reserve. Because it remains
at least one item not reserved.

3 (staff side):
Place a hold on item level with start date and expiration date.
Place another hold on item level with period overlaping this reserve.
Check you are warned about an existing reserve.

4 (opac side):
Do the same than for staff side. Instead of a warn, reserve is
prevented.

5:
Place a hold on title (which has only one items) level with start date
and expiration date.
Try to checkout the unique item from this title with period overlaping
the reserve period.
Check you are warned about an existing reserve

6:
Place a hold on title (which has more than one items) level with start
date and expiration date.
Checkout an item from this title with period overlaping the reserve
period.
Check you are NOT warned about an existing reserve.

7:
Place a hold on item level with start date and expiration date.
Checkout this item period overlaping the reserve period.
Check you are warned about an existing reserve

Rabased on master
Rebased on master (2016-06-23)
Rebased on master (2017-03-23)
Rebased on master (2018-03-15)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 59 Alex Arnaud 2018-06-21 07:48:56 UTC
Created attachment 76222 [details] [review]
Bug 15261 - Check reserves while renewing an issue

Test plan:

  - Create an issue (i.e from 15/09/2017 to 04/10/2017),
  - place a reserve on the same item from 05/10/2017 to 30/10/2017),
  - enable PreventCheckoutOnSameReservePeriod,
  - Check that you are not able to renew the issue.

Rebased (2018-03-15)
Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 60 Alex Arnaud 2018-06-21 08:02:39 UTC
(In reply to Fridolin SOMERS from comment #57)
> (In reply to Fridolin SOMERS from comment #55)
> > Oups, looks like patch makes a lot of unwanted changes in
> > koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
> 
> Oh I found it :
> Tabs instead of 4 spaces in many places.
> This brakes the file which is a YAML.

I have rebased the patches and removed unwanted changes on circulation.pref
Comment 61 Fridolin Somers 2018-07-05 06:52:24 UTC
(In reply to Alex Arnaud from comment #60)
> (In reply to Fridolin SOMERS from comment #57)
> > (In reply to Fridolin SOMERS from comment #55)
> > > Oups, looks like patch makes a lot of unwanted changes in
> > > koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
> > 
> > Oh I found it :
> > Tabs instead of 4 spaces in many places.
> > This brakes the file which is a YAML.
> 
> I have rebased the patches and removed unwanted changes on circulation.pref

Thanks a lot Alex ;)
Comment 62 Fridolin Somers 2018-10-19 12:04:55 UTC
A small issue : $borrowernumber is declared twice in reserve/placerequest.pl :
  my ( $template, $borrowernumber, $cookie, $flags ) = get_template_and_user(
  my $borrowernumber=$input->param('borrowernumber');
I think we should rename the second var.
Comment 63 Fridolin Somers 2018-10-19 12:10:59 UTC
I had dependency on Bug 20724 because it moved code from reserve/placerequest.pl
Comment 64 Laurence Rault 2018-11-22 10:23:24 UTC
Conflict when installing on sandbox

Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Comment 65 Julian Maurice 2019-04-18 12:15:34 UTC
Created attachment 88292 [details] [review]
Bug 15261: Verify if checkouts/reserves requests periods overlap...

... with existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

A user place an hold from opac whose requested period overlap an
existing reserve period => prevent reserve,

A librarian place an hold from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation),

A librarian make a checkout from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation).

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
Place a hold on title (which has only one items) level with start date
and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are warned about an existing reserve

2 (staff side):
Place a hold on title (which has more than one items) level with start
date and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are NOT warned about an existing reserve. Because it remains
at least one item not reserved.

3 (staff side):
Place a hold on item level with start date and expiration date.
Place another hold on item level with period overlaping this reserve.
Check you are warned about an existing reserve.

4 (opac side):
Do the same than for staff side. Instead of a warn, reserve is
prevented.

5:
Place a hold on title (which has only one items) level with start date
and expiration date.
Try to checkout the unique item from this title with period overlaping
the reserve period.
Check you are warned about an existing reserve

6:
Place a hold on title (which has more than one items) level with start
date and expiration date.
Checkout an item from this title with period overlaping the reserve
period.
Check you are NOT warned about an existing reserve.

7:
Place a hold on item level with start date and expiration date.
Checkout this item period overlaping the reserve period.
Check you are warned about an existing reserve

Rebased on master (2016-03-11)
Rebased on master (2016-06-23)
Rebased on master (2017-03-23)
Rebased on master (2018-03-15)
Rebased on master (2019-04-18)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 66 Julian Maurice 2019-04-18 12:15:41 UTC
Created attachment 88293 [details] [review]
Bug 15261: Check reserves while renewing an issue

Test plan:

  - Create an issue (i.e from 15/09/2017 to 04/10/2017),
  - place a reserve on the same item from 05/10/2017 to 30/10/2017),
  - enable PreventCheckoutOnSameReservePeriod,
  - Check that you are not able to renew the issue.

Rebased (2018-03-15)
Rebased (2019-04-18)

Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 67 Julian Maurice 2019-04-18 13:32:17 UTC
Created attachment 88297 [details] [review]
Bug 15261: Verify if checkouts/reserves requests periods overlap...

... with existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

A user place an hold from opac whose requested period overlap an
existing reserve period => prevent reserve,

A librarian place an hold from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation),

A librarian make a checkout from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation).

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
Place a hold on title (which has only one items) level with start date
and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are warned about an existing reserve

2 (staff side):
Place a hold on title (which has more than one items) level with start
date and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are NOT warned about an existing reserve. Because it remains
at least one item not reserved.

3 (staff side):
Place a hold on item level with start date and expiration date.
Place another hold on item level with period overlaping this reserve.
Check you are warned about an existing reserve.

4 (opac side):
Do the same than for staff side. Instead of a warn, reserve is
prevented.

5:
Place a hold on title (which has only one items) level with start date
and expiration date.
Try to checkout the unique item from this title with period overlaping
the reserve period.
Check you are warned about an existing reserve

6:
Place a hold on title (which has more than one items) level with start
date and expiration date.
Checkout an item from this title with period overlaping the reserve
period.
Check you are NOT warned about an existing reserve.

7:
Place a hold on item level with start date and expiration date.
Checkout this item period overlaping the reserve period.
Check you are warned about an existing reserve

Rebased on master (2016-03-11)
Rebased on master (2016-06-23)
Rebased on master (2017-03-23)
Rebased on master (2018-03-15)
Rebased on master (2019-04-18)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 68 Julian Maurice 2019-04-18 13:32:25 UTC
Created attachment 88298 [details] [review]
Bug 15261: Check reserves while renewing an issue

Test plan:

  - Create an issue (i.e from 15/09/2017 to 04/10/2017),
  - place a reserve on the same item from 05/10/2017 to 30/10/2017),
  - enable PreventCheckoutOnSameReservePeriod,
  - Check that you are not able to renew the issue.

Rebased (2018-03-15)
Rebased (2019-04-18)

Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 69 Laurence Rault 2019-07-25 08:48:52 UTC
Conflicts when applying on sandbox : 
CONFLICT (content): Merge conflict in reserve/placerequest.pl
Auto-merging opac/opac-reserve.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Comment 70 Julian Maurice 2019-08-20 07:13:10 UTC
Created attachment 92376 [details] [review]
Bug 15261: Verify if checkouts/reserves requests periods overlap...

... with existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

A user place an hold from opac whose requested period overlap an
existing reserve period => prevent reserve,

A librarian place an hold from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation),

A librarian make a checkout from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation).

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
Place a hold on title (which has only one items) level with start date
and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are warned about an existing reserve

2 (staff side):
Place a hold on title (which has more than one items) level with start
date and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are NOT warned about an existing reserve. Because it remains
at least one item not reserved.

3 (staff side):
Place a hold on item level with start date and expiration date.
Place another hold on item level with period overlaping this reserve.
Check you are warned about an existing reserve.

4 (opac side):
Do the same than for staff side. Instead of a warn, reserve is
prevented.

5:
Place a hold on title (which has only one items) level with start date
and expiration date.
Try to checkout the unique item from this title with period overlaping
the reserve period.
Check you are warned about an existing reserve

6:
Place a hold on title (which has more than one items) level with start
date and expiration date.
Checkout an item from this title with period overlaping the reserve
period.
Check you are NOT warned about an existing reserve.

7:
Place a hold on item level with start date and expiration date.
Checkout this item period overlaping the reserve period.
Check you are warned about an existing reserve

Rebased on master (2016-03-11)
Rebased on master (2016-06-23)
Rebased on master (2017-03-23)
Rebased on master (2018-03-15)
Rebased on master (2019-04-18)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 71 Julian Maurice 2019-08-20 07:13:16 UTC
Created attachment 92377 [details] [review]
Bug 15261: Check reserves while renewing an issue

Test plan:

  - Create an issue (i.e from 15/09/2017 to 04/10/2017),
  - place a reserve on the same item from 05/10/2017 to 30/10/2017),
  - enable PreventCheckoutOnSameReservePeriod,
  - Check that you are not able to renew the issue.

Rebased (2018-03-15)
Rebased (2019-04-18)

Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 72 Julian Maurice 2019-09-04 15:51:19 UTC
Created attachment 92584 [details] [review]
Bug 15261: Verify if checkouts/reserves requests periods overlap...

... with existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

A user place an hold from opac whose requested period overlap an
existing reserve period => prevent reserve,

A librarian place an hold from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation),

A librarian make a checkout from staff whose requested period overlap an
existing reserve period => Warn librarian (Ask for confirmation).

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
Place a hold on title (which has only one items) level with start date
and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are warned about an existing reserve

2 (staff side):
Place a hold on title (which has more than one items) level with start
date and expiration date.
Place another hold (also title level) with period overlaping this
reserve.
Check you are NOT warned about an existing reserve. Because it remains
at least one item not reserved.

3 (staff side):
Place a hold on item level with start date and expiration date.
Place another hold on item level with period overlaping this reserve.
Check you are warned about an existing reserve.

4 (opac side):
Do the same than for staff side. Instead of a warn, reserve is
prevented.

5:
Place a hold on title (which has only one items) level with start date
and expiration date.
Try to checkout the unique item from this title with period overlaping
the reserve period.
Check you are warned about an existing reserve

6:
Place a hold on title (which has more than one items) level with start
date and expiration date.
Checkout an item from this title with period overlaping the reserve
period.
Check you are NOT warned about an existing reserve.

7:
Place a hold on item level with start date and expiration date.
Checkout this item period overlaping the reserve period.
Check you are warned about an existing reserve

Rebased on master (2016-03-11)
Rebased on master (2016-06-23)
Rebased on master (2017-03-23)
Rebased on master (2018-03-15)
Rebased on master (2019-04-18)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 73 Julian Maurice 2019-09-04 15:51:25 UTC
Created attachment 92585 [details] [review]
Bug 15261: Check reserves while renewing an issue

Test plan:

  - Create an issue (i.e from 15/09/2017 to 04/10/2017),
  - place a reserve on the same item from 05/10/2017 to 30/10/2017),
  - enable PreventCheckoutOnSameReservePeriod,
  - Check that you are not able to renew the issue.

Rebased (2018-03-15)
Rebased (2019-04-18)

Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 74 Laurence Rault 2019-09-06 06:53:33 UTC
1) Place a hold on title (which has only one items) level with start date
and expiration date : 
impossible to validate the hold : 
Global symbol "@realbi" requires explicit package name (did you forget to declare "my @realbi"?) at /home/koha/src/reserve/placerequest.pl line 107.
Execution of /home/koha/src/reserve/placerequest.pl aborted due to compilation errors.
Comment 75 Julian Maurice 2019-09-09 11:32:31 UTC
Created attachment 92664 [details] [review]
Bug 15261: Fix compilation error in reserve/placerequest.pl
Comment 76 Julian Maurice 2019-09-24 08:13:18 UTC
Created attachment 93136 [details] [review]
Bug 15261: Check if checkout/reserve request overlaps existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

- A user places a hold from opac whose requested period overlaps with an
  existing reserve period => prevent reserve
- A librarian places a hold from staff whose requested period overlaps
  with an existing reserve period => Warn librarian (Ask for
  confirmation)
- A librarian makes a checkout from staff whose requested period
  overlaps with an existing reserve period => Warn librarian (Ask for
  confirmation)

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are warned about an existing reserve

2 (staff side):
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are NOT warned about an existing reserve. Because it remains
  at least one item not reserved.

3 (staff side):
- Place a hold on item level with start date and expiration date.
- Place another hold on item level with period overlaping this reserve.
- Check you are warned about an existing reserve.

4 (opac side):
- Do the same than for staff side. Instead of a warn, reserve is
  prevented.

5:
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Try to checkout the unique item from this title with period overlaping
  the reserve period.
- Check you are warned about an existing reserve

6:
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Checkout an item from this title with period overlaping the reserve
  period.
- Check you are NOT warned about an existing reserve.

7:
- Place a hold on item level with start date and expiration date.
- Checkout this item period overlaping the reserve period.
- Check you are warned about an existing reserve

8:
- Create a checkout (i.e from ${TODAY} to ${TODAY + 10 days}),
- Place a hold on the same item from ${TODAY + 11 days} to ${TODAY + 30
days}),
- Check that you cannot renew the checkout.
Comment 77 Julian Maurice 2019-10-30 12:15:52 UTC
Created attachment 94871 [details] [review]
Bug 15261: Check if checkout/reserve request overlaps existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

- A user places a hold from opac whose requested period overlaps with an
  existing reserve period => prevent reserve
- A librarian places a hold from staff whose requested period overlaps
  with an existing reserve period => Warn librarian (Ask for
  confirmation)
- A librarian makes a checkout from staff whose requested period
  overlaps with an existing reserve period => Warn librarian (Ask for
  confirmation)

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are warned about an existing reserve

2 (staff side):
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are NOT warned about an existing reserve. Because it remains
  at least one item not reserved.

3 (staff side):
- Place a hold on item level with start date and expiration date.
- Place another hold on item level with period overlaping this reserve.
- Check you are warned about an existing reserve.

4 (opac side):
- Do the same than for staff side. Instead of a warn, reserve is
  prevented.

5:
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Try to checkout the unique item from this title with period overlaping
  the reserve period.
- Check you are warned about an existing reserve

6:
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Checkout an item from this title with period overlaping the reserve
  period.
- Check you are NOT warned about an existing reserve.

7:
- Place a hold on item level with start date and expiration date.
- Checkout this item period overlaping the reserve period.
- Check you are warned about an existing reserve

8:
- Create a checkout (i.e from ${TODAY} to ${TODAY + 10 days}),
- Place a hold on the same item from ${TODAY + 11 days} to ${TODAY + 30
days}),
- Check that you cannot renew the checkout.
Comment 78 Laurence Rault 2020-01-10 10:59:49 UTC
Applied on sandbox

CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t
Comment 79 Julian Maurice 2020-02-04 12:23:25 UTC
Created attachment 98383 [details] [review]
Bug 15261: Check if checkout/reserve request overlaps existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

- A user places a hold from opac whose requested period overlaps with an
  existing reserve period => prevent reserve
- A librarian places a hold from staff whose requested period overlaps
  with an existing reserve period => Warn librarian (Ask for
  confirmation)
- A librarian makes a checkout from staff whose requested period
  overlaps with an existing reserve period => Warn librarian (Ask for
  confirmation)

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are warned about an existing reserve

2 (staff side):
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are NOT warned about an existing reserve. Because it remains
  at least one item not reserved.

3 (staff side):
- Place a hold on item level with start date and expiration date.
- Place another hold on item level with period overlaping this reserve.
- Check you are warned about an existing reserve.

4 (opac side):
- Do the same than for staff side. Instead of a warn, reserve is
  prevented.

5:
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Try to checkout the unique item from this title with period overlaping
  the reserve period.
- Check you are warned about an existing reserve

6:
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Checkout an item from this title with period overlaping the reserve
  period.
- Check you are NOT warned about an existing reserve.

7:
- Place a hold on item level with start date and expiration date.
- Checkout this item period overlaping the reserve period.
- Check you are warned about an existing reserve

8:
- Create a checkout (i.e from ${TODAY} to ${TODAY + 10 days}),
- Place a hold on the same item from ${TODAY + 11 days} to ${TODAY + 30
days}),
- Check that you cannot renew the checkout.
Comment 80 Julian Maurice 2020-02-04 12:24:02 UTC
Patch rebased on master
Comment 81 Julian Maurice 2020-02-05 07:39:30 UTC
Created attachment 98424 [details] [review]
Bug 15261: Check if checkout/reserve request overlaps existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

- A user places a hold from opac whose requested period overlaps with an
  existing reserve period => prevent reserve
- A librarian places a hold from staff whose requested period overlaps
  with an existing reserve period => Warn librarian (Ask for
  confirmation)
- A librarian makes a checkout from staff whose requested period
  overlaps with an existing reserve period => Warn librarian (Ask for
  confirmation)

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are warned about an existing reserve

2 (staff side):
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are NOT warned about an existing reserve. Because it remains
  at least one item not reserved.

3 (staff side):
- Place a hold on item level with start date and expiration date.
- Place another hold on item level with period overlaping this reserve.
- Check you are warned about an existing reserve.

4 (opac side):
- Do the same than for staff side. Instead of a warn, reserve is
  prevented.

5:
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Try to checkout the unique item from this title with period overlaping
  the reserve period.
- Check you are warned about an existing reserve

6:
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Checkout an item from this title with period overlaping the reserve
  period.
- Check you are NOT warned about an existing reserve.

7:
- Place a hold on item level with start date and expiration date.
- Checkout this item period overlaping the reserve period.
- Check you are warned about an existing reserve

8:
- Create a checkout (i.e from ${TODAY} to ${TODAY + 10 days}),
- Place a hold on the same item from ${TODAY + 11 days} to ${TODAY + 30
  days}),
- Check that you cannot renew the checkout.
Comment 82 Laurence Rault 2020-02-05 11:29:05 UTC
Patch put on biblibre sandbox 1

1 : impossible to place a hold from staff : 
First click on Place hold button in request.pl do nothing ; second click makes browser do something but next page never displays and reserve is not added (tested in firefox and chromium)
Comment 83 Julian Maurice 2020-02-05 12:32:56 UTC
Apparently holds are broken on master (bug 24410). We better wait for this bug to be resolved before rebasing it again.
Comment 84 Julian Maurice 2020-02-10 14:26:53 UTC
Resetting status to Needs Signoff now that bug 24410 is pushed to master
Comment 85 Laurence Rault 2020-02-10 16:34:45 UTC
tested on biblibre sandbox1

all tests OK

One remark on test 6 : when there are several items, and hold is placed on title, there is no warn when checking out on item, OK.
I think we could be warned if all items are checked out with overlap, maybe when checking out the last available item, as no more item is available for hold then.
But this could be be an improvement.
Comment 86 Biblibre Sandboxes 2020-02-10 16:36:12 UTC
Patch tested with a sandbox, by Laurence Rault <laurence.rault@biblibre.com>
Comment 87 Biblibre Sandboxes 2020-02-10 16:36:39 UTC
Created attachment 98672 [details] [review]
Bug 15261: Check if checkout/reserve request overlaps existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

- A user places a hold from opac whose requested period overlaps with an
  existing reserve period => prevent reserve
- A librarian places a hold from staff whose requested period overlaps
  with an existing reserve period => Warn librarian (Ask for
  confirmation)
- A librarian makes a checkout from staff whose requested period
  overlaps with an existing reserve period => Warn librarian (Ask for
  confirmation)

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are warned about an existing reserve

2 (staff side):
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are NOT warned about an existing reserve. Because it remains
  at least one item not reserved.

3 (staff side):
- Place a hold on item level with start date and expiration date.
- Place another hold on item level with period overlaping this reserve.
- Check you are warned about an existing reserve.

4 (opac side):
- Do the same than for staff side. Instead of a warn, reserve is
  prevented.

5:
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Try to checkout the unique item from this title with period overlaping
  the reserve period.
- Check you are warned about an existing reserve

6:
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Checkout an item from this title with period overlaping the reserve
  period.
- Check you are NOT warned about an existing reserve.

7:
- Place a hold on item level with start date and expiration date.
- Checkout this item period overlaping the reserve period.
- Check you are warned about an existing reserve

8:
- Create a checkout (i.e from ${TODAY} to ${TODAY + 10 days}),
- Place a hold on the same item from ${TODAY + 11 days} to ${TODAY + 30
days}),
- Check that you cannot renew the checkout.

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>
Comment 88 Katrin Fischer 2020-06-21 16:23:14 UTC
Hi Alex,

can you please rebase and test this patch really carefully? I looked at the code a while ago and had a feeling that some things probably weren't quite right due to previous rebasing. 

98672 - Bug 15261: Check if checkout/reserve request overlaps existing reserves

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15261: Check if checkout/reserve request overlaps existing reserves
Using index info to reconstruct a base tree...
M	C4/Circulation.pm
M	C4/Reserves.pm
M	Koha/DateUtils.pm
M	circ/circulation.pl
M	installer/data/mysql/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
M	koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
M	opac/opac-reserve.pl
M	reserve/placerequest.pl
M	svc/renew
M	t/db_dependent/Circulation.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Circulation.t
Auto-merging svc/renew
Auto-merging reserve/placerequest.pl
Auto-merging opac/opac-reserve.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging circ/circulation.pl
Auto-merging Koha/DateUtils.pm
Auto-merging C4/Reserves.pm
Auto-merging C4/Circulation.pm
CONFLICT (content): Merge conflict in C4/Circulation.pm
Comment 89 Arthur Suzuki 2020-07-16 08:30:09 UTC
Created attachment 106944 [details] [review]
Bug 15261: Check if checkout/reserve request overlaps existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

- A user places a hold from opac whose requested period overlaps with an
  existing reserve period => prevent reserve
- A librarian places a hold from staff whose requested period overlaps
  with an existing reserve period => Warn librarian (Ask for
  confirmation)
- A librarian makes a checkout from staff whose requested period
  overlaps with an existing reserve period => Warn librarian (Ask for
  confirmation)

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are warned about an existing reserve

2 (staff side):
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are NOT warned about an existing reserve. Because it remains
  at least one item not reserved.

3 (staff side):
- Place a hold on item level with start date and expiration date.
- Place another hold on item level with period overlaping this reserve.
- Check you are warned about an existing reserve.

4 (opac side):
- Do the same than for staff side. Instead of a warn, reserve is
  prevented.

5:
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Try to checkout the unique item from this title with period overlaping
  the reserve period.
- Check you are warned about an existing reserve

6:
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Checkout an item from this title with period overlaping the reserve
  period.
- Check you are NOT warned about an existing reserve.

7:
- Place a hold on item level with start date and expiration date.
- Checkout this item period overlaping the reserve period.
- Check you are warned about an existing reserve

8:
- Create a checkout (i.e from ${TODAY} to ${TODAY + 10 days}),
- Place a hold on the same item from ${TODAY + 11 days} to ${TODAY + 30
days}),
- Check that you cannot renew the checkout.

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>
Comment 90 Arthur Suzuki 2020-07-16 08:31:14 UTC
working on rebase right now :)
Comment 91 Arthur Suzuki 2020-07-16 13:19:54 UTC
Created attachment 106949 [details] [review]
Bug 15261: (QA follow-up)
Comment 92 Fridolin Somers 2021-01-07 09:57:34 UTC
Bug 25758 changed signature of CanBookBeRenewed() adding $cron

Update this patch in C4/Circulation.pm and svc/renew
Comment 93 Fridolin Somers 2021-04-09 15:42:47 UTC
diff --git a/circ/circulation.pl b/circ/circulation.pl
index 5ec80c8f15..abfbb9306c 100755
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -410,6 +410,10 @@ if (@$barcodes) {
         }
         unless($confirm_required) {
             my $switch_onsite_checkout = exists $messages->{ONSITE_CHECKOUT_WILL_BE_SWITCHED};
+            if ( $cancelreserve eq 'cancel' ) {
+                CancelReserve({ reserve_id => $query->param('reserveid') });
+            }
+            $cancelreserve = $cancelreserve eq 'revert' ? 'revert' : undef;

CancelReserve does not exist anymore, see Bug 19059
Comment 94 Fridolin Somers 2021-05-11 08:05:07 UTC
Created attachment 120822 [details] [review]
Bug 15261: Prevent checkout only if for a different borrower
Comment 95 Fridolin Somers 2021-05-11 08:06:19 UTC
@Julian :

You had this fix "Prevent checkout only if for a different borrower" on 17.11.
All patches need master rebase.
Comment 96 Arthur Suzuki 2021-06-02 13:58:07 UTC
Just had a quick check at this patch.
Seems the controls on the overlapping reserves is done in the interface code (opac/place-reserve.pl and so on) and not in C4/Reserves.
This is then not checked for ILS-DI and RestAPI.
Comment 97 Fridolin Somers 2021-08-02 08:44:42 UTC
In CanBookBeRenewed(), $date_due is computed even if system preference PreventCheckoutOnSameReservePeriod is off.
This has an impact on performance.
We may add if PreventCheckoutOnSameReservePeriod earlier.
Comment 98 Fridolin Somers 2021-08-03 06:45:37 UTC
Created attachment 123405 [details] [review]
Bug 15261: Always check preference PreventCheckoutOnSameReservePeriod first

Proposed patch (may not apply on current master)
Comment 99 Alex Arnaud 2021-10-06 12:32:09 UTC
Created attachment 125793 [details] [review]
Bug 15261: Check if checkout/reserve request overlaps existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

- A user places a hold from opac whose requested period overlaps with an
  existing reserve period => prevent reserve
- A librarian places a hold from staff whose requested period overlaps
  with an existing reserve period => Warn librarian (Ask for
  confirmation)
- A librarian makes a checkout from staff whose requested period
  overlaps with an existing reserve period => Warn librarian (Ask for
  confirmation)

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are warned about an existing reserve

2 (staff side):
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are NOT warned about an existing reserve. Because it remains
  at least one item not reserved.

3 (staff side):
- Place a hold on item level with start date and expiration date.
- Place another hold on item level with period overlaping this reserve.
- Check you are warned about an existing reserve.

4 (opac side):
- Do the same than for staff side. Instead of a warn, reserve is
  prevented.

5:
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Try to checkout the unique item from this title with period overlaping
  the reserve period.
- Check you are warned about an existing reserve

6:
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Checkout an item from this title with period overlaping the reserve
  period.
- Check you are NOT warned about an existing reserve.

7:
- Place a hold on item level with start date and expiration date.
- Checkout this item period overlaping the reserve period.
- Check you are warned about an existing reserve

8:
- Create a checkout (i.e from ${TODAY} to ${TODAY + 10 days}),
- Place a hold on the same item from ${TODAY + 11 days} to ${TODAY + 30
days}),
- Check that you cannot renew the checkout.

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>
Comment 100 Alex Arnaud 2021-10-06 12:32:24 UTC
Created attachment 125794 [details] [review]
Bug 15261: (QA follow-up)
Comment 101 Alex Arnaud 2021-10-06 12:32:42 UTC
Created attachment 125795 [details] [review]
Bug 15261: Prevent checkout only if for a different borrower
Comment 102 Alex Arnaud 2021-10-06 12:32:53 UTC
Created attachment 125796 [details] [review]
Bug 15261: Always check preference PreventCheckoutOnSameReservePeriod first
Comment 103 Alex Arnaud 2021-10-06 12:33:43 UTC
(In reply to Fridolin Somers from comment #98)
> Created attachment 123405 [details] [review] [review]
> Bug 15261: Always check preference PreventCheckoutOnSameReservePeriod first
> 
> Proposed patch (may not apply on current master)

All patches rebased
Comment 104 Henri-Damien LAURENT 2022-03-10 15:41:16 UTC
Created attachment 131569 [details] [review]
Bug 15261: Check if checkout/reserve request overlaps existing reserves

When checking out or placing hold, we should check if an existing
reserve whose period overlap exists.

- A user places a hold from opac whose requested period overlaps with an
  existing reserve period => prevent reserve
- A librarian places a hold from staff whose requested period overlaps
  with an existing reserve period => Warn librarian (Ask for
  confirmation)
- A librarian makes a checkout from staff whose requested period
  overlaps with an existing reserve period => Warn librarian (Ask for
  confirmation)

Test plan:

Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod

1 (staff side):
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are warned about an existing reserve

2 (staff side):
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Place another hold (also title level) with period overlaping this
  reserve.
- Check you are NOT warned about an existing reserve. Because it remains
  at least one item not reserved.

3 (staff side):
- Place a hold on item level with start date and expiration date.
- Place another hold on item level with period overlaping this reserve.
- Check you are warned about an existing reserve.

4 (opac side):
- Do the same than for staff side. Instead of a warn, reserve is
  prevented.

5:
- Place a hold on title (which has only one items) level with start date
  and expiration date.
- Try to checkout the unique item from this title with period overlaping
  the reserve period.
- Check you are warned about an existing reserve

6:
- Place a hold on title (which has more than one items) level with start
  date and expiration date.
- Checkout an item from this title with period overlaping the reserve
  period.
- Check you are NOT warned about an existing reserve.

7:
- Place a hold on item level with start date and expiration date.
- Checkout this item period overlaping the reserve period.
- Check you are warned about an existing reserve

8:
- Create a checkout (i.e from ${TODAY} to ${TODAY + 10 days}),
- Place a hold on the same item from ${TODAY + 11 days} to ${TODAY + 30
days}),
- Check that you cannot renew the checkout.

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>

https://bugs.koha-community.org/show_bug.cgi?id=1526
Comment 105 Henri-Damien LAURENT 2022-03-10 15:41:23 UTC
Created attachment 131570 [details] [review]
Bug 15261: (QA follow-up)

https://bugs.koha-community.org/show_bug.cgi?id=1526
Comment 106 Henri-Damien LAURENT 2022-03-10 15:41:30 UTC
Created attachment 131571 [details] [review]
Bug 15261: Prevent checkout only if for a different borrower

https://bugs.koha-community.org/show_bug.cgi?id=1526
Comment 107 Henri-Damien LAURENT 2022-03-10 15:41:37 UTC
Created attachment 131572 [details] [review]
Bug 15261: Always check preference PreventCheckoutOnSameReservePeriod first

https://bugs.koha-community.org/show_bug.cgi?id=1526
Comment 108 Henri-Damien LAURENT 2022-03-10 15:41:44 UTC
Created attachment 131573 [details] [review]
Bug 15261: Trying to fix after rebase

https://bugs.koha-community.org/show_bug.cgi?id=1526
Comment 109 Fridolin Somers 2022-08-19 19:58:36 UTC
(In reply to Henri-Damien LAURENT from comment #108)
> Created attachment 131573 [details] [review] [review]
> Bug 15261: Trying to fix after rebase
> 
> https://bugs.koha-community.org/show_bug.cgi?id=1526

Careful you added warns (for debug I bet) :

     if ( C4::Context->preference("PreventCheckoutOnSameReservePeriod") ) {
+	    warn $date_due;
+	    warn "Inside";
         unless ($date_due) {
Comment 110 Fridolin Somers 2022-08-19 20:05:15 UTC
Ouch, 

koha-tmpl/intranet-tmpl/prog/en/modules/reserve/placerequest.tt
is using obsolete YUI :

<div id="doc3" class="yui-t2">

  <div id="bd">
    <div id="yui-main">
      <div class="yui-b">
Comment 111 Fridolin Somers 2022-08-19 20:06:58 UTC
(In reply to Henri-Damien LAURENT from comment #108)
> Created attachment 131573 [details] [review] [review]
> Bug 15261: Trying to fix after rebase
> 
> https://bugs.koha-community.org/show_bug.cgi?id=1526

Also missing in opac/opac-reserve.pl :
         if ( $canreserve && C4::Context->preference("PreventReservesOnSamePeriod") ) {
-            if ( ReservesOnSamePeriod($biblioNum, $itemNum, $startdate, $expiration_date) ) {
+            if ( C4::Reserves::ReservesOnSamePeriod($biblioNum, $itemNum, $startdate, $expiration_date) ) {
                 $canreserve = 0;
                 $failed_holds++;
             }
Comment 112 Katrin Fischer 2023-08-28 19:46:48 UTC
I think the idea was about 'booking' things in advance. Could we maybe replace this bug with: 

Bug 29002 - Add ability to book items ahead of time 

?
Comment 113 Fridolin Somers 2023-08-31 00:47:33 UTC
Pref template must be fixed :
choices yes/no must be 1/0
Comment 114 Katrin Fischer 2023-12-31 10:47:55 UTC
Is this something that could be replaced by the new bookings development?