Bug 14377

Summary: Indicate that a record is suppressed in staff client
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: TemplatesAssignee: Winona Salesky <wsalesky>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: amy, f.demians, gaetan.boisson, jonathan.druart, jschmidt, katrin.fischer, kholten, kyle.m.hall, kyle, m.de.rooy, magnus, nick, nicole, veron
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 20846, 16380, 29406    
Attachments: Bug 14377: This development will show 'Suppressed in OPAC' if the 942$n is set to 1.
Bug 14377: This development will show 'Suppressed in OPAC' if the 942$n is set to 1. Changed id="suppressed_opac" to class="suppressed_opac".
Bug #14377 Show "BIBNO - Suppressed in OPAC" for records with 942$n=1.
Bug #14377 Show "BIBNO - Suppressed in OPAC" for records with 942$n=1.
Bug #14377 Show "BIBNO - Suppressed in OPAC" for records with 942$n=1.
Bug 14377 [QA Followup] - Use true value description for authorised value
Bug 14377 [QA Followup] - Use true value description for authorised value
Bug 14377 [QA Followup] - Remove biblionumber, move message to below title for search results
Bug 14377 [QA Followup] - Add support for NORMARC
Bug 14377 [QA Followup] - Don't do 942$n processing for UNIMARC
Bug #14377 Show "BIBNO - Suppressed in OPAC" for records with 942$n=1.
Bug 14377 [QA Followup] - Use true value description for authorised value
Bug 14377 [QA Followup] - Remove biblionumber, move message to below title for search results
Bug 14377 [QA Followup] - Add support for NORMARC
Bug 14377 [QA Followup] - Don't do 942$n processing for UNIMARC
[PASSED QA] Bug #14377 Show "BIBNO - Suppressed in OPAC" for records with 942$n=1.
[PASSED QA] Bug 14377 [QA Followup] - Use true value description for authorised value
[PASSED QA] Bug 14377 [QA Followup] - Remove biblionumber, move message to below title for search results
[PASSED QA] Bug 14377 [QA Followup] - Add support for NORMARC
[PASSED QA] Bug 14377 [QA Followup] - Don't do 942$n processing for UNIMARC
Bug 14377 - Don't replace 942$n with authorised value except when using UNIMARC
Bug 14377 - Don't replace 942$n with authorised value except when using UNIMARC
Bug #14377 Show "BIBNO - Suppressed in OPAC" for records with 942$n=1.
Bug 14377 - Don't replace 942$n with authorised value except when using UNIMARC
Bug 14377 [QA Followup] - Remove biblionumber from messages
Bug 14377 [QA Followup] - Fix conditional
Bug 14377 [QA Followup] - Fix conditional
Bug 14377 [QA Followup] - Fix conditional
Bug #14377 Show "BIBNO - Suppressed in OPAC" for records with 942$n=1.
Bug 14377 - Don't replace 942$n with authorised value except when using UNIMARC
Bug 14377 [QA Followup] - Remove biblionumber from messages
Bug 14377 [QA Followup] - Fix conditional
Bug 14377: [QA Follow-up] Correct typo and comment
Bug 14377 [QA Followup] - Add supression message to NORMARC XSLT

Description Nicole C. Engard 2015-06-11 16:32:56 UTC
Would be nice if in the staff client there was some indication that a record you are viewing is suppressed in the OPAC.  This would be an XSLT customization.
Comment 1 Winona Salesky 2015-09-07 01:44:27 UTC Comment hidden (obsolete)
Comment 2 Winona Salesky 2015-09-07 01:46:13 UTC
942 is wrapped with id="suppressed_opac" to make individual styling possible. 


(In reply to Winona Salesky from comment #1)
> Created attachment 42424 [details] [review] [review]
> Bug 14377: This development will show 'Suppressed in OPAC' if the 942$n is
> set to 1.
> 
> Test Plan:
> 1) Apply this patch
> 2) Ensure you are using the default XSLT setting for the staff view.
> 3) Find or create a record with MARC tags 942$n=1
> 4) Check for “Suppressed in OPAC” at the top of the results div and at the
> top of the full record display.
Comment 3 Katrin Fischer 2015-09-07 07:24:05 UTC
Comment on attachment 42424 [details] [review]
Bug 14377: This development will show 'Suppressed in OPAC' if the 942$n is set to 1.

Hi Winona,

can you please use a class instead of an id for the result lists as the element can show up there repeatedly?
Comment 4 Winona Salesky 2015-09-07 15:36:36 UTC Comment hidden (obsolete)
Comment 5 Winona Salesky 2015-09-18 15:03:43 UTC Comment hidden (obsolete)
Comment 6 Winona Salesky 2015-09-18 15:05:50 UTC
(In reply to Winona Salesky from comment #5)
> Created attachment 42709 [details] [review] [review]
> Bug #14377 Show "BIBNO - Suppressed in OPAC" for records with 942$n=1.
> 
> Includes class="suppressed_opac" for library specific styles.

Test Plan:
1) Apply this patch
2) Ensure you are using the default XSLT setting for the staff view.
3) Find or create a record with MARC tags 942$n=1
4) Check for “Suppressed in OPAC” at the top of the results div and at the top of the full record display.

Recommended custom CSS:
.suppressed_opac {
 color:red; 
 font-size:1.25em;
 font-weight:bold; 
 float:right; 
 margin-right:2em;
 clear:none;
}
Comment 7 Nick Clemens (kidclamp) 2015-10-01 18:29:05 UTC Comment hidden (obsolete)
Comment 8 Nick Clemens (kidclamp) 2015-10-01 18:30:35 UTC
Works as expected.  

Style note: Without recommended CSS the 'Suppressed in Opac' in search results is next to result number and above title/author.  I think the default location should be below these two if possible
Comment 9 Katrin Fischer 2015-10-03 22:56:40 UTC
Hm, this last patch doesn't work for me. I have reindexed and the MARC view shows:

942 _2ddc
_cBK
_n1

  <datafield tag="942" ind1=" " ind2=" ">
    <subfield code="2">ddc</subfield>
    <subfield code="c">BK</subfield>
    <subfield code="n">1</subfield>
  </datafield>

I think the test doesn't work quite right, but I haven't figured out why:
test="marc:datafield[@tag=942][marc:subfield[@code='n'] = '1']"

Can you see, what might go wrong here?

I am also not a fan of showing the bibnumber, I think it's more likely to confuse than being helpful. I'd like it better if we showed only the text.
Comment 10 Winona Salesky 2015-10-04 01:54:46 UTC
Would you mind sending me your marcxml test record to test?
Thanks,
-Winona
wsalesky@gmail.com

(In reply to Katrin Fischer from comment #9)
> Hm, this last patch doesn't work for me. I have reindexed and the MARC view
> shows:
> 
> 942 _2ddc
> _cBK
> _n1
> 
>   <datafield tag="942" ind1=" " ind2=" ">
>     <subfield code="2">ddc</subfield>
>     <subfield code="c">BK</subfield>
>     <subfield code="n">1</subfield>
>   </datafield>
> 
> I think the test doesn't work quite right, but I haven't figured out why:
> test="marc:datafield[@tag=942][marc:subfield[@code='n'] = '1']"
> 
> Can you see, what might go wrong here?
> 
> I am also not a fan of showing the bibnumber, I think it's more likely to
> confuse than being helpful. I'd like it better if we showed only the text.
Comment 11 Winona Salesky 2015-10-05 14:37:23 UTC
I am unable to reproduce this issue. The record you sent me runs fine on my system. Can you give me any additional details about how you are testing this patch? Also, if you are comfortable looking in the xslt could you try changing the test (line 94 on MARC21slim2intranetDetail.xsl) to:

test="marc:datafield[@tag=942][normalize-space(marc:subfield[@code='n']) = '1']

An additional tester may also be helpful. 
Thanks,
-Winona

(In reply to Katrin Fischer from comment #9)
> Hm, this last patch doesn't work for me. I have reindexed and the MARC view
> shows:
> 
> 942 _2ddc
> _cBK
> _n1
> 
>   <datafield tag="942" ind1=" " ind2=" ">
>     <subfield code="2">ddc</subfield>
>     <subfield code="c">BK</subfield>
>     <subfield code="n">1</subfield>
>   </datafield>
> 
> I think the test doesn't work quite right, but I haven't figured out why:
> test="marc:datafield[@tag=942][marc:subfield[@code='n'] = '1']"
> 
> Can you see, what might go wrong here?
> 
> I am also not a fan of showing the bibnumber, I think it's more likely to
> confuse than being helpful. I'd like it better if we showed only the text.
Comment 12 Marc Véron 2015-10-05 19:14:04 UTC
Some testing from my part:

If I put an additional blank after the 1 in 942n, the record is suppressed in OPAC (with syspref 'OpacSuppression set to 'Hide'), but the message does not show up on the staff detail view. 

If I then add the normalize-space function to the XSL as described in comment #11 the message appears.

If I have use the authorized value YesNo, it does not work. I tweaked the xsl to display the 942nvalue, and it is Yes instead of 1

As Katrin mentioned on IRC this is a problem, because the auhorized values are translated.
Comment 13 Marc Véron 2015-10-05 19:40:51 UTC
Workaround and/or possible solution:

I created an authorized value category ONE_ZERO and gave it 0 / 1 for the values and for all descriptions. Then I linked it with 942 n.

Display of suppression message is fine. And it displays 1 or 0 correctly  while editing 942 n for records that formerly had YES_NO values.
Comment 14 Katrin Fischer 2015-10-05 19:47:04 UTC
Hm, I think 0/1 as the description is not very friendly for the cataloger and might lead to some confusion. Another suggestion: We could use the mechanism that makes system preferences available in the XSLT. We could create a variable to store the description that matches the authorised value category linked to 942$n for the value 1. And then base the check on that. Does that make sense? :)
Comment 15 Winona Salesky 2015-10-05 20:00:16 UTC
This would work for me. I'm afraid I am still using learning the ins and outs of Koha, and did not realize there were other possible values to 942$n. 

-Winona



(In reply to Katrin Fischer from comment #14)
> Hm, I think 0/1 as the description is not very friendly for the cataloger
> and might lead to some confusion. Another suggestion: We could use the
> mechanism that makes system preferences available in the XSLT. We could
> create a variable to store the description that matches the authorised value
> category linked to 942$n for the value 1. And then base the check on that.
> Does that make sense? :)
Comment 16 Kyle M Hall 2015-10-07 10:58:25 UTC
(In reply to Katrin Fischer from comment #14)
> Hm, I think 0/1 as the description is not very friendly for the cataloger
> and might lead to some confusion. Another suggestion: We could use the
> mechanism that makes system preferences available in the XSLT. We could
> create a variable to store the description that matches the authorised value
> category linked to 942$n for the value 1. And then base the check on that.
> Does that make sense? :)

I don't think I followed that well ; )

Here's what I'm thinking:
1) Add a new system preference OpacSuppressionValue that defines the value ( 1, the Authorised Value description for 1, default of 1 if not set ). So this value could be 1, Yes, Si, Oui or what have you.
2) Pass this new pref to the XSLT
3) Compare against the pref value rather than just '1'

Is this what you are thinking Katrin?
Comment 17 Katrin Fischer 2015-10-14 13:49:20 UTC
Hi Kyle,

I think we don't need a system preference, because we can find out what the right value is within Koha:

1) Look up, which authorised value is linked to 942$n
2) Look up for the authorised value, which description is used for the value 1

Does this make sense?
Comment 18 Katrin Fischer 2015-10-14 13:52:36 UTC
... and the value you get, you pass into the XSLT using a similar mechanism as for the system preferences.

And later on we can enhance it to also work for mulitple translations, when a similar mechanism as proposed in 14100 is implemented for authorised values.
Comment 19 Karl Holten 2015-10-16 20:30:26 UTC
Hi, we're the institution that requested the bibnumber show up. Our staff do a lot of searching but standard numbers such as the bib record number, OCLC number, barcode, ISSN. It's a more direct way to search in Koha which tends to be very keyword heavy. Staff want to get fewer results, not more or even multiple. Seeing the record number also helps identify it quickly versus the title which can be more generic and/or be one of other similar titles. Anyway, that's the reason we made that request.
Comment 20 Katrin Fischer 2015-10-18 09:16:02 UTC
Hi Karl,

with this patch the bibnumber will only show for suppressed records - that's what didn't make so much sense to me. I am not opposed at all to add the bibnumber to display in general, but then maybe on a separate bug as it doesn't really fit here :)
Comment 21 Kyle M Hall 2015-12-10 12:35:25 UTC Comment hidden (obsolete)
Comment 22 Kyle M Hall 2015-12-10 12:35:32 UTC Comment hidden (obsolete)
Comment 23 Kyle M Hall 2015-12-10 12:37:29 UTC Comment hidden (obsolete)
Comment 24 Kyle M Hall 2015-12-10 13:00:28 UTC Comment hidden (obsolete)
Comment 25 Jonathan Druart 2016-01-19 09:27:15 UTC
This is not marcflavour independent: 942$n is hardcoded, so it won't work for UNIMARC
The XSLT changes have been done for MARC21 only, if NORMARC uses 942$n also, it would be easy to c/p the changes to the according XSLT files.

Does a UNIMARC user want to provide a patch for UNIMARC XSLT?
Comment 26 Kyle M Hall 2016-01-19 15:26:35 UTC Comment hidden (obsolete)
Comment 27 Kyle M Hall 2016-01-19 15:29:54 UTC
(In reply to Jonathan Druart from comment #25)
> This is not marcflavour independent: 942$n is hardcoded, so it won't work
> for UNIMARC
> The XSLT changes have been done for MARC21 only, if NORMARC uses 942$n also,
> it would be easy to c/p the changes to the according XSLT files.
> 
> Does a UNIMARC user want to provide a patch for UNIMARC XSLT?

As far as I can tell, UNIMARC does not have record suppression support! I based this on the fact that I can't find any reference to the Suppress index in marc_defs/unimarc, so unless I'm mistaken, it's a non-issue.
Comment 28 Katrin Fischer 2016-01-19 15:34:15 UTC
I think the 'problem' with UNIMARC was that there was no default standard field chosen. The systempreference description used to say something about adding a suppress index. So that might be more complicated - as we don't know what people have already been using and not sure how hard it would be to pick a new default fiel for that purpose in UNIMARC.
Comment 29 Jonathan Druart 2016-01-19 15:51:35 UTC
I will let a MARC21 user to QA this patchset, but I am pretty sure we need a unless UNIMARC statement in transformMARCXML4XSLT.
Comment 30 Kyle M Hall 2016-01-20 12:40:26 UTC Comment hidden (obsolete)
Comment 31 Magnus Enger 2016-03-04 09:34:04 UTC
Katrin told me to do it! 

<cait> could someone mark 14377 doesn't apply for me?
<cait> fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl).
Comment 32 Jonathan Druart 2016-03-04 09:40:55 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2016-03-04 09:41:03 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2016-03-04 09:41:08 UTC Comment hidden (obsolete)
Comment 35 Jonathan Druart 2016-03-04 09:41:12 UTC Comment hidden (obsolete)
Comment 36 Jonathan Druart 2016-03-04 09:41:17 UTC Comment hidden (obsolete)
Comment 37 Jonathan Druart 2016-03-04 09:41:52 UTC
Rebase, easy conflict with bug 4941 fixed.
Comment 38 Katrin Fischer 2016-03-31 06:16:43 UTC Comment hidden (obsolete)
Comment 39 Katrin Fischer 2016-03-31 06:16:47 UTC Comment hidden (obsolete)
Comment 40 Katrin Fischer 2016-03-31 06:16:51 UTC Comment hidden (obsolete)
Comment 41 Katrin Fischer 2016-03-31 06:16:55 UTC Comment hidden (obsolete)
Comment 42 Katrin Fischer 2016-03-31 06:16:58 UTC Comment hidden (obsolete)
Comment 43 Katrin Fischer 2016-03-31 06:24:39 UTC
I was about to pass this, but then found 2 things:

1) The text will only be true if OpacSuppression is turned on. So I think we will want to add a check for that, maybe here?
 unless ( $marcflavour eq 'UNIMARC' ) {

2) With all the discussion recently - I am a bit worried about performance here. Could it be speedier to send the original value of 942$n (the code) to the template and check for 1/something vs. 0/empty? 
This way we wouldn't have to care about the AV or framework. But not sure if this is an option.

I included a typo fix in the last patch, as the QA tools had complained about it. It's in one of the comments.
Comment 44 Kyle M Hall 2016-04-18 17:29:43 UTC Comment hidden (obsolete)
Comment 45 Kyle M Hall 2016-04-18 17:31:26 UTC Comment hidden (obsolete)
Comment 46 Marcel de Rooy 2016-04-22 07:03:34 UTC
If suppression is not turned on, the text "#bibno - Suppressed in OPAC" does not make sense.
Failed QA
Comment 47 Kyle M Hall 2016-04-26 15:11:25 UTC Comment hidden (obsolete)
Comment 48 Kyle M Hall 2016-04-26 15:11:33 UTC Comment hidden (obsolete)
Comment 49 Kyle M Hall 2016-04-26 15:12:13 UTC
(In reply to Marcel de Rooy from comment #46)
> If suppression is not turned on, the text "#bibno - Suppressed in OPAC" does
> not make sense.
> Failed QA

Patch now only displays message of OpacSuppression is enabled!
Comment 50 Katrin Fischer 2016-04-26 22:27:58 UTC
The condition doesn't seem correct:
$tag ne '942' && $subfield ne 'n' && $marcflavour ne 'UNIMARC'
Wouldn't that be false for a non-UNIMARC installation and something like 999$n or 942$c?
Comment 51 Katrin Fischer 2016-04-26 22:50:31 UTC
Giving it a try: unless (942 and n and (marc21 or normarc) ) ?

After taking another look: I think the # is not translatable here:
<xsl:value-of select="concat('#',marc:datafield[@tag=999]/marc:subfield[@code='c'])"/>

I'd still be happier with generally displaying the bibnumber somewhere and separate it from the OPAC suppression information.
Comment 52 Kyle M Hall 2016-04-27 17:30:24 UTC Comment hidden (obsolete)
Comment 53 Kyle M Hall 2016-04-27 17:38:39 UTC Comment hidden (obsolete)
Comment 54 Kyle M Hall 2016-04-27 17:45:42 UTC Comment hidden (obsolete)
Comment 55 Kyle M Hall 2016-04-27 17:46:25 UTC Comment hidden (obsolete)
Comment 56 Marcel de Rooy 2016-04-28 08:14:50 UTC
QA: Looking here now.
Comment 57 Marcel de Rooy 2016-04-28 09:33:42 UTC
Created attachment 50881 [details] [review]
Bug #14377 Show "BIBNO - Suppressed in OPAC" for records with 942$n=1.

Includes class="suppressed_opac" for library specific styles.
Suggested css: .suppressed_opac { color:red; font-size:1.25em; font-weight:bold; float:right; margin-right:2em; clear:none; }

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 58 Marcel de Rooy 2016-04-28 09:33:48 UTC
Created attachment 50882 [details] [review]
Bug 14377 - Don't replace 942$n with authorised value except when using UNIMARC

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 59 Marcel de Rooy 2016-04-28 09:33:55 UTC
Created attachment 50883 [details] [review]
Bug 14377 [QA Followup] - Remove biblionumber from messages

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 60 Marcel de Rooy 2016-04-28 09:34:02 UTC
Created attachment 50884 [details] [review]
Bug 14377 [QA Followup] - Fix conditional

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 61 Marcel de Rooy 2016-04-28 09:34:07 UTC
Created attachment 50885 [details] [review]
Bug 14377: [QA Follow-up] Correct typo and comment

[1] Remove a vim inserted i from XSLT.pm: IdRefi => IdRef.
[2] The comment about UNIMARC is somewhat confusing. Rephrased it.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 62 Marcel de Rooy 2016-04-28 09:34:46 UTC
QA Comment:
Thanks all involved.
In view of time elapsed and effort, I am passing QA on this patch set.
I am not really happy about the change in transformMARCXML4XSLT. Adding the hardcoded 942n exception there is no price winner.
Also I note that this report does not add these changes to NORMARC XSLT. No blocker, but it would have been better.
Comment 63 Magnus Enger 2016-04-28 15:17:00 UTC
*** Bug 8536 has been marked as a duplicate of this bug. ***
Comment 64 Kyle M Hall 2016-04-29 12:57:35 UTC
Created attachment 50987 [details] [review]
Bug 14377 [QA Followup] - Add supression message to NORMARC XSLT
Comment 65 Kyle M Hall 2016-04-29 12:58:36 UTC
Pushed to master for the May 2016 release. Thanks Winona!