From 2c948326cc8208bfb3e98bd1f9ed940b3705bd8d Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@jns.fi>
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 | 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<validate()> for more
 documentation.
 
+C<message_transport_types> 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<message_transport_types()> 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<validate()> 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.7.4