From ac38e293215a0b8a88e2d18f6c1972f4dd137632 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 26 Oct 2016 18:22:50 +0300 Subject: [PATCH] [SIGNED-OFF] Bug 17499: Improve object usability by enabling direct access to transport preferences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since messaging preference is a feature that has multiple related database tables, usage via Koha-objects is sometimes frustrating. This patch adds a feature for Koha::Patron::Message::Preference which enables access to message transport preferences directly via this object. It allows us to skip calls to Koha::Patron::Message::Transport::Preference(s) because we can now get and set via K::P::M::Preference. Get: $preference->message_transport_types Returns a hashref, where each key is stored transport type and value for the key is letter code for the transport. Set: $preference->set({ message_transport_types => ['sms'] }) or $preference->message_transport_types('email', 'sms') or $preference->message_transport_types(['email', 'sms']) Returns $self (Koha::Patron::Message::Preference object) To test: 1. Run t/db_dependent/Koha/Patron/Message/Preferences.t Signed-off-by: Marc Véron Signed-off-by: Josef Moravec --- Koha/Patron/Message/Preference.pm | 149 +++++++++++++++++- t/db_dependent/Koha/Patron/Message/Preferences.t | 186 ++++++++++++++++++++++- 2 files changed, 330 insertions(+), 5 deletions(-) diff --git a/Koha/Patron/Message/Preference.pm b/Koha/Patron/Message/Preference.pm index 9f63782..c7e2b95 100644 --- a/Koha/Patron/Message/Preference.pm +++ b/Koha/Patron/Message/Preference.pm @@ -25,6 +25,7 @@ use Koha::Patron::Categories; use Koha::Patron::Message::Attributes; use Koha::Patron::Message::Preferences; use Koha::Patron::Message::Transport::Preferences; +use Koha::Patron::Message::Transport::Types; use Koha::Patron::Message::Transports; use Koha::Patrons; @@ -46,6 +47,7 @@ my $preference = Koha::Patron::Message::Preference->new({ borrowernumber => 123, #categorycode => 'ABC', message_attribute_id => 4, + message_transport_types => ['email', 'sms'], # see documentation below wants_digest => 1, days_in_advance => 7, }); @@ -62,12 +64,22 @@ You can instantiate a new object without custom validation errors, but when storing, validation may throw exceptions. See C for more documentation. +C is a parameter that is not actually a column in this +Koha-object. Given this parameter, the message transport types will be added as +related transport types for this object. For get and set, you can access them via +subroutine C in this class. + =cut sub new { - my ($class, $params) = shift; + my ($class, $params) = @_; + + my $types = $params->{'message_transport_types'}; + delete $params->{'message_transport_types'}; - my $self = $class->SUPER::new(@_); + my $self = $class->SUPER::new($params); + + $self->_set_message_transport_types($types); return $self; } @@ -144,6 +156,64 @@ sub new_from_default { return $self; } +=head3 message_transport_types + +$preference->message_transport_types +Returns a HASHREF of message transport types for this messaging preference, e.g. +if ($preference->message_transport_types->{'email'}) { + # email is one of the transport preferences +} + +$preference->message_transport_types('email', 'sms'); +Sets the given message transport types for this messaging preference + +=cut + +sub message_transport_types { + my $self = shift; + + unless (@_) { + if ($self->{'_message_transport_types'}) { + return $self->{'_message_transport_types'}; + } + map { $self->{'_message_transport_types'}->{$_->message_transport_type} = + Koha::Patron::Message::Transports->find({ + message_attribute_id => $self->message_attribute_id, + message_transport_type => $_->message_transport_type, + is_digest => $self->wants_digest})->letter_code } + Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $self->borrower_message_preference_id, + })->as_list; + return $self->{'_message_transport_types'} || {}; + } + else { + $self->_set_message_transport_types(@_); + return $self; + } +} + +=head3 set + +$preference->set({ + message_transport_types => ['sms', 'phone'], + wants_digest => 0, +})->store; + +Sets preference object values and additionally message_transport_types if given. + +=cut + +sub set { + my ($self, $params) = @_; + + if ($params && $params->{'message_transport_types'}) { + $self->message_transport_types($params->{'message_transport_types'}); + delete $params->{'message_transport_types'}; + } + + return $self->SUPER::set($params) if $params; +} + =head3 store Makes a validation before actual Koha::Object->store so that proper exceptions @@ -154,7 +224,24 @@ can be thrown. See C for documentation about exceptions. sub store { my $self = shift; - return $self->validate->SUPER::store(@_); + $self->validate->SUPER::store(@_); + + # store message transport types + if (exists $self->{'_message_transport_types'}) { + Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => + $self->borrower_message_preference_id, + })->delete; + foreach my $type (keys %{$self->{'_message_transport_types'}}) { + Koha::Patron::Message::Transport::Preference->new({ + borrower_message_preference_id => + $self->borrower_message_preference_id, + message_transport_type => $type, + })->store; + } + } + + return $self; } =head3 validate @@ -252,6 +339,62 @@ sub validate { return $self; } +sub _set_message_transport_types { + my $self = shift; + + return unless $_[0]; + + $self->{'_message_transport_types'} = undef; + my $types = ref $_[0] eq "ARRAY" ? $_[0] : [@_]; + return unless $types; + $self->_validate_message_transport_types({ message_transport_types => $types }); + foreach my $type (@$types) { + unless (exists $self->{'_message_transport_types'}->{$type}) { + $self->{'_message_transport_types'}->{$type} = + Koha::Patron::Message::Transports->find({ + message_attribute_id => $self->message_attribute_id, + message_transport_type => $type, + is_digest => $self->wants_digest})->letter_code; + } + } + return $self; +} + +sub _validate_message_transport_types { + my ($self, $params) = @_; + + if (ref($params) eq 'HASH' && $params->{'message_transport_types'}) { + if (ref($params->{'message_transport_types'}) ne 'ARRAY') { + $params->{'message_transport_types'} = [$params->{'message_transport_types'}]; + } + my $types = $params->{'message_transport_types'}; + + foreach my $type (@{$types}) { + unless (Koha::Patron::Message::Transport::Types->find({ + message_transport_type => $type + })) { + Koha::Exceptions::BadParameter->throw( + error => "Message transport type '$type' does not exist", + parameter => 'message_transport_type', + ); + } + my $tmp = (ref($self) eq __PACKAGE__) ? $self->unblessed : $params; + unless (Koha::Patron::Message::Transports->find({ + message_transport_type => $type, + message_attribute_id => $tmp->{'message_attribute_id'}, + is_digest => $tmp->{'wants_digest'}, + })) { + Koha::Exceptions::BadParameter->throw( + error => "Message transport option for '$type' (". + ($tmp->{'wants_digest'} ? 'digest':'no digest').") does not exist", + parameter => 'message_transport_type', + ); + } + } + return $types; + } +} + =head3 type =cut diff --git a/t/db_dependent/Koha/Patron/Message/Preferences.t b/t/db_dependent/Koha/Patron/Message/Preferences.t index 8f09826..0c98222 100644 --- a/t/db_dependent/Koha/Patron/Message/Preferences.t +++ b/t/db_dependent/Koha/Patron/Message/Preferences.t @@ -19,7 +19,8 @@ use Modern::Perl; -use Test::More tests => 5; + +use Test::More tests => 6; use t::lib::Mocks; use t::lib::TestBuilder; @@ -173,6 +174,152 @@ subtest 'Add preferences from defaults' => sub { $schema->storage->txn_rollback; }; +subtest 'Test Koha::Patron::Message::Preference->message_transport_types' => sub { + plan tests => 4; + + ok(Koha::Patron::Message::Preference->can('message_transport_types'), + 'Method message_transport_types available'); + + subtest 'get message_transport_types' => sub { + plan tests => 4; + + $schema->storage->txn_begin; + + my $patron = build_a_test_patron(); + my ($preference, $mtt1, $mtt2) = build_a_test_complete_preference({ + patron => $patron + }); + Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + })->delete; + Koha::Patron::Message::Transport::Preference->new({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + message_transport_type => $mtt1->message_transport_type, + })->store; + Koha::Patron::Message::Transport::Preference->new({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + message_transport_type => $mtt2->message_transport_type, + })->store; + my $stored_transports = Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + }); + my $transport1 = Koha::Patron::Message::Transports->find({ + message_attribute_id => $preference->message_attribute_id, + message_transport_type => $mtt1->message_transport_type, + }); + my $transport2 = Koha::Patron::Message::Transports->find({ + message_attribute_id => $preference->message_attribute_id, + message_transport_type => $mtt2->message_transport_type, + }); + my $transports = $preference->message_transport_types; + is(keys %{$transports}, $stored_transports->count, + '->message_transport_types gets correct amount of transport types.'); + is($transports->{$stored_transports->next->message_transport_type}, + $transport1->letter_code, 'Found correct message transport type and letter code.'); + is($transports->{$stored_transports->next->message_transport_type}, + $transport2->letter_code, 'Found correct message transport type and letter code.'); + ok(!$preference->message_transport_types->{'nonexistent'}, + 'Didn\'t find nonexistent transport type.'); + + $schema->storage->txn_rollback; + }; + + subtest 'set message_transport_types' => sub { + plan tests => 6; + + $schema->storage->txn_begin; + + my $patron = build_a_test_patron(); + my ($preference, $mtt1, $mtt2) = build_a_test_complete_preference({ + patron => $patron + }); + + my $mtt1_str = $mtt1->message_transport_type; + my $mtt2_str = $mtt2->message_transport_type; + # 1/3, use message_transport_types(list) + Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + })->delete; + ok($preference->message_transport_types($mtt1_str, $mtt2_str)->store, + '1/3 Set returned true.'); + my $stored_transports = Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + '-or' => [ + message_transport_type => $mtt1_str, + message_transport_type => $mtt2_str + ] + }); + is($stored_transports->count, 2, 'Two transports selected'); + + # 2/3, use message_transport_types(ARRAYREF) + Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + })->delete; + ok($preference->message_transport_types([$mtt1_str, $mtt2_str])->store, + '2/3 Set returned true.'); + $stored_transports = Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + '-or' => [ + message_transport_type => $mtt1_str, + message_transport_type => $mtt2_str + ] + }); + is($stored_transports->count, 2, 'Two transports selected'); + + # 3/3, use set({ message_transport_types => ARRAYREF }) + Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + })->delete; + ok($preference->set({ + message_transport_types => [$mtt1_str, $mtt2_str]})->store, + '3/3 Set returned true.'); + $stored_transports = Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + '-or' => [ + message_transport_type => $mtt1_str, + message_transport_type => $mtt2_str + ] + }); + is($stored_transports->count, 2, 'Two transports selected'); + + $schema->storage->txn_rollback; + }; + + subtest 'new message_transport_types' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + my $patron = build_a_test_patron(); + my $letter = build_a_test_letter(); + my $attribute = build_a_test_attribute(); + my $mtt = build_a_test_transport_type(); + Koha::Patron::Message::Transport->new({ + message_attribute_id => $attribute->message_attribute_id, + message_transport_type => $mtt->message_transport_type, + is_digest => 0, + letter_module => $letter->module, + letter_code => $letter->code, + })->store; + ok(my $preference = Koha::Patron::Message::Preference->new({ + borrowernumber => $patron->borrowernumber, + message_attribute_id => $attribute->message_attribute_id, + wants_digest => 0, + days_in_advance => undef, + message_transport_types => $mtt->message_transport_type, + })->store, 'Added a new messaging preference and transport types to patron.'); + ok($preference->message_transport_types->{$mtt->message_transport_type}, + 'The transport type is stored in the object.'); + my $stored_transports = Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + }); + is($stored_transports->next->message_transport_type, $mtt->message_transport_type, + 'The transport type is stored in the database.'); + + $schema->storage->txn_rollback; + }; +}; + subtest 'Test adding a new preference with invalid parameters' => sub { plan tests => 4; @@ -203,7 +350,7 @@ subtest 'Test adding a new preference with invalid parameters' => sub { }; subtest 'Bad parameter' => sub { - plan tests => 8; + plan tests => 13; $schema->storage->txn_begin; @@ -250,6 +397,30 @@ subtest 'Test adding a new preference with invalid parameters' => sub { is($@->parameter, 'days_in_advance', 'The previous exception tells us it' .' was the days_in_advance.'); + eval { Koha::Patron::Message::Preference->new({ + borrowernumber => $patron->{'borrowernumber'}, + message_transport_types => ['nonexistent'] + })->store }; + is (ref $@, 'Koha::Exceptions::BadParameter', + 'Adding a message preference with invalid message_transport_type' + .' => Koha::Exceptions::BadParameter'); + is ($@->parameter, 'message_transport_type', 'The previous exception tells us it' + .' was the message_transport_type.'); + eval { + Koha::Patron::Message::Preference->new({ + borrowernumber => $patron->{'borrowernumber'}, + message_attribute_id => $attribute->message_attribute_id, + message_transport_types => ['sms'], + wants_digest => 1, + })->store }; + is (ref $@, 'Koha::Exceptions::BadParameter', + 'Adding a message preference with invalid message_transport_type' + .' => Koha::Exceptions::BadParameter'); + is ($@->parameter, 'message_transport_type', 'The previous exception tells us it' + .' was the message_transport_type.'); + like ($@->error, qr/^Message transport option/, 'Exception s because of given' + .' message_transport_type is not a valid option.'); + $schema->storage->txn_rollback; }; @@ -395,4 +566,15 @@ sub build_a_test_category_preference { return ($default, $mtt1, $mtt2); } +sub build_a_test_complete_preference { + my ($params) = @_; + + my ($default, $mtt1, $mtt2) = build_a_test_category_preference($params); + my $patron = $params->{patron}; + $patron->set_default_messaging_preferences; + return (Koha::Patron::Message::Preferences->search({ + borrowernumber => $patron->borrowernumber + })->next, $mtt1, $mtt2); +} + 1; -- 2.1.4