[Koha-patches] [PATCH] bug 2992: Request hold on multiple items at one time.

[Galen Charlton]

Hi Stephen,

On Fri, Feb 27, 2009 at 10:59 AM, Stephen Edwards
<sedwards at alloycomputing.com> wrote:
> Updated the results, cart, and shelf pages to include a button
> or link to initiate a hold request based on checked items.
>
> Updated the request CGI scripts to handle multiple biblio items.
>
> Updated the reserve confirmation page to display multiple items,
> with an optional list of copies for each one.

Thanks, this will be a very useful addition to the Koha OPAC.  I've
tested this, and the new functionality pretty much works as described.
 However, there are some small issues with the patch that I would like
addressed before I push this to the public repository.  Please read
on:

> diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tmpl b/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tmpl

As a general note, basic OPAC functionality should work without
requiring that JavaScript turned out.  While I'm not interpreting this
to mean that *every* feature should work without JavaScript or that
you should rework your patch so that multiple holds can be placed sans
JavaScript, this patch does break one important use case.
Specifically, if you click on the "place hold" link on a bib details
page and try to request a single bib, you get the error message
"ERROR: Internal error: incomplete hold request." if you don't have
JavaScript turned on.  This was working prior to this patch.

> +                    <input type="radio" name="<!-- TMPL_VAR NAME="biblionumber" -->_reqtype"
> +                        id="<!-- TMPL_VAR NAME="biblionumber" -->_reqany"
> +                        class="selectany"
> +                        value="Any"
> +                        checked="checked"
> +                        <!-- TMPL_UNLESS NAME="holdable" -->
> +                          disabled="disabled"
> +                        <!-- /TMPL_UNLESS -->
> +                    <label for="<!-- TMPL_VAR

Embedding a TMPL_UNLESS or a TMPL_VAR doesn't play well with Koha's
translation system.  Although it's more verbose, you have to do

<!-- TMPL_IF NAME="condition" -->
  <tag foo='bar" />
<!-- TMPL_ELSE -->
  <tag foo="bar" selected="selected" />
<!-- /TMPL_IF -->

You can check for this sort of error by running the following test:

prove xt/author/translatable-templates.t

If there's a problem, you'll get an error message like this:

#   Failed test 'opac templates are translatable'
#   at xt/author/translatable-templates.t line 49.
# xgettext.pl: Warning: opac-reserve.tmpl: line 308: SGML "closed
start tag" notation: <input type="radio" name="<!-- TMPL_VAR
NAME="biblionumber" -->_reqtype"

There are a couple other problems with this snippet of template:

[1] There's no '>' for the input tag.
[2] HTML tag IDs shouldn't start with digits.  Thus:

id="<!-- TMPL_VAR NAME="biblionumber" -->_reqany"

should be changed to something like

id="<!-- TMPL_VAR NAME="reqany_biblionumber" -->"

In general, all Koha OPAC and intranet HTML should be valid XHTML.  I
use the FireFox Html Validator add-on to check this.

Finally, if you go to place a hold request and none of the bibs are
requestable or if you don't end up selecting any of them, the place
hold button doesn't do anything when you click it.  Rather than having
the button go dead, *something* should happen; if nothing else it
should pop up an alert telling you to select one of the bibs.

> diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tmpl b/koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tmpl
>  $(document).ready(function(){

Another non-JavaScript comment - if the place hold button on the
search results requires JavaScript, it shouldn't be displayed when
JavaScript is turned off.  This can be accomplished by having
$(document).ready() insert the button.

Regards,

Galen
-- 
Galen Charlton
VP, Research & Development, LibLime
galen.charlton at liblime.com
p: 1-888-564-2457 x709
skype: gmcharlt