Bug 18079 - Cleanup of holds to pull page
Summary: Cleanup of holds to pull page
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Josef Moravec
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on: 8454 9320 18073
Blocks:
  Show dependency treegraph
 
Reported: 2017-02-08 08:47 UTC by Josef Moravec
Modified: 2017-03-16 19:29 UTC (History)
7 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 18079: Holds to pull cleanup (10.97 KB, patch)
2017-02-08 09:20 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED OFF] Bug 18079: Holds to pull cleanup (11.04 KB, patch)
2017-02-08 21:15 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18079: Holds to pull cleanup (11.14 KB, patch)
2017-02-09 13:18 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18079: Holds to pull cleanup (11.24 KB, patch)
2017-02-09 14:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18279: Remove C4::Items::GetLostItems (9.44 KB, patch)
2017-03-16 19:27 UTC, Marc Véron
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Josef Moravec 2017-02-08 08:47:20 UTC
There were some modifications in pendingreserves-pl script through the time, but not all were really finished.
Comment 1 Josef Moravec 2017-02-08 09:20:59 UTC
Created attachment 60008 [details] [review]
Bug 18079: Holds to pull cleanup

Changes made:

  - remove obsolete comment in pendingreserves.pl
  - use Modern::Perl in circ/pendingreserves.pl
  - get rid of unusable param run_report - followup for bug 8454
  - get rid of references to hold status - followup for bug 9320
  - remove unused data from SQL and reservedata structure

Test plan:
1) Apply patch from bug 18073
2) Apply patch on this bug
3) Enable on shelf hold in administration -> circulation and fines rules
4) Create some holds on avalaible items
5) Confirm that circulation -> holds to pull page works as expected
6) Try to find any regression
Comment 2 Katrin Fischer 2017-02-08 21:15:11 UTC
Created attachment 60038 [details] [review]
[SIGNED OFF] Bug 18079: Holds to pull cleanup

Changes made:

  - remove obsolete comment in pendingreserves.pl
  - use Modern::Perl in circ/pendingreserves.pl
  - get rid of unusable param run_report - followup for bug 8454
  - get rid of references to hold status - followup for bug 9320
  - remove unused data from SQL and reservedata structure

Test plan:
1) Apply patch from bug 18073
2) Apply patch on this bug
3) Enable on shelf hold in administration -> circulation and fines rules
4) Create some holds on avalaible items
5) Confirm that circulation -> holds to pull page works as expected
6) Try to find any regression

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 3 Jonathan Druart 2017-02-09 12:57:03 UTC
CONCAT_WS is a mysqlism, I would prefer to build the patron's name on TT side.
Comment 4 Josef Moravec 2017-02-09 13:18:13 UTC
Created attachment 60070 [details] [review]
Bug 18079: Holds to pull cleanup

Changes made:

  - remove obsolete comment in pendingreserves.pl
  - use Modern::Perl in circ/pendingreserves.pl
  - get rid of unusable param run_report - followup for bug 8454
  - get rid of references to hold status - followup for bug 9320
  - remove unused data from SQL and reservedata structure

Test plan:
1) Apply patch from bug 18073
2) Apply patch on this bug
3) Enable on shelf hold in administration -> circulation and fines rules
4) Create some holds on avalaible items
5) Confirm that circulation -> holds to pull page works as expected
6) Try to find any regression

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Josef Moravec 2017-02-09 13:19:22 UTC
s(In reply to Jonathan Druart from comment #3)
> CONCAT_WS is a mysqlism, I would prefer to build the patron's name on TT
> side.

As this was introduced on bug 18073 I added a followup up there and rebased this one on top of it.
Comment 6 Jonathan Druart 2017-02-09 13:51:36 UTC
Yes thanks!
I was QA both at the same time. Will come back soon.
Comment 7 Marcel de Rooy 2017-02-09 14:38:43 UTC
Created attachment 60078 [details] [review]
Bug 18079: Holds to pull cleanup

Changes made:

  - remove obsolete comment in pendingreserves.pl
  - use Modern::Perl in circ/pendingreserves.pl
  - get rid of unusable param run_report - followup for bug 8454
  - get rid of references to hold status - followup for bug 9320
  - remove unused data from SQL and reservedata structure

Test plan:
1) Apply patch from bug 18073
2) Apply patch on this bug
3) Enable on shelf hold in administration -> circulation and fines rules
4) Create some holds on avalaible items
5) Confirm that circulation -> holds to pull page works as expected
6) Try to find any regression

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2017-02-09 14:39:21 UTC
Looks good to me.
Passed QA
Comment 9 Kyle M Hall 2017-02-14 13:48:41 UTC
Pushed to master for 17.05, thanks Josef!
Comment 10 Katrin Fischer 2017-02-14 19:30:50 UTC
Code clean-up - good thing, but strictly speaking not a bug fix. Resetting to enh. As an enhancement this won't get backported for 16.11.x.
Comment 11 Marc Véron 2017-03-16 19:27:42 UTC Comment hidden (obsolete)
Comment 12 Marc Véron 2017-03-16 19:29:25 UTC Comment hidden (obsolete)