From a421a21c6d2345389c4da26addf72b73000cfdbe Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 15 Jul 2015 17:06:35 +0100 Subject: [PATCH] Bug 14544: Get rid of AddShelf --- 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 diff --git a/C4/VirtualShelves.pm b/C4/VirtualShelves.pm index 72aa7f4..0f5e900 100644 --- a/C4/VirtualShelves.pm +++ b/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__ diff --git a/C4/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm index 7ed7f0e..9e3b39d 100644 --- a/C4/VirtualShelves/Page.pm +++ b/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; diff --git a/Koha/Virtualshelf.pm b/Koha/Virtualshelf.pm index 51e8793..a9dd808 100644 --- a/Koha/Virtualshelf.pm +++ b/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'; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt index 066d7fd..af8c930 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt +++ b/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 %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt index 3478866..08ff95d 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt +++ b/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 @@
  1. - [% IF ( already ) %] - + [% IF shelfname %] + [% ELSE %] [% END %] diff --git a/opac/opac-addbybiblionumber.pl b/opac/opac-addbybiblionumber.pl index 67f6d00..38f95bd 100755 --- a/opac/opac-addbybiblionumber.pl +++ b/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; diff --git a/t/db_dependent/Utils/Datatables_Virtualshelves.t b/t/db_dependent/Utils/Datatables_Virtualshelves.t index abecbeb..593ae7d 100644 --- a/t/db_dependent/Utils/Datatables_Virtualshelves.t +++ b/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 = ( diff --git a/t/db_dependent/VirtualShelves.t b/t/db_dependent/VirtualShelves.t index e9af166..07933c5 100755 --- a/t/db_dependent/VirtualShelves.t +++ b/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 diff --git a/t/db_dependent/Virtualshelves.t b/t/db_dependent/Virtualshelves.t new file mode 100644 index 0000000..bfd2dc0 --- /dev/null +++ b/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'); +}; diff --git a/virtualshelves/addbybiblionumber.pl b/virtualshelves/addbybiblionumber.pl index 0586b6c..2a58ef9 100755 --- a/virtualshelves/addbybiblionumber.pl +++ b/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; -- 2.1.0