@@ -, +, @@
---
C4/VirtualShelves.pm | 72 +---------
C4/VirtualShelves/Page.pm | 46 ++++---
Koha/Virtualshelf.pm | 56 ++++++++
.../prog/en/modules/virtualshelves/shelves.tt | 26 +++-
.../opac-tmpl/bootstrap/en/modules/opac-shelves.tt | 6 +-
opac/opac-addbybiblionumber.pl | 17 ++-
t/db_dependent/Utils/Datatables_Virtualshelves.t | 140 +++++++++++---------
t/db_dependent/VirtualShelves.t | 108 ++++-----------
t/db_dependent/Virtualshelves.t | 75 +++++++++++
virtualshelves/addbybiblionumber.pl | 21 ++-
10 files changed, 312 insertions(+), 255 deletions(-)
create mode 100644 t/db_dependent/Virtualshelves.t
--- a/C4/VirtualShelves.pm
+++ a/C4/VirtualShelves.pm
@@ -41,7 +41,7 @@ BEGIN {
@ISA = qw(Exporter);
@EXPORT = qw(
&GetShelves &GetShelfContents
- &AddToShelf &AddShelf
+ &AddToShelf
&ModShelf
&ShelfPossibleAction
&DelFromShelf &DelShelf
@@ -272,48 +272,6 @@ sub GetShelfContents {
# or newer, for your version of DBI.
}
-=head2 AddShelf
-
- $shelfnumber = &AddShelf($hashref, $owner);
-
-Creates a new virtual shelf. Params passed in a hash like ModShelf.
-
-Returns a code to know what's happen.
- * -1 : if this virtualshelves already exists.
- * $shelfnumber : if success.
-
-=cut
-
-sub AddShelf {
- my ($hashref, $owner)= @_;
- my $dbh = C4::Context->dbh;
-
- #initialize missing hash values to silence warnings
- foreach('shelfname','category', 'sortfield', 'allow_add', 'allow_delete_own', 'allow_delete_other' ) {
- $hashref->{$_}= undef unless exists $hashref->{$_};
- }
-
- return -1 unless _CheckShelfName($hashref->{shelfname}, $hashref->{category}, $owner, 0);
-
- my $query = q|INSERT INTO virtualshelves
- (shelfname,owner,category,sortfield,allow_add,allow_delete_own,allow_delete_other, created_on)
- VALUES (?,?,?,?,?,?,?, NOW())
- |;
-
- my $sth = $dbh->prepare($query);
- $sth->execute(
- $hashref->{shelfname},
- $owner,
- $hashref->{category},
- $hashref->{sortfield},
- $hashref->{allow_add}//0,
- $hashref->{allow_delete_own}//1,
- $hashref->{allow_delete_other}//0 );
- return if $sth->err;
- my $shelfnumber = $dbh->{'mysql_insertid'};
- return $shelfnumber;
-}
-
=head2 AddToShelf
&AddToShelf($biblionumber, $shelfnumber, $borrower);
@@ -758,34 +716,6 @@ sub GetShelfCount {
return $total;
}
-# internal subs
-sub _CheckShelfName {
- my ($name, $cat, $owner, $number)= @_;
-
- my $dbh = C4::Context->dbh;
- my @pars;
- my $query = qq(
- SELECT DISTINCT shelfnumber
- FROM virtualshelves
- LEFT JOIN virtualshelfshares sh USING (shelfnumber)
- WHERE shelfname=? AND shelfnumber<>?);
- if($cat==1 && defined($owner)) {
- $query.= ' AND (sh.borrowernumber=? OR owner=?) AND category=1';
- @pars=($name, $number, $owner, $owner);
- }
- elsif($cat==1 && !defined($owner)) { #owner is null (exceptional)
- $query.= ' AND owner IS NULL AND category=1';
- @pars=($name, $number);
- }
- else { #public list
- $query.= ' AND category=2';
- @pars=($name, $number);
- }
- my $sth = $dbh->prepare($query);
- $sth->execute(@pars);
- return $sth->rows>0? 0: 1;
-}
-
1;
__END__
--- a/C4/VirtualShelves/Page.pm
+++ a/C4/VirtualShelves/Page.pm
@@ -53,6 +53,7 @@ BEGIN {
$debug = $ENV{DEBUG} || 0;
}
+my @messages;
our %pages = (
intranet => { redirect => '/cgi-bin/koha/virtualshelves/shelves.pl', },
opac => { redirect => '/cgi-bin/koha/opac-shelves.pl', },
@@ -341,30 +342,40 @@ sub shelfpage {
if ( $query->param('shelves') ) {
my $stay = 1;
- #Add a shelf
- if ( my $newshelf = $query->param('addshelf') ) {
+ #Add a shelf
+ my $shelfname = $query->param('addshelf');
+
+ if ( $shelfname ) {
# note: a user can always add a new shelf (except database administrator account)
- my $shelfnumber = AddShelf( {
- shelfname => $newshelf,
- sortfield => $query->param('sortfield'),
- category => $query->param('category'),
- allow_add => $query->param('allow_add'),
- allow_delete_own => $query->param('allow_delete_own'),
- allow_delete_other => $query->param('allow_delete_other'),
- },
- $query->param('owner') );
- $stay = 1;
- if( !$shelfnumber ) {
- push @paramsloop, { addshelf_failed => 1 };
- } elsif ( $shelfnumber == -1 ) { #shelf already exists.
+ my $shelf = eval {
+ Koha::Virtualshelf->new(
+ {
+ shelfname => $shelfname,
+ sortfield => $query->param('sortfield'),
+ category => $query->param('category'),
+ allow_add => $query->param('allow_add'),
+ allow_delete_own => $query->param('allow_delete_own'),
+ allow_delete_other => $query->param('allow_delete_other'),
+ owner => $query->param('owner'),
+ }
+ )->store;
+ };
+ if ( $@ ) {
$showadd = 1;
- push @paramsloop, { already => $newshelf };
- $template->param( shelfnumber => $shelfnumber );
+ push @messages, { type => 'error', code => ref($@) };
+ } elsif ( not $shelf ) {
+ $showadd = 1;
+ push @messages, { type => 'error', 'error_on_insert' };
} else {
print $query->redirect( $pages{$type}->{redirect} . "?viewshelf=$shelfnumber" );
exit;
}
+
+ $template->param(
+ shelfname => $shelfname,
+ );
+ $stay = 1;
}
#Deleting a shelf (asking for confirmation if it has entries)
@@ -503,6 +514,7 @@ sub shelfpage {
barshelvesloop => $barshelves,
pubshelves => $total->{pubtotal},
pubshelvesloop => $pubshelves,
+ messages => \@messages,
);
output_html_with_http_headers $query, $cookie, $template->output;
--- a/Koha/Virtualshelf.pm
+++ a/Koha/Virtualshelf.pm
@@ -20,6 +20,8 @@ use Modern::Perl;
use Carp;
use Koha::Database;
+use Koha::DateUtils qw( dt_from_string );
+use Koha::Exception::DuplicateObject;
use base qw(Koha::Object);
@@ -37,6 +39,60 @@ Koha::Virtualshelf - Koha Virtualshelf Object class
=cut
+our $PRIVATE = 1;
+our $PUBLIC = 2;
+
+sub store {
+ my ( $self ) = @_;
+
+ unless ( $self->is_shelfname_valid ) {
+ Koha::Exceptions::Virtualshelves::DuplicateObject->throw;
+ }
+
+ $self->allow_add( 0 )
+ unless defined $self->allow_add;
+ $self->allow_delete_own( 1 )
+ unless defined $self->allow_delete_own;
+ $self->allow_delete_other( 0 )
+ unless defined $self->allow_delete_other;
+
+ $self->created_on( dt_from_string );
+
+ return $self->SUPER::store( $self );
+}
+
+sub is_shelfname_valid {
+ my ( $self ) = @_;
+
+ my $conditions = {
+ shelfname => $self->shelfname,
+ ( $self->shelfnumber ? ( "me.shelfnumber" => { '!=', $self->shelfnumber } ) : () ),
+ };
+
+ if ( $self->category == $PRIVATE and defined $self->owner ) {
+ $conditions->{-or} = {
+ "virtualshelfshares.borrowernumber" => $self->owner,
+ "me.owner" => $self->owner,
+ };
+ $conditions->{category} = $PRIVATE;
+ }
+ elsif ( $self->category == $PRIVATE and not defined $self->owner ) {
+ $conditions->{owner} = undef;
+ $conditions->{category} = $PRIVATE;
+ }
+ else {
+ $conditions->{category} = $PUBLIC;
+ }
+
+ my $count = Koha::Virtualshelves->search(
+ $conditions,
+ {
+ join => 'virtualshelfshares',
+ }
+ )->count;
+ return $count ? 0 : 1;
+}
+
sub type {
return 'Virtualshelve';
}
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
+++ a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
@@ -309,12 +309,34 @@ function placeHold () {
[% INCLUDE 'virtualshelves-toolbar.inc' %]
+
+[% FOR m IN messages %]
+
+ [% SWITCH m.code %]
+ [% CASE 'error_on_update' %]
+ An error occurred when updating this list. Perhaps the value already exists.
+ [% CASE 'error_on_insert' %]
+ An error occurred when inserting this list. Perhaps the name already exists.
+ [% CASE 'error_on_delete' %]
+ An error occurred when deleteing this list. Check the logs.
+ [% CASE 'success_on_update' %]
+ List updated with success.
+ [% CASE 'success_on_insert' %]
+ List inserted with success.
+ [% CASE 'success_on_delete' %]
+ List deleted with success.
+ [% CASE 'Koha::Exception::DuplicateObject' %]
+ An error occurred when inserting this list. The name already [% shelfname %] exists.
+ [% CASE %]
+ [% m.code %]
+ [% END %]
+
+[% END %]
+
[% IF ( paramsloop ) %]
[% FOREACH paramsloo IN paramsloop %]
- [% IF ( paramsloo.already ) %]
A List named [% paramsloo.already %] already exists!
[% END %]
- [% IF ( paramsloo.addshelf_failed ) %]
List could not be created. [% IF loggedinuser==0 %](Do not use the database administrator account.)[% END %]
[% END %]
[% IF ( paramsloo.status ) %]
[% paramsloo.string %]
[% END %]
[% IF ( paramsloo.nobarcode ) %]
ERROR: No barcode given.
[% END %]
[% IF ( paramsloo.noshelfnumber ) %]
ERROR: No list number given.
[% END %]
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
+++ a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
@@ -109,8 +109,6 @@
[% IF ( paramsloop ) %]
[% FOREACH paramsloo IN paramsloop %]
- [% IF ( paramsloo.already ) %]
A list named [% paramsloo.already %] already exists!
[% END %]
- [% IF ( paramsloo.addshelf_failed ) %]
List could not be created. [% IF loggedinuser==0 %](Do not use the database administrator account.)[% END %]
[% END %]
[% IF ( paramsloo.status ) %]
[% paramsloo.string %]
[% END %]
[% IF ( paramsloo.nobarcode ) %]
ERROR: No barcode given.
[% END %]
[% IF ( paramsloo.noshelfnumber ) %]
ERROR: No shelfnumber given.
[% END %]
@@ -767,8 +765,8 @@
- [% IF ( already ) %]
-
+ [% IF shelfname %]
+
[% ELSE %]
[% END %]
--- a/opac/opac-addbybiblionumber.pl
+++ a/opac/opac-addbybiblionumber.pl
@@ -1,9 +1,6 @@
#!/usr/bin/perl
#script to provide virtualshelf management
-# WARNING: This file uses 4-character tabs!
-#
-# $Header$
#
# Copyright 2000-2002 Katipo Communications
#
@@ -90,10 +87,16 @@ sub AddBibliosToShelf {
sub HandleNewVirtualShelf {
if($authorized= ShelfPossibleAction($loggedinuser, undef, $category==1? 'new_private': 'new_public')) {
- $shelfnumber = AddShelf( {
- shelfname => $newvirtualshelf,
- category => $category }, $loggedinuser);
- if($shelfnumber == -1) {
+ my $shelf = eval {
+ Koha::Virtualshelf->new(
+ {
+ shelfname => $newvirtualshelf,
+ category => $category,
+ owner => $loggedinuser,
+ }
+ );
+ };
+ if ( $@ or not $shelf ) {
$authorized=0;
$errcode=1;
return;
--- a/t/db_dependent/Utils/Datatables_Virtualshelves.t
+++ a/t/db_dependent/Utils/Datatables_Virtualshelves.t
@@ -26,6 +26,9 @@ use C4::Members;
use C4::VirtualShelves;
+use Koha::Virtualshelf;
+use Koha::Virtualshelves;
+
use_ok( "C4::Utils::DataTables::VirtualShelves" );
my $dbh = C4::Context->dbh;
@@ -83,89 +86,96 @@ $john_doe{borrowernumber} = AddMember( %john_doe );
$jane_doe{borrowernumber} = AddMember( %jane_doe );
$john_smith{borrowernumber} = AddMember( %john_smith );
-my $shelf1 = {
- shelfname => 'my first private list (empty)',
- category => 1, # private
- sortfield => 'author',
-};
-$shelf1->{shelfnumber} = C4::VirtualShelves::AddShelf( $shelf1, $john_doe{borrowernumber} );
-
-my $shelf2 = {
- shelfname => 'my second private list',
- category => 1, # private
- sortfield => 'title',
-};
-$shelf2->{shelfnumber} = C4::VirtualShelves::AddShelf( $shelf2, $john_doe{borrowernumber} );
+my $shelf1 = Koha::Virtualshelf->new(
+ {
+ shelfname => 'my first private list (empty)',
+ category => 1, # private
+ sortfield => 'author',
+ owner => $john_doe{borrowernumber},
+ }
+)->store;
+
+my $shelf2 = Koha::Virtualshelf->new(
+ {
+ shelfname => 'my second private list',
+ category => 1, # private
+ sortfield => 'title',
+ owner => $john_doe{borrowernumber},
+ }
+)->store;
my $biblionumber1 = _add_biblio('title 1');
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} );
-
-my $shelf3 = {
- shelfname => 'The first public list',
- category => 2, # public
- sortfield => 'author',
-};
-$shelf3->{shelfnumber} = C4::VirtualShelves::AddShelf( $shelf3, $jane_doe{borrowernumber} );
+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} );
+
+my $shelf3 = Koha::Virtualshelf->new(
+ {
+ shelfname => 'The first public list',
+ category => 2, # public
+ sortfield => 'author',
+ owner => $jane_doe{borrowernumber},
+ }
+)->store;
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} );
-
-my $shelf4 = {
- shelfname => 'my second public list',
- category => 2, # public
- sortfield => 'title',
-};
-$shelf4->{shelfnumber} = C4::VirtualShelves::AddShelf( $shelf4, $jane_doe{borrowernumber} );
+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} );
+
+my $shelf4 = Koha::Virtualshelf->new(
+ {
+ shelfname => 'my second public list',
+ category => 2, # public
+ sortfield => 'title',
+ owner => $jane_doe{borrowernumber},
+ }
+)->store;
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} );
-
-my $shelf5 = {
- shelfname => 'my third private list',
- category => 1, # private
- sortfield => 'title',
-};
-$shelf5->{shelfnumber} = C4::VirtualShelves::AddShelf( $shelf5, $jane_doe{borrowernumber} );
+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} );
+
+my $shelf5 = Koha::Virtualshelf->new(
+ {
+ shelfname => 'my third private list',
+ category => 1, # private
+ sortfield => 'title',
+ owner => $jane_doe{borrowernumber},
+ }
+)->store;
my $biblionumber13 = _add_biblio('title 13');
my $biblionumber14 = _add_biblio('title 14');
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} );
-
-C4::VirtualShelves::AddShelf( { shelfname => 'another public list 6', category => 2}, $john_smith{borrowernumber} );
-C4::VirtualShelves::AddShelf( { shelfname => 'another public list 7', category => 2}, $john_smith{borrowernumber} );
-C4::VirtualShelves::AddShelf( { shelfname => 'another public list 8', category => 2}, $john_smith{borrowernumber} );
-C4::VirtualShelves::AddShelf( { shelfname => 'another public list 9', category => 2}, $john_smith{borrowernumber} );
-C4::VirtualShelves::AddShelf( { shelfname => 'another public list 10', category => 2}, $john_smith{borrowernumber} );
-C4::VirtualShelves::AddShelf( { shelfname => 'another public list 11', category => 2}, $john_smith{borrowernumber} );
-C4::VirtualShelves::AddShelf( { shelfname => 'another public list 12', category => 2}, $john_smith{borrowernumber} );
-C4::VirtualShelves::AddShelf( { shelfname => 'another public list 13', category => 2}, $john_smith{borrowernumber} );
-C4::VirtualShelves::AddShelf( { shelfname => 'another public list 14', category => 2}, $john_smith{borrowernumber} );
-C4::VirtualShelves::AddShelf( { shelfname => 'another public list 15', category => 2}, $john_smith{borrowernumber} );
-
-
+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} );
+
+for my $i ( 6 .. 15 ) {
+ Koha::Virtualshelf->new(
+ {
+ shelfname => "another public list $i",
+ category => 2,
+ owner => $john_smith{borrowernumber},
+ }
+ )->store;
+}
# Set common datatables params
my %dt_params = (
--- a/t/db_dependent/VirtualShelves.t
+++ a/t/db_dependent/VirtualShelves.t
@@ -5,7 +5,7 @@
# Larger modifications by Jonathan Druart and Marcel de Rooy
use Modern::Perl;
-use Test::More tests => 96;
+use Test::More tests => 56;
use MARC::Record;
use C4::Biblio qw( AddBiblio DelBiblio );
@@ -47,65 +47,40 @@ foreach(0..9) {
use_ok('C4::VirtualShelves');
-#-----------------------TEST AddShelf function------------------------#
-# usage : $shelfnumber = &AddShelf( $shelfname, $owner, $category);
+#-----------------------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 $ShelfNumber= AddShelf(
- {
- shelfname => $name,
- category => $catg,
- },
- $borrowernumbers[$i]);
-
- if($ShelfNumber>-1) {
- ok($ShelfNumber > -1, "created shelf"); # Shelf creation successful;
- }
- else {
- my $t= C4::VirtualShelves::_CheckShelfName(
- $name, $catg, $borrowernumbers[$i], 0);
- is($t, 0, "Name clash expected on shelf creation");
+ 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 => $ShelfNumber,
- name => $name,
- catg => $catg,
+ number => $shelf->shelfnumber,
+ name => $shelf->shelfname,
+ catg => $shelf->category,
owner => $borrowernumbers[$i],
- }; #also push the errors
-}
-
-# try to create shelves with duplicate names
-for my $i (0..9){
- if($shelves[$i]->{number}<0) {
- ok(1, 'skip duplicate test for earlier name clash');
- next;
- }
- my $shelf = Koha::Virtualshelves->find( $shelves[$i]->{number} );
-
- # A shelf name is not per se unique!
- if( $shelf->category == 2 ) { #public list: try to create with same name
- my $badNumShelf= AddShelf( {
- shelfname=> $shelves[$i]->{name},
- category => 2
- }, $borrowernumbers[$i]);
- is($badNumShelf, -1, 'do not create public lists with duplicate names');
- #AddShelf returns -1 if name already exist.
- DelShelf($badNumShelf) if $badNumShelf>-1; #delete if went wrong..
- }
- else { #private list, try to add another one for SAME user (owner)
- my $badNumShelf= defined($shelf->owner)? AddShelf(
- {
- shelfname=> $shelves[$i]->{name},
- category => 1,
- },
- $shelf->owner): -1;
- is($badNumShelf, -1, 'do not create private lists with duplicate name for same user');
- DelShelf($badNumShelf) if $badNumShelf>-1; #delete if went wrong..
- }
+ };
}
#-----------TEST AddToShelf & GetShelfContents & DelFromShelf functions--------------#
@@ -147,37 +122,6 @@ for my $i (0..9){
$used{$key}++;
}
-#-----------------------TEST ModShelf & Koha::Virtualshelves->find functions/methods------------------------#
-# usage : ModShelf($shelfnumber, $shelfname, $owner, $category )
-
-for my $i (0..9){
- my $rand = int(rand(9));
- my $numA = $shelves[$rand]->{number};
- if($numA<0) {
- ok(1, 'Skip ModShelf test for shelf -1');
- ok(1, 'Skip ModShelf test for shelf -1');
- ok(1, 'Skip ModShelf test for shelf -1');
- next;
- }
- my $newname= randomname();
- my $shelf = {
- shelfname => $newname,
- category => 3-$shelves[$rand]->{catg}, # tric: 1->2 and 2->1
- };
- #check name change (with category change)
- if(C4::VirtualShelves::_CheckShelfName($newname,$shelf->{category},
- $shelves[$rand]->{owner}, $numA)) {
- ModShelf($numA,$shelf);
- my $shelf_b = Koha::Virtualshelves->find( $numA );
- is($numA, $shelf_b->shelfnumber, 'modified shelf');
- is($shelf->{shelfname}, $shelf_b->shelfname, '... and name change took');
- is($shelf->{category}, $shelf_b->category, '... and category change took');
- }
- else {
- ok(1, "No ModShelf for $newname") for 1..3;
- }
-}
-
#----------------------- TEST AddShare ----------------------------------------#
#first count the number of shares in the table
--- a/t/db_dependent/Virtualshelves.t
+++ a/t/db_dependent/Virtualshelves.t
@@ -0,0 +1,75 @@
+#!/usr/bin/perl
+
+use Modern::Perl;
+use Test::More tests => 1;
+
+use C4::Context;
+use Koha::DateUtils;
+use Koha::Virtualshelf;
+use Koha::Virtualshelves;
+
+use t::lib::TestBuilder;
+
+my $dbh = C4::Context->dbh;
+$dbh->{AutoCommit} = 0;
+
+$dbh->do(q|DELETE FROM virtualshelves|);
+
+my $builder = t::lib::TestBuilder->new;
+
+subtest 'CRUD' => sub {
+ plan tests => 11;
+ my $patron = $builder->build({
+ source => 'Borrower',
+ });
+
+ my $number_of_shelves = Koha::Virtualshelves->search->count;
+
+ is( $number_of_shelves, 0, 'No shelves should exist' );
+
+ my $shelf = Koha::Virtualshelf->new({
+ shelfname => "my first shelf",
+ owner => $patron->{borrowernumber},
+ category => 1,
+ }
+ )->store;
+
+ is( ref( $shelf ), 'Koha::Virtualshelf', 'The constructor should return a valid object' );
+
+ $number_of_shelves = Koha::Virtualshelves->search->count;
+ is( $number_of_shelves, 1, '1 shelf should have been inserted' );
+ is( $shelf->allow_add, 0, 'The default value for allow_add should be 1' );
+ is( $shelf->allow_delete_own, 1, 'The default value for allow_delete_own should be 0' );
+ is( $shelf->allow_delete_other, 0, 'The default value for allow_delete_other should be 0' );
+ is( output_pref($shelf->created_on), output_pref(dt_from_string), 'The creation time should have been set to today' );
+
+ my $retrieved_shelf = Koha::Virtualshelves->find( $shelf->shelfnumber );
+
+ is( $retrieved_shelf->shelfname, $shelf->shelfname, 'Find should correctly return the shelfname' );
+
+ # Insert with the same name
+ eval {
+ $shelf = Koha::Virtualshelf->new({
+ shelfname => "my first shelf",
+ owner => $patron->{borrowernumber},
+ category => 1,
+ }
+ )->store;
+ };
+ is( ref($@), 'Koha::Exception::DuplicateObject' );
+ $number_of_shelves = Koha::Virtualshelves->search->count;
+ is( $number_of_shelves, 1, 'To be sure the number of shelves is still 1' );
+
+ my $another_patron = $builder->build({
+ source => 'Borrower',
+ });
+
+ $shelf = Koha::Virtualshelf->new({
+ shelfname => "my first shelf",
+ owner => $another_patron->{borrowernumber},
+ category => 1,
+ }
+ )->store;
+ $number_of_shelves = Koha::Virtualshelves->search->count;
+ is( $number_of_shelves, 2, 'Another patron should be able to create a shelf with an existing shelfname');
+};
--- a/virtualshelves/addbybiblionumber.pl
+++ a/virtualshelves/addbybiblionumber.pl
@@ -132,15 +132,22 @@ sub AddBibliosToShelf {
}
sub HandleNewVirtualShelf {
- $shelfnumber = AddShelf( {
- shelfname => $newvirtualshelf,
- sortfield => $sortfield,
- category => $category }, $loggedinuser);
- if($shelfnumber == -1) {
- $authorized=0;
- $errcode=1; #add failed
+ my $shelf = eval {
+ Koha::Virtualshelf->new(
+ {
+ shelfname => $newvirtualshelf,
+ category => $category,
+ sortfield => $sortfield,
+ owner => $loggedinuser,
+ }
+ );
+ };
+ if ( $@ or not $shelf ) {
+ $authorized = 0;
+ $errcode = 1;
return;
}
+
AddBibliosToShelf($shelfnumber, @biblionumber);
#Reload the page where you came from
print $query->header;
--