Bug 15485

Summary: Allow choosing different XSLTs for lists
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: ListsAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, kyle.m.hall, kyle, m.de.rooy, marjorie.barry-vila, patrick.robitaille, tomascohen
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:
Attachments: Bug 15485: Introduce new sysprefs for lists XSLTs
Bug 15485: Use lists-specific XSLT sysprefs
Bug 15485: Introduce new sysprefs for lists XSLTs
Bug 15485: Use lists-specific XSLT sysprefs
Bug 15485: Introduce new sysprefs for lists XSLTs
Bug 15485: Use lists-specific XSLT sysprefs
Bug 15485: (QA followup) Fix behaviour and default values
Bug 15485: (QA followup) Simplify logic and use *ResultsDisplay value during upgrade
Bug 15485: Introduce new sysprefs for lists XSLTs
Bug 15485: Use lists-specific XSLT sysprefs
Bug 15485: (QA followup) Fix behaviour and default values
Bug 15485: (QA followup) Simplify logic and use *ResultsDisplay value during upgrade
[SIGNED-OFF]Bug 15485: (QA followup) Fix behaviour and default values
[SIGNED-OFF]Bug 15485: (QA followup) Simplify logic and use *ResultsDisplay value during upgrade

Description Nicole C. Engard 2016-01-06 16:02:56 UTC
Goal of Development: To add 520 field to lists

Work to be done:
1) Split system preference for lists XSLT into a separate system preference from search results XSLT
2) System preference will default to existing default search results XSLT
3) Once the development is in Koha, add custom XSLT file to your server to display the 520 field for lists
Comment 1 Tomás Cohen Arazi 2016-06-08 03:15:05 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2016-06-08 03:15:13 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2016-06-13 12:57:02 UTC Comment hidden (obsolete)
Comment 4 Nick Clemens 2016-06-13 12:57:10 UTC Comment hidden (obsolete)
Comment 5 Nicole C. Engard 2016-06-13 15:11:48 UTC Comment hidden (obsolete)
Comment 6 Nicole C. Engard 2016-06-13 15:11:54 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-06-14 10:27:16 UTC
Comment on attachment 52321 [details] [review]
Bug 15485: Use lists-specific XSLT sysprefs

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

::: opac/opac-shelves.pl
@@ +248,4 @@
>  
>              my $borrower = GetMember( borrowernumber => $loggedinuser );
>  
> +            my $xslfile = C4::Context->preference('OPACXSLTListsDisplay');

it seems that || 'default' is missing here, could you confirm?
Comment 8 Jonathan Druart 2016-06-14 15:36:54 UTC
(In reply to Jonathan Druart from comment #7)
> Comment on attachment 52321 [details] [review] [review]
> Bug 15485: Use lists-specific XSLT sysprefs
> 
> Review of attachment 52321 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: opac/opac-shelves.pl
> @@ +248,4 @@
> >  
> >              my $borrower = GetMember( borrowernumber => $loggedinuser );
> >  
> > +            my $xslfile = C4::Context->preference('OPACXSLTListsDisplay');
> 
> it seems that || 'default' is missing here, could you confirm?

Actually no, it's not. But the other one should not default to 'default' (...), otherwise the non-XSLT view will never be used.
Comment 9 Tomás Cohen Arazi 2016-06-14 16:22:34 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2016-06-19 14:31:38 UTC
Tomas,
I'd prefer to insert the pref with the value of XSLT used for results and default in XSLTParse4Display.
We could use "default" as others.
What do you think about that?
Comment 11 Tomás Cohen Arazi 2016-06-22 17:57:45 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2016-06-23 15:01:50 UTC
Created attachment 52738 [details] [review]
Bug 15485: Introduce new sysprefs for lists XSLTs

This patch introduces two new sysprefs, allowing the users to
set custom XSLTs for lists display. This is done almost the usual
way.

If none is specified (value='') it defaults to the current behaviour:
using the *Results.xsl file for the corresponding interface.

Sponsored-by: Carnegie Stout Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Deb Stephenson <DStephen@dubuque.lib.ia.us>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2016-06-23 15:02:12 UTC
Created attachment 52739 [details] [review]
Bug 15485: Use lists-specific XSLT sysprefs

This patch makes the shelves.pl (staff) and opac-shelves.pl scripts
use the new sysprefs for specifying custom XSLTs for lists display.

XSLT.pm is patched so it defaults to the corresponding *Results.xsl
files if none is specified.

To test:
- Create a list
- Open the list in the staff interface
- On a new tab, open the list in the OPAC.
- Apply this patches
=== default behaviour
- Open the list (both opac and staff) on new tabs
=> SUCCESS: They look exactly the same (hint: the syspref is set to ''
   so it should fallback to using the one we were using.
=== using the new functionality
- Create custom XSLTs for lists, for example:
  $ cd /home/vagrant/kohaclone/koha-tmpl/opac-tmpl/bootstrap/en/xslt
  $ cp MARC21slim2OPACResults.xsl MARC21slim2OPACLists.xsl
- Edit your sysprefs, setting OPACXSLTListsDisplay to:
  /home/vagrant/kohaclone/koha-tmpl/opac-tmpl/bootstrap/{langcode}/xslt/MARC21slim2OPACLists.xsl
- Reload the OPAC list view
=> SUCCESS: Looks exactly as before
- Make some minor tweak (for example in line 423 replace
  <xsl:text> </xsl:text>
for
  <xsl:text> BLAH </xsl:text>
- Reload the list
=> SUCCESS: BLAH shows in several places on the title.
- Repeat for the staff interface
- Sign off :-D

So we can now set custom XSLTs for lists.

Sponsored-by: Carnegie Stout Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Deb Stephenson <DStephen@dubuque.lib.ia.us>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Jonathan Druart 2016-06-23 15:02:26 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2016-06-23 15:02:44 UTC Comment hidden (obsolete)
Comment 16 Héctor Eduardo Castro Avalos 2016-06-23 15:26:51 UTC
Created attachment 52742 [details] [review]
[SIGNED-OFF]Bug 15485: (QA followup) Fix behaviour and default values

Jonathan noticed the current behaviour is that the lists rendering
falls back to the XSLTResults, and if not defined (i.e. != 'default' and
!= some_path) it falls back to a legacy display (non-XSLT).

The patchset changed this behaviour because 'default' is not a valid value
for the lists. So it should fallback to the current behaivour (i.e. Results
XSLT configuration) if not defined. This patch fixes this by adding
 || C4::Context->preference('XSLTResultsDisplay')
(and the OPAC counterpart).

It also fixes minor glitches on the update message (oops) and bad default value
in sysprefs.sql  for 'XSLTListsDisplay'.

Thanks Jonathan!

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 17 Héctor Eduardo Castro Avalos 2016-06-23 15:27:21 UTC
Created attachment 52743 [details] [review]
[SIGNED-OFF]Bug 15485: (QA followup) Simplify logic and use *ResultsDisplay value during upgrade

This patch makes the new sysprefs work as the previously defined. Instead of falling back
to what *ResultsDisplay is set, it now has its own 'default' (that still defaults to the
*Results*.xslt).

The default values are set to 'default' as the rest of XSLT-related sysprefs, and the upgrade
picks whatever is set in OPACXSLTResultsDisplay and XSLTResultsDisplay so current behaviour
is preserved.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
New XSLTs for Lists work as advertised
Comment 18 Kyle M Hall 2016-06-24 13:33:09 UTC
Pushed to master for Koha 16.11, thanks Tomas!
Comment 19 Marjorie Barry-Vila 2016-12-15 15:36:54 UTC
*** Bug 16435 has been marked as a duplicate of this bug. ***
Comment 20 Fridolin Somers 2018-07-23 10:15:51 UTC
*** Bug 14877 has been marked as a duplicate of this bug. ***