Bug 21272 - MARC import should warn about mis-matched branch during staged import
Summary: MARC import should warn about mis-matched branch during staged import
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Matthias Le Gac
QA Contact: Testopia
URL:
Keywords:
: 14768 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-08-26 16:35 UTC by Barton Chittenden
Modified: 2024-03-29 21:00 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds a warning message when importing staged records to show library codes in the imported file that do not match with what is configured in Koha. It also does not import these items.
Version(s) released in:


Attachments
Bug 21272: MARC import should warn about mis-matched branch and item-type fields in 952 during staged import (8.38 KB, patch)
2023-08-31 19:16 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 21272: Unit test (2.63 KB, patch)
2023-08-31 19:16 UTC, Hammat wele
Details | Diff | Splinter Review
Exported Marc file that contains items with branche code (22.03 KB, application/marc)
2023-08-31 19:39 UTC, Hammat wele
Details
Bug 21272: MARC import should warn about mis-matched branch and item-type fields in 952 during staged import (8.38 KB, patch)
2023-09-12 03:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 21272: Unit test (2.62 KB, patch)
2023-09-12 03:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 21272: (follow-up) Terminology and formatting (1.58 KB, patch)
2023-09-12 04:05 UTC, David Nind
Details | Diff | Splinter Review
Bug 21272: (follow-up) using List::MoreUtils#any to get rid of %branches and fixing GetImportItemsBranches in ImportBatch.pm (4.34 KB, patch)
2023-11-09 00:28 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 21272: Change warning message (2.10 KB, patch)
2024-03-21 16:46 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Bug 21272: Change warning message (2.11 KB, patch)
2024-03-21 18:27 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Bug 21272: Improve warning message (8.41 KB, patch)
2024-03-29 17:57 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Import file with an attached items and bad branch code on one (6.27 KB, text/plain)
2024-03-29 18:36 UTC, Esther Melander
Details
Bug 21272: Improve warning message (8.63 KB, patch)
2024-03-29 19:49 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Bug 21272: Unit test for methods GetBadBranchesImportItems and GetTitleImportRecord (5.19 KB, patch)
2024-03-29 19:58 UTC, Matthias Le Gac
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Barton Chittenden 2018-08-26 16:35:32 UTC
When importing a record that has homebranch (952$a), holding branch (952$b) or item type (952$y) populated, but not matching values in branches.branchcode or itemtypes.itemtype, koha will indicate that items were not imported (via the count of imported items), but not why.

This causes frequent confusion among new users. It would be nice to offer mappings between detected branches / item types and the branches and item types already in the system.
Comment 1 Katrin Fischer 2023-08-26 19:57:46 UTC
*** Bug 14768 has been marked as a duplicate of this bug. ***
Comment 2 Hammat wele 2023-08-31 19:16:07 UTC Comment hidden (obsolete)
Comment 3 Hammat wele 2023-08-31 19:16:10 UTC Comment hidden (obsolete)
Comment 4 Hammat wele 2023-08-31 19:39:33 UTC
Created attachment 155043 [details]
Exported Marc file that contains items with branche code
Comment 5 David Nind 2023-09-12 03:35:29 UTC
Created attachment 155523 [details] [review]
Bug 21272: MARC import should warn about mis-matched branch and item-type fields in 952 during staged import

This plugin add a warning message to show missing branches during staged import, it also ignore item with wrong banches.

To test

Before make sure backgroundjobs process is running (/misc/workers/background_jobs_worker.pl --queue long_tasks)

1- Go to Cataloging -> Stage records for import
2 - Upload a MARC file (koha.mrc) that contain items with homebranch and/or homelibraary set and is not present in your installation
3- Click on Stage for import
4- Click on View batch
    --> There is no message for the missing branches
5- Click on "Import this batch into the catalog"
    --> The importation failed
6- Apply the patch
7- Run prove -t t/db_dependent/ImportBatch.t
8- Click on "Manage imported batch"
    --> There is a warning message showing the missing branches in a link
9- Click on "Import this batch into the catalog" again
    --> The records are imported
    --> The items with wrong branches are ignored

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2023-09-12 03:35:32 UTC
Created attachment 155524 [details] [review]
Bug 21272: Unit test

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2023-09-12 03:59:14 UTC
Thanks for adding a sample MARC file!

I amended the commit messages to remove the link to bug 12532 (hope I have done this correctly!).

I will add a follow-up to fix terminology and formatting:
- In US English for the warning message: no space between the end of the warning message and the colon
- "...missing branches...": changed to "...missing libraries..." (see https://wiki.koha-community.org/wiki/Terminology#B)

For me, this did not seem to show a warning message when the item types in 952 don't match. I'm assuming these patches only cover library code mismatches.
Comment 8 David Nind 2023-09-12 04:05:56 UTC
Created attachment 155525 [details] [review]
Bug 21272: (follow-up) Terminology and formatting

Change warning message to "..missing libraries...".

Remove space before colon for the warning message.

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 Kyle M Hall 2023-10-27 10:53:55 UTC
Comment on attachment 155523 [details] [review]
Bug 21272: MARC import should warn about mis-matched branch and item-type fields in 952 during staged import

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

::: C4/ImportBatch.pm
@@ +819,5 @@
> +            my ( $holdingbranchfield, $holdingbranchsubfield ) = GetMarcFromKohaField('items.holdingbranch');
> +            my $holdingbranch = $item_marc->subfield( $holdingbranchfield, $holdingbranchsubfield );
> +
> +            my @branches = map { $_->branchcode } Koha::Libraries->search->as_list;
> +            my %branches = map { $_ => 1 } @branches;

why not use https://perldoc.perl.org/List::Util#any and get rid of %branches

@@ +1235,5 @@
> +        WHERE import_batch_id = ?
> +        ORDER BY import_items_id";
> +    my @params;
> +    push( @params, $batch_id );
> +    my $sth = $dbh->prepare_cached($query);

Should this be a cached querey?
Comment 10 Hammat wele 2023-11-09 00:28:01 UTC
Created attachment 158683 [details] [review]
Bug 21272: (follow-up) using List::MoreUtils#any to get rid of %branches and fixing GetImportItemsBranches in ImportBatch.pm
Comment 11 Katrin Fischer 2023-11-09 20:59:28 UTC
Please also fix the bug description, I think this bug only covers one half of it.
Comment 12 Kyle M Hall 2024-01-26 14:52:56 UTC
I don't feel that the language "There are some missing libraries in your installation:" is good or sound. I think the better presumption is that the record has incorrect values, rather than those values should be created to accommodate those records.

It just feels a bit wild to have a link to create new branches from the cataloging module. Are catalogers often charged with creating new branches? It looks like there is no permission check for this link either.

I this feature simply needs to report the record values that don't match up with anything already in the database.
Comment 13 Matthias Le Gac 2024-03-21 16:46:26 UTC Comment hidden (obsolete)
Comment 14 Matthias Le Gac 2024-03-21 18:27:42 UTC
Created attachment 163648 [details] [review]
Bug 21272: Change warning message

To test

Before make sure backgroundjobs process is running (/misc/workers/background_jobs_worker.pl --queue long_tasks)

1- Go to Cataloging -> Stage records for import
2 - Upload a MARC file (koha.mrc) that contain items with homebranch and/or homelibraary set and is not present in your installation
3- Click on Stage for import
4- Click on View batch
    --> There is no message for the missing branches
5- Click on "Import this batch into the catalog"
    --> The importation failed
6- Apply the patch
7- Run prove -t t/db_dependent/ImportBatch.t
8- Click on "Manage imported batch"
    --> There is a warning message showing the missing branches in a link
9- Click on "Import this batch into the catalog" again
    --> The records are imported
    --> The items with wrong branches are ignored
Comment 15 Esther Melander 2024-03-25 18:00:26 UTC
I tested this. I had 5 bibliographic records with attached items. Each bibliographic record had multiple copies/items. I edited the first item on the first bibliographic record in the import file to have an incorrect branch code. When staging, I selected the option to always add the items. After staging the file, I received the warning that an item has an incorrect branchcode (nice!). I proceeded to import the file. None of the bibliographic records imported any items. I did not see a link in the warning message?

I would have expected the one item with the incorrect branch code to be ignored. I could even expect the bibliographic record that contained this item would not have any items brought in. But I would expect the rest of the bibliographic records would import their items.

It would be even more helpful if the warning message was more specific. If this was a large import file with a few hundred items, finding the one item with the bad branch code would be quite time consuming.
Comment 16 Matthias Le Gac 2024-03-27 20:36:35 UTC
Would it be possible for you to make the file you import available? 
Because when I run the test with the file provided by Hammat, it works correctly. 
I agree with you, I'll see if I can improve the display.
Comment 17 Matthias Le Gac 2024-03-29 17:57:50 UTC Comment hidden (obsolete)
Comment 18 Matthias Le Gac 2024-03-29 17:59:59 UTC
I have to add tests for the two new methods I added in Import.pm but tell me if the warning is better ?
Comment 19 Esther Melander 2024-03-29 18:36:51 UTC
Created attachment 164147 [details]
Import file with an attached items and bad branch code on one

Here is the import file I tested with. Only one item on one bibliographic record has a bad branch code. That bibliographic record is Working effectively with legacy code, the first item has a branch code of MPAL instead of MPL.
Comment 20 Esther Melander 2024-03-29 19:12:39 UTC
I tested again and the error message is improved. It would be nice if the error message was retained in some way so that it can be referenced again after the import in the Manage Stage records for the file.

Hammat's file works and the items with bad codes are ignored and the other items are brought in.

My import file contains bibliographic records with multiple items for each. The error message was triggered correctly for the one item with a bad branch code. However, none of the items were brought in.
Comment 21 Matthias Le Gac 2024-03-29 19:49:07 UTC
Created attachment 164148 [details] [review]
Bug 21272: Improve warning message

To test

Before make sure backgroundjobs process is running (/misc/workers/background_jobs_worker.pl --queue long_tasks)

1- Go to Cataloging -> Stage records for import
2 - Upload a MARC file (koha.mrc) that contain items with homebranch and/or homelibraary set and is not present in your installation
3- Click on Stage for import
4- Click on View batch
    --> There is no message for the missing branches
5- Click on "Import this batch into the catalog"
    --> The importation failed
6- Apply the patch
7- Run prove -t t/db_dependent/ImportBatch.t
8- Click on "Manage imported batch"
    --> There is a warning message showing the missing branches
    --> There is a warning message showing the titles of the wrong items
9- Click on "Import this batch into the catalog" again
    --> The records are imported
    --> The items with wrong branches are ignored


Current status: Failed QA
Comment 22 Matthias Le Gac 2024-03-29 19:49:52 UTC
(In reply to Matthias Le Gac from comment #21)
> Created attachment 164148 [details] [review] [review]
> Bug 21272: Improve warning message
> 
> To test
> 
> Before make sure backgroundjobs process is running
> (/misc/workers/background_jobs_worker.pl --queue long_tasks)
> 
> 1- Go to Cataloging -> Stage records for import
> 2 - Upload a MARC file (koha.mrc) that contain items with homebranch and/or
> homelibraary set and is not present in your installation
> 3- Click on Stage for import
> 4- Click on View batch
>     --> There is no message for the missing branches
> 5- Click on "Import this batch into the catalog"
>     --> The importation failed
> 6- Apply the patch
> 7- Run prove -t t/db_dependent/ImportBatch.t
> 8- Click on "Manage imported batch"
>     --> There is a warning message showing the missing branches
>     --> There is a warning message showing the titles of the wrong items
> 9- Click on "Import this batch into the catalog" again
>     --> The records are imported
>     --> The items with wrong branches are ignored
> 
> 
> Current status: Failed QA

Just make tideness
Comment 23 Matthias Le Gac 2024-03-29 19:50:27 UTC
(In reply to Esther Melander from comment #20)
> I tested again and the error message is improved. It would be nice if the
> error message was retained in some way so that it can be referenced again
> after the import in the Manage Stage records for the file.
> 
> Hammat's file works and the items with bad codes are ignored and the other
> items are brought in.
> 
> My import file contains bibliographic records with multiple items for each.
> The error message was triggered correctly for the one item with a bad branch
> code. However, none of the items were brought in.

Ok I will check with your file
Comment 24 Matthias Le Gac 2024-03-29 19:58:32 UTC
Created attachment 164149 [details] [review]
Bug 21272: Unit test for methods GetBadBranchesImportItems and GetTitleImportRecord

Current status: Failed QA
Comment 25 Matthias Le Gac 2024-03-29 21:00:23 UTC
I tested with your file and the items that are good imported well and those with bad branches did not. 
For the record "Working effectively with legacy code" I have the item with homebranch "FPL" but not the item with homebranch "MPAL".