Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server.
Summary: Add a new authority linker which searches for authority links on a Z39.50 ser...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 10 votes (vote)
Assignee: Bouzid Fergani
QA Contact: Testopia
URL:
Keywords:
Depends on: 11299
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-25 19:37 UTC by Frédérick Capovilla
Modified: 2019-04-14 12:37 UTC (History)
13 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Add a Z39.50 Linker that searches for authority links in the specified Z39.50 server. (20.32 KB, patch)
2013-11-25 19:37 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
Bug 11300 - Add a Z39.50 Linker that searches for authority links in the specified Z39.50 server (20.97 KB, patch)
2015-04-15 20:32 UTC, Blou
Details | Diff | Splinter Review
Bug 11300 - Add a Z39.50 Linker that searches for authority links in the specified Z39.50 server (20.78 KB, patch)
2015-04-16 16:03 UTC, Blou
Details | Diff | Splinter Review
Bug 11300 - correcting table z3950servers now uses 'servername' instead of 'name' (8.76 KB, patch)
2015-04-16 16:03 UTC, Blou
Details | Diff | Splinter Review
Bug 11300 - Add update_cache method to C4::Linker::Z3950Server to match definition (1.38 KB, patch)
2015-04-16 20:17 UTC, Blou
Details | Diff | Splinter Review
Bug 11300 - Add missing parameter in call to LinkBibHeadingsToAuthorities (1.07 KB, patch)
2015-05-28 20:33 UTC, Blou
Details | Diff | Splinter Review
Bug 11300 - Add a Z39.50 Linker that searches for authority links in the specified Z39.50 server (31.96 KB, patch)
2015-12-02 13:27 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 11300 - Add a Z39.50 Linker that searches for authority links in the specified Z39.50 server (32.83 KB, patch)
2016-10-14 14:09 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 11300: small fix for adjust using first,last and default in result search, using index Match-heading for search (2.38 KB, patch)
2016-10-14 14:17 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server. (27.38 KB, patch)
2017-03-10 16:42 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server. (27.47 KB, patch)
2017-09-15 20:41 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server. (28.82 KB, patch)
2017-11-10 21:39 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server. (25.36 KB, patch)
2017-11-17 20:24 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server. (25.36 KB, patch)
2018-09-25 19:47 UTC, Bouzid Fergani
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frédérick Capovilla 2013-11-25 19:37:08 UTC Comment hidden (obsolete)
Comment 1 François Pichenot 2015-03-05 11:32:32 UTC
Nice feature, but I can't test the patch on a sandbox.

Some problems occurred applying patches from bug 11299 11300:
<h1>Something went wrong !</h1>Applying: Add a Z39.50 Linker that searches for authority links in the specified Z39.50 server. Also modifies the automatic linker button so it won't create empty authorities when used.
fatal: sha1 information is lacking or useless (C4/Biblio.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.

Patch failed at 0001 Add a Z39.50 Linker that searches for authority links in the specified Z39.50 server. Also modifies the automatic linker button so it won't create empty authorities when used.

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.
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server.

23147 - Add a Z39.50 Linker that searches for authority links in the specified Z39.50 server.
Comment 2 Blou 2015-04-15 20:32:21 UTC Comment hidden (obsolete)
Comment 3 Blou 2015-04-15 20:37:27 UTC
I rebased the patch, on top of bz 11299.  I fixed the issues spotted by qa-test.  This now applies cleanly.

***** Remember to first apply 11299 before *****
Comment 4 Blou 2015-04-16 16:03:13 UTC Comment hidden (obsolete)
Comment 5 Blou 2015-04-16 16:03:30 UTC Comment hidden (obsolete)
Comment 6 Blou 2015-04-16 16:04:23 UTC
Rebased over 11299, and small fixes added in new patch
Comment 7 Blou 2015-04-16 20:17:58 UTC Comment hidden (obsolete)
Comment 8 Blou 2015-05-28 20:33:27 UTC Comment hidden (obsolete)
Comment 9 Bouzid Fergani 2015-12-02 13:27:22 UTC
Created attachment 45324 [details] [review]
Bug 11300 - Add a Z39.50 Linker that searches for authority links in the specified Z39.50 server

This patch depends on code added in Bug 11299, and must be applied after it.

Sponsored-by: CCSR ( http://www.ccsr.qc.ca )

This patch adds a Z39.50 Linker which searches for authority links on a remote server.

If a matching authority is found, it's imported in the local database for linking with the current biblio record.

If no matching authority was found on the remote server, the Linker falls back to a local authority search.

Configuration :
* The option "Z39.50 Server" is added to the LinkerModule preference. You must choose this to use the new Linker.
* The preference LinkerZ3950Server is required and specifies which server to use for linking. It must contain the "name" of the server, as defined in the z3950servers table.
* You can set the "local_first" option in the LinkerOptions preference to force the Linker to search for authorities in the local database first. If no local authority match was found, the Linker falls back to a remote Z39.50 search.
Comment 10 Josef Moravec 2016-02-16 13:44:29 UTC
Could you rebase please?
Comment 11 Bouzid Fergani 2016-10-14 14:09:20 UTC
Created attachment 56542 [details] [review]
Bug 11300 - Add a Z39.50 Linker that searches for authority links in the specified Z39.50 server

This patch depends on code added in Bug 11299, and must be applied after it.

Sponsored-by: CCSR ( http://www.ccsr.qc.ca )

This patch adds a Z39.50 Linker which searches for authority links on a remote server.

If a matching authority is found, it's imported in the local database for linking with the current biblio record.

If no matching authority was found on the remote server, the Linker falls back to a local authority search.

Configuration :
* The option "Z39.50 Server" is added to the LinkerModule preference. You must choose this to use the new Linker.
* The preference LinkerZ3950Server is required and specifies which server to use for linking. It must contain the "name" of the server, as defined in the z3950servers table.
* You can set the "local_first" option in the LinkerOptions preference to force the Linker to search for authorities in the local database first. If no local authority match was found, the Linker falls back to a remote Z39.50 search.
Comment 12 Bouzid Fergani 2016-10-14 14:17:25 UTC
Created attachment 56543 [details] [review]
Bug 11300: small fix for adjust using first,last and default in result search, using index Match-heading for search

- Rebase all patches .
- Fix using first,last and default in result search.
- Using index Match-heading for search z3950 server.
Comment 13 Bouzid Fergani 2017-03-10 16:42:49 UTC
Created attachment 60999 [details] [review]
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server.
Comment 14 Bouzid Fergani 2017-03-10 16:44:10 UTC
Hi all,
I rebase all patches and squash all in one good patch.
Thanks.
Comment 15 Katrin Fischer 2017-05-01 23:51:33 UTC
Blocked by failed 11299 for now - hope we can get this moving soon!
Comment 16 Bouzid Fergani 2017-09-15 20:41:08 UTC
Created attachment 67165 [details] [review]
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server.
Comment 17 Bouzid Fergani 2017-09-15 20:42:07 UTC
Rebase patch because it have conflict
Comment 18 Bouzid Fergani 2017-09-15 20:45:24 UTC
Hi katrin,
I change status for this patch, because 11299 it's needs signoff.
Thanks.
Bouzid.
Comment 19 Katrin Fischer 2017-10-09 06:36:43 UTC
Patch applies cleanly on top of bug 11299!
Comment 20 Katrin Fischer 2017-10-14 19:23:49 UTC
Comment on attachment 67165 [details] [review]
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server.

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

Hi Bouzid,

noted some thing in the splinter review. Additionally:

- atomicupdate for adding the system preference is missing
- You will have to add tests for the new module to pass QA
- QA tools show some fails:

 OK	C4/Biblio.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   OK	  pod coverage
   OK	  spelling
   OK	  valid

 FAIL	C4/Linker/Z3950Server.pm
   FAIL	  critic
		Found use of Switch. This module is deprecated by the Perl 5 Porters at line 223, column 5. Find an alternative module.
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   FAIL	  pod coverage
		POD is missing for flip_heading
		POD is missing for getZ3950Authority
		POD is missing for get_link
		POD is missing for update_cache
   OK	  spelling
   OK	  valid

 OK	cataloguing/automatic_linker.pl
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   OK	  spelling
   OK	  valid

 FAIL	installer/data/mysql/sysprefs.sql
   OK	  git manipulation
   OK	  semicolon
   FAIL	  sysprefs_order
		Not blocker: Sysprefs AdditionalFieldsInZ3950ResultSearch is bad placed (see bug 10610)

- I've tried to configure and test the feature, but see an alert with an internal server error. Plack error logs show:

CGI::param called in list context from package CGI::Compile::ROOT::home_vagrant_kohaclone_cataloguing_automatic_linker_2epl line 65, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436.
Personal-name,do-not-truncate,ext="K�rger, Walter Christian"
ZOOM error 114 "Unsupported Use attribute" (addinfo: "Personal-name") from diag-set 'Bib-1' at /home/vagrant/kohaclone/C4/Linker/Z3950Server.pm line 260.
Can't locate object method "get_link" via package "C4::Linker::Default" (perhaps you forgot to load "C4::Linker::Default"?) at /home/vagrant/kohaclone/C4/Biblio.pm line 533.

::: C4/Linker/Z3950Server.pm
@@ +15,5 @@
> +# You should have received a copy of the GNU General Public License
> +# along with Koha; if not, see <http://www.gnu.org/licenses>.
> +
> +use strict;
> +use warnings;

Please use Modern::Perl instead!

::: installer/data/mysql/sysprefs.sql
@@ +8,5 @@
>  ('AcqWarnOnDuplicateInvoice','0','','Warn librarians when they try to create a duplicate invoice','YesNo'),
>  ('AddressFormat','us','us|de|fr','Choose format to display postal addresses', 'Choice'),
> +('AdditionalFieldsInZ3950ResultSearch', '', NULL, 'Determines which MARC field/subfields are displayed in -Additional field- column in the result of a search Z3950', 'Free'),
> +('AddPatronLists','categorycode','categorycode|category_type','Allow user to choose what list to pick up from when adding patrons','Choice'),
> +('AddressFormat','us','','Choose format to display postal addresses', 'Choice'),

I think there is a merge error here - the new pref (LinkerZ3950Server) is missing and others are doubled up.
Comment 21 Bouzid Fergani 2017-11-10 21:39:16 UTC
Created attachment 69096 [details] [review]
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server.
Comment 22 Bouzid Fergani 2017-11-17 20:24:15 UTC
Created attachment 69204 [details] [review]
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server.
Comment 23 Bouzid Fergani 2017-11-17 20:35:20 UTC
Hi Katrin,
I fixed :
     - adding atomicupdate
     - cleaninig sysprefs.sql
     - Some problem of QA tools
     - Use Modern::Perl
     - internal error server
But, I do not know how to add tests for new linker, because it uses z3950 server?, any idea.

 Thanks
Comment 24 Katrin Fischer 2017-12-18 22:33:34 UTC
Comment on attachment 69204 [details] [review]
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server.

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

Hi Bouzid, did you attach the most recent patch? This one is not using Modern::Perl as you said in your last comment?

I have taken another look at the code, some are questions:

::: C4/Linker/Z3950Server.pm
@@ +1,1 @@
> +package C4::Linker::Z3950Server;

I am not sure about adding a new module to the C4 namespace at this point in time, as we have been busy moving things into the new Koha namespace. This might be something to check with the Release Manager. It might be ok because of the relationship with C4::Linker.

@@ +15,5 @@
> +# You should have received a copy of the GNU General Public License
> +# along with Koha; if not, see <http://www.gnu.org/licenses>.
> +
> +use strict;
> +use warnings;

You should always use Modern::Perl instead of use strict; use warnings;

@@ +121,5 @@
> +  ($authid, $record, $status) = $self->getZ3950Authority($heading);
> +
> +  Do a Z39.50 search for the heading using the $conn ZOOM::Connection object and the $heading Heading.
> +  The column origincode is used to check for duplicates.
> +  FIXME: Use thesaurus in search? As of Koha 3.8, the community stopped using them.

What's this comment referring to?

@@ +167,5 @@
> +    }
> +    my $query;
> +    if ($heading->{'auth_type'} eq 'PERSO_NAME' ){
> +         $query =qq(Personal-name,do-not-truncate,ext="$heading->{'search_form'}");
> +    }elsif ($heading->{'auth_type'}  eq 'UNIF_TITLE' ) {

I am not sure if these are universal or only specific to MARC21. :(

@@ +214,5 @@
> +        my $authtypecode = C4::AuthoritiesMarc::GuessAuthTypeCode($record);
> +        my $authId;
> +
> +        # Use the control number to prevent the creation of duplicate authorities.
> +        my $controlNumber = $record->field('970')->subfield('0');

What is 970? I checked authority frameworks and bibliograpic frameworks but it's defined in neither?

@@ +225,5 @@
> +        # Corrections and verifications before insertion
> +        my $format;
> +        my $leader='     nz  a22     o  4500';# Leader for incomplete MARC21 record
> +
> +        if (uc(C4::Context->preference('marcflavour')) eq 'UNIMARC') {

No need for uc() here I think.

@@ +238,5 @@
> +                $record->leader($leader);
> +            }
> +            if (!$record->field('003')) {
> +                $record->insert_fields_ordered(
> +                        MARC::Field->new('003',C4::Context->preference('MARCOrgCode'))

I think this needs to be adjusted - as of 17.11 the MARCOrgCode can also be set on library level.
Comment 25 Bouzid Fergani 2018-09-25 19:47:16 UTC
Created attachment 79400 [details] [review]
Bug 11300 - Add a new authority linker which searches for authority links on a Z39.50 server.
Comment 26 Cori Lynn Arnold 2018-10-16 16:07:42 UTC
Can we add some test steps for this?
Comment 27 Michal Denar 2019-04-14 12:37:45 UTC
It's very useful feature, but sleep very long. Time to take it back to life?