From d14a0cf5be3f6d31a9d5d23748285d077426c300 Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Fri, 15 Aug 2014 16:46:03 +0300
Subject: [PATCH] Bug 8836 - Resurrect Rotating Collections

Test Plan:
In "Tools" -> "Rotating Collections" -> "Add/Remove items":
When adding item barcodes to the collection, the input field
  <input type="text" id="barcode" name="barcode">
should become active
automatically so it is easy to add multiple barcodes rapidly without touching the
mouse or keyboard.
Enter-press is dealt by the barcode reader so automatic form submittal should be handled
by the barcode reader.

In "Rotating collections" -> "Transfer Collection":
When the collection is initially transferred, items are set to trasfer correctly.
When the collection is transferred while items are still being transferred, the transfer
destination library doesn't change from the original one.
The holding library changes for all items in the collection to the destination library on
retransfers as well.
This is tricky if a user accidentally places the wrong destination.
When I try to checkin these items to their new retransfer location, I get the following messages:
-"This item is part of a rotating collection and needs to be transferred to <original transfer destination>"
-"Please return Valkoinen ihmissyj / to <original transfer destination>"
-"Print slip or Cancel transfer"

When I checkin a Item to a arbitrary branch, I get the following messages:
"This item is part of a rotating collection and needs to be transferred to <retransfer destination>"
"Please return Valkoinen ihmissyj / to <original trasfer destination>"

Bug 8836 - Resurrect Rotating Collections - QA Followup

Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts

Bug 8836 - Resurrect Rotating Collections - Followup 3

* Fix bad TT Tag
* Fix bad sql query
* Fix capitalization ( HTML4 )
* Allow a rotating collection's location to keep AutomaticItemReturn
  from sending it back to the branch of origin
* Fix bad query

Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field

Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items

Items in a rotating collection are automatcially transferred when a
collection is transferred. This is a problem for currently checked out
items and items on hold marked as "Waiting".

This patch resolves this issue by skipping the transfer for those items.
When the items are then returned, the librarian will be alerted to
transfer the item to the library currently holding that rotating
collection.

Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode

Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr>
Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org>

http://bugs.koha-community.org/show_bug.cgi?id=8835

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
---
 C4/Circulation.pm                             |   5 +-
 C4/RotatingCollections.pm                     | 466 +++++++++---------
 circ/returns.pl                               |  24 +-
 installer/data/mysql/kohastructure.sql        |   8 +-
 installer/data/mysql/updatedatabase.pl        | 144 ++++--
 .../prog/en/modules/circ/returns.tt           |   2 +-
 .../modules/rotating_collections/addItems.tt  |   7 +
 .../rotating_collections/editCollections.tt   |  10 +-
 .../prog/en/modules/tools/tools-home.tt       |   2 -
 rotating_collections/addItems.pl              | 132 ++---
 rotating_collections/editCollections.pl       | 149 +++---
 rotating_collections/rotatingCollections.pl   |  38 +-
 rotating_collections/transferCollection.pl    |  84 ++--
 13 files changed, 604 insertions(+), 467 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 7ec5c5ea60..99dc3c8670 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -42,6 +42,7 @@ use C4::Koha qw(
     GetKohaAuthorisedValueLib
 );
 use C4::Overdues qw(CalcFine UpdateFine);
+use C4::RotatingCollections qw(GetCollectionItemBranches);
 use Algorithm::CheckDigits;
 
 use Data::Dumper;
@@ -1903,6 +1904,7 @@ sub AddReturn {
     my ($datesent,$frombranch,$tobranch) = GetTransfers( $item->{'itemnumber'} );
 
     # if we have a transfer to do, we update the line of transfers with the datearrived
+    my $is_in_rotating_collection = 1 if C4::RotatingCollections::isItemInAnyCollection( $item->{'itemnumber'} );
     if ($datesent) {
         if ( $tobranch eq $branch ) {
             my $sth = C4::Context->dbh->prepare(
@@ -2010,7 +2012,7 @@ sub AddReturn {
     #adding message if holdingbranch is non equal a userenv branch to return the document to homebranch
     #we check, if we don't have reserv or transfert for this document, if not, return it to homebranch .
 
-    if (($doreturn or $messages->{'NotIssued'}) and !$resfound and ($branch ne $hbr) and not $messages->{'WrongTransfer'}){
+    if ( !$is_in_rotating_collection && ($doreturn or $messages->{'NotIssued'}) and !$resfound and ($branch ne $hbr) and not $messages->{'WrongTransfer'}){
         if ( C4::Context->preference("AutomaticItemReturn"    ) or
             (C4::Context->preference("UseBranchTransferLimits") and
              ! IsBranchTransferAllowed($branch, $hbr, $item->{C4::Context->preference("BranchTransferLimitsType")} )
@@ -2023,6 +2025,7 @@ sub AddReturn {
             $messages->{'NeedsTransfer'} = 1;   # TODO: instead of 1, specify branchcode that the transfer SHOULD go to, $item->{homebranch}
         }
     }
+
     return ( $doreturn, $messages, $issue, $borrower );
 }
 
diff --git a/C4/RotatingCollections.pm b/C4/RotatingCollections.pm
index d6fad87b37..5a63cd8906 100644
--- a/C4/RotatingCollections.pm
+++ b/C4/RotatingCollections.pm
@@ -1,9 +1,9 @@
 package C4::RotatingCollections;
 
-# $Id: RotatingCollections.pm,v 0.1 2007/04/20 kylemhall 
+# $Id: RotatingCollections.pm,v 0.1 2007/04/20 kylemhall
 
 # This package is inteded to keep track of what library
-# Items of a certain collection should be at. 
+# Items of a certain collection should be at.
 
 # Copyright 2007 Kyle Hall
 #
@@ -22,13 +22,11 @@ package C4::RotatingCollections;
 # with Koha; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
-use strict;
-#use warnings; FIXME - Bug 2505
-
-require Exporter;
+use Modern::Perl;
 
 use C4::Context;
 use C4::Circulation;
+use C4::Reserves qw(GetReserveStatus);
 
 use DBI;
 
@@ -47,23 +45,26 @@ C4::RotatingCollections - Functions for managing rotating collections
 
 =cut
 
-@ISA = qw( Exporter );
-@EXPORT = qw( 
-  CreateCollection
-  UpdateCollection
-  DeleteCollection
-  
-  GetItemsInCollection
+BEGIN {
+    require Exporter;
+    @ISA    = qw( Exporter );
+    @EXPORT = qw(
+      CreateCollection
+      UpdateCollection
+      DeleteCollection
+
+      GetItemsInCollection
+
+      GetCollection
+      GetCollections
 
-  GetCollection
-  GetCollections
-  
-  AddItemToCollection
-  RemoveItemFromCollection
-  TransferCollection  
+      AddItemToCollection
+      RemoveItemFromCollection
+      TransferCollection
 
-  GetCollectionItemBranches
-);
+      GetCollectionItemBranches
+    );
+}
 
 =head2  CreateCollection
  ( $success, $errorcode, $errormessage ) = CreateCollection( $title, $description );
@@ -81,27 +82,29 @@ C4::RotatingCollections - Functions for managing rotating collections
 =cut
 
 sub CreateCollection {
-  my ( $title, $description ) = @_;
+    my ( $title, $description ) = @_;
 
-  ## Check for all neccessary parameters
-  if ( ! $title ) {
-    return ( 0, 1, "No Title Given" );
-  } 
-  if ( ! $description ) {
-    return ( 0, 2, "No Description Given" );
-  } 
+    ## Check for all neccessary parameters
+    if ( !$title ) {
+        return ( 0, 1, "No Title Given" );
+    }
+    if ( !$description ) {
+        return ( 0, 2, "No Description Given" );
+    }
 
-  my $success = 1;
+    my $success = 1;
 
-  my $dbh = C4::Context->dbh;
+    my $dbh = C4::Context->dbh;
 
-  my $sth;
-  $sth = $dbh->prepare("INSERT INTO collections ( colId, colTitle, colDesc ) 
-                        VALUES ( NULL, ?, ? )");
-  $sth->execute( $title, $description ) or return ( 0, 3, $sth->errstr() );
+    my $sth;
+    $sth = $dbh->prepare(
+        "INSERT INTO collections ( colId, colTitle, colDesc )
+                        VALUES ( NULL, ?, ? )"
+    );
+    $sth->execute( $title, $description ) or return ( 0, 3, $sth->errstr() );
+
+    return 1;
 
-  return 1;
- 
 }
 
 =head2 UpdateCollection
@@ -123,30 +126,33 @@ Updates a collection
 =cut
 
 sub UpdateCollection {
-  my ( $colId, $title, $description ) = @_;
-
-  ## Check for all neccessary parameters
-  if ( ! $colId ) {
-    return ( 0, 1, "No Id Given" );
-  }
-  if ( ! $title ) {
-    return ( 0, 2, "No Title Given" );
-  } 
-  if ( ! $description ) {
-    return ( 0, 3, "No Description Given" );
-  } 
-
-  my $dbh = C4::Context->dbh;
-
-  my $sth;
-  $sth = $dbh->prepare("UPDATE collections
+    my ( $colId, $title, $description ) = @_;
+
+    ## Check for all neccessary parameters
+    if ( !$colId ) {
+        return ( 0, 1, "No Id Given" );
+    }
+    if ( !$title ) {
+        return ( 0, 2, "No Title Given" );
+    }
+    if ( !$description ) {
+        return ( 0, 3, "No Description Given" );
+    }
+
+    my $dbh = C4::Context->dbh;
+
+    my $sth;
+    $sth = $dbh->prepare(
+        "UPDATE collections
                         SET 
                         colTitle = ?, colDesc = ? 
-                        WHERE colId = ?");
-  $sth->execute( $title, $description, $colId ) or return ( 0, 4, $sth->errstr() );
-  
-  return 1;
-  
+                        WHERE colId = ?"
+    );
+    $sth->execute( $title, $description, $colId )
+      or return ( 0, 4, $sth->errstr() );
+
+    return 1;
+
 }
 
 =head2 DeleteCollection
@@ -165,21 +171,21 @@ sub UpdateCollection {
 =cut
 
 sub DeleteCollection {
-  my ( $colId ) = @_;
+    my ($colId) = @_;
+
+    ## Paramter check
+    if ( !$colId ) {
+        return ( 0, 1, "No Collection Id Given" );
+    }
 
-  ## Paramter check
-  if ( ! $colId ) {
-    return ( 0, 1, "No Collection Id Given" );;
-  }
-  
-  my $dbh = C4::Context->dbh;
+    my $dbh = C4::Context->dbh;
 
-  my $sth;
+    my $sth;
 
-  $sth = $dbh->prepare("DELETE FROM collections WHERE colId = ?");
-  $sth->execute( $colId ) or return ( 0, 4, $sth->errstr() );
+    $sth = $dbh->prepare("DELETE FROM collections WHERE colId = ?");
+    $sth->execute($colId) or return ( 0, 4, $sth->errstr() );
 
-  return 1;
+    return 1;
 }
 
 =head2 GetCollections
@@ -198,17 +204,17 @@ sub DeleteCollection {
 
 sub GetCollections {
 
-  my $dbh = C4::Context->dbh;
-  
-  my $sth = $dbh->prepare("SELECT * FROM collections");
-  $sth->execute() or return ( 1, $sth->errstr() );
-  
-  my @results;
-  while ( my $row = $sth->fetchrow_hashref ) {
-    push( @results , $row );
-  }
-  
-  return \@results;
+    my $dbh = C4::Context->dbh;
+
+    my $sth = $dbh->prepare("SELECT * FROM collections");
+    $sth->execute() or return ( 1, $sth->errstr() );
+
+    my @results;
+    while ( my $row = $sth->fetchrow_hashref ) {
+        push( @results, $row );
+    }
+
+    return \@results;
 }
 
 =head2 GetItemsInCollection
@@ -229,16 +235,17 @@ sub GetCollections {
 =cut
 
 sub GetItemsInCollection {
-  my ( $colId ) = @_;
+    my ($colId) = @_;
+
+    ## Paramter check
+    if ( !$colId ) {
+        return ( 0, 0, 1, "No Collection Id Given" );
+    }
 
-  ## Paramter check
-  if ( ! $colId ) {
-    return ( 0, 0, 1, "No Collection Id Given" );;
-  }
+    my $dbh = C4::Context->dbh;
 
-  my $dbh = C4::Context->dbh;
-  
-  my $sth = $dbh->prepare("SELECT 
+    my $sth = $dbh->prepare(
+        "SELECT
                              biblio.title,
                              items.itemcallnumber,
                              items.barcode
@@ -246,15 +253,16 @@ sub GetItemsInCollection {
                            WHERE collections.colId = collections_tracking.colId
                            AND collections_tracking.itemnumber = items.itemnumber
                            AND items.biblionumber = biblio.biblionumber
-                           AND collections.colId = ? ORDER BY biblio.title");
-  $sth->execute( $colId ) or return ( 0, 0, 2, $sth->errstr() );
-  
-  my @results;
-  while ( my $row = $sth->fetchrow_hashref ) {
-    push( @results , $row );
-  }
-  
-  return \@results;
+                           AND collections.colId = ? ORDER BY biblio.title"
+    );
+    $sth->execute($colId) or return ( 0, 0, 2, $sth->errstr() );
+
+    my @results;
+    while ( my $row = $sth->fetchrow_hashref ) {
+        push( @results, $row );
+    }
+
+    return \@results;
 }
 
 =head2 GetCollection
@@ -271,23 +279,21 @@ Returns information about a collection
 =cut
 
 sub GetCollection {
-  my ( $colId ) = @_;
-
-  my $dbh = C4::Context->dbh;
-
-  my ( $sth, @results );
-  $sth = $dbh->prepare("SELECT * FROM collections WHERE colId = ?");
-  $sth->execute( $colId ) or return 0;
-    
-  my $row = $sth->fetchrow_hashref;
-  
-  return (
-      $$row{'colId'},
-      $$row{'colTitle'},
-      $$row{'colDesc'},
-      $$row{'colBranchcode'}
-  );
-    
+    my ($colId) = @_;
+
+    my $dbh = C4::Context->dbh;
+
+    my ( $sth, @results );
+    $sth = $dbh->prepare("SELECT * FROM collections WHERE colId = ?");
+    $sth->execute($colId) or return 0;
+
+    my $row = $sth->fetchrow_hashref;
+
+    return (
+        $$row{'colId'},   $$row{'colTitle'},
+        $$row{'colDesc'}, $$row{'colBranchcode'}
+    );
+
 }
 
 =head2 AddItemToCollection
@@ -307,31 +313,36 @@ Adds an item to a rotating collection.
 =cut
 
 sub AddItemToCollection {
-  my ( $colId, $itemnumber ) = @_;
-
-  ## Check for all neccessary parameters
-  if ( ! $colId ) {
-    return ( 0, 1, "No Collection Given" );
-  } 
-  if ( ! $itemnumber ) {
-    return ( 0, 2, "No Itemnumber Given" );
-  } 
-  
-  if ( isItemInThisCollection( $itemnumber, $colId ) ) {
-    return ( 0, 2, "Item is already in the collection!" );
-  } elsif ( isItemInAnyCollection( $itemnumber ) ) {
-    return ( 0, 3, "Item is already in a different collection!" );
-  }
-
-  my $dbh = C4::Context->dbh;
-
-  my $sth;
-  $sth = $dbh->prepare("INSERT INTO collections_tracking ( collections_tracking_id, colId, itemnumber )
-                        VALUES ( NULL, ?, ? )");
-  $sth->execute( $colId, $itemnumber ) or return ( 0, 3, $sth->errstr() );
-
-  return 1;
-  
+    my ( $colId, $itemnumber ) = @_;
+
+    ## Check for all neccessary parameters
+    if ( !$colId ) {
+        return ( 0, 1, "No Collection Given" );
+    }
+    if ( !$itemnumber ) {
+        return ( 0, 2, "No Itemnumber Given" );
+    }
+
+    if ( isItemInThisCollection( $itemnumber, $colId ) ) {
+        return ( 0, 2, "Item is already in the collection!" );
+    }
+    elsif ( isItemInAnyCollection($itemnumber) ) {
+        return ( 0, 3, "Item is already in a different collection!" );
+    }
+
+    my $dbh = C4::Context->dbh;
+
+    my $sth;
+    $sth = $dbh->prepare("
+        INSERT INTO collections_tracking (
+            colId,
+            itemnumber
+        ) VALUES ( ?, ? )
+    ");
+    $sth->execute( $colId, $itemnumber ) or return ( 0, 3, $sth->errstr() );
+
+    return 1;
+
 }
 
 =head2  RemoveItemFromCollection
@@ -352,25 +363,27 @@ Removes an item to a collection
 =cut
 
 sub RemoveItemFromCollection {
-  my ( $colId, $itemnumber ) = @_;
+    my ( $colId, $itemnumber ) = @_;
+
+    ## Check for all neccessary parameters
+    if ( !$itemnumber ) {
+        return ( 0, 2, "No Itemnumber Given" );
+    }
 
-  ## Check for all neccessary parameters
-  if ( ! $itemnumber ) {
-    return ( 0, 2, "No Itemnumber Given" );
-  } 
-  
-  if ( ! isItemInThisCollection( $itemnumber, $colId ) ) {
-    return ( 0, 2, "Item is not in the collection!" );
-  } 
+    if ( !isItemInThisCollection( $itemnumber, $colId ) ) {
+        return ( 0, 2, "Item is not in the collection!" );
+    }
 
-  my $dbh = C4::Context->dbh;
+    my $dbh = C4::Context->dbh;
 
-  my $sth;
-  $sth = $dbh->prepare("DELETE FROM collections_tracking 
-                        WHERE itemnumber = ?");
-  $sth->execute( $itemnumber ) or return ( 0, 3, $sth->errstr() );
+    my $sth;
+    $sth = $dbh->prepare(
+        "DELETE FROM collections_tracking
+                        WHERE itemnumber = ?"
+    );
+    $sth->execute($itemnumber) or return ( 0, 3, $sth->errstr() );
 
-  return 1;
+    return 1;
 }
 
 =head2 TransferCollection
@@ -391,36 +404,44 @@ Transfers a collection to another branch
 =cut
 
 sub TransferCollection {
-  my ( $colId, $colBranchcode ) = @_;
+    my ( $colId, $colBranchcode ) = @_;
 
-  ## Check for all neccessary parameters
-  if ( ! $colId ) {
-    return ( 0, 1, "No Id Given" );
-  }
-  if ( ! $colBranchcode ) {
-    return ( 0, 2, "No Branchcode Given" );
-  } 
+    ## Check for all neccessary parameters
+    if ( !$colId ) {
+        return ( 0, 1, "No Id Given" );
+    }
+    if ( !$colBranchcode ) {
+        return ( 0, 2, "No Branchcode Given" );
+    }
 
-  my $dbh = C4::Context->dbh;
+    my $dbh = C4::Context->dbh;
 
-  my $sth;
-  $sth = $dbh->prepare("UPDATE collections
+    my $sth;
+    $sth = $dbh->prepare(
+        "UPDATE collections
                         SET 
                         colBranchcode = ? 
-                        WHERE colId = ?");
-  $sth->execute( $colBranchcode, $colId ) or return ( 0, 4, $sth->errstr() );
-  
-  $sth = $dbh->prepare("SELECT barcode FROM items, collections_tracking 
-                        WHERE items.itemnumber = collections_tracking.itemnumber
-                        AND collections_tracking.colId = ?");
-  $sth->execute( $colId ) or return ( 0, 4, $sth->errstr );
-  my @results;
-  while ( my $item = $sth->fetchrow_hashref ) {
-    my ( $dotransfer, $messages, $iteminformation ) = transferbook( $colBranchcode, $item->{'barcode'}, my $ignore_reserves = 1);
-  }
-
-  return 1;
-  
+                        WHERE colId = ?"
+    );
+    $sth->execute( $colBranchcode, $colId ) or return ( 0, 4, $sth->errstr() );
+
+    $sth = $dbh->prepare(q{
+        SELECT items.itemnumber, items.barcode FROM collections_tracking
+        LEFT JOIN items ON collections_tracking.itemnumber = items.itemnumber
+        LEFT JOIN issues ON items.itemnumber = issues.itemnumber
+        WHERE issues.borrowernumber IS NULL
+          AND collections_tracking.colId = ?
+    });
+    $sth->execute($colId) or return ( 0, 4, $sth->errstr );
+    my @results;
+    while ( my $item = $sth->fetchrow_hashref ) {
+        transferbook( $colBranchcode, $item->{barcode},
+            my $ignore_reserves = 1 )
+          unless ( GetReserveStatus( $item->{itemnumber} ) eq "Waiting" );
+    }
+
+    return 1;
+
 }
 
 =head2 GetCollectionItemBranches
@@ -430,27 +451,26 @@ sub TransferCollection {
 =cut
 
 sub GetCollectionItemBranches {
-  my ( $itemnumber ) = @_;
+    my ($itemnumber) = @_;
 
-  if ( ! $itemnumber ) {
-    return;
-  }
+    if ( !$itemnumber ) {
+        return;
+    }
 
-  my $dbh = C4::Context->dbh;
+    my $dbh = C4::Context->dbh;
 
-  my ( $sth, @results );
-  $sth = $dbh->prepare("SELECT holdingbranch, colBranchcode FROM items, collections, collections_tracking 
+    my ( $sth, @results );
+    $sth = $dbh->prepare(
+"SELECT holdingbranch, colBranchcode FROM items, collections, collections_tracking
                         WHERE items.itemnumber = collections_tracking.itemnumber
                         AND collections.colId = collections_tracking.colId
-                        AND items.itemnumber = ?");
-  $sth->execute( $itemnumber );
-    
-  my $row = $sth->fetchrow_hashref;
-  
-  return (
-      $$row{'holdingbranch'},
-      $$row{'colBranchcode'},
-  );  
+                        AND items.itemnumber = ?"
+    );
+    $sth->execute($itemnumber);
+
+    my $row = $sth->fetchrow_hashref;
+
+    return ( $$row{'holdingbranch'}, $$row{'colBranchcode'}, );
 }
 
 =head2 isItemInThisCollection
@@ -460,16 +480,18 @@ sub GetCollectionItemBranches {
 =cut
 
 sub isItemInThisCollection {
-  my ( $itemnumber, $colId ) = @_;
-  
-  my $dbh = C4::Context->dbh;
-  
-  my $sth = $dbh->prepare("SELECT COUNT(*) as inCollection FROM collections_tracking WHERE itemnumber = ? AND colId = ?");
-  $sth->execute( $itemnumber, $colId ) or return( 0 );
-      
-  my $row = $sth->fetchrow_hashref;
-        
-  return $$row{'inCollection'};
+    my ( $itemnumber, $colId ) = @_;
+
+    my $dbh = C4::Context->dbh;
+
+    my $sth = $dbh->prepare(
+"SELECT COUNT(*) as inCollection FROM collections_tracking WHERE itemnumber = ? AND colId = ?"
+    );
+    $sth->execute( $itemnumber, $colId ) or return (0);
+
+    my $row = $sth->fetchrow_hashref;
+
+    return $$row{'inCollection'};
 }
 
 =head2 isItemInAnyCollection
@@ -479,21 +501,23 @@ $inCollection = isItemInAnyCollection( $itemnumber );
 =cut
 
 sub isItemInAnyCollection {
-  my ( $itemnumber ) = @_;
-  
-  my $dbh = C4::Context->dbh;
-  
-  my $sth = $dbh->prepare("SELECT itemnumber FROM collections_tracking WHERE itemnumber = ?");
-  $sth->execute( $itemnumber ) or return( 0 );
-      
-  my $row = $sth->fetchrow_hashref;
-        
-  $itemnumber = $row->{itemnumber};
-  if ( $itemnumber ) {
-    return 1;
-  } else {
-    return 0;
-  }
+    my ($itemnumber) = @_;
+
+    my $dbh = C4::Context->dbh;
+
+    my $sth = $dbh->prepare(
+        "SELECT itemnumber FROM collections_tracking WHERE itemnumber = ?");
+    $sth->execute($itemnumber) or return (0);
+
+    my $row = $sth->fetchrow_hashref;
+
+    $itemnumber = $row->{itemnumber};
+    if ($itemnumber) {
+        return 1;
+    }
+    else {
+        return 0;
+    }
 }
 
 1;
diff --git a/circ/returns.pl b/circ/returns.pl
index d99f58c0e1..8a17a10258 100755
--- a/circ/returns.pl
+++ b/circ/returns.pl
@@ -600,20 +600,16 @@ $template->param(
     BlockReturnOfWithdrawnItems => C4::Context->preference("BlockReturnOfWithdrawnItems"),
 );
 
-### Comment out rotating collections for now to allow it a little more time to bake
-### for 3.4; in particular, must ensure that it doesn't fight with transfers required
-### to fill hold requests
-### -- Galen Charlton 2010-10-06
-#my $itemnumber = GetItemnumberFromBarcode( $query->param('barcode') );
-#if ( $itemnumber ) {
-#   my ( $holdingBranch, $collectionBranch ) = GetCollectionItemBranches( $itemnumber );
-#    if ( ! ( $holdingBranch eq $collectionBranch ) ) {
-#        $template->param(
-#          collectionItemNeedsTransferred => 1,
-#          collectionBranch => GetBranchName($collectionBranch),
-#        );
-#    }
-#}                                                                                                            
+my $itemnumber = GetItemnumberFromBarcode( $query->param('barcode') );
+if ( $itemnumber ) {
+   my ( $holdingBranch, $collectionBranch ) = GetCollectionItemBranches( $itemnumber );
+    if ( ! ( $holdingBranch eq $collectionBranch ) ) {
+        $template->param(
+          collectionItemNeedsTransferred => 1,
+          collectionBranch => GetBranchName($collectionBranch),
+        );
+    }
+}
 
 # actually print the page!
 output_html_with_http_headers $query, $cookie, $template->output;
diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
index 12fb181f42..8c777f72f0 100644
--- a/installer/data/mysql/kohastructure.sql
+++ b/installer/data/mysql/kohastructure.sql
@@ -482,10 +482,16 @@ CREATE TABLE collections (
   colId integer(11) NOT NULL auto_increment,
   colTitle varchar(100) NOT NULL DEFAULT '',
   colDesc text NOT NULL,
-  colBranchcode varchar(4) DEFAULT NULL comment 'branchcode for branch where item should be held.',
+  colBranchcode varchar(10) DEFAULT NULL, -- 'branchcode for branch where item should be held.'
   PRIMARY KEY (colId)
 ) ENGINE=InnoDB DEFAULT CHARACTER SET utf8;
 
+--
+-- Constraints for table `collections`
+--
+ALTER TABLE `collections`
+  ADD CONSTRAINT `collections_ibfk_1` FOREIGN KEY (`colBranchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE;
+
 --
 -- Table: collections_tracking
 --
diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
index 7b7255dfbc..1f8cdc4ee0 100755
--- a/installer/data/mysql/updatedatabase.pl
+++ b/installer/data/mysql/updatedatabase.pl
@@ -2741,7 +2741,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
 $DBversion = '3.01.00.066';
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do('ALTER TABLE issuingrules ADD COLUMN `reservesallowed` smallint(6) NOT NULL default "0" AFTER `renewalsallowed`;');
-    
+
     my $maxreserves = C4::Context->preference('maxreserves');
     $sth = $dbh->prepare('UPDATE issuingrules SET reservesallowed = ?;');
     $sth->execute($maxreserves);
@@ -2948,7 +2948,7 @@ $$maxbudgetid[0] = 0 if !$$maxbudgetid[0];
     $dbh->do(<<BUDGETAUTOINCREMENT);
 ALTER TABLE aqbudget AUTO_INCREMENT=$$maxbudgetid[0]
 BUDGETAUTOINCREMENT
-    
+
     $dbh->do(<<BUDGETNAME);
 ALTER TABLE aqbudget RENAME `aqbudgets`
 BUDGETNAME
@@ -3326,10 +3326,10 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
 $DBversion = "3.01.00.101";
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
 	$dbh->do(
-        "INSERT INTO systempreferences 
+        "INSERT INTO systempreferences
            (variable, value, options, explanation, type)
          VALUES (
-            'OverdueNoticeBcc', '', '', 
+            'OverdueNoticeBcc', '', '',
             'Email address to Bcc outgoing notices sent by email',
             'free')
          ");
@@ -3364,7 +3364,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     if ($borrnotes_count == 0) {
         $dbh->do("INSERT INTO authorised_values (category,authorised_value,lib) VALUES ('BOR_NOTES','ADDR','Address Notes')");
     }
-    
+
     $dbh->do("INSERT INTO authorised_values (category,authorised_value,lib) VALUES ('LOC','CART','Book Cart')");
     $dbh->do("INSERT INTO authorised_values (category,authorised_value,lib) VALUES ('LOC','PROC','Processing Center')");
 
@@ -3384,7 +3384,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
         PRIMARY KEY  (`colId`)
       ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
     ");
-       
+
     $dbh->do("
       CREATE TABLE `collections_tracking` (
         `ctId` int(11) NOT NULL auto_increment,
@@ -3394,7 +3394,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
       ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
     ");
     $dbh->do("
-        INSERT INTO permissions (module_bit, code, description) 
+        INSERT INTO permissions (module_bit, code, description)
         VALUES ( 13, 'rotating_collections', 'Manage Rotating collections')" );
 	print "Upgrade to $DBversion done (added collection and collection_tracking tables for rotating collections functionality)\n";
     SetVersion ($DBversion);
@@ -3419,7 +3419,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
 	$dbh->do(qq{
 	ALTER TABLE `export_format` ADD `csv_separator` VARCHAR( 2 ) NOT NULL AFTER `marcfields` ,
 	ADD `field_separator` VARCHAR( 2 ) NOT NULL AFTER `csv_separator` ,
-	ADD `subfield_separator` VARCHAR( 2 ) NOT NULL AFTER `field_separator` 
+	ADD `subfield_separator` VARCHAR( 2 ) NOT NULL AFTER `field_separator`
 	});
 	print "Upgrade to $DBversion done (added separators for csv export)\n";
     SetVersion ($DBversion);
@@ -3592,7 +3592,7 @@ $DBversion = "3.01.00.126";
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
 	$dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('ILS-DI','0','Enable ILS-DI services. See http://your.opac.name/cgi-bin/koha/ilsdi.pl for online documentation.','','YesNo')");
 	$dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('ILS-DI:AuthorizedIPs','127.0.0.1','A comma separated list of IP addresses authorized to access the web services.','','free')");
-	
+
     print "Upgrade to $DBversion done (Adding ILS-DI updates and ILS-DI:AuthorizedIPs)\n";
     SetVersion ($DBversion);
 }
@@ -3819,21 +3819,21 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion) && $original
     $dbh->do("ALTER TABLE subscriptionroutinglist MODIFY COLUMN `borrowernumber` int(11) NOT NULL;");
     $dbh->do("DELETE FROM subscriptionroutinglist WHERE subscriptionid IS NULL;");
     $dbh->do("ALTER TABLE subscriptionroutinglist MODIFY COLUMN `subscriptionid` int(11) NOT NULL;");
-    $dbh->do("CREATE TEMPORARY TABLE del_subscriptionroutinglist 
+    $dbh->do("CREATE TEMPORARY TABLE del_subscriptionroutinglist
               SELECT s1.routingid FROM subscriptionroutinglist s1
               WHERE EXISTS (SELECT * FROM subscriptionroutinglist s2
                             WHERE s2.borrowernumber = s1.borrowernumber
-                            AND   s2.subscriptionid = s1.subscriptionid 
+                            AND   s2.subscriptionid = s1.subscriptionid
                             AND   s2.routingid < s1.routingid);");
     $dbh->do("DELETE FROM subscriptionroutinglist
               WHERE routingid IN (SELECT routingid FROM del_subscriptionroutinglist);");
     $dbh->do("ALTER TABLE subscriptionroutinglist ADD UNIQUE (subscriptionid, borrowernumber);");
-    $dbh->do("ALTER TABLE subscriptionroutinglist 
-                ADD CONSTRAINT `subscriptionroutinglist_ibfk_1` FOREIGN KEY (`borrowernumber`) 
+    $dbh->do("ALTER TABLE subscriptionroutinglist
+                ADD CONSTRAINT `subscriptionroutinglist_ibfk_1` FOREIGN KEY (`borrowernumber`)
                 REFERENCES `borrowers` (`borrowernumber`)
                 ON DELETE CASCADE ON UPDATE CASCADE");
-    $dbh->do("ALTER TABLE subscriptionroutinglist 
-                ADD CONSTRAINT `subscriptionroutinglist_ibfk_2` FOREIGN KEY (`subscriptionid`) 
+    $dbh->do("ALTER TABLE subscriptionroutinglist
+                ADD CONSTRAINT `subscriptionroutinglist_ibfk_2` FOREIGN KEY (`subscriptionid`)
                 REFERENCES `subscription` (`subscriptionid`)
                 ON DELETE CASCADE ON UPDATE CASCADE");
     print "Upgrade to $DBversion done (Make subscriptionroutinglist more strict)\n";
@@ -4102,7 +4102,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion) && $original
     SetVersion ($DBversion);
 }
 
-$DBversion = '3.03.00.027'; 
+$DBversion = '3.03.00.027';
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('displayFacetCount', '0', NULL, NULL, 'YesNo')");
     $dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('maxRecordsForFacets', '20', NULL, NULL, 'Integer')");
@@ -4136,7 +4136,7 @@ $DBversion = "3.03.00.031";
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('FineNotifyAtCheckin',0,'If ON notify librarians of overdue fines on the items they are checking in.',NULL,'YesNo');");
     print "Upgrade to $DBversion done (Add syspref FineNotifyAtCheckin)\n";
-    SetVersion ($DBversion);    
+    SetVersion ($DBversion);
 }
 
 $DBversion = '3.03.00.032';
@@ -4205,7 +4205,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('ShowReviewer',1,'If ON, name of reviewer will be shown above comments in OPAC',NULL,'YesNo');");
     print "Upgrade to $DBversion done (Add syspref ShowReviewer)\n";
 }
-    
+
 $DBversion = "3.03.00.040";
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('UseControlNumber',0,'If ON, record control number (w subfields) and control number (001) are used for linking of bibliographic records.','','YesNo');");
@@ -4325,7 +4325,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do("UPDATE message_attributes SET message_name = 'Advance_Notice' WHERE message_name='Advance Notice'");
     $dbh->do("UPDATE message_attributes SET message_name = 'Hold_Filled' WHERE message_name='Hold Filled'");
     $dbh->do("UPDATE message_attributes SET message_name = 'Item_Check_in' WHERE message_name='Item Check-in'");
-    $dbh->do("UPDATE message_attributes SET message_name = 'Item_Checkout' WHERE message_name='Item Checkout'");    
+    $dbh->do("UPDATE message_attributes SET message_name = 'Item_Checkout' WHERE message_name='Item Checkout'");
     SetVersion ($DBversion);
 }
 
@@ -4372,9 +4372,9 @@ $DBversion = "3.05.00.004";
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('ShowReviewerPhoto',1,'If ON, photo of reviewer will be shown beside comments in OPAC',NULL,'YesNo');");
     print "Upgrade to $DBversion done (Add syspref ShowReviewerPhoto)\n";
-    SetVersion($DBversion);    
+    SetVersion($DBversion);
 }
-    
+
 $DBversion = "3.05.00.005";
 if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
     $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES ('BasketConfirmations', '1', 'When closing or reopening a basket,', 'always ask for confirmation.|do not ask for confirmation.', 'Choice');");
@@ -4382,7 +4382,7 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
     SetVersion($DBversion);
 }
 
-$DBversion = "3.05.00.006"; 
+$DBversion = "3.05.00.006";
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('MARCAuthorityControlField008', '|| aca||aabn           | a|a     d', NULL, NULL, 'Textarea')");
     print "Upgrade to $DBversion done (Add syspref MARCAuthorityControlField008)\n";
@@ -4451,7 +4451,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     print "Upgrade to $DBversion done (add OPACResultsSidebar syspref (enh 6165))\n";
     SetVersion($DBversion);
 }
-    
+
 $DBversion = "3.05.00.012";
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('RecordLocalUseOnReturn',0,'If ON, statistically record returns of unissued items as local use, instead of return',NULL,'YesNo')");
@@ -4532,11 +4532,11 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
 $DBversion = "3.05.00.021";
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do("ALTER TABLE borrower_attribute_types ADD COLUMN display_checkout TINYINT(1) NOT NULL DEFAULT '0';");
-    print "Upgrade to $DBversion done (Added a display_checkout field in borrower_attribute_types table)\n"; 
+    print "Upgrade to $DBversion done (Added a display_checkout field in borrower_attribute_types table)\n";
     SetVersion($DBversion);
 }
 
-$DBversion = "3.05.00.022"; 
+$DBversion = "3.05.00.022";
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     $dbh->do("CREATE TABLE need_merge_authorities (id int NOT NULL auto_increment PRIMARY KEY, authid bigint NOT NULL, done tinyint DEFAULT 0) ENGINE=InnoDB DEFAULT CHARSET=utf8");
     print "Upgrade to $DBversion done (6094: Fixing ModAuthority problems, add a need_merge_authorities table)\n";
@@ -5349,24 +5349,24 @@ $DBversion ="3.09.00.014";
 if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
     # add phone message transport type
     $dbh->do("INSERT INTO message_transport_types (message_transport_type) VALUES ('phone')");
-    
+
     # adds HOLD_PHONE and PREDUE_PHONE letters (as placeholders)
     $dbh->do("INSERT INTO letter (module, code, name, title, content) VALUES
               ('reserves', 'HOLD_PHONE', 'Item Available for Pick-up (phone notice)', 'Item Available for Pick-up (phone notice)', 'Your item is available for pickup'),
               ('circulation', 'PREDUE_PHONE', 'Advance Notice of Item Due (phone notice)', 'Advance Notice of Item Due (phone notice)', 'Your item is due soon'),
               ('circulation', 'OVERDUE_PHONE', 'Overdue Notice (phone notice)', 'Overdue Notice (phone notice)', 'Your item is overdue')
               ");
-    
+
     # add phone notifications to patron message preferences options
     $dbh->do("INSERT INTO message_transports
              (message_attribute_id, message_transport_type, is_digest, letter_module, letter_code) VALUES
              (4, 'phone', 0, 'reserves', 'HOLD_PHONE'),
              (2, 'phone', 0, 'circulation', 'PREDUE_PHONE')
              ");
-    
+
     # add TalkingTechItivaPhoneNotification syspref
     $dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('TalkingTechItivaPhoneNotification',0,'If ON, enables Talking Tech I-tiva phone notifications',NULL,'YesNo');");
-    
+
     print "Upgrade done (Support for Talking Tech i-tiva phone notification system)\n";
     SetVersion($DBversion);
 }
@@ -5948,7 +5948,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
                 ADD COLUMN record_type enum('biblio', 'auth', 'holdings') NOT NULL default 'biblio'");
     $dbh->do("UPDATE import_batches SET record_type='auth' WHERE import_batch_id IN
                 (SELECT import_batch_id FROM import_records WHERE record_type='auth')");
-    
+
     print "Upgrade to $DBversion done (Added support for staging authorities)\n";
     SetVersion ($DBversion);
 }
@@ -7204,9 +7204,9 @@ if ( CheckVersion($DBversion) ) {
 $DBversion = "3.13.00.029";
 if ( CheckVersion($DBversion) ) {
     $dbh->do(q{
-        INSERT IGNORE INTO export_format( profile, description, content, csv_separator, type ) 
+        INSERT IGNORE INTO export_format( profile, description, content, csv_separator, type )
         VALUES ( "issues to claim", "efault CSV export for serial issue claims",
-                "SUPPLIER=aqbooksellers.name|TITLE=subscription.title|ISSUE NUMBER=serial.serialseq|LATE SINCE=serial.planneddate", 
+                "SUPPLIER=aqbooksellers.name|TITLE=subscription.title|ISSUE NUMBER=serial.serialseq|LATE SINCE=serial.planneddate",
                 ",", "sql" )
     });
     print "Upgrade to $DBversion done (Bug 10854: Add the default CSV profile for claiming issues)\n";
@@ -7317,7 +7317,7 @@ if ( CheckVersion($DBversion) ) {
                 SELECT DISTINCT(parent_ordernumber)
                 FROM aqorders
                 WHERE ordernumber != parent_ordernumber
-            ) AS aq 
+            ) AS aq
         )
         AND basketno IN (SELECT basketno FROM aqbasket WHERE closedate IS NOT NULL)
     });
@@ -9005,6 +9005,84 @@ if ( CheckVersion($DBversion) ) {
     SetVersion($DBversion);
 }
 
+$DBversion = "3.17.00.036";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do("DELETE FROM systempreferences WHERE variable='OpacShowLibrariesPulldownMobile'");
+    print "Upgrade to $DBversion done ( Bug 12513 - PROG/CCSR deprecation: Remove OpacShowLibrariesPulldownMobile system preference )\n";
+    SetVersion ($DBversion);
+}
+
+$DBversion = "3.17.00.037";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do("DELETE FROM systempreferences WHERE variable='OpacMainUserBlockMobile'");
+    print "Upgrade to $DBversion done ( Bug 12246 - PROG/CCSR deprecation: Remove OpacMainUserBlockMobile system preference )\n";
+    SetVersion ($DBversion);
+}
+
+$DBversion = "3.17.00.038";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do("DELETE FROM systempreferences WHERE variable='OPACMobileUserCSS'");
+    print "Upgrade to $DBversion done ( Bug 12245 - PROG/CCSR deprecation: Remove OPACMobileUserCSS system preference )\n";
+    SetVersion ($DBversion);
+}
+
+$DBversion = "3.17.00.039";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do("INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) VALUES
+    ('OPACFallback', 'prog', 'bootstrap|prog', 'Define the fallback theme for the OPAC interface.', 'Themes')");
+    print "Upgrade to $DBversion done (Bug 12539 - PROG/CCSR deprecation: Remove hardcoded theme from C4/Templates.pm)\n";
+    SetVersion ($DBversion);
+}
+
+$DBversion = "3.17.00.040";
+if ( CheckVersion($DBversion) ) {
+    my $opac_theme = C4::Context->preference( 'opacthemes' );
+    if ( $opac_theme eq 'prog' || $opac_theme eq 'ccsr' ) {
+        $dbh->do("UPDATE systempreferences SET value='bootstrap' WHERE variable='opacthemes'");
+    }
+    print "Upgrade to $DBversion done (Bug 12223: 'prog' and 'ccsr' themes removed)\n";
+    SetVersion($DBversion);
+}
+
+$DBversion = "3.17.00.041";
+if ( CheckVersion($DBversion) ) {
+    print "Upgrade to $DBversion done (Bug 11346: Deprecate the 'prog' and 'CCSR' themes)\n";
+    SetVersion($DBversion);
+}
+
+$DBversion = "3.17.00.042";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do("DELETE FROM systempreferences WHERE variable='yuipath'");
+    print "Upgrade to $DBversion done (Bug 12494: Remove yuipath system preference)\n";
+    SetVersion ($DBversion);
+}
+
+$DBversion = "3.17.00.043";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do("
+        ALTER TABLE aqorders
+        ADD COLUMN cancellationreason TEXT DEFAULT NULL AFTER datecancellationprinted
+    ");
+    print "Upgrade to $DBversion done (Bug 7162: Add aqorders.cancellationreason)\n";
+    SetVersion ($DBversion);
+}
+
+$DBversion = "3.17.00.XXX";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do(q{
+        ALTER TABLE collections CHANGE colBranchcode colBranchcode VARCHAR( 10 ) NULL DEFAULT NULL
+    });
+    $dbh->do(q{
+        ALTER TABLE collections ADD INDEX ( colBranchcode )
+    });
+    $dbh->do(q{
+        ALTER TABLE collections
+            ADD CONSTRAINT collections_ibfk_1 FOREIGN KEY (colBranchcode) REFERENCES branches (branchcode) ON DELETE CASCADE ON UPDATE CASCADE
+    });
+    print "Upgrade to $DBversion done (Bug 8836 - Resurrect Rotating Collections)\n";
+    SetVersion($DBversion);
+}
+
 =head1 FUNCTIONS
 
 =head2 TableExists($table)
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
index 068f71010e..8a12b8bf12 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
@@ -109,7 +109,7 @@ $(document).ready(function () {
 [% END %]
 
 [% IF ( collectionItemNeedsTransferred ) %]
-	<div id="rotating-collection" class="dialog message">This item is part of a Rotating Collection and needs to be Transferred to [% collectionBranch %]</div>
+ <div id="rotating-collection" class="dialog message">This item is part of a rotating collection and needs to be transferred to [% collectionBranch %]</div>
 [% END %]
 
 <!-- Patron has fines -->
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/addItems.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/addItems.tt
index 6ec945a324..195b324e5b 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/addItems.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/addItems.tt
@@ -1,6 +1,13 @@
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Tools &rsaquo; Rotating collections &rsaquo; Add/Remove items</title>
 [% INCLUDE 'doc-head-close.inc' %]
+<script type="text/javascript">
+    //<![CDATA[
+        $( document ).ready(function() {
+            $("#barcode").focus();
+        });
+    //]]>
+</script>
 </head>
 <body>
 [% INCLUDE 'header.inc' %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/editCollections.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/editCollections.tt
index 4917ff2873..e1bb2a885f 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/editCollections.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/editCollections.tt
@@ -44,13 +44,16 @@
       <div>
         [% IF ( collectionsLoop ) %]
           <table>
+           <thead>
             <tr>
               <th>Title</th>
               <th>Description</th>
               <th>Holding library</th>
-              <td></td>
-              <td></td>
+              <th>&nbsp;</th>
+              <th>&nbsp;</th>
             </tr>
+           <thead>
+           <tbody>
             [% FOREACH collectionsLoo IN collectionsLoop %]
               <tr>
                 <td>[% collectionsLoo.colTitle %]</td>
@@ -60,6 +63,7 @@
                 <td><a href="editCollections.pl?action=delete&amp;colId=[% collectionsLoo.colId %]">Delete</a></td>
               </tr>
             [% END %]
+           </tbody>
           </table>
         [% ELSE %]
           There are no collections currently defined.
@@ -99,7 +103,7 @@
                 <label for="description">Description: </label>
               </td>
               <td>
-                [% IF (editColDescription ) %]<input type="text" size="50" name="description" value="[ editColDescription %]" />
+                [% IF (editColDescription ) %]<input type="text" size="50" name="description" value="[% editColDescription %]" />
                 [% ELSE %]<input type="text" size="50" name="description" />[% END %]
               </td>
             </tr>
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt
index 4ab38c99c3..590cb47baf 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt
@@ -142,12 +142,10 @@
     <dd>Enter a barcode to generate a printable spine label. For use with dedicated label printers</dd>
     [% END %]
 
-<!--
     [% IF ( CAN_user_tools_rotating_collections ) %]
     <dt><a href="/cgi-bin/koha/rotating_collections/rotatingCollections.pl">Rotating collections</a></dt>
     <dd>Manage Rotating Collections</dd>
     [% END %]
--->
 
     [% IF ( CAN_user_tools_marc_modification_templates ) %]
     <dt><a href="/cgi-bin/koha/tools/marc_modification_templates.pl">MARC modification templates</a></dt>
diff --git a/rotating_collections/addItems.pl b/rotating_collections/addItems.pl
index 36eac4c64d..9ef4f39232 100755
--- a/rotating_collections/addItems.pl
+++ b/rotating_collections/addItems.pl
@@ -16,9 +16,7 @@
 # Suite 330, Boston, MA  02111-1307 USA
 #
 
-use strict;
-#use warnings; FIXME - Bug 2505
-require Exporter;
+use Modern::Perl;
 
 use C4::Output;
 use C4::Auth;
@@ -29,73 +27,83 @@ use C4::Items;
 use CGI;
 
 my $query = new CGI;
-my ($template, $loggedinuser, $cookie)
-    = get_template_and_user({template_name => "rotating_collections/addItems.tt",
-			     query => $query,
-			     type => "intranet",
-			     authnotrequired => 0,
-                          flagsrequired => { tools => 'rotating_collections' },
-			     debug => 1,
-			     });
 
-if ( $query->param('action') eq 'addItem' ) {
-  ## Add the given item to the collection
-  my $colId = $query->param('colId');
-  my $barcode = $query->param('barcode');
-  my $removeItem = $query->param('removeItem');
-  my $itemnumber = GetItemnumberFromBarcode( $barcode );
-
-  my ( $success, $errorCode, $errorMessage );
-  
-  if ( ! $removeItem ) {
-    ( $success, $errorCode, $errorMessage ) = AddItemToCollection( $colId, $itemnumber );
-
-    $template->param(
-      previousActionAdd => 1,
-      addedBarcode => $barcode,
-    );
-
-    if ( $success ) {
-      $template->param( addSuccess => 1 );
-    } else {
-      $template->param( addFailure => 1 );
-      $template->param( failureMessage => $errorMessage );
+my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
+    {
+        template_name   => "rotating_collections/addItems.tmpl",
+        query           => $query,
+        type            => "intranet",
+        authnotrequired => 0,
+        flagsrequired   => { tools => 'rotating_collections' },
+        debug           => 1,
     }
-  } else {
-    ## Remove the given item from the collection
-    ( $success, $errorCode, $errorMessage ) = RemoveItemFromCollection( $colId, $itemnumber );
-
-    $template->param(
-      previousActionRemove => 1,
-      removedBarcode => $barcode,
-      removeChecked => 1,
-    );
-
-    if ( $success ) {
-      $template->param( removeSuccess => 1 );
-    } else {
-      $template->param( removeFailure => 1 );
-      $template->param( failureMessage => $errorMessage );
+);
+
+if ( $query->param('action') eq 'addItem' ) {
+    ## Add the given item to the collection
+    my $colId      = $query->param('colId');
+    my $barcode    = $query->param('barcode');
+    my $removeItem = $query->param('removeItem');
+    my $itemnumber = GetItemnumberFromBarcode($barcode);
+
+    my ( $success, $errorCode, $errorMessage );
+
+    if ( !$removeItem ) {
+        ( $success, $errorCode, $errorMessage ) =
+          AddItemToCollection( $colId, $itemnumber );
+
+        $template->param(
+            previousActionAdd => 1,
+            addedBarcode      => $barcode,
+        );
+
+        if ($success) {
+            $template->param( addSuccess => 1 );
+        }
+        else {
+            $template->param( addFailure     => 1 );
+            $template->param( failureMessage => $errorMessage );
+        }
     }
+    else {
+        ## Remove the given item from the collection
+        ( $success, $errorCode, $errorMessage ) =
+          RemoveItemFromCollection( $colId, $itemnumber );
+
+        $template->param(
+            previousActionRemove => 1,
+            removedBarcode       => $barcode,
+            removeChecked        => 1,
+        );
+
+        if ($success) {
+            $template->param( removeSuccess => 1 );
+        }
+        else {
+            $template->param( removeFailure  => 1 );
+            $template->param( failureMessage => $errorMessage );
+        }
 
-  }  
+    }
 }
 
-my ( $colId, $colTitle, $colDescription, $colBranchcode ) = GetCollection( $query->param('colId') );
-my $collectionItems = GetItemsInCollection( $colId );
-if ( $collectionItems ) {
-  $template->param( collectionItemsLoop => $collectionItems );
+my ( $colId, $colTitle, $colDescription, $colBranchcode ) =
+  GetCollection( $query->param('colId') );
+my $collectionItems = GetItemsInCollection($colId);
+if ($collectionItems) {
+    $template->param( collectionItemsLoop => $collectionItems );
 }
 
 $template->param(
-                intranetcolorstylesheet => C4::Context->preference("intranetcolorstylesheet"),
-                intranetstylesheet => C4::Context->preference("intranetstylesheet"),
-                IntranetNav => C4::Context->preference("IntranetNav"),
-                                  
-                colId => $colId,
-                colTitle => $colTitle,
-                colDescription => $colDescription,
-                colBranchcode => $colBranchcode,
-                );
+    intranetcolorstylesheet =>
+      C4::Context->preference("intranetcolorstylesheet"),
+    intranetstylesheet => C4::Context->preference("intranetstylesheet"),
+    IntranetNav        => C4::Context->preference("IntranetNav"),
+
+    colId          => $colId,
+    colTitle       => $colTitle,
+    colDescription => $colDescription,
+    colBranchcode  => $colBranchcode,
+);
 
 output_html_with_http_headers $query, $cookie, $template->output;
diff --git a/rotating_collections/editCollections.pl b/rotating_collections/editCollections.pl
index 5b60047b5f..aa46f44d06 100755
--- a/rotating_collections/editCollections.pl
+++ b/rotating_collections/editCollections.pl
@@ -15,9 +15,8 @@
 # Koha; if not, write to the Free Software Foundation, Inc., 59 Temple Place,
 # Suite 330, Boston, MA  02111-1307 USA
 #
-use strict;
-#use warnings; FIXME - Bug 2505
-require Exporter;
+
+use Modern::Perl;
 
 use CGI;
 
@@ -28,92 +27,100 @@ use C4::Context;
 use C4::RotatingCollections;
 
 my $query = new CGI;
-my ($template, $loggedinuser, $cookie)
-    = get_template_and_user({template_name => "rotating_collections/editCollections.tt",
-			     query => $query,
-			     type => "intranet",
-			     authnotrequired => 0,
-                          flagsrequired => { tools => 'rotating_collections' },
-			     debug => 1,
-			     });
+
+my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
+    {
+        template_name   => "rotating_collections/editCollections.tmpl",
+        query           => $query,
+        type            => "intranet",
+        authnotrequired => 0,
+        flagsrequired   => { tools => 'rotating_collections' },
+        debug           => 1,
+    }
+);
 
 # Create new Collection
 if ( $query->param('action') eq 'create' ) {
-  my $title = $query->param('title');
-  my $description = $query->param('description');
-                            
-  my ( $createdSuccessfully, $errorCode, $errorMessage ) = CreateCollection( $title, $description );
-                              
-  $template->param(
-    previousActionCreate => 1,
-    createdTitle => $title,
-  );
-                                          
-  if ( $createdSuccessfully ) {
-    $template->param( createSuccess => 1 );
-  } else {
-    $template->param( createFailure => 1 );
-    $template->param( failureMessage => $errorMessage );
-  }                                                        
+    my $title       = $query->param('title');
+    my $description = $query->param('description');
+
+    my ( $createdSuccessfully, $errorCode, $errorMessage ) =
+      CreateCollection( $title, $description );
+
+    $template->param(
+        previousActionCreate => 1,
+        createdTitle         => $title,
+    );
+
+    if ($createdSuccessfully) {
+        $template->param( createSuccess => 1 );
+    }
+    else {
+        $template->param( createFailure  => 1 );
+        $template->param( failureMessage => $errorMessage );
+    }
 }
 
 ## Delete a club or service
 elsif ( $query->param('action') eq 'delete' ) {
-  my $colId = $query->param('colId');
-  my ( $success, $errorCode, $errorMessage ) = DeleteCollection( $colId );
-    
-  $template->param( previousActionDelete => 1 );
-  if ( $success ) {
-    $template->param( deleteSuccess => 1 );
-  } else {
-    $template->param( deleteFailure => 1 );
-    $template->param( failureMessage => $errorMessage );
-  }
+    my $colId = $query->param('colId');
+    my ( $success, $errorCode, $errorMessage ) = DeleteCollection($colId);
+
+    $template->param( previousActionDelete => 1 );
+    if ($success) {
+        $template->param( deleteSuccess => 1 );
+    }
+    else {
+        $template->param( deleteFailure  => 1 );
+        $template->param( failureMessage => $errorMessage );
+    }
 }
 
 ## Edit a club or service: grab data, put in form.
 elsif ( $query->param('action') eq 'edit' ) {
-  my $colId = $query->param('colId');
-  my ( $colId, $colTitle, $colDesc, $colBranchcode ) = GetCollection( $colId );
-
-  $template->param(
-      previousActionEdit => 1,
-      editColId => $colId,
-      editColTitle => $colTitle,
-      editColDescription => $colDesc,
-  );
+    my $colId = $query->param('colId');
+    my ( $colId, $colTitle, $colDesc, $colBranchcode ) = GetCollection($colId);
+
+    $template->param(
+        previousActionEdit => 1,
+        editColId          => $colId,
+        editColTitle       => $colTitle,
+        editColDescription => $colDesc,
+    );
 }
 
 # Update a Club or Service
 elsif ( $query->param('action') eq 'update' ) {
-  my $colId = $query->param('colId');
-  my $title = $query->param('title');
-  my $description = $query->param('description');
-                            
-  my ( $createdSuccessfully, $errorCode, $errorMessage ) 
-    = UpdateCollection( $colId, $title, $description );
-                              
-  $template->param(
-    previousActionUpdate => 1,
-    updatedTitle => $title,
-  );
-                                          
-  if ( $createdSuccessfully ) {
-    $template->param( updateSuccess => 1 );
-  } else {
-    $template->param( updateFailure => 1 );
-    $template->param( failureMessage => $errorMessage );
-  }                                                        
+    my $colId       = $query->param('colId');
+    my $title       = $query->param('title');
+    my $description = $query->param('description');
+
+    my ( $createdSuccessfully, $errorCode, $errorMessage ) =
+      UpdateCollection( $colId, $title, $description );
+
+    $template->param(
+        previousActionUpdate => 1,
+        updatedTitle         => $title,
+    );
+
+    if ($createdSuccessfully) {
+        $template->param( updateSuccess => 1 );
+    }
+    else {
+        $template->param( updateFailure  => 1 );
+        $template->param( failureMessage => $errorMessage );
+    }
 }
-                                                        
+
 my $collections = GetCollections();
 
 $template->param(
-		intranetcolorstylesheet => C4::Context->preference("intranetcolorstylesheet"),
-		intranetstylesheet => C4::Context->preference("intranetstylesheet"),
-		IntranetNav => C4::Context->preference("IntranetNav"),
-		
-		collectionsLoop => $collections,
-		);
+    intranetcolorstylesheet =>
+      C4::Context->preference("intranetcolorstylesheet"),
+    intranetstylesheet => C4::Context->preference("intranetstylesheet"),
+    IntranetNav        => C4::Context->preference("IntranetNav"),
+
+    collectionsLoop => $collections,
+);
 
 output_html_with_http_headers $query, $cookie, $template->output;
diff --git a/rotating_collections/rotatingCollections.pl b/rotating_collections/rotatingCollections.pl
index cffd9a4794..59bbcdbdc7 100755
--- a/rotating_collections/rotatingCollections.pl
+++ b/rotating_collections/rotatingCollections.pl
@@ -16,9 +16,7 @@
 # Suite 330, Boston, MA  02111-1307 USA
 #
 
-use strict;
-#use warnings; FIXME - Bug 2505
-require Exporter;
+use Modern::Perl;
 
 use CGI;
 
@@ -28,25 +26,29 @@ use C4::Context;
 use C4::RotatingCollections;
 
 my $query = new CGI;
-my ($template, $loggedinuser, $cookie)
-    = get_template_and_user({template_name => "rotating_collections/rotatingCollections.tt",
-			     query => $query,
-			     type => "intranet",
-			     authnotrequired => 0,
-                          flagsrequired => { tools => 'rotating_collections' },
-			     debug => 1,
-			     });
+
+my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
+    {
+        template_name   => "rotating_collections/rotatingCollections.tmpl",
+        query           => $query,
+        type            => "intranet",
+        authnotrequired => 0,
+        flagsrequired   => { tools => 'rotating_collections' },
+        debug           => 1,
+    }
+);
 
 my $branchcode = $query->cookie('branch');
 
 my $collections = GetCollections();
 
 $template->param(
-                intranetcolorstylesheet => C4::Context->preference("intranetcolorstylesheet"),
-                intranetstylesheet => C4::Context->preference("intranetstylesheet"),
-                IntranetNav => C4::Context->preference("IntranetNav"),
-                                  
-                collectionsLoop => $collections,
-                );
-                                                                                                
+    intranetcolorstylesheet =>
+      C4::Context->preference("intranetcolorstylesheet"),
+    intranetstylesheet => C4::Context->preference("intranetstylesheet"),
+    IntranetNav        => C4::Context->preference("IntranetNav"),
+
+    collectionsLoop => $collections,
+);
+
 output_html_with_http_headers $query, $cookie, $template->output;
diff --git a/rotating_collections/transferCollection.pl b/rotating_collections/transferCollection.pl
index c84d7e15e2..630c222eb0 100755
--- a/rotating_collections/transferCollection.pl
+++ b/rotating_collections/transferCollection.pl
@@ -16,9 +16,7 @@
 # Suite 330, Boston, MA  02111-1307 USA
 #
 
-use strict;
-#use warnings; FIXME - Bug 2505
-require Exporter;
+use Modern::Perl;
 
 use C4::Output;
 use C4::Auth;
@@ -30,55 +28,61 @@ use CGI;
 
 my $query = new CGI;
 
-my $colId = $query->param('colId');
+my $colId    = $query->param('colId');
 my $toBranch = $query->param('toBranch');
 
-my ($template, $loggedinuser, $cookie)
-    = get_template_and_user({template_name => "rotating_collections/transferCollection.tt",
-			     query => $query,
-			     type => "intranet",
-			     authnotrequired => 0,
-                          flagsrequired => { tools => 'rotating_collections' },
-			     debug => 1,
-			     });
+my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
+    {
+        template_name   => "rotating_collections/transferCollection.tmpl",
+        query           => $query,
+        type            => "intranet",
+        authnotrequired => 0,
+        flagsrequired   => { tools => 'rotating_collections' },
+        debug           => 1,
+    }
+);
 
 ## Transfer collection
 my ( $success, $errorCode, $errorMessage );
-if ( $toBranch ) {
-  ( $success, $errorCode, $errorMessage ) = TransferCollection( $colId, $toBranch );
+if ($toBranch) {
+    ( $success, $errorCode, $errorMessage ) =
+      TransferCollection( $colId, $toBranch );
 
-  if ( $success ) {
-    $template->param( transferSuccess => 1 );
-  } else {
-    $template->param( transferFailure => 1,
-                      errorCode => $errorCode,
-                      errorMessage => $errorMessage
-    );
-  }
+    if ($success) {
+        $template->param( transferSuccess => 1 );
+    }
+    else {
+        $template->param(
+            transferFailure => 1,
+            errorCode       => $errorCode,
+            errorMessage    => $errorMessage
+        );
+    }
 }
 
 ## Set up the toBranch select options
 my $branches = GetBranches();
 my @branchoptionloop;
-foreach my $br (keys %$branches) {
-  my %branch;
-  $branch{code}=$br;
-  $branch{name}=$branches->{$br}->{'branchname'};
-  push (@branchoptionloop, \%branch);
+foreach my $br ( keys %$branches ) {
+    my %branch;
+    $branch{code} = $br;
+    $branch{name} = $branches->{$br}->{'branchname'};
+    push( @branchoptionloop, \%branch );
 }
-    
+
 ## Get data about collection
-my ( $colId, $colTitle, $colDesc, $colBranchcode ) = GetCollection( $colId );                                
+my ( $colId, $colTitle, $colDesc, $colBranchcode ) = GetCollection($colId);
 $template->param(
-                intranetcolorstylesheet => C4::Context->preference("intranetcolorstylesheet"),
-                intranetstylesheet => C4::Context->preference("intranetstylesheet"),
-                IntranetNav => C4::Context->preference("IntranetNav"),
-                                  
-                colId => $colId,
-                colTitle => $colTitle,
-                colDesc => $colDesc,
-                colBranchcode => $colBranchcode,
-                branchoptionloop => \@branchoptionloop
-                );
-                                                                                                
+    intranetcolorstylesheet =>
+      C4::Context->preference("intranetcolorstylesheet"),
+    intranetstylesheet => C4::Context->preference("intranetstylesheet"),
+    IntranetNav        => C4::Context->preference("IntranetNav"),
+
+    colId            => $colId,
+    colTitle         => $colTitle,
+    colDesc          => $colDesc,
+    colBranchcode    => $colBranchcode,
+    branchoptionloop => \@branchoptionloop
+);
+
 output_html_with_http_headers $query, $cookie, $template->output;
-- 
2.39.5