Bug 14723 - Additional delivery notes to messages
Summary: Additional delivery notes to messages
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Emmi Takkinen
QA Contact: Kyle M Hall
URL:
Keywords: release-notes-needed
Depends on: 12426 18413
Blocks: 14767 14791 14843 28813
  Show dependency treegraph
 
Reported: 2015-08-25 13:27 UTC by Lari Taskula
Modified: 2021-09-20 07:34 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.05.00


Attachments
Bug 14723 - Additional delivery notes to messages (11.88 KB, patch)
2015-08-28 09:53 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723: (follow-up) Example of handling custom SMS::Send Driver reports (13.58 KB, patch)
2015-08-28 12:09 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (15.75 KB, patch)
2015-09-01 14:15 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (15.61 KB, patch)
2015-09-01 14:56 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (15.09 KB, patch)
2015-09-02 14:57 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (14.98 KB, patch)
2015-09-02 15:00 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (15.21 KB, patch)
2015-09-04 09:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (17.78 KB, patch)
2015-09-08 11:38 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (17.71 KB, patch)
2015-09-08 12:12 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (17.71 KB, patch)
2015-09-09 14:01 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (17.88 KB, patch)
2015-09-14 07:53 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (17.63 KB, patch)
2015-09-15 10:10 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (17.99 KB, patch)
2015-09-16 09:40 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (18.23 KB, patch)
2015-09-17 13:46 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages (18.59 KB, patch)
2015-09-17 13:50 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages - fixed missing kohastructure (877 bytes, patch)
2015-11-04 12:58 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 14723 - Additional delivery notes to messages - fixed missing kohastructure (877 bytes, patch)
2015-11-04 15:17 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 14723: Additional delivery notes to messages (10.40 KB, patch)
2017-04-12 13:09 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723: Additional delivery notes to messages (11.60 KB, patch)
2017-04-13 09:16 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723: Additional delivery notes to messages (11.71 KB, patch)
2018-08-30 16:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14723: Additional delivery notes to messages (11.71 KB, patch)
2019-04-08 13:59 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14723: Additional delivery notes to messages (11.78 KB, patch)
2019-04-08 15:27 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 14723: Additional delivery notes to messages (12.14 KB, patch)
2020-06-12 05:30 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 14723: Make delivery notes translatable (3.70 KB, patch)
2020-06-12 05:35 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 14723: Additional delivery notes to messages (12.40 KB, patch)
2021-02-10 10:37 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 14723: Make delivery notes translatable (3.74 KB, patch)
2021-02-10 10:37 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 14723: Make delivery notes translatable (3.81 KB, patch)
2021-03-15 10:26 UTC, Lund University Sandbox
Details | Diff | Splinter Review
Bug 14723: Add error codes and replace strings with them in template (5.20 KB, patch)
2021-04-28 06:16 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 14723: Additional delivery notes to messages (11.92 KB, patch)
2021-04-29 12:01 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 14723: Make delivery notes translatable (3.78 KB, patch)
2021-04-29 12:02 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 14723: Add error codes and replace strings with them in template (5.20 KB, patch)
2021-04-29 12:03 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 14723: (QA follow-up) Fix typo (1.10 KB, patch)
2021-04-29 13:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14723: Use DATETIME instead DATE (1.21 KB, patch)
2021-05-03 07:57 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 14723: Additional delivery notes to messages (11.99 KB, patch)
2021-05-05 11:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14723: Additional delivery notes to messages (11.99 KB, patch)
2021-05-05 11:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14723: Make delivery notes translatable (3.84 KB, patch)
2021-05-05 11:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14723: Add error codes and replace strings with them in template (5.27 KB, patch)
2021-05-05 11:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14723: (QA follow-up) Fix typo (1.15 KB, patch)
2021-05-05 11:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14723: Use DATETIME instead DATE (1.28 KB, patch)
2021-05-05 11:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14723: Make DBrev idempotent (1.20 KB, patch)
2021-05-07 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Taskula 2015-08-25 13:27:35 UTC
Currently at message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails.

Some SMS gateway providers send the customer a report about the status of delivery. For example, in our local library we have a provider that does this. With the SMS HTTP request to their Gateway, we also provide them a "report" parameter that includes an URL. Once the SMS delivery has been finished, they will notify us with the status to the given report URL.

If they notify us that the delivery was failed, we will modify its status in the database accordingly. They also provide a short description of why the delivery was failed.

To be able to store more detailed information about the delivery, I propose adding a new column (e.g "delivery_notes") to message_queue table.

In our local library we sometimes have angry borrowers complaining that they have not received SMS messages, so this type of enhancement would be very useful to be able to find out why such has happened.
Comment 1 Lari Taskula 2015-08-28 09:53:08 UTC Comment hidden (obsolete)
Comment 2 Lari Taskula 2015-08-28 12:05:33 UTC
To test template modifications:

1. Find a borrower with notices at members/notices.pl
2. Observe that there is no column for Delivery notes
3. Apply patch and run the given database update
4. Repeat step 1.
5. Observe that there is now a column for Delivery notes
Comment 3 Lari Taskula 2015-08-28 12:09:13 UTC Comment hidden (obsolete)
Comment 4 Lari Taskula 2015-09-01 14:15:35 UTC Comment hidden (obsolete)
Comment 5 Lari Taskula 2015-09-01 14:56:23 UTC Comment hidden (obsolete)
Comment 6 Lari Taskula 2015-09-01 15:20:50 UTC
Selenium tests are using PageObjects and Authentication rewrite (therefore dependencey on Bug 7174).

The patch only covers delivery notes at failure cases in C4::Letters. If you wish to see how delivery notes can be taken further with custom SMS::Send driver and SMS Gateway reports, please see Bug 14767.
Comment 7 Lari Taskula 2015-09-02 09:03:20 UTC
(In reply to Lari Taskula from comment #3)
> Created attachment 42074 [details] [review] [review]
> Bug 14723: (follow-up) Example of handling custom SMS::Send Driver reports
> 
Patch moved to bug 14767.
Comment 8 Lari Taskula 2015-09-02 14:57:29 UTC Comment hidden (obsolete)
Comment 9 Lari Taskula 2015-09-02 15:00:14 UTC Comment hidden (obsolete)
Comment 10 Lari Taskula 2015-09-04 09:20:37 UTC Comment hidden (obsolete)
Comment 11 Lari Taskula 2015-09-08 11:38:38 UTC Comment hidden (obsolete)
Comment 12 Lari Taskula 2015-09-08 12:12:04 UTC Comment hidden (obsolete)
Comment 13 Lari Taskula 2015-09-09 14:01:53 UTC Comment hidden (obsolete)
Comment 14 Lari Taskula 2015-09-14 07:53:00 UTC Comment hidden (obsolete)
Comment 15 Lari Taskula 2015-09-15 10:10:24 UTC Comment hidden (obsolete)
Comment 16 Lari Taskula 2015-09-16 09:40:52 UTC Comment hidden (obsolete)
Comment 17 Lari Taskula 2015-09-17 13:46:22 UTC Comment hidden (obsolete)
Comment 18 Lari Taskula 2015-09-17 13:50:06 UTC Comment hidden (obsolete)
Comment 19 Olli-Antti Kivilahti 2015-11-04 12:58:28 UTC Comment hidden (obsolete)
Comment 20 Olli-Antti Kivilahti 2015-11-04 15:17:06 UTC Comment hidden (obsolete)
Comment 21 Frédéric Demians 2015-11-06 13:42:16 UTC
Get this:

fatal: sha1 information is lacking or useless (C4/Letters.pm).
error: could not build fake ancestor
Comment 22 Lari Taskula 2017-04-12 13:09:59 UTC Comment hidden (obsolete)
Comment 23 Lari Taskula 2017-04-12 13:11:43 UTC
(In reply to Frédéric Demians from comment #21)
> Get this:
> 
> fatal: sha1 information is lacking or useless (C4/Letters.pm).
> error: could not build fake ancestor

I removed some dependencies and rebased this on master. Should now apply.
Comment 24 Lari Taskula 2017-04-13 09:12:14 UTC
Depends on Bug 18413 (otherwise Letters.t fails)
Comment 25 Lari Taskula 2017-04-13 09:16:34 UTC Comment hidden (obsolete)
Comment 26 Alex Buckley 2017-10-04 04:23:20 UTC
Hi Lari  

When applying your patch I got the following merge error on Letters.t file, (note: as I am using an up to date master branch patch 18413 is already applied (I double checked to make sure)): 

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 14723: Additional delivery notes to messages
Using index info to reconstruct a base tree...
M	C4/Letters.pm
M	installer/data/mysql/kohastructure.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
M	t/db_dependent/Letters.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Letters.t
CONFLICT (content): Merge conflict in t/db_dependent/Letters.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging C4/Letters.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 14723: Additional delivery notes to messages
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-14723-Additional-delivery-notes-to-messages-Zqwu8p.patch
Comment 27 Nick Clemens 2018-08-30 16:29:50 UTC Comment hidden (obsolete)
Comment 28 José-Mario Monteiro-Santos 2019-01-18 19:53:57 UTC
Failed to apply patch 

Bug 14723 - Additional delivery notes to messages

78313 - Bug 14723: Additional delivery notes to messages

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 14723: Additional delivery notes to messages
Using index info to reconstruct a base tree...
M	C4/Letters.pm
M	installer/data/mysql/kohastructure.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
M	t/db_dependent/Letters.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Letters.t
CONFLICT (content): Merge conflict in t/db_dependent/Letters.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging C4/Letters.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 14723: Additional delivery notes to messages
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-14723-Additional-delivery-notes-to-messages-vyvgwK.patch
Comment 29 Lari Taskula 2019-04-08 13:59:33 UTC Comment hidden (obsolete)
Comment 30 Liz Rea 2019-04-08 15:27:56 UTC Comment hidden (obsolete)
Comment 31 Josef Moravec 2019-04-17 13:23:09 UTC
Comment on attachment 87531 [details] [review]
Bug 14723: Additional delivery notes to messages

Review of attachment 87531 [details] [review]:
-----------------------------------------------------------------

::: C4/Letters.pm
@@ +1272,5 @@
>          unless ($patron) {
>              warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})";
>              _set_message_status( { message_id => $message->{'message_id'},
> +                                   status     => 'failed',
> +                                   delivery_note => 'Invalid borrowernumber '.$message->{borrowernumber} } );

These notes are untranslatable.
Comment 32 Katrin Fischer 2019-05-11 13:23:00 UTC
Comment on attachment 87531 [details] [review]
Bug 14723: Additional delivery notes to messages

Review of attachment 87531 [details] [review]:
-----------------------------------------------------------------

I took another look at the code and Joseph is right. Made some additional notes too.

::: C4/Letters.pm
@@ +912,2 @@
>  VALUES
> +( ?,              ?,       ?,       ?,        ?,           ?,                      ?,      NOW(),       ?,          ?,            ?,            ? )

I think NOW() might be a MySQLism from my research.

@@ +1281,5 @@
>              # warn "FAIL: No 'to_address' and no email for " . ($member->{surname} ||'') . ", borrowernumber ($message->{borrowernumber})";
>              # warning too verbose for this more common case?
>              _set_message_status( { message_id => $message->{'message_id'},
> +                                   status     => 'failed',
> +                                   delivery_note => 'Unable to find an email address for this borrower' } );

See above, these need to be dealt with in the template.

::: installer/data/mysql/atomicupdate/Bug-14723_-_Additional_delivery_notes_to_messages.perl
@@ +1,3 @@
> +$DBversion = 'XXX';  # will be replaced by the RM
> +if( CheckVersion( $DBversion ) ) {
> +    $dbh->do("ALTER TABLE message_queue ADD delivery_note TEXT");

Please use BEFORE or AFTER column to specify where the new column will be added exactly. This will help to keep things the same between new and old installations.
Comment 33 Lari Taskula 2020-03-17 21:20:11 UTC
I'm no longer able to work on this, so I'm setting assignee to default. Feel free to continue this work.
Comment 34 Emmi Takkinen 2020-06-12 05:30:10 UTC Comment hidden (obsolete)
Comment 35 Emmi Takkinen 2020-06-12 05:35:13 UTC Comment hidden (obsolete)
Comment 36 Victoria Faafia 2021-01-19 23:04:24 UTC
can you please add a test plan?
Comment 37 Victoria Faafia 2021-01-19 23:15:44 UTC
Applying: Bug 14723: Additional delivery notes to messages
Using index info to reconstruct a base tree...
M	C4/Letters.pm
M	installer/data/mysql/kohastructure.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
M	t/db_dependent/Letters.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Letters.t
CONFLICT (content): Merge conflict in t/db_dependent/Letters.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
Auto-merging installer/data/mysql/kohastructure.sql
CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql
Auto-merging C4/Letters.pm
CONFLICT (content): Merge conflict in C4/Letters.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 14723: Additional delivery notes to messages
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-14723-Additional-delivery-notes-to-messages-_b90Al.patch

Patch doesn't apply
Comment 38 Mazen Khallaf 2021-01-19 23:20:59 UTC
Failed to apply the patch in step 4


$ git bz apply 14723
Bug 14723 - Additional delivery notes to messages

105776 - Bug 14723: Additional delivery notes to messages
105777 - Bug 14723: Make delivery notes translatable

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 14723: Additional delivery notes to messages
Using index info to reconstruct a base tree...
M	C4/Letters.pm
M	installer/data/mysql/kohastructure.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
M	t/db_dependent/Letters.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Letters.t
CONFLICT (content): Merge conflict in t/db_dependent/Letters.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
Auto-merging installer/data/mysql/kohastructure.sql
CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql
Auto-merging C4/Letters.pm
CONFLICT (content): Merge conflict in C4/Letters.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 14723: Additional delivery notes to messages
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-14723-Additional-delivery-notes-to-messages-h__pEr.patch
Comment 39 Emmi Takkinen 2021-02-10 10:37:13 UTC
Created attachment 116642 [details] [review]
Bug 14723: Additional delivery notes to messages

This patch adds additional delivery notes to messages in message queue as there
can be multiple reasons for a delivery to fail.

Currently in message_queue we are given only two delivery statuses for messages,
"sent" and "failed". When the status becomes failed, we have no idea why it fails.

This feature can be useful with SMS gateway providers. Many SMS gateways inform
the application the reason of SMS delivery failure. With this feature, this
information can now be stored. As well as for emails, instead of simply logging
failures, we can now store the reason of failure directly into the message row
of message_queue.

Test plan:

1. Enable EnhancedMessagingPreferences syspref
2. Find a borrower with notices at members/notices.pl
3. Observe that there is no column for Delivery notes
4. Apply patch and run the given database update
5. Repeat step 1.
6. Observe that there is now a column for Delivery notes

Sponsored-by: Hypernova Oy

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 40 Emmi Takkinen 2021-02-10 10:37:44 UTC
Created attachment 116643 [details] [review]
Bug 14723: Make delivery notes translatable

Delivery notes weren't translatable. This patch adds
them in notices.tt. If note is Mail::Sendmail error generic
'Error occured while sending email' is shown.

Also fixes MySQLisms and adds AFTER to atomicupdate file.

To test prove t/db_dependent/Letters.t

Sponsored-by: Koha-Suomi Oy
Comment 41 Lund University Sandbox 2021-03-15 10:26:03 UTC
Created attachment 118225 [details] [review]
Bug 14723: Make delivery notes translatable

Delivery notes weren't translatable. This patch adds
them in notices.tt. If note is Mail::Sendmail error generic
'Error occured while sending email' is shown.

Also fixes MySQLisms and adds AFTER to atomicupdate file.

To test prove t/db_dependent/Letters.t

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Comment 42 Kyle M Hall 2021-04-09 18:09:15 UTC
Can you change the English strings with translatable codes? For example, "Missing SMS number" could be "MISSING_SMS". Martin just did work like this for the accounts tables.
Comment 43 Emmi Takkinen 2021-04-28 06:16:08 UTC
Created attachment 120252 [details] [review]
Bug 14723: Add error codes and replace strings with them in template

This patch adds error codes for delivery notes and replaces English
strings with them in template.

To test confirm that everything works according to the first test plan.
Comment 44 Stina Hallin 2021-04-29 11:35:54 UTC
The patch didn't apply:

CONFLICT (content): Merge conflict in t/db_dependent/Letters.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging C4/Letters.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 14723: Additional delivery notes to messages
The copy of the patch that failed is found in: .git/rebase-apply/patch
Comment 45 Emmi Takkinen 2021-04-29 12:01:51 UTC
Created attachment 120309 [details] [review]
Bug 14723: Additional delivery notes to messages

This patch adds additional delivery notes to messages in message queue as there
can be multiple reasons for a delivery to fail.

Currently in message_queue we are given only two delivery statuses for messages,
"sent" and "failed". When the status becomes failed, we have no idea why it fails.

This feature can be useful with SMS gateway providers. Many SMS gateways inform
the application the reason of SMS delivery failure. With this feature, this
information can now be stored. As well as for emails, instead of simply logging
failures, we can now store the reason of failure directly into the message row
of message_queue.

Test plan:

1. Enable EnhancedMessagingPreferences syspref
2. Find a borrower with notices at members/notices.pl
3. Observe that there is no column for Delivery notes
4. Apply patch and run the given database update
5. Repeat step 1.
6. Observe that there is now a column for Delivery notes

Sponsored-by: Hypernova Oy

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 46 Emmi Takkinen 2021-04-29 12:02:32 UTC
Created attachment 120310 [details] [review]
Bug 14723: Make delivery notes translatable

Delivery notes weren't translatable. This patch adds
them in notices.tt. If note is Mail::Sendmail error generic
'Error occured while sending email' is shown.

Also fixes MySQLisms and adds AFTER to atomicupdate file.

To test prove t/db_dependent/Letters.t

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Comment 47 Emmi Takkinen 2021-04-29 12:03:21 UTC
Created attachment 120312 [details] [review]
Bug 14723: Add error codes and replace strings with them in template

This patch adds error codes for delivery notes and replaces English
strings with them in template.

To test confirm everything works as intended.
Comment 48 Kyle M Hall 2021-04-29 13:51:19 UTC
prove t/db_dependent/Letters.t
t/db_dependent/Letters.t .. 6/86
#   Failed test 'Time status changed equals time queued when created in message_queue table'
#   at t/db_dependent/Letters.t line 148.
#          got: '2021-04-29 13:50:54'
#     expected: '2021-04-29 00:00:00'
t/db_dependent/Letters.t .. 70/86 # Looks like you failed 1 test of 86.
t/db_dependent/Letters.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/86 subtests

Test Summary Report
-------------------
t/db_dependent/Letters.t (Wstat: 256 Tests: 86 Failed: 1)
  Failed test:  21
  Non-zero exit status: 1
Files=1, Tests=86,  4 wallclock secs ( 0.03 usr  0.01 sys +  2.48 cusr  0.39 csys =  2.91 CPU)
Result: FAIL
Comment 49 Kyle M Hall 2021-04-29 13:51:49 UTC
Created attachment 120314 [details] [review]
Bug 14723: (QA follow-up) Fix typo
Comment 50 Emmi Takkinen 2021-05-03 07:57:44 UTC
Created attachment 120376 [details] [review]
Bug 14723: Use DATETIME instead DATE

Letters.t tests failed since time_queued didn't
include timecode. Use DATETIME when inserting
into message_queue.

To test prove t/db_dependent/Letters.t
Comment 51 Kyle M Hall 2021-05-05 11:18:00 UTC
Created attachment 120487 [details] [review]
Bug 14723: Additional delivery notes to messages

This patch adds additional delivery notes to messages in message queue as there
can be multiple reasons for a delivery to fail.

Currently in message_queue we are given only two delivery statuses for messages,
"sent" and "failed". When the status becomes failed, we have no idea why it fails.

This feature can be useful with SMS gateway providers. Many SMS gateways inform
the application the reason of SMS delivery failure. With this feature, this
information can now be stored. As well as for emails, instead of simply logging
failures, we can now store the reason of failure directly into the message row
of message_queue.

Test plan:

1. Enable EnhancedMessagingPreferences syspref
2. Find a borrower with notices at members/notices.pl
3. Observe that there is no column for Delivery notes
4. Apply patch and run the given database update
5. Repeat step 1.
6. Observe that there is now a column for Delivery notes

Sponsored-by: Hypernova Oy

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 52 Kyle M Hall 2021-05-05 11:37:42 UTC
Created attachment 120488 [details] [review]
Bug 14723: Additional delivery notes to messages

This patch adds additional delivery notes to messages in message queue as there
can be multiple reasons for a delivery to fail.

Currently in message_queue we are given only two delivery statuses for messages,
"sent" and "failed". When the status becomes failed, we have no idea why it fails.

This feature can be useful with SMS gateway providers. Many SMS gateways inform
the application the reason of SMS delivery failure. With this feature, this
information can now be stored. As well as for emails, instead of simply logging
failures, we can now store the reason of failure directly into the message row
of message_queue.

Test plan:

1. Enable EnhancedMessagingPreferences syspref
2. Find a borrower with notices at members/notices.pl
3. Observe that there is no column for Delivery notes
4. Apply patch and run the given database update
5. Repeat step 1.
6. Observe that there is now a column for Delivery notes

Sponsored-by: Hypernova Oy

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 53 Kyle M Hall 2021-05-05 11:38:00 UTC
Created attachment 120489 [details] [review]
Bug 14723: Make delivery notes translatable

Delivery notes weren't translatable. This patch adds
them in notices.tt. If note is Mail::Sendmail error generic
'Error occured while sending email' is shown.

Also fixes MySQLisms and adds AFTER to atomicupdate file.

To test prove t/db_dependent/Letters.t

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 54 Kyle M Hall 2021-05-05 11:38:06 UTC
Created attachment 120490 [details] [review]
Bug 14723: Add error codes and replace strings with them in template

This patch adds error codes for delivery notes and replaces English
strings with them in template.

To test confirm everything works as intended.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 55 Kyle M Hall 2021-05-05 11:38:10 UTC
Created attachment 120491 [details] [review]
Bug 14723: (QA follow-up) Fix typo

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 56 Kyle M Hall 2021-05-05 11:38:13 UTC
Created attachment 120492 [details] [review]
Bug 14723: Use DATETIME instead DATE

Letters.t tests failed since time_queued didn't
include timecode. Use DATETIME when inserting
into message_queue.

To test prove t/db_dependent/Letters.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 57 Jonathan Druart 2021-05-07 09:49:59 UTC
Created attachment 120667 [details] [review]
Bug 14723: Make DBrev idempotent
Comment 58 Jonathan Druart 2021-05-07 12:40:54 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 59 Fridolin Somers 2021-05-11 14:30:11 UTC
Enhancement not pushed to 20.11.x
Comment 60 Martin Renvoize 2021-08-05 07:34:44 UTC
Ouch.. there's stuff missing here.

We don't update kohastructure at all and we don't do a DB update to add the error_code field to the database either.  Finally, we don't add error_code to the _set_message_status routine.
Comment 61 Emmi Takkinen 2021-09-17 09:51:29 UTC
(In reply to Martin Renvoize from comment #60)
> Ouch.. there's stuff missing here.
> 
> We don't update kohastructure at all and we don't do a DB update to add the
> error_code field to the database either.  Finally, we don't add error_code
> to the _set_message_status routine.

I don't understand what you mean with error_code? We add delivery_note column where fail message is added and it's also added to _set_message_status routine. Also kohastructure is updated on first patch?
Comment 62 Martin Renvoize 2021-09-17 18:30:57 UTC
See bug 28813 where I fixed the issues.
Comment 63 Martin Renvoize 2021-09-17 18:34:12 UTC
To clarify, the followups here that converted from string note to translatable code broke the feature entirely ;)
Comment 64 Emmi Takkinen 2021-09-20 05:22:34 UTC
Gosh, I didn't realise I had made that change in follow up and hadn't tested it properly. My bad, sorry. Thanks Martin for fixing this :)
Comment 65 Martin Renvoize 2021-09-20 07:34:07 UTC
No worries at all, we all do it sometimes :)