Bug 18208 - Add RecordProcessor filter to inject not onloan count to MARC records
Summary: Add RecordProcessor filter to inject not onloan count to MARC records
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 18098 18579
  Show dependency treegraph
 
Reported: 2017-03-03 18:29 UTC by Tomás Cohen Arazi
Modified: 2017-12-07 22:16 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 18208: Add RecordProcessor filter to inject not onloan count to MARC records (8.36 KB, patch)
2017-03-03 18:43 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18208: Add RecordProcessor filter to inject not onloan count to MARC records (8.51 KB, patch)
2017-03-03 19:29 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18208: Add RecordProcessor filter to inject not onloan count to MARC records (8.61 KB, patch)
2017-04-07 09:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18208: [QA Follow-up] Get count from Koha::Items (1.41 KB, patch)
2017-04-07 09:58 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 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.