@@ -, +, @@ --- C4/VirtualShelves.pm | 86 +------------------------ C4/VirtualShelves/Page.pm | 8 +-- Koha/Virtualshelf.pm | 64 +++++++++++++++++++ Koha/Virtualshelfcontent.pm | 45 ++++++++++++++ Koha/Virtualshelfcontents.pm | 50 +++++++++++++++ t/db_dependent/VirtualShelves.t | 135 ---------------------------------------- t/db_dependent/Virtualshelves.t | 92 ++++++++++++++++++++++++++- 7 files changed, 255 insertions(+), 225 deletions(-) create mode 100644 Koha/Virtualshelfcontent.pm create mode 100644 Koha/Virtualshelfcontents.pm delete mode 100755 t/db_dependent/VirtualShelves.t --- a/C4/VirtualShelves.pm +++ a/C4/VirtualShelves.pm @@ -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. --- a/C4/VirtualShelves/Page.pm +++ a/C4/VirtualShelves/Page.pm @@ -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}; } } --- a/Koha/Virtualshelf.pm +++ a/Koha/Virtualshelf.pm @@ -23,6 +23,7 @@ use Koha::Database; use Koha::DateUtils qw( dt_from_string ); use Koha::Exceptions; use Koha::Virtualshelfshare; +use Koha::Virtualshelfcontent; use base qw(Koha::Object); @@ -99,6 +100,11 @@ sub get_shares { return $self->{_result}->virtualshelfshares; } +sub get_contents { + my ( $self ) = @_; + return $self->{_result}->virtualshelfcontents; +} + sub share { my ( $self, $key ) = @_; unless ( $key ) { @@ -136,6 +142,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_deleted = 0; + for my $biblionumber ( @$biblionumbers ) { + if ( $self->allow_delete_own ) { + $number_deleted += $self->get_contents->search( + { + biblionumber => $biblionumber, + borrowernumber => $borrowernumber, + } + )->delete; + } + if ( $self->allow_delete_other ) { + $number_deleted += $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_deleted; +} + sub type { return 'Virtualshelve'; } --- a/Koha/Virtualshelfcontent.pm +++ a/Koha/Virtualshelfcontent.pm @@ -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; --- a/Koha/Virtualshelfcontents.pm +++ a/Koha/Virtualshelfcontents.pm @@ -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; --- a/t/db_dependent/VirtualShelves.t +++ a/t/db_dependent/VirtualShelves.t @@ -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; --- a/t/db_dependent/Virtualshelves.t +++ a/t/db_dependent/Virtualshelves.t @@ -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; @@ -143,3 +146,88 @@ 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( scalar(@contents), 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, 0, ); + $number_of_contents = Koha::Virtualshelfcontents->search->count; + is( $number_of_contents, 1, 'The biblio should not have been deleted to the shelf by the patron, even if it is his own content (allow_delete_own=0)' ); + + # 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)' ); +}; --