Bug 19299 - Replace C4::Reserves::GetReservesForBranch with Koha::Holds->waiting
Summary: Replace C4::Reserves::GetReservesForBranch with Koha::Holds->waiting
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 17728
  Show dependency treegraph
 
Reported: 2017-09-12 21:33 UTC by Jonathan Druart
Modified: 2017-12-14 17:45 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19299: Replace C4::Reserves::GetReservesForBranch with Koha::Holds->waiting (6.29 KB, patch)
2017-09-13 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19299: Replace C4::Reserves::GetReservesForBranch with Koha::Holds->waiting (6.29 KB, patch)
2017-09-13 19:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19299: Replace C4::Reserves::GetReservesForBranch with Koha::Holds->waiting (6.33 KB, patch)
2017-10-29 21:47 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19299: Replace C4::Reserves::GetReservesForBranch with Koha::Holds->waiting (6.42 KB, patch)
2017-11-03 08:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-09-12 21:33:48 UTC

    
Comment 1 Jonathan Druart 2017-09-13 18:56:16 UTC
Created attachment 67117 [details] [review]
Bug 19299: Replace C4::Reserves::GetReservesForBranch with Koha::Holds->waiting

GetReservesForBranch simply returns the waiting holds, for a specific
branch of not. This can easily be replaced with a call to
Koha::Holds->waiting

To avoid any regressions, I reuse the exact conditions (priority = 0),
but I do not think it is useful.

Test plan:
Make sure the holds information are correctly displayed on the waiting
holds screen.
Comment 2 Jonathan Druart 2017-09-13 19:20:39 UTC
Created attachment 67118 [details] [review]
Bug 19299: Replace C4::Reserves::GetReservesForBranch with Koha::Holds->waiting

GetReservesForBranch simply returns the waiting holds, for a specific
branch of not. This can easily be replaced with a call to
Koha::Holds->waiting

To avoid any regressions, I reuse the exact conditions (priority = 0),
but I do not think it is useful.

Test plan:
Make sure the holds information are correctly displayed on the waiting
holds screen.
Comment 3 Josef Moravec 2017-10-29 21:47:45 UTC
Created attachment 68815 [details] [review]
Bug 19299: Replace C4::Reserves::GetReservesForBranch with Koha::Holds->waiting

GetReservesForBranch simply returns the waiting holds, for a specific
branch of not. This can easily be replaced with a call to
Koha::Holds->waiting

To avoid any regressions, I reuse the exact conditions (priority = 0),
but I do not think it is useful.

Test plan:
Make sure the holds information are correctly displayed on the waiting
holds screen.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 4 Marcel de Rooy 2017-11-03 08:27:32 UTC
+# FIXME - Is priority => 0 useful? If yes it must be moved to waiting, otherwise we need to remove it from here.

Since set_waiting sets priority to 0, we should not need to check priority again. But the test has been used more often in that way:
return 'Waiting'  if $found eq 'W' and $priority == 0;

IF found is W or T, priority should be 0. Priority is no longer of importance when an item has been set to Waiting.
Comment 5 Marcel de Rooy 2017-11-03 08:28:21 UTC
Created attachment 68908 [details] [review]
Bug 19299: Replace C4::Reserves::GetReservesForBranch with Koha::Holds->waiting

GetReservesForBranch simply returns the waiting holds, for a specific
branch of not. This can easily be replaced with a call to
Koha::Holds->waiting

To avoid any regressions, I reuse the exact conditions (priority = 0),
but I do not think it is useful.

Test plan:
Make sure the holds information are correctly displayed on the waiting
holds screen.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Jonathan Druart 2017-12-11 14:35:38 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 7 Nick Clemens 2017-12-14 17:45:03 UTC
Awesome work all! Skipping for 17.11, enhancement.