Bug 9032: (follow-up) restore documented intepretation of virtualshelfshares.sharedate

The sharedate column is documented as having the following meaning:

"date of invitation or acceptance of invitation"

This patch adjust the new list-sharing code to stick with that
interpretation, as otherwise the column should have been renamed
to 'invite_expiration_date' or the like.

It also removes the "housekeeping" functionality from AddShare, as
otherwise the routine should have been named AddShareAndDoOtherStuff.

To prevent list shares from piling up, a new --list-invites flag
has been added to cleanup_database.pl.  The default crontabs have
been modified to use the --list-invites flag by default.

To test
-------
[1] Make some list share invites and accept some, but now all of them.
[2] Wait 14 days (or more reasonably, manually edit the sharedate
    values for the unaccepted shares to put them at least 14 days in the
    past.).
[3] Run cleanup_database.pl --list-invites
[4] Verify that accepted shares remain, as to share invites that have
    not yet reached more than 14 days of age.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>

C4/VirtualShelves.pm

@@ -675,13 +675,9 @@ sub AddShare {
     my ($shelfnumber, $key)= @_;
     return if !$shelfnumber || !$key;
 
-    my $sql;
     my $dbh = C4::Context->dbh;
-    $sql="DELETE FROM virtualshelfshares WHERE sharedate<NOW() LIMIT 10";
+    my $sql = "INSERT INTO virtualshelfshares (shelfnumber, invitekey, sharedate) VALUES (?, ?, NOW())";
-        #housekeeping: add one, remove max 10 expired ones
+    $dbh->do($sql, undef, ($shelfnumber, $key));
-    $dbh->do($sql);
-    $sql="INSERT INTO virtualshelfshares (shelfnumber, invitekey, sharedate) VALUES (?, ?, ADDDATE(NOW(),?))";
-    $dbh->do($sql, undef, ($shelfnumber, $key, SHARE_INVITATION_EXPIRY_DAYS));
     return !$dbh->err;
 }
 
@@ -703,10 +699,10 @@ sub AcceptShare {
     my $dbh = C4::Context->dbh;
     $sql="
 UPDATE virtualshelfshares
-SET invitekey=NULL, sharedate=NULL, borrowernumber=?
+SET invitekey=NULL, sharedate=NOW(), borrowernumber=?
-WHERE shelfnumber=? AND invitekey=? AND sharedate>NOW()
+WHERE shelfnumber=? AND invitekey=? AND (sharedate + INTERVAL ? DAY) >NOW()
     ";
-    my $i= $dbh->do($sql, undef, ($borrowernumber, $shelfnumber, $key));
+    my $i= $dbh->do($sql, undef, ($borrowernumber, $shelfnumber, $key,  SHARE_INVITATION_EXPIRY_DAYS));
     return if !defined($i) || !$i || $i eq '0E0'; #not found
     return 1;
 }

debian/koha-common.cron.daily

@@ -20,7 +20,7 @@ koha-foreach --enabled /usr/share/koha/bin/cronjobs/fines.pl
 koha-foreach --enabled --email /usr/share/koha/bin/cronjobs/advance_notices.pl -c
 koha-foreach --enabled /usr/share/koha/bin/cronjobs/holds/cancel_expired_holds.pl >/dev/null 2>&1
 koha-foreach --enabled /usr/share/koha/bin/cronjobs/services_throttle.pl > /dev/null 2>&1
-koha-foreach --enabled /usr/share/koha/bin/cronjobs/cleanup_database.pl --sessions --zebraqueue 10
+koha-foreach --enabled /usr/share/koha/bin/cronjobs/cleanup_database.pl --sessions --zebraqueue 10 --list-invites
 koha-foreach --enabled --noemail /usr/share/koha/bin/cronjobs/cleanup_database.pl --mail
 koha-foreach --enabled /usr/share/koha/bin/cronjobs/holds/auto_unsuspend_holds.pl > /dev/null 2>&1
 koha-run-backups --days 2 --output /var/spool/koha

misc/cronjobs/cleanup_database.pl

@@ -25,6 +25,7 @@ use constant DEFAULT_MAIL_PURGEDAYS => 30;
 use constant DEFAULT_IMPORT_PURGEDAYS => 60;
 use constant DEFAULT_LOGS_PURGEDAYS => 180;
 use constant DEFAULT_SEARCHHISTORY_PURGEDAYS => 30;
+use constant DEFAULT_SHARE_INVITATION_EXPIRY_DAYS => 14;
 
 BEGIN {
     # find Koha's Perl modules
@@ -64,6 +65,8 @@ Usage: $0 [-h|--help] [--sessions] [--sessdays DAYS] [-v|--verbose] [--zebraqueu
                       Defaults to 180 days if no days specified.
    --searchhistory DAYS  purge entries from search_history older than DAYS days.
                          Defaults to 30 days if no days specified
+   --list-invites  DAYS  purge (unaccepted) list share invites older than DAYS
+                         days.  Defaults to 14 days if no days specified.
 USAGE
     exit $_[0];
 }
@@ -71,7 +74,8 @@ USAGE
 my (
     $help,            $sessions,       $sess_days,    $verbose,
     $zebraqueue_days, $mail,           $purge_merged, $pImport,
-    $pLogs,           $pSearchhistory, $pZ3950
+    $pLogs,           $pSearchhistory, $pZ3950,
+    $pListShareInvites,
 );
 
 GetOptions(
@@ -86,6 +90,7 @@ GetOptions(
     'z3950'        => \$pZ3950,
     'logs:i'       => \$pLogs,
     'searchhistory:i' => \$pSearchhistory,
+    'list-invites:i'  => \$pListShareInvites,
 ) || usage(1);
 
 $sessions=1 if $sess_days && $sess_days>0;
@@ -96,6 +101,7 @@ $pLogs= DEFAULT_LOGS_PURGEDAYS if defined($pLogs) && $pLogs==0;
 $zebraqueue_days= DEFAULT_ZEBRAQ_PURGEDAYS if defined($zebraqueue_days) && $zebraqueue_days==0;
 $mail= DEFAULT_MAIL_PURGEDAYS if defined($mail) && $mail==0;
 $pSearchhistory= DEFAULT_SEARCHHISTORY_PURGEDAYS if defined($pSearchhistory) && $pSearchhistory==0;
+$pListShareInvites = DEFAULT_SHARE_INVITATION_EXPIRY_DAYS if defined($pListShareInvites) && $pListShareInvites == 0;
 
 if ($help) {
     usage(0);
@@ -108,7 +114,8 @@ unless ( $sessions
     || $pImport
     || $pLogs
     || $pSearchhistory
-    || $pZ3950 )
+    || $pZ3950
+    || $pListShareInvites )
 {
     print "You did not specify any cleanup work for the script to do.\n\n";
     usage(1);
@@ -206,6 +213,17 @@ if($pSearchhistory) {
     print "Done with purging search_history.\n" if $verbose;
 }
 
+if ($pListShareInvites) {
+    print "Purging unaccepted list share invites older than $pListShareInvites days.\n" if $verbose;
+    $sth = $dbh->prepare("
+        DELETE FROM virtualshelfshares
+        WHERE invitekey IS NOT NULL
+        AND (sharedate + INTERVAL ? DAY) < NOW()
+    ");
+    $sth->execute($pListShareInvites);
+    print "Done with purging unaccepted list share invites.\n" if $verbose;
+}
+
 exit(0);
 
 sub RemoveOldSessions {

misc/cronjobs/crontab.example

@@ -81,7 +81,7 @@ KOHA_CRON_PATH = /usr/share/koha/bin/cronjobs
 59 23 * * *  __KOHA_USER__ $KOHA_CRON_PATH/services_throttle.pl > /dev/null 2>&1
 
 # clean up databases nightly.  Be sure not to run this with --sessions during a time when the system is in use!
-16 1 * * * __KOHA_USER__ $KOHA_CRON_PATH/cleanup_database.pl --sessions --zebraqueue 10
+16 1 * * * __KOHA_USER__ $KOHA_CRON_PATH/cleanup_database.pl --sessions --zebraqueue 10 --list-invites
 
 # delete old purchase suggestions weekly. Replace XX with a number to define the age of suggestions to delete.
 @weekly	__KOHA_USER__  $KOHA_CRON_PATH/purge_suggestions.pl --days XX > /dev/null 2>&1

t/db_dependent/VirtualShelves.t

@@ -179,9 +179,8 @@ for my $i (0..9){
 
 #----------------------- TEST AddShare ----------------------------------------#
 
-#first count the number of shares in the table; keep in mind that AddShare may
+#first count the number of shares in the table
-#delete some expired records while housekeeping
+my $sql_sharecount="select count(*) from virtualshelfshares";
-my $sql_sharecount="select count(*) from virtualshelfshares where DATEDIFF(sharedate, NOW())>0";
 my $cnt1=$dbh->selectrow_array($sql_sharecount);
 
 #try to add a share without shelfnumber: should fail