@@ -, +, @@ editing patron attribute type --- Koha/Exceptions/Patron/Attribute/Type.pm | 31 ++++++ Koha/Patron/Attribute/Type.pm | 85 +++++++++++++++ admin/patron-attr-types.pl | 24 ++++- .../en/modules/admin/patron-attr-types.tt | 17 +-- t/db_dependent/Koha/Patron/Attribute/Types.t | 102 +++++++++++++++++- 5 files changed, 249 insertions(+), 10 deletions(-) create mode 100644 Koha/Exceptions/Patron/Attribute/Type.pm --- a/Koha/Exceptions/Patron/Attribute/Type.pm +++ a/Koha/Exceptions/Patron/Attribute/Type.pm @@ -0,0 +1,31 @@ +package Koha::Exceptions::Patron::Attribute::Type; + +use Modern::Perl; + +use Exception::Class ( + + 'Koha::Exceptions::Patron::Attribute::Type' => { + description => 'Something went wrong' + }, + 'Koha::Exceptions::Patron::Attribute::Type::CannotChangeProperty' => { + isa => 'Koha::Exceptions::Patron::Attribute::Type', + description => "Cannot change property", + fields => ['property'], + }, +); + +sub full_message { + my $self = shift; + + my $msg = $self->message; + + unless ( $msg) { + if ( $self->isa('Koha::Exceptions::Patron::Attribute::Type::CannotChangeProperty') ) { + $msg = sprintf("The property '%s' cannot be changed, some patron attributes are using it that way.", $self->property ); + } + } + + return $msg; +} + +1; --- a/Koha/Patron/Attribute/Type.pm +++ a/Koha/Patron/Attribute/Type.pm @@ -18,6 +18,7 @@ package Koha::Patron::Attribute::Type; use Modern::Perl; use Koha::Database; +use Koha::Exceptions::Patron::Attribute::Type; use base qw(Koha::Object Koha::Object::Limit::Library); @@ -31,6 +32,90 @@ Koha::Patron::Attribute::Type - Koha::Patron::Attribute::Type Object class =cut +=head3 store + + my $attribute = Koha::Patron::Attribute->new({ code => 'a_code', ... }); + try { $attribute->store } + catch { handle_exception }; + +=cut + +sub store { + + my $self = shift; + + $self->check_repeatables; + $self->check_unique_ids; + + return $self->SUPER::store(); +} + +=head3 attributes + +=cut + +sub attributes { + my ($self) = @_; + my $attributes_rs = $self->_result->borrower_attributes; + Koha::Patron::Attributes->_new_from_dbic($attributes_rs); +} + +=head2 Internal Methods + +=cut + +=head3 check_repeatables + +=cut + +sub check_repeatables { + my ($self) = @_; + + return $self if $self->repeatable; + + my $count = $self->attributes->search( + {}, + { + select => [ { count => 'id', '-as' => 'c' } ], + group_by => 'borrowernumber', + having => { c => { '>' => 1 } } + } + )->count; + + Koha::Exceptions::Patron::Attribute::Type::CannotChangeProperty->throw( + property => 'repeatable' ) + if $count; + + return $self; +} + +=head3 check_unique_ids + +=cut + +sub check_unique_ids { + my ($self) = @_; + + return $self unless $self->unique_id; + + my $count = $self->attributes->search( + {}, + { + select => [ { count => 'id', '-as' => 'c' } ], + group_by => 'attribute', + having => { c => { '>' => 1 } } + } + )->count; + + Koha::Exceptions::Patron::Attribute::Type::CannotChangeProperty->throw( + property => 'unique_id' ) + if $count; + + return $self; +} + + + =head3 _type =cut --- a/admin/patron-attr-types.pl +++ a/admin/patron-attr-types.pl @@ -142,13 +142,14 @@ sub add_update_attribute_type { { code => $code, description => $description, - repeatable => $repeatable, - unique_id => $unique_id, } ); } + $attr_type->set( { + repeatable => $repeatable, + unique_id => $unique_id, opac_display => $opac_display, opac_editable => $opac_editable, staff_searchable => $staff_searchable, @@ -219,12 +220,29 @@ sub edit_attribute_type_form { my $code = shift; my $attr_type = Koha::Patron::Attribute::Types->find($code); - $template->param(attribute_type => $attr_type); my $patron_categories = Koha::Patron::Categories->search({}, {order_by => ['description']}); + + my $can_be_set_to_nonrepeatable = 1; + if ( $attr_type->repeatable == 1 ) { + $attr_type->repeatable(0); + eval {$attr_type->check_repeatables}; + $can_be_set_to_nonrepeatable = 0 if $@; + $attr_type->repeatable(1); + } + my $can_be_set_to_unique = 1; + if ( $attr_type->unique_id == 0 ) { + $attr_type->unique_id(1); + eval {$attr_type->check_unique_ids}; + $can_be_set_to_unique = 0 if $@; + $attr_type->unique_id(0); + } $template->param( + attribute_type => $attr_type, attribute_type_form => 1, edit_attribute_type => 1, + can_be_set_to_nonrepeatable => $can_be_set_to_nonrepeatable, + can_be_set_to_unique => $can_be_set_to_unique, confirm_op => 'edit_attribute_type_confirmed', categories => $patron_categories, ); --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt @@ -79,29 +79,34 @@
  • [% IF attribute_type %] - [% IF attribute_type.repeatable %] + [% IF attribute_type.repeatable AND NOT can_be_set_to_nonrepeatable %] + + [% ELSIF attribute_type.repeatable %] + [% ELSE %] - + [% END %] [% ELSE %] [% END %] - Check to let a patron record have multiple values of this attribute. - This setting cannot be changed after an attribute is defined. + Check to let a patron record have multiple values of this attribute.
  • [% IF attribute_type %] [% IF attribute_type.unique_id %] - + + [% ELSIF can_be_set_to_unique %] + [% ELSE %] + [% END %] [% ELSE %] [% END %] If checked, attribute will be a unique identifier — if a value is given to a patron record, the same value - cannot be given to a different record. This setting cannot be changed after an attribute is defined. + cannot be given to a different record.
  • [% IF attribute_type AND attribute_type.opac_display %] --- a/t/db_dependent/Koha/Patron/Attribute/Types.t +++ a/t/db_dependent/Koha/Patron/Attribute/Types.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 6; +use Test::More tests => 7; use t::lib::TestBuilder; use t::lib::Mocks; @@ -69,6 +69,106 @@ subtest 'new() tests' => sub { $schema->storage->txn_rollback; }; +subtest 'store' => sub { + + plan tests => 4; + + $schema->storage->txn_begin; + + # Create 2 attribute types without restrictions: + # Repeatable and can have the same values + my $attr_type_1 = $builder->build_object( + { + class => 'Koha::Patron::Attribute::Types', + value => { repeatable => 1, unique_id => 0 } + } + ); + my $attr_type_2 = $builder->build_object( + { + class => 'Koha::Patron::Attribute::Types', + value => { repeatable => 1, unique_id => 0 } + } + ); + + # Patron 1 has twice the attribute 1 and attribute 2 + # Patron 2 has attribute 1 and attribute 2="42" + # Patron 3 has attribute 2="42" + # Attribute 1 cannot remove repeatable + # Attribute 2 cannot set unique_id + my $patron_1 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron_2 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron_3 = $builder->build_object( { class => 'Koha::Patrons' } ); + + my $attribute_111 = $builder->build_object( + { + class => 'Koha::Patron::Attributes', + value => { + borrowernumber => $patron_1->borrowernumber, + code => $attr_type_1->code + } + } + ); + my $attribute_112 = $builder->build_object( + { + class => 'Koha::Patron::Attributes', + value => { + borrowernumber => $patron_1->borrowernumber, + code => $attr_type_1->code + } + } + ); + + my $attribute_211 = $builder->build_object( + { + class => 'Koha::Patron::Attributes', + value => { + borrowernumber => $patron_2->borrowernumber, + code => $attr_type_1->code + } + } + ); + my $attribute_221 = $builder->build_object( + { + class => 'Koha::Patron::Attributes', + value => { + borrowernumber => $patron_2->borrowernumber, + code => $attr_type_2->code, + attribute => '42', + } + } + ); + + my $attribute_321 = $builder->build_object( + { + class => 'Koha::Patron::Attributes', + value => { + borrowernumber => $patron_3->borrowernumber, + code => $attr_type_2->code, + attribute => '42', + } + } + ); + + throws_ok { + $attr_type_1->repeatable(0)->store; + } + 'Koha::Exceptions::Patron::Attribute::Type::CannotChangeProperty', ""; + + $attribute_112->delete; + ok($attr_type_1->set({ unique_id => 1, repeatable => 0 })->store); + + throws_ok { + $attr_type_2->unique_id(1)->store; + } + 'Koha::Exceptions::Patron::Attribute::Type::CannotChangeProperty', ""; + + $attribute_321->attribute(43)->store; + ok($attr_type_2->set({ unique_id => 1, repeatable => 0 })->store); + + $schema->storage->txn_rollback; + +}; + subtest 'library_limits() tests' => sub { plan tests => 13; --