Created attachment 46429 [details] Original style Both EPrints and DSpace projects ship XSLT stylesheets that are included in the OAI response so using a web browser to navigate the repository. We should add one ourselves. See the attached screenshots for better understanding this enhancement proposal.
Created attachment 46430 [details] Proposed style
Created attachment 46434 [details] [review] Bug 15527: Add a stylesheet for OAI-PMH to aid usability This patch introduces an XSLT file grabbed from DSpace and a couple css files. To test: - Point to your dev Koha's oai endpoint. For example: http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListMetadataFormats => FAIL: Look how ugly it is for humans. - Apply this patch - Reload => SUCCES: It is styled and provides really useful links to interact with the OAI server verbs. - Sign off :-D Sponsored-by: Universidad Empresarial Siglo 21
Created attachment 46435 [details] [review] Bug 15527: (followup) Allow using custom stylesheets This patch introduces a syspref to let users choose their own XSLT stylesheets for the OAI-PMH front end. Regards Sponsored-by: Universidad Empresarial Siglo 21
Created attachment 46438 [details] [review] Bug 15527: (followup) Avoid replacing spaces for non-breaking spaces This causes the template not to wrap long lines correctly,
It doesn't work for me when your stylesheet is in opac-tmpl/xsl/. I have to put it in opac-tmpl directly, and modify Repository.pm file to make it works. Could you try on your side? I've seen in your screenshot you're working with 'localhost' as your server URL. It may come from there. A remark: you introduce a new syspref OAI-PMH:CustomStylesheet. I think that this kind of parameter belongs to the 'Extended Mode'. The YAML config file could be used to link to a local stylesheet.
(In reply to Frédéric Demians from comment #5) > It doesn't work for me when your stylesheet is in opac-tmpl/xsl/. I have to > put it in opac-tmpl directly, and modify Repository.pm file to make it > works. Could you try on your side? I've seen in your screenshot you're > working with 'localhost' as your server URL. It may come from there. I'm using kohadevbox:ansible. I think the permissions (apache ones) on a regular dev/normal install have diverged from the packages. I'll take a look. > A remark: you introduce a new syspref OAI-PMH:CustomStylesheet. I think that > this kind of parameter belongs to the 'Extended Mode'. The YAML config file > could be used to link to a local stylesheet. That's probably a good idea. I'll dig into it.
Looks very good. Good job, Tomás.
(In reply to Tomás Cohen Arazi from comment #6) > I'm using kohadevbox:ansible. I think the permissions (apache ones) on a > regular dev/normal install have diverged from the packages. I'll take a look. This part in apache conf is the culprit: <DirectoryMatch "__OPAC_WWW_DIR__/.*/(modules|xslt|includes)"> Order deny,allow Deny from all </DirectoryMatch> I will propose a patch fixing that, and using OAI server conf file to define custom XSLT for web view.
Created attachment 46459 [details] [review] Bug 15527: Add a stylesheet for OAI-PMH to aid usability This patch introduces an XSLT file grabbed from DSpace and a couple css files. To test: - Point to your dev Koha's oai endpoint. For example: http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListMetadataFormats => FAIL: Look how ugly it is for humans. - Apply this patch - Reload => SUCCES: It is styled and provides really useful links to interact with the OAI server verbs. - Sign off :-D Sponsored-by: Universidad Empresarial Siglo 21 Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Works as described. Nice looking.
Created attachment 46460 [details] [review] Bug 15527 Custom stylesheet in extended mode Add a parameter to YAML config file in order to specify a local XSLT used style the XML returned by OAI Server. Some doc. Move default XSLT from /opac-tmpl/xslt to /opac-tmpl/bootstrap/css/OAI.xslt.
(In reply to Frédéric Demians from comment #10) > Created attachment 46460 [details] [review] [review] > Bug 15527 Custom stylesheet in extended mode > > Add a parameter to YAML config file in order to specify a local XSLT > used style the XML returned by OAI Server. Some doc. > > Move default XSLT from /opac-tmpl/xslt to > /opac-tmpl/bootstrap/css/OAI.xslt. Tomás, I've signed-off your first patch, and obsoleted the 2 others. You should take a look at mine (2nd patch attached) and sign it whether it seems OK to you. Maybe a third eye (Hector) and sign-off would be welcome. Thanks for the good job.
> > Tomás, I've signed-off your first patch, and obsoleted the 2 others. You > should take a look at mine (2nd patch attached) and sign it whether it seems > OK to you. Maybe a third eye (Hector) and sign-off would be welcome. Thanks > for the good job. For me looks very good I'll signed-off if you don't care Tomás.
Created attachment 46464 [details] [review] [SIGNED-OFF]Bug 15527: Custom stylesheet in extended mode Add a parameter to YAML config file in order to specify a local XSLT used style the XML returned by OAI Server. Some doc. Move default XSLT from /opac-tmpl/xslt to /opac-tmpl/bootstrap/css/OAI.xslt. Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised
Comment on attachment 46464 [details] [review] [SIGNED-OFF]Bug 15527: Custom stylesheet in extended mode Review of attachment 46464 [details] [review]: ----------------------------------------------------------------- ::: Koha/OAI/Server/Repository.pm @@ +111,5 @@ > > # Load configuration file if defined in OAI-PMH:ConfFile syspref > + my $conf = C4::Context->preference("OAI-PMH:ConfFile"); > + if ( $conf ) { > + $self->{ conf } = $conf = LoadFile( $conf ); What's the point of this change? I personally prefer the previous version.
(In reply to Jonathan Druart from comment #14) > Comment on attachment 46464 [details] [review] [review] > [SIGNED-OFF]Bug 15527: Custom stylesheet in extended mode > > Review of attachment 46464 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: Koha/OAI/Server/Repository.pm > @@ +111,5 @@ > > > > # Load configuration file if defined in OAI-PMH:ConfFile syspref > > + my $conf = C4::Context->preference("OAI-PMH:ConfFile"); > > + if ( $conf ) { > > + $self->{ conf } = $conf = LoadFile( $conf ); > > What's the point of this change? I personally prefer the previous version. The $conf variable is used above (line 148) in order to get the new extended 'xslt' parameter: 148 $response->xslt( 149 $conf && $conf->{xslt} 150 ? $conf->{xslt} 151 : "/opac-tmpl/bootstrap/css/OAI.xslt" 152 );
QA comments: 1/ Could you please add the DSpace license (linked to the xslt) to the about page? 2/ Frédéric, why did you move the xslt file to the OPAC css dir? Why not koha-tmpl/opac-tmpl/bootstrap/en/xslt/?
3/ Should the new file be translatable?
> 2/ Frédéric, why did you move the xslt file to the OPAC css dir? Why not > koha-tmpl/opac-tmpl/bootstrap/en/xslt/? See comment 8. The XSL file is accessed directly by the web browser by http. Files in koha-tmpl/opac-tmpl/bootstrap/en/xslt/ can only be accessed by Koha web scripts.
(In reply to Jonathan Druart from comment #17) > 3/ Should the new file be translatable? In my opinion, humble as appropriate, no.
What point(s) prevent this patch to return to 'signed-off' status?
I think the issue at hand is that this patch revealed a new bug in testing where the non-package version of Koha's apache conf no longer matches the package version in behavior and functionality. While appreciated, I think Frédéric's followup is not needed and we should instead fix the bug for non-package installs of Koha. That should solve the issue and once that and the license followup have been submitted this can move forward.
Kyle, my patch was also about adding (In reply to Kyle M Hall from comment #21) > I think the issue at hand is that this patch revealed a new bug in testing > where the non-package version of Koha's apache conf no longer matches the > package version in behavior and functionality. While appreciated, I think > Frédéric's followup is not needed and we should instead fix the bug for > non-package installs of Koha. That should solve the issue and once that and > the license followup have been submitted this can move forward. I agree with this plan... Where is located the Apache conf file? Do you already know what is its difference with etc/koha-http.conf? My followup patch was also introducing a way to specify a local XSLT in extended mode. See this: https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46460&action=diff But it could be added later when this patch will be in master.
(In reply to Frédéric Demians from comment #22) > Kyle, my patch was also about adding (In reply to Kyle M Hall from comment > #21) > > I think the issue at hand is that this patch revealed a new bug in testing > > where the non-package version of Koha's apache conf no longer matches the > > package version in behavior and functionality. While appreciated, I think > > Frédéric's followup is not needed and we should instead fix the bug for > > non-package installs of Koha. That should solve the issue and once that and > > the license followup have been submitted this can move forward. > > I agree with this plan... > > Where is located the Apache conf file? Do you already know what is its > difference with etc/koha-http.conf? > > My followup patch was also introducing a way to specify a local XSLT in > extended mode. See this: > > https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46460&action=diff > > But it could be added later when this patch will be in master. I agree. I think it would be good as a followup on a new bug report. I believe Tomás is going to file the bug and patch for the issue at hand.
(In reply to Jonathan Druart from comment #16) > QA comments: > 1/ Could you please add the DSpace license (linked to the xslt) to the about > page? This is still valid.
Created attachment 49414 [details] [review] Bug 15527: (qa followup) Add DSpace license to the about page Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Created attachment 49451 [details] [review] Bug 15527: Add a stylesheet for OAI-PMH to aid usability This patch introduces an XSLT file grabbed from DSpace and a couple css files. To test: - Point to your dev Koha's oai endpoint. For example: http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListMetadataFormats => FAIL: Look how ugly it is for humans. - Apply this patch - Reload => SUCCES: It is styled and provides really useful links to interact with the OAI server verbs. - Sign off :-D Sponsored-by: Universidad Empresarial Siglo 21 Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Works as described. Nice looking. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 49452 [details] [review] Bug 15527: (qa followup) Add DSpace license to the about page Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to Master - Should be in the May 2016 release. Thanks!