| Summary: | Cleanup of holds to pull page | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Josef Moravec <josef.moravec> |
| Component: | Circulation | Assignee: | Josef Moravec <josef.moravec> |
| Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | gmcharlt, jonathan.druart, kyle.m.hall, kyle, m.de.rooy, veron |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 8454, 9320, 18073 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 18079: Holds to pull cleanup
[SIGNED OFF] Bug 18079: Holds to pull cleanup Bug 18079: Holds to pull cleanup Bug 18079: Holds to pull cleanup Bug 18279: Remove C4::Items::GetLostItems |
||
|
Description
Josef Moravec
2017-02-08 08:47:20 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 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> CONCAT_WS is a mysqlism, I would prefer to build the patron's name on TT side. 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> 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. Yes thanks! I was QA both at the same time. Will come back soon. 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> Looks good to me. Passed QA Pushed to master for 17.05, thanks Josef! 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. Created attachment 61189 [details] [review] Bug 18279: Remove C4::Items::GetLostItems The JOIN done by this subroutine are not always useful (depending on item-level_itypes). They also search with LIKE when it is not needed. Since we have now Koha::Items, we can replace this subroutine with a call to Koha::Items->search with the correct parameters. A change in previous behaviours can happen: If a items.itemlost contains a value that is not defined as a LOST authorised value, the item will not be displayed. I think it's the expected behaviour, even if it should not happen in correctly configured installations. Test plan: To test with item-level_itypes set to item and biblio: List the lost items you have on your system, using the different filters available. The result table should contain the correct item's info. Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Comment on attachment 61189 [details] [review] Bug 18279: Remove C4::Items::GetLostItems Mistaked with Bug number, sorry for the noise. |