It would be great to have a way on the XML syntax to specify conditions for fields to get indexed. For example: we could want to index the 100$a subfield on "Author" only if the 1st indicator is not 'z'. The current syntax doesn't allow us to specify that.
Created attachment 40841 [details] Sample MARCXML record Sample record containing a 100$a field, with ind2=7 for testing purposes
Created attachment 40842 [details] [review] Bug 14217: Add 'condition' attribute for DOM index definition This patch introduces an extension to the current syntax for DOM index definition. Specifically, it extends the 'index_subfields' tag to allow adding a 'condition' attribute that is used as a condition ofr applying the specified index. This (exotic) example is self-explanatory: The previous syntax (which is keeped by this patch) took this snippet from biblio-koha-indexdefs.xml <index_subfields tag="100" subfields="acbd"> <target_index>Encuadernador:w</target_index> </index_subfields> and generated an XSLT snippet in the DOM indexing XSLT that looks like this: <xslo:for-each select="marc:subfield"> <xslo:if test="contains('acbd', @code)"> <z:index name="Encuadernador:w"> <xslo:value-of select="."/> </z:index> </xslo:if> </xslo:for-each> This patch introduces this syntax change (note the 'condition' attribute: <index_subfields tag="100" subfields="acbd" condition="@ind2='7'"> <target_index>Encuadernador:w</target_index> </index_subfields> which yields to this XSLT snippet in the DOM indexing XSLT: <xslo:if test="@ind2='7'"> <xslo:for-each select="marc:subfield"> <xslo:if test="contains('acbd', @code)"> <z:index name="Encuadernador:w"> <xslo:value-of select="."/> </z:index> </xslo:if> </xslo:for-each> </xslo:if> To test: - Verify that the shipped XSLT files are current regarding the shipped index definitions: $ for i in marc21 normarc unimarc; do xsltproc etc/zebradb/xsl/xsl/koha-indexdefs-to-zebra.xsl \ etc/zebradb/marc_defs/$i/biblios/biblio-koha-indexdefs.xml \ > etc/zebradb/marc_defs/$i/biblios/biblio-zebra-indexdefs.xsl done $ git status (repeat for authorities, skip normarc which doesn't have authorities) - Apply the patch - Re-run the previous commands => SUCCESS: no changes - Add a condition to an index_subfields tag (for example, condition="@ind2='7'" in the Author's index - Regenerate the specific XSLT => SUCCESS: doing a diff shows the only change is the code has been wrapped inside an xslo:if using the condition for the test - Apply the generated xsl to a MARCXML record that has a field matching the condition like this: $ xsltproc .../biblio-zebra-indexdefs.xsl sample_record.xml => SUCCESS: There's an index on the result, containing the configured field/subfields, that matches the criteria. - Sign off and feel really happy :-D Note: the attached sample record includes a 100 field, with ind2=7 and $a=Tomasito Sponsored-by: Orex Digital
(In reply to Tomás Cohen Arazi from comment #2) > To test: > - Verify that the shipped XSLT files are current regarding the shipped index > definitions: > $ for i in marc21 normarc unimarc; do > xsltproc etc/zebradb/xsl/xsl/koha-indexdefs-to-zebra.xsl \ > etc/zebradb/marc_defs/$i/biblios/biblio-koha-indexdefs.xml \ > > etc/zebradb/marc_defs/$i/biblios/biblio-zebra-indexdefs.xsl > done Small change in test plan: etc/zebradb/xsl/xsl/koha-indexdefs-to-zebra.xsl should be etc/zebradb/xsl/koha-indexdefs-to-zebra.xsl so the first step in the test plan becomes. - Verify that the shipped XSLT files are current regarding the shipped index definitions: $ for i in marc21 normarc unimarc; do xsltproc etc/zebradb/xsl/koha-indexdefs-to-zebra.xsl \ etc/zebradb/marc_defs/$i/biblios/biblio-koha-indexdefs.xml \ > etc/zebradb/marc_defs/$i/biblios/biblio-zebra-indexdefs.xsl done
Created attachment 42045 [details] [review] Bug 14217: Add 'condition' attribute for DOM index definition This patch introduces an extension to the current syntax for DOM index definition. Specifically, it extends the 'index_subfields' tag to allow adding a 'condition' attribute that is used as a condition ofr applying the specified index. This (exotic) example is self-explanatory: The previous syntax (which is keeped by this patch) took this snippet from biblio-koha-indexdefs.xml <index_subfields tag="100" subfields="acbd"> <target_index>Encuadernador:w</target_index> </index_subfields> and generated an XSLT snippet in the DOM indexing XSLT that looks like this: <xslo:for-each select="marc:subfield"> <xslo:if test="contains('acbd', @code)"> <z:index name="Encuadernador:w"> <xslo:value-of select="."/> </z:index> </xslo:if> </xslo:for-each> This patch introduces this syntax change (note the 'condition' attribute: <index_subfields tag="100" subfields="acbd" condition="@ind2='7'"> <target_index>Encuadernador:w</target_index> </index_subfields> which yields to this XSLT snippet in the DOM indexing XSLT: <xslo:if test="@ind2='7'"> <xslo:for-each select="marc:subfield"> <xslo:if test="contains('acbd', @code)"> <z:index name="Encuadernador:w"> <xslo:value-of select="."/> </z:index> </xslo:if> </xslo:for-each> </xslo:if> To test: - Verify that the shipped XSLT files are current regarding the shipped index definitions: $ for i in marc21 normarc unimarc; do xsltproc etc/zebradb/xsl/xsl/koha-indexdefs-to-zebra.xsl \ etc/zebradb/marc_defs/$i/biblios/biblio-koha-indexdefs.xml \ > etc/zebradb/marc_defs/$i/biblios/biblio-zebra-indexdefs.xsl done $ git status (repeat for authorities, skip normarc which doesn't have authorities) - Apply the patch - Re-run the previous commands => SUCCESS: no changes - Add a condition to an index_subfields tag (for example, condition="@ind2='7'" in the Author's index - Regenerate the specific XSLT => SUCCESS: doing a diff shows the only change is the code has been wrapped inside an xslo:if using the condition for the test - Apply the generated xsl to a MARCXML record that has a field matching the condition like this: $ xsltproc .../biblio-zebra-indexdefs.xsl sample_record.xml => SUCCESS: There's an index on the result, containing the configured field/subfields, that matches the criteria. - Sign off and feel really happy :-D Note: the attached sample record includes a 100 field, with ind2=7 and $a=Tomasito Sponsored-by: Orex Digital Signed-off-by: Barton Chittenden <barton@bywatersolutions.com>
I also tested the case where the indicator does *not* match, just for completeness.
This is certainly an interesting development! But it might need some further (broader) discussion before getting in. In the first place: Why do we really need it? Can you elaborate/illustrate? If we add conditions, what kind of conditions do we want? Syntax? Etc. A broader design may be easier to maintain.. How do comparable systems implement this? If we add this for Zebra, what about other search engines?
(In reply to Marcel de Rooy from comment #6) > This is certainly an interesting development! > > But it might need some further (broader) discussion before getting in. > In the first place: Why do we really need it? Can you elaborate/illustrate? > If we add conditions, what kind of conditions do we want? Syntax? Etc. A > broader design may be easier to maintain.. > How do comparable systems implement this? > If we add this for Zebra, what about other search engines? Marcel, The functionality definitely *is* needed -- see, for instance bug 14277, where the meaning of 521$a varies drastically based on the value of ind1. The 'condition=' syntax feels like overkill to me -- before work with the inicators, I assumed that the arguments to the <index_subfields> xml tag were essentially limits that were ANDed together... I guess they're not, but I think that we could pull a 'convention over configuration' fast one and pretend that they are: 'tag' and 'subfield' are required attributes, specifying the marc field and subfield, alll other attributes are limits -- if not present, all matching tags/subfields will be used. ...or we could go with the 'condition=' syntax, and make it explicit. I'm fine either way, as long as I can limit by indicator. :-)
(In reply to Marcel de Rooy from comment #6) > This is certainly an interesting development! > > But it might need some further (broader) discussion before getting in. > In the first place: Why do we really need it? Can you elaborate/illustrate? > If we add conditions, what kind of conditions do we want? Syntax? Etc. A > broader design may be easier to maintain.. > How do comparable systems implement this? There are some situations where you need to add conditions as Barton told you. Possible conditions (the ones I had in mind are conditions on the indicator values, as I stated on the commit message). As Barton wrote, if you want to send a field/subfield to a different index depending on the value of the indicators, you can do it with this. If you look at it, we already do it for authority records, but with a more complex syntax. Look at authority-koha-indexdefs.xml: <kohaidx:index_heading_conditional tag="450" test="substring(marc:subfield[@code='w']/text(), 2, 1)" subfields="abvxyz" subdivisions="vxyz"> <kohaidx:target_index>Previous-heading-see-from:p</kohaidx:target_index> </kohaidx:index_heading_conditional> the whole syntax is similar to the one i implement, but you need to add a new tag name if you want conditional indexing, while i just overload the current syntax, which is really convenient. > If we add this for Zebra, what about other search engines? Each search engine provides means to specify what/how to index. I guess stuff like ES are more flexible than Zebra, for sure.
I haven't tested yet, but looking at the code I am wondering if there is not a way to add a flag 'index_it!' instead of c/p the 2 foreach marc::subfield loops?
(In reply to Jonathan Druart from comment #9) > I haven't tested yet, but looking at the code I am wondering if there is not > a way to add a flag 'index_it!' instead of c/p the 2 foreach marc::subfield > loops? Maybe elaborate a bit more? Remember the current syntax lets you choose what subfields to include for indexing, so you need to loop. Anyway, I just hooked the extra 'condition' attribute and then test for it to skip if condition is not met. If you propose a major XSLT refactoring it should probably belong to it's own bug
If I am not wrong, the algorithm is: if conditions exist: if the condition is satisfied: do a big loop on subfield else: do the same big loop on subfield You have c/p the "big loop" I suggest to change with something like need_to_loop = 1 if conditions exist: need_to_loop = 0 if the condition is satisfied: need_to_loop = 1 if need_to_loop: do the same big loop on subfield Just to avoid the code duplication.
Created attachment 42737 [details] [review] Bug 14217: (QA followup) avoid repeating code This patch refactors the handle-one-index-subfields template to call a new template handle-one-index-subfields-helper instead of repeating the code for subfields indexing (index-subfields) which gets wrapped inside the new condition test if needed. This avoids code duplication, with the same results. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 42739 [details] [review] Bug 14217: (QA followup) avoid repeating code This patch refactors the handle-one-index-subfields template to call a new template handle-one-index-subfields-helper instead of repeating the code for subfields indexing (index-subfields) which gets wrapped inside the new condition test if needed. This avoids code duplication, with the same results. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 42740 [details] [review] Bug 14217: Add 'condition' attribute for DOM index definition This patch introduces an extension to the current syntax for DOM index definition. Specifically, it extends the 'index_subfields' tag to allow adding a 'condition' attribute that is used as a condition ofr applying the specified index. This (exotic) example is self-explanatory: The previous syntax (which is keeped by this patch) took this snippet from biblio-koha-indexdefs.xml <index_subfields tag="100" subfields="acbd"> <target_index>Encuadernador:w</target_index> </index_subfields> and generated an XSLT snippet in the DOM indexing XSLT that looks like this: <xslo:for-each select="marc:subfield"> <xslo:if test="contains('acbd', @code)"> <z:index name="Encuadernador:w"> <xslo:value-of select="."/> </z:index> </xslo:if> </xslo:for-each> This patch introduces this syntax change (note the 'condition' attribute: <index_subfields tag="100" subfields="acbd" condition="@ind2='7'"> <target_index>Encuadernador:w</target_index> </index_subfields> which yields to this XSLT snippet in the DOM indexing XSLT: <xslo:if test="@ind2='7'"> <xslo:for-each select="marc:subfield"> <xslo:if test="contains('acbd', @code)"> <z:index name="Encuadernador:w"> <xslo:value-of select="."/> </z:index> </xslo:if> </xslo:for-each> </xslo:if> To test: - Verify that the shipped XSLT files are current regarding the shipped index definitions: $ for i in marc21 normarc unimarc; do xsltproc etc/zebradb/xsl/xsl/koha-indexdefs-to-zebra.xsl \ etc/zebradb/marc_defs/$i/biblios/biblio-koha-indexdefs.xml \ > etc/zebradb/marc_defs/$i/biblios/biblio-zebra-indexdefs.xsl done $ git status (repeat for authorities, skip normarc which doesn't have authorities) - Apply the patch - Re-run the previous commands => SUCCESS: no changes - Add a condition to an index_subfields tag (for example, condition="@ind2='7'" in the Author's index - Regenerate the specific XSLT => SUCCESS: doing a diff shows the only change is the code has been wrapped inside an xslo:if using the condition for the test - Apply the generated xsl to a MARCXML record that has a field matching the condition like this: $ xsltproc .../biblio-zebra-indexdefs.xsl sample_record.xml => SUCCESS: There's an index on the result, containing the configured field/subfields, that matches the criteria. - Sign off and feel really happy :-D Note: the attached sample record includes a 100 field, with ind2=7 and $a=Tomasito Edit: This patch was squashed once I figured it got too complex and Jonathan required a followup to avoid code duplication. This avoids code duplication, with the same results. Sponsored-by: Orex Digital Signed-off-by: Barton Chittenden <barton@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 42741 [details] [review] Bug 14217: Add 'condition' attribute for DOM index definition This patch introduces an extension to the current syntax for DOM index definition. Specifically, it extends the 'index_subfields' tag to allow adding a 'condition' attribute that is used as a condition ofr applying the specified index. This (exotic) example is self-explanatory: The previous syntax (which is keeped by this patch) took this snippet from biblio-koha-indexdefs.xml <index_subfields tag="100" subfields="acbd"> <target_index>Encuadernador:w</target_index> </index_subfields> and generated an XSLT snippet in the DOM indexing XSLT that looks like this: <xslo:for-each select="marc:subfield"> <xslo:if test="contains('acbd', @code)"> <z:index name="Encuadernador:w"> <xslo:value-of select="."/> </z:index> </xslo:if> </xslo:for-each> This patch introduces this syntax change (note the 'condition' attribute: <index_subfields tag="100" subfields="acbd" condition="@ind2='7'"> <target_index>Encuadernador:w</target_index> </index_subfields> which yields to this XSLT snippet in the DOM indexing XSLT: <xslo:if test="@ind2='7'"> <xslo:for-each select="marc:subfield"> <xslo:if test="contains('acbd', @code)"> <z:index name="Encuadernador:w"> <xslo:value-of select="."/> </z:index> </xslo:if> </xslo:for-each> </xslo:if> To test: - Verify that the shipped XSLT files are current regarding the shipped index definitions: $ for i in marc21 normarc unimarc; do xsltproc etc/zebradb/xsl/koha-indexdefs-to-zebra.xsl \ etc/zebradb/marc_defs/$i/biblios/biblio-koha-indexdefs.xml \ > etc/zebradb/marc_defs/$i/biblios/biblio-zebra-indexdefs.xsl done $ git status (repeat for authorities, skip normarc which doesn't have authorities) - Apply the patch - Re-run the previous commands => SUCCESS: no changes - Add a condition to an index_subfields tag (for example, condition="@ind2='7'" in the Author's index - Regenerate the specific XSLT => SUCCESS: doing a diff shows the only change is the code has been wrapped inside an xslo:if using the condition for the test - Apply the generated xsl to a MARCXML record that has a field matching the condition like this: $ xsltproc .../biblio-zebra-indexdefs.xsl sample_record.xml => SUCCESS: There's an index on the result, containing the configured field/subfields, that matches the criteria. - Sign off and feel really happy :-D Note: the attached sample record includes a 100 field, with ind2=7 and $a=Tomasito Edit: This patch was squashed once I figured it got too complex and Jonathan required a followup to avoid code duplication. This avoids code duplication, with the same results. Sponsored-by: Orex Digital Signed-off-by: Barton Chittenden <barton@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Ooooh. I really like this. It might be useful for "Private" notes as well. Example: http://www.loc.gov/marc/bibliographic/bd541.html First Indicator Privacy # - No information provided 0 - Private 1 - Not private Of course, in this case, it means it wouldn't be indexed for both the OPAC and the Staff Client... but I suppose that's a choice that could be made at the library level when tweaking their indexing.
Patch pushed to master.