Bug 14420 - bulkmarcimport.pl - Some problems
Summary: bulkmarcimport.pl - Some problems
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Vitor Fernandes
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-19 10:22 UTC by Vitor Fernandes
Modified: 2018-10-30 00:44 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
bulkmarcimport.pl (3.49 KB, patch)
2015-06-19 10:47 UTC, Vitor Fernandes
Details | Diff | Splinter Review
Bug 14420 : bulkmarcimport.pl - Some problems (3.49 KB, patch)
2015-08-04 11:56 UTC, Vitor Fernandes
Details | Diff | Splinter Review
Bug 14420 : bulkmarcimport.pl - Some problems (3.49 KB, patch)
2015-08-04 12:14 UTC, Vitor Fernandes
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Vitor Fernandes 2015-06-19 10:22:52 UTC
PROBLEM 1

If a record as some than one ISBN, the values are recorded in database like 'isbn1 | isbn2 | isbn3'.

In the script the ISBN match are only checking for exact ISBN matching.

my $sth_isbn = $dbh->prepare("SELECT biblionumber,biblioitemnumber FROM biblioitems WHERE isbn=? OR isbn like ? OR isbn like ? OR isbn like ?");


The script should verify if the ISBN is a exact match or if it appears in start, in the middle or in the end of the database isbn column:

my $sth_isbn = $dbh->prepare("SELECT biblionumber,biblioitemnumber FROM biblioitems WHERE isbn=? OR isbn like ? OR isbn like ? OR isbn like ?");


The query execution should also be changed:

$sth_isbn->execute($isbn, $isbn." | %", "% | ".$isbn, "% | ".$isbn." | %");



PROBLEM 2

The ISBN is not being saved in the variable $isbn because of the following line:

my $isbn = $field && $field->subfield('a');

The "my" before the variable should be removed.



PROBLEM 3

The $biblionumber and $biblioitemnumber are being lost after the record modification in the case of being a duplicate record. ModBiblio returns 1 if everything works right in the record modification.

The following line code should be modified:

eval { ( $biblionumber, $biblioitemnumber ) = ModBiblio( $record, $biblionumber, GetFrameworkCode($biblionumber) ) };

The result should be:

eval { ModBiblio( $record, $biblionumber, GetFrameworkCode($biblionumber) ) };



PROBLEM 4

When modifying a record, maybe we want to append the items to the existing record.
So we should pass a clone of the record to the method ModBiblio, because that method strips all items in the record.
This should be done with a new argument.

-updateadditems

The line:

eval { ModBiblio( $record, $biblionumber, GetFrameworkCode($biblionumber) ) };

Is changed to:

my $clone = $updateadditems ? $record->clone() : $record;
eval { ModBiblio( $clone, $biblionumber, GetFrameworkCode($biblionumber) ) };
Comment 1 Vitor Fernandes 2015-06-19 10:47:42 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2015-07-09 12:20:12 UTC
Hi Vitor,
can you please add a test plan? Some sample records might also be helpful to make this easier to test.
Comment 3 Katrin Fischer 2015-07-09 12:20:59 UTC
Ah, also make sure test plan and problem description are part of your commit message!
Comment 4 Vitor Fernandes 2015-08-04 11:56:57 UTC Comment hidden (obsolete)
Comment 5 Vitor Fernandes 2015-08-04 12:14:32 UTC
Created attachment 41339 [details] [review]
Bug 14420 : bulkmarcimport.pl - Some problems

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14420

Test plan:

- Add one record with two ISBN using bulkmarcimport
- Add the same record using bulkmarcimport with ISBN comparing
- The comparing of ISBN isn't working because the bulkmarcimport are trying to check if isbn1 is in the column isbn of biblio table (check if equal). The column isbn of the first record is "isbn1 | isbn2".

- Add one record with one ISBN using bulkmarcimport
- Add the same record using bulkmarcimport and ISBN comparing
- The first record will be updated but the items of the second record will not be added to the first one

- Apply the patch

- Add one record with two ISBN using bulkmarcimport
- Add the same record using bulkmarcimport with ISBN comparing
- The comparing of ISBN will work because the bulkmarcimport will use "like" comparing instead of "equal" 

- Add one record with one ISBN using bulkmarcimport
- Add the same record using bulkmarcimport with ISBN comparing and updateadditems
- The first record will be updated and the items of the second record will be added to the first one

Created by: Vitor Fernandes <vfernandes@keep.pt>
Sponsored by: KEEP SOLUTIONS
Comment 6 Mark Tompsett 2018-04-05 03:13:06 UTC
(In reply to Vitor Fernandes from comment #0)
> PROBLEM 1
> 
> If a record as some than one ISBN, the values are recorded in database like
> 'isbn1 | isbn2 | isbn3'.
> 
> In the script the ISBN match are only checking for exact ISBN matching.

This is because:
020 - International Standard Book Number (R)
-- The 020 field is repeatable. This is clearly bad data, but yes, we should
attempt to parse for it.

I don't like the multiple likes fix provided here.
I would prefer a:
my @isbns = parseIsbnField($isbn_field_value);
foreach my $isbn (@isbns) {
    search for the individual isbn
    ...
}

sub parseIsbnField {
    my ($isbn_data) = @_;
    see if we can split out some ISBNs. ...
    return an array of the results.
}
Comment 7 Alex Buckley 2018-10-30 00:44:15 UTC
Can you please rebase your patch it is currently not applying cleanly on master, as the below output shows:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 14420 : bulkmarcimport.pl - Some problems
Using index info to reconstruct a base tree...
M	misc/migration_tools/bulkmarcimport.pl
Falling back to patching base and 3-way merge...
Auto-merging misc/migration_tools/bulkmarcimport.pl
CONFLICT (content): Merge conflict in misc/migration_tools/bulkmarcimport.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 14420 : bulkmarcimport.pl - Some problems
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
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".
Patch left in /tmp/Bug-14420--bulkmarcimportpl---Some-problems-T8e2dA.patch
You have new mail in /var/mail/vagrant