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
Created attachment 8115 [details] [review] Bug 5342: Adds a claims_count field in the serial table This patch adds 1 column in the serial table : - claims_count : number of claims for an orders You can add a <order> tag around your item fields, like this : -- Begin example <<LibrarianFirstname>> <<LibrarianSurname>> The following issues are in late: <order><<biblio.title>>, <<biblio.author>> (<<biblio.serial>>)</order> -- End example For more explication, see the discussion on the Bug 5347
Created attachment 8116 [details] [review] Bug 5342: Followup: Delete all occurrences of preview This feature is useless, apparently not used.
<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
Created attachment 8309 [details] [review] Bug 5342: Adds a claims_count field in the serial table Rebased patch
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".
Created attachment 8334 [details] [review] Bug 5342: Adds a claims_count field in the serial table Rebased patch
Created attachment 8335 [details] [review] Bug 5342: Followup: Delete all occurrences of preview Rebased patch
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
Please fix this conflict: CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt
Created attachment 8611 [details] [review] Bug 5342: Adds a claims_count field in the serial table Rebased patch
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) ?
Created attachment 8682 [details] [review] Bug 5342: Adds a claims_count field in the serial table Rebased patch
Created attachment 8683 [details] [review] Bug 5342: Followup: Delete all occurrences of preview Rebased patch
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?
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.
Created attachment 8910 [details] [review] Bug 5342: Followup : DataTables server-side processing FIX problems about email and errors management Add Datatables with server-side processing In fact, filters are broken in the master branch.
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.
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)
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.
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
Created attachment 9168 [details] [review] Bug 5342: Adds a claims_count field in the serial table Rebased patch. (In reply to comment #15) Hi Katrin, I'm a little lost on this bug :) If we don't want DataTable for now, I think this first patch can be signed-off (this rebase fix filters display and behaviour after claiming (yours notes 1/ and 2/. 3/ is normaly fixed by Bug 5347) DataTable will be a followup for this bug.
Created attachment 9169 [details] [review] Bug 5342: Followup: Delete all occurrences of preview Rebased patch
Comment on attachment 8910 [details] [review] Bug 5342: Followup : DataTables server-side processing This patch implement DataTable server-side procesing for serials/claims.pl.
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!
Created attachment 9976 [details] [review] Bug 5342: Adds a claims_count field in the serial table rebased patch
Created attachment 9977 [details] [review] Bug 5342: Followup: Delete all occurrences of preview rebased patch
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 :(
Created attachment 10211 [details] [review] Bug 5342: Adds a claims_count field in the serial table 1) Done 2) It appears it is not easy for this page. There are filters, etc. and I don't want to break something. But it's a good idea :) 3) It seems a general problem for all letters. Perhaps a new bug report is needed for this problem. 4) Arf, I forgot. I try to do that in a followup. Currently I have no time :-/ 5) Hum, It is a problem for Bug 5347 too. Bug I don't find where I can do this without introducing side effect with Bug 7001.
Created attachment 10212 [details] [review] Bug 5342: Followup: Delete all occurrences of preview rebased patch
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)
Created attachment 10245 [details] [review] Bug 5342: Followup Delete tags on letters This patch remove the tag order.
Created attachment 10258 [details] [review] Bug 5342: Followup Delete tags for letters which are not html
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).
(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.
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?
(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.
*** Bug 8681 has been marked as a duplicate of this bug. ***
Created attachment 14444 [details] [review] Bug 5342: Adds a claims_count field in the serial table This patch adds 1 column in the serial table : - claims_count : number of claims for an orders You can add a <order> tag around your item fields, like this : -- Begin example <<LibrarianFirstname>> <<LibrarianSurname>> The following issues are in late: <order><<biblio.title>>, <<biblio.author>> (<<biblio.serial>>)</order> -- End example For more explication, see the discussion on the Bug 5347
Created attachment 14445 [details] [review] Bug 5342: Followup: Delete all occurrences of preview This feature is useless, apparently not used.
Created attachment 14446 [details] [review] Bug 5342: Followup Delete tags for letters which are not html
Rebased patches
I would like to test it, but patch doesn't apply. Could you rebase it please? M. Saby
Created attachment 16217 [details] [review] Bug 5342: Adds a claims_count field in the serial table This patch adds 1 column in the serial table : - claims_count : number of claims for an orders You can add a <order> tag around your item fields, like this : -- Begin example <<LibrarianFirstname>> <<LibrarianSurname>> The following issues are in late: <order><<biblio.title>>, <<biblio.author>> (<<biblio.serial>>)</order> -- End example For more explication, see the discussion on the Bug 5347
Created attachment 16218 [details] [review] Bug 5342: Followup: Delete all occurrences of preview This feature is useless, apparently not used.
Created attachment 16219 [details] [review] Bug 5342: Followup Delete tags for letters which are not html
patch does not apply. could you rebase it? Mathieu
They apply without error here, could retest please?
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
*** Bug 8186 has been marked as a duplicate of this bug. ***
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
Mathieu, Unfortunately it seems that acquisition and serial features don't interest many people.
Does the sandboxes send mail ? Mathieu
(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.
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
Sandbox 1 is now configured to send emails.
Oh, great! So I try to tet 5342 today
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
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
(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.
Created attachment 19512 [details] [review] Bug 5342: Catch errors raise by sendmail For example, Mail::Sendmail can raise an error if no 'From' address if defined.
I still don't have a way to send mail with my VM, so please ask somebody else to test it. Mathieu
*** Bug 4217 has been marked as a duplicate of this bug. ***
<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.
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.
I am willing to test or QA it.
*** Bug 12602 has been marked as a duplicate of this bug. ***
Created attachment 31277 [details] [review] 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.
(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.
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.
Created attachment 31375 [details] [review] 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.
(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.
Created attachment 31454 [details] [review] Bug 5342: Serial claiming improvements: add a counter I've applied the patch against master 3.17.00.021 I pass the patch to "Signed Off" status. NB. I beg your pardon for I didn't realize before: At 3/ the message is: "No claims notice defined" instead of the desired one: "No claimissue notice defined"
Created attachment 31694 [details] [review] 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>
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).
Created attachment 31902 [details] [review] 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
(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.
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>
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.
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.
Patches pushed to master. Thanks Jonathan! Note: a small fix added on rebasing so it applies, please review, just in case.
*** Bug 12179 has been marked as a duplicate of this bug. ***