Bug 24852 - XSLT paths not adjusted for non-dev_install
Summary: XSLT paths not adjusted for non-dev_install
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-11 16:40 UTC by Jonathan Druart
Modified: 2021-12-13 21:10 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 24852: Adjust XSLT paths for dev installs (4.13 KB, patch)
2020-03-11 16:55 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-03-11 16:40:47 UTC
We are building the XSLT paths with intrahtdocs, it should be intranetdir (and assume that have the usual git structure)
Comment 1 Jonathan Druart 2020-03-11 16:55:01 UTC
Created attachment 100589 [details] [review]
Bug 24852: Adjust XSLT paths for dev installs

We are building the XSLT paths with intrahtdocs, it should be intranetdir
(and assume that have the usual git structure).

For instance, on a dev box, the XSLT base path was outside of the git
repo:
/usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/xslt/

Now it will be:
/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/xslt/

Test plan:
0. Do not apply the patch, set dev_install to 1 in $KOHA_CONF
1. Modify a xslt in the git repo, and in the "htdocs" path, in a
different way:
/usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl
/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl
2. Restart all the things and refresh a bibliographic record detail
page
Notice that you see the change from the "htdocs", outside of the git
repo
3. Apply the patch
4. Repeat 2.
Notice that you now see the change you made on the file from the git
repo
5. Set dev_install to 0 in $KOHA_CONF
6. Repeat 2.
Notice that you now see the change from htdocs
Comment 2 Jonathan Druart 2020-03-11 16:55:51 UTC
We need that, right?
Comment 3 Tomás Cohen Arazi 2020-03-11 17:14:02 UTC
(In reply to Jonathan Druart from comment #2)
> We need that, right?

Yes
Comment 4 Marcel de Rooy 2020-03-12 07:47:33 UTC
Do we need more?

For a gitify I am doing this for koha-conf:

s/koha\/opac\/cgi-bin\/opac/koha\/opac/
s/koha\/opac\/htdocs/koha\/koha-tmpl/
s/koha\/intranet\/cgi-bin/koha/
s/koha\/intranet\/htdocs/koha\/koha-tmpl/
s/doc\/koha-common/koha\/docs/
s/bin\/cronjobs/misc\/cronjobs/
Comment 5 Jonathan Druart 2020-03-12 08:35:25 UTC
Maybe, I do not know exactly what is missing.
In debian scripts we are testing the dev_install config flag, so I think it makes sense to test it from the code as well.

Or we go backward, remove the specific paths and adjust KOHA_CONF?
Comment 6 Marcel de Rooy 2020-03-12 14:06:27 UTC
(In reply to Jonathan Druart from comment #5)
> Maybe, I do not know exactly what is missing.
> In debian scripts we are testing the dev_install config flag, so I think it
> makes sense to test it from the code as well.
> 
> Or we go backward, remove the specific paths and adjust KOHA_CONF?

I am open for both solutions.
From a maintenance perspective it may be easier to adjust some koha-conf paths in a Dockerfile or so than duplicating code in various places.
Note that the dev_install flag helped us to run the debian scripts as-is without gitifying them as well.
Comment 7 Tomás Cohen Arazi 2020-03-12 14:16:22 UTC
I never liked the way we specify things in the koha-conf.xml file :-D

If we did that, we would need:
intrahtdocs
intracgidir
opachtdocs
opaccgidir
and why not something for the API as well...
Comment 8 Marcel de Rooy 2020-03-12 14:32:11 UTC
(In reply to Tomás Cohen Arazi from comment #7)
> I never liked the way we specify things in the koha-conf.xml file :-D
> 
> If we did that, we would need:
> intrahtdocs
> intracgidir
> opachtdocs
> opaccgidir
> and why not something for the API as well...

I have the impression that you did not understand me or vice versa :)
Comment 9 Jonathan Druart 2020-03-12 14:46:46 UTC
I have the feeling that we are going to far. I would go for this patch, it's simple and written. Then we can discuss something bigger on a separate bug report?
Comment 10 Tomás Cohen Arazi 2020-03-12 15:01:22 UTC
(In reply to Marcel de Rooy from comment #8)
> (In reply to Tomás Cohen Arazi from comment #7)
> > I never liked the way we specify things in the koha-conf.xml file :-D
> > 
> > If we did that, we would need:
> > intrahtdocs
> > intracgidir
> > opachtdocs
> > opaccgidir
> > and why not something for the API as well...
> 
> I have the impression that you did not understand me or vice versa :)

I understand what you propose and I like it. I just took the chance to say I never liked our current config. And I love the dev_install thing.
Comment 11 Marcel de Rooy 2020-03-13 10:11:51 UTC
(In reply to Jonathan Druart from comment #9)
> I have the feeling that we are going to far. I would go for this patch, it's
> simple and written. Then we can discuss something bigger on a separate bug
> report?

Well, this would be the first occurrence in a perl module. So it is not only a matter of "I wrote it already".
Comment 12 Jonathan Druart 2020-03-13 11:32:45 UTC
I do not want to overthink this patch. Feel free to suggest something if you think it's not good enough for a first step.
Comment 13 Jonathan Druart 2020-03-31 10:45:06 UTC
Just saying, that's definitely a bug. It works for the OPAC, only.

Please suggest something if you disagree with this patch.
Comment 14 Marcel de Rooy 2020-03-31 11:49:22 UTC
(In reply to Jonathan Druart from comment #13)
> Just saying, that's definitely a bug. It works for the OPAC, only.
> 
> Please suggest something if you disagree with this patch.

I dont object to it. But former comments still apply.
Comment 15 Jonathan Druart 2020-03-31 12:11:32 UTC
(In reply to Marcel de Rooy from comment #14)
> (In reply to Jonathan Druart from comment #13)
> > Just saying, that's definitely a bug. It works for the OPAC, only.
> > 
> > Please suggest something if you disagree with this patch.
> 
> I dont object to it. But former comments still apply.

Which ones? The one where you ask if we need more paths?
Comment 16 Marcel de Rooy 2020-03-31 12:24:35 UTC
(In reply to Jonathan Druart from comment #15)
> (In reply to Marcel de Rooy from comment #14)
> > (In reply to Jonathan Druart from comment #13)
> > > Just saying, that's definitely a bug. It works for the OPAC, only.
> > > 
> > > Please suggest something if you disagree with this patch.
> > 
> > I dont object to it. But former comments still apply.
> 
> Which ones? The one where you ask if we need more paths?

6/11
Comment 17 Victor Grousset/tuxayo 2021-05-01 18:15:15 UTC
> 2. Restart all the things and refresh a bibliographic record detail page
> Notice that you see the change from the "htdocs", outside of the git repo

That's puzzling, I see the git repo version. Can't find the issue after double checking.

> 6. Repeat 2.
> Notice that you now see the change from htdocs

Still git version.

So with and without the patch. With dev_install 1 and dev_install 0, I always see the git repo version.
A diff between the two files confirmed the difference.

Does anyone else has the same issue?
Comment 18 Jonathan Druart 2021-05-05 09:08:23 UTC
(In reply to Victor Grousset/tuxayo from comment #17)
> So with and without the patch. With dev_install 1 and dev_install 0, I
> always see the git repo version.
> A diff between the two files confirmed the difference.
> 
> Does anyone else has the same issue?

Hum indeed. There is something that changed somewhere but I cannot find what/where.
So there is still an issue but less important.

Marking as won't fix (for now at least).