Bug 11936 - Consistent log message for item insert
Summary: Consistent log message for item insert
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Magnus Enger
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-13 16:13 UTC by Magnus Enger
Modified: 2018-04-24 13:21 UTC (History)
6 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 11936 - Consistent log message for item insert (2.17 KB, patch)
2018-02-22 09:48 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 11936 - Consistent log message for item insert (2.22 KB, patch)
2018-03-09 19:47 UTC, Maksim Sen
Details | Diff | Splinter Review
Bug 11936: Consistent log message for item insert (2.28 KB, patch)
2018-03-12 10:53 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11936: (QA follow-up) Consistent log message for item insert (1.12 KB, patch)
2018-03-12 10:53 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2014-03-13 16:13:21 UTC
Output from: sudo koha-shell -c "perl misc/migration_tools/bulkmarcimport.pl -b -file /some/path/koha.mrc -l /tmp/log.txt" kohadev

id;operation;status
1;insert;ok
1;insert;ok
2;insert;ok
2;insert;ok
3;insert;ok
3;insert;ok
4;insert;ok
4;insert;ok
5;insert;ok
5;insert;ok
file : /home/magnus/Nedlastinger/koha.mrc
5 MARC records done in 0.0617420673370361 seconds

Looks like every inserted record is reported twice. Or the second line is saying an item was imported, but then it might be a good idea to make that more explicit, perhaps?
Comment 1 David Gustafsson 2016-12-22 12:43:49 UTC
The second line is for an inserted item i believe. But should be reported as such. Currently looking into this script and have fixed this in my local branch.
Comment 2 Magnus Enger 2018-02-22 09:27:38 UTC
This is the code that needs to be changed: 

 527                 eval { ( $itemnumbers_ref, $more_errors ) = AddItemBatchFromMarc( $record, $biblionumber, $biblioitemnumber, '' ); };
 528                 if ( $@ ) {
 529                     warn "ERROR: Adding items to bib $biblionumber failed: $@\n";
 530                     printlog({id=>$id||$originalid||$biblionumber, op=>"insertitem",status=>"ERROR"}) if ($logfile);
 531                     # if we failed because of an exception, assume that
 532                     # the MARC columns in biblioitems were not set.
 533                     ModBiblioMarc( $record, $biblionumber, $framework );
 534                     next RECORD;
 535                 } else {
 536                     printlog({id=>$id||$originalid||$biblionumber, op=>"insert",status=>"ok"}) if ($logfile);
 537                 }

It would be clearer if line 536 had something like op=>"insert item"
Comment 3 Magnus Enger 2018-02-22 09:34:59 UTC
Or rather op=>"insertitem", to match line 530.
Comment 4 Magnus Enger 2018-02-22 09:48:52 UTC
Created attachment 72066 [details] [review]
Bug 11936 - Consistent log message for item insert

If you use bulkmarcimport.pl to import records with items it looks
like the successfull insert of the record is reported multiple time,
but the second and subsequent "ok" is really related to importing
the item(s).

This patch changes the log message on successfully inserting an item
to match the log message given when inserting an item fails.

To test, the easy way:
- Look at lines 530 and 536 of bulkmarcimport.pl, and note that the
  "op" in those two lines are different
- Apply the patch
- Look at lines 530 and 536 again, and note that the "op" is now
  identical, and that this makes sense, since they are both related
  to the same operation, specifically inserting an item

To test, the hard way
- Have some records with items
- Import the records with bulkmarcimport.pl, and make sure to specify
  the -l option, to create a log of the actions taken
- Look at the log and verify it looks something like this:
  id;operation;status
  1;insert;ok
  1;insert;ok
  2;insert;ok
  2;insert;ok
- Apply this patch and import some more records with items. The log
  should now be similar to this:
  id;operation;status
  1;insert;ok
  1;insertitem;ok
  2;insert;ok
  2;insertitem;ok
Comment 5 Maksim Sen 2018-03-09 19:47:28 UTC
Created attachment 72619 [details] [review]
Bug 11936 - Consistent log message for item insert

If you use bulkmarcimport.pl to import records with items it looks
like the successfull insert of the record is reported multiple time,
but the second and subsequent "ok" is really related to importing
the item(s).

This patch changes the log message on successfully inserting an item
to match the log message given when inserting an item fails.

To test, the easy way:
- Look at lines 530 and 536 of bulkmarcimport.pl, and note that the
  "op" in those two lines are different
- Apply the patch
- Look at lines 530 and 536 again, and note that the "op" is now
  identical, and that this makes sense, since they are both related
  to the same operation, specifically inserting an item

To test, the hard way
- Have some records with items
- Import the records with bulkmarcimport.pl, and make sure to specify
  the -l option, to create a log of the actions taken
- Look at the log and verify it looks something like this:
  id;operation;status
  1;insert;ok
  1;insert;ok
  2;insert;ok
  2;insert;ok
- Apply this patch and import some more records with items. The log
  should now be similar to this:
  id;operation;status
  1;insert;ok
  1;insertitem;ok
  2;insert;ok
  2;insertitem;ok

Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Comment 6 Julian Maurice 2018-03-12 09:50:35 UTC
There is a similar error at lines 490/496. Should it be fixed too ?
Comment 7 Magnus Enger 2018-03-12 10:26:58 UTC
(In reply to Julian Maurice from comment #6)
> There is a similar error at lines 490/496. Should it be fixed too ?

Yes!
Comment 8 Julian Maurice 2018-03-12 10:53:13 UTC
Created attachment 72641 [details] [review]
Bug 11936: Consistent log message for item insert

If you use bulkmarcimport.pl to import records with items it looks
like the successfull insert of the record is reported multiple time,
but the second and subsequent "ok" is really related to importing
the item(s).

This patch changes the log message on successfully inserting an item
to match the log message given when inserting an item fails.

To test, the easy way:
- Look at lines 530 and 536 of bulkmarcimport.pl, and note that the
  "op" in those two lines are different
- Apply the patch
- Look at lines 530 and 536 again, and note that the "op" is now
  identical, and that this makes sense, since they are both related
  to the same operation, specifically inserting an item

To test, the hard way
- Have some records with items
- Import the records with bulkmarcimport.pl, and make sure to specify
  the -l option, to create a log of the actions taken
- Look at the log and verify it looks something like this:
  id;operation;status
  1;insert;ok
  1;insert;ok
  2;insert;ok
  2;insert;ok
- Apply this patch and import some more records with items. The log
  should now be similar to this:
  id;operation;status
  1;insert;ok
  1;insertitem;ok
  2;insert;ok
  2;insertitem;ok

Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 9 Julian Maurice 2018-03-12 10:53:17 UTC
Created attachment 72642 [details] [review]
Bug 11936: (QA follow-up) Consistent log message for item insert

Fix the same error in another place

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 10 Jonathan Druart 2018-03-19 18:30:08 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 11 Nick Clemens 2018-04-24 01:34:18 UTC
Awesome work all, backported to stable for 17.11.05
Comment 12 Fridolin SOMERS 2018-04-24 13:21:09 UTC
Pushed to 17.05.x for 17.05.11