Bug 12851

(In reply to Jonathan Druart from comment #1)

The tag <order> is used in the email's body of ACQCLAIM and ACQCLAIM-ISSUE notices at default.
But the <order> tag has not been yet defined as a markup tag for the email's body of any notice (see <http://manual.koha-community.org/3.16/en/noticesappendix.html#noticesfieldguide>).

So (1) the tag <order> might be defined, and not removed from C4/Letters.pm.
Otherwise, (2) the tag <order> might be removed from the email message body of two default notices above (sample_notices.sql).

On /cgi-bin/koha/serials/claims.pl, a row is created for any issue-number in late, even for issue-numbers of the same biblio-serial.
So "multiple" rows for the same biblio-serial might be shown in the table at the bottom of the page.
Without <order> in the pattern, for all the rows koha writes a corresponding line in the message body of the email notice for serial claims. If the notice would use a pattern like f.i. the following simple one:
------------------
 the following issues are in late:
-Issue <<serial.serialseq>> for serial: <<biblio.title>> / <<biblio.author>>
------------------
all the issues in late could appear clearly in the mail, even in the "multiple" case.

<<biblio.serial>> should be always 1 for a serial in subscription: it would be simply repeated in the mail, and its meaning would not be clear to the recipient.

Whaou, I was sure the order tag was necessary!
Katrin, did you know it works without the order tag??

I think I made a note somewhere that I saw that too - but I lost track where and when :(

Maybe it is necessary in some circumstances? For example if you put the order fields on multiple lines?

(In reply to Katrin Fischer from comment #5)
> Maybe it is necessary in some circumstances? For example if you put the
> order fields on multiple lines?

Yes, I think it does not work for multiple lines.

It would be great if someone could test this.

Hi all,

in fact now the tag '<order>' is not used by Koha.

We need to delete it, but not with perl code.
In my opinion the best option is to delete it in sql files
(sample_notices.sql).

So the patch do the work but not in the good place.

Zeno, are you sure it's not used? I thought it might be used to have a multi-line order layout (similar to items) - but haven't had the time to check yet.

<order>
Title
Price
Note...
</order>

(In reply to Zeno Tajoli from comment #8)
> Hi all,

Hi Zeno,

> in fact now the tag '<order>' is not used by Koha.

Why do you say that? It's still in used.

> We need to delete it, but not with perl code.
> In my opinion the best option is to delete it in sql files
> (sample_notices.sql).

We need it to know we have to loop on orders.

> So the patch do the work but not in the good place.

I didn't find a better place.

Dear Johnatyhan and Katrin,
if you see here: http://manual.koha-community.org/3.16/en/noticesappendix.html#noticesfieldguide

you can see that in Notices and slips only those 'special' tags are supported:
<<items.content>> or <item> </item>
<checkedout> </checkedout>
<overdue> </overdue> 
<news> </news>
<checkedout> </checkedout> 

Those tags needs special routines in the code because the generic routine (C4:: Letters::GetPreparedLetter) works only with name of tables.
And the name of table for order is 'aqorders', not 'orders'.

As I know, '<oder>' and '</order>' are only strings without meanning now in Koha.

So the best option is to delete the tags in sample_notices.sql

In comment 2 Paola Rossi found that ACQCLAIM and ACQCLAIM-ISSUE extrat all data that we need without '<oder>' and '</order>', also for multiple situations

(In reply to Zeno Tajoli from comment #11)
> Dear Johnatyhan and Katrin,

Simply Jonathan :)

> In comment 2 Paola Rossi found that ACQCLAIM and ACQCLAIM-ISSUE extrat all
> data that we need without '<oder>' and '</order>', also for multiple
> situations

I am really not sure about that. How to know when the repeatable element is ended?

Ok, this patch looks to be the way to do.

Hi, Jonathan, Katrin and Zeno.

In my tests, one subscription about a serial ("IJoC") have been opened to the vendor "vendor12851".
[My own email was set as the mailing address ("Email") of the vendor12851]
Two issues (Vol.2014 No.11 and Suppl.11) of the subscription were in late, as I could see in the CSV file, selecting "Downloading selected claims":
--------------------
SUPPLIER,TITLE,ISSUE NUMBER,LATE SINCE 			
vendor12851,"International journal on Communication.","Vol. 2014, No. 11",11/06/2014
vendor12851,"International journal on Communication.","Suppl 11",11/06/2014
--------------------

A notice (of module "claimissues") was cloned to CPL. The cloned notice was:
From <<borrowers.firstname>> <<borrowers.surname>> of  <<branches.branchname>> Library:
Dear <<aqbooksellers.name>>
The following issues are in late:
<order>Issue <<serial.serialseq>> for serial: <<biblio.title>> has not been received.</order>
--------------------------------------------
--------------------------------------------
I've applied the patch against master 3.17.00.046 HEAD 10582

I claimed the 2 late issues on Serials > Claims, by selecting "Send notification". The results in the received email were: 

From   of  Centerville Library:
Dear vendor12851
The following issues are in late:
Issue  for serial: International journal on Communication has not been received.
Issue  for serial: International journal on Communication has not been received.
- - - -
On the contrary, on master 3.17.00.046 HEAD 10582 I received this mail:

From   of  Centerville Library:
Dear vendor12851
The following issues are in late:
<order>Issue Vol. 2014, No. 11 for serial: International journal on Communication. has not been received.</order>
From   of  Centerville Library:
Dear vendor12851
The following issues are in late:
<order>Issue Suppl 11 for serial: International journal on Communication. has not been received.</order>
- - - -
==============================================================================
The tag <order> ws not show anymore, as required, but the Serial.serialseq field was not shown anymore against master.
I think this is an error. 
It occurred the same claiming for one issue only.
And so I pass the patch to "Failed QA" status.

Otherwise, if the noted difference is not a problem [or not related to this bug], I invite you to pass tha patch back to "Needs Sign Off", and I'll sign off the patch, thanks.

------------------------------------------------------------------------------
To test 12851 I saw at the bug 5342's test plan as indicated. I'm available for further tests on ACQ (for example about the ACQCLAIM' notice, which is used by ACQ > Late orders, and has a "<order>"'s tag inside) and further cases of "multi-lines" orders-to-claim (a 2 books' order to claim to a vendor on ACQ). 
I thought to these 2 cases of "multi-lines" orders-to-claim to a vendor: 
- 2 late issues (1case already tested), 
- 2 late books (2case). 
Please suggest me any possible interesting other ones. 
Or other cases of <<fields>> to test inside the notices, thanks.

I am not confident with the last patch. I have a fix for the issue you raised, but it will certainly introduce regression somewhere else.
I would prefer to keep the first patch, even if it is not the best way to do, it is the one which will induce the fewer side-effects.

Hi Jonathan, is the 'active' patch the one you wanted to have tested?

(In reply to Katrin Fischer from comment #17)
> Hi Jonathan, is the 'active' patch the one you wanted to have tested?

Yes, it is the only one to fix correctly the issue without introducing any side-effect.

Created attachment 33770 [details] [review]
[PASSED QA] Bug 12851: order tags should be removed from the claiming letter

If you use a claimissue notice to claim serials, the generated letter
will be

<order>Title1, Author1</order>
<order>Title2, Author2</order>
...
<order>TitleN, AuthorN</order>

This patch geds rid of these tags.

Test plan:
1/ Create a claimissue notice with something like:
  <<LibrarianFirstname>>
  <<LibrarianSurname>>
  The following issues are in late:
  <order><<biblio.title>>, <<biblio.author>> (<<biblio.serial>>)</order>

2/ Generated late serial issues.
3/ Send notifications to vendor.
4/ The order tags should not exist anymore in the sent email.

You can see bug 5342 for a more detailled test plan.

Note for QA: This should have been done in GetPreparedLetter, but I did
not find a better way to do.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described. Tested having the <order> tags on one line
and also for a multi-line layout.

Created attachment 33924 [details] [review]
Bug 12851: (QA followup) tests should not expect the <order> tag

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Patch pushed to master.

Thanks Jonathan!