[Koha-patches] [PATCH] Made changes to add sorting on the catalouging screen in the intranet section.

[Joe Atzberger]

Satyanarayana --

Thanks for sending your patch.

My suspicion is that much of this code is duplicated from other parts of
Koha where it could be reused more efficiently.  Copying all the code for
searching make maintenance and updating a very difficult task.  Also, the
autocomplete stuff should already be available in another .inc file, so you
shouldn't have to paste it in when you could include it.

You do a lot of work to rebuild the pagination logic.  Can you describe why
pagination_bar won't work for your purposes?

+my $branches = GetBranches();
> +my @branch_loop;
> +
> +for my $branch_hash (sort { $branches->{$a}->{branchname} cmp
> $branches->{$b}->{branchname} } keys %$branches) {
> +    push @branch_loop, {value => "$branch_hash" , branchname =>
> $branches->{$branch_hash}->{'branchname'}, };
> +}


Use GetBranchesLoop to do the same thing.  It's designed exactly for that.

+if (C4::Context->preference('NoZebra')) {
> +    $query=~s/yr(:|=)\s*([\d]{1,4})-([\d]{1,4})/(yr>=$2 and yr<=$3)/g;
> +    $simple_query=~s/yr\s*(:|=)([\d]{1,4})-([\d]{1,4})/(yr>=$2 and
> yr<=$3)/g;
> +    warn $query;


Unconditional warns should be avoided.


> +    if (C4::Context->userenv->{'flags'}==1
> ||(C4::Context->userenv->{'flags'} & ( 2**9 ) )){


This is the wrong check, as I mentioned recently.  flags == 1 asks "does the
user have ONLY superlibrarian priveleges" when you really want to know if
they have AT LEAST superlibrarian priveleges.


> -<title>Koha &rsaquo; Cataloging</title>
> +<title>Koha &rsaquo; Cataloguing</title>


I think "Cataloging" is the spelling we agreed to standardize on for the
default template.  That makes it en-US instead of en-UK.  It is sort of
funny to have to "translate" between one version of English and another, but
that is the best way to make sure everybody can get the result they want.


> -                <td><!-- TMPL_VAR NAME="title" escape="html" -->
> +                <td><!-- TMPL_VAR NAME="title" -->
>

The title should still be escaped for HTML display, right?

--Joe Atzberger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/koha-patches/attachments/20090701/cee1f650/attachment-0002.htm>