Bug 5596 - add author to holds to pull report
Summary: add author to holds to pull report
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-07 20:38 UTC by Nicole C. Engard
Modified: 2013-12-05 20:04 UTC (History)
4 users (show)

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


Attachments
Bug 5596 - add author to holds to pull report (2.59 KB, patch)
2012-03-06 15:27 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 5596 [REVISED] add author to holds to pull report (2.48 KB, patch)
2012-03-12 12:53 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 5596 [REVISED] add author to holds to pull report (2.59 KB, patch)
2012-03-12 17:04 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 5596 [REVISED 2] add author to holds to pull report (1.18 KB, patch)
2012-03-14 14:48 UTC, Owen Leonard
Details | Diff | Splinter Review
signed-off patch (1.20 KB, patch)
2012-03-19 00:51 UTC, Kristina Hoeppner
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.