When importing biblio records from another Koha instance via Z39.50, the authority links (subfields 9) also get imported. Because of this, the imported biblio record has its authorities linked with the wrong authids.
Created attachment 4892 [details] [review] Remove Koha-specific fields from records imported via Z39.50 Patch attached that removes the content of subfield 9 for all authority headings in biblios imported from a Z39.50 server. This patch is an improvement of Patch 4102 from Bug 5684.
GBSD#4 This patch is marked for release 3.2, but 3.2 has reached end-of-life. It fails to apply on current master: error: patch failed: C4/Biblio.pm:2490 error: C4/Biblio.pm: patch does not apply Changing status.
Comment on attachment 4892 [details] [review] Remove Koha-specific fields from records imported via Z39.50 A bug was found in this patch : If a field contains only one subfield and this subfield is $9, the $9 gets deleted, but the field isn't deleted. This can cause the record to become invalid MARC (empty field). Patch marked as obsolete. Keeping the bug open. A new, corrected version of this patch can be sent when it is adapted for Koha version 3.6 .
Created attachment 6929 [details] [review] Corrected patch, ported to Koha 3.6 Here is a corrected version of this patch. Empty fields should now be deleted. The patch was also ported to Koha 3.6
Patch sponsored by the CCSR ( http://www.ccsr.qc.ca )
Hmm it looks to me like this only does MARC21, and it will try to do it for UNIMARC and NORMARC probably making a mess of the record. I think it needs to check the syspref for MARC Flavour and do the right thing for each flavour.
Created attachment 7115 [details] [review] Corrected patch - Now checks system preferences Here is a new version that checks the marcflavour and also uses the configured koha item tag. I only did the code for the MARC21 marcflavour since I only use and understand the MARC21 format. Somebody else should add the code to take care of the authority links for UNIMARC or NORMARC records.
Created attachment 7117 [details] [review] Bug 6681: Remove Koha-specific fields from records imported via Z39.50 - Corrected RemoveItemsAndAuthidsFromRecord should now use the system preferences. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed off, this now has the added benefit of fixing neworderempty.pl which used to blindly delete 952, wether the record was UNIMARC, NORMARC or MARC21. A follow up to add removing the authority id's for UNIMARC and NORMARC would be good. But this introduces no regressions, adds a feature and fixes a bug so should not have to wait for a follow up.
QA comment This patch depends on MARC flavour, and unimarc should be added. But there's an other option: make it marc independant, and use select DISTINCT tagfield FROM marc_subfield_structure WHERE authtypecode<>''; to get the tags that are "connected" to an authority. could you update your patch ? (marking failed QA this one)
Having never worked with unimarc or normarc records and Z39.50 servers, we are not equipped to easily adapt our patch for it to work with these formats. Could anyone improve this patch so it becomes flexible enough for inclusion on master?
Created attachment 30795 [details] [review] This patch update the code and fix MARC flavour problem by removing all subfields $9 from the imported record.
Comment on attachment 30795 [details] [review] This patch update the code and fix MARC flavour problem by removing all subfields $9 from the imported record. Review of attachment 30795 [details] [review]: ----------------------------------------------------------------- Starting with a code review here: 1) This patch lacks a test plan and a description of the changes - please add those to the commit message. 2) This patch also doesn't do, what the bug description says. It not only removes $9 subfields, but it also removes items. I am not quite happy with both those features combined into one method. I'd like it a bit better if they were separated - but there might be reason to have both combined? 3) Anyway, we need a unit test for the new routine(s). 4) It's not MARC-agnostic, but only adds code for MARC21 - not for UNIMARC or NORMARC. It would be nice to add code for the other dialects as well or at least file bugs for them. ::: C4/Biblio.pm @@ +2800,5 @@ > + } > + > + # Remove all authid links ($9) in the record > + if ( $marcflavour eq "MARC21" ) { > + my @heading = qw(100 110 111 130 440 600 610 611 630 648 650 651 655 700 710 711 730 800 810 811 830); I am a bit torn about hardcoding the fields. One idea I had was to look up the authority linked fields in the frameworks - but you might have a library that doesn't use authorities at all or differently and import from another that does use them - in that case it won't work. So I don't have a better idea currently.
Hi Katrin, 1) Done. 2) When a record is imported from Z39.50, both authorities and items are specific to the library imported, so should be removed. 3) Done. 4) I sent a wrong patch.
Created attachment 31181 [details] [review] Bug 6681 - When importing a biblio record via Z39.50, authority links and items also get imported. This patch removes the content of subfield 9 for all authority headings and items in biblios imported from a Z39.50 server. Changes: C4/Biblio.pm - new function RemoveItemsAndAuthidsFromRecord added acqui/neworderempty.pl - calls new function cataloguing/addbiblio.pl - calls new function t/Biblio.t - new tests added Testing: I Before applying the patch 0) Add a koha z39.50 server (Home › Administration › Z39.50 servers); 1) in Home > Cataloging click <<New from z39.50>>; 2) Import a record with an autority link (with zone 100 subzone 9 for example); 3) Validate: The record imported have zone 100, subzone 9; II After applying the patch 1) in Home > Cataloging click <<New from z39.50>>; 2) Import a record with an autority link (with zone 100 subzone 9 for example); 3) Validate: The record imported don't have the subzone 9 in the zone 100;
Hi Simith, (In reply to simith.doliveira from comment #14) > 2) When a record is imported from Z39.50, both authorities and items are > specific to the library imported, so should be removed. The main problem here was that the change was hidden and undocumented - noone would know that the patch will have this consequence without reading the patch code. Much better now :)
I'm not sure about the approach. Comments: 1) We should fix Koha's public server configuration so that it doesn't share information we don't want it to. Maybe applying an XSLT transformation. 2) We still might use Z39.50 targets that give us wrong data in their records (not Koha servers for example), so a way of defining what to remove is needed. This is harder to implement, but it can be achieved Z39.50 server-wise, using YAML or the usual | to separate fields$subfields. 3) This should be an opt-in, people who wants stuff removed should be able to choose to do it, not a forced record munging. So, if you continue with the current approach, please introduce a way to control it. 4) I like that you capture the warnings on the unit tests, and the fact that there are unit tests :-D But we need functional tests, in the sense that the behaviour should be tested. I can help if you need advise on the tests writing.
(In reply to Tomás Cohen Arazi from comment #17) > > 2) We still might use Z39.50 targets that give us wrong data in their > records (not Koha servers for example), so a way of defining what to remove > is needed. This is harder to implement, but it can be achieved Z39.50 > server-wise, using YAML or the usual | to separate fields$subfields. Thinking a bit more about this: i forgot to mention that Marcel's work on bug 6536 will be in master soon. While it might be improved, it provides a framework for using XSLT on z39.50 search and import operations. So this specific bug might reduce to just implementing a good default XSLT for removing Koha's authid's and items on top of bug 6536.
Created attachment 33177 [details] [review] Bug 6681: introduce XSLT scripts to remove items and links This patch adds the following sample files: - Del952.xsl - Del995.xsl - Del9LinksExcept952.xsl - Del9LinksExcept995.xsl Thay can be used on the new SRU/Z39.50 XSLT processing feature. At the same time they can be used to solve this bug. To test: - Have an SRU/Z39.50 target configured - Have a search that returns at least a record with the following properties: * It contains $9 links * It contains items (952 or 995 fields in MARC21/NORMARC and UNIMARC respectively). MARC21/NORMARC test: - Do a Z39.50/SRU cataloguing search that retrieves the desired record - Verify it includes $9 and 952 field(s) by clicking on the MARC link - Edit your Z39.50 target and add Del952.xsl to the "XSLT File(s) for transforming results:" field - Re-run the search => SUCCESS: the MARC view of the imported record doesn't contain the 952 field - Edit your Z39.50 target and add Del9LinksExcept952.xsl to the "XSLT File(s) for transforming results:" field - Re-run the search => SUCCESS: the MARC view contains the 952 field (including the $9 subfield), and all other $9 subfields have been removed - Edit your Z39.50 target and add Del952.xsl, Del9LinksExcept952.xsl to the "XSLT File(s) for transforming results:" field (both, comma separated) - Re-run the search => SUCCESS: the MARC view doesn't contain $9 subfields nor items. - Repeat for UNIMARC, replacing 952 for 995. - Sign off :-D Regards Tomas Sponsored-by: Universidad Nacional de Cordoba
It works well. This would be a good thing to have those XSL files in Koha. And at the first place, bug 6536 offers new interesting perspectives. But aren't this functionality (XSL transfo of z3950 search result) and those specific XSL files under-documented?
(In reply to Frédéric Demians from comment #20) > It works well. This would be a good thing to have those XSL files in Koha. > And at the first place, bug 6536 offers new interesting perspectives. But > aren't this functionality (XSL transfo of z3950 search result) and those > specific XSL files under-documented? I'm definitely in favour of Tomas's patch using bug 6536. I think this makes a lot of sense. I also agree about the XSL files being under-documented. I'll add some suggestions in splinter review.
Comment on attachment 33177 [details] [review] Bug 6681: introduce XSLT scripts to remove items and links Review of attachment 33177 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/xslt/Del952.xsl @@ +1,5 @@ > +<xsl:stylesheet > + xmlns:xsl="http://www.w3.org/1999/XSL/Transform" > + xmlns:marc="http://www.loc.gov/MARC21/slim" > + version="1.0"> > + <!-- This XSLT is intended for use when importing records via Z39.50 --> @@ +4,5 @@ > + version="1.0"> > + > + <xsl:output method="xml" indent="yes" encoding="UTF-8"/> > + <xsl:strip-space elements="*"/> > + This template matches datafields with the 952 tag. As it provides no specific instructions for how to output this input, the datafield will be deleted from the output. @@ +6,5 @@ > + <xsl:output method="xml" indent="yes" encoding="UTF-8"/> > + <xsl:strip-space elements="*"/> > + > + <xsl:template match="marc:datafield[@tag=952]"/> > + The following template is an "Identity Transform" which will copy all attributes and nodes from the input to the output. Without this template, the output will just be a text document with no XML tags. ::: koha-tmpl/intranet-tmpl/prog/en/xslt/Del995.xsl @@ +1,5 @@ > +<xsl:stylesheet > + xmlns:xsl="http://www.w3.org/1999/XSL/Transform" > + xmlns:marc="http://www.loc.gov/MARC21/slim" > + version="1.0"> > + <!-- This XSLT is intended for use when importing records via Z39.50 --> @@ +4,5 @@ > + version="1.0"> > + > + <xsl:output method="xml" indent="yes" encoding="UTF-8"/> > + <xsl:strip-space elements="*"/> > + This template matches datafields with the 995 tag. As it provides no specific instructions for how to output this input, the datafield will be deleted from the output. @@ +6,5 @@ > + <xsl:output method="xml" indent="yes" encoding="UTF-8"/> > + <xsl:strip-space elements="*"/> > + > + <xsl:template match="marc:datafield[@tag=995]"/> > + The following template is an "Identity Transform" which will copy all attributes and nodes from the input to the output. Without this template, the output will just be a text document with no XML tags. ::: koha-tmpl/intranet-tmpl/prog/en/xslt/Del9LinksExcept952.xsl @@ +1,4 @@ > +<xsl:stylesheet > + xmlns:xsl="http://www.w3.org/1999/XSL/Transform" > + xmlns:marc="http://www.loc.gov/MARC21/slim" > + version="1.0"> <!-- This XSLT is intended for use when importing records via Z39.50 --> @@ +4,5 @@ > + version="1.0"> > + > + <xsl:output method="xml" indent="yes" encoding="UTF-8"/> > + <xsl:strip-space elements="*"/> > + The following template is an "Identity Transform" which will copy all attributes and nodes from the input to the output. Without this template, the output will just be a text document with no XML tags. @@ +10,5 @@ > + <xsl:copy> > + <xsl:apply-templates select="@* | node()"/> > + </xsl:copy> > + </xsl:template> > + The following template will match every datafield which contains a subfield with a code of 9, except those datafields that have a tag of 952. Since it has no output instructions, this datafield will be deleted from the output. It's important to delete the subfield with a code of 9, since this subfield contains links to authority records, which will most likely not link correctly in Koha when importing from Z39.50. ::: koha-tmpl/intranet-tmpl/prog/en/xslt/Del9LinksExcept995.xsl @@ +1,5 @@ > +<xsl:stylesheet > + xmlns:xsl="http://www.w3.org/1999/XSL/Transform" > + xmlns:marc="http://www.loc.gov/MARC21/slim" > + version="1.0"> > + <!-- This XSLT is intended for use when importing records via Z39.50 --> @@ +4,5 @@ > + version="1.0"> > + > + <xsl:output method="xml" indent="yes" encoding="UTF-8"/> > + <xsl:strip-space elements="*"/> > + The following template is an "Identity Transform" which will copy all attributes and nodes from the input to the output. Without this template, the output will just be a text document with no XML tags. @@ +10,5 @@ > + <xsl:copy> > + <xsl:apply-templates select="@* | node()"/> > + </xsl:copy> > + </xsl:template> > + The following template will match every datafield which contains a subfield with a code of 9, except those datafields that have a tag of 995. Since it has no output instructions, this datafield will be deleted from the output. It's important to delete the subfield with a code of 9, since this subfield contains links to authority records, which will most likely not link correctly in Koha when importing from Z39.50.
Created attachment 33407 [details] [review] Bug 6681: introduce XSLT scripts to remove items and links This patch adds the following sample files: - Del952.xsl - Del995.xsl - Del9LinksExcept952.xsl - Del9LinksExcept995.xsl Thay can be used on the new SRU/Z39.50 XSLT processing feature. At the same time they can be used to solve this bug. To test: - Have an SRU/Z39.50 target configured - Have a search that returns at least a record with the following properties: * It contains $9 links * It contains items (952 or 995 fields in MARC21/NORMARC and UNIMARC respectively). MARC21/NORMARC test: - Do a Z39.50/SRU cataloguing search that retrieves the desired record - Verify it includes $9 and 952 field(s) by clicking on the MARC link - Edit your Z39.50 target and add Del952.xsl to the "XSLT File(s) for transforming results:" field - Re-run the search => SUCCESS: the MARC view of the imported record doesn't contain the 952 field - Edit your Z39.50 target and add Del9LinksExcept952.xsl to the "XSLT File(s) for transforming results:" field - Re-run the search => SUCCESS: the MARC view contains the 952 field (including the $9 subfield), and all other $9 subfields have been removed - Edit your Z39.50 target and add Del952.xsl, Del9LinksExcept952.xsl to the "XSLT File(s) for transforming results:" field (both, comma separated) - Re-run the search => SUCCESS: the MARC view doesn't contain $9 subfields nor items. - Repeat for UNIMARC, replacing 952 for 995. - Sign off :-D Regards Tomas Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested under MARC21. Works fine. (Additionally verified 995 with xsltproc.) As noted on Bugzilla, we could still improve documentation but imo no need to block these example transformations.
Created attachment 33410 [details] [review] Bug 6681: introduce XSLT scripts to remove items and links This patch adds the following sample files: - Del952.xsl - Del995.xsl - Del9LinksExcept952.xsl - Del9LinksExcept995.xsl Thay can be used on the new SRU/Z39.50 XSLT processing feature. At the same time they can be used to solve this bug. To test: - Have an SRU/Z39.50 target configured - Have a search that returns at least a record with the following properties: * It contains $9 links * It contains items (952 or 995 fields in MARC21/NORMARC and UNIMARC respectively). MARC21/NORMARC test: - Do a Z39.50/SRU cataloguing search that retrieves the desired record - Verify it includes $9 and 952 field(s) by clicking on the MARC link - Edit your Z39.50 target and add Del952.xsl to the "XSLT File(s) for transforming results:" field - Re-run the search => SUCCESS: the MARC view of the imported record doesn't contain the 952 field - Edit your Z39.50 target and add Del9LinksExcept952.xsl to the "XSLT File(s) for transforming results:" field - Re-run the search => SUCCESS: the MARC view contains the 952 field (including the $9 subfield), and all other $9 subfields have been removed - Edit your Z39.50 target and add Del952.xsl, Del9LinksExcept952.xsl to the "XSLT File(s) for transforming results:" field (both, comma separated) - Re-run the search => SUCCESS: the MARC view doesn't contain $9 subfields nor items. - Repeat for UNIMARC, replacing 952 for 995. - Sign off :-D Regards Tomas Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested under MARC21. Works fine. (Additionally verified 995 with xsltproc.) As noted on Bugzilla, we could still improve documentation but imo no need to block these example transformations. Signed-off-by: Frederic Demians <f.demians@tamil.fr> I can't resist to put my own signature on this patch, confirming it works as described with z39.50 Unimarc targets.
Created attachment 33425 [details] [review] Bug 6681: (qa followup) document the existence of the sample files As requestedby people testing the patch, I add references to the new xslt files on the Z39.50/SRU servers help. Example usages are also provided. Test: - Apply the patch - Go to the help page on the 'Z39.50/SRU Servers' page => SUCCESS: Notice there's a section documenting XSLT file(s) usage and provides some examples that cover the introduced files. - Sign off Thanks Tomas Sponsored-by: Universidad Nacional de Cordoba
Created attachment 33427 [details] [review] Bug 6681: (qa followup) document the existence of the sample files As requestedby people testing the patch, I add references to the new xslt files on the Z39.50/SRU servers help. Example usages are also provided. Test: - Apply the patch - Go to the help page on the 'Z39.50/SRU Servers' page => SUCCESS: Notice there's a section documenting XSLT file(s) usage and provides some examples that cover the introduced files. - Sign off Thanks Tomas Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Frederic Demians <f.demians@tamil.fr> Help page does shed some light to the XSLT usage. Enough to my taste.
For the future, a better way for storing XSL could be designed. In the disorder: - a z3950severs table new field, - a new db table for XSL, - a centralized repository, - a link to a Koha community page (like for SQL report library).
(In reply to Frédéric Demians from comment #27) > For the future, a better way for storing XSL could be designed. In the > disorder: - a z3950severs table new field, - a new db table for XSL, - a > centralized repository, - a link to a Koha community page (like for SQL > report library). I've been thinking about this for a little while as well. I've been working again on an OAI harvester which uses XSLTs to filter out the 952, 999, and $9, and I figure "koha-tmpl\intranet-tmpl\prog\en\xslt" isn't always the best place to put these things. I'm intrigued by the idea of using a db field or db table. If we allowed uploads, that would make it easier for libraries to add their own XSLTs (for better or worse) without meddling around on the file system. Actually, a db table could be interesting. We could have the blob, the filename, an id... and then have a db field in z3950servers (or whatever other server config table) that has a foreign key to the primary key id in the XSLT table. I'd kind of like the ability to set defaults as well though... so you could set up XSLT profiles to apply on import (rather than having a string of filenames). Of course, we'd probably need a linking table then for profiles to files. I think that'd be a good way to go about setting up defaults though. You could have a "Default Z3950" profile (and maybe a "Default OAI" table). -- Actually, as I work on this OAI client, I start thinking about ways to integrate code and storage of Z3950 and OAI targets. I'm trying to think if there are other protocols we use for importing records as well... and how instead of making more tables, we could make some tables more generic. I wonder too about using XSLTs when importing records via the batch import, although that would probably slow things down a lot as you'd be going from Iso Marc to Marc Object to MARCXML to XSLT to MARCXML. I suppose I like the idea of making storage and code more generalizable... </ramble>
I think as the initial feature is already pushed, having examples for what you can do with it is a good start - but more documentation is of course always better :)
Created attachment 33540 [details] [review] [PASSED QA] Bug 6681: introduce XSLT scripts to remove items and links This patch adds the following sample files: - Del952.xsl - Del995.xsl - Del9LinksExcept952.xsl - Del9LinksExcept995.xsl Thay can be used on the new SRU/Z39.50 XSLT processing feature. At the same time they can be used to solve this bug. To test: - Have an SRU/Z39.50 target configured - Have a search that returns at least a record with the following properties: * It contains $9 links * It contains items (952 or 995 fields in MARC21/NORMARC and UNIMARC respectively). MARC21/NORMARC test: - Do a Z39.50/SRU cataloguing search that retrieves the desired record - Verify it includes $9 and 952 field(s) by clicking on the MARC link - Edit your Z39.50 target and add Del952.xsl to the "XSLT File(s) for transforming results:" field - Re-run the search => SUCCESS: the MARC view of the imported record doesn't contain the 952 field - Edit your Z39.50 target and add Del9LinksExcept952.xsl to the "XSLT File(s) for transforming results:" field - Re-run the search => SUCCESS: the MARC view contains the 952 field (including the $9 subfield), and all other $9 subfields have been removed - Edit your Z39.50 target and add Del952.xsl, Del9LinksExcept952.xsl to the "XSLT File(s) for transforming results:" field (both, comma separated) - Re-run the search => SUCCESS: the MARC view doesn't contain $9 subfields nor items. - Repeat for UNIMARC, replacing 952 for 995. - Sign off :-D Regards Tomas Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested under MARC21. Works fine. (Additionally verified 995 with xsltproc.) As noted on Bugzilla, we could still improve documentation but imo no need to block these example transformations. Signed-off-by: Frederic Demians <f.demians@tamil.fr> I can't resist to put my own signature on this patch, confirming it works as described with z39.50 Unimarc targets. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 33541 [details] [review] [PASSED QA] Bug 6681: (qa followup) document the existence of the sample files As requestedby people testing the patch, I add references to the new xslt files on the Z39.50/SRU servers help. Example usages are also provided. Test: - Apply the patch - Go to the help page on the 'Z39.50/SRU Servers' page => SUCCESS: Notice there's a section documenting XSLT file(s) usage and provides some examples that cover the introduced files. - Sign off Thanks Tomas Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Frederic Demians <f.demians@tamil.fr> Help page does shed some light to the XSLT usage. Enough to my taste. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Patches pushed to master.