From 8f80df6067676288e7aeb65ec704292d497f30f4 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Fri, 26 Aug 2022 11:25:24 +0200 Subject: [PATCH] Bug 26573: Limit patron attribute types to more than one patron category Test plan: 1. Create patron attribute types, some with a category limit, some without 2. Apply patch 3. Run installer/data/mysql/updatedatabase.pl 4. Run misc/devel/update_dbix_class_files.pl --koha-conf 5. Restart koha 6. Verify that existing category limits were kept 7. Create a new patron attribute type and select multiple category limits. Verify that they have been saved correctly. 8. Edit this patron attribute type and modify category limits. Verify that the changes have been saved correctly 9. Go to the patron creation form, and verify that patron attributes are shown/hidden accordingly when categorycode is changed. 10. Go to patron batch modification tool, enter a cardnumber (or a borrowernumber) and submit the form to go to the next step. Now select a patron attribute type that has category limits and verify that the message shown to the right is correct Signed-off-by: Aude Charillon --- Koha/Patron/Attribute/Type.pm | 22 +++++++++ admin/patron-attr-types.pl | 8 +++- .../data/mysql/atomicupdate/bug-26573.pl | 45 +++++++++++++++++++ installer/data/mysql/kohastructure.sql | 21 +++++++-- .../en/modules/admin/patron-attr-types.tt | 25 ++++++++--- .../prog/en/modules/members/memberentrygen.tt | 2 +- .../en/modules/reports/borrowers_stats.tt | 2 +- .../prog/en/modules/tools/modborrowers.tt | 2 +- koha-tmpl/intranet-tmpl/prog/js/members.js | 2 +- .../bootstrap/en/modules/opac-memberentry.tt | 2 +- members/memberentry.pl | 2 +- reports/borrowers_stats.pl | 1 - t/db_dependent/Auth_with_ldap.t | 6 --- t/db_dependent/Koha/Patron/Attribute.t | 1 - t/db_dependent/Koha/Patron/Attribute/Type.t | 35 +++++++++++++++ t/db_dependent/Koha/Patron/Attribute/Types.t | 6 +-- t/db_dependent/api/v1/patrons.t | 2 - .../api/v1/patrons_extended_attributes.t | 9 ---- tools/modborrowers.pl | 6 +-- 19 files changed, 155 insertions(+), 44 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug-26573.pl create mode 100755 t/db_dependent/Koha/Patron/Attribute/Type.t diff --git a/Koha/Patron/Attribute/Type.pm b/Koha/Patron/Attribute/Type.pm index 772cad3c43..7f35b5f94e 100644 --- a/Koha/Patron/Attribute/Type.pm +++ b/Koha/Patron/Attribute/Type.pm @@ -60,6 +60,28 @@ sub attributes { Koha::Patron::Attributes->_new_from_dbic($attributes_rs); } +=head3 categories + +Get or set attribute type's categories + + my @categories = $attribute_type->categories->as_list; + $attribute_type->categories(\@categories); + +=cut + +sub categories { + my ($self, $categories) = @_; + + if ($categories) { + my @categorycodes = map { $_->_result } @$categories; + $self->_result->set_categorycodes(\@categorycodes); + return $self; + } + + my $rs = $self->_result->categorycodes; + return Koha::Patron::Categories->_new_from_dbic($rs); +} + =head2 Internal Methods =cut diff --git a/admin/patron-attr-types.pl b/admin/patron-attr-types.pl index 2cf2bf6f3b..2dcbada557 100755 --- a/admin/patron-attr-types.pl +++ b/admin/patron-attr-types.pl @@ -121,7 +121,6 @@ sub add_update_attribute_type { my $mandatory = $input->param('mandatory') ? 1 : 0; my $authorised_value_category = $input->param('authorised_value_category'); my $display_checkout = $input->param('display_checkout') ? 1 : 0; - my $category_code = $input->param('category_code') || undef; my $class = $input->param('class'); my $attr_type = Koha::Patron::Attribute::Types->find($code); @@ -156,7 +155,6 @@ sub add_update_attribute_type { mandatory => $mandatory, authorised_value_category => $authorised_value_category, display_checkout => $display_checkout, - category_code => $category_code, class => $class, } )->store; @@ -164,6 +162,11 @@ sub add_update_attribute_type { my @branches = grep { ! /^\s*$/ } $input->multi_param('branches'); $attr_type->library_limits( \@branches ); + my @categories = grep { ! /^\s*$/ } $input->multi_param('category_code'); + $attr_type->categories([ + Koha::Patron::Categories->search({ categorycode => \@categories })->as_list + ]); + if ( $op eq 'edit' ) { $template->param( edited_attribute_type => $attr_type->code() ); } @@ -236,6 +239,7 @@ sub edit_attribute_type_form { $can_be_set_to_unique = 0 if $@; $attr_type->unique_id(0); } + $template->param( attribute_type => $attr_type, attribute_type_form => 1, diff --git a/installer/data/mysql/atomicupdate/bug-26573.pl b/installer/data/mysql/atomicupdate/bug-26573.pl new file mode 100644 index 0000000000..91c17d3e7f --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug-26573.pl @@ -0,0 +1,45 @@ +use Modern::Perl; + +return { + bug_number => '26573', + description => 'Add table borrower_attribute_types_categories', + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + + $dbh->do(q{ + CREATE TABLE IF NOT EXISTS `borrower_attribute_types_categories` ( + `borrower_attribute_type_code` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL, + `categorycode` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL, + KEY `borrower_attribute_type_code` (`borrower_attribute_type_code`), + KEY `categorycode` (`categorycode`), + CONSTRAINT `borrower_attribute_types_categories_borrower_attribute_type_code` FOREIGN KEY (`borrower_attribute_type_code`) REFERENCES `borrower_attribute_types` (`code`) ON DELETE CASCADE ON UPDATE CASCADE, + CONSTRAINT `borrower_attribute_types_categories_categorycode` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`) ON DELETE CASCADE ON UPDATE CASCADE, + PRIMARY KEY (`borrower_attribute_type_code`, `categorycode`) + ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; + }); + + say $out "Table 'borrower_attribute_types_categories' created"; + + if (column_exists('borrower_attribute_types', 'category_code')) { + $dbh->do(q{ + INSERT IGNORE INTO borrower_attribute_types_categories (borrower_attribute_type_code, categorycode) + SELECT `code`, `category_code` FROM `borrower_attribute_types` + WHERE `category_code` IS NOT NULL + }); + + say $out "Data migrated from 'borrower_attribute_types.category_code' to 'borrower_attribute_types_categories'"; + + $dbh->do(q{ + ALTER TABLE `borrower_attribute_types` + DROP FOREIGN KEY `borrower_attribute_types_ibfk_1` + }); + $dbh->do(q{ + ALTER TABLE `borrower_attribute_types` + DROP COLUMN `category_code` + }); + + say $out "Column 'borrower_attribute_types.category_code' deleted"; + } + }, +}; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index e88c75f5d4..dc281eff7e 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1129,14 +1129,11 @@ CREATE TABLE `borrower_attribute_types` ( `staff_searchable` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this field is searchable via the patron search in the staff interface (1 for yes, 0 for no)', `authorised_value_category` varchar(32) COLLATE utf8mb4_unicode_ci DEFAULT NULL COMMENT 'foreign key from authorised_values that links this custom field to an authorized value category', `display_checkout` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this field displays in checkout screens', - `category_code` varchar(10) COLLATE utf8mb4_unicode_ci DEFAULT NULL COMMENT 'defines a category for an attribute_type', `class` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '' COMMENT 'defines a class for an attribute_type', `keep_for_pseudonymization` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this field is copied to anonymized_borrower_attributes (1 for yes, 0 for no)', `mandatory` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if the attribute is mandatory or not', PRIMARY KEY (`code`), KEY `auth_val_cat_idx` (`authorised_value_category`), - KEY `category_code` (`category_code`), - CONSTRAINT `borrower_attribute_types_ibfk_1` FOREIGN KEY (`category_code`) REFERENCES `categories` (`categorycode`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; @@ -1157,6 +1154,24 @@ CREATE TABLE `borrower_attribute_types_branches` ( ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; +-- +-- Table structure for table `borrower_attribute_types_categories` +-- + +DROP TABLE IF EXISTS `borrower_attribute_types_categories`; +/*!40101 SET @saved_cs_client = @@character_set_client */; +/*!40101 SET character_set_client = utf8 */; +CREATE TABLE `borrower_attribute_types_categories` ( + `borrower_attribute_type_code` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL, + `categorycode` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL, + KEY `borrower_attribute_type_code` (`borrower_attribute_type_code`), + KEY `categorycode` (`categorycode`), + CONSTRAINT `borrower_attribute_types_categories_borrower_attribute_type_code` FOREIGN KEY (`borrower_attribute_type_code`) REFERENCES `borrower_attribute_types` (`code`) ON DELETE CASCADE ON UPDATE CASCADE, + CONSTRAINT `borrower_attribute_types_categories_categorycode` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`) ON DELETE CASCADE ON UPDATE CASCADE, + PRIMARY KEY (`borrower_attribute_type_code`, `categorycode`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; +/*!40101 SET character_set_client = @saved_cs_client */; + -- -- Table structure for table `borrower_attributes` -- diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt index f1dc68b302..28240998e6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt @@ -224,13 +224,25 @@ Patron attribute types › Administration › Koha
  • - + + [% FOREACH category IN categories %] + [% SET selected = 0 %] + [% IF attribute_type %] + [% FOREACH cat IN attribute_type.categories %] + [% IF cat.categorycode == category.categorycode %] + [% SET selected = 1 %] + [% END %] + [% END %] + [% END %] + [% IF selected %] + + [% ELSE %] + + [% END %] [% END %] -
    Choose one to limit this attribute to one patron type. Please leave blank if you want these attributes to be available for all types of patrons.
    +
    Select "All categories" if this attribute type should always be displayed. Otherwise select categories you want to associate with this value.
  • @@ -378,6 +390,9 @@ Patron attribute types › Administration › Koha if ( $("#branches option:selected").length < 1 ) { $("#branches option:first").attr("selected", "selected"); } + if ( $("#category option:selected").length < 1 ) { + $("#category option:first").attr("selected", "selected"); + } $("#opac_display").change( function() { if ( this.checked ) { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt index 73262a8ba1..e0fa9bdce4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -1546,7 +1546,7 @@ legend:hover { [% pa_loo.lib | html %] [% END %] [% FOREACH patron_attribute IN pa_loo.items %] -
  • +
  • [% IF patron_attribute.mandatory %] [% ELSE %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt index 68d13e0289..a58efd3b5a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt @@ -248,7 +248,7 @@ [% END %] [% FOREACH patron_attribute IN pa_loo.items %] - + [% patron_attribute.code | html %] ([% patron_attribute.description | html %]) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt index 2200c9d52e..412b080492 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt @@ -518,7 +518,7 @@ information_category_node.html(""); if ( category && category.length > 0 ) { - information_category_node.html(_("This attribute will be only applied to the patron's category %s").format(category)); + information_category_node.html(_("This attribute will be only applied to the following patron categories: %s").format(category)); } var disable_input_node = $(li_node).find("input:checkbox[name='disable_input']"); if ( type == 'select' ) { diff --git a/koha-tmpl/intranet-tmpl/prog/js/members.js b/koha-tmpl/intranet-tmpl/prog/js/members.js index 597bb5d141..806c434366 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/members.js +++ b/koha-tmpl/intranet-tmpl/prog/js/members.js @@ -66,7 +66,7 @@ function update_category_code(category_code) { } var mytables = $(".attributes_table"); $(mytables).find("li").hide(); - $(mytables).find(" li[data-category_code='"+category_code+"']").show(); + $(mytables).find(" li[data-category_code~='"+category_code+"']").show(); $(mytables).find(" li[data-category_code='']").show(); //Change password length hint diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt index 04c8c3e655..be5d20b5b1 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt @@ -920,7 +920,7 @@ [% FOREACH pa_value IN pa.values %] [% IF loop.first %][% END %] [% form_id = 'patron-attr-' _ Math.int( Math.rand(1000000) ) %] -
  • +
  • [% IF pa.type.mandatory && pa.type.opac_editable %] [% ELSE %] diff --git a/members/memberentry.pl b/members/memberentry.pl index 95f6e9dd58..64a57f287f 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -857,7 +857,7 @@ sub patron_attributes_form { description => $attr_type->description(), repeatable => $attr_type->repeatable(), category => $attr_type->authorised_value_category(), - category_code => $attr_type->category_code(), + categorycodes => [ map { $_->categorycode } $attr_type->categories->as_list ], mandatory => $attr_type->mandatory(), }; if (exists $attr_hash{$attr_type->code()}) { diff --git a/reports/borrowers_stats.pl b/reports/borrowers_stats.pl index 4e0f645999..a370d2ee4f 100755 --- a/reports/borrowers_stats.pl +++ b/reports/borrowers_stats.pl @@ -486,7 +486,6 @@ sub patron_attributes_form { description => $attr_type->description(), repeatable => $attr_type->repeatable(), category => $attr_type->authorised_value_category(), - category_code => $attr_type->category_code(), }; my $newentry = { %$entry }; diff --git a/t/db_dependent/Auth_with_ldap.t b/t/db_dependent/Auth_with_ldap.t index 7afba77014..0e1516fa95 100755 --- a/t/db_dependent/Auth_with_ldap.t +++ b/t/db_dependent/Auth_with_ldap.t @@ -89,17 +89,11 @@ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; my $attr_type = $builder->build( { source => 'BorrowerAttributeType', - value => { - category_code => $categorycode - } } ); my $attr_type2 = $builder->build( { source => 'BorrowerAttributeType', - value => { - category_code => $categorycode - } } ); diff --git a/t/db_dependent/Koha/Patron/Attribute.t b/t/db_dependent/Koha/Patron/Attribute.t index a24de0fb2d..52cfa512b8 100755 --- a/t/db_dependent/Koha/Patron/Attribute.t +++ b/t/db_dependent/Koha/Patron/Attribute.t @@ -230,7 +230,6 @@ subtest 'store() tests' => sub { mandatory => 0, repeatable => 0, unique_id => 1, - category_code => undef } } ); diff --git a/t/db_dependent/Koha/Patron/Attribute/Type.t b/t/db_dependent/Koha/Patron/Attribute/Type.t new file mode 100755 index 0000000000..1f2283404a --- /dev/null +++ b/t/db_dependent/Koha/Patron/Attribute/Type.t @@ -0,0 +1,35 @@ +#!/usr/bin/perl + +use Modern::Perl; + +use Test::More; +use Koha::Database; +use t::lib::TestBuilder; + +plan tests => 2; + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +use_ok('Koha::Patron::Attribute::Type'); + +subtest 'categories' => sub { + plan tests => 1; + + $schema->storage->txn_begin; + + my $category1 = $builder->build_object( { class => 'Koha::Patron::Categories' } ); + my $category2 = $builder->build_object( { class => 'Koha::Patron::Categories' } ); + Koha::Patron::Attribute::Types->delete(); + my $attribute_type = Koha::Patron::Attribute::Type->new({ + code => 'TEST', + description => 'Test attribute type', + }); + $attribute_type->store(); + $attribute_type->categories([ $category1, $category2 ]); + $attribute_type->store(); + my @categories = $attribute_type->categories->as_list; + is (scalar @categories, 2); + + $schema->storage->txn_rollback; +}; diff --git a/t/db_dependent/Koha/Patron/Attribute/Types.t b/t/db_dependent/Koha/Patron/Attribute/Types.t index c7bb0d2fbc..e0139eaf04 100755 --- a/t/db_dependent/Koha/Patron/Attribute/Types.t +++ b/t/db_dependent/Koha/Patron/Attribute/Types.t @@ -52,13 +52,9 @@ subtest 'new() tests' => sub { is( Koha::Patron::Attribute::Types->search()->count, 1, 'Only one object created' ); - my $cateogory_code - = $builder->build( { source => 'Category' } )->{categorycode}; - - my $attribute_type_with_category = Koha::Patron::Attribute::Type->new( + my $attribute_type_2 = Koha::Patron::Attribute::Type->new( { code => 'code_2', description => 'description', - category_code => $cateogory_code, repeatable => 0 } )->store(); diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index ca742986c2..7cf2b15824 100755 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -506,7 +506,6 @@ subtest 'add() tests' => sub { mandatory => 0, repeatable => 1, unique => 0, - category_code => 'ST' } } ); @@ -517,7 +516,6 @@ subtest 'add() tests' => sub { mandatory => 0, repeatable => 1, unique => 0, - category_code => 'ST' } } ); diff --git a/t/db_dependent/api/v1/patrons_extended_attributes.t b/t/db_dependent/api/v1/patrons_extended_attributes.t index 1766576fcb..1673010fa1 100755 --- a/t/db_dependent/api/v1/patrons_extended_attributes.t +++ b/t/db_dependent/api/v1/patrons_extended_attributes.t @@ -92,7 +92,6 @@ subtest 'add() tests' => sub { mandatory => 1, repeatable => 0, unique_id => 0, - category_code => undef } } ); @@ -103,7 +102,6 @@ subtest 'add() tests' => sub { mandatory => 0, repeatable => 1, unique_id => 0, - category_code => undef } } ); @@ -114,7 +112,6 @@ subtest 'add() tests' => sub { mandatory => 0, repeatable => 0, unique_id => 1, - category_code => undef } } ); @@ -221,7 +218,6 @@ subtest 'overwrite() tests' => sub { mandatory => 1, repeatable => 0, unique_id => 0, - category_code => undef } } ); @@ -232,7 +228,6 @@ subtest 'overwrite() tests' => sub { mandatory => 0, repeatable => 1, unique_id => 0, - category_code => undef } } ); @@ -243,7 +238,6 @@ subtest 'overwrite() tests' => sub { mandatory => 0, repeatable => 0, unique_id => 1, - category_code => undef } } ); @@ -369,7 +363,6 @@ subtest 'delete() tests' => sub { mandatory => 0, repeatable => 1, unique_id => 0, - category_code => undef } } ); @@ -418,7 +411,6 @@ subtest 'update() tests' => sub { mandatory => 0, repeatable => 1, unique_id => 0, - category_code => undef } } ); @@ -429,7 +421,6 @@ subtest 'update() tests' => sub { mandatory => 0, repeatable => 0, unique_id => 1, - category_code => undef } } ); diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl index bb35b8e1c0..5d931b5a67 100755 --- a/tools/modborrowers.pl +++ b/tools/modborrowers.pl @@ -149,10 +149,8 @@ if ( $op eq 'show' ) { options => $options, }; - my $category_code = $attr_type->category_code; - my ( $category_lib ) = map { - ( defined $category_code and $attr_type->category_code eq $category_code ) ? $attr_type->description : () - } @patron_categories; + my @categories = $attr_type->categories->as_list; + my ( $category_lib ) = join(', ', map { $_->description } @categories); push @patron_attributes_codes, { attribute_code => $attr_type->code, -- 2.30.2