From ec743d8932b2edcbcc87fdb0f51be0f143ec6c10 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 10 Nov 2014 14:11:20 +0100 Subject: [PATCH] [PASSED QA] Bug 10363: Use Koha::AuthorisedValue[s] in the admin page Now we have packages, we need use them in the pl script. Test plan: Verify there are no regression on addind/editing/deleting authorised values. Done forget to test the branch limitation. Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi Signed-off-by: Katrin Fischer --- Koha/AuthorisedValues.pm | 40 ++- admin/authorised_values.pl | 313 ++++++++------------- .../prog/en/modules/admin/authorised_values.tt | 105 ++++--- t/db_dependent/AuthorisedValues.t | 18 +- 4 files changed, 206 insertions(+), 270 deletions(-) diff --git a/Koha/AuthorisedValues.pm b/Koha/AuthorisedValues.pm index 8d8193b..ec201c3 100644 --- a/Koha/AuthorisedValues.pm +++ b/Koha/AuthorisedValues.pm @@ -23,7 +23,7 @@ use Carp; use Koha::Database; -use Koha::Borrower; +use Koha::AuthorisedValue; use base qw(Koha::Objects); @@ -46,27 +46,39 @@ my @objects = Koha::AuthorisedValues->search($params); sub search { my ( $self, $params ) = @_; - carp("No branchcode passed in for authorised values search!") - unless $params->{branchcode}; - my $branchcode = $params->{branchcode}; delete( $params->{branchcode} ); - my $rs = $self->_resultset()->search( - { - %$params, - -or => [ - 'authorised_values_branches.branchcode' => undef, - 'authorised_values_branches.branchcode' => $branchcode, - ], - }, - { join => 'authorised_values_branches' } - ); + my $or = + $branchcode + ? { + '-or' => [ + 'authorised_values_branches.branchcode' => undef, + 'authorised_values_branches.branchcode' => $branchcode, + ] + } + : {}; + my $join = $branchcode ? { join => 'authorised_values_branches' } : {}; + my $rs = $self->_resultset() + ->search( { %$params, %$or, }, $join ); my $class = ref($self); return wantarray ? $self->_wrap( $rs->all() ) : $class->new_from_dbic($rs); } +sub categories { + my ( $self ) = @_; + my $rs = $self->_resultset->search( + undef, + { + select => ['category'], + distinct => 1, + order_by => 'category', + }, + ); + return map $_->get_column('category'), $rs->all; +} + =head3 type =cut diff --git a/admin/authorised_values.pl b/admin/authorised_values.pl index 9e83512..022348e 100755 --- a/admin/authorised_values.pl +++ b/admin/authorised_values.pl @@ -17,8 +17,7 @@ # You should have received a copy of the GNU General Public License # along with Koha; if not, see . -use strict; -use warnings; +use Modern::Perl; use CGI qw ( -utf8 ); use C4::Auth; @@ -27,31 +26,15 @@ use C4::Context; use C4::Koha; use C4::Output; - -sub AuthorizedValuesForCategory { - my ($searchstring) = shift or return; - my $dbh = C4::Context->dbh; - $searchstring=~ s/\'/\\\'/g; - my @data=split(' ',$searchstring); - my $sth=$dbh->prepare(' - SELECT id, category, authorised_value, lib, lib_opac, imageurl - FROM authorised_values - WHERE (category = ?) - ORDER BY category, authorised_value - '); - $sth->execute("$data[0]"); - return $sth->fetchall_arrayref({}); -} +use Koha::AuthorisedValues; my $input = new CGI; my $id = $input->param('id'); -my $op = $input->param('op') || ''; -our $offset = $input->param('offset') || 0; -our $searchfield = $input->param('searchfield'); +my $op = $input->param('op') || 'list'; +my $searchfield = $input->param('searchfield'); $searchfield = '' unless defined $searchfield; $searchfield =~ s/\,//g; -our $script_name = "/cgi-bin/koha/admin/authorised_values.pl"; -our $dbh = C4::Context->dbh; +my @messages; our ($template, $borrowernumber, $cookie)= get_template_and_user({ template_name => "admin/authorised_values.tt", @@ -62,31 +45,22 @@ our ($template, $borrowernumber, $cookie)= get_template_and_user({ debug => 1, }); -$template->param( script_name => $script_name, - ($op||'else') => 1 ); ################## ADD_FORM ################################## # called by default. Used to create form to add or modify a record if ($op eq 'add_form') { - my $data; - my @selected_branches; - if ($id) { - my $sth=$dbh->prepare("select id, category, authorised_value, lib, lib_opac, imageurl from authorised_values where id=?"); - $sth->execute($id); - $data=$sth->fetchrow_hashref; - $sth = $dbh->prepare("SELECT b.branchcode, b.branchname FROM authorised_values_branches AS avb, branches AS b WHERE avb.branchcode = b.branchcode AND avb.av_id = ?;"); - $sth->execute( $id ); - while ( my $branch = $sth->fetchrow_hashref ) { - push @selected_branches, $branch; - } - } else { - $data->{'category'} = $input->param('category'); - } + my ( $selected_branches, $category, $av ); + if ($id) { + $av = Koha::AuthorisedValues->new->find( $id ); + $selected_branches = $av->branch_limitations; + } else { + $category = $input->param('category'); + } my $branches = GetBranches; my @branches_loop; foreach my $branchcode ( sort { uc($branches->{$a}->{branchname}) cmp uc($branches->{$b}->{branchname}) } keys %$branches ) { - my $selected = ( grep {$_->{branchcode} eq $branchcode} @selected_branches ) ? 1 : 0; + my $selected = ( grep {$_->{branchcode} eq $branchcode} @$selected_branches ) ? 1 : 0; push @branches_loop, { branchcode => $branchcode, branchname => $branches->{$branchcode}->{branchname}, @@ -96,152 +70,104 @@ if ($op eq 'add_form') { if ($id) { $template->param(action_modify => 1); - $template->param('heading_modify_authorized_value_p' => 1); - } elsif ( ! $data->{'category'} ) { + } elsif ( ! $category ) { $template->param(action_add_category => 1); - $template->param('heading_add_new_category_p' => 1); } else { $template->param(action_add_value => 1); - $template->param('heading_add_authorized_value_p' => 1); } - $template->param('use_heading_flags_p' => 1); - $template->param( category => $data->{'category'}, - authorised_value => $data->{'authorised_value'}, - lib => $data->{'lib'}, - lib_opac => $data->{'lib_opac'}, - id => $data->{'id'}, - imagesets => C4::Koha::getImageSets( checked => $data->{'imageurl'} ), - offset => $offset, - branches_loop => \@branches_loop, - ); - -################## ADD_VALIDATE ################################## -# called by add_form, used to insert/modify data in DB -} elsif ($op eq 'add_validate') { + + if ( $av ) { + $template->param( + category => $av->category, + authorised_value => $av->authorised_value, + lib => $av->lib, + lib_opac => $av->lib_opac, + id => $av->id, + imagesets => C4::Koha::getImageSets( checked => $av->imageurl ), + ); + } else { + $template->param( + category => $category, + imagesets => C4::Koha::getImageSets(), + ); + } + $template->param( + branches_loop => \@branches_loop, + ); + +} elsif ($op eq 'add') { my $new_authorised_value = $input->param('authorised_value'); my $new_category = $input->param('category'); my $imageurl = $input->param( 'imageurl' ) || ''; - $imageurl = '' if $imageurl =~ /removeImage/; + $imageurl = '' if $imageurl =~ /removeImage/; my $duplicate_entry = 0; my @branches = $input->param('branches'); if ( $id ) { # Update - my $sth = $dbh->prepare( "SELECT category, authorised_value FROM authorised_values WHERE id = ? "); - $sth->execute($id); - my ($category, $authorised_value) = $sth->fetchrow_array(); - if ( $authorised_value ne $new_authorised_value ) { - my $sth = $dbh->prepare_cached( "SELECT COUNT(*) FROM authorised_values " . - "WHERE category = ? AND authorised_value = ? and id <> ? "); - $sth->execute($new_category, $new_authorised_value, $id); - ($duplicate_entry) = $sth->fetchrow_array(); - } - unless ( $duplicate_entry ) { - my $sth=$dbh->prepare( 'UPDATE authorised_values - SET category = ?, - authorised_value = ?, - lib = ?, - lib_opac = ?, - imageurl = ? - WHERE id=?' ); - my $lib = $input->param('lib'); - my $lib_opac = $input->param('lib_opac'); - undef $lib if ($lib eq ""); # to insert NULL instead of a blank string - undef $lib_opac if ($lib_opac eq ""); # to insert NULL instead of a blank string - $sth->execute($new_category, $new_authorised_value, $lib, $lib_opac, $imageurl, $id); - if ( @branches ) { - $sth = $dbh->prepare("DELETE FROM authorised_values_branches WHERE av_id = ?"); - $sth->execute( $id ); - $sth = $dbh->prepare( - "INSERT INTO authorised_values_branches - ( av_id, branchcode ) - VALUES ( ?, ? )" - ); - for my $branchcode ( @branches ) { - next if not $branchcode; - $sth->execute($id, $branchcode); - } - } - $sth->finish; - print $input->redirect("/cgi-bin/koha/admin/authorised_values.pl?searchfield=$new_category&offset=$offset"); - exit; + my $av = Koha::AuthorisedValues->new->find( $id ); + + $av->lib( $input->param('lib') || undef ); + $av->lib_opac( $input->param('lib_opac') || undef ); + $av->category( $new_category ); + $av->authorised_value( $new_authorised_value ); + $av->imageurl( $imageurl ); + eval{ + $av->store; + $av->branch_limitations( \@branches ); + }; + if ( $@ ) { + push @messages, {type => 'error', code => 'error_on_update' }; + } else { + push @messages, { type => 'message', code => 'success_on_update' }; } } else { # Insert - my $sth = $dbh->prepare_cached( "SELECT COUNT(*) FROM authorised_values " . - "WHERE category = ? AND authorised_value = ? "); - $sth->execute($new_category, $new_authorised_value); - ($duplicate_entry) = $sth->fetchrow_array(); - unless ( $duplicate_entry ) { - my $sth=$dbh->prepare( 'INSERT INTO authorised_values - ( category, authorised_value, lib, lib_opac, imageurl ) - values (?, ?, ?, ?, ?)' ); - my $lib = $input->param('lib'); - my $lib_opac = $input->param('lib_opac'); - undef $lib if ($lib eq ""); # to insert NULL instead of a blank string - undef $lib_opac if ($lib_opac eq ""); # to insert NULL instead of a blank string - $sth->execute( $new_category, $new_authorised_value, $lib, $lib_opac, $imageurl ); - $id = $dbh->{'mysql_insertid'}; - if ( @branches ) { - $sth = $dbh->prepare( - "INSERT INTO authorised_values_branches - ( av_id, branchcode ) - VALUES ( ?, ? )" - ); - for my $branchcode ( @branches ) { - next if not $branchcode; - $sth->execute($id, $branchcode); - } - } - my $category = $input->param('category'); - print $input->redirect("/cgi-bin/koha/admin/authorised_values.pl?searchfield=$category&offset=$offset"); - exit; + my $av = Koha::AuthorisedValue->new( { + category => $new_category, + authorised_value => $new_authorised_value, + lib => $input->param('lib') || undef, + lib_opac => $input->param('lib_opac') || undef, + imageurl => $imageurl, + } ); + eval { + $av->store; + $av->branch_limitations( \@branches ); + }; + if ( $@ ) { + push @messages, {type => 'error', code => 'error_on_insert' }; + } else { + push @messages, { type => 'message', code => 'success_on_insert' }; } } - if ( $duplicate_entry ) { - $template->param(duplicate_category => $new_category, - duplicate_value => $new_authorised_value, - else => 1); - default_form(); - } - -################## DELETE_CONFIRM ################################## -# called by default form, used to confirm deletion of data in DB -} elsif ($op eq 'delete_confirm') { - my $sth=$dbh->prepare("select category,authorised_value,lib,lib_opac from authorised_values where id=?"); - $sth->execute($id); - my $data=$sth->fetchrow_hashref; - $id = $input->param('id') unless $id; - $template->param(searchfield => $searchfield, - Tlib => $data->{'lib'}, - Tlib_opac => $data->{'lib_opac'}, - Tvalue => $data->{'authorised_value'}, - id =>$id, - ); - # END $OP eq DELETE_CONFIRM -################## DELETE_CONFIRMED ################################## -# called by delete_confirm, used to effectively confirm deletion of data in DB -} elsif ($op eq 'delete_confirmed') { - my $id = $input->param('id'); - my $sth=$dbh->prepare("delete from authorised_values where id=?"); - $sth->execute($id); - print $input->redirect("/cgi-bin/koha/admin/authorised_values.pl?searchfield=$searchfield&offset=$offset"); - exit; - # END $OP eq DELETE_CONFIRMED -################## DEFAULT ################################## -} else { # DEFAULT - default_form(); -} #---- END $OP eq DEFAULT -output_html_with_http_headers $input, $cookie, $template->output; + $op = 'list'; + $searchfield = $new_category; +} elsif ($op eq 'delete') { + my $av = Koha::AuthorisedValues->new->find( $input->param('id') ); + my $deleted = eval {$av->delete}; + if ( $@ or not $deleted ) { + push @messages, {type => 'error', code => 'error_on_delete' }; + } else { + push @messages, { type => 'message', code => 'success_on_delete' }; + } -exit 0; + $op = 'list'; + $template->param( delete_success => 1 ); +} + +$template->param( + op => $op, + searchfield => $searchfield, + messages => \@messages, +); -sub default_form { +if ( $op eq 'list' ) { # build categories list - my $category_list = C4::Koha::GetAuthorisedValueCategories(); - my @category_list = @{$category_list}; + my @categories = Koha::AuthorisedValues->new->categories; + my @category_list; my %categories; # a hash, to check that some hardcoded categories exist. - for my $category ( @category_list ) { + for my $category ( @categories ) { + push( @category_list, $category ); $categories{$category} = 1; } @@ -250,42 +176,31 @@ sub default_form { push @category_list, $_ unless $categories{$_}; } - #reorder the list - @category_list = sort {lc($a) cmp lc($b)} @category_list; - if (!$searchfield) { - $searchfield=$category_list[0]; - } - my $tab_list = { - values => \@category_list, - default => $searchfield, - }; - my ($results) = AuthorizedValuesForCategory($searchfield); - my $count = scalar(@$results); - my @loop_data = (); - # builds value list - my $sth = $dbh->prepare("SELECT b.branchcode, b.branchname FROM authorised_values_branches AS avb, branches AS b WHERE avb.branchcode = b.branchcode AND avb.av_id = ?"); - for (my $i=0; $i < $count; $i++){ - $sth->execute( $results->[$i]{id} ); - my @selected_branches; - while ( my $branch = $sth->fetchrow_hashref ) { - push @selected_branches, $branch; - } - my %row_data; # get a fresh hash for the row data - $row_data{category} = $results->[$i]{'category'}; - $row_data{authorised_value} = $results->[$i]{'authorised_value'}; - $row_data{lib} = $results->[$i]{'lib'}; - $row_data{lib_opac} = $results->[$i]{'lib_opac'}; - $row_data{imageurl} = getitemtypeimagelocation( 'intranet', $results->[$i]{'imageurl'} ); - $row_data{edit} = "$script_name?op=add_form&id=".$results->[$i]{'id'}."&offset=$offset"; - $row_data{delete} = "$script_name?op=delete_confirm&searchfield=$searchfield&id=".$results->[$i]{'id'}."&offset=$offset"; - $row_data{branches} = \@selected_branches; - push(@loop_data, \%row_data); - } + #reorder the list + @category_list = sort {$a cmp $b} @category_list; + + $searchfield ||= $category_list[0]; + + my @avs_by_category = Koha::AuthorisedValues->new->search( { category => $searchfield } ); + my @loop_data = (); + # builds value list + for my $av ( @avs_by_category ) { + my %row_data; # get a fresh hash for the row data + $row_data{category} = $av->category; + $row_data{authorised_value} = $av->authorised_value; + $row_data{lib} = $av->lib; + $row_data{lib_opac} = $av->lib_opac; + $row_data{imageurl} = getitemtypeimagelocation( 'intranet', $av->imageurl ); + $row_data{branches} = $av->branch_limitations; + $row_data{id} = $av->id; + push(@loop_data, \%row_data); + } $template->param( - loop => \@loop_data, - tab_list => $tab_list, - category => $searchfield, + loop => \@loop_data, + category => $searchfield, + categories => \@category_list, ); -} +} +output_html_with_http_headers $input, $cookie, $template->output; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt index 7dae71e..3d34cf1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt @@ -1,9 +1,11 @@ [% INCLUDE 'doc-head-open.inc' %] -Koha › Administration › Authorized values [% IF ( add_form ) %] › [% IF ( action_modify ) %]Modify authorized value[% END %] - [% IF ( action_add_value ) %] › New authorized value[% END %] - [% IF ( action_add_category ) %] › New category[% END %][% END %] -[% IF ( delete_confirm ) %] › Confirm deletion[% END %] -[% IF ( else ) %]Authorized values[% END %] +Koha › Administration › Authorized values +[% IF op == 'add_form' %] + [% IF ( action_modify ) %] › Modify authorized value[% END %] + [% IF ( action_add_value ) %] › New authorized value[% END %] + [% IF ( action_add_category ) %] › New category[% END %] +[% END %] + [% INCLUDE 'doc-head-close.inc' %] @@ -23,11 +25,15 @@ $("#branches option:first").attr("selected", "selected"); } $('#icons').tabs(); + + $("a.delete").click(function(){ + return confirm(_("Are you sure you want to delete this authorised value?")); + }); }); //]]> -[% IF ( else ) %] +[% IF op == 'list' %]