From 5ca56676ff509f88cb22217ca409b103343326b0 Mon Sep 17 00:00:00 2001 From: Yohann Dufour Date: Thu, 7 Aug 2014 16:39:50 +0200 Subject: [PATCH] [ALTERNATE POC] Bug 10363 - There is no package for authorised values. I don't think we need to create entirely new pm modules to accomplish this. I think it's better to just extend the Result and ResultSet classes. This is a proof of concept with unit tests. If reactions are positive, I will be more the happy to create a full implementation. --- Koha/Schema/Result/AuthorisedValue.pm | 32 +++++++- Koha/Schema/ResultSet/AuthorisedValue.pm | 41 +++++++++ installer/data/mysql/kohastructure.sql | 1 + installer/data/mysql/updatedatabase.pl | 9 ++ t/db_dependent/AuthorisedValues.t | 137 ++++++++++++++++++++++++++++++ 5 files changed, 218 insertions(+), 2 deletions(-) create mode 100644 Koha/Schema/ResultSet/AuthorisedValue.pm create mode 100644 t/db_dependent/AuthorisedValues.t diff --git a/Koha/Schema/Result/AuthorisedValue.pm b/Koha/Schema/Result/AuthorisedValue.pm index 0366aec..ba35d20 100644 --- a/Koha/Schema/Result/AuthorisedValue.pm +++ b/Koha/Schema/Result/AuthorisedValue.pm @@ -90,6 +90,22 @@ __PACKAGE__->add_columns( __PACKAGE__->set_primary_key("id"); +=head1 UNIQUE CONSTRAINTS + +=head2 C + +=over 4 + +=item * L + +=item * L + +=back + +=cut + +__PACKAGE__->add_unique_constraint("av_uniq", ["category", "authorised_value"]); + =head1 RELATIONS =head2 authorised_values_branches @@ -108,9 +124,21 @@ __PACKAGE__->has_many( ); -# Created by DBIx::Class::Schema::Loader v0.07025 @ 2013-10-14 20:56:21 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:N6Q7Y4sHL03X170zJ3APUA +# Created by DBIx::Class::Schema::Loader v0.07039 @ 2014-08-05 16:05:45 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:9LllSeL5DPjwR2ucEOqU8w + +__PACKAGE__->has_many( + "branch_limitations", + "Koha::Schema::Result::AuthorisedValuesBranch", + { "foreign.av_id" => "self.id" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + +sub add_branch_limitation { + my ( $self, $branchcode ) = @_; + return $self->update_or_create_related('branch_limitations', { branchcode => $branchcode } ); +} # You can replace this text with custom content, and it will be preserved on regeneration 1; diff --git a/Koha/Schema/ResultSet/AuthorisedValue.pm b/Koha/Schema/ResultSet/AuthorisedValue.pm new file mode 100644 index 0000000..28b4ab8 --- /dev/null +++ b/Koha/Schema/ResultSet/AuthorisedValue.pm @@ -0,0 +1,41 @@ +package Koha::Schema::ResultSet::AuthorisedValue; + +use Modern::Perl; + +use List::MoreUtils qw(uniq); + +use base 'DBIx::Class::ResultSet'; + +sub categories { + my ($self) = @_; + + return [ uniq( $self->get_column('category')->all() ) ]; +} + +sub in_category { + my ( $self, $category ) = @_; + + return $self->search( { category => $category } ); +} + +sub for_branch { + my ( $self, $branchcode ) = @_; + + return $self->search( + { + -or => [ + 'authorised_values_branches.branchcode' => $branchcode, + 'authorised_values_branches.branchcode' => undef + ] + }, + { join => 'authorised_values_branches' } + ); +} + +sub by_category_and_branch { + my ( $self, $category, $branchcode ) = @_; + + return $self->in_category( $category )->for_branch( $branchcode ); +} + +1; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 54ac922..ef34271 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -104,6 +104,7 @@ CREATE TABLE `authorised_values` ( -- stores values for authorized values catego `lib_opac` varchar(200) default NULL, -- authorized value description as printed in the OPAC `imageurl` varchar(200) default NULL, -- authorized value URL PRIMARY KEY (`id`), + UNIQUE KEY `av_uniq` (`category`,`authorised_value`), KEY `name` (`category`), KEY `lib` (`lib`), KEY `auth_value_idx` (`authorised_value`) diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index bfe5f71..65d95f2 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -8751,6 +8751,15 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.17.00.XXX"; +if ( CheckVersion($DBversion) ) { + $dbh->do(q{ + ALTER TABLE authorised_values ADD UNIQUE KEY av_uniq(category, authorised_value); + }); + print "Upgrade to $DBversion done (Bug XXXXX: Add unique key on the authorised_values table)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) diff --git a/t/db_dependent/AuthorisedValues.t b/t/db_dependent/AuthorisedValues.t new file mode 100644 index 0000000..a5e3678 --- /dev/null +++ b/t/db_dependent/AuthorisedValues.t @@ -0,0 +1,137 @@ +#!/usr/bin/perl + +use Modern::Perl; +use Test::More tests => 25; + +use C4::Context; +use Koha::Database; + +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +my $schema = Koha::Database->new()->schema(); + +my $rs = $schema->resultset('AuthorisedValue'); + +$dbh->do("DELETE FROM authorised_values"); + +# Tests for Koha::AuthorisedValue + + +# insert +my $av1 = $rs->create({ + category => 'av_for_testing', + authorised_value => 'value 1', + lib => 'display value 1', + lib_opac => 'opac display value 1', + imageurl => 'image1.png', +}); + +my $av1_bis = $rs->create({ + category => 'av_for_testing', + authorised_value => 'value 2', + lib => 'display value 2', + lib_opac => 'opac display value 2', + imageurl => 'image2.png', +}); + +my $id = $av1->id(); + +ok( $id, 'AV 1 is inserted' ); + +my $new_av1 = $rs->find( $id ); + +is( $new_av1->id, $id, "insert: id has been correctly get back" ); +is( $new_av1->authorised_value, $av1->authorised_value, "insert: authorised_value has been correctly get back" ); +is( $new_av1->lib, $av1->lib, "insert: lib has been correctly get back" ); +is( $new_av1->lib_opac, $av1->lib_opac, "insert: lib_opac has been correctly get back" ); +is( $new_av1->imageurl, $av1->imageurl, "insert: imageurl has been correctly get back" ); +is( $new_av1->branch_limitations()->count(), 0 , "insert: branches_limitations is not get back with fetch" ); + +# update +$av1->authorised_value( 'new value 1' ); +$av1->lib( 'new lib 1' ); +$av1->lib_opac( 'new lib opac 1' ); +$av1->imageurl( 'new_image2.png' ); +$av1->update(); + +$av1->add_branch_limitation('CPL'); +$av1->add_branch_limitation('MPL'); +$av1->add_branch_limitation('MPL'); + +$new_av1 = $rs->find( $id ); +is( $new_av1->id, $id, "update: id has been correctly get back" ); +is( $new_av1->authorised_value, 'new value 1', "update: authorised_value has been correctly get back" ); +is( $new_av1->lib, 'new lib 1', "update: lib has been correctly get back" ); +is( $new_av1->lib_opac, 'new lib opac 1', "update: lib_opac has been correctly get back" ); +is( $new_av1->imageurl, 'new_image2.png', "update: imageurl has been correctly get back" ); +is( $new_av1->branch_limitations()->count(), 2, "Authorised value now has two branch limitations" ); + +# delete +$av1->delete(); +is( $av1->in_storage, 0, 'AV 1 is deleted without error' ); + +$new_av1 = $rs->find( $id ); +ok( ! $new_av1, 'AV 1 is removed from the database' ); + +# Tests for Koha::AuthorisedValues +$av1 = $rs->create({ + category => 'av_for_testing_1', + authorised_value => 'value 1', + lib => 'display value 1', + lib_opac => 'opac display value 1', + imageurl => 'image1.png', +}); +my $av2 = $rs->create({ + category => 'av_for_testing_1', + authorised_value => 'value 2', + lib => 'display value 2', + lib_opac => 'opac display value 2', + imageurl => 'image2.png', +}); +my $av3 = $rs->create({ + category => 'av_for_testing_1', + authorised_value => 'value 3', + lib => 'display value 3', + lib_opac => 'opac display value 3', + imageurl => 'image3.png', +}); +my $av4 = $rs->create({ + category => 'av_for_testing_2', + authorised_value => 'value 4', + lib => 'display value 4', + lib_opac => 'opac display value 4', + imageurl => 'image4.png', +}); +my $av5 = $rs->create({ + category => 'av_for_testing_2', + authorised_value => 'value 5', + lib => 'display value 5', + lib_opac => 'opac display value 5', + imageurl => 'image5.png', +}); + +ok( $av1 && $av2 && $av3 && $av4 && $av5, "5 AV inserted with success" ); + +# get categories +my $categories = $rs->categories; +is( grep ({/av_for_testing_1/} @$categories), 1, "av_for_testing_1 is a category"); +is( grep ({/av_for_testing_2/} @$categories), 1, "av_for_testing_2 is a category"); +is( grep ({/av_for_testing_3/} @$categories), 0, "av_for_testing_3 is not a category"); + +# filter +my $avs1 = $rs->in_category( 'av_for_testing_1' ); +my $avs2 = $rs->in_category('av_for_testing_2' ); +is( $avs1->count() , 3, 'There are 3 AVs for category 1' ); +is( $avs2->count(), 2, 'There are 2 AVs for category 2' ); + +is( $rs->in_category('av_for_testing_1')->for_branch('MPL'), 3, "MPL can access 3 values" ); +my $av = $rs->in_category('av_for_testing_1')->first(); +$av->add_branch_limitation( 'CPL' ); +is( $rs->in_category('av_for_testing_1')->for_branch('MPL'), 2, "MPL can access 2 values" ); +$av->add_branch_limitation( 'MPL' ); +is( $rs->in_category('av_for_testing_1')->for_branch('MPL'), 3, "MPL can access 3 values again" ); + +is( $rs->by_category_and_branch('av_for_testing_1', 'MPL'), 3, "MPL can access 3 values" ); + -- 1.7.2.5