Bug 31448 - Add option to re-send email receipt when UseEmailReceipts is enabled
Summary: Add option to re-send email receipt when UseEmailReceipts is enabled
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 33473
  Show dependency treegraph
 
Reported: 2022-08-24 07:00 UTC by Martin Renvoize
Modified: 2023-12-28 20:42 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/691
Text to go in the release notes:
**Sponsored by** *PTFS Europe*
Version(s) released in:
23.05.00


Attachments
Bug 31448: Replace 'Print' with 'Receipt' dropdown (6.98 KB, patch)
2022-08-24 07:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31448: Replace 'Print' with 'Receipt' dropdown (5.38 KB, patch)
2023-02-27 10:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31448: Replace 'Print' with 'Receipt' dropdown (5.44 KB, patch)
2023-02-28 14:02 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31448: Replace 'Print' with 'Receipt' dropdown (5.50 KB, patch)
2023-04-10 16:15 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2022-08-24 07:00:49 UTC
We already have a 'Print' option in the actions available on transactions in the borrower accounts page.  It would be nice to re-trigger the email receipt if UseEmailReceipts is enabled too.
Comment 1 Martin Renvoize 2022-08-24 07:30:49 UTC
Created attachment 139702 [details] [review]
Bug 31448: Replace 'Print' with 'Receipt' dropdown

This patch replaces the 'Print' button with a 'Receipt' menu dropdown
exposing 'Print' and 'Email' options when `UseEmailReceipts` is enabled

Test plan
1. Enable `UseEmailReceipts`
2. Navigate to a patron with paid charges
3. Note the new dropdown 'Receipt' menu
4. Confirm 'Print' works as expected
5. Confirm 'Email' works as expected
Comment 2 Martin Renvoize 2022-08-24 07:33:19 UTC
I quickly threw this together as a counterpart to bug 30619 which adds the option to email from the UI for the Point Of Sale screen.

Untested; I'll try to return to it soon, but wanted to leave it hear in case anyone wanted to continue in my absence.

The 'fa-receipt' font icon doesn't work for some reason.. not sure why yet.
Comment 3 Marius 2023-01-16 16:51:24 UTC
Points 1, 2 and 3 work very well.
But points 4 and 5 did not work.
When I click on "Print" or "Email" it makes an error 500.
Comment 4 Martin Renvoize 2023-02-27 10:09:31 UTC
Created attachment 147427 [details] [review]
Bug 31448: Replace 'Print' with 'Receipt' dropdown

This patch replaces the 'Print' button with a 'Receipt' menu dropdown
exposing 'Print' and 'Email' options when `UseEmailReceipts` is enabled

Test plan
1. Enable `UseEmailReceipts`
2. Navigate to a patron with paid charges
3. Note the new dropdown 'Receipt' menu
4. Confirm 'Print' works as expected
5. Confirm 'Email' works as expected
Comment 5 Martin Renvoize 2023-02-27 10:10:56 UTC
Reworked and tested.. all seems to be working now for me.  Thanks for showing interest Marius
Comment 6 PTFS Europe Sandboxes 2023-02-28 14:02:36 UTC
Created attachment 147488 [details] [review]
Bug 31448: Replace 'Print' with 'Receipt' dropdown

This patch replaces the 'Print' button with a 'Receipt' menu dropdown
exposing 'Print' and 'Email' options when `UseEmailReceipts` is enabled

Test plan
1. Enable `UseEmailReceipts`
2. Navigate to a patron with paid charges
3. Note the new dropdown 'Receipt' menu
4. Confirm 'Print' works as expected
5. Confirm 'Email' works as expected

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 7 Sally 2023-02-28 14:03:42 UTC
Works great, thanks for the patch.  Signing off.
Comment 8 Katrin Fischer 2023-03-04 14:58:41 UTC
There are some small 'glitches' here:

* The email option is visible even if the patron has no email address. But: this matches the current behavior as the initial email is also created in any case. So if we want to fix this, we should probably do it on a separate bug and catch the initial email and the resend one at the same time.

* When we resend an email from notices tab (look at the preview of a failed notice, 'resend' button on the right), we resend the original notice. This feature creates a new notice. I think this is ok, since we don't have the ID of the original notice and it's not a big issue to have a separate one, it might even be good to give more history.
Comment 9 Katrin Fischer 2023-03-04 15:03:38 UTC
(In reply to Katrin Fischer from comment #8)
> There are some small 'glitches' here:
> 
> * The email option is visible even if the patron has no email address. But:
> this matches the current behavior as the initial email is also created in
> any case. So if we want to fix this, we should probably do it on a separate
> bug and catch the initial email and the resend one at the same time.
> 
> * When we resend an email from notices tab (look at the preview of a failed
> notice, 'resend' button on the right), we resend the original notice. This
> feature creates a new notice. I think this is ok, since we don't have the ID
> of the original notice and it's not a big issue to have a separate one, it
> might even be good to give more history.

* Code duplication: the code for sending the notices is not duplicated in Account.pm and boraccount.pl. This is not ideal. It might be better to move it into a separate method and call it from both places?

Setting to failed QA for feedback.
Comment 10 Martin Renvoize 2023-03-13 17:37:09 UTC
That's actually a fair bit more work Katrin.. work I think should be undertaken as a distinct bug after the move to separate out Account::Lines to Account::Credits and Account::Debits.

* We would need to support credits taken via point of sale (thus having no related patron).
* We would also need to add message passing to the object to allow chaining whilst including additional information in the returned object.

For a relatively simple enhancement, we're now jumping into days of work territory.
Comment 11 Katrin Fischer 2023-03-13 21:25:24 UTC
Hm, actually I only point I wanted to talk about was number 3, the code duplication. For the other 2 I already noted that I would be ok with keeping things as they are.

And only talking about this piece of code here:

my $receipt_sent = 0;
+if ( $action eq 'send_receipt' ) {
+    my $credit_id = scalar $input->param('accountlines_id');
+    my $credit    = Koha::Account::Lines->find($credit_id);
+    my @credit_offsets =
+      $credit->credit_offsets( { type => 'APPLY' } )->as_list;
+    if (
+        my $letter = C4::Letters::GetPreparedLetter(
+            module      => 'circulation',
+            letter_code => uc( "ACCOUNT_" . $credit->credit_type_code ),
+            message_transport_type => 'email',
+            lang                   => $patron->lang,
+            tables                 => {
+                borrowers => $patron->borrowernumber,
+                branches  => C4::Context::mybranch,
+            },
+            substitute => {
+                credit  => $credit,
+                offsets => \@credit_offsets,
+            },
+        )
+      )
+    {
+        my $message_id = C4::Letters::EnqueueLetter(
+            {
+                letter                 => $letter,
+                borrowernumber         => $patron->borrowernumber,
+                message_transport_type => 'email',
+            }
+        );
+        C4::Letters::SendQueuedMessages( { message_id => $message_id } );
+        $receipt_sent = 1;
+    }
+    else {
+        $receipt_sent = -1;
+    }
+}

Compared to here:
https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/Account.pm#L113

Maybe it's not worth moving it into a separate thing, not sure. Maybe I missed some major differences?
But now also a little confused about the lot of other clean-up you were talking about?
Comment 12 Katrin Fischer 2023-04-10 16:15:56 UTC
Created attachment 149393 [details] [review]
Bug 31448: Replace 'Print' with 'Receipt' dropdown

This patch replaces the 'Print' button with a 'Receipt' menu dropdown
exposing 'Print' and 'Email' options when `UseEmailReceipts` is enabled

Test plan
1. Enable `UseEmailReceipts`
2. Navigate to a patron with paid charges
3. Note the new dropdown 'Receipt' menu
4. Confirm 'Print' works as expected
5. Confirm 'Email' works as expected

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Tomás Cohen Arazi 2023-05-05 13:14:58 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 14 Martin Renvoize 2023-05-12 14:42:57 UTC
Enhancements, won't be backported to 22.11.x