Bug 14544: Get rid of AddShelf
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Wed, 15 Jul 2015 16:06:35 +0000 (17:06 +0100)
committerTomas Cohen Arazi <tomascohen@theke.io>
Thu, 5 Nov 2015 12:58:00 +0000 (09:58 -0300)
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-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
opac/opac-addbybiblionumber.pl
t/db_dependent/Utils/Datatables_Virtualshelves.t
t/db_dependent/VirtualShelves.t
t/db_dependent/Virtualshelves.t [new file with mode: 0644]
virtualshelves/addbybiblionumber.pl

index 72aa7f4..0f5e900 100644 (file)
@@ -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__
index 7ed7f0e..9e3b39d 100644 (file)
@@ -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;
index 51e8793..a9dd808 100644 (file)
@@ -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';
 }
index 46cc916..ea5812a 100644 (file)
@@ -309,12 +309,34 @@ function placeHold () {
 
    <div class="yui-g">[% INCLUDE 'virtualshelves-toolbar.inc' %]
    </div>
+
+[% FOR m IN messages %]
+    <div class="dialog [% m.type %]">
+        [% 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 %]
+    </div>
+[% END %]
+
 [% IF ( paramsloop ) %]
 [% FOREACH paramsloo IN paramsloop %]
 <div class="yui-ge">
     <div class="yui-u first">
-        [% IF ( paramsloo.already ) %]<div class="dialog alert">A List named [% paramsloo.already %] already exists!</div>[% END %]
-        [% IF ( paramsloo.addshelf_failed ) %]<div class="dialog alert">List could not be created. [% IF loggedinuser==0 %](Do not use the database administrator account.)[% END %]</div>[% END %]
                [% IF ( paramsloo.status ) %]<div class="dialog alert">[% paramsloo.string %]</div>[% END %]
                [% IF ( paramsloo.nobarcode ) %]<div class="dialog alert">ERROR: No barcode given.</div>[% END %] 
         [% IF ( paramsloo.noshelfnumber ) %]<div class="dialog alert">ERROR: No list number given.</div>[% END %]
index 1d72ba8..24000db 100644 (file)
 
                     [% IF ( paramsloop ) %]
                         [% FOREACH paramsloo IN paramsloop %]
-                            [% IF ( paramsloo.already ) %]<div class="alert">A list named <b>[% paramsloo.already %]</b> already exists!</div>[% END %]
-                            [% IF ( paramsloo.addshelf_failed ) %]<div class="dialog alert">List could not be created. [% IF loggedinuser==0 %](Do not use the database administrator account.)[% END %]</div>[% END %]
                             [% IF ( paramsloo.status ) %]<div class="alert">[% paramsloo.string %]</div>[% END %]
                             [% IF ( paramsloo.nobarcode ) %]<div class="alert">ERROR: No barcode given.</div>[% END %]
                             [% IF ( paramsloo.noshelfnumber ) %]<div class="alert">ERROR: No shelfnumber given.</div>[% END %]
                                     <ol>
                                         <li>
                                             <label class="required" for="addshelf">List name:</label>
-                                            [% IF ( already ) %]
-                                              <input id="addshelf" type="text" name="addshelf" value="[% already %]" maxlength="255" class="input-fluid" />
+                                            [% IF shelfname %]
+                                              <input id="addshelf" type="text" name="addshelf" value="[% shelfname %]" maxlength="255" class="input-fluid" />
                                             [% ELSE %]
                                               <input id="addshelf" type="text" name="addshelf" maxlength="255" class="input-fluid" />
                                             [% END %]
index 67f6d00..38f95bd 100755 (executable)
@@ -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;
index abecbeb..593ae7d 100644 (file)
@@ -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 = (
index e9af166..07933c5 100755 (executable)
@@ -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 (file)
index 0000000..bfd2dc0
--- /dev/null
@@ -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');
+};
index 0586b6c..2a58ef9 100755 (executable)
@@ -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;