Bug 8207 - Modify authority type frameworks to allow see also fields to link to thesauri
Summary: Modify authority type frameworks to allow see also fields to link to thesauri
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Authority data support (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jared Camins-Esakov
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 8212 8332 8523
  Show dependency treegraph
 
Reported: 2012-06-07 14:02 UTC by Jared Camins-Esakov
Modified: 2013-12-05 20:01 UTC (History)
5 users (show)

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


Attachments
Bug 8207: Allow see also fields in auths to link to thesauri (227.87 KB, patch)
2012-06-28 22:55 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8207: Allow see also fields in auths to link to thesauri (227.86 KB, patch)
2012-07-08 18:59 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8207: Allow see also fields in auths to link to thesauri (228.23 KB, patch)
2012-07-09 06:00 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 8207 follow-up: correct typo in authority types (1.85 KB, patch)
2012-08-23 13:24 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8207 follow-up: correct typo in authority types (1.86 KB, patch)
2012-08-23 21:59 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jared Camins-Esakov 2012-06-07 14:02:11 UTC
Right now if you were to link a field in an authority to a thesaurus, it will not work properly. The authority type frameworks require some adjustments to allow see also headings to be linked to thesauri (such as adding subfield $9).
Comment 1 Jared Camins-Esakov 2012-06-28 22:55:52 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2012-07-08 18:38:58 UTC
Hi Jared, 

I am not sure about the field description "9 (RLIN)". Would it be possible to use something more descriptive? Is this is a Koha specific use of the subfield?
Can we get some more opinions on that?
Comment 3 Jared Camins-Esakov 2012-07-08 18:44:03 UTC
(In reply to comment #2)
> Hi Jared, 
> 
> I am not sure about the field description "9 (RLIN)". Would it be possible
> to use something more descriptive? Is this is a Koha specific use of the
> subfield?
> Can we get some more opinions on that?

At present we use "RLIN" in the bibliographic frameworks, so I used the same. I'd be happy to change the description for the authority frameworks, but I can't translate it as part of the update, and I am definitely not going to be editing the bibliographic frameworks (to preserve consistency). Our use of the subfield 9 is entirely Koha-specific.
Comment 4 Jared Camins-Esakov 2012-07-08 18:59:10 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2012-07-09 06:00:51 UTC Comment hidden (obsolete)
Comment 6 Paul Poulain 2012-07-26 15:26:56 UTC
mmm...

In the patch, there are some things like:
-		('', '451', 'a', 'Geographic name', 'Geographic name', 0, 0, 4, NULL, NULL, '''451z'',''451x'',''451y'',''451v''', 0, 0, '', '', ''),
+        ('', '450', '9', '9 (RLIN)', '9 (RLIN)', 0, 0, 4, NULL, NULL, NULL, 0, 1, '', '', ''),
+        ('', '451', 'a', 'Geographic name', 'Geographic name', 0, 0, 4, NULL, NULL, '''451z'',''451x'',''451y'',''451v''', 0, 0, '', '', 'GEOGR_NAME'),

where the frameworkcode is GEOGR_NAME just on this line.

Later :
-		('', '455', 'a', 'Genre/form term', 'Genre/form term', 0, 0, 4, NULL, NULL, '''455v'',''455x'',''455z'',''455y''', 0, 0, '', '', ''),
+        ('', '453', '9', '9 (RLIN)', '9 (RLIN)', 0, 0, 4, NULL, NULL, NULL, 0, 1, '', '', ''),
+        ('', '455', 'a', 'Genre/form term', 'Genre/form term', 0, 0, 4, NULL, NULL, '''455v'',''455x'',''455z'',''455y''', 0, 0, '', '', 'GENRE/FORM'),

and also
-		('', '548', 'a', 'Chronological term', 'Chronological term', 0, 0, 5, NULL, NULL, '''548y'',''548x'',''548z'',''548v''', 0, 0, '', '', ''),
+        ('', '530', '9', '9 (RLIN)', '9 (RLIN)', 0, 0, 5, NULL, NULL, NULL, 0, 1, '', '', ''),
+        ('', '548', 'a', 'Chronological term', 'Chronological term', 0, 0, 5, NULL, NULL, '''548y'',''548x'',''548z'',''548v''', 0, 0, '', '', 'CHRON_TERM'),

(there are others, just pointing 3)

Are you sure this is intended ?

I think it's not the case, but do we need something for UNIMARC ?
Comment 7 Jared Camins-Esakov 2012-07-26 15:30:10 UTC
(In reply to comment #6)
> mmm...
> 
> In the patch, there are some things like:
> -		('', '451', 'a', 'Geographic name', 'Geographic name', 0, 0, 4, NULL,
> NULL, '''451z'',''451x'',''451y'',''451v''', 0, 0, '', '', ''),
> +        ('', '450', '9', '9 (RLIN)', '9 (RLIN)', 0, 0, 4, NULL, NULL, NULL,
> 0, 1, '', '', ''),
> +        ('', '451', 'a', 'Geographic name', 'Geographic name', 0, 0, 4,
> NULL, NULL, '''451z'',''451x'',''451y'',''451v''', 0, 0, '', '',
> 'GEOGR_NAME'),
> 
> where the frameworkcode is GEOGR_NAME just on this line.
> 
> Later :
> -		('', '455', 'a', 'Genre/form term', 'Genre/form term', 0, 0, 4, NULL,
> NULL, '''455v'',''455x'',''455z'',''455y''', 0, 0, '', '', ''),
> +        ('', '453', '9', '9 (RLIN)', '9 (RLIN)', 0, 0, 4, NULL, NULL, NULL,
> 0, 1, '', '', ''),
> +        ('', '455', 'a', 'Genre/form term', 'Genre/form term', 0, 0, 4,
> NULL, NULL, '''455v'',''455x'',''455z'',''455y''', 0, 0, '', '',
> 'GENRE/FORM'),
> 
> and also
> -		('', '548', 'a', 'Chronological term', 'Chronological term', 0, 0, 5,
> NULL, NULL, '''548y'',''548x'',''548z'',''548v''', 0, 0, '', '', ''),
> +        ('', '530', '9', '9 (RLIN)', '9 (RLIN)', 0, 0, 5, NULL, NULL, NULL,
> 0, 1, '', '', ''),
> +        ('', '548', 'a', 'Chronological term', 'Chronological term', 0, 0,
> 5, NULL, NULL, '''548y'',''548x'',''548z'',''548v''', 0, 0, '', '',
> 'CHRON_TERM'),
> 
> (there are others, just pointing 3)
> 
> Are you sure this is intended ?

Yes. This is the problem with line-based diffs. Basically all I did was add a line, but git sees it as removing a line, adding a line, and then adding back the original line.

> I think it's not the case, but do we need something for UNIMARC ?

Nope. UNIMARC is sensible. :)
Comment 8 Chris Cormack 2012-07-26 19:47:41 UTC
(In reply to comment #7)

> > 
> > Are you sure this is intended ?
> 
> Yes. This is the problem with line-based diffs. Basically all I did was add
> a line, but git sees it as removing a line, adding a line, and then adding
> back the original line.
> 

Here's a handy tip, apply the patch then run

 git show --word-diff=color

Be happy and amazed :)

What I have is a git alias

 git config alias.sh "show --word-diff=color"

So i can go
 git sh

Not seeing whitespace in the diff ftw!
Comment 9 Paul Poulain 2012-08-01 13:26:49 UTC
OK, I understood the origin of my comment 6. I was wrong, everything is OK. Passed QA
Comment 10 Paul Poulain 2012-08-01 13:31:08 UTC
Patch pushed, does nothing for UNIMARC, as expected

Added Magnus to this bug = Magnus, this patch changes something when upgrading a NORMARC setup.
OTOH, it does not change the default NORMARC frameworks.

I feel there's a follow-up for NORMARC that could be useful ;-)
Comment 11 Marcel de Rooy 2012-08-23 12:43:10 UTC
Is the following code in master really correct:

if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
    unless ( C4::Context->preference('marcflavour') eq 'UNIMARC' ) {
        my %referencetypes = (  '00' => 'PERSO_CODE',
                                '10' => 'ORGO_CODE',

I thought we used PERSO_NAME and CORPO_NAME ??
Comment 12 Marcel de Rooy 2012-08-23 12:58:11 UTC
There is a similar problem in auth_finder (editor plugin)
It shows me PERSO_CODE on the form. That should be PERSO_NAME. Therefore I do not find anything.
Just writing it here; do not know where it originated from. Probably Jared does :)
Comment 13 Jared Camins-Esakov 2012-08-23 13:07:37 UTC
(In reply to comment #12)
> There is a similar problem in auth_finder (editor plugin)
> It shows me PERSO_CODE on the form. That should be PERSO_NAME. Therefore I
> do not find anything.
> Just writing it here; do not know where it originated from. Probably Jared
> does :)

Good catch. It's a problem with the updatedatabase. I'll do a follow-up fixing it.
Comment 14 Jared Camins-Esakov 2012-08-23 13:24:59 UTC Comment hidden (obsolete)
Comment 15 Chris Cormack 2012-08-23 21:59:39 UTC
Created attachment 11810 [details] [review]
Bug 8207 follow-up: correct typo in authority types

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 16 Marcel de Rooy 2012-08-27 08:46:10 UTC
Followup: Passed QA
Comment 17 Paul Poulain 2012-09-03 10:43:24 UTC
Follow-up pushed, 2 comments:
  * the follow-up is pushed on master branch, as it can't be merged properly on new/bug_8207 (the conflict is trivial, bug the merge to master is silly, due to other updatedatabases)
  * there is a risk, if someone has his own PERSO_CODE authtype, that this DB update break something. Fortunately, this is highly un-probable.
Comment 18 Jared Camins-Esakov 2012-12-03 13:27:06 UTC
This has been done in Master.