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.
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
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.
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.
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
Reworked and tested.. all seems to be working now for me. Thanks for showing interest Marius
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>
Works great, thanks for the patch. Signing off.
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.
(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.
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.
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?
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>
Pushed to master for 23.05. Nice work everyone, thanks!
Enhancements, won't be backported to 22.11.x