I would like to try to revive this. The merge issues don't seem very complicated. However, I'm not sure what I should be seeing if the patch is working. Could you attach some screenshots?

*** Bug 22708 has been marked as a duplicate of this bug. ***

Created attachment 91885 [details] [review]
Bug 8732: Allowing biblio level itemtypes to display instead of MARC ones

This patchset adds a new syspref: BiblioItemtypeImages

If you set BiblioItemtypeImage to Koha, you will see the Koha
itemtypes, if it is Control you will get the ones generated from
the MARC control fields.

The control field images are formed in XSLT and use label 'Material type'

When switching to 'Koha' the images are added in the tmeplate and labeled 'Item type' if labelled

TODO: Image size cleanup - adjust UNIMARC xslt

To test:
 1 - Apply patch
 2 - Update database
 3 - Do an opac search and view results
 4 - Switch pref to 'Koha' and refresh
 5 - Note itemtypes appear and material types dissappear
 6 - Click in to a record details
 7 - Note you see itemtype and icon
 8 - Switch the pref
 9 - Note you now see material type and icon
10 - Repeat in staff interface

This patch sounds wonderful and would specifically be helpful as we try to make  large print titles more distinguishable in our OPAC for our seniors and other patrons who enjoy the large print collection. Thank you to everyone who is contributing to this patch!

Created attachment 92383 [details] [review]
Bug 8732: Allowing biblio level itemtypes to display instead of MARC ones

This patchset adds a new syspref: BiblioItemtypeImages

If you set BiblioItemtypeImage to Koha, you will see the Koha
itemtypes, if it is Control you will get the ones generated from
the MARC control fields.

The control field images are formed in XSLT and use label 'Material type'

When switching to 'Koha' the images are added in the tmeplate and labeled 'Item type' if labelled

TODO: Image size cleanup - adjust UNIMARC xslt

To test:
 1 - Apply patch
 2 - Update database
 3 - Do an opac search and view results
 4 - Switch pref to 'Koha' and refresh
 5 - Note itemtypes appear and material types dissappear
 6 - Click in to a record details
 7 - Note you see itemtype and icon
 8 - Switch the pref
 9 - Note you now see material type and icon
10 - Repeat in staff interface

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed off on behalf of Karen Raborn, kraborn@nrhtx.com

1) QA tools fail: 
 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
   FAIL	  filters
		missing_filter at line 111 (            [% description %])
		missing_filter at line 113 (            [% itemtype %])

2) It looks like this is only implemented for MARC21 (XSLT) - can we please add a note to the system preference description to document that?

3) While you are editing that file... could we shorten the options in the pull down a bit by putting the shared part into the text? Then the pull down would only be: 'Koha biblio level itemtype' and 'MARC control fields'.

4) When there is no itemtype image set, there is nothing displayed. Might be intended behaviour - so just noting. (no blocker)

Created attachment 103488 [details] [review]
Bug 8732: Allowing biblio level itemtypes to display instead of MARC ones

This patchset adds a new syspref: BiblioItemtypeImages

If you set BiblioItemtypeImage to Koha, you will see the Koha
itemtypes, if it is Control you will get the ones generated from
the MARC control fields.

The control field images are formed in XSLT and use label 'Material type'

When switching to 'Koha' the images are added in the tmeplate and labeled 'Item type' if labelled

TODO: Image size cleanup - adjust UNIMARC xslt

To test:
 1 - Apply patch
 2 - Update database
 3 - Do an opac search and view results
 4 - Switch pref to 'Koha' and refresh
 5 - Note itemtypes appear and material types dissappear
 6 - Click in to a record details
 7 - Note you see itemtype and icon
 8 - Switch the pref
 9 - Note you now see material type and icon
10 - Repeat in staff interface

Created attachment 103548 [details] [review]
Bug 8732: Allowing biblio level itemtypes to display instead of MARC ones

This patchset adds a new syspref: BiblioItemtypeImages

If you set BiblioItemtypeImage to Koha, you will see the Koha
itemtypes, if it is Control you will get the ones generated from
the MARC control fields.

The control field images are formed in XSLT and use label 'Material type'

When switching to 'Koha' the images are added in the tmeplate and labeled 'Item type' if labelled

TODO: Image size cleanup - adjust UNIMARC xslt

To test:
 1 - Apply patch
 2 - Update database
 3 - Do an opac search and view results
 4 - Switch pref to 'Koha' and refresh
 5 - Note itemtypes appear and material types dissappear
 6 - Click in to a record details
 7 - Note you see itemtype and icon
 8 - Switch the pref
 9 - Note you now see material type and icon
10 - Repeat in staff interface

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Comment on attachment 103548 [details] [review]
Bug 8732: Allowing biblio level itemtypes to display instead of MARC ones

Review of attachment 103548 [details] [review]:
-----------------------------------------------------------------

This patch shows it's age a little, but let's work it out!

::: catalogue/detail.pl
@@ +588,4 @@
>  
>  $template->param (basketsorders => \@baskets_orders);
>  $template->param (basketsdeletedorders => \@baskets_deletedorders);
> +$template->{VARS}->{'BiblioItemtypeImage'} = C4::Context->preference('BiblioItemtypeImage');

1) This can be removed in favor us using the TT plugin in the template instead.

::: installer/data/mysql/atomicupdate/bug_8732_add_BiblioItemtypeImage_system_preference.perl
@@ +1,3 @@
> +$DBversion = 'XXX'; # will be replaced by the RM
> +if( CheckVersion( $DBversion ) ) {
> +    $dbh->do( "INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('BiblioItemtypeImage', 'Control','Control what biblio level itemtype image displays','Control|Koha','Choice')" );

2) Please switch to INSERT IGNORE INTO...

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/searching.pref
@@ +275,5 @@
> +            - pref: BiblioItemtypeImage
> +              choices:
> +                  Control: "MARC control fields"
> +                  Koha: "Koha biblio level itemtype"
> +            - "This preference only affects MARC21 XSLT views.

3) (pref description in yml file):

Currently reads:
Display itemtype information from the [MARC control fields|Koha biblio level itemtype] This preference only affects MARC21 XSLT views.

I wonder if we could improve the description a little. 

a) In current version the icon this changes is labelled 'material type', so it might not be so easy to locate where and what happens here.
b) We've been trying to remove biblio in favor of bibliographic
c) missing punctuation :)

Suggestion:

Display [material type icon derived from MARC control fields|Koha record level item type icon] on detail and results pages in the OPAC. Only works for MARC21.

(We are in the process of removing non-XSLT this release)

But actually, I'd much prefer a more radical approach in merging this new pref with the existing DisplayIconsXSLT and DisplayOpacIconsXSLT prefs:

[Show|Don't show] the format, audience, and material type icons and descriptions in XSLT MARC21 results and detail pages in the OPAC. 

Show [
no format, audience and material type information|
the record level item type icon|
format, audience and material type information derived from MARC control fields
]
in XSLT MARC21 results and detail pages in the OPAC.

This way, we'd not need a new pref and we could avoid the conflict of different settings (turn off display and turn on itemtype icon display = still nothing shows) And we'd have this controlled separately for OPAC and staff interface. 

But then we also got OpacNoItemTypeImages... just wondering how to get these all to play together in a way that is most clear to end users about the effects!

::: koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl
@@ +171,5 @@
>              <xsl:with-param name="UseAuthoritiesForTracings" select="$UseAuthoritiesForTracings"/>
>          </xsl:call-template>
>  
> +        <xsl:if test="$BiblioItemtypeImage='Control'">
> +        <xsl:if test="$DisplayIconsXSLT!='0' and $materialTypeCode!=''">

4) Full quote:

<xsl:if test="$BiblioItemtypeImage='Control'"> <xsl:if <test="$DisplayIconsXSLT!='0' and $materialTypeCode!=''">

I think this highlights a conceptual issue as we already have a pref dealing with the display of this information (DisplayIconsXSLT and DisplayOpacIconsXSLT - trying to explain in 3)

Also there appears to be no code for the actual icon display?

@@ +181,2 @@
>  
>      <xsl:call-template name="show-lang-041"/>

5) I believe there is a merge/rebase error here - the xsl:if should be before the 041 display.

::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
@@ -134,5 @@
> -                            [% ELSE %]
> -                                [% itemtype | html %]
> -                            [% END %]
> -                            ] [% END %]
> -                            [% IF ( unititle ) %], [% unititle | html %][% END %]

6) Is this removed intentionally?

@@ -246,4 @@
>                              <span class="results_summary year"><span class="label">Year: </span>[% copyrightdate | html %]</span>
>                          [% END %]
>  
> -                        [% UNLESS ( item_level_itypes ) %]

Similar here (see above).

::: opac/opac-detail.pl
@@ +1189,4 @@
>      @serialcollections > 0 
>          ? 'serialcollection' : 'subscriptions';
>  $template->param('defaulttab' => $defaulttab);
> +$template->{VARS}->{'BiblioItemtypeImage'} = C4::Context->preference('BiblioItemtypeImage');

See above!

::: opac/opac-search.pl
@@ +1024,4 @@
>  $template->{VARS}->{DidYouMean} =
>    ( defined C4::Context->preference('OPACdidyoumean')
>        && C4::Context->preference('OPACdidyoumean') =~ m/enable/ );
> +$template->{VARS}->{'BiblioItemtypeImage'} = C4::Context->preference('BiblioItemtypeImage');

See above!

This is rebuilt after Katrin's comment

It no longer touches XSLT, only the templates. XSLT display is controlled by existing prefs, the new pref only controls whether record level info is shown

Created attachment 106836 [details] [review]
Bug 8732: Allowing biblio level itemtypes to display instead of MARC ones

This patchset adds a new syspref: BiblioItemtypeInfo

If you set BiblioItemtypeInfo to Koha, you will see the Koha
record level itemtype info. This info is also shown if item-level_itypes
is set to biblio

Display of icons are controlled by
noItemTypeImages/OpacnoItemTypeImages

Material type display is controlled by DisplayIconsXSLT/DisplayOPACIconsXSLT

To test:
 1 - Apply patch
 2 - Update database
 3 - Do an opac search and view results
 4 - Switch pref to 'Koha' and refresh
 5 - Note itemtypes appear
 6 - Click in to a record details
 7 - Note you see itemtype and icon
 8 - Switch the pref
 9 - Note you don't see record level itemtype and icon
10 - Repeat in staff interface

Created attachment 106848 [details] [review]
Bug 8732: Allowing biblio level itemtypes to display instead of MARC ones

This patchset adds a new syspref: BiblioItemtypeInfo

If you set BiblioItemtypeInfo to Koha, you will see the Koha
record level itemtype info. This info is also shown if item-level_itypes
is set to biblio

Display of icons are controlled by
noItemTypeImages/OpacnoItemTypeImages

Material type display is controlled by DisplayIconsXSLT/DisplayOPACIconsXSLT

To test:
 1 - Apply patch
 2 - Update database
 3 - Do an opac search and view results
 4 - Switch pref to 'Koha' and refresh
 5 - Note itemtypes appear
 6 - Click in to a record details
 7 - Note you see itemtype and icon
 8 - Switch the pref
 9 - Note you don't see record level itemtype and icon
10 - Repeat in staff interface

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Created attachment 106948 [details] [review]
Bug 8732: Update statement

Hi Nick, 

2 issues:

1) There are still some XSLT changes in the patch set that look like remnants from the rewrite and unintentional - can you please check?

2) Patch doesn't apply...

Created attachment 107705 [details] [review]
Bug 8732: Allowing biblio level itemtypes to display instead of MARC ones

This patchset adds a new syspref: BiblioItemtypeInfo

If you set BiblioItemtypeInfo to Koha, you will see the Koha
record level itemtype info. This info is also shown if item-level_itypes
is set to biblio

Display of icons are controlled by
noItemTypeImages/OpacnoItemTypeImages

Material type display is controlled by DisplayIconsXSLT/DisplayOPACIconsXSLT

To test:
 1 - Apply patch
 2 - Update database
 3 - Do an opac search and view results
 4 - Switch pref to 'Koha' and refresh
 5 - Note itemtypes appear
 6 - Click in to a record details
 7 - Note you see itemtype and icon
 8 - Switch the pref
 9 - Note you don't see record level itemtype and icon
10 - Repeat in staff interface

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Created attachment 107706 [details] [review]
Bug 8732: Update statement

Created attachment 107707 [details] [review]
Bug 8732: undo XSLT changes

Created attachment 107709 [details] [review]
Bug 8732: undo XSLT changes

Created attachment 108322 [details] [review]
Bug 8732: (QA follow-up) Fix capitalization and terminology on BiblioItemtypeInfo description

- opac = OPAC
- add "system preference" to make context easier to understand

I really like the simplification here compared to the initial patches.
I see that the opac-detail page is changed (code removed), but the icon display doesn't work there.

Can you please check?

Created attachment 108441 [details] [review]
Bug 8732: (follow-up) Make icon show on opac details and correct pref

Created attachment 108764 [details] [review]
Bug 8732: (QA follow-up) Terminology - staff client to staff interface

Created attachment 108765 [details] [review]
Bug 8732: Allowing biblio level itemtypes to display instead of MARC ones

This patchset adds a new syspref: BiblioItemtypeInfo

If you set BiblioItemtypeInfo to Koha, you will see the Koha
record level itemtype info. This info is also shown if item-level_itypes
is set to biblio

Display of icons are controlled by
noItemTypeImages/OpacnoItemTypeImages

Material type display is controlled by DisplayIconsXSLT/DisplayOPACIconsXSLT

To test:
 1 - Apply patch
 2 - Update database
 3 - Do an opac search and view results
 4 - Switch pref to 'Koha' and refresh
 5 - Note itemtypes appear
 6 - Click in to a record details
 7 - Note you see itemtype and icon
 8 - Switch the pref
 9 - Note you don't see record level itemtype and icon
10 - Repeat in staff interface

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

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

Created attachment 108766 [details] [review]
Bug 8732: Update statement

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

Created attachment 108767 [details] [review]
Bug 8732: undo XSLT changes

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

Created attachment 108768 [details] [review]
Bug 8732: (QA follow-up) Fix capitalization and terminology on BiblioItemtypeInfo description

- opac = OPAC
- add "system preference" to make context easier to understand

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

Created attachment 108769 [details] [review]
Bug 8732: (follow-up) Make icon show on opac details and correct pref

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

Created attachment 108770 [details] [review]
Bug 8732: (QA follow-up) Terminology - staff client to staff interface

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

I am passing this, but we do have a CSS issue in the OPAC result list with and without these patches.

I've filed bug 26263.

Pushed to master for 20.11, thanks to everybody involved!

enhancement will not be backported to 20.05.x

Preference 'BiblioItemtypeInfo' should say it depends on preference 'OpacNoItemTypeImages' right ?

(In reply to Fridolin Somers from comment #56)
> Preference 'BiblioItemtypeInfo' should say it depends on preference
> 'OpacNoItemTypeImages' right ?

I don't think so. This controls whether we display information about the record itemtype (description or code) the other controls if the images are displayed or not.

(In reply to Nick Clemens from comment #57)
> (In reply to Fridolin Somers from comment #56)
> > Preference 'BiblioItemtypeInfo' should say it depends on preference
> > 'OpacNoItemTypeImages' right ?
> 
> I don't think so. This controls whether we display information about the
> record itemtype (description or code) the other controls if the images are
> displayed or not.

Ah ok thanks :D