View | Details | Raw Unified | Return to bug 14723
Collapse All | Expand All

(-)a/C4/Letters.pm (-12 / +22 lines)
Lines 898-906 sub EnqueueLetter { Link Here
898
    my $dbh       = C4::Context->dbh();
898
    my $dbh       = C4::Context->dbh();
899
    my $statement = << 'ENDSQL';
899
    my $statement = << 'ENDSQL';
900
INSERT INTO message_queue
900
INSERT INTO message_queue
901
( borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, to_address, from_address, content_type )
901
( borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, to_address, from_address, content_type, delivery_note )
902
VALUES
902
VALUES
903
( ?,              ?,       ?,       ?,        ?,           ?,                      ?,      NOW(),       ?,          ?,            ? )
903
( ?,              ?,       ?,       ?,        ?,           ?,                      ?,      NOW(),       ?,          ?,            ?,            ? )
904
ENDSQL
904
ENDSQL
905
905
906
    my $sth    = $dbh->prepare($statement);
906
    my $sth    = $dbh->prepare($statement);
Lines 915-920 ENDSQL Link Here
915
        $params->{'to_address'},                  # to_address
915
        $params->{'to_address'},                  # to_address
916
        $params->{'from_address'},                # from_address
916
        $params->{'from_address'},                # from_address
917
        $params->{'letter'}->{'content-type'},    # content_type
917
        $params->{'letter'}->{'content-type'},    # content_type
918
        $params->{'delivery_note'}        || '',  # delivery_note
918
    );
919
    );
919
    return $dbh->last_insert_id(undef,undef,'message_queue', undef);
920
    return $dbh->last_insert_id(undef,undef,'message_queue', undef);
920
}
921
}
Lines 1004-1010 sub GetQueuedMessages { Link Here
1004
1005
1005
    my $dbh = C4::Context->dbh();
1006
    my $dbh = C4::Context->dbh();
1006
    my $statement = << 'ENDSQL';
1007
    my $statement = << 'ENDSQL';
1007
SELECT message_id, borrowernumber, subject, content, message_transport_type, status, time_queued
1008
SELECT message_id, borrowernumber, subject, content, message_transport_type, status, time_queued, delivery_note
1008
FROM message_queue
1009
FROM message_queue
1009
ENDSQL
1010
ENDSQL
1010
1011
Lines 1137-1143 sub _send_message_by_email { Link Here
1137
        unless ($member) {
1138
        unless ($member) {
1138
            warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})";
1139
            warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})";
1139
            _set_message_status( { message_id => $message->{'message_id'},
1140
            _set_message_status( { message_id => $message->{'message_id'},
1140
                                   status     => 'failed' } );
1141
                                   status     => 'failed',
1142
                                   delivery_note => 'Invalid borrowernumber '.$message->{borrowernumber} } );
1141
            return;
1143
            return;
1142
        }
1144
        }
1143
        $to_address = C4::Members::GetNoticeEmailAddress( $message->{'borrowernumber'} );
1145
        $to_address = C4::Members::GetNoticeEmailAddress( $message->{'borrowernumber'} );
Lines 1145-1151 sub _send_message_by_email { Link Here
1145
            # warn "FAIL: No 'to_address' and no email for " . ($member->{surname} ||'') . ", borrowernumber ($message->{borrowernumber})";
1147
            # warn "FAIL: No 'to_address' and no email for " . ($member->{surname} ||'') . ", borrowernumber ($message->{borrowernumber})";
1146
            # warning too verbose for this more common case?
1148
            # warning too verbose for this more common case?
1147
            _set_message_status( { message_id => $message->{'message_id'},
1149
            _set_message_status( { message_id => $message->{'message_id'},
1148
                                   status     => 'failed' } );
1150
                                   status     => 'failed',
1151
                                   delivery_note => 'Unable to find an email address for this borrower' } );
1149
            return;
1152
            return;
1150
        }
1153
        }
1151
    }
1154
    }
Lines 1186-1196 sub _send_message_by_email { Link Here
1186
    _update_message_to_address($message->{'message_id'},$to_address) unless $message->{to_address}; #if initial message address was empty, coming here means that a to address was found and queue should be updated
1189
    _update_message_to_address($message->{'message_id'},$to_address) unless $message->{to_address}; #if initial message address was empty, coming here means that a to address was found and queue should be updated
1187
    if ( sendmail( %sendmail_params ) ) {
1190
    if ( sendmail( %sendmail_params ) ) {
1188
        _set_message_status( { message_id => $message->{'message_id'},
1191
        _set_message_status( { message_id => $message->{'message_id'},
1189
                status     => 'sent' } );
1192
                status     => 'sent',
1193
                delivery_note => '' } );
1190
        return 1;
1194
        return 1;
1191
    } else {
1195
    } else {
1192
        _set_message_status( { message_id => $message->{'message_id'},
1196
        _set_message_status( { message_id => $message->{'message_id'},
1193
                status     => 'failed' } );
1197
                status     => 'failed',
1198
                delivery_note => $Mail::Sendmail::error } );
1194
        carp $Mail::Sendmail::error;
1199
        carp $Mail::Sendmail::error;
1195
        return;
1200
        return;
1196
    }
1201
    }
Lines 1239-1251 sub _send_message_by_sms { Link Here
1239
1244
1240
    unless ( $member->{smsalertnumber} ) {
1245
    unless ( $member->{smsalertnumber} ) {
1241
        _set_message_status( { message_id => $message->{'message_id'},
1246
        _set_message_status( { message_id => $message->{'message_id'},
1242
                               status     => 'failed' } );
1247
                               status     => 'failed',
1248
                               delivery_note => 'Missing SMS number' } );
1243
        return;
1249
        return;
1244
    }
1250
    }
1245
1251
1246
    if ( _is_duplicate( $message ) ) {
1252
    if ( _is_duplicate( $message ) ) {
1247
        _set_message_status( { message_id => $message->{'message_id'},
1253
        _set_message_status( { message_id => $message->{'message_id'},
1248
                               status     => 'failed' } );
1254
                               status     => 'failed',
1255
                               delivery_note => 'Message is duplicate' } );
1249
        return;
1256
        return;
1250
    }
1257
    }
1251
1258
Lines 1253-1259 sub _send_message_by_sms { Link Here
1253
                                       message     => $message->{'content'},
1260
                                       message     => $message->{'content'},
1254
                                     } );
1261
                                     } );
1255
    _set_message_status( { message_id => $message->{'message_id'},
1262
    _set_message_status( { message_id => $message->{'message_id'},
1256
                           status     => ($success ? 'sent' : 'failed') } );
1263
                           status     => ($success ? 'sent' : 'failed'),
1264
                           delivery_note => ($success ? '' : 'No notes from SMS driver') } );
1265
1257
    return $success;
1266
    return $success;
1258
}
1267
}
1259
1268
Lines 1271-1280 sub _set_message_status { Link Here
1271
    }
1280
    }
1272
1281
1273
    my $dbh = C4::Context->dbh();
1282
    my $dbh = C4::Context->dbh();
1274
    my $statement = 'UPDATE message_queue SET status= ? WHERE message_id = ?';
1283
    my $statement = 'UPDATE message_queue SET status= ?, delivery_note= ? WHERE message_id = ?';
1275
    my $sth = $dbh->prepare( $statement );
1284
    my $sth = $dbh->prepare( $statement );
1276
    my $result = $sth->execute( $params->{'status'},
1285
    my $result = $sth->execute( $params->{'status'},
1277
                                $params->{'message_id'} );
1286
                                $params->{'delivery_note'},
1287
                                $params->{'message_id'} || '' );
1278
    return $result;
1288
    return $result;
1279
}
1289
}
1280
1290
(-)a/installer/data/mysql/atomicupdate/Bug-14723_-_Additional_delivery_notes_to_messages.pl (+8 lines)
Line 0 Link Here
1
#! /usr/bin/perl
2
use strict;
3
use warnings;
4
use C4::Context;
5
my $dbh = C4::Context->dbh;
6
7
$dbh->do("ALTER TABLE message_queue ADD delivery_note TEXT");
8
print "Upgrade to done (Bug 14723 - Additional delivery notes to messages)\n";
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt (+4 lines)
Lines 49-54 Link Here
49
		<th>Type</th>
49
		<th>Type</th>
50
		<th>Status</th>
50
		<th>Status</th>
51
		<th>Time</th>
51
		<th>Time</th>
52
		<th>Delivery note</th>
52
	    </tr>
53
	    </tr>
53
	</thead>
54
	</thead>
54
	<tbody>
55
	<tbody>
Lines 75-80 Link Here
75
            [% ELSE %][% QUEUED_MESSAGE.status %][% END %]
76
            [% ELSE %][% QUEUED_MESSAGE.status %][% END %]
76
        </td>
77
        </td>
77
        <td><span title="[% QUEUED_MESSAGE.time_queued %]">[% QUEUED_MESSAGE.time_queued | $KohaDates with_hours => 1 %]</span></td>
78
        <td><span title="[% QUEUED_MESSAGE.time_queued %]">[% QUEUED_MESSAGE.time_queued | $KohaDates with_hours => 1 %]</span></td>
79
        <td>
80
            [% QUEUED_MESSAGE.delivery_note %]
81
        </td>
78
	    </tr>
82
	    </tr>
79
	    [% END %]
83
	    [% END %]
80
	</tbody>
84
	</tbody>
(-)a/t/db_dependent/Letters.t (-2 / +5 lines)
Lines 18-24 Link Here
18
# along with Koha; if not, see <http://www.gnu.org/licenses>.
18
# along with Koha; if not, see <http://www.gnu.org/licenses>.
19
19
20
use Modern::Perl;
20
use Modern::Perl;
21
use Test::More tests => 61;
21
use Test::More tests => 63;
22
use Test::MockModule;
22
use Test::MockModule;
23
use Test::Warn;
23
use Test::Warn;
24
24
Lines 123-129 is( $messages->[0]->{subject}, $my_message->{letter}->{title}, 'EnqueueLetter st Link Here
123
is( $messages->[0]->{content}, $my_message->{letter}->{content}, 'EnqueueLetter stores the content correctly' );
123
is( $messages->[0]->{content}, $my_message->{letter}->{content}, 'EnqueueLetter stores the content correctly' );
124
is( $messages->[0]->{message_transport_type}, $my_message->{message_transport_type}, 'EnqueueLetter stores the message type correctly' );
124
is( $messages->[0]->{message_transport_type}, $my_message->{message_transport_type}, 'EnqueueLetter stores the message type correctly' );
125
is( $messages->[0]->{status}, 'pending', 'EnqueueLetter stores the status pending correctly' );
125
is( $messages->[0]->{status}, 'pending', 'EnqueueLetter stores the status pending correctly' );
126
126
is( $messages->[0]->{delivery_note}, '', 'Delivery note for successful message correctly empty');
127
127
128
# SendQueuedMessages
128
# SendQueuedMessages
129
my $messages_processed = C4::Letters::SendQueuedMessages();
129
my $messages_processed = C4::Letters::SendQueuedMessages();
Lines 136-141 is( Link Here
136
    'message marked failed if tried to send SMS message for borrower with no smsalertnumber set (bug 11208)'
136
    'message marked failed if tried to send SMS message for borrower with no smsalertnumber set (bug 11208)'
137
);
137
);
138
138
139
# Delivery notes
140
is($messages->[0]->{delivery_note}, 'Missing SMS number', 'Delivery note for no smsalertnumber correctly set');
141
139
# GetLetters
142
# GetLetters
140
my $letters = C4::Letters::GetLetters();
143
my $letters = C4::Letters::GetLetters();
141
is( @$letters, 0, 'GetLetters returns the correct number of letters' );
144
is( @$letters, 0, 'GetLetters returns the correct number of letters' );
(-)a/t/db_dependent/MessageDeliveryNote.t (-1 / +212 lines)
Line 0 Link Here
0
- 
1
#!/usr/bin/env perl
2
3
# Copyright 2015 Open Source Freedom Fighters
4
#
5
# This file is part of Koha.
6
#
7
# Koha is free software; you can redistribute it and/or modify it
8
# under the terms of the GNU General Public License as published by
9
# the Free Software Foundation; either version 3 of the License, or
10
# (at your option) any later version.
11
#
12
# Koha is distributed in the hope that it will be useful, but
13
# WITHOUT ANY WARRANTY; without even the implied warranty of
14
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15
# GNU General Public License for more details.
16
#
17
# You should have received a copy of the GNU General Public License
18
# along with Koha; if not, see <http://www.gnu.org/licenses>.
19
$ENV{KOHA_PAGEOBJECT_DEBUG} = 1;
20
use Modern::Perl;
21
22
use Test::More;
23
use Try::Tiny; #Even Selenium::Remote::Driver uses Try::Tiny :)
24
25
use Koha::Auth::PermissionManager;
26
27
use t::lib::Page::Mainpage;
28
use t::lib::Page::Opac::OpacMain;
29
use t::lib::Page::Opac::OpacMemberentry;
30
use t::lib::Page::Members::Memberentry;
31
use t::lib::Page::Members::Moremember;
32
use t::lib::Page::Members::Notices;
33
34
use t::lib::TestObjects::BorrowerFactory;
35
use t::lib::TestObjects::MessageQueueFactory;
36
use t::lib::TestObjects::SystemPreferenceFactory;
37
38
use C4::Context;
39
use C4::Members;
40
41
##Setting up the test context
42
my $testContext = {};
43
44
my $password = '1234';
45
my $invalidBorrowerNumber = '000';
46
my $borrowerFactory = t::lib::TestObjects::BorrowerFactory->new();
47
my $borrowers = $borrowerFactory->createTestGroup([
48
            {firstname  => 'Testthree',
49
             surname    => 'Testfour',
50
             cardnumber => 'superuberadmin',
51
             branchcode => 'CPL',
52
             userid     => 'god',
53
             address    => 'testi',
54
             city       => 'joensuu',
55
             zipcode    => '80100',
56
             password   => $password,
57
            },
58
        ], undef, $testContext);
59
my $messages = t::lib::TestObjects::MessageQueueFactory->createTestGroup([
60
            {letter => {
61
                title => "Test title",
62
                content => "Tessst content",
63
                },
64
             cardnumber => $borrowers->{'superuberadmin'}->cardnumber,
65
             message_transport_type => 'sms',
66
             from_address => 'A001@example.com',
67
            },
68
            {letter => {
69
                title => "Test title",
70
                content => "Tessst content",
71
                },
72
             cardnumber => $borrowers->{'superuberadmin'}->cardnumber,
73
             message_transport_type => 'sms',
74
             from_address => 'A002@example.com',
75
            },
76
            {letter => {
77
                title => "Test title",
78
                content => "Tessst content",
79
                },
80
             cardnumber => $borrowers->{'superuberadmin'}->cardnumber,
81
             message_transport_type => 'email',
82
             to_address   => '',
83
             from_address => 'B001@example.com',
84
            },
85
            {letter => {
86
                title => "Test title",
87
                content => "Tessst content",
88
                },
89
             cardnumber => $borrowers->{'superuberadmin'}->cardnumber,
90
             message_transport_type => 'email',
91
             to_address   => 'nobody@example.com',
92
             from_address => 'B002@example.com',
93
            },
94
            {letter => {
95
                title => "INVALID USER",
96
                content => "INVALID USER",
97
                },
98
             cardnumber => $borrowers->{'superuberadmin'}->cardnumber,
99
             message_transport_type => 'email',
100
             to_address   => '',
101
             from_address => 'B003@example.com',
102
            },
103
        ], undef, $testContext);
104
my $systempreferences = t::lib::TestObjects::SystemPreferenceFactory->createTestGroup([
105
            {preference => 'SMSSendDriver',
106
             value      => 'nonexistentdriver'
107
            },
108
            {
109
             preference => 'EnhancedMessagingPreferences',
110
             value      => '1'
111
            }
112
        ], undef, $testContext);
113
114
my $permissionManager = Koha::Auth::PermissionManager->new();
115
$permissionManager->grantPermissions($borrowers->{'superuberadmin'}, {superlibrarian => 'superlibrarian'});
116
117
eval {
118
119
    # staff client
120
    my $notices = t::lib::Page::Members::Notices->new({borrowernumber => $borrowers->{'superuberadmin'}->borrowernumber});
121
122
    # let's send the messages
123
124
    # first, send one email message with invalid borrower number
125
    $messages->{'B003@example.com'}->{borrowernumber} = $invalidBorrowerNumber;
126
    C4::Letters::_send_message_by_email($messages->{'B003@example.com'});
127
128
    # the rest should produce errors automatically without our modificaitons
129
    C4::Letters::SendQueuedMessages();
130
131
    # then, login to intranet
132
    $notices->doPasswordLogin($borrowers->{'superuberadmin'}->userid(), $password);
133
134
    # test delivery notes for SMS messaging
135
    DoSMSTests($notices);
136
    # check delivery notes for email messaging
137
    CheckEmailMessages($notices);
138
139
};
140
if ($@) { #Catch all leaking errors and gracefully terminate.
141
    warn $@;
142
    tearDown();
143
    exit 1;
144
}
145
146
##All tests done, tear down test context
147
tearDown();
148
done_testing;
149
150
sub tearDown {
151
    t::lib::TestObjects::ObjectFactory->tearDownTestContext($testContext);
152
}
153
154
155
156
157
158
159
160
161
######################################################
162
    ###  STARTING TEST IMPLEMENTATIONS         ###
163
######################################################
164
165
sub DoSMSTests {
166
    my ($notices) = @_;
167
168
    # login and check that our table is displayed correctly
169
    $notices->
170
    hasDeliveryNoteColumnInNoticesTable()->
171
    hasTextInTableCell("Missing SMS number");
172
173
    # now let's give the user a SMS number so we proceed to next step of failure
174
    ModMember( borrowernumber => $borrowers->{superuberadmin}->borrowernumber,
175
                            smsalertnumber => 'just_some_number' );
176
177
    # set first to sent and second to pending to check "duplicate" (Letters::_is_duplicate())
178
    setMessageStatus($messages->{'A001@example.com'}->{message_id}, "sent");
179
    setMessageStatus($messages->{'A002@example.com'}->{message_id}, "pending");
180
181
    C4::Letters::SendQueuedMessages();
182
    # A002 should go into failed: message is duplicate status
183
184
    # set A001 back to pending to check that we get no reply from driver
185
    setMessageStatus($messages->{'A001@example.com'}->{message_id}, "pending");
186
187
    # let's send the message and check for delivery note, we should get a warning
188
    # for non-installed driver.
189
    C4::Letters::SendQueuedMessages();
190
191
    $notices->navigateToNotices()->
192
    hasTextInTableCell("Message is duplicate")->
193
    hasTextInTableCell("No notes from SMS driver");
194
}
195
196
sub CheckEmailMessages {
197
    my ($notices) = @_;
198
199
    $notices->
200
    hasTextInTableCell("Unable to find an email address")->
201
    hasTextInTableCell("Invalid borrowernumber")->
202
    hasTextInTableCell("Connection refused"); #sendmail should also fail
203
}
204
sub setMessageStatus {
205
    my ($message_id, $status) = @_;
206
    my $dbh = C4::Context->dbh;
207
208
    my $sth = $dbh->prepare("UPDATE message_queue SET status=?, delivery_note='' WHERE message_id=?");
209
    my $result = $sth->execute($status, $message_id);
210
211
    return $result;
212
}

Return to bug 14723