Bug 14544: Koha::Virtualshelfcontent[s]
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Thu, 16 Jul 2015 16:13:40 +0000 (17:13 +0100)
committerTomas Cohen Arazi <tomascohen@theke.io>
Thu, 5 Nov 2015 12:58:01 +0000 (09:58 -0300)
Get rid of AddToShelf and DelFromShelf

Bug 14544: Allow a user to delete his own contents

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/VirtualShelves.pm
C4/VirtualShelves/Page.pm
Koha/Virtualshelf.pm
Koha/Virtualshelfcontent.pm [new file with mode: 0644]
Koha/Virtualshelfcontents.pm [new file with mode: 0644]
opac/opac-addbybiblionumber.pl
t/db_dependent/Utils/Datatables_Virtualshelves.t
t/db_dependent/VirtualShelves.t [deleted file]
t/db_dependent/Virtualshelves.t
virtualshelves/addbybiblionumber.pl

index 5668a78..5c072f9 100644 (file)
@@ -41,9 +41,7 @@ BEGIN {
     @ISA    = qw(Exporter);
     @EXPORT = qw(
             &GetShelves &GetShelfContents
-            &AddToShelf
             &ShelfPossibleAction
-            &DelFromShelf
             &GetBibliosShelves
     );
         @EXPORT_OK = qw(
@@ -234,41 +232,6 @@ sub GetShelfContents {
     # or newer, for your version of DBI.
 }
 
-=head2 AddToShelf
-
-  &AddToShelf($biblionumber, $shelfnumber, $borrower);
-
-Adds bib number C<$biblionumber> to virtual virtualshelves number
-C<$shelfnumber>, unless that bib is already on that shelf.
-
-=cut
-
-sub AddToShelf {
-    my ($biblionumber, $shelfnumber, $borrowernumber) = @_;
-    return unless $biblionumber;
-    my $dbh = C4::Context->dbh;
-    my $query = qq(
-        SELECT *
-        FROM   virtualshelfcontents
-        WHERE  shelfnumber=? AND biblionumber=?
-    );
-    my $sth = $dbh->prepare($query);
-
-    $sth->execute( $shelfnumber, $biblionumber );
-    ($sth->rows) and return; # already on shelf
-    $query = qq(
-        INSERT INTO virtualshelfcontents
-            (shelfnumber, biblionumber, flags, borrowernumber)
-        VALUES (?, ?, 0, ?));
-    $sth = $dbh->prepare($query);
-    $sth->execute( $shelfnumber, $biblionumber, $borrowernumber);
-    $query = qq(UPDATE virtualshelves
-                SET lastmodified = CURRENT_TIMESTAMP
-                WHERE shelfnumber = ?);
-    $sth = $dbh->prepare($query);
-    $sth->execute( $shelfnumber );
-}
-
 =head2 ShelfPossibleAction
 
 ShelfPossibleAction($loggedinuser, $shelfnumber, $action);
@@ -280,7 +243,7 @@ New additional actions are: invite, acceptshare.
 Note that add/delete here refers to adding/deleting entries from the list. Deleting the list itself falls under manage.
 new_public and new_private refers to creating a new public or private list.
 The distinction between deleting your own entries from the list or entries from
-others is made in DelFromShelf.
+others is made when deleting a content from the shelf.
 
 Returns 1 if the user can do the $action in the $shelfnumber shelf.
 Returns 0 otherwise.
@@ -339,7 +302,7 @@ sub ShelfPossibleAction {
         #this answer is just diplomatic: it says that you may be able to delete
         #some items from that shelf
         #it does not answer the question about a specific biblio
-        #DelFromShelf checks the situation per biblio
+        #Koha::Virtualshelf->remove_biblios checks the situation per biblio
         return 1 if $user>0 && ($shelf->{allow_delete_own}==1 || $shelf->{allow_delete_other}==1);
     }
     elsif($action eq 'invite') {
@@ -370,51 +333,6 @@ sub ShelfPossibleAction {
     return 0;
 }
 
-=head2 DelFromShelf
-
-    $result= &DelFromShelf( $bibref, $shelfnumber, $user);
-
-Removes biblionumbers in passed arrayref from shelf C<$shelfnumber>.
-If the bib wasn't on that virtualshelves to begin with, nothing happens.
-
-Returns 0 if no items have been deleted.
-
-=cut
-
-sub DelFromShelf {
-    my ($bibref, $shelfnumber, $user) = @_;
-    my $dbh = C4::Context->dbh;
-    my $query = qq(SELECT allow_delete_own, allow_delete_other FROM virtualshelves WHERE shelfnumber=?);
-    my $sth= $dbh->prepare($query);
-    $sth->execute($shelfnumber);
-    my ($del_own, $del_oth)= $sth->fetchrow;
-    my $r; my $t=0;
-
-    if($del_own) {
-        $query = qq(DELETE FROM virtualshelfcontents
-            WHERE shelfnumber=? AND biblionumber=? AND borrowernumber=?);
-        $sth= $dbh->prepare($query);
-        foreach my $biblionumber (@$bibref) {
-            $sth->execute($shelfnumber, $biblionumber, $user);
-            $r= $sth->rows; #Expect -1, 0 or 1 (-1 means Don't know; count as 1)
-            $t+= ($r==-1)? 1: $r;
-        }
-    }
-    if($del_oth) {
-        #includes a check if borrowernumber is null (deleted patron)
-        $query = qq/DELETE FROM virtualshelfcontents
-            WHERE shelfnumber=? AND biblionumber=? AND
-            (borrowernumber IS NULL OR borrowernumber<>?)/;
-        $sth= $dbh->prepare($query);
-        foreach my $biblionumber (@$bibref) {
-            $sth->execute($shelfnumber, $biblionumber, $user);
-            $r= $sth->rows;
-            $t+= ($r==-1)? 1: $r;
-        }
-    }
-    return $t;
-}
-
 =head2 GetBibliosShelves
 
 This finds all the public lists that this bib record is in.
index 84f74a8..f22e014 100644 (file)
@@ -106,7 +106,7 @@ sub shelfpage {
                     $item = GetItem( 0, $barcode);
                     if (defined $item && $item->{'itemnumber'}) {
                         $biblio = GetBiblioFromItemNumber( $item->{'itemnumber'} );
-                        AddToShelf( $biblio->{'biblionumber'}, $shelfnumber, $loggedinuser)
+                        Koha::Virtualshelves->find( $shelfnumber )->add_biblio( $biblio->{biblionumber}, $loggedinuser )
                           or push @paramsloop, { duplicatebiblio => $barcode };
                     }
                     else {
@@ -120,17 +120,17 @@ sub shelfpage {
             elsif(grep { /REM-(\d+)/ } $query->param) {
             #remove item(s) from shelf
                 if(ShelfPossibleAction($loggedinuser, $shelfnumber, 'delete')) {
-                #This is just a general okay; DelFromShelf checks further
                     my @bib;
                     foreach($query->param) {
                         /REM-(\d+)/ or next;
                         push @bib, $1; #$1 is biblionumber
                     }
-                    my $t= DelFromShelf(\@bib, $shelfnumber, $loggedinuser);
-                    if($t==0) {
+                    my $shelf = Koha::Virtualshelves->find( $shelfnumber );
+                    my $number_of_biblios_removed = $shelf->remove_biblios( { biblionumbers => \@bib, borrowernumber => $loggedinuser } );
+                    if( $number_of_biblios_removed == 0) {
                         push @paramsloop, {nothingdeleted => $shelfnumber};
                     }
-                    elsif($t<@bib) {
+                    elsif( $number_of_biblios_removed < @bib ) {
                         push @paramsloop, {somedeleted => $shelfnumber};
                     }
                 }
index 0a9ab73..7edacaa 100644 (file)
@@ -24,6 +24,7 @@ use Koha::DateUtils qw( dt_from_string );
 use Koha::Exceptions;
 use Koha::Virtualshelfshare;
 use Koha::Virtualshelfshares;
+use Koha::Virtualshelfcontent;
 
 use base qw(Koha::Object);
 
@@ -100,6 +101,11 @@ sub get_shares {
     return $self->{_result}->virtualshelfshares;
 }
 
+sub get_contents {
+    my ( $self ) = @_;
+    return $self->{_result}->virtualshelfcontents;
+}
+
 sub share {
     my ( $self, $key ) = @_;
     unless ( $key ) {
@@ -137,6 +143,64 @@ sub remove_share {
     return $shelves->next->delete;
 }
 
+sub add_biblio {
+    my ( $self, $biblionumber, $borrowernumber ) = @_;
+    return unless $biblionumber;
+    my $already_exists = $self->get_contents->search(
+        {
+            biblionumber => $biblionumber,
+        }
+    )->count;
+    return if $already_exists;
+    my $content = Koha::Virtualshelfcontent->new(
+        {
+            shelfnumber => $self->shelfnumber,
+            biblionumber => $biblionumber,
+            borrowernumber => $borrowernumber,
+        }
+    )->store;
+    $self->lastmodified(dt_from_string);
+    $self->store;
+
+    return $content;
+}
+
+sub remove_biblios {
+    my ( $self, $params ) = @_;
+    my $biblionumbers = $params->{biblionumbers} || [];
+    my $borrowernumber = $params->{borrowernumber};
+    return unless @$biblionumbers;
+
+    my $number_removed = 0;
+    for my $biblionumber ( @$biblionumbers ) {
+        if ( $self->owner == $borrowernumber or $self->allow_delete_own ) {
+            $number_removed += $self->get_contents->search(
+                {
+                    biblionumber => $biblionumber,
+                    borrowernumber => $borrowernumber,
+                }
+            )->delete;
+        }
+        if ( $self->allow_delete_other ) {
+            $number_removed += $self->get_contents->search(
+                {
+                    biblionumber => $biblionumber,
+                    # FIXME
+                    # This does not make sense, but it's has been backported from DelFromShelf.
+                    # Why shouldn't we allow to remove his own contribution if allow_delete_other is on?
+                    borrowernumber => {
+                        -or => {
+                            '!=' => $borrowernumber,
+                            '=' => undef
+                        }
+                    },
+                }
+            )->delete;
+        }
+    }
+    return $number_removed;
+}
+
 sub type {
     return 'Virtualshelve';
 }
diff --git a/Koha/Virtualshelfcontent.pm b/Koha/Virtualshelfcontent.pm
new file mode 100644 (file)
index 0000000..d7fc0c7
--- /dev/null
@@ -0,0 +1,45 @@
+package Koha::Virtualshelfcontent;
+
+# 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::Exceptions;
+
+use base qw(Koha::Object);
+
+=head1 NAME
+
+Koha::Virtualshelfcontent - Koha Virtualshelfcontent Object class
+
+=head1 API
+
+=head2 Class Methods
+
+=cut
+
+=head3 type
+
+=cut
+
+sub type {
+    return 'Virtualshelfcontent';
+}
+
+1;
diff --git a/Koha/Virtualshelfcontents.pm b/Koha/Virtualshelfcontents.pm
new file mode 100644 (file)
index 0000000..2a6b777
--- /dev/null
@@ -0,0 +1,50 @@
+package Koha::Virtualshelfcontents;
+
+# 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::Virtualshelfcontents - Koha Virtualshelfcontents Object class
+
+=head1 API
+
+=head2 Class Methods
+
+=cut
+
+=head3 type
+
+=cut
+
+sub type {
+    return 'Virtualshelfcontent';
+}
+
+sub object_class {
+    return 'Koha::Virtualshelfcontent';
+}
+
+1;
index 4c730d1..43d4489 100755 (executable)
@@ -81,7 +81,8 @@ sub AddBibliosToShelf {
         @biblionumber = (split /\//,$biblionumber[0]);
     }
     for my $bib (@biblionumber) {
-        AddToShelf($bib, $shelfnumber, $loggedinuser);
+        my $shelf = Koha::Virtualshelves->find( $shelfnumber );
+        $shelf->add_biblio( $bib, $loggedinuser );
     }
 }
 
index 593ae7d..ce56aaa 100644 (file)
@@ -108,11 +108,11 @@ my $biblionumber2 = _add_biblio('title 2');
 my $biblionumber3 = _add_biblio('title 3');
 my $biblionumber4 = _add_biblio('title 4');
 my $biblionumber5 = _add_biblio('title 5');
-C4::VirtualShelves::AddToShelf( $biblionumber1, $shelf2->shelfnumber, $john_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber2, $shelf2->shelfnumber, $john_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber3, $shelf2->shelfnumber, $john_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber4, $shelf2->shelfnumber, $john_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber5, $shelf2->shelfnumber, $john_doe{borrowernumber} );
+$shelf2->add_biblio( $biblionumber1, $john_doe{borrowernumber} );
+$shelf2->add_biblio( $biblionumber2, $john_doe{borrowernumber} );
+$shelf2->add_biblio( $biblionumber3, $john_doe{borrowernumber} );
+$shelf2->add_biblio( $biblionumber4, $john_doe{borrowernumber} );
+$shelf2->add_biblio( $biblionumber5, $john_doe{borrowernumber} );
 
 my $shelf3 = Koha::Virtualshelf->new(
     {
@@ -125,9 +125,9 @@ my $shelf3 = Koha::Virtualshelf->new(
 my $biblionumber6 = _add_biblio('title 6');
 my $biblionumber7 = _add_biblio('title 7');
 my $biblionumber8 = _add_biblio('title 8');
-C4::VirtualShelves::AddToShelf( $biblionumber6, $shelf3->shelfnumber, $jane_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber7, $shelf3->shelfnumber, $jane_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber8, $shelf3->shelfnumber, $jane_doe{borrowernumber} );
+$shelf3->add_biblio( $biblionumber6, $jane_doe{borrowernumber} );
+$shelf3->add_biblio( $biblionumber7, $jane_doe{borrowernumber} );
+$shelf3->add_biblio( $biblionumber8, $jane_doe{borrowernumber} );
 
 my $shelf4 = Koha::Virtualshelf->new(
     {
@@ -141,10 +141,10 @@ my $biblionumber9  = _add_biblio('title 9');
 my $biblionumber10 = _add_biblio('title 10');
 my $biblionumber11 = _add_biblio('title 11');
 my $biblionumber12 = _add_biblio('title 12');
-C4::VirtualShelves::AddToShelf( $biblionumber9,  $shelf4->shelfnumber, $jane_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber10, $shelf4->shelfnumber, $jane_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber11, $shelf4->shelfnumber, $jane_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber12, $shelf4->shelfnumber, $jane_doe{borrowernumber} );
+$shelf3->add_biblio( $biblionumber9, $jane_doe{borrowernumber} );
+$shelf3->add_biblio( $biblionumber10, $jane_doe{borrowernumber} );
+$shelf3->add_biblio( $biblionumber11, $jane_doe{borrowernumber} );
+$shelf3->add_biblio( $biblionumber12, $jane_doe{borrowernumber} );
 
 my $shelf5 = Koha::Virtualshelf->new(
     {
@@ -160,12 +160,12 @@ my $biblionumber15 = _add_biblio('title 15');
 my $biblionumber16 = _add_biblio('title 16');
 my $biblionumber17 = _add_biblio('title 17');
 my $biblionumber18 = _add_biblio('title 18');
-C4::VirtualShelves::AddToShelf( $biblionumber13, $shelf5->shelfnumber, $jane_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber14, $shelf5->shelfnumber, $jane_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber15, $shelf5->shelfnumber, $jane_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber16, $shelf5->shelfnumber, $jane_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber17, $shelf5->shelfnumber, $jane_doe{borrowernumber} );
-C4::VirtualShelves::AddToShelf( $biblionumber18, $shelf5->shelfnumber, $jane_doe{borrowernumber} );
+$shelf5->add_biblio( $biblionumber13, $jane_doe{borrowernumber} );
+$shelf5->add_biblio( $biblionumber14, $jane_doe{borrowernumber} );
+$shelf5->add_biblio( $biblionumber15, $jane_doe{borrowernumber} );
+$shelf5->add_biblio( $biblionumber16, $jane_doe{borrowernumber} );
+$shelf5->add_biblio( $biblionumber17, $jane_doe{borrowernumber} );
+$shelf5->add_biblio( $biblionumber18, $jane_doe{borrowernumber} );
 
 for my $i ( 6 .. 15 ) {
     Koha::Virtualshelf->new(
diff --git a/t/db_dependent/VirtualShelves.t b/t/db_dependent/VirtualShelves.t
deleted file mode 100755 (executable)
index 18f272b..0000000
+++ /dev/null
@@ -1,135 +0,0 @@
-#!/usr/bin/perl
-
-# This file is a test script for C4::VirtualShelves.pm
-# Author : Antoine Farnault, antoine@koha-fr.org
-# Larger modifications by Jonathan Druart and Marcel de Rooy
-
-use Modern::Perl;
-use Test::More tests => 56;
-use MARC::Record;
-
-use C4::Biblio qw( AddBiblio DelBiblio );
-use C4::Context;
-use C4::Members qw( AddMember );
-
-use Koha::Virtualshelves;
-
-
-my $dbh = C4::Context->dbh;
-$dbh->{RaiseError} = 1;
-$dbh->{AutoCommit} = 0;
-
-# Create some borrowers
-my @borrowernumbers;
-for my $i ( 1 .. 10 ) {
-    my $borrowernumber = AddMember(
-        firstname =>  'my firstname',
-        surname => 'my surname ' . $i,
-        categorycode => 'S',
-        branchcode => 'CPL',
-    );
-    push @borrowernumbers, $borrowernumber;
-}
-
-# Creating some biblios
-my @biblionumbers;
-foreach(0..9) {
-    my ($biblionumber)= AddBiblio(MARC::Record->new, '');
-        #item number ignored
-    push @biblionumbers, $biblionumber;
-}
-
-#----------------------------------------------------------------------#
-#
-#           TESTS START HERE
-#
-#----------------------------------------------------------------------#
-
-use_ok('C4::VirtualShelves');
-
-#-----------------------TEST Virtualshelf constructor------------------------#
-
-# creating shelves (could be <10 when names are not unique)
-my @shelves;
-for my $i (0..9){
-    my $name= randomname();
-    my $catg= int(rand(2))+1;
-    my $shelf = eval {
-        Koha::Virtualshelf->new(
-            {
-                shelfname => $name,
-                category  => $catg,
-                owner     =>$borrowernumbers[$i],
-            }
-        )->store;
-    };
-    if ( $@ or not $shelf ) {
-        my $valid_name = Koha::Virtualshelf->new(
-            {
-                shelfname => $name,
-                category  => $catg,
-                owner     =>$borrowernumbers[$i],
-            }
-        )->is_shelfname_valid;
-        is( $valid_name, 0, 'If the insert has failed, it should be caused by an invalid shelfname (or maybe not?)' );
-    } else {
-        ok($shelf->shelfnumber > -1, "The shelf $i should have been inserted");
-    }
-    push @shelves, {
-        number => $shelf->shelfnumber,
-        name   => $shelf->shelfname,
-        catg   => $shelf->category,
-        owner  => $borrowernumbers[$i],
-    };
-}
-
-#-----------TEST AddToShelf & GetShelfContents &  DelFromShelf functions--------------#
-# usage : &AddToShelf($biblionumber, $shelfnumber);
-# usage : $biblist = &GetShelfContents($shelfnumber);
-# usage : $biblist = GetShelfContents($shelfnumber);
-
-my %used = ();
-for my $i (0..9){
-    my $bib = $biblionumbers[int(rand(9))];
-    my $shelfnumber = $shelves[int(rand(9))]->{number};
-    if($shelfnumber<0) {
-        ok(1, 'skip add to list-test for shelf -1');
-        ok(1, 'skip counting list entries for shelf -1');
-        next;
-    }
-
-    my $key = "$bib\t$shelfnumber";
-    my $should_fail = exists($used{$key}) ? 1 : 0;
-    #FIXME We assume here that we have permission to add..
-    #The different permissions could be tested too.
-
-    my ($biblistBefore,$countbefore) = GetShelfContents($shelfnumber);
-    my $status = AddToShelf($bib,$shelfnumber,$borrowernumbers[$i]);
-    my ($biblistAfter,$countafter) = GetShelfContents($shelfnumber);
-
-    if ($should_fail) {
-        ok(!defined($status), 'failed to add to list when we should');
-    } else {
-        ok(defined($status), 'added to list when we should');
-    }
-
-    if (defined $status) {
-        is($countbefore, $countafter - 1, 'added bib to list');  # the bib has been successfully added.
-    } else {
-        is($countbefore, $countafter, 'did not add duplicate bib to list');
-    }
-
-    $used{$key}++;
-}
-
-#----------------------- SOME SUBS --------------------------------------------#
-
-sub randomname {
-    my $rv='';
-    for(0..19) {
-        $rv.= ('a'..'z','A'..'Z',0..9) [int(rand()*62)];
-    }
-    return $rv;
-}
-
-$dbh->rollback;
index ff0bb1a..36e6188 100644 (file)
@@ -1,20 +1,23 @@
 #!/usr/bin/perl
 
 use Modern::Perl;
-use Test::More tests => 2;
+use Test::More tests => 3;
+use DateTime::Duration;
 
 use C4::Context;
 use Koha::DateUtils;
 use Koha::Virtualshelves;
 use Koha::Virtualshelfshares;
+use Koha::Virtualshelfcontents;
 
 use t::lib::TestBuilder;
 
 my $dbh = C4::Context->dbh;
 $dbh->{AutoCommit} = 0;
 
-$dbh->do(q|DELETE FROM virtualshelves|);
 $dbh->do(q|DELETE FROM virtualshelfshares|);
+$dbh->do(q|DELETE FROM virtualshelfcontents|);
+$dbh->do(q|DELETE FROM virtualshelves|);
 
 my $builder = t::lib::TestBuilder->new;
 
@@ -147,3 +150,89 @@ subtest 'Sharing' => sub {
     $number_of_shelves_shared = Koha::Virtualshelfshares->search->count;
     is( $number_of_shelves_shared, 1, 'To be sure the share has been removed' );
 };
+
+subtest 'Shelf content' => sub {
+
+    plan tests => 18;
+    my $patron1 = $builder->build( { source => 'Borrower', } );
+    my $patron2 = $builder->build( { source => 'Borrower', } );
+    my $biblio1 = $builder->build( { source => 'Biblio', } );
+    my $biblio2 = $builder->build( { source => 'Biblio', } );
+    my $biblio3 = $builder->build( { source => 'Biblio', } );
+    my $biblio4 = $builder->build( { source => 'Biblio', } );
+    my $number_of_contents = Koha::Virtualshelfcontents->search->count;
+
+    is( $number_of_contents, 0, 'No content should exist' );
+
+    my $dt_yesterday = dt_from_string->subtract_duration( DateTime::Duration->new( days => 1 ) );
+    my $shelf = Koha::Virtualshelf->new(
+        {   shelfname    => "my first shelf",
+            owner        => $patron1->{borrowernumber},
+            category     => 1,
+            lastmodified => $dt_yesterday,
+        }
+    )->store;
+
+    $shelf = Koha::Virtualshelves->find( $shelf->shelfnumber );
+    is( output_pref( dt_from_string $shelf->lastmodified ), output_pref($dt_yesterday), 'The lastmodified has been set to yesterday, will be useful for another test later' );
+    my $content1 = $shelf->add_biblio( $biblio1->{biblionumber}, $patron1->{borrowernumber} );
+    is( ref($content1), 'Koha::Virtualshelfcontent', 'add_biblio to a shelf should return a Koha::Virtualshelfcontent object if inserted' );
+    $shelf = Koha::Virtualshelves->find( $shelf->shelfnumber );
+    is( output_pref( dt_from_string( $shelf->lastmodified ) ), output_pref(dt_from_string), 'Adding a biblio to a shelf should update the lastmodified for the shelf' );
+    my $content2 = $shelf->add_biblio( $biblio2->{biblionumber}, $patron1->{borrowernumber} );
+    $number_of_contents = Koha::Virtualshelfcontents->search->count;
+    is( $number_of_contents, 2, '2 biblio should have been inserted' );
+
+    my $content1_bis = $shelf->add_biblio( $biblio1->{biblionumber}, $patron1->{borrowernumber} );
+    is( $content1_bis, undef, 'add_biblio should return undef on duplicate' );    # Or an exception ?
+    $number_of_contents = Koha::Virtualshelfcontents->search->count;
+    is( $number_of_contents, 2, 'The biblio should not have been duplicated' );
+
+    $shelf = Koha::Virtualshelves->find( $shelf->shelfnumber );
+    my $contents = $shelf->get_contents;
+    is( $contents->count, 2, 'There are 2 biblios on this shelf' );
+
+    # Patron 2 will try to remove a content
+    # allow_add = 0, allow_delete_own = 1, allow_delete_other = 0 => Default values
+    my $number_of_deleted_biblios = $shelf->remove_biblios( { biblionumbers => [ $biblio1->{biblionumber} ], borrowernumber => $patron2->{borrowernumber} } );
+    is( $number_of_deleted_biblios, 0, );
+    $number_of_deleted_biblios = $shelf->remove_biblios( { biblionumbers => [ $biblio1->{biblionumber} ], borrowernumber => $patron1->{borrowernumber} } );
+    is( $number_of_deleted_biblios, 1, );
+
+    $number_of_contents = Koha::Virtualshelfcontents->search->count;
+    is( $number_of_contents, 1, 'To be sure the content has been deleted' );
+
+    # allow_delete_own = 0
+    $shelf->allow_delete_own(0);
+    $shelf->store;
+    $number_of_deleted_biblios = $shelf->remove_biblios( { biblionumbers => [ $biblio2->{biblionumber} ], borrowernumber => $patron1->{borrowernumber} } );
+    is( $number_of_deleted_biblios, 1, );
+    $number_of_contents = Koha::Virtualshelfcontents->search->count;
+    is( $number_of_contents, 0, 'The biblio should have been deleted to the shelf by the patron, it is his own content (allow_delete_own=0)' );
+    $shelf->add_biblio( $biblio2->{biblionumber}, $patron1->{borrowernumber} );
+
+    # allow_add = 1, allow_delete_own = 1
+    $shelf->allow_add(1);
+    $shelf->allow_delete_own(1);
+    $shelf->store;
+
+    my $content3 = $shelf->add_biblio( $biblio3->{biblionumber}, $patron2->{borrowernumber} );
+    my $content4 = $shelf->add_biblio( $biblio4->{biblionumber}, $patron2->{borrowernumber} );
+
+    $number_of_contents = Koha::Virtualshelfcontents->search->count;
+    is( $number_of_contents, 3, 'The biblio should have been added to the shelf by the patron 2' );
+
+    $number_of_deleted_biblios = $shelf->remove_biblios( { biblionumbers => [ $biblio3->{biblionumber} ], borrowernumber => $patron2->{borrowernumber} } );
+    is( $number_of_deleted_biblios, 1, );
+    $number_of_contents = Koha::Virtualshelfcontents->search->count;
+    is( $number_of_contents, 2, 'The biblio should have been deleted to the shelf by the patron, it is his own content (allow_delete_own=1) ' );
+
+    # allow_add = 1, allow_delete_own = 1, allow_delete_other = 1
+    $shelf->allow_delete_other(1);
+    $shelf->store;
+
+    $number_of_deleted_biblios = $shelf->remove_biblios( { biblionumbers => [ $biblio2->{biblionumber} ], borrowernumber => $patron2->{borrowernumber} } );
+    is( $number_of_deleted_biblios, 1, );
+    $number_of_contents = Koha::Virtualshelfcontents->search->count;
+    is( $number_of_contents, 1, 'The biblio should have been deleted to the shelf by the patron 2, even if it is not his own content (allow_delete_other=1)' );
+};
index c769c3a..37d300e 100755 (executable)
@@ -127,7 +127,7 @@ sub HandleBiblioPars {
 sub AddBibliosToShelf {
     my ($shelfnumber, @biblionumber)=@_;
     for my $bib (@biblionumber){
-        AddToShelf($bib, $shelfnumber, $loggedinuser);
+        Koha::Virtualshelves->find( $shelfnumber )->add_biblio( $bib, $loggedinuser );
     }
 }