Summary: | Modify authority type frameworks to allow see also fields to link to thesauri | ||
---|---|---|---|
Product: | Koha | Reporter: | Jared Camins-Esakov <jcamins> |
Component: | MARC Authority data support | Assignee: | Jared Camins-Esakov <jcamins> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | chris, m.de.rooy, magnus, paul.poulain |
Version: | 3.10 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | Sponsored | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 8212, 8332, 8523 | ||
Attachments: |
Bug 8207: Allow see also fields in auths to link to thesauri
Bug 8207: Allow see also fields in auths to link to thesauri [SIGNED-OFF] Bug 8207: Allow see also fields in auths to link to thesauri Bug 8207 follow-up: correct typo in authority types Bug 8207 follow-up: correct typo in authority types |
Description
Jared Camins-Esakov
2012-06-07 14:02:11 UTC
Created attachment 10551 [details] [review] Bug 8207: Allow see also fields in auths to link to thesauri Right now if you were to link a field in an authority to a thesaurus, it would 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). This patch adds subfield $9 and thesaurus links to existing installs for those 5xx fields (under MARC21 and NORMARC only) which do not yet have any subfield $9 or thesauri configured, as well as adding that to new English language installs. There will be a follow-up patch adding the subfields to new non-English language install files later, once it is clear that the changes proposed and demonstrated in the English files are accepable to the community. 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? (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. Created attachment 10703 [details] [review] Bug 8207: Allow see also fields in auths to link to thesauri Right now if you were to link a field in an authority to a thesaurus, it would 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). This patch adds subfield $9 and thesaurus links to existing installs for those 5xx fields (under MARC21 and NORMARC only) which do not yet have any subfield $9 or thesauri configured, as well as adding that to new English language installs. There will be a follow-up patch adding the subfields to new non-English language install files later, once it is clear that the changes proposed and demonstrated in the English files are accepable to the community. Created attachment 10707 [details] [review] [SIGNED-OFF] Bug 8207: Allow see also fields in auths to link to thesauri Right now if you were to link a field in an authority to a thesaurus, it would 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). This patch adds subfield $9 and thesaurus links to existing installs for those 5xx fields (under MARC21 and NORMARC only) which do not yet have any subfield $9 or thesauri configured, as well as adding that to new English language installs. There will be a follow-up patch adding the subfields to new non-English language install files later, once it is clear that the changes proposed and demonstrated in the English files are accepable to the community. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tested frameworks install cleanly on a new installation. Tested database update and made sure, that configuration and subfields of a field were not changed if - a $9 subfield already existed for a 5xx field - a $9 subfield and a link to another thesaurus in $a existed - no $9 but a link to another thesaurus in $a existed This seems safe to me. 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 ? (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. :) (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! OK, I understood the origin of my comment 6. I was wrong, everything is OK. Passed QA 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 ;-) 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 ?? 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 :) (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. Created attachment 11784 [details] [review] Bug 8207 follow-up: correct typo in authority types Created attachment 11810 [details] [review] Bug 8207 follow-up: correct typo in authority types Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Followup: Passed QA 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. This has been done in Master. |