Bug 9167: Standardize the sort field copyrightdate for lists

How to reproduce the issue:
Create a list at the OPAC and select the sort field 'year'. Go to the
list management page at the intranet: the sort field "copyrightdate" is
not selected by default.

How to test this patch:
Check the issue is not still present with this patch.
Create several lists at the OPAC with different sort field.
Check results are consistent on both interfaces.
Check the selected sort field is selected on the edit page.
Check there is no regression.

What this patch does:
- change the way to send the selected sort field to the templates
- remove the select tests on the new list page (useless)
- the copyrightdate sortfield is named "copyrightdate" everywhere
- update your database : set virtualshelves.sortfield = 'copyrightdate'
  if virtualshelves.sortfield = 'year'

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Works as advertised. Improves code.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>

diff --git a/C4/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm
index bf53563..6a4d431 100644 (file)
--- a/C4/VirtualShelves/Page.pm
+++ b/C4/VirtualShelves/Page.pm
@@ -214,7 +214,6 @@ sub shelfpage {
                 my $member = GetMember( 'borrowernumber' => $owner );
                 my $ownername = defined($member) ? $member->{firstname} . " " . $member->{surname} : '';
                 $edit = 1;
-                $sortfield='' unless $sortfield;
                 $template->param(
                     edit                => 1,
                     display             => $displaymode,
@@ -224,7 +223,7 @@ sub shelfpage {
                     ownername           => $ownername,
                     "category$category" => 1,
                     category            => $category,
-                    "sort_$sortfield"   => 1,
+                    sortfield           => $sortfield,
                     allow_add           => $allow_add,
                     allow_delete_own    => $allow_delete_own,
                     allow_delete_other  => $allow_delete_other,
@@ -248,17 +247,9 @@ sub shelfpage {
             #check that the user can view the shelf
             if ( ShelfPossibleAction( $loggedinuser, $shelfnumber, 'view' ) ) {
                 my $items;
-                my $authorsort;
-                my $yearsort;
                 my $tag_quantity;
                 my $sortfield = ( $sorton ? $sorton : 'title' );
-                if ( $sortfield eq 'author' ) {
-                    $authorsort = 'author';
-                }
-                if ( $sortfield eq 'year' ) {
-                    $yearsort = 'year';
-                }
-                ( $items, $totitems ) = GetShelfContents( $shelfnumber, $shelflimit, $shelfoffset, $sortfield eq 'year' ? 'copyrightdate' : $sortfield );
+                ( $items, $totitems ) = GetShelfContents( $shelfnumber, $shelflimit, $shelfoffset, $sortfield );
                 for my $this_item (@$items) {
                     my $biblionumber = $this_item->{'biblionumber'};
                     my $record = GetMarcBiblio($biblionumber);
@@ -302,9 +293,7 @@ sub shelfpage {
                     shelfname           => $shelfname,
                     shelfnumber         => $shelfnumber,
                     viewshelf           => $shelfnumber,
-                    authorsort          => $authorsort,
-                    yearsort            => $yearsort,
-                    itemcallnumbersort  => $sortfield eq 'itemcallnumber',
+                    sortfield           => $sortfield,
                     manageshelf         => $manageshelf,
                     allowremovingitems  => ShelfPossibleAction( $loggedinuser, $shelfnumber, 'delete'),
                     allowaddingitem     => ShelfPossibleAction( $loggedinuser, $shelfnumber, 'add'),
@@ -413,16 +402,6 @@ sub shelfpage {
         my $category  = $shelflist->{$element}->{'category'};
         my $owner     = $shelflist->{$element}->{'owner'}||0;
         my $canmanage = ShelfPossibleAction( $loggedinuser, $element, 'manage' );
-        my $sortfield = $shelflist->{$element}->{'sortfield'};
-        if ( $sortfield ){
-            if ( $sortfield eq 'author' ) {
-                $shelflist->{$element}->{"authorsort"} = 'author';
-            } elsif ( $sortfield eq 'year' ) {
-                $shelflist->{$element}->{"yearsort"} = 'year';
-            } elsif ( $sortfield eq 'itemcallnumber' ) {
-                $shelflist->{$element}->{"itemcallnumbersort"} = 'itemcallnumber';
-            }
-        }
         $shelflist->{$element}->{"viewcategory$category"} = 1;
         $shelflist->{$element}->{manageshelf} = $canmanage;
         if($canmanage || ($loggedinuser && $owner==$loggedinuser)) {

diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
index 6908704..6080ff2 100755 (executable)
--- a/installer/data/mysql/updatedatabase.pl
+++ b/installer/data/mysql/updatedatabase.pl
@@ -6125,6 +6125,15 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
     SetVersion($DBversion);
 }
 
+$DBversion = "3.11.00.XXX";
+if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
+    $dbh->do(q{
+        UPDATE virtualshelves SET sortfield="copyrightdate" where sortfield="year";
+    });
+    print "Upgrade to $DBversion done (Bug 9167: Update the virtualshelves.sortfield column with 'copyrightdate' if needed)\n";
+    SetVersion ($DBversion);
+}
+
 =head1 FUNCTIONS
 
 =head2 TableExists($table)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
index 20ae8ee..81fe513 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
@@ -298,15 +298,15 @@ function placeHold () {
         <ol>
         <li><label class="required" for="addshelf">List name:</label><input id="addshelf" type="text" name="addshelf" size="25" /></li>
         <li><span class="label">Owner: </span><input type="hidden" name="owner" id="owner" value="[% loggedinuser %]" />[% loggedinusername %]</li>
-               <li><label for="sortfield" >Sort this list by: </label>
-               <select name="sortfield" id="sortfield">
-               [% IF ( sort_title ) %]<option value="title" selected="selected">Title</option>[% ELSE %]<option value="title">Title</option>[% END %]
-               [% IF ( sort_author ) %]<option value="author" selected="selected">Author</option>[% ELSE %]<option value="author">Author</option>[% END %]
-               [% IF ( sort_copyrightdate ) %]<option value="copyrightdate" selected="selected">Copyrightdate</option>[% ELSE %]<option value="copyrightdate">Copyrightdate</option>[% END %]
-        [% IF ( sort_itemcallnumber ) %]<option value="itemcallnumber" selected="selected">Call number</option>[% ELSE %]<option value="itemcallnumber">Call number</option>[% END %]
-               </select></li>
+        <li><label for="sortfield" >Sort this list by: </label>
+        <select name="sortfield" id="sortfield">
+            <option value="title">Title</option>
+            <option value="author">Author</option>
+            <option value="copyrightdate">Copyrightdate</option>
+            <option value="itemcallnumber">Call number</option>
+        </select></li>
         <li><label for="category">Category: </label>
-                       <select name="category" id="category">
+            <select name="category" id="category">
                   <option value="1">Private</option>
                   <option value="2">Public</option>
                      </select></li>
@@ -324,10 +324,10 @@ function placeHold () {
                <li><label for="owner">Owner: </label><input type="hidden" id="owner" name="owner" value="[% IF ( owner ) %][% ownername %][% ELSE %][% loggedinusername %][% END %]" />[% IF ( owner ) %][% ownername %][% ELSE %][% loggedinusername %][% END %]</li>
                <li><label for="sortfield" >Sort this list by: </label>
                <select name="sortfield">
-               [% IF ( sort_title ) %]<option value="title" selected="selected">Title</option>[% ELSE %]<option value="title">Title</option>[% END %]
-               [% IF ( sort_author ) %]<option value="author" selected="selected">Author</option>[% ELSE %]<option value="author">Author</option>[% END %]
-               [% IF ( sort_copyrightdate ) %]<option value="copyrightdate" selected="selected">Copyrightdate</option>[% ELSE %]<option value="copyrightdate">Copyrightdate</option>[% END %]
-        [% IF ( sort_itemcallnumber ) %]<option value="itemcallnumber" selected="selected">Call number</option>[% ELSE %]<option value="itemcallnumber">Call number</option>[% END %]
+        [% IF ( sortfield == "title" ) %]<option value="title" selected="selected">Title</option>[% ELSE %]<option value="title">Title</option>[% END %]
+        [% IF ( sortfield == "author" ) %]<option value="author" selected="selected">Author</option>[% ELSE %]<option value="author">Author</option>[% END %]
+        [% IF ( sortfield == "copyrightdate" ) %]<option value="copyrightdate" selected="selected">Copyrightdate</option>[% ELSE %]<option value="copyrightdate">Copyrightdate</option>[% END %]
+        [% IF ( sortfield == "itemcallnumber" ) %]<option value="itemcallnumber" selected="selected">Call number</option>[% ELSE %]<option value="itemcallnumber">Call number</option>[% END %]
                </select></li>
                <li><label for="category">Category: </label>
                        <select id="category" name="category">
@@ -392,7 +392,7 @@ function placeHold () {
                     [% IF ( shelveslooppri.toggle ) %]<tr class="highlight">[% ELSE %]<tr>[% END %]
         <td><a href="shelves.pl?[% IF ( shelveslooppri.showprivateshelves ) %]display=privateshelves&amp;[% END %]viewshelf=[% shelveslooppri.shelf %]&amp;shelfoff=[% shelfoff %]">[% shelveslooppri.shelfname |html %]</a></td>
         <td>[% shelveslooppri.count %] item(s)</td>
-        <td>[% IF ( shelveslooppri.authorsort ) %]Author[% ELSIF ( shelveslooppri.yearsort ) %]Year[% ELSIF (shelveslooppri.itemcallnumbersort) %]Call number[% ELSE %]Title[% END %]</td>
+        <td>[% IF ( shelveslooppri.sortfield == "author" ) %]Author[% ELSIF ( shelveslooppri.sortfield == "copyrightdate" ) %]Year[% ELSIF (shelveslooppri.sortfield == "itemcallnumber") %]Call number[% ELSE %]Title[% END %]</td>
         <td>[% IF ( shelveslooppri.viewcategory1 ) %]Private[% END %]
                        [% IF ( shelveslooppri.viewcategory2 ) %]Public[% END %]
                </td>
@@ -445,7 +445,7 @@ function placeHold () {
                <td><a href="shelves.pl?viewshelf=[% shelvesloo.shelf %]">[% shelvesloo.shelfname |html %]</a></td>
         <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% shelvesloo.owner %]">[% shelvesloo.ownername %]</td>
                <td>[% shelvesloo.count %] item(s)</td>
-        <td>[% IF ( shelvesloo.authorsort ) %]Author[% ELSIF ( shelvesloo.yearsort ) %]Year[% ELSIF (shelvesloo.itemcallnumbersort) %]Call number[% ELSE %]Title[% END %]</td>
+        <td>[% IF ( shelvesloo.sortfield == "author" ) %]Author[% ELSIF ( shelvesloo.sortfield == "copyrightdate" ) %]Year[% ELSIF (shelvesloo.sortfield == "itemcallnumber") %]Call number[% ELSE %]Title[% END %]</td>
         <td>[% IF ( shelvesloo.viewcategory1 ) %]Private[% END %]
                        [% IF ( shelvesloo.viewcategory2 ) %]Public[% END %]
                </td>

diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
index 2e21d12..8f31917 100644 (file)
--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
@@ -485,10 +485,10 @@ $(function() {
                       <li>
                         <label for="sortfield" >Sort this list by: </label>
                         <select name="sortfield" id="sortfield">
-                          [% IF ( sort_title ) %]<option value="title" selected="selected">Title</option>[% ELSE %]<option value="title">Title</option>[% END %]
-                          [% IF ( sort_author ) %]<option value="author" selected="selected">Author</option>[% ELSE %]<option value="author">Author</option>[% END %]
-                          [% IF ( sort_year ) %]<option value="year" selected="selected">Year</option>[% ELSE %]<option value="year">Year</option>[% END %]
-                          [% IF ( sort_itemcallnumber ) %]<option value="itemcallnumber" selected="selected">Call number</option>[% ELSE %]<option value="itemcallnumber">Call number</option>[% END %]
+                          [% IF ( sortfield == "title" ) %]<option value="title" selected="selected">Title</option>[% ELSE %]<option value="title">Title</option>[% END %]
+                          [% IF ( sortfield == "author" ) %]<option value="author" selected="selected">Author</option>[% ELSE %]<option value="author">Author</option>[% END %]
+                          [% IF ( sortfield == "copyrightdate" ) %]<option value="copyrightdate" selected="selected">Year</option>[% ELSE %]<option value="copyrightdate">Year</option>[% END %]
+                          [% IF ( sortfield == "itemcallnumber" ) %]<option value="itemcallnumber" selected="selected">Call number</option>[% ELSE %]<option value="itemcallnumber">Call number</option>[% END %]
                         </select>
                       </li>
                       <li>
@@ -675,10 +675,10 @@ $(function() {
                       <li>
                         <label for="sortfield" >Sort this list by: </label>
                         <select name="sortfield" id="sortfield">
-                          [% IF ( sort_title ) %]<option value="title" selected="selected">Title</option>[% ELSE %]<option value="title">Title</option>[% END %]
-                          [% IF ( sort_author ) %]<option value="author" selected="selected">Author</option>[% ELSE %]<option value="author">Author</option>[% END %]
-                          [% IF ( sort_year ) %]<option value="year" selected="selected">Year</option>[% ELSE %]<option value="year">Year</option>[% END %]
-                          [% IF ( sort_itemcallnumber ) %]<option value="itemcallnumber" selected="selected">Call number</option>[% ELSE %]<option value="itemcallnumber">Call number</option>[% END %]
+                          <option value="title">Title</option>
+                          <option value="author">Author</option>
+                          <option value="copyrightdate">Year</option>
+                          <option value="itemcallnumber">Call number</option>
                         </select>
                       </li>
                       <li>