From 2e74f0bbafbacea8ec92e584a4e9036086dc65e5 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 26 Oct 2016 18:22:50 +0300 Subject: [PATCH] Bug 17499: Improve object usability by enabling direct access to transport preferences 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 --- Koha/Patron/Message/Preference.pm | 149 ++++++++++++++++++++++- t/db_dependent/Koha/Patron/Message/Preferences.t | 132 +++++++++++++++++++- 2 files changed, 273 insertions(+), 8 deletions(-) diff --git a/Koha/Patron/Message/Preference.pm b/Koha/Patron/Message/Preference.pm index 8fa1910..4ca3fe5 100644 --- a/Koha/Patron/Message/Preference.pm +++ b/Koha/Patron/Message/Preference.pm @@ -22,8 +22,11 @@ use Modern::Perl; use Koha::Database; use Koha::Exceptions; 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; use base qw(Koha::Object); @@ -44,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, }); @@ -54,12 +58,22 @@ either borrowernumber or categorycode, but not both. Throws Koha::Exceptions::DuplicateObject if trying to create a preference that already exists for the same message_attribute_id and borrowernumber/categorycode. +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); my $previous = Koha::Patron::Message::Preferences->search({ '-and' => [ @@ -140,6 +154,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 @@ -152,9 +224,22 @@ Deletes any previous messaging preferences to avoid duplication. sub store { my $self = shift; - $self->validate; + $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->SUPER::store(@_); + return $self; } =head3 validate @@ -203,6 +288,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 07a6e04..4998d80 100644 --- a/t/db_dependent/Koha/Patron/Message/Preferences.t +++ b/t/db_dependent/Koha/Patron/Message/Preferences.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 16; +use Test::More tests => 17; use t::lib::Mocks; use t::lib::TestBuilder; @@ -111,7 +111,7 @@ subtest 'Add a test messaging attribute' => sub { }; subtest 'Add a test messaging transport' => sub { - plan tests => 2; + plan tests => 4; ok(Koha::Patron::Message::Transport->new({ message_attribute_id => $attribute->message_attribute_id, @@ -119,7 +119,21 @@ subtest 'Add a test messaging transport' => sub { is_digest => 0, letter_module => "circulation", letter_code => "CHECKIN", - })->store, "Added a new messaging transport type."); + })->store, "Added a new messaging transport type test."); + ok(Koha::Patron::Message::Transport->new({ + message_attribute_id => $attribute->message_attribute_id, + message_transport_type => 'sms', + is_digest => 0, + letter_module => "circulation", + letter_code => "CHECKIN", + })->store, "Added a new messaging transport type sms."); + ok(Koha::Patron::Message::Transport->new({ + message_attribute_id => $attribute->message_attribute_id, + message_transport_type => 'email', + is_digest => 0, + letter_module => "circulation", + letter_code => "CHECKIN", + })->store, "Added a new messaging transport type email."); is(Koha::Patron::Message::Transports->find({ message_transport_type => "test" })->message_attribute_id, $attribute->message_attribute_id , "Found test messaging transport from database."); @@ -182,7 +196,7 @@ subtest 'Add a messaging preference to patron' => sub { }; subtest 'Attempt to add a messaging preference with invalid parameters' => sub { - plan tests => 6; + plan tests => 11; eval { Koha::Patron::Message::Preference->new->store }; is (ref $@, "Koha::Exceptions::MissingParameter", @@ -211,6 +225,29 @@ subtest 'Add a messaging preference to patron' => sub { ." => Koha::Exceptions::BadParameter"); is ($@->parameter, "categorycode", "The previous exception tells us it" ." was the categorycode."); + 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."); }; }; @@ -227,6 +264,93 @@ subtest 'Add a messaging transport preference to patron' => sub { "Found test messaging transport preference from database."); }; +subtest 'Test Koha::Patron::Message::Preference->message_transport_types' => sub { + plan tests => 3; + + subtest 'Get' => sub { + plan tests => 5; + + 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 => 'sms', + })->store; + Koha::Patron::Message::Transport::Preference->new({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + message_transport_type => 'email', + })->store; + my $stored_transports = Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + }); + + ok(Koha::Patron::Message::Preference->can('message_transport_types'), "Method available-"); + 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}, "CHECKIN", "Found correct message transport type and letter code."); + is($transports->{$stored_transports->next->message_transport_type}, "CHECKIN", "Found correct message transport type and letter code."); + ok(!$preference->message_transport_types->{'nonexistent'}, "Didn't find nonexistent transport type."); + }; + + subtest 'Set' => sub { + plan tests => 9; + + # 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('sms', 'email')->store, "1/3 Set returned true."); + 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, 'email', "First type is email."); + is($stored_transports->next->message_transport_type, 'sms', "Second type is sms."); + + # 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(['sms', 'email'])->store, "2/3 Set returned true."); + $stored_transports = Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + }); + is($stored_transports->next->message_transport_type, 'email', "First type is email."); + is($stored_transports->next->message_transport_type, 'sms', "Second type is sms."); + + # 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 => ['sms', 'email']})->store, "3/3 Set returned true."); + $stored_transports = Koha::Patron::Message::Transport::Preferences->search({ + borrower_message_preference_id => $preference->borrower_message_preference_id, + }); + is($stored_transports->next->message_transport_type, 'email', "First type is email."); + is($stored_transports->next->message_transport_type, 'sms', "Second type is sms."); + }; + + subtest 'New' => sub { + plan tests => 3; + + $preference->delete; + ok($preference = Koha::Patron::Message::Preference->new({ + borrowernumber => $patron->{'borrowernumber'}, + message_attribute_id => $attribute->message_attribute_id, + wants_digest => 0, + days_in_advance => 1, + message_transport_types => $transport_type->message_transport_type, + })->store, "Added a new messaging preference and transport types to patron."); + ok($preference->message_transport_types->{$transport_type->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, $transport_type->message_transport_type, + "The transport type is stored in the database."); + }; +}; + $schema->storage->txn_rollback; 1; -- 2.7.4