Bug 5596

Summary: add author to holds to pull report
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: enhancement    
Priority: P5 - low CC: koha.sekjal, kristina, paul.poulain, wizzyrea
Version: 3.8   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3550
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 5596 - add author to holds to pull report
Bug 5596 [REVISED] add author to holds to pull report
Bug 5596 [REVISED] add author to holds to pull report
Bug 5596 [REVISED 2] add author to holds to pull report
signed-off patch

Description Nicole C. Engard 2011-01-07 20:38:49 UTC
It would be nice if the holds to pull report had author listed.
Comment 1 Owen Leonard 2012-03-06 15:27:06 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2012-03-11 19:47:47 UTC
Bug 5596 - add author to holds to pull report
Apply? [yn] y

Applying: Bug 5596 - add author to holds to pull report
error: patch failed: circ/pendingreserves.pl:184
error: circ/pendingreserves.pl: patch does not apply
error: patch failed: koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt:125
error: koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt: patch does not apply
Patch failed at 0001 Bug 5596 - add author to holds to pull report
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Patch left in /tmp/Bug-5596---add-author-to-holds-to-pull-report-yfIo_v.patch
Comment 3 Owen Leonard 2012-03-12 12:53:48 UTC Comment hidden (obsolete)
Comment 4 Liz Rea 2012-03-12 17:04:49 UTC Comment hidden (obsolete)
Comment 5 Ian Walls 2012-03-14 14:31:09 UTC
Adding author is easy enough; requires a template only change, since the value is already pulled in the SQL.  Unfortunately, the addition of subtitle introduces n calls to GetMarcBiblio, where n = number of pending reserves.  This is a serious performance killer, as it requires parsing each MARCXML record to extract the subtitle.

Given that subtitle is not part of the original request of this bug report, and it'll cost a lot of extra CPU cycles to extract it, I'm marking this patch as Failed QA.  Please submit a separate bug report for adding subtitle to the holds to pull report.
Comment 6 Owen Leonard 2012-03-14 14:48:16 UTC Comment hidden (obsolete)
Comment 7 Owen Leonard 2012-03-14 14:51:09 UTC
I agree that I broke the rules in including an additional change, but I disagree that extra CPU cycles is a reason why a change like this might be rejected. Using GetMarcBiblio is currently the ONLY way we have to get subtitle, for better or worse.
Comment 8 Kristina Hoeppner 2012-03-19 00:51:08 UTC
Created attachment 8253 [details] [review]
signed-off patch

Tested this on BibLibre sandbox and it works fine.
Comment 9 Ian Walls 2012-03-19 12:19:24 UTC
Adds output of author to template, since it was already exported.  Marking as Passed QA.

Issue of subtitles will be addressed in bug 7715
Comment 10 Paul Poulain 2012-03-19 17:37:17 UTC
(In reply to comment #7)
> I agree that I broke the rules in including an additional change, but I
> disagree that extra CPU cycles is a reason why a change like this might be
> rejected. Using GetMarcBiblio is currently the ONLY way we have to get
> subtitle, for better or worse.

Agreeing with Owen and Ian in fact...

My feeling is that an acceptable circumventing would be to memoize the GetMarcBiblio sub. Once it's done, subsequent calls to the function will cost nothing (note : I've said memoize, not memoize with memcache, as this 2nd option will give a longer persistency, but at the -heavy- price of a http call to memcached)

The best/long term option would be to use a good ORM & internal caching & things like that, but they are not close.

patch pushed (that removes subtitle, that was already here before the patch))
Comment 11 Jared Camins-Esakov 2012-05-23 23:59:42 UTC
Will not be pushed to 3.6.x.
Comment 12 Jared Camins-Esakov 2012-12-31 01:00:06 UTC
There have been no further reports of problems so I am marking this bug resolved.