Bug 12860 - Tools > Data export : "Don't export fields" option doesn't work
Summary: Tools > Data export : "Don't export fields" option doesn't work
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P3 normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 10996
Blocks:
  Show dependency treegraph
 
Reported: 2014-09-02 19:05 UTC by Francois Charbonnier
Modified: 2015-12-03 22:02 UTC (History)
5 users (show)

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


Attachments
Bug 12860: the export tools explodes if a field is not exported (1.20 KB, patch)
2015-01-29 15:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12860: the export tools explodes if a field is not exported (1.29 KB, patch)
2015-01-29 22:04 UTC, Robin Sheat
Details | Diff | Splinter Review
[PASSED QA] [PASSED QA] Bug 12860: the export tools explodes if a field is not exported (1.43 KB, patch)
2015-01-30 12:39 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Francois Charbonnier 2014-09-02 19:05:53 UTC
I have just tested the data export tool on master.

The option "Don't export fields" doesn't work for a field tag (ie 650) but works for a subfield tag (ie 650a).
Comment 1 Robin Sheat 2014-10-23 01:00:33 UTC
on Bug 13040 Comment 3 I do a bit of diagnosis of this, copy pasting the relevant bits here:

Looking at the code, this can't work:

if ( $f =~ m/^(\d{3})(.)?$/ ) {
    my ( $field, $subfield ) = ( $1, $2 );

    # skip if this record doesn't have this field
    if ( defined $record->field($field) ) {
        if ( defined $subfield ) {
            my @tags = $record->field($field);
            foreach my $t (@tags) {
                $t->delete_subfields($subfield);
            }
        }
        else {
            $record->delete_fields($field);
        }
    }
}

where the 'delete_fields' line is the one causing the error. It can't take a tag like the regex pulls out, it must take a MARC::Field.
Comment 2 Jonathan Druart 2015-01-29 15:22:43 UTC Comment hidden (obsolete)
Comment 3 Robin Sheat 2015-01-29 22:04:58 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-01-30 08:08:09 UTC
(In reply to Robin Sheat from comment #3)
> Created attachment 35614 [details] [review] [review]

> Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
> 
> Export fails to asplode with this patch

Hum? What happen with this patch?
Comment 5 Robin Sheat 2015-01-30 10:08:59 UTC
(In reply to Jonathan Druart from comment #4)
> (In reply to Robin Sheat from comment #3)
> > Created attachment 35614 [details] [review] [review] [review]
> 
> > Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
> > 
> > Export fails to asplode with this patch
> 
> Hum? What happen with this patch?

Nothing, that's what I said :-)
Comment 6 Kyle M Hall 2015-01-30 12:39:21 UTC
Created attachment 35619 [details] [review]
[PASSED QA] [PASSED QA] Bug 12860: the export tools explodes if a field is not exported

To reproduce:
1/ Go on tools/export.pl
2/ Export some records and specify you don't want the fields 245 (the
whole field, do not specify a subfield).
3/ Export, BOOM
Software error:
Arguments must be MARC::Field object at /home/koha/src/tools/export.pl line 400.

Test plan:
Apply the patch and confirm the fields 245 are not exported and the
export works as expected.

Signed-off-by: Robin Sheat <robin@catalyst.net.nz>

Export fails to asplode with this patch

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Tomás Cohen Arazi 2015-02-05 18:29:49 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 8 Chris Cormack 2015-02-10 22:03:31 UTC
Pushed to 3.18.x will be in 3.18.4
Comment 9 Mason James 2015-02-26 03:34:27 UTC
(In reply to Chris Cormack from comment #8)
> Pushed to 3.18.x will be in 3.18.4

Pushed to 3.16.x, will be in 3.16.8