Bug 11122

Summary: Fix display of publication year/copyrightdate and publishercode on various pages in acquisitions
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: AcquisitionsAssignee: Fridolin Somers <fridolin.somers>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: chris, dcook, gitbot, jonathan.druart, joonas.kylmala, katrin.fischer, mathsabypro, mtompset, severine.queune
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 11122 - publisher code and publication year not fetched in acq orders
Bug 11242: fix opac-MARCdetail.pl display and warnings
Bug 11122 - publisher code and publication year not fetched in acq orders
Bug 11122: Address MARC21 vs. UNIMARC issue
Bug 11122: Follow up
Bug 11122 - publisher code and publication year not fetched in acq orders
Bug 11122: Address MARC21 vs. UNIMARC issue
Bug 11122: Follow up
[SIGNED OFF] Bug 11122 - publisher code and publication year not fetched in acq orders
[SIGNED OFF] Bug 11122: Address MARC21 vs. UNIMARC issue
Bug 11122: Follow up - Fix some display issues and typos
[SIGNED-OFF] Bug 11122 - publisher code and publication year not fetched in acq orders
[SIGNED-OFF] Bug 11122: Address MARC21 vs. UNIMARC issue
[SIGNED-OFF] Bug 11122: Follow up - Fix some display issues and typos
Bug 11122 - publisher code and publication year not fetched in acq orders
Bug 11122: Address MARC21 vs. UNIMARC issue
Bug 11122: Follow up - Fix some display issues and typos

Description Fridolin Somers 2013-10-23 10:01:53 UTC
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.
Comment 1 Fridolin Somers 2013-10-23 10:20:13 UTC Comment hidden (obsolete)
Comment 2 I'm just a bot 2013-10-30 07:51:57 UTC
Patch applied cleanly, go forth and signoff
Comment 3 Fridolin Somers 2013-11-29 15:29:54 UTC Comment hidden (obsolete)
Comment 4 Fridolin Somers 2013-11-29 15:31:54 UTC
Comment on attachment 23227 [details] [review]
Bug 11242: fix opac-MARCdetail.pl display and warnings

Error, sorry
I was experimenting git-bz.
Comment 5 Mathieu Saby 2013-11-29 17:34:18 UTC
(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
Comment 6 Katrin Fischer 2013-12-01 19:06:58 UTC
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.
Comment 7 Mathieu Saby 2013-12-01 19:14:57 UTC
(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
Comment 8 Katrin Fischer 2013-12-01 19:36:41 UTC
I thnk this is a display problem here?
Comment 9 Katrin Fischer 2013-12-01 19:41:07 UTC
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 :)
Comment 10 Mathieu Saby 2013-12-01 19:43:32 UTC
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
Comment 11 Katrin Fischer 2013-12-01 19:49:52 UTC
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.
Comment 12 Fridolin Somers 2013-12-04 07:53:03 UTC
(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 ?
Comment 13 David Cook 2014-01-20 04:10:56 UTC
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".
Comment 14 Mark Tompsett 2014-04-09 06:21:18 UTC
Question of interest, are the changes in C4/Acquisition.pm properly tested in some test file somewhere?
Comment 15 Mark Tompsett 2015-04-19 13:29:54 UTC Comment hidden (obsolete)
Comment 16 Mark Tompsett 2015-04-19 13:30:40 UTC
My question about testing changes more completely still stands (comment #14).
Comment 17 Katrin Fischer 2015-04-19 19:35:35 UTC
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.
Comment 18 Mark Tompsett 2015-04-20 00:13:12 UTC
(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?
Comment 19 Mark Tompsett 2015-04-20 00:32:04 UTC Comment hidden (obsolete)
Comment 20 Katrin Fischer 2015-04-20 05:04:28 UTC
> Are you saying that copyrightdate should be used too, so that UNIMARC and
> MARC21 installations will have identical output?

Yep.
Comment 21 Jonathan Druart 2015-04-29 10:33:31 UTC
t/db_dependent/Acquisition.t .. Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/87 subtests
Comment 22 Mark Tompsett 2015-04-29 14:56:30 UTC Comment hidden (obsolete)
Comment 23 Fridolin Somers 2015-11-10 15:19:56 UTC
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] &ndash; [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
Comment 24 Fridolin Somers 2015-11-10 15:20:07 UTC
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.
Comment 25 Fridolin Somers 2015-11-10 15:20:24 UTC
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.
Comment 26 Fridolin Somers 2015-11-10 15:23:04 UTC
Patches rebased : no real conflict
Comment 27 I'm just a bot 2016-03-08 18:31:23 UTC
fatal: previous rebase directory .git/rebase-apply still exists but mbox given. at ./getter.pl line 196.
Comment 28 Mark Tompsett 2016-06-02 20:08:36 UTC
There is no conflict. Applies cleanly.
Comment 29 Mark Tompsett 2016-11-16 19:46:02 UTC
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.
Comment 30 Mark Tompsett 2016-11-16 22:33:34 UTC
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.
Comment 31 Katrin Fischer 2017-05-01 23:15:53 UTC
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] &ndash; [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>
Comment 32 Katrin Fischer 2017-05-01 23:15:57 UTC
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>
Comment 33 Katrin Fischer 2017-05-01 23:16:02 UTC
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
Comment 34 Katrin Fischer 2017-05-01 23:18:54 UTC
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.
Comment 35 Josef Moravec 2017-05-12 07:08:50 UTC
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] &ndash; [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>
Comment 36 Josef Moravec 2017-05-12 07:08:56 UTC
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>
Comment 37 Josef Moravec 2017-05-12 07:09:07 UTC
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>
Comment 38 Jonathan Druart 2017-05-18 18:27:38 UTC
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] &ndash; [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>
Comment 39 Jonathan Druart 2017-05-18 18:36:52 UTC
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>
Comment 40 Jonathan Druart 2017-05-18 18:36:58 UTC
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>
Comment 41 Jonathan Druart 2017-06-05 14:56:03 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 42 Fridolin Somers 2017-06-09 13:32:58 UTC
Pushed to 17.05.x, will be in 17.05.01
Comment 43 Katrin Fischer 2017-06-11 22:40:52 UTC
Glad to finally have this!

These patches have been pushed to 16.11.x and will be in 16.11.09.