I applied the patch. The following is as you describe:

* Patch can be applied
* The folder /koha-tmpl/intranet-tmpl/prog/en/xslt/biblioexport for intranet is there
* The folder /koha-tmpl/opac-tmpl/bootstrap/en/xslt/biblioexport for opac is there
* In the folder /koha-tmpl/intranet-tmpl/prog/en/xslt/biblioexport-samples there is the sample file "MARC21_simple_export.xsl"
* This file can also be found in /koha-tmpl/intranet-tmpl/prog/en/xslt/biblioexport for intranet and in /koha-tmpl/opac-tmpl/bootstrap/en/xslt/biblioexport for opac

As the next point you write: "Open intranet or opac interface and submit any search, open any notices found."

I opened both Intranet and OPAC and searched for the only book in my test catalog. But there were no "notices" found so I couldn't open any. What do you mean with "notices"?

notice = record (French?)

Hi Michael,
sorry, i not write correctly
    - Apply patch
    - Open intranet or opac interface and submit any search, click title of one item found.
    - Click Save and you found Simple Export.
    - Click Simple Export and you have a file.html, it contain informations returned by the xslt file.
    - You can use it in basket and lists.
    - The name 'Simple Export' it's extract in the xslt file, curently the name is MARC21_simple_export.xsl.
    
Bouzid.

I applied the patch again. The following is as you describe:

* Patch can be applied
* The folder /koha-tmpl/intranet-tmpl/prog/en/xslt/biblioexport for intranet is there
* The folder /koha-tmpl/opac-tmpl/bootstrap/en/xslt/biblioexport for opac is there
* In the folder /koha-tmpl/intranet-tmpl/prog/en/xslt/biblioexport-samples there is the sample file "MARC21_simple_export.xsl"
* This file can also be found in /koha-tmpl/intranet-tmpl/prog/en/xslt/biblioexport for intranet and in /koha-tmpl/opac-tmpl/bootstrap/en/xslt/biblioexport for opac
* I open the Intranet interface, submit a search, click the title of a found item

Then I click the button "Save", but I can only see formats BIBTEX, Dublin Core, MARCXML, three kinds of MARC, MODS and RIS - but no "Simple Export".

In the OPAC interface it is almost the same. There I click the button "Save record" but there are only the formats already given above, plus one more (ISBD). But no "Simple Export".

Are you sure MARC21_simple_export.xsl exist in folders /koha-tmpl/intranet-tmpl/prog/en/xslt/biblioexport and /koha-tmpl/opac-tmpl/bootstrap/en/xslt/biblioexport, because for me, when i apply this patch in actual master, it work correctly.

I have set up a completely new kohadevbox.

vagrant@kohadevbox:~$ cd kohaclone
vagrant@kohadevbox:kohaclone(master)$ git pull
Already up-to-date.
vagrant@kohadevbox:kohaclone(master)$ git checkout -b 001
Switched to a new branch '001'

Then I tried to apply patch 17385 again, but I was not able to do it:

vagrant@kohadevbox:kohaclone(001)$ git bz apply 17385
Bug 17385 - Add custom export notices

55961 - Bug 17385 - Add custom export notices

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 17385 - Add custom export notices
fatal: sha1 information is lacking or useless (opac/opac-detail.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 17385 - Add custom export notices
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-17385---Add-custom-export-notices-0lLL49.patch

Rebase patch

QA: Looking here now

QA Comment:
Thanks for your work. I think this is not yet ready for inclusion.

 sub XSLTParse4Display {
-    my ( $biblionumber, $orig_record, $xslsyspref, $fixamps, $hidden_items, $sysxml, $xslfilename, $lang ) = @_;
+    my ( $biblionumber, $orig_record, $xslsyspref, $fixamps, $hidden_items, $sysxml, $xslfilename, $lang, $xslFile, $opac ) = @_;
We already have $xslfilename. You add $xslFile ? Does not look good. 
The added code seems repetitive.

You add a new routine CustomXSLTExportList. It needs unit tests.
In that routine you are parsing the xsl files for something like +<!-- xml title="Simple Export" -->. I am not sure if that is a good idea. Another QA pov is welcome too.
Note that you are doing it each time again too.
If you provide clear xslt names, that should probably be sufficient. Please think about another way to enrich that information if needed.

--- a/Koha/XSLT_Handler.pm
+++ b/Koha/XSLT_Handler.pm
@@ -168,7 +168,6 @@ sub transform {
         ( $xml, $filename, $format ) = @_;
         $format ||= 'chars';
     }
-
     #Initialized yet?
     if ( !$self->{xslt_hash} ) {
         $self->_init;
No need to remove an empty line if you are not touching this module anyway.

diff --git a/koha-tmpl/intranet-tmpl/prog/en/xslt/biblioexport/README b/koha-tmpl/intranet-tmpl/prog/en/xslt/biblioexport/README
new file mode 100644
Empty README ?
And another further on.

koha-tmpl/opac-tmpl/bootstrap/en/xslt/biblioexport-samples/MARC21_simple_export.xsl koha-tmpl/opac-tmpl/bootstrap/en/xslt/biblioexport/MARC21_simple_export.xsl
What is the use of adding two identical files here ?
Actually, all files here are sort of sample files.

Thx for your adjustments! Looks good to me. Final comments:

This is not a blocker for me, but OpacExportOptions controls the options listed by detail sidebar. Your custom exports are listed no matter what. It might be better to let OpacExportOptions also control if the custom exports should be added to the list somehow. Maybe not one by one, but just on directory level (show them all or not)?
But do not make big adjustments here. It could be handled on a new report or we could simply state: remove the xsl files from biblioexport.

XSLTParse4Display is not yet tested, but your change should officially be tested ;)
    Also I recommend to remove the opac parameter. It is not consistent with the approach followed in the sub. Not changing the parameters could help you here.
    Please send XSLTCustomExport and OPACXSLTCustomExport to the routine; this is consistent and does the same thing.
    Since your change only deals with the $xslfilename, I could live without a test script (but I am not sure about the RM).

CustomXSLTExportList is new and should really have its own test (should not be hard)
    Mock opachtdocs and create the dir in a tempdir. Check on returned title and method.

Comment on attachment 74660 [details] [review]
Bug 17385: Add custom export XSLT

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

Generally looks good with a few minor code comments added for review.

::: C4/XSLT.pm
@@ +197,5 @@
>      $xslfilename ||= C4::Context->preference($xslsyspref);
>      $lang ||= C4::Languages::getlanguage();
>  
> +    if ($xslsyspref eq "XSLTCustomExport") {
> +        my $dir;

The code used to defined $dir in CustomXSLTExportList which does the same thing is cleaner.. why not use that here too... or even factor it out and use the same factored out routine for both to keep it DRY?

@@ +370,5 @@
>  }
>  
> +=head2 CustomXSLTExportList
> +
> + Returns list of file for custom xslt conversion

Please add accepted parameters (i.e. $opac) to the POD section and note their required status and what they accept (string, int, etc)

Ignore the first comment in that review.. you've already changed it in the followup.

I am sorry, patch doesn't apply with a lot of conflicts. Can you please rebase?

I've attached rebased patches and added Cori Lynn's sign-off to the first patch as well. 

1) Can you please add POD to CustomXSLTExportList (see comment #19)

2) There is lots of minor (fast to fix) fails in the QA script:

FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc
   FAIL	  filters
		wrong_html_filter at line 100 (            <li><a href="/cgi-bin/koha/catalogue/export.pl?format=[% filesOption.format %]&amp;op=exportxsl&amp;file=[% filesOption.value %]&amp;bib=[% biblionumber | html %]">[% filesOption.filename %]</a></li>)
		missing_filter at line 100 (            <li><a href="/cgi-bin/koha/catalogue/export.pl?format=[% filesOption.format %]&amp;op=exportxsl&amp;file=[% filesOption.value %]&amp;bib=[% biblionumber | html %]">[% filesOption.filename %]</a></li>)
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/virtualshelves-toolbar.inc
   FAIL	  filters
		wrong_html_filter at line 27 (                        <li><a href="/cgi-bin/koha/virtualshelves/downloadshelf.pl?format=xsl.[% xsl_export.value %]&amp;shelfid=[% shelf.shelfnumber %]">XSL - [% xsl_export.filename %]</a></li>)
		missing_filter at line 27 (                        <li><a href="/cgi-bin/koha/virtualshelves/downloadshelf.pl?format=xsl.[% xsl_export.value %]&amp;shelfid=[% shelf.shelfnumber %]">XSL - [% xsl_export.filename %]</a></li>)
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/basket/basket.tt
   FAIL	  filters
		missing_filter at line 60 (                                <li><a href="/cgi-bin/koha/basket/downloadcart.pl?format=xsl.[% xsl_export.value %]&amp;bib_list=[% bib_list %]">XSL - [% xsl_export.filename %]</a></li>)
		wrong_html_filter at line 60 (                                <li><a href="/cgi-bin/koha/basket/downloadcart.pl?format=xsl.[% xsl_export.value %]&amp;bib_list=[% bib_list %]">XSL - [% xsl_export.filename %]</a></li>)
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
   FAIL	  filters
		missing_filter at line 860 (            <option value="[% filesOption.value %]">[% filesOption.filename %]</option>)
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/downloadshelf.tt
   FAIL	  filters
		missing_filter at line 40 (            <option value="xsl.[% xsl_export.value %]">XSL - [% xsl_export.filename %]</option>)
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 OK	koha-tmpl/intranet-tmpl/prog/en/xslt/biblioexport/MARC21_simple_export.xsl
   OK	  git manipulation
   OK	  xml_valid

 FAIL	koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc
   FAIL	  filters
		missing_filter at line 71 (                                        <li><a role="menuitem" href="/cgi-bin/koha/opac-export.pl?format=[% filesOption.format %]&amp;op=exportxsl&amp;file=[% filesOption.value %]&amp;bib=[% biblionumber %]">[% filesOption.filename %]</a></li>)
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-downloadcart.tt
   FAIL	  filters
		missing_filter at line 24 (                                    <option value="xsl.[% xsl_export.value %]">XSL - [% xsl_export.filename %]</option>)
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-downloadshelf.tt
   FAIL	  filters
		missing_filter at line 63 (                                                <option value="xsl.[% xsl_export.value %]">XSL - [% xsl_export.filename %]</option>)
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

3) t/XSLT.t passes, but there is a warning that should be fixed:
t/XSLT.t .. 2/12 Use of uninitialized value $outputformat in string eq at /home/vagrant/kohaclone/C4/XSLT.pm line 404.

4) I think we should leave the example out of production. You only provide an example for MARC21 that will not work for UNIMARC and NORMARC users and that can't be removed from display unless you delete the file (and will come back on each update using packages).

comment#25!

Rebased on current master.

QA: Looking here now

QA Comment:
Thanks for your perseverance, guys. This took quite some time.
Since the Custom option is not checked by default, I do not see a problem pushing this in its current form. 

[1] We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.
=> See follow-up.
[2] We should add caching in CustomXSLTExportList.
=> See second follow-up.
[3] The example xslt perhaps should be renamed (as mentioned before earlier). If you would only check for the .xsl extension, we could add a README and rename the extension of the simple export sample. Since it is pref controlled now, this may be done later imo. No blocker for me.
[4] t/XSLT.t depends on a pref, so on the database. Should imo be moved to dependent. Removing Test::DBIx::Class :) No blocker, maybe later ?
[5] I personally do not like using LibXML to parse the filename and format from the xsl file. And was just thinking about adding a small json or yaml file to do the same? No blocker, maybe later..

Passed QA

Hmm. One small thing still..

Obviously, we needed two cache keys.

Excellent work, this looks like a great base for future customization.

Two things:
1 - Many qa script failures - missing filters and a critic complaint on XSLT:
 FAIL	C4/XSLT.pm
   FAIL	  critic
		# BuiltinFunctions::RequireGlobFunction: Got 1 violation(s).  
		# Variables::ProhibitConditionalDeclarations: Got 1 violation(s).  

2 - I don't see the new option in lists - it works on detail and cart

Remove merge conflicts.
Fixed qa script failures - missing filters and a critic complaint on XSLT.

Patch no longer applies:

...
Applying: Bug 17385: QA Critics Fixed and added Missing Filters
error: sha1 information is lacking or useless (C4/XSLT.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 17385: QA Critics Fixed and added Missing Filters
...

Thanks Gabriel, the patch now applies!

I've changed the status to Failed QA as:
- The test plan indicates it should be available for carts and lists: carts work, for lists there is no option to download (only shows RIS, BibTeX, ISBD, MARC as options).
- The tests don't pass for me.

root@ffb0d824040d:koha(bz17385)$ prove t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. Global symbol "$builder" requires explicit package name (did you forget to declare "my $builder"?) at t/db_dependent/XSLT.t line 28.
Global symbol "$builder" requires explicit package name (did you forget to declare "my $builder"?) at t/db_dependent/XSLT.t line 77.
Global symbol "$builder" requires explicit package name (did you forget to declare "my $builder"?) at t/db_dependent/XSLT.t line 85.
Global symbol "$builder" requires explicit package name (did you forget to declare "my $builder"?) at t/db_dependent/XSLT.t line 95.
Execution of t/db_dependent/XSLT.t aborted due to compilation errors.
# Looks like your test exited with 255 before it could output anything.
t/db_dependent/XSLT.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 2/2 subtests 

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 65280 Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 2 tests but ran 0.
Files=1, Tests=0,  1 wallclock secs ( 0.02 usr  0.01 sys +  0.87 cusr  0.09 csys =  0.99 CPU)
Result: FAIL

root@ffb0d824040d:koha(bz17385)$ prove t/XSLT.t
t/XSLT.t .. 2/12 
#   Failed test 'CustomXSLTExportList finds custom XSLTs'
#   at t/XSLT.t line 107.

#   Failed test 'Title is specified in root node'
#   at t/XSLT.t line 108.
Use of uninitialized value in string eq at t/XSLT.t line 109.

#   Failed test 'Title is filename if not specified'
#   at t/XSLT.t line 109.
# Looks like you failed 3 tests of 12.
t/XSLT.t .. Dubious, test returned 3 (wstat 768, 0x300)
Failed 3/12 subtests 

Test Summary Report
-------------------
t/XSLT.t (Wstat: 768 Tests: 12 Failed: 3)
  Failed tests:  10-12
  Non-zero exit status: 3
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  3.63 cusr  0.14 csys =  3.79 CPU)
Result: FAIL

Notes from testing:
- Need to enable 'Custom' option for OpacExportOptions system preference.
- Naming:
  . For cart shows as: XSL - Simple Export
  . System preference OpacExportOptions shows as: Custom
  . Staff interface save shows as: Simple Export (from the sample .xsl file)
  . OPAC save record shows as: Simple Export (from the sample .xsl file)

The tests are failing for me:

t/XSLT.t ............... 
1..12
ok 1 - use C4::XSLT;
ok 2 - Found test/en
ok 3 - Found test/es-ES
ok 4 - Found test/en
ok 5 - Found test/es-ES
ok 6 - Fell back to test/en for test/fr-FR
ok 7 - Fell back to prog/es-ES for nope/es-ES
ok 8 - Fell back to prog/en for nope/fr-FR
ok 9 - singleBranchMode has a value of 0
Use of uninitialized value in pattern match (m//) at /home/vagrant/kohaclone/C4/XSLT.pm line 422.
not ok 10 - CustomXSLTExportList finds custom XSLTs

#   Failed test 'CustomXSLTExportList finds custom XSLTs'
#   at t/XSLT.t line 107.
Use of uninitialized value in string eq at t/XSLT.t line 108.
not ok 11 - Title is specified in root node

#   Failed test 'Title is specified in root node'
#   at t/XSLT.t line 108.
Use of uninitialized value in string eq at t/XSLT.t line 109.
not ok 12 - Title is filename if not specified

#   Failed test 'Title is filename if not specified'
#   at t/XSLT.t line 109.
# Looks like you failed 3 tests of 12.
Dubious, test returned 3 (wstat 768, 0x300)
Failed 3/12 subtests 

Test Summary Report
-------------------
t/XSLT.t             (Wstat: 768 Tests: 12 Failed: 3)
  Failed tests:  10-12
  Non-zero exit status: 3

Patch rebased. I changed test 9 - singleBranchMode in order to manage the case where the libraries has multiple branches (related bug 18089)

Created attachment 112680 [details] [review]
Bug 17385: QA Critics Fixed and added Missing Filters

Signed-off-by: David Nind <david@davidnind.com>

I made 3 tests and none of them was fully satisfying.


1) to 5) : no problem

6) I had to "Restart services" for "XSL - Simple Export" to appear in the menu
(but i don't know if this really is a problem)

7) is ok

8) the downloaded file seems to be empty and contains no info on the record

I get an error when I try to export a record in the OPAC:

Undefined subroutine &CGI::Compile::ROOT::kohadevbox_koha_opac_opac_2dexport_2epl::XSLTParse4Display called at /kohadevbox/koha/opac/opac-export.pl line 147

It works in the staff interface. The "Simple Export" doesn't seem very useful... I wonder if we shouldn't have a more complex example?

"<p xmlns:marc="http://www.loc.gov/MARC21/slim">The chronicles of Narnia / C.S. Lewis ; with illustrations by Pauline Baynes.<br>New York : HarperCollinsPublishers, 2001.</p>"

The changes to opac-detail-sidebar.inc are reverting some updates we don't want to lose. Please double-check to make sure you're only making essential changes there.

Finally, there are QA warnings. Please run the QA tool to check.

Created attachment 132488 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Created attachment 132489 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Created attachment 132490 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 132491 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Created attachment 132492 [details] [review]
Bug 17385: Add custum export format option when saving a record

This patch adds "XSL-Simple Export" when saving a bibliographic
record. Previous patches are combined into this one.

To test:
1) Search for a record in the intranet.
2) Click on the "Save" button and observe the dropdown menu.
3) Notice that "XSL- Simple Export" is not in the menu.
4) Apply the patch.
5) Repeat the steps 1 and 2.
6) Notice that "XSL - Simple Export" is in the menu.
7) Click on "XSL - Simple Export". A .html file should be downloaded.
8) Open the .html file. You should see the record's informations.

Created attachment 132493 [details] [review]
Bug 17385: Correction of opac-detail-sidebar.inc

This patch reverts the changes made in opac-detail-sidebar.inc, so the sidebar is
displayed as wanted. Tests are now passing properly.

TEST PLAN:
1- Apply the patch and search for a record in the OPAC.
2- Click on "Save record". Notice that the dropdown menu is unchanged.
3- Run t/db_dependent/XSLT.t (All tests should pass)

Created attachment 132581 [details] [review]
Bug 17385: Fixing display for XSL-Simple Export in OPAC

This attachment correct the placement of XSL-Simple Export in the OPAC's page.
It is now displayed in the "Save record" dropdown menu.

Created attachment 133498 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Created attachment 133499 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Created attachment 133500 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 133501 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Created attachment 133502 [details] [review]
Bug 17385: Add custum export format option when saving a record

This patch adds "XSL-Simple Export" when saving a bibliographic
record. Previous patches are combined into this one.

To test:
1) Search for a record in the intranet.
2) Click on the "Save" button and observe the dropdown menu.
3) Notice that "XSL- Simple Export" is not in the menu.
4) Apply the patch.
5) Repeat the steps 1 and 2.
6) Notice that "XSL - Simple Export" is in the menu.
7) Click on "XSL - Simple Export". A .html file should be downloaded.
8) Open the .html file. You should see the record's informations.

Created attachment 133503 [details] [review]
Bug 17385: Correction of opac-detail-sidebar.inc

This patch reverts the changes made in opac-detail-sidebar.inc, so the sidebar is
displayed as wanted. Tests are now passing properly.

TEST PLAN:
1- Apply the patch and search for a record in the OPAC.
2- Click on "Save record". Notice that the dropdown menu is unchanged.
3- Run t/db_dependent/XSLT.t (All tests should pass)

Created attachment 133504 [details] [review]
Bug 17385: Fixing display for XSL-Simple Export in OPAC

This attachment correct the placement of XSL-Simple Export in the OPAC's page.
It is now displayed in the "Save record" dropdown menu.

Created attachment 138031 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Created attachment 138032 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Created attachment 138033 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 138034 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Created attachment 138035 [details] [review]
Bug 17385: Add custum export format option when saving a record

This patch adds "XSL-Simple Export" when saving a bibliographic
record. Previous patches are combined into this one.

To test:
1) Search for a record in the intranet.
2) Click on the "Save" button and observe the dropdown menu.
3) Notice that "XSL- Simple Export" is not in the menu.
4) Apply the patch.
5) Repeat the steps 1 and 2.
6) Notice that "XSL - Simple Export" is in the menu.
7) Click on "XSL - Simple Export". A .html file should be downloaded.
8) Open the .html file. You should see the record's informations.

Created attachment 138036 [details] [review]
Bug 17385: Correction of opac-detail-sidebar.inc

This patch reverts the changes made in opac-detail-sidebar.inc, so the sidebar is
displayed as wanted. Tests are now passing properly.

TEST PLAN:
1- Apply the patch and search for a record in the OPAC.
2- Click on "Save record". Notice that the dropdown menu is unchanged.
3- Run t/db_dependent/XSLT.t (All tests should pass)

Created attachment 138037 [details] [review]
Bug 17385: Fixing display for XSL-Simple Export in OPAC

This attachment correct the placement of XSL-Simple Export in the OPAC's page.
It is now displayed in the "Save record" dropdown menu.

Apologies, I was too slow to retest - the patches no longer apply 8-(..

Applying: Bug 17385: Add custom export XSLT
.git/rebase-apply/patch:163: trailing whitespace.
} else { 
.git/rebase-apply/patch:166: trailing whitespace.
    $template->param(bib_list => $bib_list); 
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	basket/basket.pl
M	basket/downloadcart.pl
M	catalogue/detail.pl
M	catalogue/export.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
M	opac/opac-detail.pl
M	opac/opac-downloadcart.pl
M	opac/opac-downloadshelf.pl
M	opac/opac-export.pl
M	virtualshelves/downloadshelf.pl
M	virtualshelves/shelves.pl
.git/rebase-apply/patch:163: trailing whitespace.
} else { 
.git/rebase-apply/patch:166: trailing whitespace.
    $template->param(bib_list => $bib_list); 
warning: 2 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging virtualshelves/shelves.pl
Auto-merging virtualshelves/downloadshelf.pl
Auto-merging opac/opac-export.pl
CONFLICT (content): Merge conflict in opac/opac-export.pl
Auto-merging opac/opac-downloadshelf.pl
CONFLICT (content): Merge conflict in opac/opac-downloadshelf.pl
Auto-merging opac/opac-downloadcart.pl
CONFLICT (content): Merge conflict in opac/opac-downloadcart.pl
Auto-merging opac/opac-detail.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging catalogue/export.pl
Auto-merging catalogue/detail.pl
Auto-merging basket/downloadcart.pl
Auto-merging basket/basket.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 17385: Add custom export XSLT

Created attachment 138082 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Created attachment 138083 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Created attachment 138084 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 138085 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Created attachment 138086 [details] [review]
Bug 17385: Add custum export format option when saving a record

This patch adds "XSL-Simple Export" when saving a bibliographic
record. Previous patches are combined into this one.

To test:
1) Search for a record in the intranet.
2) Click on the "Save" button and observe the dropdown menu.
3) Notice that "XSL- Simple Export" is not in the menu.
4) Apply the patch.
5) Repeat the steps 1 and 2.
6) Notice that "XSL - Simple Export" is in the menu.
7) Click on "XSL - Simple Export". A .html file should be downloaded.
8) Open the .html file. You should see the record's informations.

Created attachment 138087 [details] [review]
Bug 17385: Correction of opac-detail-sidebar.inc

This patch reverts the changes made in opac-detail-sidebar.inc, so the sidebar is
displayed as wanted. Tests are now passing properly.

TEST PLAN:
1- Apply the patch and search for a record in the OPAC.
2- Click on "Save record". Notice that the dropdown menu is unchanged.
3- Run t/db_dependent/XSLT.t (All tests should pass)

Created attachment 138088 [details] [review]
Bug 17385: Fixing display for XSL-Simple Export in OPAC

This attachment correct the placement of XSL-Simple Export in the OPAC's page.
It is now displayed in the "Save record" dropdown menu.

Rebased

Thanks Shi Yao, the patches now apply!

However, I've changed the status to Failed QA as there are:

1. Error traces after the patches are applied:

   a. See error traces for the staff interface - see [1], [2], and [3] (occurs if the 'XSL - Simple Export' option for OpacExportOptions is either selected or not selected).

   b. See error traces for the OPAC - see [4].

2. If the 'XSL - Simple Export' option for OpacExportOptions is not selected, it still appears in the staff interface record details page as a save option.

3. The t/XSLT.t tests fail - see [5].

There is no option in the OPAC cart and lists to download as 'XSL - Simple Export' - I'm not sure whether there should be or not...

Consolidated test plan so far:
1. Before applying the patches run the tests - these should pass:
   a. prove -l t/XSLT.t
   b. prove -l t/db_dependent/XSLT.t
2. Check current download and save options:
   a. OpacExportOptions system preference: all options selected, note that there is no 'XSL - Simple Export' option.
   b. for both the OPAC and the staff interface:
      (i) search and add some selected items to the cart: check the download options - there is no 'XSL - Simple Export' option
      (ii) search and add some items to a new list: check the download lists options - there is no 'XSL - Simple Export' option
      (iii) from the details page for a record in the OPAC: check the save record options in the right sidebar, there is no 'XSL - Simple Export' option
      (iv) from the details page for a record in the staff interface: check the save record options from the menu bar, there is no 'XSL - Simple Export' option
4. Apply the patches and reload everything (flush_memcached, restart_all, clear your browser cache (or use an incognito/private window)).
5. For the OpacExportOptions system preference, select the new option 'XSL - Simple Export'.
6. Repeat step 2 - there should now be an 'XSL - Simple Export' option available for download or saving.
7. Select the 'XSL - Simple Export' in all the paces it appears in the OPAC and  staff interface - an HTML file should be downloaded.
8. Open the HTML file - you should see the record's information.
9. Run the tests - they should pass:
   a. prove -l t/XSLT.t
   b. prove -l t/db_dependent/XSLT.t

[1] Error trace after clicking on the cart in the staff interface: 
   
    Undefined subroutine &CGI::Compile::ROOT::kohadevbox_koha_basket_basket_2epl::CustomXSLTExportList called at /kohadevbox/koha/basket/basket.pl line 120
    ....

[2] Error trace when accessing the lists from the staff interface:

    Undefined subroutine &CGI::Compile::ROOT::kohadevbox_koha_virtualshelves_shelves_2epl::CustomXSLTExportList called at /kohadevbox/koha/virtualshelves/shelves.pl line 381
    ....

[3] Error trace when selecting Save > XSL - Simple Export from a record's details page in the staff interface:

    Undefined subroutine &CGI::Compile::ROOT::kohadevbox_koha_catalogue_export_2epl::GetMarcBiblio called at /kohadevbox/koha/catalogue/export.pl line 95
    ....

[4] Error trace when selecting Save record > XSL - Simple Export from a record's details page in the OPAC:
    
    Undefined subroutine &C4::Biblio::GetMarcBiblio called at /kohadevbox/koha/opac/opac-export.pl line 145
    ....

[5] Failed tests for t/XSLT.t

prove -l t/XSLT.t
t/XSLT.t .. 2/12 
#   Failed test 'CustomXSLTExportList finds custom XSLTs'
#   at t/XSLT.t line 107.

#   Failed test 'Title is specified in root node'
#   at t/XSLT.t line 108.
Use of uninitialized value in string eq at t/XSLT.t line 109.

#   Failed test 'Title is filename if not specified'
#   at t/XSLT.t line 109.
# Looks like you failed 3 tests of 12.
t/XSLT.t .. Dubious, test returned 3 (wstat 768, 0x300)
Failed 3/12 subtests 

Test Summary Report
-------------------
t/XSLT.t (Wstat: 768 Tests: 12 Failed: 3)
  Failed tests:  10-12
  Non-zero exit status: 3
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.01 sys +  3.46 cusr  0.19 csys =  3.68 CPU)
Result: FAIL

(In reply to David Nind from comment #128)
> Thanks Shi Yao, the patches now apply!
> 
> However, I've changed the status to Failed QA as there are:
> 
> 1. Error traces after the patches are applied:
> 
>    a. See error traces for the staff interface - see [1], [2], and [3]
> (occurs if the 'XSL - Simple Export' option for OpacExportOptions is either
> selected or not selected).
> 
>    b. See error traces for the OPAC - see [4].
> 
> 2. If the 'XSL - Simple Export' option for OpacExportOptions is not
> selected, it still appears in the staff interface record details page as a
> save option.
> 
> 3. The t/XSLT.t tests fail - see [5].

1. There shouldn't be any errors now

2. The system preference has the word "Opac" in it so I assume it only affects OPAC interface and not the staff interface

3. Couldn't run the test on my machine because I have trouble installing Test::DBIx::Class. Hope it works

Created attachment 139277 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Created attachment 139278 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Created attachment 139279 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 139280 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Created attachment 139281 [details] [review]
Bug 17385: Add custum export format option when saving a record

This patch adds "XSL-Simple Export" when saving a bibliographic
record. Previous patches are combined into this one.

To test:
1) Search for a record in the intranet.
2) Click on the "Save" button and observe the dropdown menu.
3) Notice that "XSL- Simple Export" is not in the menu.
4) Apply the patch.
5) Repeat the steps 1 and 2.
6) Notice that "XSL - Simple Export" is in the menu.
7) Click on "XSL - Simple Export". A .html file should be downloaded.
8) Open the .html file. You should see the record's informations.

Created attachment 139282 [details] [review]
Bug 17385: Correction of opac-detail-sidebar.inc

This patch reverts the changes made in opac-detail-sidebar.inc, so the sidebar is
displayed as wanted. Tests are now passing properly.

TEST PLAN:
1- Apply the patch and search for a record in the OPAC.
2- Click on "Save record". Notice that the dropdown menu is unchanged.
3- Run t/db_dependent/XSLT.t (All tests should pass)

Created attachment 139283 [details] [review]
Bug 17385: Fixing display for XSL-Simple Export in OPAC

This attachment correct the placement of XSL-Simple Export in the OPAC's page.
It is now displayed in the "Save record" dropdown menu.

Created attachment 139284 [details] [review]
Bug 17385: Add missing imports and equivalent code for GetMarcBiblio

Rebase

Created attachment 141157 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Created attachment 141158 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Created attachment 141159 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 141160 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Created attachment 141161 [details] [review]
Bug 17385: Add custum export format option when saving a record

This patch adds "XSL-Simple Export" when saving a bibliographic
record. Previous patches are combined into this one.

To test:
1) Search for a record in the intranet.
2) Click on the "Save" button and observe the dropdown menu.
3) Notice that "XSL- Simple Export" is not in the menu.
4) Apply the patch.
5) Repeat the steps 1 and 2.
6) Notice that "XSL - Simple Export" is in the menu.
7) Click on "XSL - Simple Export". A .html file should be downloaded.
8) Open the .html file. You should see the record's informations.

Created attachment 141162 [details] [review]
Bug 17385: Correction of opac-detail-sidebar.inc

This patch reverts the changes made in opac-detail-sidebar.inc, so the sidebar is
displayed as wanted. Tests are now passing properly.

TEST PLAN:
1- Apply the patch and search for a record in the OPAC.
2- Click on "Save record". Notice that the dropdown menu is unchanged.
3- Run t/db_dependent/XSLT.t (All tests should pass)

Created attachment 141163 [details] [review]
Bug 17385: Fixing display for XSL-Simple Export in OPAC

This attachment correct the placement of XSL-Simple Export in the OPAC's page.
It is now displayed in the "Save record" dropdown menu.

Created attachment 141164 [details] [review]
Bug 17385: Add missing imports and equivalent code for GetMarcBiblio

Hi Shi Yao Wang.

I've retested, using the consolidated test plan in comment #128 - please provide an update if anything for testing needs changing.

Things I've noted - and the reasons for changing to Failed QA:

1. For the staff interface:
   - For an individual record, the Save > XSL - Simple Export works as expected
   - For the cart and lists: 
     . XSL - Simple Export appears in the list of download options   
     . there are error traces if this option is selected

Error trace - staff interface cart:
Undefined subroutine &CGI::Compile::ROOT::kohadevbox_koha_basket_downloadcart_2epl::XSLTParse4Display called at /kohadevbox/koha/basket/downloadcart.pl line 83.
 at /kohadevbox/koha/basket/downloadcart.pl line 71

Error trace - staff interface lists:
Undefined subroutine &CGI::Compile::ROOT::kohadevbox_koha_virtualshelves_downloadshelf_2epl::XSLTParse4Display called at /kohadevbox/koha/virtualshelves/downloadshelf.pl line 85.
 at /kohadevbox/koha/virtualshelves/downloadshelf.pl line 75

2. For the OPAC:
   - For an individual record, the Save record > XSL - Simple Export works as expected
   - For the cart and lists: XSL - Simple Export does not appear in the list of download options

3. Tests fail for prove -l t/XSLT.t:

root@kohadevbox:koha(bz17385)$ prove -l t/XSLT.t
t/XSLT.t .. 2/12 
#   Failed test 'CustomXSLTExportList finds custom XSLTs'
#   at t/XSLT.t line 107.

#   Failed test 'Title is specified in root node'
#   at t/XSLT.t line 108.
Use of uninitialized value in string eq at t/XSLT.t line 109.

#   Failed test 'Title is filename if not specified'
#   at t/XSLT.t line 109.
# Looks like you failed 3 tests of 12.
t/XSLT.t .. Dubious, test returned 3 (wstat 768, 0x300)
Failed 3/12 subtests 

Test Summary Report
-------------------
t/XSLT.t (Wstat: 768 Tests: 12 Failed: 3)
  Failed tests:  10-12
  Non-zero exit status: 3
Files=1, Tests=12,  5 wallclock secs ( 0.02 usr  0.00 sys +  3.91 cusr  0.17 csys =  4.10 CPU)
Result: FAIL

Created attachment 151095 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Created attachment 151096 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Created attachment 151097 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 151098 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Created attachment 151099 [details] [review]
Bug 17385: (Fix) Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList

This patch adds 'XSL-Simple Export' to the cart and the lists and correct the fail test in t/XSLT.t
it contains Add custum export format option when saving a record

TO TEST:

Before applying:
1) Search for a record in the intranet.
2) Click on the 'Save' button and observe the dropdown menu.
    --> There is no 'XSL- Simple Export' in the dropdown menu.
3) Apply the patch.
4) In System preferences search for OpacExportOptions and enable XSL
5) Repeat the steps 1 and 2.
    ---> There is now 'XSL- Simple Export'  option in the dropdown menu
6) Click on 'XSL - Simple Export'. a .html file should be downloaded.
7) Open the .html file. You should see the record's informations.
8) Add some items to your Cart
9) Open the Cart
10) Click on the 'Download' and 'XSL - Simple Export'.
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
11) Add some items to a List
12) Open the List
13) Click on the 'Download' and 'XSL - Simple Export'.
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
14) Do the same steps in OPAC
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
15) In System preferences search for OpacExportOptions and disable XSL
16) Repeat all steps
17) Notice that 'XSL - Simple Export' is no more in the menus.
18) Run prove -l t/XSLT.t

Created attachment 151102 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 151103 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 151104 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 151105 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 151106 [details] [review]
Bug 17385: (Fix) Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList

This patch adds 'XSL-Simple Export' to the cart and the lists and correct the fail test in t/XSLT.t
it contains Add custum export format option when saving a record

TO TEST:

Before applying:
1) Search for a record in the intranet.
2) Click on the 'Save' button and observe the dropdown menu.
    --> There is no 'XSL- Simple Export' in the dropdown menu.
3) Apply the patch.
4) In System preferences search for OpacExportOptions and enable XSL
5) Repeat the steps 1 and 2.
    ---> There is now 'XSL- Simple Export'  option in the dropdown menu
6) Click on 'XSL - Simple Export'. a .html file should be downloaded.
7) Open the .html file. You should see the record's informations.
8) Add some items to your Cart
9) Open the Cart
10) Click on the 'Download' and 'XSL - Simple Export'.
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
11) Add some items to a List
12) Open the List
13) Click on the 'Download' and 'XSL - Simple Export'.
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
14) Do the same steps in OPAC
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
15) In System preferences search for OpacExportOptions and disable XSL
16) Repeat all steps
17) Notice that 'XSL - Simple Export' is no more in the menus.
18) Run prove -l t/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 151377 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 151378 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 151379 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 151380 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 151381 [details] [review]
Bug 17385: (Fix) Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList

This patch adds 'XSL-Simple Export' to the cart and the lists and correct the fail test in t/XSLT.t
it contains Add custum export format option when saving a record

TO TEST:

Before applying:
1) Search for a record in the intranet.
2) Click on the 'Save' button and observe the dropdown menu.
    --> There is no 'XSL- Simple Export' in the dropdown menu.
3) Apply the patch.
4) In System preferences search for OpacExportOptions and enable XSL
5) Repeat the steps 1 and 2.
    ---> There is now 'XSL- Simple Export'  option in the dropdown menu
6) Click on 'XSL - Simple Export'. a .html file should be downloaded.
7) Open the .html file. You should see the record's informations.
8) Add some items to your Cart
9) Open the Cart
10) Click on the 'Download' and 'XSL - Simple Export'.
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
11) Add some items to a List
12) Open the List
13) Click on the 'Download' and 'XSL - Simple Export'.
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
14) Do the same steps in OPAC
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
15) In System preferences search for OpacExportOptions and disable XSL
16) Repeat all steps
17) Notice that 'XSL - Simple Export' is no more in the menus.
18) Run prove -l t/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 154379 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 154380 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 154381 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 154382 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 154383 [details]
Bug 17385: (Fix) Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList

This patch adds 'XSL-Simple Export' to the cart and the lists and correct the fail test in t/XSLT.t
it contains Add custum export format option when saving a record

TO TEST:

Before applying:
1) Search for a record in the intranet.
2) Click on the 'Save' button and observe the dropdown menu.
    --> There is no 'XSL- Simple Export' in the dropdown menu.
3) Apply the patch.
4) In System preferences search for OpacExportOptions and enable XSL
5) Repeat the steps 1 and 2.
    ---> There is now 'XSL- Simple Export'  option in the dropdown menu
6) Click on 'XSL - Simple Export'. a .html file should be downloaded.
7) Open the .html file. You should see the record's informations.
8) Add some items to your Cart
9) Open the Cart
10) Click on the 'Download' and 'XSL - Simple Export'.
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
11) Add some items to a List
12) Open the List
13) Click on the 'Download' and 'XSL - Simple Export'.
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
14) Do the same steps in OPAC
    ----> a .html file should be downloaded.
    ----> you should see the record's informations in the downloaded file.
15) In System preferences search for OpacExportOptions and disable XSL
16) Repeat all steps
17) Notice that 'XSL - Simple Export' is no more in the menus.
18) Run prove -l t/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Rebase

Hi Hamat, 

can you please rebase and re-attach all patches? 

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 17385: Add custom export XSLT
Applying: Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder
Applying: Bug 17385: (QA follow-up) Add caching
Applying: Bug 17385: Add tests for CustomXSLTExportList
Applying: Bug 17385: (Fix) Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/includes/virtualshelves-toolbar.inc).
error: could not build fake ancestor
Patch failed at 0001 Bug 17385: (Fix) Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList


Please also run the QA test tools and fix the commit message of the last patch (Fix) should get you a warning in the tests.

Created attachment 156815 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 156816 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 156817 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 156818 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 156819 [details] [review]
Bug 17385: Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList

I can see we had a lot of iterations here already. I did some testing to help out, but I think I'll leave it to Marcel to finish:

1) QA test tools

We have a fail for TT filters:

 FAIL	koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc
   FAIL	  filters
		wrong_html_filter at line 90 (                                    <a href="/cgi-bin/koha/opac-export.pl?format=[% filesOption.format | html %]&amp;op=exportxsl&amp;file=[% filesOption.value | html %]&amp;bib=[% biblionumber | html %]" >XSL - [% filesOption.filename | html %]</a>)
		wrong_html_filter at line 90 (                                    <a href="/cgi-bin/koha/opac-export.pl?format=[% filesOption.format | html %]&amp;op=exportxsl&amp;file=[% filesOption.value | html %]&amp;bib=[% biblionumber | html %]" >XSL - [% filesOption.filename | html %]</a>)
		wrong_html_filter at line 90 (                                    <a href="/cgi-bin/koha/opac-export.pl?format=[% filesOption.format | html %]&amp;op=exportxsl&amp;file=[% filesOption.value | html %]&amp;bib=[% biblionumber | html %]" >XSL - [% filesOption.filename | html %]</a>)

... and a lot of perltidy warns.

2) Export format pull down

+            <li><a href="/cgi-bin/koha/catalogue/export.pl?format=[% filesOption.format | uri %]&amp;op=exportxsl&amp;file=[% filesOption.value | uri %]&amp;bib=[% biblionumber | uri %]">XSL - [% filesOption.filename | html %]</a></li>

and other occurences.

As we are not exporting an XSL file, it would be nice to rephrase this. Maybe we should just remove XSL in favor of just having the file name?


3) Export format description

In the system preference we have:
+                custom: XSL - Simple Export

* Capitalization
* We don't export XSL, I believe something like: Simple HTML format would probably be a better match?
* It would also guide any other added custom export format, if I understand correctly, so maybe even: Custom export formats, including simple HTML?

+    $format = "html" if ($format =~ /^xsl\./);

It looks like we always assume it will be an HTML file?

4) Commit messages include no test plan.

5) New option for system preference

I think we need a database update adding the option to OpacExportOptions and also update sysprefs.sql.

6) I am struggling with adding the sample files to Koha. They are great for testing this, but I am not sure if they are helpful as they are. At the moment, they are incomplete/invalid HTML files. 

<p xmlns:marc="http://www.loc.gov/MARC21/slim">Programming Perl / Tom Christiansen, Brian D. Foy &amp; Larry Wall.<br>Beijing ; O'Reilly, c2012.</p>

7) The path for the custom formats is not configurable. I guess any files added there by libraries could be potentially be overwritten in updates. If this is for custom formats, maybe we should allow for a custom path as well.


Overall the concept here feels a little rough.

Hi Katrin,
I ran the QA Test Tools, and I couldn't find any errors or warnings. What parameters did you use to get errors and warnings for the opac-detail-sidebar.inc file ? On my end, I ran koha-qa.pl -v 2 -c 5 on a git installation. I use the most recent version of master and only apply the patch for this bug.

I'd like to provide some clarification regarding point 6. Initially, this patch was created so that students could have a file containing the basic notices informations. This would enable them to copy and paste these informations more easily into their assignments. Therefore, HTML files are not generated for importing notices afterward, which is why they are incomplete/invalid for import.
I also have some questions regarding point 7. To make sure I understand correctly, should we add a system preference (e.g., CustomExportFilePath) so that libraries can choose where they want to place their custom XSL file? If no path is specified, would the default path be to the "MARC21_simple_export.xsl" files? Additionally, should we consider renaming these files to "MARC21CustomSimpleExport.xsl" to align with the naming conventions of other .xsl files?

(In reply to Emily-Rose Francoeur from comment #181)
> Hi Katrin,
> I ran the QA Test Tools, and I couldn't find any errors or warnings. What
> parameters did you use to get errors and warnings for the
> opac-detail-sidebar.inc file ? On my end, I ran koha-qa.pl -v 2 -c 5 on a
> git installation. I use the most recent version of master and only apply the
> patch for this bug.

I use the tools without any additional params as "git qa" in ktd. 

Do you have the latest version of QA script? (separate repository from Koha)

(In reply to Emily-Rose Francoeur from comment #182)
> I'd like to provide some clarification regarding point 6. Initially, this
> patch was created so that students could have a file containing the basic
> notices informations. This would enable them to copy and paste these
> informations more easily into their assignments. Therefore, HTML files are
> not generated for importing notices afterward, which is why they are
> incomplete/invalid for import.

I didn't assume they were for import or export, but I think it would not be hard to add the necessary HTML elements to make it a valid file. This will also avoid issues with display in different browsers that might handle invalid diffferently.


> I also have some questions regarding point 7. To make sure I understand
> correctly, should we add a system preference (e.g., CustomExportFilePath) so
> that libraries can choose where they want to place their custom XSL file? If
> no path is specified, would the default path be to the
> "MARC21_simple_export.xsl" files? Additionally, should we consider renaming
> these files to "MARC21CustomSimpleExport.xsl" to align with the naming
> conventions of other .xsl files?

I need a bit more time for this one.

(In reply to Katrin Fischer from comment #183)
> Do you have the latest version of QA script? (separate repository from Koha)

Yes, I use the last version of QA Test Tools : https://wiki.koha-community.org/wiki/QA_Test_Tools

I can confirm the QA script failures.. 'ktd pull -> ktd up -> ktd --shell -> qa'

I tend to run `qa --more-tests` but that won't make any difference here other than to try and run some additional tests if it thinks there are any related.

I also agree the hardcoded xslt filepath is incorrect.. those files would get wiped out/replaced at any upgrade as they're assumed to be community maintained. The other xslt driven preferences take a path to allow for overriden sheets that you maintain yourselves.

In most cases those custom versions would be under /var/lib/koha/*site*/ for example.. these won't get nuked at upgrades.

Created attachment 159142 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 159143 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 159144 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 159145 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 159146 [details] [review]
Bug 17385: Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList

Created attachment 159147 [details] [review]
Bug 17385: (QA follow-up) Add CustomXSLTExportPath & improve descriptions

Here is a summary the corrections :
 - Addressed all warnings and failures identified by the QA testing tool
 - Revised the description of the export format (Removed all instances of 'XSL - ')
 - Updated the description of the custom option in the OpacExportOptions system
   preference
 - Corrected the MARC21_simple_export.xsl file to generate a valid HTML file
 - Introduced the new preference system CustomXSLTExportPath, allowing users to choose the path for the custom XSL files
 - Updated the sysprefs.sql file for OpacExportOptions and CustomXSLTExportPath
 - Added a new atomic update file for OpacExportOptions and CustomXSLTExportPath
 - Updated XSLT tests (t/db_dependent/XSLT.t)

NEW TEST PLAN
Prerequisites:
 - Have more than one record in your catalog

Test steps:
1) Search for a record in the intranet
2) Click on the 'Save' button and observe the dropdown menu
    ---> There is no 'Simple Export' in the dropdown menu
3) Apply the patch
4) In 'Koha administration > System preferences' search 'OpacExportOptions'
5) Enable 'Simple HTML formats' and click on 'Save all OPAC preferences'
6) Repeat steps 1 and 2
    ---> There is now 'Simple Export' option in the dropdown menu
7) Click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the record informations in the downloaded file
8) Add some item to your Cart
9) Open your Cart
10) Click on the 'Download' button, then click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the records informations in the downloaded file
11) Add some items to a list
12) Open the list
13) Click on the 'Download list' button, then click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the records informations in the downloaded file
14) Do the same steps in OPAC (steps 6 to 13)
15) On your computer, create a folder that will contain the xsl files in the attachment
16 [review]) In the intranet, go to 'Koha administration > System preferences' and search for 'CustomXSLTExportPath'
17) Write the full path to your created folder and click on the 'Save all Cataloging preferences'
18) Repeat steps 6 to 14 (no need to download this time, only check that 'Simple Export 1' and 'Simple Export 2' are displaying when you click on the 'Save'/'Download'/'Download list' button)
19) In the system preferences search for OpacExportOptions and disable 'Simple HTML formats'
20) Repeat steps 6 to 13 in OPAC
    ---> Notice that 'Simple Export 1' and 'Simple Export 2' are not in the menu anymore
21) Run 'prove t/db_dependent/XSLT.t'

Created attachment 159148 [details]
XSLT test files

Created attachment 163623 [details] [review]
Bug 17385: (QA follow-up) Add CustomXSLTExportPath & improve descriptions

TEST PLAN
Prerequisites:
 - Have more than one record in your catalog

Test steps:
1) Search for a record in the intranet
2) Click on the 'Save' button and observe the dropdown menu
    ---> There is no 'Simple Export' in the dropdown menu
3) Apply the patch
4) In 'Koha administration > System preferences' search 'OpacExportOptions'
5) Enable 'Simple HTML formats' and click on 'Save all OPAC preferences'
6) Repeat steps 1 and 2
    ---> There is now 'Simple Export' option in the dropdown menu
7) Click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the record informations in the downloaded file
8) Add some item to your Cart
9) Open your Cart
10) Click on the 'Download' button, then click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the records informations in the downloaded file
11) Add some items to a list
12) Open the list
13) Click on the 'Download list' button, then click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the records informations in the downloaded file
14) Do the same steps in OPAC (steps 6 to 13)
15) On your computer, create a folder that will contain the xsl files in the attachment
16 [review] [review]) In the intranet, go to 'Koha administration > System preferences' and search for 'CustomXSLTExportPath'
17) Write the full path to your created folder and click on the 'Save all Cataloging preferences'
18) Repeat steps 6 to 14 (no need to download this time, only check that 'Simple Export 1' and 'Simple Export 2' are displaying when you click on the 'Save'/'Download'/'Download list' button)
19) In the system preferences search for OpacExportOptions and disable 'Simple HTML formats'
20) Repeat steps 6 to 13 in OPAC
    ---> Notice that 'Simple Export 1' and 'Simple Export 2' are not in the menu anymore
21) Run 'prove t/db_dependent/XSLT.t'

When viewing a record, opac or staff : 
Could not compile /kohadevbox/koha/opac/opac-detail.pl: "CustomXSLTExportList" is not exported by the C4::XSLT module
Could not compile /kohadevbox/koha/catalogue/detail.pl: "CustomXSLTExportList" is not exported by the C4::XSLT module

I don't understand your problem because when I run the test plan, everything works and I don't get any errors.

Got those errors on kdt and on Biblibre's sandboxes.
No error in PTFS's sandbox.

Tested : everything works OK until 14.
From 15 it is not possible to test on a sandbox.

I have forgotten : 
html files are exported in ISO-8859 encoding, ot utf8, some diacritics are not well encoded in the file, example éèàêë

(In reply to Laurence Rault from comment #199)
> I have forgotten : 
> html files are exported in ISO-8859 encoding, ot utf8, some diacritics are
> not well encoded in the file, example éèàêë

So I need to change how is export ?

Created attachment 170014 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170015 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170016 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170017 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170018 [details] [review]
Bug 17385: Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList

Created attachment 170019 [details] [review]
Bug 17385: (QA follow-up) Add CustomXSLTExportPath & improve descriptions

Here is a summary the corrections :
 - Addressed all warnings and failures identified by the QA testing tool
 - Revised the description of the export format (Removed all instances of 'XSL - ')
 - Updated the description of the custom option in the OpacExportOptions system
   preference
 - Corrected the MARC21_simple_export.xsl file to generate a valid HTML file
 - Introduced the new preference system CustomXSLTExportPath, allowing users to choose the path for the custom XSL files
 - Updated the sysprefs.sql file for OpacExportOptions and CustomXSLTExportPath
 - Added a new atomic update file for OpacExportOptions and CustomXSLTExportPath
 - Updated XSLT tests (t/db_dependent/XSLT.t)

NEW TEST PLAN
Prerequisites:
 - Have more than one record in your catalog

Test steps:
1) Search for a record in the intranet
2) Click on the 'Save' button and observe the dropdown menu
    ---> There is no 'Simple Export' in the dropdown menu
3) Apply the patch
4) In 'Koha administration > System preferences' search 'OpacExportOptions'
5) Enable 'Simple HTML formats' and click on 'Save all OPAC preferences'
6) Repeat steps 1 and 2
    ---> There is now 'Simple Export' option in the dropdown menu
7) Click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the record informations in the downloaded file
8) Add some item to your Cart
9) Open your Cart
10) Click on the 'Download' button, then click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the records informations in the downloaded file
11) Add some items to a list
12) Open the list
13) Click on the 'Download list' button, then click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the records informations in the downloaded file
14) Do the same steps in OPAC (steps 6 to 13)
15) On your computer, create a folder that will contain the xsl files in the attachment
16 [review] [review]) In the intranet, go to 'Koha administration > System preferences' and search for 'CustomXSLTExportPath'
17) Write the full path to your created folder and click on the 'Save all Cataloging preferences'
18) Repeat steps 6 to 14 (no need to download this time, only check that 'Simple Export 1' and 'Simple Export 2' are displaying when you click on the 'Save'/'Download'/'Download list' button)
19) In the system preferences search for OpacExportOptions and disable 'Simple HTML formats'
20) Repeat steps 6 to 13 in OPAC
    ---> Notice that 'Simple Export 1' and 'Simple Export 2' are not in the menu anymore
21) Run 'prove t/db_dependent/XSLT.t'

Created attachment 170026 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170027 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170028 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170029 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170030 [details] [review]
Bug 17385: Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170031 [details] [review]
Bug 17385: (QA follow-up) Add CustomXSLTExportPath & improve descriptions

Here is a summary the corrections :
 - Addressed all warnings and failures identified by the QA testing tool
 - Revised the description of the export format (Removed all instances of 'XSL - ')
 - Updated the description of the custom option in the OpacExportOptions system
   preference
 - Corrected the MARC21_simple_export.xsl file to generate a valid HTML file
 - Introduced the new preference system CustomXSLTExportPath, allowing users to choose the path for the custom XSL files
 - Updated the sysprefs.sql file for OpacExportOptions and CustomXSLTExportPath
 - Added a new atomic update file for OpacExportOptions and CustomXSLTExportPath
 - Updated XSLT tests (t/db_dependent/XSLT.t)

NEW TEST PLAN
Prerequisites:
 - Have more than one record in your catalog

Test steps:
1) Search for a record in the intranet
2) Click on the 'Save' button and observe the dropdown menu
    ---> There is no 'Simple Export' in the dropdown menu
3) Apply the patch
4) In 'Koha administration > System preferences' search 'OpacExportOptions'
5) Enable 'Simple HTML formats' and click on 'Save all OPAC preferences'
6) Repeat steps 1 and 2
    ---> There is now 'Simple Export' option in the dropdown menu
7) Click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the record informations in the downloaded file
8) Add some item to your Cart
9) Open your Cart
10) Click on the 'Download' button, then click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the records informations in the downloaded file
11) Add some items to a list
12) Open the list
13) Click on the 'Download list' button, then click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the records informations in the downloaded file
14) Do the same steps in OPAC (steps 6 to 13)
15) On your computer, create a folder that will contain the xsl files in the attachment
16 [review] [review]) In the intranet, go to 'Koha administration > System preferences' and search for 'CustomXSLTExportPath'
17) Write the full path to your created folder and click on the 'Save all Cataloging preferences'
18) Repeat steps 6 to 14 (no need to download this time, only check that 'Simple Export 1' and 'Simple Export 2' are displaying when you click on the 'Save'/'Download'/'Download list' button)
19) In the system preferences search for OpacExportOptions and disable 'Simple HTML formats'
20) Repeat steps 6 to 13 in OPAC
    ---> Notice that 'Simple Export 1' and 'Simple Export 2' are not in the menu anymore
21) Run 'prove t/db_dependent/XSLT.t'

Signed-off-by: David Nind <david@davidnind.com>

Testing notes (using KTD):
1. After applying the patch: updatedatabase and restart_all
2. OpacExportOptions system preference: 'Simple HTML formats' is already selected.
3. I created a folder and put the files in /kohadevbox/koha/xslt/
4. I had to restart_all after creating the folder and adding the files, otherwise they didn't show up in the download menu.
5. After unselecting 'Simple HTML formats', had to clear the browser cache for them to disappear (a hard refresh didn't work for me).

Created attachment 170983 [details] [review]
Bug 17385: Add custom export XSLT

Rebase

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170984 [details] [review]
Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.

Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170985 [details] [review]
Bug 17385: (QA follow-up) Add caching

Performance: We should add caching in CustomXSLTExportList.

Note: This resolves the lack of an intranet test in the former patch too.

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170986 [details] [review]
Bug 17385: Add tests for CustomXSLTExportList

Tests are now in one patch.
One test does not pass and I am not able to fix it.

The test who fails :

<pre>
$ prove -l t/db_dependent/XSLT.t
t/db_dependent/XSLT.t .. 1/3
    #   Failed test 'This list comes from the cache and that is fine'
    #   at t/db_dependent/XSLT.t line 122.
    #          got: '1'
    #     expected: '2'
    # Looks like you failed 1 test of 1.

 #   Failed test 'CustomXSLTExportList: Caching'
 #   at t/db_dependent/XSLT.t line 124.
 # Looks like you failed 1 test of 3.
t/db_dependent/XSLT.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/db_dependent/XSLT.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=3,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.74 cusr  0.14 csys =  1.91 CPU)
Result: FAIL
</pre>

This one works fine :

<pre>
$ prove -l t/XSLT.t
t/XSLT.t .. ok
All tests successful.
Files=1, Tests=12,  4 wallclock secs ( 0.02 usr  0.00 sys +  2.55 cusr  0.17 csys =  2.74 CPU)
Result: PASS
</pre>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170987 [details] [review]
Bug 17385: Add custum export format option to the cart and the lists and fix tests for CustomXSLTExportList

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170988 [details] [review]
Bug 17385: (QA follow-up) Add CustomXSLTExportPath & improve descriptions

Here is a summary the corrections :
 - Addressed all warnings and failures identified by the QA testing tool
 - Revised the description of the export format (Removed all instances of 'XSL - ')
 - Updated the description of the custom option in the OpacExportOptions system
   preference
 - Corrected the MARC21_simple_export.xsl file to generate a valid HTML file
 - Introduced the new preference system CustomXSLTExportPath, allowing users to choose the path for the custom XSL files
 - Updated the sysprefs.sql file for OpacExportOptions and CustomXSLTExportPath
 - Added a new atomic update file for OpacExportOptions and CustomXSLTExportPath
 - Updated XSLT tests (t/db_dependent/XSLT.t)

NEW TEST PLAN
Prerequisites:
 - Have more than one record in your catalog

Test steps:
1) Search for a record in the intranet
2) Click on the 'Save' button and observe the dropdown menu
    ---> There is no 'Simple Export' in the dropdown menu
3) Apply the patch
4) In 'Koha administration > System preferences' search 'OpacExportOptions'
5) Enable 'Simple HTML formats' and click on 'Save all OPAC preferences'
6) Repeat steps 1 and 2
    ---> There is now 'Simple Export' option in the dropdown menu
7) Click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the record informations in the downloaded file
8) Add some item to your Cart
9) Open your Cart
10) Click on the 'Download' button, then click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the records informations in the downloaded file
11) Add some items to a list
12) Open the list
13) Click on the 'Download list' button, then click on 'Simple Export'
    ---> a .html file should be downloaded
    ---> you should see the records informations in the downloaded file
14) Do the same steps in OPAC (steps 6 to 13)
15) On your computer, create a folder that will contain the xsl files in the attachment
16 [review] [review]) In the intranet, go to 'Koha administration > System preferences' and search for 'CustomXSLTExportPath'
17) Write the full path to your created folder and click on the 'Save all Cataloging preferences'
18) Repeat steps 6 to 14 (no need to download this time, only check that 'Simple Export 1' and 'Simple Export 2' are displaying when you click on the 'Save'/'Download'/'Download list' button)
19) In the system preferences search for OpacExportOptions and disable 'Simple HTML formats'
20) Repeat steps 6 to 13 in OPAC
    ---> Notice that 'Simple Export 1' and 'Simple Export 2' are not in the menu anymore
21) Run 'prove t/db_dependent/XSLT.t'

Signed-off-by: David Nind <david@davidnind.com>