We should add these templates to the notices system once it is pushed.

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?

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

*** Bug 7983 has been marked as a duplicate of this bug. ***

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.

*** Bug 22931 has been marked as a duplicate of this bug. ***

*** Bug 23882 has been marked as a duplicate of this bug. ***

*** Bug 26786 has been marked as a duplicate of this bug. ***

Created attachment 114127 [details] [review]
Bug 3150: WIP

This is not ready yet!

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.

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).

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...

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.

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.

Yup... What Joubu has said is just a longer way of saying what I said in my comment :) so glad we're all agreed.

(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 :)

Starting with GetMarcAuthors at Bug 27266

GetMarcNotes at Bug 27268

GetMarcSubjects at Bug 27269

GetMarcSeries at Bug 27270

GetMarcUrls at Bug 27271

GetItemsInfo at Bug 27272

(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)

(In reply to Aleisha Amohia from comment #18)
> GetMarcNotes at Bug 27268

No occurrences of "GetMarcNotes" from send-basket and send-shelves scripts.

(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.

(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.

Ah I see what you're both saying. i'll give that a go

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.

Created attachment 114545 [details] [review]
Bug 3150: WIP

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!

(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.

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.

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).

Why don't you HTML filter the variables in the template notice?

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

(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.

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

Looking here

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]

+        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?

This would be a great improvement imo. But it still needs soem attention..

(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.

(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.

Created attachment 125225 [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>

Created attachment 125226 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 125227 [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.

I get an error from the opac-sendbasket popup whether or not I'm logged in:

Could not compile /kohadevbox/koha/opac/opac-sendbasket.pl: syntax error at /kohadevbox/koha/opac/opac-sendbasket.pl line 122, near "else"
BEGIN not safe after errors--compilation aborted at /kohadevbox/koha/opac/opac-sendbasket.pl line 137.

The QA tool also raises several warnings.

Created attachment 125379 [details] [review]
Bug 3150: (follow-up) Add missing bracket

(In reply to Owen Leonard from comment #60)
> I get an error from the opac-sendbasket popup whether or not I'm logged in:
> 
> Could not compile /kohadevbox/koha/opac/opac-sendbasket.pl: syntax error at
> /kohadevbox/koha/opac/opac-sendbasket.pl line 122, near "else"
> BEGIN not safe after errors--compilation aborted at
> /kohadevbox/koha/opac/opac-sendbasket.pl line 137.

Have fixed

> 
> The QA tool also raises several warnings.

The QA test tools all pass for me - can you please paste the output?

Patch no longer applies 8-(..

Applying: Bug 3150: Move emails for sending cart and list contents to notices
Using index info to reconstruct a base tree...
M	basket/sendbasket.pl
M	opac/opac-sendbasket.pl
M	opac/opac-sendshelf.pl
M	virtualshelves/sendshelf.pl
Falling back to patching base and 3-way merge...
Auto-merging virtualshelves/sendshelf.pl
CONFLICT (content): Merge conflict in virtualshelves/sendshelf.pl
Auto-merging opac/opac-sendshelf.pl
CONFLICT (content): Merge conflict in opac/opac-sendshelf.pl
Auto-merging opac/opac-sendbasket.pl
CONFLICT (content): Merge conflict in opac/opac-sendbasket.pl
Auto-merging basket/sendbasket.pl
CONFLICT (content): Merge conflict in basket/sendbasket.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 3150: Move emails for sending cart and list contents to notices
hint: Use 'git am --show-current-patch' to see the failed patch

Created attachment 126544 [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>

Created attachment 126545 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 126546 [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.

Created attachment 126547 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Patch doesn't apply :/

Sorry this applied cleanly for me! Can you please paste your merge conflict output?

Oops commented on the wrong bug!

Created attachment 130150 [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>

Created attachment 130151 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 130152 [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.

Created attachment 130153 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Merge conflict when applying 27266:

Falling back to patching base and 3-way merge...
Auto-merging virtualshelves/sendshelf.pl
CONFLICT (content): Merge conflict in virtualshelves/sendshelf.pl
Auto-merging t/db_dependent/Koha/Biblio.t
CONFLICT (content): Merge conflict in t/db_dependent/Koha/Biblio.t
Auto-merging t/Biblio.t
Auto-merging opac/opac-sendshelf.pl
CONFLICT (content): Merge conflict in opac/opac-sendshelf.pl
Auto-merging opac/opac-sendbasket.pl
CONFLICT (content): Merge conflict in opac/opac-sendbasket.pl
Auto-merging opac/opac-detail.pl
Auto-merging opac/opac-basket.pl
CONFLICT (content): Merge conflict in opac/opac-basket.pl
Auto-merging basket/sendbasket.pl
CONFLICT (content): Merge conflict in basket/sendbasket.pl
Auto-merging basket/basket.pl
CONFLICT (content): Merge conflict in basket/basket.pl
Auto-merging Koha/Biblio.pm
CONFLICT (content): Merge conflict in Koha/Biblio.pm
Auto-merging C4/Biblio.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 27266: Move GetMarcAuthors to Koha namespace

Bug 27266 has already been upstreamed so you don't need to apply it first. Bug 3150 should apply cleanly on the latest main branch.

Hmm, should we be dropping the X-Abuse-Report headers here?.. was that deliberate?

Maybe a reabase issue - we do want to keep them.

Hi both,

Would it be reasonable to add the X-Abuse-Report part to the C4::Letters::_send_message_by_email? This is the only place I can see to specify a header. If so I can create a different bug that this one can depend on.

Also, is that specified because we use KohaAdminEmailAddress as the from address?

Aleisha

I think I have confused it with another thing we did a while ago as an anti-Spam measure (bug 24588)

I am not sure if it has to do with the from address, but there are other notices we send or potentially send from KohaAdminEmailAddress, like the cart emails.

Martin, as you pointed it out, do we need to keep (and maybe move to a separate report to make it consistent) or can we drop that here?

Hi everyone,

i'm sorry, step 7 in the test plan doesn't work for me. There are no messages in the message queue.

I hope i'm not doing anything wrong. I also tried applying bug 3150 and 27266, one after the other, because i wasn't sure, but the results were the same.

I got errors from each of these scripts:

basket/sendbasket.pl
opac/opac-sendbasket.pl
opac/opac-sendshelf.pl
virtualshelves/sendshelf.pl

The default CART and LIST notices contain HTML but the "HTML message" checkbox is not checked when I edit the notice.

When viewing the list of notices, I see this:

+─────────+───────+────────────+
| module  | Code  | Name       |
+─────────+───────+────────────+
| catalog | CART  | Send cart  |
| catalog | LIST  | Send list  |
+─────────+───────+────────────+

But when I edit either of those notices "Acquisitions" is selected as the module. This isn't the case with other notices.

Created attachment 134304 [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>

Created attachment 134305 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 134306 [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.

Created attachment 134307 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Created attachment 134308 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Hi Aleisha.

The same as Christian in comment #81, nothing is showing up for me in the message_queue table (using koha-testing-docker) as per step 7.

Email address set for the account used (koha in this case) and I also added a valid email address for KohaAdminEmailAddress.

I also tested with the sending of emails enabled,* and nothing came through (they did come through before applying the patch).

* To enable basic email sending by adding this to koha-conf.xml (user_name = gmail address, password = app password set up for your account):

<smtp_server>
    <host>smtp.gmail.com</host>
    <port>587</port>
    <timeout>5</timeout>
    <ssl_mode>STARTTLS</ssl_mode>
    <user_name>youraddress@gmail.com</user_name>
    <password>yourpassword</password>
    <debug>1</debug>
 </smtp_server>

David

I notice this in the error logs, FWIW:

[2022/05/02 16:47:55] [WARN] No catalog CART letter transported by email at /kohadevbox/koha/C4/Letters.pm line 583.
[2022/05/02 16:48:20] [WARN] No catalog LIST letter transported by email at /kohadevbox/koha/C4/Letters.pm line 583.

I do see the CART and LIST notices in the staff interface.

+-----------+------+-----------+----------------------------------+
| module    | code | name      | title                            |
+-----------+------+-----------+----------------------------------+
| catalogue | CART | Send cart | Your cart                        |
| catalogue | LIST | Send list | Your list: [% listname | html %] |
+-----------+------+-----------+----------------------------------+

Created attachment 134477 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Patch no longer applies 8-(..

git bz apply 3150

Bug 3150 - Move emails for sending cart and list contents into notices tool

134304 - Bug 3150: Move emails for sending cart and list contents to notices
134305 - Bug 3150: (follow-up) HTML filtering TT notices and removing old files
134306 - Bug 3150: (follow-up) Reformat notices, don't send if no sender email
134307 - Bug 3150: (follow-up) Add missing bracket
134477 - Bug 3150: (follow-up) Fixing errors and notices

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
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/en/mandatory/sample_notices.yml
CONFLICT (content): Merge conflict in 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

I tried to fix the conflict, but then I get:

git bz apply --continue
Applying: Bug 3150: Move emails for sending cart and list contents to notices
Applying: Bug 3150: (follow-up) HTML filtering TT notices and removing old files
Applying: Bug 3150: (follow-up) Reformat notices, don't send if no sender email
Applying: Bug 3150: (follow-up) Add missing bracket
Applying: Bug 3150: (follow-up) Fixing errors and notices
error: sha1 information is lacking or useless (installer/data/mysql/en/mandatory/sample_notices.yml).
error: could not build fake ancestor
Patch failed at 0001 Bug 3150: (follow-up) Fixing errors and notices
hint: Use 'git am --show-current-patch=diff' to see the failed 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-3150-follow-up-Fixing-errors-and-notices-m9LiH9.patch

Aleisha, can you please have a look?

Created attachment 137690 [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>

Created attachment 137691 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 137692 [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.

Created attachment 137693 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Created attachment 137694 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Created attachment 137716 [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>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 137717 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 137718 [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.

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 137719 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 137720 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Signed-off-by: David Nind <david@davidnind.com>

Testing notes (using koha-testing-docker):

1. I ran through the test plan both before and after applying the patches.

2. After the patches are applied, remember to update the database! Otherwise, the new notices are not added, and you get a warning in the log files:
 [2022/07/14 13:01:00] [WARN] No catalogue LIST letter transported by email at /kohadevbox/koha/C4/Letters.pm line 583.
  [2022/07/14 13:07:33] [WARN] No catalogue CART letter transported by email at /kohadevbox/koha/C4/Letters.pm line 583.

3. Enable basic email sending by adding this to the instance koha-conf.xml (user_name = gmail address, password = app password set up for your account, not your normal password):

<smtp_server>
    <host>smtp.gmail.com</host>
    <port>587</port>
    <timeout>5</timeout>
    <ssl_mode>STARTTLS</ssl_mode>
    <user_name>youraddress@gmail.com</user_name>
    <password>yourapppassword</password>
    <debug>1</debug>
 </smtp_server>

4. Added an email address to koha user, added email addresses for KohaAdminEmailAddress and ReplytoDefault (not sure if this was required).

5. Before the patch was applied, steps 3 and 4 were sufficient for the lists and cart contents to be sent to the email address.

6. After the patches are applied, the messages are now put in the message queue (koha-mysql kohadev, then select * from message_queue;).

7. To send the messages so you can review them in your email client, run misc/cronjobs/process_message_queue.pl

Created attachment 137878 [details] [review]
Bug 3150: (follow-up) Fix .yml formatting

 FAIL	  yaml_valid
 YAML::XS::Load Error: The problem:      did not find expected key  was found at document: 1, line: 1748, column: 1 while parsing a block mapping at line: 23, column: 1

Patches currently don't apply and the conflicts looked a bit too intimidating for me:

Applying: Bug 3150: Move emails for sending cart and list contents to notices
Using index info to reconstruct a base tree...
M	basket/sendbasket.pl
M	installer/data/mysql/en/mandatory/sample_notices.yml
M	opac/opac-sendbasket.pl
M	opac/opac-sendshelf.pl
M	virtualshelves/sendshelf.pl
Falling back to patching base and 3-way merge...
Auto-merging virtualshelves/sendshelf.pl
CONFLICT (content): Merge conflict in virtualshelves/sendshelf.pl
Auto-merging opac/opac-sendshelf.pl
CONFLICT (content): Merge conflict in opac/opac-sendshelf.pl
Auto-merging opac/opac-sendbasket.pl
CONFLICT (content): Merge conflict in opac/opac-sendbasket.pl
Auto-merging installer/data/mysql/en/mandatory/sample_notices.yml
Auto-merging basket/sendbasket.pl
CONFLICT (content): Merge conflict in basket/sendbasket.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 3150: Move emails for sending cart and list contents to notices
hint: Use 'git am --show-current-patch=diff' to see the failed 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-3150-Move-emails-for-sending-cart-and-list-con-Oc2KWz.patch

(In reply to Katrin Fischer from comment #80)
> I think I have confused it with another thing we did a while ago as an
> anti-Spam measure (bug 24588)
> 
> I am not sure if it has to do with the from address, but there are other
> notices we send or potentially send from KohaAdminEmailAddress, like the
> cart emails.
> 
> Martin, as you pointed it out, do we need to keep (and maybe move to a
> separate report to make it consistent) or can we drop that here?

I think we can probably drop it actually.. I can't find anywhere in the wider world that recognises that header at all.

Comment on attachment 137716 [details] [review]
Bug 3150: Move emails for sending cart and list contents to notices

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

::: basket/sendbasket.pl
@@ +101,5 @@
> +        my $user_email = $patron->first_valid_email_address || C4::Context->preference('KohaAdminEmailAddress');
> +        C4::Letters::EnqueueLetter({
> +            letter => $letter,
> +            message_transport_type => 'email',
> +            borrowernumber => $patron->borrowernumber,

I'm not sure we should be setting borrowernumber here.. it's somewhat misleading as the notice will appear as being sent to this borrower under their notices.. we don't really have the directionality in the UI to clearly show if a message was inbound or outbound so I don't know that it should be tied to the user here.

Agree with Katrin.. I'm not close enough to this code to do the rebase.. not sure what's changed to cause the conflict so unpicking the diff is challenging.

Created attachment 140246 [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>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 140247 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 140248 [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.

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 140249 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 140250 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 140251 [details] [review]
Bug 3150: (follow-up) Fix .yml formatting

 FAIL	  yaml_valid
 YAML::XS::Load Error: The problem:      did not find expected key  was found at document: 1, line: 1748, column: 1 while parsing a block mapping at line: 23, column: 1

Created attachment 140252 [details] [review]
Bug 3150: (QA follow-up) Remove borrowernumber from EnqueueLetter

Created attachment 140253 [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>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 140254 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 140255 [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.

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 140256 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 140257 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 140258 [details] [review]
Bug 3150: (follow-up) Fix .yml formatting

 FAIL	  yaml_valid
 YAML::XS::Load Error: The problem:      did not find expected key  was found at document: 1, line: 1748, column: 1 while parsing a block mapping at line: 23, column: 1

Created attachment 140259 [details] [review]
Bug 3150: (QA follow-up) Remove borrowernumber from EnqueueLetter

The rebase wasn't too terrible actually..  back to you Katrin

Thanks Martin!

Looking forward to this one...

Created attachment 145229 [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>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 145230 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 145231 [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.

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 145232 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 145233 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 145234 [details] [review]
Bug 3150: (QA follow-up) Remove borrowernumber from EnqueueLetter

I'm getting an sha1 error when trying to apply the patch:

git bz apply 3150

Bug 3150 - Move emails for sending cart and list contents into notices tool

140258 - Bug 3150: (follow-up) Fix .yml formatting
145229 - Bug 3150: Move emails for sending cart and list contents to notices
145230 - Bug 3150: (follow-up) HTML filtering TT notices and removing old files
145231 - Bug 3150: (follow-up) Reformat notices, don't send if no sender email
145232 - Bug 3150: (follow-up) Add missing bracket
145233 - Bug 3150: (follow-up) Fixing errors and notices
145234 - Bug 3150: (QA follow-up) Remove borrowernumber from EnqueueLetter

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 3150: (follow-up) Fix .yml formatting
error: sha1 information is lacking or useless (installer/data/mysql/en/mandatory/sample_notices.yml).
error: could not build fake ancestor
Patch failed at 0001 Bug 3150: (follow-up) Fix .yml formatting

That patch should've been made obsolete, fixed

Created attachment 145676 [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>

Created attachment 145677 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 145678 [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.

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 145679 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 145680 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 145681 [details] [review]
Bug 3150: (QA follow-up) Remove borrowernumber from EnqueueLetter

Signed-off-by: David Nind <david@davidnind.com>

(In reply to Aleisha Amohia from comment #133)
> That patch should've been made obsolete, fixed

Thanks Aleisha!

Testing notes -- unchanged: see comment #103

Hi Aleisha,

with bug 16522 now pushed, this no longer applies. Could you pleas rebase? 

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	basket/sendbasket.pl
M	opac/opac-sendbasket.pl
M	opac/opac-sendshelf.pl
M	virtualshelves/sendshelf.pl
Falling back to patching base and 3-way merge...
Auto-merging virtualshelves/sendshelf.pl
CONFLICT (content): Merge conflict in virtualshelves/sendshelf.pl
Auto-merging opac/opac-sendshelf.pl
CONFLICT (content): Merge conflict in opac/opac-sendshelf.pl
Auto-merging opac/opac-sendbasket.pl
CONFLICT (content): Merge conflict in opac/opac-sendbasket.pl
Auto-merging basket/sendbasket.pl
CONFLICT (content): Merge conflict in basket/sendbasket.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 3150: Move emails for sending cart and list contents to notices
hint: Use 'git am --show-current-patch=diff' to see the failed 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-3150-Move-emails-for-sending-cart-and-list-con-7TAJ75.patch

Created attachment 146560 [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>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146561 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146562 [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.

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146563 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146564 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146565 [details] [review]
Bug 3150: (QA follow-up) Remove borrowernumber from EnqueueLetter

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146566 [details] [review]
Bug 3150: (follow-up) Use get_marc_contributors

Created attachment 146858 [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>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146859 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146860 [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.

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146861 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146862 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146863 [details] [review]
Bug 3150: (QA follow-up) Remove borrowernumber from EnqueueLetter

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 146864 [details] [review]
Bug 3150: (follow-up) Use get_marc_contributors

Created attachment 146865 [details] [review]
Bug 3150: Fix file permission on database update file

Created attachment 146866 [details] [review]
Bug 3150: (follow-up) Send list and cart emails immediately again

With this patch set the cart and list emails are sent via
the message_queue instead of bypassing it and being sent
immediately. This patch keeps them in message_queue, but
also sends them immediately, restoring the previous behavior.

Created attachment 146867 [details] [review]
Bug 3150: (follow-up) Improve and fix new notice templates

This fixes several issues found and improves formatting:

* Add missing line breaks <br> where required as we are now using HTML
* Add missing USE statements to show
  * library
  * location
  * Link to the OPAC (Preference was not resolved)
* Make sure there is a comma between library and location
* Make sure Author(s): only displays when there is content after
* The comment field allows multi-line comments, formatting is now preserved

I have to say, I am not sad to see a lot of this old code go away,especially the old template files. This appears as quite a code clean-up.

10 files changed, 306 insertions(+), 1083 deletions(-)

1) The database update and the sample files are both marked as "is_html", but were missing some line breaks and such. I ended up also fixing some issues with missing USE statements causing the OPAC link to not display.

--> Follow-up provided, see commit message for more details

2) Currently the mails are sent immediately, this should be kept moving forward.

--> Follow-up provided

3) The notices need to be amended to include the changes from bug 16522 to avoid a regression:

Bug 16522 - Add 773 (Host item entry) to the cart and list displays and e-mails

--> I got stuck on this one, could you please take a look, Aleisha?

Created attachment 146987 [details] [review]
Bug 3150: Include host item entries in LIST and CART notices

Incorporating fixes from Bug 16522

With the last patch I get an error in the send form:

ERROR PROCESSING TEMPLATE: undef error - ... at input text line 21.
at /kohadevbox/koha/virtualshelves/sendshelf.pl line 100.
at /usr/lib/x86_64-linux-gnu/perl-base/Carp.pm line 289

Created attachment 147423 [details] [review]
Bug 3150: (follow-up) Make subs to get host/related parts for notices

Created attachment 147868 [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>

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147869 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147870 [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.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147871 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147872 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147873 [details] [review]
Bug 3150: (QA follow-up) Remove borrowernumber from EnqueueLetter

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147874 [details] [review]
Bug 3150: (follow-up) Use get_marc_contributors

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147875 [details] [review]
Bug 3150: Fix file permission on database update file

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147876 [details] [review]
Bug 3150: (follow-up) Send list and cart emails immediately again

With this patch set the cart and list emails are sent via
the message_queue instead of bypassing it and being sent
immediately. This patch keeps them in message_queue, but
also sends them immediately, restoring the previous behavior.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147877 [details] [review]
Bug 3150: (follow-up) Improve and fix new notice templates

This fixes several issues found and improves formatting:

* Add missing line breaks <br> where required as we are now using HTML
* Add missing USE statements to show
  * library
  * location
  * Link to the OPAC (Preference was not resolved)
* Make sure there is a comma between library and location
* Make sure Author(s): only displays when there is content after
* The comment field allows multi-line comments, formatting is now preserved

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147878 [details] [review]
Bug 3150: Include host item entries in LIST and CART notices

Incorporating fixes from Bug 16522

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147879 [details] [review]
Bug 3150: (follow-up) Make subs to get host/related parts for notices

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 147880 [details] [review]
Bug 3150: (follow-up) Make sure host information displays for unlinked records

When the 773 entry is not linked to a record using $w or
Easyanalytics we still want to display the information
about the source/host item in the emails.

To test:
1. Activate the UseControlNumber system preference
2. Search for a record and make sure it has 001 set to some value.
3. Use Edit > Add child record to create an analytical record from this record.
4. Make sure 773$w was filled in and finish by adding any mandatory fields, save.
5. Add this record to your cart. Also add a 773$g with the pages or similar.
6. Create another record with 773$t and $g, but without $w.
7. Also add this record to your cart.
8. Add a few other records to the cart.
9. Host item information (In: ) should display for all entries with 773 in the record.

This also makes use of the biblio-title include to display the
different parts of the title (245$abnp), if the record is linked.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 148045 [details] [review]
Bug 3150: (follow-up) Make sure host information displays for unlinked records

When the 773 entry is not linked to a record using $w or
Easyanalytics we still want to display the information
about the source/host item in the emails.

To test:
1. Activate the UseControlNumber system preference
2. Search for a record and make sure it has 001 set to some value.
3. Use Edit > Add child record to create an analytical record from this record.
4. Make sure 773$w was filled in and finish by adding any mandatory fields, save.
5. Add this record to your cart. Also add a 773$g with the pages or similar.
6. Create another record with 773$t and $g, but without $w.
7. Also add this record to your cart.
8. Add a few other records to the cart.
9. Host item information (In: ) should display for all entries with 773 in the record.

This also makes use of the biblio-title include to display the
different parts of the title (245$abnp), if the record is linked.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Katrin's follow-up looks great so have done an additional sign-off

Created attachment 148170 [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>

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148171 [details] [review]
Bug 3150: (follow-up) HTML filtering TT notices and removing old files

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148172 [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.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148173 [details] [review]
Bug 3150: (follow-up) Add missing bracket

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148174 [details] [review]
Bug 3150: (follow-up) Fixing errors and notices

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148175 [details] [review]
Bug 3150: (QA follow-up) Remove borrowernumber from EnqueueLetter

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148176 [details] [review]
Bug 3150: (follow-up) Use get_marc_contributors

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148177 [details] [review]
Bug 3150: Fix file permission on database update file

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148178 [details] [review]
Bug 3150: (follow-up) Send list and cart emails immediately again

With this patch set the cart and list emails are sent via
the message_queue instead of bypassing it and being sent
immediately. This patch keeps them in message_queue, but
also sends them immediately, restoring the previous behavior.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148179 [details] [review]
Bug 3150: (follow-up) Improve and fix new notice templates

This fixes several issues found and improves formatting:

* Add missing line breaks <br> where required as we are now using HTML
* Add missing USE statements to show
  * library
  * location
  * Link to the OPAC (Preference was not resolved)
* Make sure there is a comma between library and location
* Make sure Author(s): only displays when there is content after
* The comment field allows multi-line comments, formatting is now preserved

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148180 [details] [review]
Bug 3150: Include host item entries in LIST and CART notices

Incorporating fixes from Bug 16522

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148181 [details] [review]
Bug 3150: (follow-up) Make subs to get host/related parts for notices

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148182 [details] [review]
Bug 3150: (follow-up) Make sure host information displays for unlinked records

When the 773 entry is not linked to a record using $w or
Easyanalytics we still want to display the information
about the source/host item in the emails.

To test:
1. Activate the UseControlNumber system preference
2. Search for a record and make sure it has 001 set to some value.
3. Use Edit > Add child record to create an analytical record from this record.
4. Make sure 773$w was filled in and finish by adding any mandatory fields, save.
5. Add this record to your cart. Also add a 773$g with the pages or similar.
6. Create another record with 773$t and $g, but without $w.
7. Also add this record to your cart.
8. Add a few other records to the cart.
9. Host item information (In: ) should display for all entries with 773 in the record.

This also makes use of the biblio-title include to display the
different parts of the title (245$abnp), if the record is linked.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148183 [details] [review]
Bug 3150: (QA follow-up) Remove unused variables

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148184 [details] [review]
Bug 3150: (QA follow-up) Don't load unused and deleted tt files

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 148185 [details] [review]
Bug 3150: (QA follow-up) Tidy scripts

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Thanks, Kyle!

Pushed to master for 23.05.

Nice work everyone, thanks!

Enhancement - not backporting to 22.11.x

Nice work everyone!

*** Bug 11443 has been marked as a duplicate of this bug. ***

This exposed intranet-facing data in opac baskets :(.  We dropped the filters from the metadata record fetching :S

I'm fixing it in whilst rebasing bug 31224.. but it's a bit upsetting that we opened up that leak.

*** Bug 36980 has been marked as a duplicate of this bug. ***