In acquisition, several templates try to display publisher code and publication year : invoice.tt, parcel.tt, transferorder.tt. Thoses pages use C4::Acquisition methods GetPendingOrders or GetInvoiceDetails. The bug is that in the SQL query of those methods, biblioitems.publishercode and biblioitems.publicationyear. In uncertainprice.pl those datas are fetch using GetBiblioData. It whould be better to fetch them in GetPendingOrders and GetInvoiceDetails.
Created attachment 22306 [details] [review] Bug 11122 - publisher code and publication year not fetched in acq orders See commit message
Patch applied cleanly, go forth and signoff
Created attachment 23227 [details] [review] Bug 11242: fix opac-MARCdetail.pl display and warnings This patch improves the public catalog MARC view display of records that contain subfields that are not defined in the framework used by the bib record. To test: (RM note: it may be easier to find or create a MARC record that contains a subfield not present in the default framework, then load it, rather than fiddle with the MARC frameworks in your test database.) BACK UP YOUR DATABASE! 1) Search for an item in OPAC 2) Open OPAC detail 3) Click 'MARC View' link 4) Look for something with multiple lines (e.g. 260 or 942) 5) In another tab, Go to staff client and log in 6) Go to the staff client URL similar to: /cgi-bin/koha/catalogue/detail.pl?biblionumber=##### page where ##### is the biblionumber. 7) Edit -> record 8) Note the framework used. 9) More -> Administration 10) MARC bibliographic frameworks 11) Click 'MARC structure' for the framework used by the item shown in OPAC. 12) Find the tag (e.g. 260 or 942) and click 'Subfields' 13) Delete of the matching subfields (e.g. 260$b) shown in the OPAC tab 14) Clear your opac error log. 15) Refresh OPAC tab. The tag subfields are split. 16) There are opac-MARCdetail.pl warnings. 17) Apply patch 18) Clear your opac error log. 19) Refresh OPAC tab. The tag subfields are no longer split strangely. 20) There are no opac-MARCdetail.pl warnings. RESTORE YOUR DATABASE. This also silences a series of warnings triggered by catching undef hash references. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Comment on attachment 23227 [details] [review] Bug 11242: fix opac-MARCdetail.pl display and warnings Error, sorry I was experimenting git-bz.
(In reply to Fridolin SOMERS from comment #0) > In acquisition, several templates try to display publisher code and > publication year : invoice.tt, parcel.tt, transferorder.tt. > Thoses pages use C4::Acquisition methods GetPendingOrders or > GetInvoiceDetails. > The bug is that in the SQL query of those methods, biblioitems.publishercode > and biblioitems.publicationyear. > In uncertainprice.pl those datas are fetch using GetBiblioData. > It whould be better to fetch them in GetPendingOrders and GetInvoiceDetails. "The bug is that in the SQL query of those methods, biblioitems.publishercode and biblioitems.publicationyear." I think your sentence was broken ;-) What's the bug exactly? Mathieu
Hi there, I haven't tested it yet, but there I think it won't work for MARC21. While UNIMARC uses publicationyear, MARC21 uses copyrightyear by default to store the publication year from 260$c.
(In reply to Katrin Fischer from comment #6) > Hi there, I haven't tested it yet, but there I think it won't work for > MARC21. While UNIMARC uses publicationyear, MARC21 uses copyrightyear by > default to store the publication year from 260$c. The structure of the 2 marcflavor is the same (date in plain text in 260/210 + date in coded form). They should be indexed the same way. Mathieu
I thnk this is a display problem here?
What I meant to say is that we save the data into 2 different fields in the database, so one field is always empty depending on the marcflavour. Hope that explains it better :)
I did not look the patch, I thought of Zebra and not of the db... I don't know well MARC21, but at a first glance, the date in 260/210 is not meant to be searchable, but displayed (as the dates in coded fields are supposed to be the same, but machine-readable). Am I wrong? Mathieu
I think we are misunderstanding each other - the publication year data will be in a different field in a MARC21 database, so I think the SQL query changed here needs to take biblio.copyrightdate into account too.
(In reply to Katrin Fischer from comment #11) > the publication year data will > be in a different field in a MARC21 database, so I think the SQL query > changed here needs to take biblio.copyrightdate into account too. Ok, but actually the copyrightdate is missing in templates. This is could be a new bug no ?
Fridolyn: I agree that changing the templates to include copyrightdate would be another bug, since the templates don't include it already. However, I think it would still be prudent to add biblio.copyrightdate to your patch, since it was already included in the past with the biblio.* select. In any case, this patch needs to be rebased, as 3.14.0 (I think) merged "SearchOrders" and "GetPendingOrders". -- Bug 11122 - publisher code and publication year not fetched in acq orders 22306 - Bug 11122 - publisher code and publication year not fetched in acq orders Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 11122 - publisher code and publication year not fetched in acq orders Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt Auto-merging acqui/uncertainprice.pl CONFLICT (content): Merge conflict in acqui/uncertainprice.pl Auto-merging C4/Acquisition.pm CONFLICT (content): Merge conflict in C4/Acquisition.pm Failed to merge in the changes. Patch failed at 0001 Bug 11122 - publisher code and publication year not fetched in acq orders 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".
Question of interest, are the changes in C4/Acquisition.pm properly tested in some test file somewhere?
Created attachment 38130 [details] [review] Bug 11122 - publisher code and publication year not fetched in acq orders In acquisition, several templates try to display publisher code and publication year : invoice.tt, parcel.tt, transferorder.tt. Thoses pages use C4::Acquisition methods GetPendingOrders or GetInvoiceDetails. The bug is that in the SQL query of those methods, biblioitems.publishercode and biblioitems.publicationyear. In uncertainprice.pl those datas are fetch using GetBiblioData. It whould be better to fetch them in GetPendingOrders and GetInvoiceDetails. This patch changes SQL queries to fetch wanted datas : aqorders.*,biblio.title,biblio.author,biblioitems.isbn,biblioitems.publishercode,biblioitems.publicationyear. GetInvoiceDetails also needs : biblio.seriestitle,biblioitems.volume. This patch also unifies the way biblio datas are displayed : <a href="link to catalog using biblionumber">[title]</a> <em>by</em> [author] – [isbn] <em>Publisher:</em> [publishercode], [publicationyear] Test plan : - Choose a biblio record containing a data in : biblio.title, biblio.author, biblioitems.isbn, biblioitems.publishercode, biblioitems.publicationyear, biblio.seriestitle, biblioitems.volume. - Create an order using this biblio. - Look at this order in pages : parcel.pl, transferorder.pl, uncertainprice.pl => You see publisher code and publication year - Look at this order in page : invoice.pl => You see publisher code, publication year, series title and volume
My question about testing changes more completely still stands (comment #14).
I am sorry, but this is not MARC agnostic - while UNIMARC uses publicationyear, MARC21 standard installations save this information in copyrightdate. Please take this into account.
(In reply to Katrin Fischer from comment #17) > I am sorry, but this is not MARC agnostic - while UNIMARC uses > publicationyear, MARC21 standard installations save this information in > copyrightdate. Please take this into account. Actually, this is agnostic. For MARC, the output should be unchanged. For UNIMARC, the publication year will be added. Are you saying that copyrightdate should be used too, so that UNIMARC and MARC21 installations will have identical output?
Created attachment 38146 [details] [review] Bug 11122: Address MARC21 vs. UNIMARC issue In comment #6 and comment #17, Katrin pointed out the discrepancy between UNIMARC (using publisheryear) vs. Other MARC installations (using copyrightdate). This was dealt with in invoice.tt already. This patch does similar logic for the other 3 template files.
> Are you saying that copyrightdate should be used too, so that UNIMARC and > MARC21 installations will have identical output? Yep.
t/db_dependent/Acquisition.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/87 subtests
Created attachment 38667 [details] [review] Bug 11122: Follow up Based on comment #21, this patch corrects the failings in the t/db_dependent/Acquisition.t tests. When two fields were added to the query, the corresponding array in the the test file was not updated to match. TEST PLAN --------- 1) Apply the first two patches 2) prove -v t/db_dependent/Acquisition.t -- fails 2 tests 3) Apply this patch 4) prove -v t/db_dependent/Acquisition.t -- passes 5) Run koha qa test tools.
Created attachment 44710 [details] [review] Bug 11122 - publisher code and publication year not fetched in acq orders In acquisition, several templates try to display publisher code and publication year : invoice.tt, parcel.tt, transferorder.tt. Thoses pages use C4::Acquisition methods GetPendingOrders or GetInvoiceDetails. The bug is that in the SQL query of those methods, biblioitems.publishercode and biblioitems.publicationyear. In uncertainprice.pl those datas are fetch using GetBiblioData. It whould be better to fetch them in GetPendingOrders and GetInvoiceDetails. This patch changes SQL queries to fetch wanted datas : aqorders.*,biblio.title,biblio.author,biblioitems.isbn,biblioitems.publishercode,biblioitems.publicationyear. GetInvoiceDetails also needs : biblio.seriestitle,biblioitems.volume. This patch also unifies the way biblio datas are displayed : <a href="link to catalog using biblionumber">[title]</a> <em>by</em> [author] – [isbn] <em>Publisher:</em> [publishercode], [publicationyear] Test plan : - Choose a biblio record containing a data in : biblio.title, biblio.author, biblioitems.isbn, biblioitems.publishercode, biblioitems.publicationyear, biblio.seriestitle, biblioitems.volume. - Create an order using this biblio. - Look at this order in pages : parcel.pl, transferorder.pl, uncertainprice.pl => You see publisher code and publication year - Look at this order in page : invoice.pl => You see publisher code, publication year, series title and volume
Created attachment 44711 [details] [review] Bug 11122: Address MARC21 vs. UNIMARC issue In comment #6 and comment #17, Katrin pointed out the discrepancy between UNIMARC (using publisheryear) vs. Other MARC installations (using copyrightdate). This was dealt with in invoice.tt already. This patch does similar logic for the other 3 template files.
Created attachment 44712 [details] [review] Bug 11122: Follow up Based on comment #21, this patch corrects the failings in the t/db_dependent/Acquisition.t tests. When two fields were added to the query, the corresponding array in the the test file was not updated to match. TEST PLAN --------- 1) Apply the first two patches 2) prove -v t/db_dependent/Acquisition.t -- fails 2 tests 3) Apply this patch 4) prove -v t/db_dependent/Acquisition.t -- passes 5) Run koha qa test tools.
Patches rebased : no real conflict
fatal: previous rebase directory .git/rebase-apply still exists but mbox given. at ./getter.pl line 196.
There is no conflict. Applies cleanly.
Comment on attachment 44712 [details] [review] Bug 11122: Follow up Bug 17562 removed the code that checked the structure of the Acquisitions table. No need for this patch.
Eyeballs fine, but seeing as a test plan written for a person who is brand new to Koha isn't available (because I am just a programmer, not a librarian), I can't test this properly.
Created attachment 62935 [details] [review] [SIGNED OFF] Bug 11122 - publisher code and publication year not fetched in acq orders In acquisition, several templates try to display publisher code and publication year : invoice.tt, parcel.tt, transferorder.tt. Thoses pages use C4::Acquisition methods GetPendingOrders or GetInvoiceDetails. The bug is that in the SQL query of those methods, biblioitems.publishercode and biblioitems.publicationyear. In uncertainprice.pl those datas are fetch using GetBiblioData. It whould be better to fetch them in GetPendingOrders and GetInvoiceDetails. This patch changes SQL queries to fetch wanted datas : aqorders.*,biblio.title,biblio.author,biblioitems.isbn,biblioitems.publishercode,biblioitems.publicationyear. GetInvoiceDetails also needs : biblio.seriestitle,biblioitems.volume. This patch also unifies the way biblio datas are displayed : <a href="link to catalog using biblionumber">[title]</a> <em>by</em> [author] – [isbn] <em>Publisher:</em> [publishercode], [publicationyear] Test plan : - Choose a biblio record containing a data in : biblio.title, biblio.author, biblioitems.isbn, biblioitems.publishercode, biblioitems.publicationyear, biblio.seriestitle, biblioitems.volume. - Create an order using this biblio. - Look at this order in pages : parcel.pl, transferorder.pl, uncertainprice.pl => You see publisher code and publication year - Look at this order in page : invoice.pl => You see publisher code, publication year, series title and volume Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 62936 [details] [review] [SIGNED OFF] Bug 11122: Address MARC21 vs. UNIMARC issue In comment #6 and comment #17, Katrin pointed out the discrepancy between UNIMARC (using publisheryear) vs. Other MARC installations (using copyrightdate). This was dealt with in invoice.tt already. This patch does similar logic for the other 3 template files. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 62937 [details] [review] Bug 11122: Follow up - Fix some display issues and typos This patch fixes the display of copyrightdate for MARC21 installations. As MARC21 already requires you to add punctuation in cataloguing, there is usually no need for punctutation in the templates. Also fixes a template variable name typo and the basket summary page. To test (all 3 patches): - Add several order lines to an order, one should be uncertain - Verify that the publisher and publication year are displayed - Check the uncertain price page - Verify that the publisher code and publication year are displayed - Fix uncertain price and close your order - Basket summary: Verify... (you know what) - Cancel one of your orders - Verify... for cancelled orders - Receive shipment - Verify... for unreceived orders - Receive order - Verify ... for received orders - Finish receiving - Verify ... on the invoice summary page
Leaving this as 'needs signoff' because of my follow-up patch. I've written up a test plan on the last patch. I am unclear about the code for transferring orders. I have tested from the basket summary and from the order receive pages, but there is never any order information displayed. Eyeballing the code it looks ok.
Created attachment 63405 [details] [review] [SIGNED-OFF] Bug 11122 - publisher code and publication year not fetched in acq orders In acquisition, several templates try to display publisher code and publication year : invoice.tt, parcel.tt, transferorder.tt. Thoses pages use C4::Acquisition methods GetPendingOrders or GetInvoiceDetails. The bug is that in the SQL query of those methods, biblioitems.publishercode and biblioitems.publicationyear. In uncertainprice.pl those datas are fetch using GetBiblioData. It whould be better to fetch them in GetPendingOrders and GetInvoiceDetails. This patch changes SQL queries to fetch wanted datas : aqorders.*,biblio.title,biblio.author,biblioitems.isbn,biblioitems.publishercode,biblioitems.publicationyear. GetInvoiceDetails also needs : biblio.seriestitle,biblioitems.volume. This patch also unifies the way biblio datas are displayed : <a href="link to catalog using biblionumber">[title]</a> <em>by</em> [author] – [isbn] <em>Publisher:</em> [publishercode], [publicationyear] Test plan : - Choose a biblio record containing a data in : biblio.title, biblio.author, biblioitems.isbn, biblioitems.publishercode, biblioitems.publicationyear, biblio.seriestitle, biblioitems.volume. - Create an order using this biblio. - Look at this order in pages : parcel.pl, transferorder.pl, uncertainprice.pl => You see publisher code and publication year - Look at this order in page : invoice.pl => You see publisher code, publication year, series title and volume Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63406 [details] [review] [SIGNED-OFF] Bug 11122: Address MARC21 vs. UNIMARC issue In comment #6 and comment #17, Katrin pointed out the discrepancy between UNIMARC (using publisheryear) vs. Other MARC installations (using copyrightdate). This was dealt with in invoice.tt already. This patch does similar logic for the other 3 template files. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63407 [details] [review] [SIGNED-OFF] Bug 11122: Follow up - Fix some display issues and typos This patch fixes the display of copyrightdate for MARC21 installations. As MARC21 already requires you to add punctuation in cataloguing, there is usually no need for punctutation in the templates. Also fixes a template variable name typo and the basket summary page. To test (all 3 patches): - Add several order lines to an order, one should be uncertain - Verify that the publisher and publication year are displayed - Check the uncertain price page - Verify that the publisher code and publication year are displayed - Fix uncertain price and close your order - Basket summary: Verify... (you know what) - Cancel one of your orders - Verify... for cancelled orders - Receive shipment - Verify... for unreceived orders - Receive order - Verify ... for received orders - Finish receiving - Verify ... on the invoice summary page Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63538 [details] [review] Bug 11122 - publisher code and publication year not fetched in acq orders In acquisition, several templates try to display publisher code and publication year : invoice.tt, parcel.tt, transferorder.tt. Thoses pages use C4::Acquisition methods GetPendingOrders or GetInvoiceDetails. The bug is that in the SQL query of those methods, biblioitems.publishercode and biblioitems.publicationyear. In uncertainprice.pl those datas are fetch using GetBiblioData. It whould be better to fetch them in GetPendingOrders and GetInvoiceDetails. This patch changes SQL queries to fetch wanted datas : aqorders.*,biblio.title,biblio.author,biblioitems.isbn,biblioitems.publishercode,biblioitems.publicationyear. GetInvoiceDetails also needs : biblio.seriestitle,biblioitems.volume. This patch also unifies the way biblio datas are displayed : <a href="link to catalog using biblionumber">[title]</a> <em>by</em> [author] – [isbn] <em>Publisher:</em> [publishercode], [publicationyear] Test plan : - Choose a biblio record containing a data in : biblio.title, biblio.author, biblioitems.isbn, biblioitems.publishercode, biblioitems.publicationyear, biblio.seriestitle, biblioitems.volume. - Create an order using this biblio. - Look at this order in pages : parcel.pl, transferorder.pl, uncertainprice.pl => You see publisher code and publication year - Look at this order in page : invoice.pl => You see publisher code, publication year, series title and volume Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 63539 [details] [review] Bug 11122: Address MARC21 vs. UNIMARC issue In comment #6 and comment #17, Katrin pointed out the discrepancy between UNIMARC (using publisheryear) vs. Other MARC installations (using copyrightdate). This was dealt with in invoice.tt already. This patch does similar logic for the other 3 template files. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 63540 [details] [review] Bug 11122: Follow up - Fix some display issues and typos This patch fixes the display of copyrightdate for MARC21 installations. As MARC21 already requires you to add punctuation in cataloguing, there is usually no need for punctutation in the templates. Also fixes a template variable name typo and the basket summary page. To test (all 3 patches): - Add several order lines to an order, one should be uncertain - Verify that the publisher and publication year are displayed - Check the uncertain price page - Verify that the publisher code and publication year are displayed - Fix uncertain price and close your order - Basket summary: Verify... (you know what) - Cancel one of your orders - Verify... for cancelled orders - Receive shipment - Verify... for unreceived orders - Receive order - Verify ... for received orders - Finish receiving - Verify ... on the invoice summary page Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to master for 17.11, thanks to everybody involved!
Pushed to 17.05.x, will be in 17.05.01
Glad to finally have this! These patches have been pushed to 16.11.x and will be in 16.11.09.