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

[satyanarayana holenarsipur]

Hi Joe,

Thanks for the quick response.

Yes. It is very similar to main search code. I wanted a sorting feature very
similar to what we have now was the search results page. The sort dropdown
comes right next to the results table. But I faced a lot of problems while
doing this.

The search.pl uses 'getRecords' function to get the records, store it and
pas it to the templates.
The addbooks.pl uses 'SimpleSearch' function to get the records, store it
and pass it to the templates.

SimpleSearch has 'offset' as its parameter, but it is only used for
pagination function. and cannot be used for sorting the result set. I could
have modified this search function to accomodate the sorting, but i didn't
want to touch the .pm files. Hence i re-used the code in search.pl to
develop a query with sorting and pagination added to it and sue the
]getRecords' function to the same job.

Also, what ever paramater i was passing (q=satya& )for the addbooks.pl to
use sorting, it would simply do the basic search and return the results
without sorting as SimpleSearch is not equipped to do the sorting. Hence i
had to use getRecords. To use getRecords i had to copy most of the code from
search.pl to addbooks.pl.

Once i decided to use getRecords function instead of SimpleSearch, i was
clear that i would not use the 'pagination_bar' and use whatever the
search.pl was using.

Please let me know if you need more clarification.

Regards,
Satya

On Wed, Jul 1, 2009 at 8:09 PM, Joe Atzberger <joe.atzberger at liblime.com>wrote:

> 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
>



-- 
Regards,
Satyanarayana H.S.
Development Head (www.osslabs.biz)
-- 
"A small group of determined spirits, with an unquenchable thirst for their
mission, can alter the course of history." -Mahatma Gandhi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/koha-patches/attachments/20090702/24e306d1/attachment-0002.htm>