Bug 18208

Summary: Add RecordProcessor filter to inject not onloan count to MARC records
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: katrin.fischer, kyle, m.de.rooy, nick, 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:
Bug Depends on:    
Bug Blocks: 18098, 18579    
Attachments: Bug 18208: Add RecordProcessor filter to inject not onloan count to MARC records
Bug 18208: Add RecordProcessor filter to inject not onloan count to MARC records
Bug 18208: Add RecordProcessor filter to inject not onloan count to MARC records
Bug 18208: [QA Follow-up] Get count from Koha::Items

Description Tomás Cohen Arazi 2017-03-03 18:29:29 UTC

    
Comment 1 Tomás Cohen Arazi 2017-03-03 18:43:07 UTC
Created attachment 60812 [details] [review]
Bug 18208: Add RecordProcessor filter to inject not onloan count to MARC records

This patch adds a new filter for MARC records to be used with
Koha::RecordProcessor. It's purpose is to inject the information about
items not-onloan in a fixed subfield, 999$x.

To test:
- Apply the patch
- Run:
  $ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: ByWater Solutions
Comment 2 Marc Véron 2017-03-03 19:29:58 UTC
Created attachment 60815 [details] [review]
Bug 18208: Add RecordProcessor filter to inject not onloan count to MARC records

This patch adds a new filter for MARC records to be used with
Koha::RecordProcessor. It's purpose is to inject the information about
items not-onloan in a fixed subfield, 999$x.

To test:
- Apply the patch
- Run:
  $ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: ByWater Solutions

Followed test plan, test passes OK
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 3 Marcel de Rooy 2017-03-10 10:56:02 UTC
my $items = Koha::Items->search({ biblionumber => $biblionumber });
+
+    while ( my $item = $items->next ) {
+        $not_onloan_items++
+            if not $item->onloan;
+    }
Can't you combine that in one statement?

In general: Do I understand correctly that this is not an optional filter? We will always save the not onloan count to 999$x?
Can you also add some explanation on this report why we definitely need it?
At this point we are not using it in the codebase. But if we should do after some time, would it feel good to rely on a field in the search engine when we can generate the count in one Koha::Items call ?
No blockers, just convince me..
Comment 4 Tomás Cohen Arazi 2017-03-10 11:58:37 UTC
(In reply to Marcel de Rooy from comment #3)
> my $items = Koha::Items->search({ biblionumber => $biblionumber });
> +
> +    while ( my $item = $items->next ) {
> +        $not_onloan_items++
> +            if not $item->onloan;
> +    }
> Can't you combine that in one statement?
> 
> In general: Do I understand correctly that this is not an optional filter?
> We will always save the not onloan count to 999$x?
> Can you also add some explanation on this report why we definitely need it?
> At this point we are not using it in the codebase. But if we should do after
> some time, would it feel good to rely on a field in the search engine when
> we can generate the count in one Koha::Items call ?
> No blockers, just convince me..

The record will always inject that. And is going to be used on indexing only. See bug 18098.

When it comes to filtering search results (records with available items this time) we need the index for being able to build the filtering query. We already do that but the query is not working, and this way writing the query seems straightfowrad. Just check if the index has a value higher than 0. Period.

RecordProcessor filters should be used for lots of things (specially stuff done in C4::Biblios) like embedding items...
Comment 5 Marcel de Rooy 2017-04-07 09:58:47 UTC
Created attachment 61964 [details] [review]
Bug 18208: Add RecordProcessor filter to inject not onloan count to MARC records

This patch adds a new filter for MARC records to be used with
Koha::RecordProcessor. It's purpose is to inject the information about
items not-onloan in a fixed subfield, 999$x.

To test:
- Apply the patch
- Run:
  $ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: ByWater Solutions

Followed test plan, test passes OK
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2017-04-07 09:58:52 UTC
Created attachment 61965 [details] [review]
Bug 18208: [QA Follow-up] Get count from Koha::Items

Removes the loop by moving the onloan test to the query.

Test plan:
Run the test again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Kyle M Hall 2017-04-13 12:51:15 UTC
Pushed to master for 17.05, thanks Tomas, Marcel!
Comment 8 Katrin Fischer 2017-04-16 12:23:47 UTC
This won't get ported back to 16.11.x as it is an enhancement.