Bug 5342 - Serial claiming improvements
Summary: Serial claiming improvements
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL: http://wiki.koha-community.org/wiki/C...
Keywords:
: 4217 8186 8681 12179 12602 (view as bug list)
Depends on:
Blocks: 7912 4217 7155 7756 12852
  Show dependency treegraph
 
Reported: 2010-11-02 02:47 UTC by Paul Poulain
Modified: 2018-08-12 21:06 UTC (History)
15 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 5342: Adds a claims_count field in the serial table (12.17 KB, patch)
2012-03-09 11:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup: Delete all occurrences of preview (6.63 KB, patch)
2012-03-09 11:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Adds a claims_count field in the serial table (12.16 KB, patch)
2012-03-19 16:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Adds a claims_count field in the serial table (12.18 KB, patch)
2012-03-20 09:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup: Delete all occurrences of preview (6.63 KB, patch)
2012-03-20 09:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Adds a claims_count field in the serial table (6.66 KB, patch)
2012-03-23 16:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Adds a claims_count field in the serial table (12.18 KB, patch)
2012-03-29 11:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup: Delete all occurrences of preview (6.66 KB, patch)
2012-03-29 11:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
screenshot (36.10 KB, image/png)
2012-04-05 06:41 UTC, Katrin Fischer
Details
Bug 5342: Followup : DataTables server-side processing (32.22 KB, patch)
2012-04-05 11:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Adds a claims_count field in the serial table (14.30 KB, patch)
2012-04-13 08:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup: Delete all occurrences of preview (6.80 KB, patch)
2012-04-13 08:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Adds a claims_count field in the serial table (14.31 KB, patch)
2012-06-07 12:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup: Delete all occurrences of preview (6.80 KB, patch)
2012-06-07 12:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Adds a claims_count field in the serial table (14.33 KB, patch)
2012-06-11 14:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup: Delete all occurrences of preview (6.85 KB, patch)
2012-06-11 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup Delete tags on letters (675 bytes, patch)
2012-06-12 08:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup Delete tags for letters which are not html (744 bytes, patch)
2012-06-13 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Adds a claims_count field in the serial table (14.07 KB, patch)
2013-01-07 10:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup: Delete all occurrences of preview (6.82 KB, patch)
2013-01-07 10:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup Delete tags for letters which are not html (736 bytes, patch)
2013-01-07 10:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Adds a claims_count field in the serial table (13.81 KB, patch)
2013-03-18 10:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup: Delete all occurrences of preview (6.41 KB, patch)
2013-03-18 10:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Followup Delete tags for letters which are not html (736 bytes, patch)
2013-03-18 10:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Catch errors raise by sendmail (6.55 KB, patch)
2013-07-10 08:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Serial claiming improvements: add a counter (27.28 KB, patch)
2014-08-29 13:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Serial claiming improvements: add a counter (26.23 KB, patch)
2014-09-04 14:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: Serial claiming improvements: add a counter (26.98 KB, patch)
2014-09-08 13:18 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 5342: Serial claiming improvements: add a counter (26.45 KB, patch)
2014-09-18 09:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5342: (qa-followup) FIX some issues (2.37 KB, patch)
2014-09-26 15:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 5342: Serial claiming improvements: add a counter (26.70 KB, patch)
2014-10-10 15:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 5342: (qa-followup) FIX some issues (3.21 KB, patch)
2014-10-10 15:21 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Poulain 2010-11-02 02:47:05 UTC
we will add
a claim counter, to know how many times an issue has been claimed
a claim table, containing all the claims an issue has, with the date of the claim and the content of the claim
Comment 1 Jonathan Druart 2012-03-09 11:22:44 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2012-03-09 11:23:19 UTC Comment hidden (obsolete)
Comment 3 Alexandre Lengereau 2012-03-19 15:21:23 UTC
<h1>Something went wrong !</h1>Applying: Bug 5342: Adds a claims_count field in the serial table<br/>
Using index info to reconstruct a base tree...<br/>
Falling back to patching base and 3-way merge...<br/>
Auto-merging installer/data/mysql/kohastructure.sql<br/>
Auto-merging installer/data/mysql/updatedatabase.pl<br/>
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl<br/>
Failed to merge in the changes.<br/>
Patch failed at 0001 Bug 5342: Adds a claims_count field in the serial table<br/>
When you have resolved this problem run git am --resolved.<br/>
If you would prefer to skip this patch, instead run git am --skip.<br/>
To restore the original branch and stop patching run git am --abort.<br/>
Bug 5342 - Serial claiming improvements<br/>
<br/>
Bug 5342: Adds a claims_count field in the serial table<br/>
Apply? [yn] <br/>
Patch left in /tmp/Bug-5342-Adds-a-claimscount-field-in-the-serial-ta-b7tIaH.patch<br/>
<br/>=== translation installation ===<br/>Install templates
  Install templates 'opac
    From: /home/koha/src/koha-tmpl/opac-tmpl/prog/en/
    To  : /home/koha/src/koha-tmpl/opac-tmpl/prog/fr-FR
    With: /home/koha/src/misc/translator/po/fr-FR-i-opac-t-prog-v-3006000.po
The install seems to be successful.
  Install templates 'intranet
    From: /home/koha/src/koha-tmpl/intranet-tmpl/prog/en/
    To  : /home/koha/src/koha-tmpl/intranet-tmpl/prog/fr-FR
    With: /home/koha/src/misc/translator/po/fr-FR-i-staff-t-prog-v-3006000.po
The install seems to be successful.

If you use this system for the 1st time, you may find usefull to reach the wiki page http://wiki.koha-community.org/wiki/Sandboxes to have some information about those results, to check if everything went well & if you can test the patch properly
Comment 4 Jonathan Druart 2012-03-19 16:44:39 UTC Comment hidden (obsolete)
Comment 5 jmbroust 2012-03-20 09:11:25 UTC
tested on sandbox 1
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 5342: Followup: Delete all occurrences of preview
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
Comment 6 Jonathan Druart 2012-03-20 09:33:53 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2012-03-20 09:34:21 UTC Comment hidden (obsolete)
Comment 8 jmbroust 2012-03-20 11:36:23 UTC
tested on biblibre sandbox 1 : ok for count of claims but new bug found : the mail of claim is not sent. Logs says vendor has no mail (apache error claims.pl bookseller without email at home/koha/src/c4/letters.pm line 340
Comment 9 Marcel de Rooy 2012-03-23 15:13:53 UTC
Please fix this conflict:
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt
Comment 10 Jonathan Druart 2012-03-23 16:01:14 UTC Comment hidden (obsolete)
Comment 11 Paul Poulain 2012-03-29 10:20:31 UTC
Jonathan, i'm sorry but you must have made a mistake somewhere : the "add claims" and "followup" are the same.
If I look for obsoleted patches, I see there's another one.

You probably have obsoleded the wrong patch. Isn't :
* http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8334 (Bug 5342: Adds a claims_count field in the serial table (12.18 KB, patch) )
* http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8611 (Bug 5342: Adds a claims_count field in the serial table (6.66 KB, patch) )

correct ?
(ie = unobsolete attachment 4 and obsolete attachment 5) ?
Comment 12 Jonathan Druart 2012-03-29 11:37:45 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2012-03-29 11:38:10 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2012-04-05 06:20:16 UTC
Looking at the comments I am not sure 'signed off' is correct here - there is no note about it and the patches don't have sign off lines?
Comment 15 Katrin Fischer 2012-04-05 06:41:02 UTC
Created attachment 8900 [details]
screenshot

Some notes about this feature:

1) After sending a claim letter I get feedback (which is good), but I am also redirected to the start page and asked to select a vendor again. This is irritating, as I want to check the claim count and date. It would be nicer, if it would stay on the same page after saving.

2) There is something broken in the layout of the filters - see Title and Libray (screenshot)

3) I got the message, that the email was sent out - but I got no mail. Investigating I am not sure which email address should be used. My current branch had no email addres, and my user didn't have one. The subscription had no library associated to it. The admin address was configured

Looking at the logs I found this:
[Thu Apr 05 08:30:46 2012] [error] [client 127.0.0.1] [Thu Apr  5 08:30:46 2012] claims.pl: Use of uninitialized value in concatenation (.) or string at /home/katrin/kohaclone/C4/Letters.pm line 340., referer: http://localhost:8080/cgi-bin/koha/serials/claims.pl
[Thu Apr 05 08:30:46 2012] [error] [client 127.0.0.1] [Thu Apr  5 08:30:46 2012] claims.pl: Bookseller  without emails at /home/katrin/kohaclone/C4/Letters.pm line 340., referer: http://localhost:8080/cgi-bin/koha/serials/claims.pl

I added an email to my patron, the branch and the bookseller - but I still don't get email and the logs remain the same. The 'success message' is wrong here too.
Comment 16 Jonathan Druart 2012-04-05 11:44:08 UTC Comment hidden (obsolete)
Comment 17 Katrin Fischer 2012-04-08 10:16:48 UTC
I am linking some bugs to this which are related to serial claims and should build upon this development or are likely to be resolved (in part) by it.
Comment 18 Katrin Fischer 2012-04-08 10:58:19 UTC
I am sorry, but the last follow-up doesn't seem to work correctly. :(

Perhaps it would be better to keep the datatable part of the last follow up for after the release and try to make the interface consistent with the changes already done for late orders for now?

I think we really should fix bug 7756 before release (sending claim letters). And it would also be nice to have a similar interface for acq order claims and serials in place.

Found problems/questions:

1) Change in workflow
Before the patch the first step on the serial claim screen is selecting a vendor. A number behind the vendor name indicates the number of late issues to be claimed. After the patch, there is a big datatable with various search options, but I am not sure how all of those work. I am not against this, but it is a little confusing. For example the vendor search is a free text field now instead of a pull down.

2) Number of issues to be claimed
Before the patch I had 6 issues to be claimed for the vendor "bookstore". After the patch there is only 1 line in my table and for this line no vendor is shown. I tried searching for "bookstore" but it didn't work. Something seems to be wrong here or the logic has changed in a way I can't figure out right now.

3) Claiming issues without vendor
When I try to claim the issue without vendor, the shown message is:
no_email. It should show a real error message.

4) No mail generated, but claim count increases
Although there was an error message, the claims count and date are still set. It should only do that after a successfull claim or after confirming it's ok to do so. I can imagine people want to write letters manually or call, so perhaps it should increase even when no mail was sent, but for the moment it seems wrong to increase the count, when there was a problem with the mail on the vendor.

Failing this for 2)
Comment 19 Katrin Fischer 2012-04-08 10:59:47 UTC
Addition for 2) I figured out what the problem was - the subscription had no library set. It seems the table does not show subscriptions where no library is set. I think we can't assume people have used those field in all subscriptions as it is currently not mandatory. Table should also show late issues without a library.
Comment 20 Katrin Fischer 2012-04-08 11:03:00 UTC
5) I fixed the subscription to have a vendor and a library set. I claimed and got a success message: Email has been sent. But I didn't get any email. My system is set up to sent mail and it worked for several other mails in testing today - I have no idea, what's going wrong here.

6) Warnings in the logs
[Sun Apr 08 13:00:26 2012] [error] [client 127.0.0.1] [Sun Apr  8 13:00:26 2012] claims.pl: Use of uninitialized value in concatenation (.) or string at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/serials/tables/claims.tt line 36., referer: http://localhost:8080/cgi-bin/koha/serials/claims.pl
[Sun Apr 08 13:00:26 2012] [error] [client 127.0.0.1] [Sun Apr  8 13:00:26 2012] claims.pl: Use of uninitialized value in concatenation (.) or string at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/serials/tables/claims.tt line 36., referer: http://localhost:8080/cgi-bin/koha/serials/claims.pl
[Sun Apr 08 13:00:26 2012] [error] [client 127.0.0.1] [Sun Apr  8 13:00:26 2012] claims.pl: Use of uninitialized value in concatenation (.) or string at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/serials/tables/claims.tt line 36., referer: http://localhost:8080/cgi-bin/koha/serials/claims.pl
[Sun Apr 08 13:00:26 2012] [error] [client 127.0.0.1] [Sun Apr  8 13:00:26 2012] claims.pl: Use of uninitialized value in concatenation (.) or string at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/serials/tables/claims.tt line 36., referer: http://localhost:8080/cgi-bin/koha/serials/claims.pl
[Sun Apr 08 13:00:26 2012] [error] [client 127.0.0.1] [Sun Apr  8 13:00:26 2012] claims.pl: Use of uninitialized value in concatenation (.) or string at /home/katrin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/serials/tables/claims.tt line 36., referer: http://localhost:8080/cgi-bin/koha/serials/claims.pl
Comment 21 Jonathan Druart 2012-04-13 08:53:52 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2012-04-13 08:54:15 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2012-04-13 08:56:51 UTC
Comment on attachment 8910 [details] [review]
Bug 5342: Followup : DataTables server-side processing

This patch implement DataTable server-side procesing for serials/claims.pl.
Comment 24 Katrin Fischer 2012-04-13 08:57:58 UTC
Hi Jonathan, feeling a bit lost too, but will try to find my way out of it. I hope to do some more testing at the weekend and will start with the bugfixes on 5342. Thanks for all your work on this!
Comment 25 Jonathan Druart 2012-06-07 12:35:50 UTC Comment hidden (obsolete)
Comment 26 Jonathan Druart 2012-06-07 12:36:50 UTC Comment hidden (obsolete)
Comment 27 Katrin Fischer 2012-06-10 15:45:30 UTC
Found a small problem in the database update:

1) Database update
kohastructure:
`claimdate` date default NULL,
+  claims_count int(11) default 0,
   `routingnotes` text,

updatedatabase:
+if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
+    $dbh->do("ALTER TABLE serial ADD COLUMN claims_count INT(11) DEFAULT 0");
+    print "Upgrade to $DBversion done (Add claims_count field in serial table)\n";
+    SetVersion($DBversion);
+}
+

So the result will be a slightly different sequence of fields in the tables. I think would be better to make it the same to avoid confusion and possible problems.

2) Dates could be displayed using the TT Dates plugin

3) It would be nice if dates in the email could be formatted according to dateformat system preference (order date for example). Currently shows as: 2012-02-01

4) It would be good to update the sample notice to have the new syntax so people know how to use it. (like we did for 5347)

5) Commit message tells to use <order> tags, but when I do, it does not get replaced:

Template:
<order>
<<biblio.title>>
<<biblio.author>>
<<biblio.serial>>
<<serial.serialid>>
<<serial.subscriptionid>>
<<serial.biblionumber>>
<<serial.planneddate>>
<<serial.routingnotes>>
<<serial.claimdate>>
<<serial.serialseq>>
</order>

Generated email:
<order>
Quellensammlung zur Kulturgeschichte


24
8
12
2012-05-01

2012-04-10
Vol 2011, Issue 55
Quellensammlung zur Kulturgeschichte


25
8
12
2012-06-01

2012-04-10
Vol 2011, Issue 56
</order>

Failing QA mainly for 1 and 5. Sorry :(
Comment 28 Jonathan Druart 2012-06-11 14:05:52 UTC Comment hidden (obsolete)
Comment 29 Jonathan Druart 2012-06-11 14:07:47 UTC Comment hidden (obsolete)
Comment 30 Katrin Fischer 2012-06-11 14:21:28 UTC
I think we need to fix 5) then :( Because sending the email is the main part of this functionality and you can't claim without sending a notice (perhaps you should be able to do so, but that's outside of the scope of this bug)
Comment 31 Jonathan Druart 2012-06-12 08:35:08 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2012-06-13 12:47:13 UTC Comment hidden (obsolete)
Comment 33 Katrin Fischer 2012-06-17 17:35:49 UTC
1) <<biblio.title>>, <<biblio.author>> (<<biblio.serial>>)
will not get replaced, when inside of <order>. This is a problem if you want to claim issues for various titles at the same vendor.

2) The date columns are not sorting correctly - only noting, i think that can be fixed later when introducing datatables?

3) CSV export has supplier, title, issue number and late since. Ideally it should also show library, status, claim count and claim date.

4) I am worried, that <order> will not work for HTML notices, as you write. This needs documentation, but it would be better to fix it. Does the <item> tag also not work for HTML notices?

5) When I mark the email template as HTML, it's still sent out as text mail.

Failing QA because of 1).
Comment 34 Jonathan Druart 2012-06-18 09:59:23 UTC
(In reply to comment #33)

Hi Katrin, thank for your tests !

> 1,4 and 5)
My test seems good.
Could you test the following procedure please:
1/ Create a notice with

=== BEGIN ===
<<LibrarianFirstname>>
<<LibrarianSurname>>
<<LibrarianEmailaddress>>

<order>
<<biblio.title>> <<biblio.author>> (<<biblio.serial>>)
</order>
=== END ===

and don't select the checkbox "HTML message". (note the "order" tag could be "whatever")

2/ Claim 2 serials.

My result:

=== BEGIN ===
firstname
joubu_admin
my_email@example.org

Title1 author1 (1)
Title2 author2 (2)
=== END ===

3/ Edit the letter and mark it as "HTML message"
with :

=== BEGIN ===
<<LibrarianFirstname>>
<<LibrarianSurname>>
<<LibrarianEmailaddress>>

<ul>
<li><<biblio.title>> <<biblio.author>> (<<biblio.serial>>)</li>
</ul>
=== END ===

4/ make claims.
My result:

=== BEGIN ===
firstname
joubu_admin
my_email@example.org

<ul>
<li>Title1 author1 (1)</li>
<li>Title2 author2 (2)</li>
</ul>
=== END ===

It seems works as expected.

> 2) The date columns are not sorting correctly - only noting, i think that
> can be fixed later when introducing datatables?
Yes, later :)

> 3) CSV export has supplier, title, issue number and late since. Ideally it
> should also show library, status, claim count and claim date.
It's not the goal of this patch. It does not modify the export feature.
Comment 35 Katrin Fischer 2012-08-10 06:05:54 UTC
Hi Jonathan,

sorry that it took me so long to get back to this. I have a question about the functionality of the <order> tag.

You wrote: "(note the "order" tag could be "whatever")" What does this mean?
I could write <xyz> around my item fields and it would still get repeated correctly? 
I see that there is no tag to mark the repeating section in the HTML message you posted - how does the template know which parts to repeat for each serial that needs to be claimed?
Comment 36 Jonathan Druart 2012-08-10 07:53:32 UTC
(In reply to comment #35)
> Hi Jonathan,

Hi Katrin,

> You wrote: "(note the "order" tag could be "whatever")" What does this mean?
> I could write <xyz> around my item fields and it would still get repeated
> correctly? 
Yes

> I see that there is no tag to mark the repeating section in the HTML message
> you posted - how does the template know which parts to repeat for each
> serial that needs to be claimed?

My development has been modified a lot of time and is not the one used by our customer. First patch needed an order tag. Now, since Bug 7001, this behaviour changed and I don't know exactly how it works.

But I think the results (see comment 34) are what we want.
Comment 37 Nicole C. Engard 2012-08-27 13:08:22 UTC
*** Bug 8681 has been marked as a duplicate of this bug. ***
Comment 38 Jonathan Druart 2013-01-07 10:11:01 UTC Comment hidden (obsolete)
Comment 39 Jonathan Druart 2013-01-07 10:11:11 UTC Comment hidden (obsolete)
Comment 40 Jonathan Druart 2013-01-07 10:11:16 UTC Comment hidden (obsolete)
Comment 41 Jonathan Druart 2013-01-07 10:11:44 UTC
Rebased patches
Comment 42 mathieu saby 2013-03-17 16:09:02 UTC
I would like to test it, but patch doesn't apply. Could you rebase it please?
M. Saby
Comment 43 Jonathan Druart 2013-03-18 10:04:29 UTC Comment hidden (obsolete)
Comment 44 Jonathan Druart 2013-03-18 10:04:35 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2013-03-18 10:04:39 UTC Comment hidden (obsolete)
Comment 46 mathieu saby 2013-03-21 09:39:35 UTC
patch does not apply. could you rebase it?
Mathieu
Comment 47 Jonathan Druart 2013-03-22 08:59:56 UTC
They apply without error here, could retest please?
Comment 48 mathieu saby 2013-03-22 17:20:53 UTC
Before applying, I created a notice pattern for serial claims. I filled it with your example.
I applied the patch
I ran  updatedatabase
I checked claims :I could see "O" in the new column.
I made some claims. The number in the new colmun was incremented each time.

Pbm : my VM is not configured to send email. So I coulds not check the sent  email :( I checked message_queue but did not see anything, but maybe is it sent directly?
Do you know if I can find pending emails somewhere in ubuntu 12.10 ? I tried to look in var/log. There is a mail.err file, empty, a mail.log with references to nullmailer, and a syslog file with no references to email.


Mathieu Saby
Comment 49 Owen Leonard 2013-07-08 19:34:22 UTC
*** Bug 8186 has been marked as a duplicate of this bug. ***
Comment 50 mathieu saby 2013-07-09 14:04:14 UTC
I still cannot test it properly, because my VM don't send email, and I don't know how to fix it (and I also have other things to do...)
Anybody wants to sign off?

Mathieu
Comment 51 Jonathan Druart 2013-07-09 14:11:10 UTC
Mathieu,

Unfortunately it seems that acquisition and serial features don't interest many people.
Comment 52 mathieu saby 2013-07-09 14:15:05 UTC
Does the sandboxes send mail ?

Mathieu
Comment 53 Jonathan Druart 2013-07-09 14:24:04 UTC
(In reply to mathieu saby from comment #52)
> Does the sandboxes send mail ?
> 
> Mathieu

I don't think so, but you can easily install exim, postfix, mutt, etc.
Comment 54 mathieu saby 2013-07-09 14:29:28 UTC
I will try again to install postfix. The pbm was to set it up with my google account, but maybe I did it the wrong way...

Mathieu
Comment 55 Jonathan Druart 2013-07-09 14:36:00 UTC
Sandbox 1 is now configured to send emails.
Comment 56 mathieu saby 2013-07-09 14:36:42 UTC
Oh, great! So I try to tet 5342 today
Comment 57 mathieu saby 2013-07-09 14:49:19 UTC
There is a "+$DBversion = "3.11.00.XXX";" line in your patch. 
So I suspect it won't apply properly in the sandbox. Could you update it to 3.13 ?

Mathieu
Comment 58 mathieu saby 2013-07-09 15:53:23 UTC
I used "test" vendor, with my email adress in contact mail for this vendor.
Then I defined a template, I define 3 late issues, and I tried the claiming feature.
=> I received no mail at all

(tried 2 times, with 2 different email adress).

Are you sure the sandbox is sending mails?

Mathieu
Comment 59 Jonathan Druart 2013-07-10 08:05:31 UTC
(In reply to mathieu saby from comment #58)
> I used "test" vendor, with my email adress in contact mail for this vendor.
> Then I defined a template, I define 3 late issues, and I tried the claiming
> feature.
> => I received no mail at all
> 
> (tried 2 times, with 2 different email adress).
> 
> Are you sure the sandbox is sending mails?

Yes, it is. In fact sendmail raises an error if no address is defined for the logged in user. Maybe this error could be catch in order to display a warning message instead of a confirm message.
I will submit a followup.
Comment 60 Jonathan Druart 2013-07-10 08:06:13 UTC Comment hidden (obsolete)
Comment 61 mathieu saby 2013-11-29 20:27:39 UTC
I still don't have a way to send mail with my VM, so please ask somebody else to test it.

Mathieu
Comment 62 Katrin Fischer 2014-03-26 22:56:50 UTC
*** Bug 4217 has been marked as a duplicate of this bug. ***
Comment 63 Koha Team Lyon 3 2014-06-18 15:29:47 UTC
<h1>Something went wrong !</h1>Applying: Bug 5342: Adds a claims_count field in the serial table Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging C4/Serials.pm
CONFLICT (content): Merge conflict in C4/Serials.pm Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt
Auto-merging serials/claims.pl
CONFLICT (content): Merge conflict in serials/claims.pl Failed to merge in the changes.
Comment 64 Jonathan Druart 2014-06-18 15:59:06 UTC
2 years and half later, the patch set is completely broken, nothing works...

I have to rewrite it.

But I won't rewrite it to wait 2 years more.
Please raise your hand if you are interesting in signing off or qa it.
Comment 65 Katrin Fischer 2014-06-18 21:27:30 UTC
I am willing to test or QA it.
Comment 66 Katrin Fischer 2014-07-20 21:10:39 UTC
*** Bug 12602 has been marked as a duplicate of this bug. ***
Comment 67 Jonathan Druart 2014-08-29 13:02:07 UTC Comment hidden (obsolete)
Comment 68 Jonathan Druart 2014-08-29 13:02:57 UTC
(In reply to Jonathan Druart from comment #45)
> Created attachment 16219 [details] [review] [review]
> Bug 5342: Followup Delete tags for letters which are not html

Note this patch has been replaced by bug 12851.
Comment 69 Paola Rossi 2014-09-02 14:32:23 UTC
I've applied the patch against 3.17.00.017

At point:
4/ Verify the vendor list is correct ... You should not get any changes here.

After selecting "Download selected claims", the file had the right numbers of rows, but no data filled the 4 fields SUPPLIER,TITLE,ISSUE NUMBER,LATE SINCE.
On the contrary the master is OK.

So I pass tha patch to "Failed QA" status.

NB.1) At points 10/ and 13/ the same error occurred: after selecting "Download selected claims" no data filled the 4 fields SUPPLIER,TITLE,ISSUE NUMBER,LATE SINCE of the file. Moreover, the increasing number in "Claims count" seemed strange to me.

NB.2) The vendor can have multiple contacts details, then many different emails: one of these might be set especially for "Contacts about late issues". At point 9/ the message sent now by the patch is:
"This vendor has no email" 
But perhaps a message like this one could be used:
"This vendor has no email contacts about late issues"

NB.3) At point 10/ the patch sends the message:
"No email is configured for this account."
instead of the required:
"no email defined for your user". 
I don't know if this difference can bring any translation problem or any difficulty to the users.
Comment 70 Jonathan Druart 2014-09-04 14:42:49 UTC Comment hidden (obsolete)
Comment 71 Jonathan Druart 2014-09-04 14:44:34 UTC
(In reply to Paola Rossi from comment #69)

Hello Paola,

> At point:
> 4/ Verify the vendor list is correct ... You should not get any changes here.
> 
> After selecting "Download selected claims", the file had the right numbers
> of rows, but no data filled the 4 fields SUPPLIER,TITLE,ISSUE NUMBER,LATE
> SINCE.
> On the contrary the master is OK.

I removed to much code in GetLateOrMissingIssues. I reintroduced it in the last patch. It should be good now.

> But perhaps a message like this one could be used:
> "This vendor has no email contacts about late issues"

Yes, you are right, I modified it.

> NB.3) At point 10/ the patch sends the message:
> "No email is configured for this account."
> instead of the required:
> "no email defined for your user". 
> I don't know if this difference can bring any translation problem or any
> difficulty to the users.

Not sure what is the best. I changed it too.
Comment 72 Paola Rossi 2014-09-08 13:18:54 UTC Comment hidden (obsolete)
Comment 73 Jonathan Druart 2014-09-18 09:20:29 UTC Comment hidden (obsolete)
Comment 74 Katrin Fischer 2014-09-26 14:51:12 UTC
Hi Jonathan, 

some notes about from testing:

1) (trivial) The QA script complained about a char character and about a missing
blank line before a =cut in Serials.pm - I am attaching a small patch
to fix those. If you think they are good, feel free to squash them
into the follow up.

2) (normal) Your patch removes the ISSN column on the claims.tt page. I have
confirmed the ISSN is displayed before your patch - so I think we should
keep it.

3) (question/normal) Without the patch, a vendor won't show up in the list
of vendors on the claims page, if the number of late issues is 0. Now
the vendor is shown in the list with a (0) behind the name. I wonder
if it would be more ergonomic to not list them or if libraries want to
have those listed as well.

4) (note) When you select no issue in the list, a message is shown:
  Please select at least one item.
I think for translation it would be a bit nicer to make it more
explicit:
  Please select at least one issue.

5) (major) The tests are failing on my database :(
t/db_dependent/Serials/Claims.t ......... 2/13 
#   Failed test 'There is no late issues yet'
#   at t/db_dependent/Serials/Claims.t line 78.
#          got: '1'
#     expected: '0'

#   Failed test 'There is still no late issues yet'
#   at t/db_dependent/Serials/Claims.t line 89.
#          got: '1'
#     expected: '0'

#   Failed test '2 suppliers should have issues in late'
#   at t/db_dependent/Serials/Claims.t line 120.
#          got: '3'
#     expected: '2'
# Looks like you failed 3 tests of 13.
t/db_dependent/Serials/Claims.t ......... Dubious, test returned 3 (wstat 768, 0x300)
Failed 3/13 subtests 

6) (normal) My staff user has a primary email defined, but I still get the message:
  No email is configured for your user.
The vendor has an email defined as well and all checkboxes are checked.
I double checked - I am not sure what's going wrong here.

Failing for 2), 5) and 6).
Comment 75 Jonathan Druart 2014-09-26 15:15:24 UTC Comment hidden (obsolete)
Comment 76 Jonathan Druart 2014-09-26 15:18:33 UTC
(In reply to Katrin Fischer from comment #74)
> 1) (trivial) The QA script complained about a char character and about a
> missing
> blank line before a =cut in Serials.pm - I am attaching a small patch
> to fix those. If you think they are good, feel free to squash them
> into the follow up.

I don't reproduce :-/ I will be happy if you can provide a followup for that :)

> 2) (normal) Your patch removes the ISSN column on the claims.tt page. I have
> confirmed the ISSN is displayed before your patch - so I think we should
> keep it.

Oops, fixed!

> 3) (question/normal) Without the patch, a vendor won't show up in the list
> of vendors on the claims page, if the number of late issues is 0. Now
> the vendor is shown in the list with a (0) behind the name. I wonder
> if it would be more ergonomic to not list them or if libraries want to
> have those listed as well.

I don't have supplier with (0).
Maybe it's suppliers with serials already claimed?

> 4) (note) When you select no issue in the list, a message is shown:
>   Please select at least one item.
> I think for translation it would be a bit nicer to make it more
> explicit:
>   Please select at least one issue.

Not introduced by this patch, but fixed :)

> 5) (major) The tests are failing on my database :(

I don't reproduce neither, but it should be fixed.
I remove all issues before to launch tests in the followup.

> 6) (normal) My staff user has a primary email defined, but I still get the
> message:
>   No email is configured for your user.
> The vendor has an email defined as well and all checkboxes are checked.
> I double checked - I am not sure what's going wrong here.

You have to logout/login to take into account the email address modification.
Comment 77 Katrin Fischer 2014-10-09 22:00:30 UTC
Hi Jonathan,

sorry, I found some small issues:

1) the ISSN and Issue number column descriptions are switched
2) there are 2 tabs hiding in claims.tt according to the qa script
3) the <order> tags are not removed when generating the email, I used the suggested claim notice template:

  The following issues are in late:
<order><<biblio.title>>, <<biblio.author>> (<<biblio.serial>>)</order>

The result email for the 2 claimed issues looks like this:

  The following issues are in late:
<order>Some serial title...</order>
<order>Some serial title...</order>
Comment 78 Katrin Fischer 2014-10-10 15:19:20 UTC
Created attachment 32171 [details] [review]
[PASSED QA] Bug 5342: Serial claiming improvements: add a counter

This patch adds a new DB field serial.claims_count
This field already exists for late orders. It makes sense to introduce
it for serial.

Test plan:
0/
 a) Does not apply the patch.
 b) Remove all your claimissues notices and be sure you have some serial issues
    in late.
 c) remove email address for the vendor you will use.
 d) remove email address for the logged in user.
 e) Export claims using the csv export => The selected issues will be
 marked as claimed.
 f) logout/login (to update the email address).
1/ Apply the patch and execute the updatedb entry.
2/ Go on the Serials > Claims page
3/ Verify that you get a warning message 'No claimissue notice defined'
4/ Verify the vendor list is correct (with the number of serial in late.
You should not get any changes here.
5/ Select one vendor and verify that the issue which was claimed before
has a claim count set to 1.
6/ Verify that you are not able to send notification to the vendor.
7/ Create a claimissue notice.
Something like:
  <<LibrarianFirstname>>
  <<LibrarianSurname>>
  The following issues are in late:
  <order><<biblio.title>>, <<biblio.author>> (<<biblio.serial>>)</order>
8/ Go on the Serials > Claims page, the warning message does not appear
anymore.
9/ Select issues. Select a notice. And "Send notification".
You should get an error (no email defined for this vendor).
10/ Add an email for the vendor.
11/ Select issues. Select a notice. And "Send notification".
You should get an error (no email defined for your user).
12/ Add an email address to your user
logout/login
13/ Select issues. Select a notice. And "Send notification".
You should get a happy message: the email has been sent!
14/ The email will contain the order tags if bug 12851 is not
pushed/applied.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, some small issues fixed in a follow-up.
Note: If you change the email address of your staff user, you will
have to log out and back in to make the change take effect.
Comment 79 Katrin Fischer 2014-10-10 15:21:49 UTC
Created attachment 32172 [details] [review]
[PASSED QA] Bug 5342: (qa-followup) FIX some issues

this patch:
- reintroduces the ISSN column
- fix a wording (already there before the main patch)
- fix the tests if a serial was already in late

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Small change: Removed 2 tabs in claims.tt and fixed order
of ISSN/Issue number column descriptions as they were switched.

Note: The <order> tags are currently not stripped out of the
notice.
Comment 80 Tomás Cohen Arazi 2014-10-28 13:23:10 UTC
Patches pushed to master.

Thanks Jonathan!

Note: a small fix added on rebasing so it applies, please review, just in case.
Comment 81 Katrin Fischer 2014-11-10 22:37:44 UTC
*** Bug 12179 has been marked as a duplicate of this bug. ***