Bug 3150

Summary: Move emails for sending cart and list contents into notices tool
Product: Koha Reporter: Liz Rea <wizzyrea>
Component: NoticesAssignee: Aleisha Amohia <aleisha>
Status: Needs Signoff --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: aleisha, andrew, christian.stelzenmueller, david, hagud, jonathan.druart+koha, katrin.fischer, lucas, m.de.rooy, marjorie.barry-vila, martin.renvoize, nengard, sally.healey, tomascohen, wizzyrea
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17762
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12643
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14615
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23554
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16522
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17868
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23552
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22352
Change sponsored?: Sponsored Patch complexity: Large patch
Text to go in the release notes:
This patch creates notices using Template Toolkit syntax for sending emails containing cart (named CART) and list (named LIST) contents. This provides libraries more flexibility around what the emails contain and means they can be more translatable.
Version(s) released in:
Bug Depends on: 27266    
Bug Blocks: 7001    
Attachments: Bug 3150: WIP
Bug 3150: WIP
Bug 3150: WIP
Bug 3150: Move emails for sending cart and list contents to notices
Bug 3150: Move emails for sending cart and list contents to notices
Bug 3150: Move emails for sending cart and list contents to notices
Bug 3150: Move emails for sending cart and list contents to notices
Bug 3150: (follow-up) HTML filtering TT notices and removing old files
Bug 3150: Move emails for sending cart and list contents to notices
Bug 3150: (follow-up) HTML filtering TT notices and removing old files
Bug 3150: Move emails for sending cart and list contents to notices
Bug 3150: (follow-up) HTML filtering TT notices and removing old files
Bug 3150: Move emails for sending cart and list contents to notices
Bug 3150: (follow-up) HTML filtering TT notices and removing old files
Bug 3150: (follow-up) Reformat notices, don't send if no sender email

Description Chris Cormack 2010-05-21 01:07:03 UTC


---- Reported by wizzyrea@gmail.com 2009-04-23 14:40:27 ----

This is currently uneven in application. Holds notifications are editable through the web interface, but cart/list/shelf/suggestion response emails are governed by templates.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:07 UTC  ---

This bug was previously known as _bug_ 3150 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3150

Actual time not defined. Setting to 0.0
CC member kdr@ckls.org does not have an account here

Comment 1 Liz Rea 2012-02-20 16:32:01 UTC
We should add these templates to the notices system once it is pushed.
Comment 2 Katrin Fischer 2013-01-28 12:52:16 UTC
The one nice thing about the emails being in the templates is, that they are translatable. Maybe we could have an option to send cart/shelf list mails depending on chosen templates?
Comment 3 Katrin Fischer 2016-12-28 14:11:12 UTC
We had a lot of libraries ask about this by now - I didn't like the idea at the beginning because it would mean losing translatability. But there seems to be some recent work on making notices translatable - so that would be great.

Adding other bugs as see alsos:
Bug 17762 - Ability to translate notices
Comment 4 Katrin Fischer 2017-01-04 09:45:48 UTC
*** Bug 7983 has been marked as a duplicate of this bug. ***
Comment 5 Katrin Fischer 2019-05-04 20:08:34 UTC
The only hardcoded notifications at the moment that I can think of that are not dealt with on other bugs (ILL, slips for payment + fines) are cart and list. Changing the bug title to reflect that.

We'll need to make sure they work with the TranslateNotices system preference.
Comment 6 Katrin Fischer 2019-08-18 21:22:59 UTC
*** Bug 22931 has been marked as a duplicate of this bug. ***
Comment 7 Katrin Fischer 2019-11-24 12:53:37 UTC
*** Bug 23882 has been marked as a duplicate of this bug. ***
Comment 8 Katrin Fischer 2020-10-22 18:56:29 UTC
*** Bug 26786 has been marked as a duplicate of this bug. ***
Comment 9 Aleisha Amohia 2020-12-03 02:48:16 UTC
Created attachment 114127 [details] [review]
Bug 3150: WIP

This is not ready yet!
Comment 10 Aleisha Amohia 2020-12-17 00:38:53 UTC
I'm currently working on this, and a big goal is to make it work as a notice with the template toolkit syntax. I believe this depends on moving some of the old C4/Biblio.pm subroutines (GetMarcAuthors etc) into Koha/Biblio.pm. That work would be substantial and needs to be done first, in separate bugs.
Comment 11 Katrin Fischer 2020-12-17 09:35:27 UTC
Could we imagine replacing the old subroutines by a more generic approach of accessing the information in the MARC record instead of moving them? 

Thinking of something like:

[% record.245a %]
We can actually do something similar in CSV profiles, but it appears sadly undocumented in the manual: Bug 12404 (the help file with the documentation was removed, but some examples in the comment of the bug)

I think if we don't match the current output 100% that would actually be a good thing as the old routines sometimes display subfields that you don't really want to see in the notices, and it's all hard coded and not nice to use or change or format. So we will likely have more issues to deal with later on always coming back to these methods.

And it could be really useful for other notices as well (later).
Comment 12 Jonathan Druart 2020-12-17 09:59:51 UTC
Hello Aleisha,

This is a very interesting part to move to the notice templates!

However I am expecting it to be tricky, as you saw already :)

Before starting to implement anything I would suggest you to come with a plan. I can provide you early feedback to see if we are starting with the best solution.

In my opinion we should only pass Koha::Objects-based variables to the template. Reusing the existing variables will make us start with obsolete code (it will be harder to improve it in the future as the libraries will have this TT code in their templates).

(In reply to Katrin Fischer from comment #11)
> Could we imagine replacing the old subroutines by a more generic approach of
> accessing the information in the MARC record instead of moving them? 
> 
> Thinking of something like:
> 
> [% record.245a %]

Says we are talking about GetMarcUrls, it could be something like:
[% SET marc_record = biblio.metadata.record %]
[% FOR f IN marc_record.field('856') %]
  [%# Then do something for every 856 field
[% END %]

But then you are going to rewrite the code that is in a subroutine to a TT syntax, that will be copy/pasted in different places.
And it's not what we want, it's too error prone (no test coverage, dup of code).

The only long term and viable solution is to move them to the Koha namespace.
Have the Get* subroutines moved to Koha::Biblio methods, have the test coverage, replace the existing occurrences, and finally reuse them from the template notices.
But this task is huge...
Comment 13 Jonathan Druart 2020-12-17 10:58:51 UTC
I would start with passing Koha::Biblios to the template and see how easy/hard it is to replace the existing template file with a notice template using the TT syntax.

[% FOR biblio IN biblios %]

  [%# title and subtitle are easy to retrieve %]

  [%# authors, we loop on the MARC fields %]

  [%# isbn, we need Koha::Biblio->get_isbns %]

  [%# and so on...

[% END %]

First with send shelves at the OPAC.
Comment 14 Katrin Fischer 2020-12-17 11:25:52 UTC
Hi Aleisha, 

I think Joubu's right... after looking at the methods I realized that we need something easier to use for people in the templates and also something that will work with MARC21 and UNIMARC. 

Moving them one by one on separate bugs like you suggested is a good idea. Maybe we could use the feedback on bug 16522 or start with one to make sure we got a good pattern before moving all of them.
Comment 15 Aleisha Amohia 2020-12-17 19:57:08 UTC
Yup... What Joubu has said is just a longer way of saying what I said in my comment :) so glad we're all agreed.
Comment 16 Katrin Fischer 2020-12-17 20:05:44 UTC
(In reply to Aleisha Amohia from comment #15)
> Yup... What Joubu has said is just a longer way of saying what I said in my
> comment :) so glad we're all agreed.

Took me a little bit to get there :)
Comment 17 Aleisha Amohia 2020-12-17 23:09:07 UTC
Starting with GetMarcAuthors at Bug 27266
Comment 18 Aleisha Amohia 2020-12-18 01:21:22 UTC
GetMarcNotes at Bug 27268
Comment 19 Aleisha Amohia 2020-12-18 02:35:35 UTC
GetMarcSubjects at Bug 27269
Comment 20 Aleisha Amohia 2020-12-18 02:53:05 UTC
GetMarcSeries at Bug 27270
Comment 21 Aleisha Amohia 2020-12-18 02:53:43 UTC
GetMarcUrls at Bug 27271
Comment 22 Aleisha Amohia 2020-12-18 02:58:33 UTC
GetItemsInfo at Bug 27272
Comment 23 Jonathan Druart 2020-12-18 09:28:04 UTC
(In reply to Aleisha Amohia from comment #20)
> GetMarcSeries at Bug 27270

This is not used and is not needed from send-basket and send-shelves scripts. (ie. no "MARCSERIES" in templates)
Comment 24 Jonathan Druart 2020-12-18 09:29:32 UTC
(In reply to Aleisha Amohia from comment #18)
> GetMarcNotes at Bug 27268

No occurrences of "GetMarcNotes" from send-basket and send-shelves scripts.
Comment 25 Jonathan Druart 2020-12-18 09:46:04 UTC
(In reply to Aleisha Amohia from comment #17)
> Starting with GetMarcAuthors at Bug 27266

Are you sure you will want to reuse that from the template notice?
It seems that to answer bug 17868 you will only need to loop over the MARC subfields (for instance GetMarcAuthors is building links we don't need).

Additional questions: should not we take advantage of the move to fix the inconsistencies in the return values?

I don't think "just moving" code from C4 to Koha is sufficient.
Comment 26 Katrin Fischer 2020-12-18 10:03:28 UTC
(In reply to Jonathan Druart from comment #25)
> (In reply to Aleisha Amohia from comment #17)
> > Starting with GetMarcAuthors at Bug 27266
> 
> Are you sure you will want to reuse that from the template notice?
> It seems that to answer bug 17868 you will only need to loop over the MARC
> subfields (for instance GetMarcAuthors is building links we don't need).
> 
> Additional questions: should not we take advantage of the move to fix the
> inconsistencies in the return values?
> 
> I don't think "just moving" code from C4 to Koha is sufficient.

To further simplify things we could let go of the marcflavour parameter as this only seems to be used in context where the marcflavour of the installation is used.

I think another question is if we are using them only for the notices or to replace all calls to templates - if we want to replace the other calls, we'd need to keep the links etc.
Comment 27 Aleisha Amohia 2020-12-20 20:32:33 UTC
Ah I see what you're both saying. i'll give that a go
Comment 28 Tomás Cohen Arazi 2020-12-20 22:18:30 UTC
In my opinion, we should pass Koha::Object(s) to the template as much as possible. It will be much easier to document the things that are available for rendering, as opposed to the current situation, on which it is not straight forward to know what is available in which context.
Comment 29 Aleisha Amohia 2020-12-21 02:12:43 UTC
Created attachment 114545 [details] [review]
Bug 3150: WIP
Comment 30 Aleisha Amohia 2020-12-21 02:14:28 UTC
I've made some starting changes to the notices to use Koha::Biblio objects mostly (you can see it in the atomic update in the patch).

I need some guidance/help with a few things:
- I wasn't sure where to get the dewey and classification info from
- I wasn't sure which notes field to use (biblioitems or biblio)
- the Author and Subjects subroutines are the hardest for me to refactor because I'm not sure how different libraries use them

Some feedback would be great!
Comment 31 Jonathan Druart 2020-12-21 11:00:29 UTC
(In reply to Aleisha Amohia from comment #30)
> I've made some starting changes to the notices to use Koha::Biblio objects
> mostly (you can see it in the atomic update in the patch).
> 
> I need some guidance/help with a few things:
> - I wasn't sure where to get the dewey and classification info from

It looks like they are always empty.
It's broken since the following commit:
  commit af466ca41ab02150f4f6c60bf6262b079b2e6e85
  Date:   Thu Oct 18 18:53:52 2007 -0500
  start of reworking call number handling and other Biblio changes

IMO we should not reintroduce them on this bug report.

> - I wasn't sure which notes field to use (biblioitems or biblio)

It's coming GetBiblioData
 729     my $query = " SELECT * , biblioitems.notes AS bnotes, itemtypes.notforloan as bi_notforloan, biblio.notes                                                                                                

So it's the 'notes' from biblio.

> - the Author and Subjects subroutines are the hardest for me to refactor
> because I'm not sure how different libraries use them

Hum... Another weirdness.
On opac-sendshelf: MARCSUBJCTS is set but not used in the template, 'subject' is used instead, but never sent from the pl...
and it's the same for the other ones (sendbasket, sendshelf both OPAC and staff interfaces).
I'd drop it as well.

For MARCAUTHORS I'd try and write the equivalent of C4::Biblio::GetMarcAuthors in TT, and see how it goes (10 lines or 30 lines?). If it's too ugly then move to Koha::Biblio->get_MARC_authors (or get_authors_from_MARC?)

> Some feedback would be great!

Don't forget that you are in a quite dirty place. Don't loose time with things that are not working, drop them.
I'd suggest that you focus on the simplest template first, no need to work in parallel on the four ones.

Additional note, in your patch you are not looping at all on a Koha::Biblio set. As it's a WIP maybe you are aware of it, but I prefer to highlight it in case you missed it.
Comment 32 Aleisha Amohia 2020-12-21 21:10:44 UTC
Created attachment 114579 [details] [review]
Bug 3150: WIP
Comment 33 Aleisha Amohia 2020-12-22 01:38:07 UTC
Created attachment 114582 [details] [review]
Bug 3150: Move emails for sending cart and list contents to notices

This patch creates notices using Template Toolkit syntax for sending
emails containing cart and list contents.

To test:
1. Apply Bug 27266
2. Run update database and restart services
3. In the staff client, add multiple items to your cart and to a list
4. Go to your cart and click Send to email the contents
5. Add an email and a comment and click Send
6. Confirm the information shown in the success message is correct
7. In your terminal, log into the database. View the message queue ( i.e. select * from message_queue; ). Confirm that your email has been queued and the content is all correct. Confirm the cart contents has been included as an attachment.
8. Go to your list and click Send list to email the contents
9. Repeat steps 5-7
10. Log into the OPAC
11. Add multiple items to your cart and to a list
12. Repeat steps 4-9
13. By the end, you should have four emails in your message queue. All
of the data about the items should be correct, they should all have
attachments, and be addressed to the correct email address.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Comment 34 David Nind 2021-02-24 05:12:08 UTC
Created attachment 117237 [details] [review]
Bug 3150: Move emails for sending cart and list contents to notices

This patch creates notices using Template Toolkit syntax for sending
emails containing cart and list contents.

To test:
1. Apply Bug 27266
2. Run update database and restart services
3. In the staff client, add multiple items to your cart and to a list
4. Go to your cart and click Send to email the contents
5. Add an email and a comment and click Send
6. Confirm the information shown in the success message is correct
7. In your terminal, log into the database. View the message queue ( i.e. select * from message_queue; ). Confirm that your email has been queued and the content is all correct. Confirm the cart contents has been included as an attachment.
8. Go to your list and click Send list to email the contents
9. Repeat steps 5-7
10. Log into the OPAC
11. Add multiple items to your cart and to a list
12. Repeat steps 4-9
13. By the end, you should have four emails in your message queue. All
of the data about the items should be correct, they should all have
attachments, and be addressed to the correct email address.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: David Nind <david@davidnind.com>
Comment 35 David Nind 2021-02-24 05:20:50 UTC
Testing notes (koha-testing-docker):
- I was able to test the emails being sent by copying the HTML for the message from the database and pasting it into a Thunderbird email message (Insert > HTML) - everything looked okay to me.
- I wasn't able to properly test the attachment, but I'm going to assume the code creates a valid iso2709 file for attaching, sending, and saving/viewing by the recipient.
- There was some minor things I noticed with the email input forms (the ones that pop up after you click the email buttons in Koha) - the 'Comment' label for the input box follows directly after the email address input box. Depending on the width of the pop up window the label isn't aligned properly to the left, but this behaviour is the same as what it was before the patch was applied.
Comment 36 Martin Renvoize 2021-03-16 11:46:48 UTC
Created attachment 118313 [details] [review]
Bug 3150: Move emails for sending cart and list contents to notices

This patch creates notices using Template Toolkit syntax for sending
emails containing cart and list contents.

To test:
1. Apply Bug 27266
2. Run update database and restart services
3. In the staff client, add multiple items to your cart and to a list
4. Go to your cart and click Send to email the contents
5. Add an email and a comment and click Send
6. Confirm the information shown in the success message is correct
7. In your terminal, log into the database. View the message queue ( i.e. select * from message_queue; ). Confirm that your email has been queued and the content is all correct. Confirm the cart contents has been included as an attachment.
8. Go to your list and click Send list to email the contents
9. Repeat steps 5-7
10. Log into the OPAC
11. Add multiple items to your cart and to a list
12. Repeat steps 4-9
13. By the end, you should have four emails in your message queue. All
of the data about the items should be correct, they should all have
attachments, and be addressed to the correct email address.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 37 Martin Renvoize 2021-03-16 11:49:27 UTC
This feels good in testing and QA scripts are passing.

Well done working through this Aleisha, I imagine it was rather painful.

Passing QA

(Caveat applies for dependanct bug, I think get_marc_authors is nicer than get_authors_from_MARC).
Comment 38 Jonathan Druart 2021-04-07 10:25:05 UTC
Why don't you HTML filter the variables in the template notice?
Comment 39 Jonathan Druart 2021-04-07 10:37:32 UTC
Do we still need the following files?

koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasket.tt
koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/sendshelf.tt
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendbasket.tt
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelf.tt
Comment 40 Aleisha Amohia 2021-05-05 04:32:14 UTC
(In reply to Jonathan Druart from comment #38)
> Why don't you HTML filter the variables in the template notice?

The original notices use $raw filtering (sendbasket.tt etc), so that's what I used. I can change them all to HTML.

(In reply to Jonathan Druart from comment #39)
> Do we still need the following files?
> 
> koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasket.tt
> koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/sendshelf.tt
> koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendbasket.tt
> koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelf.tt

I don't think so, I'll remove them.
Comment 41 Aleisha Amohia 2021-05-05 04:41:50 UTC
Created attachment 120477 [details] [review]
Bug 3150: Move emails for sending cart and list contents to notices

This patch creates notices using Template Toolkit syntax for sending
emails containing cart and list contents.

To test:
1. Apply Bug 27266
2. Run update database and restart services
3. In the staff client, add multiple items to your cart and to a list
4. Go to your cart and click Send to email the contents
5. Add an email and a comment and click Send
6. Confirm the information shown in the success message is correct
7. In your terminal, log into the database. View the message queue ( i.e. select * from message_queue; ). Confirm that your email has been queued and the content is all correct. Confirm the cart contents has been included as an attachment.
8. Go to your list and click Send list to email the contents
9. Repeat steps 5-7
10. Log into the OPAC
11. Add multiple items to your cart and to a list
12. Repeat steps 4-9
13. By the end, you should have four emails in your message queue. All
of the data about the items should be correct, they should all have
attachments, and be addressed to the correct email address.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 42 Aleisha Amohia 2021-05-05 04:41:56 UTC
Created attachment 120478 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files
Comment 43 David Nind 2021-05-13 12:18:36 UTC
I was too slow on this one as well... the patch no longer applies 8-(..

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 3150: Move emails for sending cart and list contents to notices
Using index info to reconstruct a base tree...
M	installer/data/mysql/en/mandatory/sample_notices.yml
M	installer/data/mysql/fr-CA/obligatoire/sample_notices.sql
M	installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
A	installer/data/mysql/it-IT/necessari/notices.sql
M	installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql
A	installer/data/mysql/pl-PL/mandatory/sample_notices.sql
A	installer/data/mysql/ru-RU/mandatory/sample_notices.sql
A	installer/data/mysql/uk-UA/mandatory/sample_notices.sql
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): installer/data/mysql/uk-UA/mandatory/sample_notices.sql deleted in HEAD and modified in Bug 3150: Move emails for sending cart and list contents to notices. Version Bug 3150: Move emails for sending cart and list contents to notices of installer/data/mysql/uk-UA/mandatory/sample_notices.sql left in tree.
CONFLICT (modify/delete): installer/data/mysql/ru-RU/mandatory/sample_notices.sql deleted in HEAD and modified in Bug 3150: Move emails for sending cart and list contents to notices. Version Bug 3150: Move emails for sending cart and list contents to notices of installer/data/mysql/ru-RU/mandatory/sample_notices.sql left in tree.
CONFLICT (modify/delete): installer/data/mysql/pl-PL/mandatory/sample_notices.sql deleted in HEAD and modified in Bug 3150: Move emails for sending cart and list contents to notices. Version Bug 3150: Move emails for sending cart and list contents to notices of installer/data/mysql/pl-PL/mandatory/sample_notices.sql left in tree.
Auto-merging installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql
CONFLICT (modify/delete): installer/data/mysql/it-IT/necessari/notices.sql deleted in HEAD and modified in Bug 3150: Move emails for sending cart and list contents to notices. Version Bug 3150: Move emails for sending cart and list contents to notices of installer/data/mysql/it-IT/necessari/notices.sql left in tree.
Auto-merging installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
Auto-merging installer/data/mysql/fr-CA/obligatoire/sample_notices.sql
Auto-merging installer/data/mysql/en/mandatory/sample_notices.yml
error: Failed to merge in the changes.
Patch failed at 0001 Bug 3150: Move emails for sending cart and list contents to notices
Comment 44 Aleisha Amohia 2021-05-17 01:29:03 UTC
Created attachment 121026 [details] [review]
Bug 3150: Move emails for sending cart and list contents to notices

This patch creates notices using Template Toolkit syntax for sending
emails containing cart and list contents.

To test:
1. Apply Bug 27266
2. Run update database and restart services
3. In the staff client, add multiple items to your cart and to a list
4. Go to your cart and click Send to email the contents
5. Add an email and a comment and click Send
6. Confirm the information shown in the success message is correct
7. In your terminal, log into the database. View the message queue ( i.e. select * from message_queue; ). Confirm that your email has been queued and the content is all correct. Confirm the cart contents has been included as an attachment.
8. Go to your list and click Send list to email the contents
9. Repeat steps 5-7
10. Log into the OPAC
11. Add multiple items to your cart and to a list
12. Repeat steps 4-9
13. By the end, you should have four emails in your message queue. All
of the data about the items should be correct, they should all have
attachments, and be addressed to the correct email address.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Aleisha Amohia 2021-05-17 01:29:09 UTC
Created attachment 121027 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files
Comment 46 David Nind 2021-05-17 10:35:43 UTC
Created attachment 121037 [details] [review]
Bug 3150: Move emails for sending cart and list contents to notices

This patch creates notices using Template Toolkit syntax for sending
emails containing cart and list contents.

To test:
1. Apply Bug 27266
2. Run update database and restart services
3. In the staff client, add multiple items to your cart and to a list
4. Go to your cart and click Send to email the contents
5. Add an email and a comment and click Send
6. Confirm the information shown in the success message is correct
7. In your terminal, log into the database. View the message queue ( i.e. select * from message_queue; ). Confirm that your email has been queued and the content is all correct. Confirm the cart contents has been included as an attachment.
8. Go to your list and click Send list to email the contents
9. Repeat steps 5-7
10. Log into the OPAC
11. Add multiple items to your cart and to a list
12. Repeat steps 4-9
13. By the end, you should have four emails in your message queue. All
of the data about the items should be correct, they should all have
attachments, and be addressed to the correct email address.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 47 David Nind 2021-05-17 10:35:48 UTC
Created attachment 121038 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>
Comment 48 Marcel de Rooy 2021-06-25 08:59:49 UTC
Looking here
Comment 49 Marcel de Rooy 2021-06-25 09:27:13 UTC
There is a problem with these mails. I do not have an attachment, but this is in the body:

This is a multi-part message in MIME format.

--_----------=_1624613146275290
Content-Disposition: inline
Content-Length: 1127
Content-Transfer-Encoding: binary
Content-Type: text/html; charset="utf-8"

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"

[etc]
Comment 50 Marcel de Rooy 2021-06-25 09:28:09 UTC
+        my $user_email = $patron->first_valid_email_address || C4::Context->preference('KohaAdminEmailAddress');

You might wonder if we should send this mail if the first_valid_email_address would be empty. Why should the recipient reply to the library here?
Comment 51 Marcel de Rooy 2021-06-25 09:29:01 UTC
This would be a great improvement imo. But it still needs soem attention..
Comment 52 Aleisha Amohia 2021-07-22 03:04:03 UTC
Created attachment 123020 [details] [review]
Bug 3150: Move emails for sending cart and list contents to notices

This patch creates notices using Template Toolkit syntax for sending
emails containing cart and list contents.

To test:
1. Apply Bug 27266
2. Run update database and restart services
3. In the staff client, add multiple items to your cart and to a list
4. Go to your cart and click Send to email the contents
5. Add an email and a comment and click Send
6. Confirm the information shown in the success message is correct
7. In your terminal, log into the database. View the message queue ( i.e. select * from message_queue; ). Confirm that your email has been queued and the content is all correct. Confirm the cart contents has been included as an attachment.
8. Go to your list and click Send list to email the contents
9. Repeat steps 5-7
10. Log into the OPAC
11. Add multiple items to your cart and to a list
12. Repeat steps 4-9
13. By the end, you should have four emails in your message queue. All
of the data about the items should be correct, they should all have
attachments, and be addressed to the correct email address.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: David Nind <david@davidnind.com>
Comment 53 Aleisha Amohia 2021-07-22 03:04:09 UTC
Created attachment 123021 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>
Comment 54 Aleisha Amohia 2021-07-22 03:04:15 UTC
Created attachment 123022 [details] [review]
Bug 3150: (follow-up) Reformat notices, don't send if no sender email

This patch reformats the notices so that the is_html flag is disabled
and the notices display better

It also throws an error if trying to send an email but the logged in
borrower has no valid email address.
Comment 55 Aleisha Amohia 2021-07-22 03:05:21 UTC
(In reply to Marcel de Rooy from comment #49)
> There is a problem with these mails. I do not have an attachment, but this
> is in the body:
>

I think that is the way MIME::Lite works but correct me if I'm wrong! I think the attachment gets embedded in the email. So hard to test by looking at the message queue or notices tab, would be good to test with the actual email sent.
Comment 56 Marcel de Rooy 2021-07-28 12:05:40 UTC
(In reply to Aleisha Amohia from comment #55)
> (In reply to Marcel de Rooy from comment #49)
> > There is a problem with these mails. I do not have an attachment, but this
> > is in the body:
> >
> 
> I think that is the way MIME::Lite works but correct me if I'm wrong! I
> think the attachment gets embedded in the email. So hard to test by looking
> at the message queue or notices tab, would be good to test with the actual
> email sent.

IIRC I looked here at the emails via Outlook web client.