It would be nice if the holds to pull report had author listed.
Created attachment 8035 [details] [review] Bug 5596 - add author to holds to pull report Adding output of author and, for good measure, subtitle.
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
Created attachment 8180 [details] [review] Bug 5596 [REVISED] add author to holds to pull report Adding output of author and, for good measure, subtitle.
Created attachment 8187 [details] [review] Bug 5596 [REVISED] add author to holds to pull report Adding output of author and, for good measure, subtitle. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Passes tests, author and subtitle shown on holds to pull report.
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.
Created attachment 8205 [details] [review] Bug 5596 [REVISED 2] add author to holds to pull report Adding output of author.
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.
Created attachment 8253 [details] [review] signed-off patch Tested this on BibLibre sandbox and it works fine.
Adds output of author to template, since it was already exported. Marking as Passed QA. Issue of subtitles will be addressed in bug 7715
(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))
Will not be pushed to 3.6.x.
There have been no further reports of problems so I am marking this bug resolved.