| Summary: | C4::Items - Remove GetLostItems | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
| Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | fridolin.somers, m.de.rooy, veron |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Crowdfunding goal: | 0 |
| Patch complexity: | Small patch | Documentation contact: | |
| Documentation submission: | Text to go in the release notes: | ||
| Version(s) released in: | Circulation function: | ||
| Bug Depends on: | 18278, 18295 | ||
| Bug Blocks: | 18252 | ||
| Attachments: |
Bug 18279: Remove C4::Items::GetLostItems
Bug 18279: Remove C4::Items::GetLostItems Bug 18279: Remove C4::Items::GetLostItems Bug 18279: [QA Follow-up] Correct @EXPORT |
||
|
Description
Jonathan Druart
2017-03-15 21:22:26 UTC
Created attachment 61152 [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. DOes not apply: Merge conflict in C4/Items.pm Forgot to add the dependency, sorry. Created attachment 61191 [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> https://bugs.koha-community.org/show_bug.cgi?id=18079 Created attachment 63919 [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> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 63920 [details] [review] Bug 18279: [QA Follow-up] Correct @EXPORT Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> QA Comment:
Looks good to me.
In the lost script, this construction is little bit strange:
my $loststatusfilter = $params->{'loststatusfilter'} || undef;
Normally you would expect something as:
my $loststatusfilter = $params->{'loststatusfilter'} // '';
In this particular case, it might have a weird side effect. If you add a LOST status for value 0 in auth values, and you select that lost value on the report form, it will return you the lost items instead of the items with value 0.
But adding a LOST auth value for 0 is not really useful. Selecting the not lost items on the lost form makes no sense too.
Pushed to master for 17.11, thanks to everybody involved! Enhancement not pushed to 17.05.x |