Bug 2060

Summary: Ability to import Authorities from Staff Client
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: MARC Authority data supportAssignee: Jared Camins-Esakov <jcamins>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P3 CC: brendan, chris, dpk, elliott, katrin.fischer, kyle, m.de.rooy, mcoalwell, paul.poulain, smoreland, steven_jeffery
Version: 3.10   
Hardware: PC   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13296
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 7475, 8212    
Attachments: Bug 7475: Update configuration
Bug 7475: Teach matching rules to handle authorities
Bug 2060: Update command line MARC import scripts
Bug 8063 - Followup - Bug fix
Bug 7475: Update configuration
Bug 7475: Teach matching rules to handle authorities
Bug 2060: Update command line MARC import scripts
Bug 7475: Update configuration
Bug 7475: Teach matching rules to handle authorities
Bug 2060: Update command line MARC import scripts
Bug 7475: Update configuration
Bug 7475: Teach matching rules to handle authorities
Bug 2060: Update command line MARC import scripts
screenshot for my previous comment
unimarc authority used for testing
UNIMARC authorities
screenshot of my testing with UNIMARC (and Plack)
Bug 2060 follow-up: add support for UNIMARCAUTH
Bug 7475: Teach matching rules to handle authorities
Bug 2060: Update command line MARC import scripts
Bug 2060 follow-up: add support for UNIMARCAUTH

Description Chris Cormack 2010-05-21 00:37:45 UTC


---- Reported by nengard@gmail.com 2008-04-28 08:16:24 ----

<nengard> is there a way to import an authorities file into Koha? Or do you have to add all the records one by one?
 * fbcit-away is now known as fbcit
 <fbcit> nengard: yes to the first, holdon
 <nengard> thanks fbcit
 <fbcit> there is a script, misc/migration_tools/bulkauthimport.pl
 <fbcit> you can import 1 or more records with it
 <nengard> fbcit is there a way to access that for the average user? a link in the admin menu anywhere? Just wondering fo the manual
 * fbcit thinks there should be a nice feature in the staff-client to do it
 <nengard> fbcit - ahh
 <fbcit> afaik there is no way to import from inside Koha
 <fbcit> it would not be too hard to add, though



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:37 UTC  ---

This bug was previously known as _bug_ 2060 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=2060

Actual time not defined. Setting to 0.0
CC member bchurch@ptfs.com does not have an account here

Comment 1 Jared Camins-Esakov 2012-06-27 16:59:50 UTC
Patches are available in the bug_2060 branch in my repository: https://github.com/jcamins/koha.git
Comment 2 Nicole C. Engard 2012-07-13 20:09:04 UTC
Jared,

Can you attach the patches here instead? 

Nicole
Comment 3 Jared Camins-Esakov 2012-07-17 13:49:01 UTC Comment hidden (obsolete)
Comment 4 Jared Camins-Esakov 2012-07-17 13:49:20 UTC Comment hidden (obsolete)
Comment 5 Jared Camins-Esakov 2012-07-17 13:49:30 UTC Comment hidden (obsolete)
Comment 6 Elliott Davis 2012-07-17 14:31:07 UTC Comment hidden (obsolete)
Comment 7 Elliott Davis 2012-07-17 14:32:35 UTC
Comment on attachment 10899 [details] [review]
Bug 8063 - Followup - Bug fix

Ignore the follow up for bug 8063.  When attaching through git bz I entered one too many commitid's.
Comment 8 Elliott Davis 2012-07-17 14:35:43 UTC Comment hidden (obsolete)
Comment 9 Elliott Davis 2012-07-17 14:37:00 UTC Comment hidden (obsolete)
Comment 10 Elliott Davis 2012-07-17 14:38:58 UTC Comment hidden (obsolete)
Comment 11 Jared Camins-Esakov 2012-08-01 13:42:16 UTC Comment hidden (obsolete)
Comment 12 Jared Camins-Esakov 2012-08-01 13:42:34 UTC Comment hidden (obsolete)
Comment 13 Jared Camins-Esakov 2012-08-01 13:42:49 UTC Comment hidden (obsolete)
Comment 14 Magnus Enger 2012-08-10 10:04:52 UTC
*** Bug 5332 has been marked as a duplicate of this bug. ***
Comment 15 Marcel de Rooy 2012-08-30 12:00:18 UTC
QA Comment:
Quite impressive changes to incorporate auths into import functionality.
Code looks good to me. Did not test, refer to signoff.
I will upgrade the status, but with the note to RM that with these changes it would not hurt if we had yet another signoff or two :)
It includes an atomicupdate to be pasted into updatedatabase.

Passed QA
Comment 16 Kyle M Hall 2012-08-30 13:40:56 UTC
How can you possibly pass QA if you HAVEN'T TESTED IT!!!!

This is a major failure in QA. I'm sorry, but if you haven't tested it you shouldn't be changing the status.

Koha 3.8.x has been quite buggy, and actions like this won't help at all.

Kyle

(In reply to comment #15)
> QA Comment:
> Quite impressive changes to incorporate auths into import functionality.
> Code looks good to me. Did not test, refer to signoff.
> I will upgrade the status, but with the note to RM that with these changes
> it would not hurt if we had yet another signoff or two :)
> It includes an atomicupdate to be pasted into updatedatabase.
> 
> Passed QA
Comment 17 Kyle M Hall 2012-08-30 13:45:18 UTC
I know you suggest another sign-off or two, but if you think that is necessary, mark it as 'Needs Signoff', not 'Passed QA'.

Kyle

(In reply to comment #15)
> QA Comment:
> Quite impressive changes to incorporate auths into import functionality.
> Code looks good to me. Did not test, refer to signoff.
> I will upgrade the status, but with the note to RM that with these changes
> it would not hurt if we had yet another signoff or two :)
> It includes an atomicupdate to be pasted into updatedatabase.
> 
> Passed QA
Comment 18 Marcel de Rooy 2012-08-30 19:23:18 UTC
(In reply to comment #16)
> How can you possibly pass QA if you HAVEN'T TESTED IT!!!!
Shouting to volunteers in the QA team will not help you. Your tone is not acceptable. So I will not even  respond.
Comment 19 Kyle M Hall 2012-08-31 14:02:58 UTC
(In reply to comment #18)
> (In reply to comment #16)
> > How can you possibly pass QA if you HAVEN'T TESTED IT!!!!
> Shouting to volunteers in the QA team will not help you. Your tone is not
> acceptable. So I will not even  respond.

But you did respond ; )

I do apologize if my reply came off as yelling. Tone doesn't really come through well when it comes to plain text. I was only trying to add emphasis. I do stand by what I wrote but I hope there are no hard feelings.
Comment 20 Marcel de Rooy 2012-08-31 19:22:13 UTC
(In reply to comment #19)
> But you did respond ; )
Could not resist it

> I do apologize if my reply came off as yelling. Tone doesn't really come
> through well when it comes to plain text. I was only trying to add emphasis.
> I do stand by what I wrote but I hope there are no hard feelings.
OK
I think we should discuss what QA is/should be on the list. For me it is not a second signoff. If the queue and time allows, I would be happy to test and sign off additionally. But that is not always the case. Please send a mail to the list..
Comment 21 Brendan Gallagher 2012-09-04 21:15:36 UTC
(In reply to comment #20)
> (In reply to comment #19)
> > But you did respond ; )
> Could not resist it
> 
> > I do apologize if my reply came off as yelling. Tone doesn't really come
> > through well when it comes to plain text. I was only trying to add emphasis.
> > I do stand by what I wrote but I hope there are no hard feelings.
> OK
> I think we should discuss what QA is/should be on the list. For me it is not
> a second signoff. If the queue and time allows, I would be happy to test and
> sign off additionally. But that is not always the case. Please send a mail
> to the list..

I do agree tone is hard to understand. 

This is a great discussion to bring to the mailing list.  Maybe for future QA(x)'s and RM's - a note on the bug of - "it passes code quality but WE need more (someone) to test the functionality".  

Marcel this was in no way directed at you (we all need more testing - wish I had the magic wand for that).  We all appreciate the hard work that you are volunteering.  Marcel++
Comment 22 Paul Poulain 2012-09-07 16:47:21 UTC
(In reply to comment #16)
> How can you possibly pass QA if you HAVEN'T TESTED IT!!!!
Jumping in the discution = Kyle, the QA team does not test the feature, it investigates code to detect something not respecting coding guidelines, coding practices.
Sometimes, the QAer also find some possible side effect. Something that is not in the test plan, but could break another thing. In this case, there are 2 possibilities:
 * the QAer test itself
 * the QAer ask for more tests & switch back to "need signoff"
 * the QAer passes QA, warning the RM, that has the final cut anyway
(yes, that's 3 and not 2, but it's friday, so it's permitted)

;-)

Time to leave for me now, i'll test this patch lated & deeply
Comment 23 Jared Camins-Esakov 2012-09-11 19:02:45 UTC
Created attachment 12150 [details] [review]
Bug 7475: Update configuration

In order to make matching rules more useful for MARC21 authorities,
this patch adds special indexes on previous see-from headings and
LCCN. This patch does not change UNIMARC authority configuration in
any way. Also modifies the Koha schema in preparation for adding
authority import and matching to the Staging tools.

To install:
1. Run installer/data/mysql/atomicupdate/importauthorities.pl
2. Update the following four files in your koha-dev:
    etc/zebradb/authorities/etc/bib1.att
    etc/zebradb/marc_defs/marc21/authorities/authority-koha-indexdefs.xml
    etc/zebradb/marc_defs/marc21/authorities/authority-zebra-indexdefs.xsl
    etc/zebradb/xsl/koha-indexdefs-to-zebra.xsl
3. Reindex your authorities:
    misc/migration_tools/rebuild_zebra.pl -a -r -v

NOTE TO RM: this patch adds an atomicupdate file that needs to be
incorporated into updatedatabase.pl if bug 7167 is not pushed.

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

Signed-off-by: Elliott Davis <elliott@bywatersolutions.com>

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Rebased on master 1 August 2012
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Rebased on master 11 September 2012
Comment 24 Jared Camins-Esakov 2012-09-11 19:02:58 UTC Comment hidden (obsolete)
Comment 25 Jared Camins-Esakov 2012-09-11 19:03:14 UTC Comment hidden (obsolete)
Comment 26 SJeffery 2012-09-12 15:11:34 UTC
Is this going to make it into 3.10? We have some major projects in our library that would make extensive use of this functionality.
Comment 27 Katrin Fischer 2012-09-12 15:15:50 UTC
Hi SJeffery, you could help getting this in by testing it very well and then giving it another sign-off!
Comment 28 Katrin Fischer 2012-09-12 15:16:38 UTC
Really, it would be a great addition - and more sign-offs are always helpful. :)
Comment 29 SJeffery 2012-09-13 14:47:47 UTC
I tested this and have the following comments:

1. Possible future enhancement: I can see many libraries requiring the script link_bibs_to_authorities.pl to be running in order for this to function properly. As a librarian, I would never know that there is a separate script that needs to be run in order to get my imported authorities to actually work. This script may need to be integrated into Koha a bit better.

2. Possible future enhancement: There does not seem to be a good way to match author (and possibly other types of) authority records at time of import.

That said, this patch works and I am signing off on it. Please note that I am working remotely so can not sign off in the code.
Comment 30 Paul Poulain 2012-09-13 15:45:42 UTC
I'm trying to test this patch.
Test plan:
 * search an authority, and export it in iso2709 unicode
 * go to stage-marc-import.pl and upload it, specifying it's an authority
 * go to manage-marc-import.pl, check it's an authority (Type: Authority records is displayed => OK)

the manage-marc-import.pl says:
 Add new bibliographic records into this framework: <list of biblio frameworks>

There's also:
Item processing: <Always add items>

I think it's wrong.

Just behind, the citation is empty

Hitting "Import this batch into the catalogue" fails = the job progress stays to 0% forever


Side question = is it supposed to work well in UNIMARC ?
Comment 31 Paul Poulain 2012-09-13 15:46:10 UTC Comment hidden (obsolete)
Comment 32 Jared Camins-Esakov 2012-09-13 15:50:08 UTC
Did you run the database update? That's what it looks like when you don't.
Comment 33 Paul Poulain 2012-09-13 16:47:06 UTC Comment hidden (obsolete)
Comment 34 Jared Camins-Esakov 2012-09-13 17:02:24 UTC
Marking back to Passed QA, as it would seem the UNIMARC authority in question was entirely garbled by the Koha export.
Comment 35 Paul Poulain 2012-09-14 10:34:54 UTC
Created attachment 12213 [details]
UNIMARC authorities

These authorities comes from BNF
Comment 36 Paul Poulain 2012-09-14 10:38:22 UTC Comment hidden (obsolete)
Comment 37 Jared Camins-Esakov 2012-09-14 12:49:20 UTC Comment hidden (obsolete)
Comment 38 Jared Camins-Esakov 2012-09-15 12:28:05 UTC
Returning to Passed QA so that Paul will see the updated follow-up.
Comment 39 Paul Poulain 2012-09-19 13:10:18 UTC
Sorry, but patch does not apply anymore, and it's a non trivial conflict (for me at least ;-) )
CONFLICT (content): Merge conflict in C4/ImportBatch.pm
Comment 40 Jared Camins-Esakov 2012-09-19 14:08:15 UTC
Created attachment 12346 [details] [review]
Bug 7475: Teach matching rules to handle authorities

* Add the code necessary to handle authorities with matching rules and
  import batches.
* Update all the scripts that use the matcher and import batch code
  to use the new API.
* Add authority records to the matching rules interface in the staff
  client.

http://bugs.koha-community.org/show_bug.cgi?id=2060
Signed-off-by: Elliott Davis <elliott@bywatersolutions.com>

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Rebased on latest master 11 September 2012
Comment 41 Jared Camins-Esakov 2012-09-19 14:08:52 UTC
Created attachment 12347 [details] [review]
Bug 2060: Update command line MARC import scripts

Expose authority import functionality to the command line import
scripts, and rename them from commit_biblios_file.pl and
stage_biblios_file.pl to commit_file.pl and stage_file.pl.

To test (note that these instructions assume you have a MARC21
installation and are using the provided sample file):
1. Find a file of authorities (a sample file with MARC21 authorities
   is attached to bug 7475) and download it to your server
2. Stage the file using the following command (replace <filename> with
   the name of the file you saved in step 1):
   > misc/stage_file.pl --file <filename> --authorities
3. Note the batch number the script assigns to your batch
4. Commit the records using the following command (replace <batchnumber>
   with the batch number you made note of in step 3):
   > misc/commit_file.pl --batch-number <batchnumber>
5. Index the authorities Zebraqueue (or wait)
6. Confirm that the new authorities appear.
7. Create a matching rule with the following settings:
   Code: AUTHTEST
   Description: Personal name main entry
   Match threshold: 999
   Record type: Authority record
   Search index: Heading-main
   Score: 1000
   Tag: 100
   Subfields: a
   Offset: 0
   Length: 0
   (note the ID of this matching rule)
8. Stage the authority file again, this time using the following
   command:
   > misc/stage_file.pl --file <filename> --authorities \
     --match <matchingrule>
7. Revert the import with the following command:
   > misc/commit_file.pl --batch-number <batchnumber> --revert
8. Index the authorities Zebraqueue (or wait)
9. Confirm that the records have been removed
10. Import an authority record with the Stage MARC/Manage staged MARC
    tools in exactly the way you would for a bibliographic record,
    but choose "Authority" instead of "Bibliographic" for the record
    type.

Signed-off-by: Elliott Davis <elliott@bywatersolutions.com>

Testing plan delivers as it should.

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Rebased on latest master 11 September 2012
Comment 42 Jared Camins-Esakov 2012-09-19 14:09:11 UTC
Created attachment 12348 [details] [review]
Bug 2060 follow-up: add support for UNIMARCAUTH

The staged MARC management script was not correctly informing
the decoder ring that we had UNIMARC authorities, and the decoder
ring was dutifully trying to turn the authority records into a
bibliographic box of cereal.
Comment 43 Paul Poulain 2012-09-19 15:21:49 UTC
Patch pushed to master

This time, everything work afaics !