Bug 14544: Move share routines to Koha::Virtualshelfshare and Koha::Virtualshelfshares

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

C4/Installer/PerlDependencies.pm

@@ -99,6 +99,11 @@ our $PERL_DEPS = {
         'required' => '1',
         'min_ver'  => '1.103'
     },
+    'Exception::Class' => {
+        'usage'    => 'Core',
+        'required' => '1.39',
+        'min_ver'  => '1.39'
+    },
     'HTML::Scrubber' => {
         'usage'    => 'Core',
         'required' => '1',

C4/VirtualShelves.pm

@@ -45,7 +45,6 @@ BEGIN {
             &ShelfPossibleAction
             &DelFromShelf
             &GetBibliosShelves
-            &AddShare &AcceptShare &RemoveShare &IsSharedList
     );
         @EXPORT_OK = qw(
             &GetAllShelves &ShelvesMax
@@ -390,7 +389,7 @@ sub ShelfPossibleAction {
         }
     }
     elsif($action eq 'acceptshare') {
-        #the key for accepting is checked later in AcceptShare
+        #the key for accepting is checked later in Koha::Virtualshelf->share
         #you must not be the owner, list must be private
         if( $shelf->{category}==1 ) {
             return (0, 8) if $shelf->{owner}==$user;
@@ -526,98 +525,6 @@ sub HandleDelBorrower {
     #Koha::Virtualshelf->new->delete too.
 }
 
-=head2 AddShare
-
-     AddShare($shelfnumber, $key);
-
-Adds a share request to the virtualshelves table.
-Authorization must have been checked, and a key must be supplied. See script
-opac-shareshelf.pl for an example.
-This request is not yet confirmed. So it has no borrowernumber, it does have an
-expiry date.
-
-=cut
-
-sub AddShare {
-    my ($shelfnumber, $key)= @_;
-    return if !$shelfnumber || !$key;
-
-    my $dbh = C4::Context->dbh;
-    my $sql = "INSERT INTO virtualshelfshares (shelfnumber, invitekey, sharedate) VALUES (?, ?, NOW())";
-    $dbh->do($sql, undef, ($shelfnumber, $key));
-    return !$dbh->err;
-}
-
-=head2 AcceptShare
-
-     my $result= AcceptShare($shelfnumber, $key, $borrowernumber);
-
-Checks acceptation of a share request.
-Key must be found for this shelf. Invitation must not have expired.
-Returns true when accepted, false otherwise.
-
-=cut
-
-sub AcceptShare {
-    my ($shelfnumber, $key, $borrowernumber)= @_;
-    return if !$shelfnumber || !$key || !$borrowernumber;
-
-    my $sql;
-    my $dbh = C4::Context->dbh;
-    $sql="
-UPDATE virtualshelfshares
-SET invitekey=NULL, sharedate=NOW(), borrowernumber=?
-WHERE shelfnumber=? AND invitekey=? AND (sharedate + INTERVAL ? DAY) >NOW()
-    ";
-    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;
-}
-
-=head2 IsSharedList
-
-     my $bool= IsSharedList( $shelfnumber );
-
-IsSharedList checks if a (private) list has shares.
-Note that such a check would not be useful for public lists. A public list has
-no shares, but is visible for anyone by nature..
-Used to determine the list type in the display of Your lists (all private).
-Returns boolean value.
-
-=cut
-
-sub IsSharedList {
-    my ($shelfnumber) = @_;
-    my $dbh = C4::Context->dbh;
-    my $sql="SELECT id FROM virtualshelfshares WHERE shelfnumber=? AND borrowernumber IS NOT NULL";
-    my $sth = $dbh->prepare($sql);
-    $sth->execute($shelfnumber);
-    my ($rv)= $sth->fetchrow_array;
-    return defined($rv);
-}
-
-=head2 RemoveShare
-
-     RemoveShare( $user, $shelfnumber );
-
-RemoveShare removes a share for specific shelf and borrower.
-Returns true if a record could be deleted.
-
-=cut
-
-sub RemoveShare {
-    my ($user, $shelfnumber)= @_;
-    my $dbh = C4::Context->dbh;
-    my $sql="
-DELETE FROM virtualshelfshares
-WHERE borrowernumber=? AND shelfnumber=?
-    ";
-    my $n= $dbh->do($sql,undef,($user, $shelfnumber));
-    return if !defined($n) || !$n || $n eq '0E0'; #nothing removed
-    return 1;
-}
-
-
 sub GetShelfCount {
     my ($owner, $category) = @_;
     my @params;

C4/VirtualShelves/Page.pm

@@ -389,7 +389,8 @@ sub shelfpage {
                 }
                 #remove a share
                 if(/REMSHR/) {
-                    RemoveShare($loggedinuser, $number);
+                    my $shelf = Koha::Virtualshelves->find( $number );
+                    $shelf->delete_share( $loggedinuser );
                     delete $shelflist->{$number} if exists $shelflist->{$number};
                     $stay=0;
                     next;
@@ -461,7 +462,8 @@ sub shelfpage {
         $shelflist->{$element}->{ownername} = defined($member) ? $member->{firstname} . " " . $member->{surname} : '';
         $numberCanManage++ if $canmanage;    # possibly outmoded
         if ( $shelflist->{$element}->{'category'} eq '1' ) {
-            $shelflist->{$element}->{shares} = IsSharedList($element);
+            my $shelf = Koha::Virtualshelves->find( $element );
+            $shelflist->{$element}->{shares} = $shelf->is_shared;
             push( @shelveslooppriv, $shelflist->{$element} );
         } else {
             push( @shelvesloop, $shelflist->{$element} );

Koha/Exceptions.pm

@@ -0,0 +1,34 @@
+package Koha::Exceptions;
+
+use Modern::Perl;
+
+use Exception::Class (
+
+    'Koha::Exceptions::Exception' => {
+        description => 'Something went wrong!',
+    },
+
+    'Koha::Exceptions::DuplicateObject' => {
+        isa => 'Koha::Exceptions::Exception',
+        description => 'Same object already exists',
+    },
+
+    'Koha::Exceptions::Virtualshelves::DuplicateObject' => {
+        isa => 'Koha::Exceptions::DuplicateObject',
+        description => "poeut",
+    },
+    'Koha::Exceptions::Virtualshelves::InvalidInviteKey' => {
+        isa => 'Koha::Exceptions::Exception',
+        description => 'Invalid key on accepting the share',
+    },
+    'Koha::Exceptions::Virtualshelves::InvalidKeyOnSharing' => {
+        isa => 'Koha::Exceptions::Exception',
+        description=> 'Invalid key on sharing a shelf',
+    },
+    'Koha::Exceptions::Virtualshelves::ShareHasExpired' => {
+        isa => 'Koha::Exceptions::Exception',
+        description=> 'Cannot share this shelf, the share has expired',
+    }
+);
+
+1;

Koha/Virtualshelf.pm

@@ -21,7 +21,9 @@ use Carp;
 
 use Koha::Database;
 use Koha::DateUtils qw( dt_from_string );
-use Koha::Exception::DuplicateObject;
+use Koha::Exceptions;
+use Koha::Virtualshelfshare;
+use Koha::Virtualshelfshares;
 
 use base qw(Koha::Object);
 
@@ -93,6 +95,48 @@ sub is_shelfname_valid {
     return $count ? 0 : 1;
 }
 
+sub get_shares {
+    my ( $self ) = @_;
+    return $self->{_result}->virtualshelfshares;
+}
+
+sub share {
+    my ( $self, $key ) = @_;
+    unless ( $key ) {
+        Koha::Exceptions::Virtualshelves::InvalidKeyOnSharing->throw;
+    }
+    Koha::Virtualshelfshare->new(
+        {
+            shelfnumber => $self->shelfnumber,
+            invitekey => $key,
+            sharedate => dt_from_string,
+        }
+    )->store;
+}
+
+sub is_shared {
+    my ( $self ) = @_;
+    return  $self->get_shares->search(
+        {
+            borrowernumber => { '!=' => undef },
+        }
+    )->count;
+}
+
+sub remove_share {
+    my ( $self, $borrowernumber ) = @_;
+    my $shelves = Koha::Virtualshelfshares->search(
+        {
+            shelfnumber => $self->shelfnumber,
+            borrowernumber => $borrowernumber,
+        }
+    );
+    return 0 unless $shelves->count;
+
+    # Only 1 share with 1 patron can exist
+    return $shelves->next->delete;
+}
+
 sub type {
     return 'Virtualshelve';
 }

Koha/Virtualshelfshare.pm

@@ -0,0 +1,74 @@
+package Koha::Virtualshelfshare;
+
+# This file is part of Koha.
+#
+# Koha is free software; you can redistribute it and/or modify it under the
+# terms of the GNU General Public License as published by the Free Software
+# Foundation; either version 3 of the License, or (at your option) any later
+# version.
+#
+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with Koha; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+use Modern::Perl;
+
+use Carp;
+use DateTime;
+use DateTime::Duration;
+
+use Koha::Database;
+use Koha::DateUtils;
+use Koha::Exceptions;
+
+use base qw(Koha::Object);
+
+use constant SHARE_INVITATION_EXPIRY_DAYS => 14; #two weeks to accept
+
+=head1 NAME
+
+Koha::Virtualshelfshare - Koha Virtualshelfshare Object class
+
+=head1 API
+
+=head2 Class Methods
+
+=cut
+
+=head3 type
+
+=cut
+
+sub accept {
+    my ( $self, $invitekey, $borrowernumber ) = @_;
+    if ( $self->has_expired ) {
+        Koha::Exceptions::Virtualshelves::ShareHasExpired->throw;
+    }
+    if ( $self->invitekey ne $invitekey ) {
+        Koha::Exceptions::Virtualshelves::InvalidInviteKey->throw;
+    }
+    $self->invitekey(undef);
+    $self->sharedate(dt_from_string);
+    $self->borrowernumber($borrowernumber);
+    $self->store;
+}
+
+sub has_expired {
+    my ($self) = @_;
+    my $dt_sharedate     = dt_from_string( $self->sharedate, 'sql' );
+    my $today            = dt_from_string;
+    my $expiration_delay = DateTime::Duration->new( days => SHARE_INVITATION_EXPIRY_DAYS );
+    my $has_expired = DateTime->compare( $today, $dt_sharedate->add_duration($expiration_delay) );
+    # Note: has_expired = 0 if the share expires today
+    return $has_expired == 1 ? 1 : 0
+}
+
+sub type {
+    return 'Virtualshelfshare';
+}
+
+1;

Koha/Virtualshelfshares.pm

@@ -0,0 +1,50 @@
+package Koha::Virtualshelfshares;
+
+# This file is part of Koha.
+#
+# Koha is free software; you can redistribute it and/or modify it under the
+# terms of the GNU General Public License as published by the Free Software
+# Foundation; either version 3 of the License, or (at your option) any later
+# version.
+#
+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with Koha; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+use Modern::Perl;
+
+use Carp;
+
+use Koha::Database;
+
+use Koha::Virtualshelf;
+
+use base qw(Koha::Objects);
+
+=head1 NAME
+
+Koha::Virtualshelfshares - Koha Virtualshelfshares Object class
+
+=head1 API
+
+=head2 Class Methods
+
+=cut
+
+=head3 type
+
+=cut
+
+sub type {
+    return 'Virtualshelfshare';
+}
+
+sub object_class {
+    return 'Koha::Virtualshelfshare';
+}
+
+1;

opac/opac-shareshelf.pl

@@ -122,20 +122,35 @@ sub show_accept {
     #errorcode 5: should be private list
     #errorcode 8: should not be owner
 
-    my $dbkey = keytostring( stringtokey( $param->{key}, 0 ), 1 );
-    if ( AcceptShare( $param->{shelfnumber}, $dbkey, $param->{loggedinuser} ) )
-    {
-        notify_owner($param);
-
-        #redirect to view of this shared list
-        print $param->{query}->redirect(
-            -uri    => SHELVES_URL . $param->{shelfnumber},
-            -cookie => $param->{cookie}
-        );
-        exit;
-    }
-    else {
+    # We could have used ->find with the share id, but we don't want to change
+    # the url sent to the patron
+    my $shared_shelf = Koha::Virtualshelfshares->search(
+        {
+            shelfnumber => $param->{shelfnumber},
+        },
+        {
+            order_by => 'sharedate desc',
+            limit => 1,
+        }
+    );
+
+    if ( $shared_shelf ) {
+        $shared_shelf = $shared_shelf->next;
+        my $key = keytostring( stringtokey( $param->{key}, 0 ), 1 );
+        my $is_accepted = eval { $shared_shelf->accept( $key, $param->{loggedinuser} ) };
+        if ( $is_accepted ) {
+            notify_owner($param);
+
+            #redirect to view of this shared list
+            print $param->{query}->redirect(
+                -uri    => SHELVES_URL . $param->{shelfnumber},
+                -cookie => $param->{cookie}
+            );
+            exit;
+        }
         $param->{errcode} = 7;    #not accepted (key not found or expired)
+    } else {
+        # This shelf is not shared
     }
 }
 
@@ -200,7 +215,11 @@ sub send_invitekey {
         my @newkey = randomlist( KEYLENGTH, 64 );    #generate a new key
 
         #add a preliminary share record
-        if ( !AddShare( $param->{shelfnumber}, keytostring( \@newkey, 1 ) ) ) {
+        my $shelf = Koha::Virtualshelves->find( $param->{shelfnumber} );
+        my $key = keytostring( \@newkey, 1 );
+        my $is_shared = eval { $shelf->share( $key ); };
+        # TODO Better error handling, catch the exceptions
+        if ( $@ or not $is_shared ) {
             push @{ $param->{fail_addr} }, $a;
             next;
         }

t/db_dependent/VirtualShelves.t

@@ -122,80 +122,6 @@ for my $i (0..9){
     $used{$key}++;
 }
 
-#----------------------- TEST AddShare ----------------------------------------#
-
-#first count the number of shares in the table
-my $sql_sharecount="select count(*) from virtualshelfshares";
-my $cnt1=$dbh->selectrow_array($sql_sharecount);
-
-#try to add a share without shelfnumber: should fail
-AddShare(0, 'abcdefghij');
-my $cnt2=$dbh->selectrow_array($sql_sharecount);
-is($cnt1,$cnt2, "Did not add an invalid share record");
-
-#add another share: should be okay
-#AddShare assumes that you tested if category==private (so we could actually
-#be doing something illegal here :)
-my $n=$shelves[0]->{number};
-if($n<0) {
-    ok(1, 'Skip AddShare for shelf -1');
-}
-else {
-    AddShare($n, 'abcdefghij');
-    my $cnt3=$dbh->selectrow_array($sql_sharecount);
-    is(1+$cnt2, $cnt3, "Added one new share record with invitekey");
-}
-
-#----------------------- TEST AcceptShare -------------------------------------#
-
-# test accepting a wrong key
-my $testkey= 'keyisgone9';
-my $acctest="delete from virtualshelfshares where invitekey=?";
-$dbh->do($acctest,undef,($testkey)); #just be sure it does not exist
-$acctest="select shelfnumber from virtualshelves";
-my ($accshelf)= $dbh->selectrow_array($acctest);
-is(AcceptShare($accshelf,$testkey,$borrowernumbers[0]),undef,'Did not accept invalid key');
-
-# test accepting a good key
-if( AddShare($accshelf,$testkey) && $borrowernumbers[0] ) {
-    is(AcceptShare($accshelf, $testkey, $borrowernumbers[0]),1,'Accepted share');
-}
-else { #cannot accept if addshared failed somehow
-    ok(1, 'Skipped second AcceptShare test');
-}
-
-#----------------------- TEST IsSharedList ------------------------------------#
-
-for my $i (0..9) {
-    my $sql="select count(*) from virtualshelfshares where shelfnumber=? and borrowernumber is not null";
-    my $sh=$shelves[$i]->{number};
-    my ($n)=$dbh->selectrow_array($sql,undef,($sh));
-    is(IsSharedList($sh),$n? 1: '', "Checked IsSharedList for shelf $sh");
-}
-
-#----------------TEST Koha::Virtualshelf->delete & DelFromShelf functions------------------------#
-
-for my $i (0..9){
-    my $shelfnumber = $shelves[$i]->{number};
-    if($shelfnumber<0) {
-        ok(1, 'Skip Koha::Virtualshelf->delete for shelf -1');
-        next;
-    }
-    my $status = Koha::Virtualshelves->find($shelfnumber)->delete;
-    is($status, 1, "deleted shelf $shelfnumber and its contents");
-}
-
-#----------------------- TEST RemoveShare -------------------------------------#
-
-my $remshr_test="select borrowernumber, shelfnumber from virtualshelfshares where borrowernumber is not null";
-my @remshr_shelf= $dbh->selectrow_array($remshr_test);
-if(@remshr_shelf) {
-    is(RemoveShare(@remshr_shelf),1,'Removed a shelf share');
-}
-else {
-    ok(1,'Skipped RemoveShare test');
-}
-
 #----------------------- SOME SUBS --------------------------------------------#
 
 sub randomname {

t/db_dependent/Virtualshelves.t

@@ -1,12 +1,12 @@
 #!/usr/bin/perl
 
 use Modern::Perl;
-use Test::More tests => 1;
+use Test::More tests => 2;
 
 use C4::Context;
 use Koha::DateUtils;
-use Koha::Virtualshelf;
 use Koha::Virtualshelves;
+use Koha::Virtualshelfshares;
 
 use t::lib::TestBuilder;
 
@@ -14,6 +14,7 @@ my $dbh = C4::Context->dbh;
 $dbh->{AutoCommit} = 0;
 
 $dbh->do(q|DELETE FROM virtualshelves|);
+$dbh->do(q|DELETE FROM virtualshelfshares|);
 
 my $builder = t::lib::TestBuilder->new;
 
@@ -56,7 +57,7 @@ subtest 'CRUD' => sub {
             }
         )->store;
     };
-    is( ref($@), 'Koha::Exception::DuplicateObject' );
+    is( ref($@), 'Koha::Exceptions::Virtualshelves::DuplicateObject' );
     $number_of_shelves = Koha::Virtualshelves->search->count;
     is( $number_of_shelves, 1, 'To be sure the number of shelves is still 1' );
 
@@ -78,3 +79,71 @@ subtest 'CRUD' => sub {
     $number_of_shelves = Koha::Virtualshelves->search->count;
     is( $number_of_shelves, 1, 'To be sure the shelf has been deleted' );
 };
+
+subtest 'Sharing' => sub {
+    plan tests => 18;
+    my $patron_wants_to_share = $builder->build({
+        source => 'Borrower',
+    });
+    my $share_with_me = $builder->build({
+        source => 'Borrower',
+    });
+    my $just_another_patron = $builder->build({
+        source => 'Borrower',
+    });
+
+    my $number_of_shelves_shared = Koha::Virtualshelfshares->search->count;
+    is( $number_of_shelves_shared, 0, 'No shelves should exist' );
+
+    my $shelf_to_share = Koha::Virtualshelf->new({
+            shelfname => "my first shelf",
+            owner => $patron_wants_to_share->{borrowernumber},
+            category => 1,
+        }
+    )->store;
+
+    my $shelf_not_to_share = Koha::Virtualshelf->new({
+            shelfname => "my second shelf",
+            owner => $patron_wants_to_share->{borrowernumber},
+            category => 1,
+        }
+    )->store;
+
+    my $shared_shelf = eval { $shelf_to_share->share };
+    is ( ref( $@ ), 'Koha::Exceptions::Virtualshelves::InvalidKeyOnSharing', 'Do not share if no key given' );
+    $shared_shelf = eval { $shelf_to_share->share('this is a valid key') };
+    is( ref( $shared_shelf ), 'Koha::Virtualshelfshare', 'On sharing, the method should return a valid Koha::Virtualshelfshare object' );
+
+    my $another_shared_shelf = eval { $shelf_to_share->share('this is another valid key') }; # Just to have 2 shares in DB
+
+    $number_of_shelves_shared = Koha::Virtualshelfshares->search->count;
+    is( $number_of_shelves_shared, 2, '2 shares should have been inserted' );
+
+    my $is_accepted = eval {
+        $shared_shelf->accept( 'this is an invalid key', $share_with_me->{borrowernumber} );
+    };
+    is( $is_accepted, undef, 'The share should have not been accepted if the key is invalid' );
+    is( ref( $@ ), 'Koha::Exceptions::Virtualshelves::InvalidInviteKey', 'accept with an invalid key should raise an exception' );
+
+    $is_accepted = $shared_shelf->accept( 'this is a valid key', $share_with_me->{borrowernumber} );
+    ok( defined($is_accepted), 'The share should have been accepted if the key valid' );
+
+    is( $shelf_to_share->is_shared, 1 );
+    is( $shelf_not_to_share->is_shared, 0 );
+
+    is( $shelf_to_share->is_shared_with( $patron_wants_to_share->{borrowernumber} ), 0 , "The shelf should not be shared with the owner" );
+    is( $shelf_to_share->is_shared_with( $share_with_me->{borrowernumber} ), 1 , "The shelf should be shared with share_with_me" );
+    is( $shelf_to_share->is_shared_with( $just_another_patron->{borrowernumber} ), 0, "The shelf should not be shared with just_another_patron" );
+
+    is( $shelf_to_share->remove_share( $just_another_patron->{borrowernumber} ), 0, 'No share should be removed if the share has not been done with this patron' );
+    $number_of_shelves_shared = Koha::Virtualshelfshares->search->count;
+    is( $number_of_shelves_shared, 2, 'To be sure no shares have been removed' );
+
+    is( $shelf_not_to_share->remove_share( $share_with_me->{borrowernumber} ), 0, '0 share should have been removed if the shelf is not share' );
+    $number_of_shelves_shared = Koha::Virtualshelfshares->search->count;
+    is( $number_of_shelves_shared, 2, 'To be sure no shares have been removed' );
+
+    is( $shelf_to_share->remove_share( $share_with_me->{borrowernumber} ), 1, '1 share should have been removed if the shelf was shared with this patron' );
+    $number_of_shelves_shared = Koha::Virtualshelfshares->search->count;
+    is( $number_of_shelves_shared, 1, 'To be sure the share has been removed' );
+};