Bug 12357 - Enhancements to RIS and BibTeX exporting
Summary: Enhancements to RIS and BibTeX exporting
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-03 15:08 UTC by Kyle M Hall
Modified: 2016-12-05 21:29 UTC (History)
10 users (show)

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


Attachments
Bug 12357 [1] - Add ability to export arbitrary fields for RIS format (5.98 KB, patch)
2014-06-03 15:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [2] - Add ability to use record level itemtype for RIS export type (5.37 KB, patch)
2014-06-03 15:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [3]- Add ability to export arbitrary fields for BibTex format (6.53 KB, patch)
2014-06-03 15:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [4] - Add ability to use record level itemtype for BibTeX export type (5.69 KB, patch)
2014-06-03 15:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [4] - Add ability to use record level itemtype for BibTeX export type (5.66 KB, patch)
2014-06-03 15:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [1] - Add ability to export arbitrary fields for RIS format (5.98 KB, patch)
2014-06-26 17:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [2] - Add ability to use record level itemtype for RIS export type (5.37 KB, patch)
2014-06-26 17:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [3]- Add ability to export arbitrary fields for BibTex format (6.33 KB, patch)
2014-06-26 17:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [4] - Add ability to use record level itemtype for BibTeX export type (5.66 KB, patch)
2014-06-26 17:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [2] - Add ability to use record level itemtype for RIS export type (5.37 KB, patch)
2014-06-26 17:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [3]- Add ability to export arbitrary fields for BibTex format (6.33 KB, patch)
2014-06-26 17:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [4] - Add ability to use record level itemtype for BibTeX export type (5.66 KB, patch)
2014-06-26 17:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [1] - Add ability to export arbitrary fields for RIS format (5.98 KB, patch)
2014-09-17 14:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [2] - Add ability to use record level itemtype for RIS export type (5.37 KB, patch)
2014-09-17 14:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [3]- Add ability to export arbitrary fields for BibTex format (6.31 KB, patch)
2014-09-17 14:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [4] - Add ability to use record level itemtype for BibTeX export type (5.68 KB, patch)
2014-09-17 14:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [1] - Add ability to export arbitrary fields for RIS format (6.16 KB, patch)
2014-11-01 18:25 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 12357 [2] - Add ability to use record level itemtype for RIS export type (5.51 KB, patch)
2014-11-01 18:25 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 12357 [3]- Add ability to export arbitrary fields for BibTex format (6.40 KB, patch)
2014-11-01 18:26 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 12357 [4] - Add ability to use record level itemtype for BibTeX export type (5.79 KB, patch)
2014-11-01 18:26 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 12357 - Enhancements to RIS and BibTeX exporting (17.24 KB, patch)
2015-01-23 12:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 - Tidy Ris.pm (50.62 KB, patch)
2015-01-23 12:21 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 - Tidy Record.pm (36.65 KB, patch)
2015-01-23 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 - Enhancements to RIS and BibTeX exporting (17.24 KB, patch)
2015-02-11 12:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 - Tidy Ris.pm (52.60 KB, patch)
2015-02-11 12:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 - Tidy Record.pm (38.76 KB, patch)
2015-02-11 12:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 - Enhancements to RIS and BibTeX exporting (15.42 KB, patch)
2015-04-28 12:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357 [QA Followup] - Unit Test (1.66 KB, patch)
2015-04-28 13:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12357: Fix comma issues related to additional fields. (1.06 KB, patch)
2015-05-25 04:09 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 12357 - Following whitespace failures on koha qa test tools. (8.04 KB, patch)
2015-05-25 04:24 UTC, M. Tompsett
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 12357 - Enhancements to RIS and BibTeX exporting (15.50 KB, patch)
2015-06-02 15:54 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 12357 [QA Followup] - Unit Test (1.76 KB, patch)
2015-06-02 15:54 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 12357: Fix comma issues related to additional fields. (1.19 KB, patch)
2015-06-02 15:54 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 12357 - Following whitespace failures on koha qa test tools. (8.15 KB, patch)
2015-06-02 15:54 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 12357 - Enhancements to RIS and BibTeX exporting (15.54 KB, patch)
2015-06-03 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12357 [QA Followup] - Unit Test (1.81 KB, patch)
2015-06-03 13:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12357: Fix comma issues related to additional fields. (1.24 KB, patch)
2015-06-03 13:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12357 - Following whitespace failures on koha qa test tools. (8.20 KB, patch)
2015-06-03 13:46 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2014-06-03 15:08:25 UTC
Some libraries would like to be able to add arbitrary fields to both the RIS and BibTeX citation formats that a record can be saved as from the staff intranet and public catalog. In addition, they would like to be able to override the default record type and use Koha's itemtype as the record type for those formats as well.
Comment 1 Kyle M Hall 2014-06-03 15:49:37 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2014-06-03 15:49:40 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2014-06-03 15:49:42 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2014-06-03 15:49:44 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2014-06-03 15:52:23 UTC Comment hidden (obsolete)
Comment 6 M. Tompsett 2014-06-03 20:28:15 UTC
Just thought I'd ask how this might interact with Bug 11592.
Comment 7 Kyle M Hall 2014-06-26 17:27:27 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2014-06-26 17:27:41 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2014-06-26 17:27:44 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall 2014-06-26 17:27:46 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2014-06-26 17:31:40 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2014-06-26 17:32:02 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2014-06-26 17:32:04 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall 2014-09-17 14:13:34 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall 2014-09-17 14:13:49 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall 2014-09-17 14:13:52 UTC Comment hidden (obsolete)
Comment 17 Kyle M Hall 2014-09-17 14:13:54 UTC Comment hidden (obsolete)
Comment 18 Frédéric Demians 2014-11-01 18:25:07 UTC Comment hidden (obsolete)
Comment 19 Frédéric Demians 2014-11-01 18:25:20 UTC Comment hidden (obsolete)
Comment 20 Frédéric Demians 2014-11-01 18:26:49 UTC Comment hidden (obsolete)
Comment 21 Frédéric Demians 2014-11-01 18:26:59 UTC Comment hidden (obsolete)
Comment 22 Katrin Fischer 2014-12-27 18:12:52 UTC
I have a few concerns about this feature:

I think we have several features here on one bug report, so it's a bit hard to work through. I really like the idea of the arbitrary fields, but I am concerned about using the record level itemtype.

This seems to assume that different itemtypes are used for different type of media. I am not sure this is how all libraries use itemtypes in Koha, some differentiate by loan types. But this is just a note, my main concern is about the missing flexibility for existing installations.

If you are a long term user of Koha you would probably have to change all your itemtype codes in order to be able to make good use of the new feature. This feels quite problematic to me.

I am not sure what's the best way to resolve this, some thoughts I had while reading through it:

Instead of using a separate system preference, maybe allowing overwriting existing fields would be an option? Then you could use *ExportAdditionalFields with 942$c for the itemtype feature or even use another arbitrary field the library thinks is best suited.

Or a little easier: allow defining the MARC subfield to use for the record type instead of hardcoding it?
Comment 23 Kyle M Hall 2015-01-23 12:13:19 UTC
> Instead of using a separate system preference, maybe allowing overwriting
> existing fields would be an option? Then you could use
> *ExportAdditionalFields with 942$c for the itemtype feature or even use
> another arbitrary field the library thinks is best suited.

That is a capital idea! I will be implementing this shortly!
Comment 24 Kyle M Hall 2015-01-23 12:19:19 UTC Comment hidden (obsolete)
Comment 25 Kyle M Hall 2015-01-23 12:21:06 UTC Comment hidden (obsolete)
Comment 26 Kyle M Hall 2015-01-23 12:26:40 UTC Comment hidden (obsolete)
Comment 27 Jonathan Druart 2015-01-23 14:57:23 UTC
Kyle, How did you tidy Ris.pm?
It seems you did not used perltidy:
  $ perltidy C4/Ris.pm
  $ diff C4/Ris.pm C4/Ris.pm.tdy | wc
    214    1064   10403
Comment 28 Kyle M Hall 2015-01-26 12:12:06 UTC
(In reply to Jonathan Druart from comment #27)
> Kyle, How did you tidy Ris.pm?
> It seems you did not used perltidy:
>   $ perltidy C4/Ris.pm
>   $ diff C4/Ris.pm C4/Ris.pm.tdy | wc
>     214    1064   10403

Yes, I did indeed use perltidy.

I think this is a really good opportunity to tidy both of these modules. Only bug 12404 modifies Record.pm in any serious way, and that will be pushed to master before this one ( and I'll have to submit a fresh followup ). Ris.pm has no other patches in process that touch it at all.
Comment 29 Jonathan Druart 2015-01-26 12:24:07 UTC
(In reply to Kyle M Hall from comment #28)
> (In reply to Jonathan Druart from comment #27)
> > Kyle, How did you tidy Ris.pm?
> > It seems you did not used perltidy:
> >   $ perltidy C4/Ris.pm
> >   $ diff C4/Ris.pm C4/Ris.pm.tdy | wc
> >     214    1064   10403
> 
> Yes, I did indeed use perltidy.

Do you use a specific .perltidyrc file?
Comment 30 Kyle M Hall 2015-02-11 12:01:09 UTC
(In reply to Jonathan Druart from comment #29)
> (In reply to Kyle M Hall from comment #28)
> > (In reply to Jonathan Druart from comment #27)
> > > Kyle, How did you tidy Ris.pm?
> > > It seems you did not used perltidy:
> > >   $ perltidy C4/Ris.pm
> > >   $ diff C4/Ris.pm C4/Ris.pm.tdy | wc
> > >     214    1064   10403
> > 
> > Yes, I did indeed use perltidy.
> 
> Do you use a specific .perltidyrc file?

Here is the entiretly of my tidy rc file. I know it's deviating from the standard default, but we live in an age where monitors are a bit bigger than they used to be:

-l=120

I can re-tidy the file with pure default Tidy ( the currently accepted Koha standard iirc ) if you wish.
Comment 31 Kyle M Hall 2015-02-11 12:08:02 UTC Comment hidden (obsolete)
Comment 32 Kyle M Hall 2015-02-11 12:08:12 UTC Comment hidden (obsolete)
Comment 33 Kyle M Hall 2015-02-11 12:08:17 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2015-02-11 13:36:39 UTC
Kyle, same with these patch.

For instance:

diff C4/Ris.pm C4/Ris.pm.tdy returns:
<     elsif ( $fieldname eq "534" ) {
<         @subfields =
<           ( 'a', 'b', 'c', 'e', 'f', 'k', 'l', 'm', 'n', 'p', 't', 'x', 'z' );
<     }
<     elsif ( $fieldname eq "535" ) {
---
>     } elsif ( $fieldname eq "534" ) {
>         @subfields = ( 'a', 'b', 'c', 'e', 'f', 'k', 'l', 'm', 'n', 'p', 't', 'x', 'z' );
>     } elsif ( $fieldname eq "535" ) {


and 

898,901c811
<     my @subfields = (
<         'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'j', 'k', 'l', 'm', 'n', 'o',
<         'p', 'q', 'r', 's', 't', 'u', 'v', 'x', 'y', 'z', '2', '3', '4'
<     );
---
>     my @subfields = ( 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'x', 'y', 'z', '2', '3', '4' );


It's weird because I have
  40 --maximum-line-length=178

in my ~/.perltidyrc
But it does not seem to be take into account.
If I launch it with
  perltidy C4/Ris.pm --noprofile

I get the same file as you.

I think I have spot a problem in the file:
 275             '607', '608', '610', '615', '620', '660' . '661',
should be
 275             '607', '608', '610', '615', '620', '660' , '661',

It existed before this patch.
I won't block this patch, but maybe it could be good to have a specific bug report for tidying these files and talk about a common perltidyrc file to use.
Comment 35 Jonathan Druart 2015-03-17 10:51:57 UTC
Kyle, could you please provide tests, at least of the marc2bibtex subroutine.
And I would prefer to see the tidy job (the 2 last patches) moved to another bug report.
Moreover, the indentation changes in C4::Record::marcrecord2csv will be in conflict with bug 12404.

Marked as Failed QA.
Comment 36 Kyle M Hall 2015-04-28 12:07:12 UTC Comment hidden (obsolete)
Comment 37 Kyle M Hall 2015-04-28 13:00:06 UTC Comment hidden (obsolete)
Comment 38 Jonathan Druart 2015-05-24 14:25:32 UTC
Kyle,
The test does not pass.

#   Failed test 'testing bibtex'
#   at /home/koha/src/t/db_dependent/Record.t line 123.
#          got: '@Scholastic{testID,
#       author = {Rowling, J.K.},
#       title = {Harry potter},
#       publisher = {Scholastic},
#       year = {2001}
#       test = {Scholastic}
# }
# '
#     expected: '@Scholastic{testID,
#       author = {Rowling, J.K.},
#       title = {Harry potter},
#       publisher = {Scholastic},
#       year = {2001},
#       test = {Scholastic}
# }
# '
Comment 39 M. Tompsett 2015-05-25 04:09:18 UTC Comment hidden (obsolete)
Comment 40 M. Tompsett 2015-05-25 04:24:31 UTC Comment hidden (obsolete)
Comment 41 Bernardo Gonzalez Kriegel 2015-06-02 15:54:30 UTC Comment hidden (obsolete)
Comment 42 Bernardo Gonzalez Kriegel 2015-06-02 15:54:39 UTC Comment hidden (obsolete)
Comment 43 Bernardo Gonzalez Kriegel 2015-06-02 15:54:47 UTC Comment hidden (obsolete)
Comment 44 Bernardo Gonzalez Kriegel 2015-06-02 15:54:53 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2015-06-03 13:45:59 UTC
Created attachment 39803 [details] [review]
Bug 12357 - Enhancements to RIS and BibTeX exporting

Some libraries would like to be able to add arbitrary fields to both the
RIS and BibTeX citation formats that a record can be saved as from the
staff intranet and public catalog. In addition, they would like to be
able to override the default record type and use Koha's itemtype as the
record type for those formats as well.

Test Plan:
 1) Apply this patch
 2) Run updatedatabase.pl
 3) Add the following to the new syspref RisExportAdditionalFields:
TY: 942$c
LC: 010$a
NT: [501$a, 505$g]
 4) Find or create a record with an 010$a (lccn) field, a 501$a field,
    a 942$c field, and multiple 505$g fields.
 5) Locate the record in the catalog, choose "Save" and select RIS
 6) Inspect the downloaded file, note the replaced TY field, the LC
    field, and multiple NT fields
 7) Add the following to the new syspref BibtexExportAdditionalFields:
'@': 942$c
lccn: 010$a
notes: [501$a, 505$g]
 9) Using the previously selected record, choose "Save" and select BIBTEX
10) Inspect the downloaded file, note the lccn, the multiple note
    fields, and the new record type value

Signed-off-by: Frederic Demians <f.demians@tamil.fr>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 46 Jonathan Druart 2015-06-03 13:46:04 UTC
Created attachment 39804 [details] [review]
Bug 12357 [QA Followup] - Unit Test

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
This works with next patch

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 47 Jonathan Druart 2015-06-03 13:46:09 UTC
Created attachment 39805 [details] [review]
Bug 12357: Fix comma issues related to additional fields.

TEST PLAN
---------
1) Apply patches, except this one.
2) prove t/db_dependent/Record.t
   -- fails like comment #38
3) Apply this patch
4) prove t/db_dependent/Record.t
   -- now it passes.
5) koha qa test tools.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Make test work, koha-qa problems fixed in next patch

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 48 Jonathan Druart 2015-06-03 13:46:13 UTC
Created attachment 39806 [details] [review]
Bug 12357 - Following whitespace failures on koha qa test tools.

TEST PLAN
---------
1) Apply all patches before this.
2) run koha qa test tools
   -- whitespace failures
3) Apply this patch
4) run koha qa test tools
   -- no whitespace failures.

NOTE: More tabs were fixed than required, to also clean up a little indenting.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Koha-qa tools now happy

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 49 Jonathan Druart 2015-06-03 13:51:41 UTC
Note that now strict and warnings are enabled, the log contains a lot of 'Use of uninitialized value". But it's not introduced by this patch set.
Comment 50 Tomás Cohen Arazi 2015-09-07 17:13:59 UTC
Patches pushed to master.

Thanks Kyle!
Comment 51 M. Tompsett 2016-02-02 04:24:44 UTC
Comment on attachment 39803 [details] [review]
Bug 12357 - Enhancements to RIS and BibTeX exporting

Review of attachment 39803 [details] [review]:
-----------------------------------------------------------------

::: C4/Ris.pm
@@ +1016,4 @@
>  	## return unaltered if already utf-8
>  	return @_;
>      }
> +    elsif (my $uniout eq "t") {

This runs with warns, but where is uniout defined?!
Comment 52 Jonathan Druart 2016-02-02 09:22:23 UTC
(In reply to M. Tompsett from comment #51)
> Comment on attachment 39803 [details] [review] [review]
> Bug 12357 - Enhancements to RIS and BibTeX exporting
> 
> Review of attachment 39803 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/Ris.pm
> @@ +1016,4 @@
> >  	## return unaltered if already utf-8
> >  	return @_;
> >      }
> > +    elsif (my $uniout eq "t") {
> 
> This runs with warns, but where is uniout defined?!

On the same line...
Please open a new bug report.
Comment 53 Jonathan Druart 2016-02-02 09:27:09 UTC
Note that uniout is an argument of the original script http://sourceforge.net/p/refdb/code/HEAD/tree/refdb/trunk/scripts/marc2ris.in
Comment 54 Hugo Agud 2016-04-11 09:27:55 UTC
Good morning

We have found one potential bug on this patch.

This patch is not recognizing propertly the type of the record, if leader position 6 is 'a' it publishs as book even the position 7 is b=article


We have tried to add TY: [942$c] to force the patch to replace the TY value, but it keeps the default Book value

Koha 3.22.5 (debian jessy 8 with plack enabled)

Test:
Catalogue a record with LDR 6 and 7 'ab' --> this is an article
check the record type is article in koha leader

Download as RIS and check that type is book

Modify syspref  RisExportAdditionalFields 

TY: [337$a] (or whatever you wish)

Download as RIS
the ty is still book, it doesn't matter the value of 337$a or whatever
Comment 55 Katrin Fischer 2016-04-12 06:30:09 UTC
Hi Hugo, 
please report as a separate bug and use the 'blocks' to link to this one. This bug is already pushed - it will make it easier to work on a fix.