|
Description
Jared Camins-Esakov
2012-10-19 17:10:32 UTC
Created attachment 20653 [details] [review] Bug 8948: MARC21 field 787 doesn't display Applying: Bug 8948: MARC21 field 787 doesn't display Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl M koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACDetail.xsl Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACDetail.xsl CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACDetail.xsl Auto-merging koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl Patch failed at 0001 Bug 8948: MARC21 field 787 doesn't display The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Created attachment 26946 [details] [review] Bug 8948: MARC21 field 787 doesn't display Need to check my attempted rebase because of the </xsl:element> tag. Not obsoleting patch which does not apply. What do you think, Jared? (In reply to M. Tompsett from comment #4) > What do you think, Jared? I think I don't know what the question is. I was wondering if you think I positioned the </xsl:element> correctly. If so, could you change this to needs sign off? If this doesn't work, the tester can change the status to "Failed QA." I'm not sure how to test. Any test plan? The logic in this patch makes little sense, and is invalid XSL in some cases. In regards to the XSL, there are two <xsl:otherwise> elements in the <xsl:choose> element. There should be only one as it is the default "otherwise" condition. The first "otherwise" condition in these XSLTs is useless, so I'd just delete it all together. The test for "$UseControlNumber = '0'" is counter-productive in that it functions as a de facto "otherwise" which means the test for "marc:subfield[@code='0']" will never be triggered. The final "otherwise" is also useless as it is missing its href attribute. The tests for "marc:subfield[@code='t']" are problematically placed in almost every case well. Rather than salvage this patch, I would suggest starting again with the existing 773 as a model. I added the 787 to our local XSLT a while ago. Perhaps I'll add an alternative patch when I work on porting some other customizations. Still valid? (In reply to Marc Véron from comment #10) > Still valid? Not sure. I haven't looked in a while. A quick look at ./koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl and ./koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACDetail.xsl suggests that this is still valid. I mean that the 787 is missing from the detail page. As for the patch, I imagine my comments from the past are also still valid, but I haven't reviewed the patch recently. Still valid. It would be great to see 787 added to the details page for both OPAC and intranet. We have needed to customise the xslt files to add this for several libraries. We are beginning to use this field to record instrument relationships. The idea behind is that a catalog should be capable to show the network of relationships between people, organizations, things, ideas, etc. But MARC21 reserves agency only to persons and organizations. In a lot of fields (science, art, etc.) instruments, tools, recording devices and other technical individuals have strong agencies and it should be possible to relate them to a resource. For example, we are starting to link publications and archival resources (photographs, logbooks, notebooks, etc.) to the telescopes that were used in their creation. For the telescopes we are using uniform title authority records. Publications, logbooks and data recording notebooks will receive links to the authority records. This practice could also be useful to link recording devices for older recordings in addition to notes (5XX fields). In a future maybe we can have a more inclusive, simple but also granular data model. But for now we are stuck with MARC and should be able to get the best out of this situation. Created attachment 142781 [details] [review] Bug 8948: Display MARC 787 relationship field This field is defined similar to 775, as a related work: https://www.loc.gov/marc/bibliographic/bd787.html This patch adds a display of this field, linking to the referenced record as the 775 does It also tests for 580 field, and displays the text there if present, using the 787 a and t subfields otherwise To test: 1 - Edit the default framework to make 787atw and 580a visible in editor 2 - Add a 787 to a record: $aThe ideas behind the chess openings $w2470231 3 - Confirm there is no display 4 - Apply patch / restart all 5 - 'Other related works' now shows 6 - Link should be using title of record 7 - Set system preference UseControlNumber to Use 8 - Reload details, link should now use control number 9 - Add a 580 to the record: $a This is a friendly work to this one 10 - Confirm the text of the link now displays the text etnered above Current status: Failed QA Created attachment 142782 [details] [review] Bug 8948: Display MARC 787 relationship field This field is defined similar to 775, as a related work: https://www.loc.gov/marc/bibliographic/bd787.html This patch adds a display of this field, linking to the referenced record as the 775 does It also tests for 580 field, and displays the text there if present, using the 787 a and t subfields otherwise To test: 1 - Edit the default framework to make 787atw and 580a visible in editor 2 - Add a 787 to a record: $aThe ideas behind the chess openings $w2470231 3 - Confirm there is no display 4 - Apply patch / restart all 5 - 'Other related works' now shows 6 - Link should be using title of record 7 - Set system preference UseControlNumber to Use 8 - Reload details, link should now use control number 9 - Add a 580 to the record: $a This is a friendly work to this one 10 - Confirm the text of the link now displays the text etnered above Created attachment 142791 [details] [review] Bug 8948: Display MARC 787 relationship field This field is defined similar to 775, as a related work: https://www.loc.gov/marc/bibliographic/bd787.html This patch adds a display of this field, linking to the referenced record as the 775 does It also tests for 580 field, and displays the text there if present, using the 787 a and t subfields otherwise To test: 1 - Edit the default framework to make 787atw and 580a visible in editor 2 - Add a 787 to a record: $aThe ideas behind the chess openings $w2470231 3 - Confirm there is no display 4 - Apply patch / restart all 5 - 'Other related works' now shows 6 - Link should be using title of record 7 - Set system preference UseControlNumber to Use 8 - Reload details, link should now use control number 9 - Add a 580 to the record: $a This is a friendly work to this one 10 - Confirm the text of the link now displays the text etnered above Signed-off-by: David Nind <david@davidnind.com> Have signed off as this works in the staff interface. Are there any plans to add this to the OPAC as well? (see comment #14). This looks pretty good and happy to see this tackled, but I have some comments: 1) This copies an old mistake we made in the beginning by using $a and $t for the title search. This often won't work. We need to split these up ti=t and au=a, like we have done now for 773 and should also correct in other templates: <xsl:attribute name="href">/cgi-bin/koha/catalogue/search.pl?q=ti,phr:<xsl:value-of select="str:encode-uri(translate($f787, '()', ''), true())"/></xsl:attribute> 2) We should add this to OPAC and staff interface at the same time. 3) 580 is an issue. a) There can be mulitple 580 and you only display the first. b) It might belong to a totally different 7xx linking field, creating a false display. https://www.loc.gov/marc/bibliographic/bd580.html Description of the complex relationship between the item described in the record and other items that cannot be adequately generated from the linking entry fields 760-787. I am not sure how this is supposed to be done... show only if it actually directly preceeds the first 787 we are looking at here? (is this even possible?) Show the 580 separately? Leave it off for now and deal with it later? 3) I believe there was some change to the separators between fields in OPAC at least sometime ago, or maybe it got stuck? I was not able to find the bug :( I know in our records there are often a lot of 787 so we changed it to a list with separate lines for each entry. A class for the separator would help a lot here. In patch: + <xsl:choose> + <xsl:when test="position()=last()"></xsl:when> + <xsl:otherwise><xsl:text>; </xsl:text></xsl:otherwise> + </xsl:choose> In other instances in the XSLT file: <xsl:if test="position() != last()"><span class="separator"><xsl:text> | </xsl:text></span></xsl:if> Created attachment 143083 [details] [review] Bug 8948: Display MARC 787 relationship field This field is defined similar to 775, as a related work: https://www.loc.gov/marc/bibliographic/bd787.html This patch adds a display of this field, linking to the referenced record as the 775 does It also tests for 580 field, and displays the text there if present, using the 787 a and t subfields otherwise To test: 1 - Edit the default framework to make 787atw and 580a visible in editor 2 - Add a 787 to a record: $tThe ideas behind the chess openings $aFine, Reuben< $w2470231 3 - Confirm there is no display 4 - Apply patch / restart all 5 - 'Other related works' now shows 6 - Link should be using title of record 7 - Set system preference UseControlNumber to Use 8 - Reload details, link should now use control number 9 - Add a 580 to the record: $a This is a friendly work to this one 10 - Confirm the text of the link now displays before the link 11 - Add a second 580, confirm it displays as well - there is no seperator as each should end with a period or other punctuation according to MARC - there is a dpan with a class surrounding each so styling can be added as desired 12 - Add a second 787 - confirm seperator present and wrapped in a span with a class - confirm each 787 is shown with a span/class for styling Created attachment 143084 [details] [review] Bug 8948: Changes for OPAC Test with same record, confirm OPAC view is as expected The link for the OPAC results in a 404 error. The link should start with ../cgi-bin/koha/opac-search.pl?.. instead of ../cgi-bin/koha/catalogue/search.pl?.. ? Created attachment 143111 [details] [review] Bug 8948 (follow-up) Correct OPAC link Created attachment 143141 [details] [review] Bug 8948: Display MARC 787 relationship field This field is defined similar to 775, as a related work: https://www.loc.gov/marc/bibliographic/bd787.html This patch adds a display of this field, linking to the referenced record as the 775 does It also tests for 580 field, and displays the text there if present, using the 787 a and t subfields otherwise To test: 1 - Edit the default framework to make 787atw and 580a visible in editor 2 - Add a 787 to a record: $tThe ideas behind the chess openings $aFine, Reuben< $w2470231 3 - Confirm there is no display 4 - Apply patch / restart all 5 - 'Other related works' now shows 6 - Link should be using title of record 7 - Set system preference UseControlNumber to Use 8 - Reload details, link should now use control number 9 - Add a 580 to the record: $a This is a friendly work to this one 10 - Confirm the text of the link now displays before the link 11 - Add a second 580, confirm it displays as well - there is no seperator as each should end with a period or other punctuation according to MARC - there is a dpan with a class surrounding each so styling can be added as desired 12 - Add a second 787 - confirm seperator present and wrapped in a span with a class - confirm each 787 is shown with a span/class for styling Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 143142 [details] [review] Bug 8948: Changes for OPAC Test with same record, confirm OPAC view is as expected Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 143143 [details] [review] Bug 8948 (follow-up) Correct OPAC link Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 143562 [details] [review] Bug 8948: Display MARC 787 relationship field This field is defined similar to 775, as a related work: https://www.loc.gov/marc/bibliographic/bd787.html This patch adds a display of this field, linking to the referenced record as the 775 does It also tests for 580 field, and displays the text there if present, using the 787 a and t subfields otherwise To test: 1 - Edit the default framework to make 787atw and 580a visible in editor 2 - Add a 787 to a record: $tThe ideas behind the chess openings $aFine, Reuben< $w2470231 3 - Confirm there is no display 4 - Apply patch / restart all 5 - 'Other related works' now shows 6 - Link should be using title of record 7 - Set system preference UseControlNumber to Use 8 - Reload details, link should now use control number 9 - Add a 580 to the record: $a This is a friendly work to this one 10 - Confirm the text of the link now displays before the link 11 - Add a second 580, confirm it displays as well - there is no seperator as each should end with a period or other punctuation according to MARC - there is a dpan with a class surrounding each so styling can be added as desired 12 - Add a second 787 - confirm seperator present and wrapped in a span with a class - confirm each 787 is shown with a span/class for styling Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 143563 [details] [review] Bug 8948: Changes for OPAC Test with same record, confirm OPAC view is as expected Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 143564 [details] [review] Bug 8948: (follow-up) Correct OPAC link Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pushed to master for 22.11. Nice work everyone, thanks! This wasn't PQA for a reason. Displaying 580 in context of 787 is not correct, I'd even say it can lead to things looking quite wrong. Please see comment#20. (In reply to Katrin Fischer from comment #32) > This wasn't PQA for a reason. Displaying 580 in context of 787 is not > correct, I'd even say it can lead to things looking quite wrong. Please see > comment#20. We should remove 580 display for the time being. (In reply to Katrin Fischer from comment #33) > (In reply to Katrin Fischer from comment #32) > > This wasn't PQA for a reason. Displaying 580 in context of 787 is not > > correct, I'd even say it can lead to things looking quite wrong. Please see > > comment#20. > > We should remove 580 display for the time being. I did move the 580, they display after the 787 While they could mismatch, in general the documentation makes it seem it will be clear which note relates to which link. The 580 can now also be easily styled Hm, but: + <!-- 787 Other Relationship Entry --> + <xsl:if test="marc:datafield[@tag=787]"> + <span class="results_summary other_relationship_entry"><span class="label">Other related works: </span> + <xsl:for-each select="marc:datafield[@tag=580]"> + <span class="linking_complexity_note"> + <xsl:call-template name="subfieldSelect"> + <xsl:with-param name="codes">a</xsl:with-param> + </xsl:call-template> + <xsl:text> </xsl:text> + </span> + </xsl:for-each> You only display the 580 of a 787 exists and it displays after the "Other related works" which is the 787 label. The problem is: 580 might belong to ANY 7xx field, not just the 787. In the documentation there are examples for 773 and 785 respectively. The problem is not if there are mulitple 787, but if we give a totally wrong context here. Created attachment 143642 [details] [review] Bug 8948: (follow-up) Remove 580 display from 787 Katrin, does the follow-up answer your concerns? My mistake, I hadn't pushed this. Waiting for Katrin to answer. Created attachment 143684 [details] [review] Bug 8948: Display MARC 787 relationship field This field is defined similar to 775, as a related work: https://www.loc.gov/marc/bibliographic/bd787.html This patch adds a display of this field, linking to the referenced record as the 775 does It also tests for 580 field, and displays the text there if present, using the 787 a and t subfields otherwise To test: 1 - Edit the default framework to make 787atw and 580a visible in editor 2 - Add a 787 to a record: $tThe ideas behind the chess openings $aFine, Reuben< $w2470231 3 - Confirm there is no display 4 - Apply patch / restart all 5 - 'Other related works' now shows 6 - Link should be using title of record 7 - Set system preference UseControlNumber to Use 8 - Reload details, link should now use control number 9 - Add a 580 to the record: $a This is a friendly work to this one 10 - Confirm the text of the link now displays before the link 11 - Add a second 580, confirm it displays as well - there is no seperator as each should end with a period or other punctuation according to MARC - there is a dpan with a class surrounding each so styling can be added as desired 12 - Add a second 787 - confirm seperator present and wrapped in a span with a class - confirm each 787 is shown with a span/class for styling Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 143685 [details] [review] Bug 8948: Changes for OPAC Test with same record, confirm OPAC view is as expected Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 143686 [details] [review] Bug 8948: (follow-up) Correct OPAC link Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 143687 [details] [review] Bug 8948: (follow-up) Remove 580 display from 787 Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Thanks for your patience and the follow-up, Nick :) Pushed to master for 22.11. Nice work everyone, thanks! Marking as a bug - this field should be displayed - I think this is eligible for backport Backported to 22.05.x for upcoming 22.05.10 applied to 21.11.x for 21.11.16 Not backported to 21.05.x In 21.11.x it has an unexpected side effect (commit 21ea11ff49a8564c09cb310fc77b471265722cfc), "OPAC view: " is displayed twice. Could you report that as a new bug please, I think it's got lost at the end of this resolved older report. |