Bugzilla – Attachment 42201 Details for
Bug 14544
Move the list related code to Koha::Virtualshelves
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14544: Get rid of AddShelf
Bug-14544-Get-rid-of-AddShelf.patch (text/plain), 31.29 KB, created by
Alex Arnaud
on 2015-09-02 12:34:41 UTC
(
hide
)
Description:
Bug 14544: Get rid of AddShelf
Filename:
MIME Type:
Creator:
Alex Arnaud
Created:
2015-09-02 12:34:41 UTC
Size:
31.29 KB
patch
obsolete
>From 4cdba69a48153fcfa17ecdd9e602fc0e333c9fae Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Wed, 15 Jul 2015 17:06:35 +0100 >Subject: [PATCH] Bug 14544: Get rid of AddShelf > >Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> >--- > 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 () { > > <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 %] >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 ) %]<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 %] >@@ -767,8 +765,8 @@ > <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 %] >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; >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14544
:
41034
|
41035
|
41036
|
41037
|
41038
|
41039
|
41040
|
41041
|
41042
|
41403
|
41404
|
41405
|
41406
|
41407
|
41408
|
41409
|
41410
|
41411
|
41412
|
41584
|
41585
|
41586
|
41587
|
41588
|
41589
|
41590
|
41591
|
41592
|
41593
|
41594
|
41595
|
41596
|
41597
|
41598
|
41599
|
41600
|
41601
|
41716
|
41717
|
41718
|
41719
|
41720
|
41721
|
41722
|
41723
|
41724
|
41725
|
41726
|
41727
|
41728
|
41729
|
41730
|
41731
|
41732
|
41733
|
41766
|
41909
|
41910
|
41913
|
41914
|
41915
|
41917
|
41918
|
41919
|
41924
|
41927
|
41931
|
41932
|
41953
|
41954
|
41955
|
41956
|
41957
|
41958
|
41959
|
41960
|
41961
|
41962
|
41963
|
41964
|
41965
|
41966
|
41967
|
41968
|
41969
|
41970
|
41973
|
41974
|
41975
|
41976
|
41977
|
41978
|
41979
|
41980
|
41981
|
41982
|
41983
|
41986
|
41988
|
42192
|
42199
|
42200
|
42201
|
42202
|
42203
|
42204
|
42205
|
42206
|
42207
|
42208
|
42209
|
42210
|
42211
|
42212
|
42213
|
42214
|
42215
|
42216
|
42217
|
42218
|
42219
|
42638
|
43223
|
43224
|
43225
|
43226
|
43227
|
43228
|
43229
|
43230
|
43231
|
43232
|
43233
|
43234
|
43235
|
43236
|
43237
|
43238
|
43239
|
43240
|
43241
|
43242
|
43243
|
43296
|
43334
|
43335
|
43403
|
43456
|
43457
|
43458
|
43459
|
43460
|
43461
|
43462
|
43463
|
43464
|
43465
|
43466
|
43467
|
43469
|
43470
|
43471
|
43472
|
43473
|
43474
|
43475
|
43476
|
43477
|
43478
|
43479
|
43480
|
43481
|
43482
|
43485
|
43486
|
43487
|
44467
|
44468
|
44469
|
44470
|
44471
|
44472
|
44473
|
44474
|
44475
|
44476
|
44477
|
44478
|
44479
|
44480
|
44481
|
44482
|
44483
|
44484
|
44485
|
44486
|
44487
|
44758
|
44845
|
44852
|
44863
|
44872