Bug 12567 - Catalog Statistitics wizard's publication year doesn't work for MARC21
Summary: Catalog Statistitics wizard's publication year doesn't work for MARC21
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Katrin Fischer
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-14 12:02 UTC by Katrin Fischer
Modified: 2016-06-21 21:37 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 12567: Make catalog statistics wizard publication year work for MARC21 (3.46 KB, patch)
2014-11-10 21:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
[Signed Off] Bug 12567: Make catalog statistics wizard publication year work for MARC21 (3.61 KB, patch)
2014-11-11 09:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12567: Make catalog statistics wizard publication year work for MARC21 (3.89 KB, patch)
2014-11-27 09:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2014-07-14 12:02:12 UTC
The cataloguing statistics wizard offers a filter on publication year. For a MARC21 installation this filter won't ever work, because MARC21 maps the publication year internally to copyrightdate instead of publicationyear like UNIMARC. 

To test (in a MARC21 database):
- Reports > Statistics wizards > Catalog
- Check publication year and another column
- Verify that publication year is NULL and the report is only one line
Comment 1 Katrin Fischer 2014-11-10 21:39:37 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2014-11-10 21:39:51 UTC
Please test well!
Comment 3 Martin Renvoize 2014-11-11 09:00:04 UTC
Looks good to me... though your patches are full of tabs cait ;)
Comment 4 Martin Renvoize 2014-11-11 09:04:19 UTC Comment hidden (obsolete)
Comment 5 Martin Renvoize 2014-11-11 09:04:56 UTC
Works well for me, fixed tab issues and signed off.
Comment 6 Marcel de Rooy 2014-11-17 14:19:12 UTC
The search results do not make sense to me.
Without the patch, I have one row with 0 results. OK (wrong index)
With the patch, I have a lot of years with all 0 results. (I look for 1900 to 1902 e.g. and I get years between 1807 and 1990 but all zero, including 1902 that should have two results.)

Failed QA
Comment 7 Katrin Fischer 2014-11-17 18:07:06 UTC
Hi Marcel, did you compare if the results are different than to UNIMARC?
Could you share which filters you checked for your search?
Comment 8 Katrin Fischer 2014-11-17 18:08:35 UTC
And did you compare it to a UNIMARC result set for the same filters?

I agree that something in this report is a bit weird, but I had good and working examples for both flavours in testing, I think it depends on the filters you select - if you limit the search further down, the results are weird before and after (for example a branchcode limit being in effect can make a difference)
Comment 9 Marcel de Rooy 2014-11-19 12:06:00 UTC
Will have a look soon.
Comment 10 Marcel de Rooy 2014-11-27 09:44:27 UTC
This script really needs further attention: If I do not select a field for row or column, the sql in sub calculate just crashes.
Comment 11 Marcel de Rooy 2014-11-27 09:49:24 UTC
If I am filtering on itemtype and publication year, just having two books in that specific type, I have lots of pubyears. Not related to this patch, but this code is bad.
Comment 12 Marcel de Rooy 2014-11-27 09:57:58 UTC
Created attachment 33986 [details] [review]
Bug 12567: Make catalog statistics wizard publication year work for MARC21

Publication year was useless for MARC21, as it was using the
biblioitems.publicationyear (UNIMARC) instead of biblio.copyrightdate
(MARC21).

This patch tries to fix the search options to work for both MARC
flavours.

To test:
In Reports > Statistics wizards > Catalog:
- Test the publication year search options on a UNIMARC and
  a MARC21 installation.
- Make sure search results make sense for both.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
The changes are fine with me, but this script is certainly not bug free.
Also note that all its SQL should be moved out according to our
coding guidelines. When doing so, a rewrite would be appropriate.
Comment 13 Katrin Fischer 2014-11-27 10:35:50 UTC
Hi Marcel, I think we can agree the code is bad - but I think I don't have the skill to completely fix it :(
Comment 14 Tomás Cohen Arazi 2014-12-01 13:29:10 UTC
Patch pushed to master.

Thanks Katrin!
Comment 15 Chris Cormack 2014-12-01 19:23:32 UTC
Pushed to 3.18.x will be in 3.18.01