Attachment 21833 Details for Bug 10363 – [SIGNED-OFF] Bug 10363: Add 2 packages for authorised values

[patch] [SIGNED-OFF] Bug 10363: Add 2 packages for authorised values

SIGNED-OFF-Bug-10363-Add-2-packages-for-authorised.patch (text/plain), 37.84 KB, created by Bernardo Gonzalez Kriegel on 2013-10-06 17:17:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Bernardo Gonzalez Kriegel

Created: 2013-10-06 17:17:06 UTC

Size: 37.84 KB

patch

obsolete

>From 49e93b447007ebebf7dfb10581249bebc9ff8928 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@biblibre.com>
>Date: Tue, 28 May 2013 15:47:59 +0200
>Subject: [PATCH] [SIGNED-OFF] Bug 10363: Add 2 packages for authorised values
>
>Currently, the authorised values are anarchically managed :)
>We have to have a better way to manage them.
>I propose 2 packages in order to execute the sql queries into a package
>instead of directly into the pl script.
>
>This development adds:
>- A new unique key (category and authorised values) for the
>  authorised_values table.
>- 2 new packages Koha::AuthorisedValue and Koha::AuthorisedValues
>- 1 new unit test file in order to test these 2 packages.
>
>It is a code source evolution, but there is no change for the final user.
>
>Test plan:
>1/ Apply the patches
>2/ Check that existing authorised values is displayed on your admin
>page (admin/authorised_values.pl).
>3/ Update an existing one
>4/ Create a new category
>5/ Create a new AV for this category
>6/ Create another one with the same value, an error should be occur.
>7/ Delete one AV.
>8/ Click everywhere on the authorised values admin page and try to find
>a bug.
>9/ Launch t/db_dependent/AuthorisedValues.t and check that all tests
>pass (1 sql error appears, it is normal).
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
>
>Just arrived to this trying to remove one scrolling_list
>from a function on C4/Input.pm, and trying to find a module
>for authorised values.
>
>Re-tested adding, removing and updating authorised values
>No koha-qa errors.
>Unit test reports sucess
>
>Think that this is a good module to have.
>---
> Koha/AuthorisedValue.pm                            |  244 +++++++++++++++
> Koha/AuthorisedValues.pm                           |  160 ++++++++++
> admin/authorised_values.pl                         |  324 ++++++++------------
> .../prog/en/modules/admin/authorised_values.tt     |  101 +++---
> 4 files changed, 580 insertions(+), 249 deletions(-)
> create mode 100644 Koha/AuthorisedValue.pm
> create mode 100644 Koha/AuthorisedValues.pm
>
>diff --git a/Koha/AuthorisedValue.pm b/Koha/AuthorisedValue.pm
>new file mode 100644
>index 0000000..8443976
>--- /dev/null
>+++ b/Koha/AuthorisedValue.pm
>@@ -0,0 +1,244 @@
>+package Koha::AuthorisedValue;
>+
>+use Modern::Perl;
>+
>+use base qw(Class::Accessor);
>+
>+use C4::Context;
>+
>+sub new {
>+    my ( $class, $args ) = @_;
>+    my $av = $args->{av};
>+
>+    unless ( $av ) {
>+        $av->{id} = $args->{id};
>+        $av->{category} = $args->{category};
>+        $av->{authorised_value} = $args->{authorised_value};
>+        $av->{lib} = $args->{lib};
>+        $av->{lib_opac} = $args->{lib_opac};
>+        $av->{imageurl} = $args->{imageurl};
>+        $av->{branches_limitations} = $args->{branches_limitations};
>+    }
>+
>+    my $self = $class->SUPER::new( $av );
>+
>+    bless $self, $class;
>+    return $self;
>+}
>+
>+sub fetch {
>+    my ( $self ) = @_;
>+    my $dbh = C4::Context->dbh;
>+    my $query = q{
>+        SELECT *
>+        FROM authorised_values
>+    };
>+    if ( $self->{id} ) {
>+        $query .= q{ WHERE id = ? }
>+    } else {
>+        $query .= q{ WHERE category = ?
>+            AND authorised_value = ?
>+        };
>+    }
>+    my $data = $dbh->selectrow_hashref(
>+        $query, {},
>+        ( $self->{id} ? $self->{id} : ( $self->{category}, $self->{authorised_value} ) )
>+    );
>+    die "This authorised value does not exist"
>+        . ( $self->{id} ? "id=".$self->{id} : "category=$self->{category} and authorised_value=$self->{authorised_value}" )
>+            unless $data;
>+    $self->{id} = $data->{id};
>+    $self->{category} = $data->{category};
>+    $self->{authorised_value} = $data->{authorised_value};
>+    $self->{lib} = $data->{lib};
>+    $self->{lib_opac} = $data->{lib_opac};
>+    $self->{imageurl} = $data->{imageurl};
>+    return $self;
>+}
>+
>+sub update {
>+    my ( $self ) = @_;
>+    my $dbh = C4::Context->dbh;
>+    local $dbh->{RaiseError} = 1;
>+
>+    die "There is no id defined for this authorised value. I cannot update it" unless $self->{id};
>+
>+    my $query = q{
>+        UPDATE authorised_values
>+        SET category = ?,
>+            authorised_value = ?,
>+            lib = ?,
>+            lib_opac = ?,
>+            imageurl = ?
>+        WHERE id = ?
>+    };
>+    $dbh->do( $query, {}, $self->{category}, $self->{authorised_value}, $self->{lib}, $self->{lib_opac}, $self->{imageurl}, $self->{id} );
>+
>+    $query = q{
>+        DELETE FROM authorised_values_branches
>+        WHERE av_id = ?
>+    };
>+    $dbh->do( $query, {}, $self->{id} );
>+    $query = q{
>+        INSERT INTO authorised_values_branches
>+        ( av_id, branchcode ) VALUES ( ?, ? )
>+    };
>+    my $sth = $dbh->prepare( $query );
>+    for my $branch ( @{ $self->{branches_limitations} } ) {
>+        next unless $branch->{branchcode};
>+        $sth->execute( $self->{id}, $branch->{branchcode} );
>+    }
>+}
>+
>+sub delete {
>+    my ( $self ) = @_;
>+    my $dbh = C4::Context->dbh;
>+    local $dbh->{RaiseError} = 1;
>+    my $query = q{
>+        DELETE FROM authorised_values
>+        WHERE id = ?
>+    };
>+    $dbh->do( $query, {}, $self->{id} );
>+}
>+
>+sub insert {
>+    my ( $self ) = @_;
>+    my $dbh = C4::Context->dbh;
>+    local $dbh->{RaiseError} = 1;
>+    my $query = q{
>+        INSERT INTO authorised_values
>+        ( category, authorised_value, lib, lib_opac, imageurl )
>+        VALUES (?, ?, ?, ?, ?)
>+    };
>+    $dbh->do( $query, {}, $self->{category}, $self->{authorised_value}, $self->{lib}, $self->{lib_opac}, $self->{imageurl} );
>+    $self->{id} = $dbh->{'mysql_insertid'};
>+    $query = q{
>+        INSERT INTO authorised_values_branches
>+        ( av_id, branchcode ) VALUES ( ?, ? )
>+    };
>+    my $sth = $dbh->prepare( $query );
>+    for my $branch ( @{ $self->{branches_limitations} } ) {
>+        next unless $branch->{branchcode};
>+        $sth->execute( $self->{id}, $branch->{branchcode} );
>+    }
>+}
>+
>+sub branches_limitations {
>+    my ( $self, $limitations ) = @_;
>+    my $dbh = C4::Context->dbh;
>+    unless ( defined $limitations ) {
>+        my $sth = $dbh->prepare( q{
>+            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( $self->{id} );
>+        my @limitations;
>+        while ( my $r = $sth->fetchrow_hashref ) {
>+            push @limitations, {
>+                branchcode => $r->{branchcode},
>+                branchname => $r->{branchname},
>+            }
>+        }
>+        $self->{branches_limitations} = \@limitations;
>+        return $self->{branches_limitations};
>+    } else {
>+        die "setter not implemented yet";
>+    }
>+}
>+
>+1;
>+
>+__END__
>+
>+=head1 NAME
>+
>+Koha::AuthorisedValue
>+
>+=head1 SYNOPSIS
>+
>+    use Koha::AuthorisedValue;
>+    my $av1 = Koha::AuthorisedValue->new({id => $id});
>+    my $av2 = Koha::AuthorisedValue->new({av => $av});
>+    my $av3 = Koha::AuthorisedValue->new({category => $category, authorised_value => $authorised_value});
>+    $av1->fetch;
>+    $av2->delete;
>+
>+=head1 DESCRIPTION
>+
>+Class for managing an authorised value into Koha.
>+
>+=head1 METHODS
>+
>+=head2 new
>+
>+Create a new Koha::AuthorisedValue object. This method can be called using several ways.
>+No sql query is done in this method.
>+
>+=over 4
>+
>+=item B<av>
>+
>+    The caller already has all information about the authorised value.
>+    It does want to retrieve them from the database.
>+    The new object is created using these values.
>+
>+=item B<id>
>+
>+    The caller just knows the id of the authorised value.
>+
>+=item B<category and authorised_value>
>+
>+    The caller knows the category and the value of the authorised value.
>+
>+=back
>+
>+=head2 fetch
>+
>+The information will be retrieved from the database.
>+
>+=head2 update
>+
>+If the AuthorisedValue object has been modified and the values have to be modified into the database, call this method.
>+
>+=head2 delete
>+
>+Remove a the record in the database using the id the object.
>+
>+=head2 insert
>+
>+Insert a new AuthorisedValue object into the database.
>+
>+=head2 branches_limitations
>+
>+In the interest of being pleasant with the performance, the branches limitations is not retrieve with the fetch method.
>+If they are required, you should call this method.
>+
>+=head1 AUTHOR
>+
>+Jonathan Druart <jonathan.druart at biblibre.com>
>+
>+=head1 COPYRIGHT
>+
>+Copyright 2013 BibLibre
>+
>+=head1 LICENSE
>+
>+This file is part of Koha.
>+
>+Koha is free software; you can redistribute it and/or modify it under the
>+terms of the GNU General Public License as published by the Free Software
>+Foundation; either version 2 of the License, or (at your option) any later
>+version.
>+
>+Koha is distributed in the hope that it will be useful, but WITHOUT ANY
>+WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
>+A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
>+
>+You should have received a copy of the GNU General Public License along
>+with Koha; if not, write to the Free Software Foundation, Inc.,
>+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>+
>+=cut
>diff --git a/Koha/AuthorisedValues.pm b/Koha/AuthorisedValues.pm
>new file mode 100644
>index 0000000..98543fb
>--- /dev/null
>+++ b/Koha/AuthorisedValues.pm
>@@ -0,0 +1,160 @@
>+package Koha::AuthorisedValues;
>+
>+use Modern::Perl;
>+use List::MoreUtils qw( any uniq );
>+
>+use Koha::AuthorisedValue;
>+
>+use base qw(Class::Accessor);
>+
>+use C4::Context;
>+
>+__PACKAGE__->mk_accessors(qw( authorised_values ));
>+
>+sub new {
>+    my ( $class, $args ) = @_;
>+
>+    my $category = $args->{category};
>+    my $value = $args->{value};
>+    my @authorised_values;
>+
>+    my $dbh = C4::Context->dbh;
>+    my $query = q{
>+        SELECT *
>+        FROM authorised_values
>+    };
>+    $query .= " WHERE category = ?"
>+        if defined $category;
>+    $query .= " AND authorised_value = ?"
>+        if defined $value;
>+    my $sth = $dbh->prepare( $query );
>+    $sth->execute(
>+        defined $category ? $category : (),
>+        defined $value ? $value : ()
>+    );
>+
>+    while ( my $av = $sth->fetchrow_hashref ) {
>+        push @authorised_values,
>+            Koha::AuthorisedValue->new({ av => $av });
>+    }
>+
>+    my $self = $class->SUPER::new( { authorised_values => \@authorised_values } );
>+
>+    bless $self, $class;
>+    return $self;
>+}
>+
>+sub all {
>+    my ( $self ) = @_;
>+    return wantarray
>+        ? @{ $self->authorised_values }
>+        : $self->authorised_values;
>+}
>+
>+sub select {
>+    my ( $self, $select_value ) = @_;
>+    my @avs = $self->all;
>+    for my $av ( @avs ) {
>+        if ( $av->{authorised_value} eq $select_value ) {
>+            $av->{selected} = 1;
>+        }
>+    }
>+    $self->{authorised_values} = \@avs;
>+}
>+
>+sub filter {
>+    my ( $self, $filters ) = @_;
>+    my @avs = $self->all;
>+    while( my ( $key, $value ) = each %$filters ) {
>+        @avs = map {($_->{$key} and $_->{$key} eq $value) ? $_ : ()} @avs;
>+    }
>+    return \@avs;
>+}
>+
>+sub categories {
>+    my ( $self ) = @_;
>+    my @avs = $self->all;
>+    my @categories = uniq map {
>+        $_->{category}
>+    } @avs;
>+
>+    return wantarray
>+        ? @categories
>+        : \@categories;
>+}
>+
>+1;
>+
>+__END__
>+
>+=head1 NAME
>+
>+Koha::AuthorisedValues
>+
>+=head1 SYNOPSIS
>+
>+    use Koha::AuthorisedValues;
>+    my $avs = Koha::AuthorisedValues->new;
>+    my $categories = $avs->categories;
>+    my $av_for_category = $avs->filter( { category => $category } );
>+    $avs->select('value_to_select');
>+
>+=head1 DESCRIPTION
>+
>+Class for managing authorised values into Koha.
>+
>+=head1 METHODS
>+
>+=head2 new
>+
>+Create an array of Koha::AuthorisedValue object.
>+2 optional parameters: authorised_value and category, if at least one of them
>+is specified, the constructor will only build matched authorised values.
>+
>+=head2 all
>+
>+It will get back all constructed objects.
>+
>+=head2 select
>+
>+Select an authorised value given it value.
>+Set the 'selected' key with 1.
>+
>+=head2 filter
>+
>+Return a list of authorised values matching the filters.
>+filters is a arrayref of hashref, i.e.:
>+  [{category => $category, selected => 1}]
>+
>+=head2 categories
>+
>+Return a list of categories exist in the known authorised values.
>+
>+=head1 AUTHOR
>+
>+Jonathan Druart <jonathan.druart at biblibre.com>
>+
>+=head1 COPYRIGHT
>+
>+Copyright 2013 BibLibre
>+
>+=head1 LICENSE
>+
>+Copyright 2013 BibLibre
>+
>+This file is part of Koha.
>+
>+Koha is free software; you can redistribute it and/or modify it under the
>+terms of the GNU General Public License as published by the Free Software
>+Foundation; either version 2 of the License, or (at your option) any later
>+version.
>+
>+Koha is distributed in the hope that it will be useful, but WITHOUT ANY
>+WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
>+A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
>+
>+You should have received a copy of the GNU General Public License along
>+with Koha; if not, write to the Free Software Foundation, Inc.,
>+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>+
>+=cut
>diff --git a/admin/authorised_values.pl b/admin/authorised_values.pl
>index 332f09a..6a0d183 100755
>--- a/admin/authorised_values.pl
>+++ b/admin/authorised_values.pl
>@@ -1,6 +1,7 @@
> #!/usr/bin/perl
> 
> # Copyright 2000-2002 Katipo Communications
>+# Copyright 2013 BibLibre
> #
> # This file is part of Koha.
> #
>@@ -17,8 +18,7 @@
> # with Koha; if not, write to the Free Software Foundation, Inc.,
> # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> 
>-use strict;
>-use warnings;
>+use Modern::Perl;
> 
> use CGI;
> use C4::Auth;
>@@ -26,35 +26,20 @@ use C4::Branch;
> use C4::Context;
> use C4::Koha;
> use C4::Output;
>+use Koha::AuthorisedValue;
>+use Koha::AuthorisedValues;
> 
> 
>-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({});
>-}
>-
> 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.tmpl",
>+    template_name => "admin/authorised_values.tt",
>     authnotrequired => 0,
>     flagsrequired => {parameters => 'parameters_remaining_permissions'},
>     query => $input,
>@@ -62,31 +47,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::AuthorisedValue->new( { id => $id } )->fetch;
>+        $selected_branches = $av->branches_limitations;
>+    } else {
>+        $category = $input->param('category');
>+    }
> 
>     my $branches = GetBranches;
>     my @branches_loop;
> 
>     foreach my $branch (sort keys %$branches) {
>-        my $selected = ( grep {$_->{branchcode} eq $branch} @selected_branches ) ? 1 : 0;
>+        my $selected = ( grep {$_->{branchcode} eq $branch} @$selected_branches ) ? 1 : 0;
>         push @branches_loop, {
>             branchcode => $branches->{$branch}{branchcode},
>             branchname => $branches->{$branch}{branchname},
>@@ -96,152 +72,108 @@ 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') {
>+
>+    $template->param(
>+        category => ( $av ? $av->{category} : $category ),
>+        authorised_value => $av->{authorised_value},
>+        lib              => $av->{lib},
>+        lib_opac         => $av->{lib_opac},
>+        id               => $av->{id},
>+        imagesets        => C4::Koha::getImageSets( checked => $av->{imageurl} ),
>+        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);
>-                }
>+        my $av = Koha::AuthorisedValue->new( { id => $id } );
>+
>+        # TODO in updatedatabase and kohastructure
>+        # Add a uniq key on auv.category and av.authorised_values
>+        #if ( $av->{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 ) {
>+            $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;
>+            $av->{id} = $id;
>+            $av->{branches_limitations} = [
>+                map { {branchcode => $_, branchname => undef} } @branches
>+            ];
>+            if ( $@ ) {
>+                push @messages, {type => 'error', code => 'error_on_update' };
>+            } else {
>+                push @messages, { type => 'message', code => 'success_on_update' };
>             }
>-            $sth->finish;
>-            print "Content-Type: text/html\n\n<META HTTP-EQUIV=Refresh CONTENT=\"0; URL=authorised_values.pl?searchfield=".$new_category."&offset=$offset\"></html>";
>-            exit;
>-        }
>+            $av->update;
>+            $template->param( update_success => 1 );
>+        #}
>     }
>     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);
>-                }
>-            }
>-    	    print "Content-Type: text/html\n\n<META HTTP-EQUIV=Refresh CONTENT=\"0; URL=authorised_values.pl?searchfield=".$input->param('category')."&offset=$offset\"></html>";
>-    	    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,
>+            branches_limitations => [ map { {branchcode => $_, branchname => undef} } @branches ],
>+        } );
>+        eval {$av->insert};
>+        if ( $@ ) {
>+            push @messages, {type => 'error', code => 'error_on_insert' };
>+        } else {
>+            push @messages, { type => 'message', code => 'success_on_insert' };
>         }
>     }
>-    if ( $duplicate_entry ) {       
>+    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,
>-							);
>+                         duplicate_value =>  $new_authorised_value);
>+    }
> 
>-													# 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 "Content-Type: text/html\n\n<META HTTP-EQUIV=Refresh CONTENT=\"0; URL=authorised_values.pl?searchfield=$searchfield&offset=$offset\"></html>";
>-	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::AuthorisedValue->new( { id => $input->param('id') } );
>+    eval {$av->delete};
>+    if ( $@ ) {
>+        push @messages, {type => 'error', code => 'error_on_delete' };
>+    } else {
>+        push @messages, { type => 'message', code => 'success_on_delete' };
>+    }
>+
>+    $op = 'list';
>+    $template->param( delete_success => 1 );
>+}
> 
>-exit 0;
>+$template->param(
>+    op => $op,
>+    searchfield => $searchfield,
>+    messages => \@messages,
>+);
> 
>-sub default_form {
>+if ( $op eq 'list' ) {
>     # build categories list
>-    my $sth = $dbh->prepare("select distinct category from authorised_values");
>-    $sth->execute;
>+    my $avs = Koha::AuthorisedValues->new;
>     my @category_list;
>     my %categories;    # a hash, to check that some hardcoded categories exist.
>-    while ( my ($category) = $sth->fetchrow_array ) {
>+    for my $category ( $avs->categories ) {
>         push( @category_list, $category );
>         $categories{$category} = 1;
>     }
>@@ -251,44 +183,32 @@ sub default_form {
>         push @category_list, $_ unless $categories{$_};
>     }
> 
>-	#reorder the list
>-	@category_list = sort {$a cmp $b} @category_list;
>-	my $tab_list = CGI::scrolling_list(-name=>'searchfield',
>-	        -id=>'searchfield',
>-			-values=> \@category_list,
>-			-default=>"",
>-			-size=>1,
>-			-multiple=>0,
>-			);
>-	if (!$searchfield) {
>-		$searchfield=$category_list[0];
>-	}
>-    my ($results) = AuthorizedValuesForCategory($searchfield);
>-    my $count = scalar(@$results);
>-	my @loop_data = ();
>-	# builds value list
>-    my $dbh = C4::Context->dbh;
>-    $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&amp;id=".$results->[$i]{'id'}."&amp;offset=$offset";
>-		$row_data{delete}                = "$script_name?op=delete_confirm&amp;searchfield=$searchfield&amp;id=".$results->[$i]{'id'}."&amp;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_for_category = $avs->filter( { category => $searchfield } );
>+    my @loop_data = ();
>+    # builds value list
>+    for my $av ( @$avs_for_category ) {
>+        my $branches = $av->branches_limitations;
>+        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}              = $branches;
>+        $row_data{id}                    = $av->{id};
>+        push(@loop_data, \%row_data);
>+    }
> 
>-	$template->param( loop     => \@loop_data,
>-                          tab_list => $tab_list,
>-                          category => $searchfield );
>-}
>+    $template->param(
>+        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 afdd882..1949c64 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' %]
>-<title>Koha &rsaquo; Administration &rsaquo; Authorized values [% IF ( add_form ) %] &rsaquo; [% IF ( action_modify ) %]Modify authorized value[% END %]
>-	   [% IF ( action_add_value ) %] &rsaquo;  New authorized value[% END %]
>-	   [% IF ( action_add_category ) %] &rsaquo; New category[% END %][% END %]
>-[% IF ( delete_confirm ) %] &rsaquo; Confirm deletion[% END %]
>-[% IF ( else ) %]Authorized values[% END %]</title>
>+<title>Koha &rsaquo; Administration &rsaquo; Authorized values
>+[% IF op == 'add_form' %]
>+  [% IF ( action_modify ) %] &rsaquo; Modify authorized value[% END %]
>+  [% IF ( action_add_value ) %] &rsaquo;  New authorized value[% END %]
>+  [% IF ( action_add_category ) %] &rsaquo; New category[% END %]
>+[% END %]
>+</title>
> [% INCLUDE 'doc-head-close.inc' %]
> 
> <link rel="stylesheet" type="text/css" href="[% themelang %]/css/datatables.css" />
>@@ -25,11 +27,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?"));
>+    });
> });
> //]]>
> </script>
> 
>-[% IF ( else ) %]
>+[% IF op == 'list' %]
> <script type="text/javascript">
> //<![CDATA[
> $(document).ready(function() {
>@@ -52,11 +58,10 @@ $(document).ready(function() {
> <body id="admin_authorised_values" class="admin">
> [% INCLUDE 'header.inc' %]
> [% INCLUDE 'cat-search.inc' %]
>-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/admin/admin-home.pl">Administration</a> &rsaquo; [% IF ( add_form ) %] <a href="/cgi-bin/koha/admin/authorised_values.pl">Authorized values</a> &rsaquo; [% IF ( action_modify ) %]Modify authorized value[% END %]
>+<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/admin/admin-home.pl">Administration</a> &rsaquo; [% IF op == 'add_form' %] <a href="/cgi-bin/koha/admin/authorised_values.pl">Authorized values</a> &rsaquo; [% 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 ) %] <a href="/cgi-bin/koha/admin/authorised_values.pl">Authorized values</a> &rsaquo; Confirm deletion[% END %]
>-[% IF ( else ) %]Authorized values[% END %]</div>
>+[% IF op == 'list' %]Authorized values[% END %]</div>
> 
> <div id="doc3" class="yui-t2">
>    
>@@ -64,7 +69,7 @@ $(document).ready(function() {
> 	<div id="yui-main">
> 	<div class="yui-b">
> 
>-[% IF ( add_form ) %]
>+[% IF op == 'add_form' %]
> 	<h1>
> 	   [% IF ( action_modify ) %]Modify authorized value[% END %]
> 	   [% IF ( action_add_value ) %]New authorized value[% END %]
>@@ -73,9 +78,8 @@ $(document).ready(function() {
> 
>     [% IF ( action_modify ) %]<div class="note"><strong>NOTE:</strong> If you change an authorized value, existing records using it won't be updated.</div>[% END %]
> 
>-	<form action="[% script_name %]" name="Aform" method="post">
>-	<input type="hidden" name="op" value="add_validate" />
>-    <input type="hidden" name="offset" value="[% offset %]" />
>+ <form action="/cgi-bin/koha/admin/authorised_values.pl" name="Aform" method="post">
>+    <input type="hidden" name="op" value="add" />
>         <fieldset class="rows"><ol>
>         <li>
>         [% IF ( action_add_category ) %]<label for="category">Category: </label>
>@@ -158,32 +162,7 @@ $(document).ready(function() {
> [% END %]
> 
> 
>-[% IF ( delete_confirm ) %]
>-	<div class="dialog alert">
>-<h3>Confirm deletion</h3>
>-<table>
>-	<tr>
>-		<th>Category</th>
>-		<th>Value</th>
>-		<th>Description</th>
>-		<th>Description (OPAC)</th>
>-	</tr>
>-	<tr>
>-	    <td>[% searchfield %]</td>
>-	    <td>[% Tvalue %]</td>
>-	    <td>[% Tlib %]</td>
>-	    <td>[% Tlib_opac %]</td>
>-	</tr>
>-	</table>
>-	<form action="[% script_name %]" method="post">
>-		<input type="hidden" name="op" value="delete_confirmed" />
>-		<input type="hidden" name="id" value="[% id %]" />
>-        <input type="hidden" name="searchfield" value="[% searchfield %]" /><fieldset class="action"><input type="submit" value="Yes, delete" class="approve" /></form>
>-<form action="[% script_name %]" method="get"><input type="hidden" name="searchfield" value="[% searchfield %]" /><input type="submit" value="No, do not delete" class="deny" /></form>
>-</div>
>-[% END %]
>-
>-[% IF ( else ) %]
>+[% IF op == 'list' %]
> 
> <div id="toolbar" class="btn-toolbar">
>     <a id="addauth" class="btn btn-small" href= "/cgi-bin/koha/admin/authorised_values.pl?op=add_form&amp;category=[% category %]">New authorized value for [% category %]</a>
>@@ -193,11 +172,39 @@ $(document).ready(function() {
> <h1>Authorized values</h1>
> <div class="note"><strong>NOTE:</strong> If you change an authorized value, existing records using it won't be updated.</div>
> 
>-[% IF ( duplicate_category ) %]
>-<div class="dialog alert">Could not add value &quot;[% duplicate_value %]&quot; for category &quot;[% duplicate_category %]&quot; &mdash; value already present.
>-</div>
>+[% FOR m IN messages %]
>+    <div class="dialog [% m.type %]">
>+        [% SWITCH m.code %]
>+        [% CASE 'error_on_update' %]
>+            An error occurred when updating this authorised values. Perhaps the value already exists.
>+        [% CASE 'error_on_insert' %]
>+            An error occurred when inserting this authorised values. Perhaps the value or the category already exists.
>+        [% CASE 'error_on_delete' %]
>+            An error occurred when deleteing this authorised values. Check the logs.
>+        [% CASE 'success_on_update' %]
>+            Authorised value updated with success.
>+        [% CASE 'success_on_insert' %]
>+            Authorised value inserted with success.
>+        [% CASE 'success_on_delete' %]
>+            Authorised value deleted with success.
>+        [% CASE %]
>+            [% m.code %]
>+        [% END %]
>+    </div>
> [% END %]
>-<form action="/cgi-bin/koha/admin/authorised_values.pl" method="post" id="category"><label for="searchfield">Show category: </label>[% tab_list %] <input type="submit" value="Submit" /></form>
>+
>+<form action="/cgi-bin/koha/admin/authorised_values.pl" method="post" id="category">
>+  <label for="searchfield">Show category: </label>
>+  <select id="searchfield" name="searchfield">
>+  [% FOR c IN categories %]
>+    [% IF c == searchfield %]
>+      <option value="[% c %]" selected="selected">[% c %]</option>
>+    [% ELSE %]
>+      <option value="[% c %]">[% c %]</option>
>+    [% END %]
>+  [% END %]
>+  <input type="submit" value="Submit" />
>+</form>
> [% IF ( category == 'Bsort1' ) %]
>     <p>An authorized value attached to patrons, that can be used for stats purposes</p>
> [% END %]
>@@ -276,8 +283,8 @@ $(document).ready(function() {
>             No limitation
>         [% END %]
>     </td>
>-	<td><a href="[% loo.edit %]">Edit</a></td>
>-	<td><a href="[% loo.delete %]">Delete</a></td>
>+    <td><a href="/cgi-bin/koha/admin/authorised_values.pl?op=add_form&amp;id=[% loo.id %]">Edit</a></td>
>+    <td><a class="delete" href="/cgi-bin/koha/admin/authorised_values.pl?op=delete&amp;searchfield=[% searchfield %]&amp;id=[% loo.id %]">Delete</a></td>
> </tr>
> [% END %]
> </tbody></table>[% ELSE %]
>@@ -285,8 +292,8 @@ $(document).ready(function() {
> [% END %]
> 
> [% IF ( isprevpage ) %]
>-<form class="inline" action="[% script_name %]" method="post">
>-<input type="hidden" name="offset" value="[% prevpage %]" /><input type="hidden" name="searchfield" value="[% searchfield %]" />
>+<form class="inline" action="/cgi-bin/koha/admin/authorised_values.pl" method="post">
>+<input type="hidden" name="searchfield" value="[% searchfield %]" />
> 	<input type="submit" value="&lt;&lt; Previous" /></form>
> [% END %] 
> 
>-- 
>1.7.9.5

Actions: View | Diff | Splinter Review

Attachments on bug 10363: 18430 | 18431 | 18432 | 18751 | 18752 | 18753 | 20729 | 21833 | 21834 | 21835 | 21836 | 25284 | 25285 | 25286 | 25287 | 30583 | 30584 | 30585 | 30710 | 30711 | 30712 | 31565 | 32632 | 32633 | 32634 | 33416 | 33417 | 33422 | 33423 | 33860 | 33861 | 33862 | 33889 | 33949 | 35838 | 35839 | 35840 | 35841 | 41290 | 41291 | 41292 | 41293 | 41294 | 41302 | 41343 | 41344 | 41345 | 43112 | 43113 | 43114 | 43550 | 43551 | 43552 | 43553 | 43554 | 43555 | 43556 | 43557 | 43558 | 43559 | 43696 | 43827