Bug 38340

Summary: Translatability - Standard form include files are not translatable
Product: Koha Reporter: Pedro Amorim <pedro.amorim>
Component: ILLAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Failed QA --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: blawlor, jonathan.druart, m.de.rooy, pedro.amorim, tomascohen, victor
Version: MainKeywords: RM_priority
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35716
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36221
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38339
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 35570    
Bug Blocks: 38523    
Attachments: Bug 38340: Move templates
Bug 38340: Make Standard ILL backend templates translatable
Bug 38340: Move templates
Bug 38340: Make Standard ILL backend templates translatable
Bug 38340: Move templates
Bug 38340: Make Standard ILL backend templates translatable
Bug 38340: Move templates
Bug 38340: Make Standard ILL backend templates translatable
Bug 38340: (follow-up) Refactor and tidy template files
Bug 38340: (follow-up) Refactor and tidy template files
Bug 38340: Move templates
Bug 38340: Make Standard ILL backend templates translatable
Bug 38340: (follow-up) Refactor and tidy template files

Description Pedro Amorim 2024-11-04 12:52:10 UTC
.inc files may need to be moved to koha-tmpl and opac-tmpl
Comment 1 Katrin Fischer 2024-12-10 12:08:01 UTC
I am upping the severity here because this is not only about the staff interface, but also about the OPAC (end users). The ILL request form is not translatable.
Comment 2 Katrin Fischer 2025-01-23 14:41:51 UTC
I think this was a major miss when moving FreeForm to core and really should be fixed for stable releases.
Comment 3 Tomás Cohen Arazi (tcohen) 2025-02-10 13:43:52 UTC
Hi all. Is there any short-term plans to work on this? It affects es-ES users right now.
Comment 4 Jonathan Druart 2025-02-10 13:49:41 UTC
When did we decide that having templates under the Koha namespace was a good idea?
Comment 5 Katrin Fischer 2025-02-10 13:52:23 UTC
It sounds like moving them out of there... could fix the translation issue right away if they were part of the normal template structure. Is that a correct assumption?
Comment 6 Pedro Amorim 2025-02-10 14:17:26 UTC
Moving the files to koha-tmpl and opac-tmpl will likely fix it but the files will become duplicated.
I was hoping we could get bug 35716 finished before this but I don't think that'll happen.
Comment 7 Tomás Cohen Arazi (tcohen) 2025-02-10 14:21:46 UTC
Hi, I filed bug 39031 a few days ago. I hit that road block when implementing an ILL backend with 'the new schema'.

I feel like we could solve this by:

* Accepting my patch.
* Making Koha::ILL::Backend::Standard implement the right method.
* Put the templates in the right place

I also thought, while in this context, that we should implement methods for the OPAC and the Staff interface instead of a single one. And we could just remove the hardcoded `<interface-includes` dir and just use whatever the method returns.
Comment 8 Katrin Fischer 2025-02-10 14:34:33 UTC
I think there might also be a use case for having them a bit different. For example the "extra fields" you can add in the form are not suitable for a end user right now and should not be shown in OPAC like they are now.
Comment 9 Pedro Amorim 2025-02-10 15:02:01 UTC
I'm on board with any of the proposed solutions. I just think that duplicating any file to both staff and OPAC templates is a bad practice we've been perpetuating for ever in Koha.
Duplicating files for both staff+OPAC where the files are an exact match or a >95% overlap should be an option, sure, but not enforced. This is why I mentioned bug 35716 but, again, I understand that it may be out of scope here.

(In reply to Katrin Fischer from comment #8)
> I think there might also be a use case for having them a bit different. For
> example the "extra fields" you can add in the form are not suitable for a
> end user right now and should not be shown in OPAC like they are now.

This is how FreeForm has always worked. I understand this is not a vallid excuse, but some functionality may actually make use of it e.g. openURL. Some third-parties that link back to Koha using openURL may return additional key:values that are not considered in the hardcoded form fields and this information may be needed for reporting or something similar.
I'm not saying this is a huge blocker, I'm saying there's a use case where the additional fields may be needed even though the user may never have the need to use'em. Perhaps others.
Comment 10 Katrin Fischer 2025-02-10 15:37:59 UTC
Less discuss those fields elsewhere, I think we could find a way to make it work better for manual entry and the semi-automated ones, but later. :)

I know duplication is not good, but I feel translatability and a fix for stable releases trumps that concern here for now if we don't have something up our sleeves to fix this is in a way we can also safely backport.
Comment 11 Tomás Cohen Arazi (tcohen) 2025-02-10 17:10:59 UTC
(In reply to Pedro Amorim from comment #9)
> I'm on board with any of the proposed solutions. I just think that
> duplicating any file to both staff and OPAC templates is a bad practice
> we've been perpetuating for ever in Koha.
> Duplicating files for both staff+OPAC where the files are an exact match or
> a >95% overlap should be an option, sure, but not enforced. This is why I
> mentioned bug 35716 but, again, I understand that it may be out of scope
> here.

I think we can focus on the problem to solve (i.e. templates not being translatable) and then have the discussion about 35716, as it has a broader impact on the project. But I fully understand where you are coming from.

I'll try to improve 39031 and then use it on this bug for moving the templates.

> (In reply to Katrin Fischer from comment #8)
> > I think there might also be a use case for having them a bit different. For
> > example the "extra fields" you can add in the form are not suitable for a
> > end user right now and should not be shown in OPAC like they are now.
> 
> This is how FreeForm has always worked. I understand this is not a vallid
> excuse, but some functionality may actually make use of it e.g. openURL.
> Some third-parties that link back to Koha using openURL may return
> additional key:values that are not considered in the hardcoded form fields
> and this information may be needed for reporting or something similar.
> I'm not saying this is a huge blocker, I'm saying there's a use case where
> the additional fields may be needed even though the user may never have the
> need to use'em. Perhaps others.

I agree this could be discussed on some other bug, as Katrin said.
Comment 12 Tomás Cohen Arazi (tcohen) 2025-02-11 11:18:54 UTC
I'm gonna give this a try now.
Comment 13 Tomás Cohen Arazi (tcohen) 2025-02-11 18:53:30 UTC
Created attachment 177749 [details] [review]
Bug 38340: Move templates

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Tomás Cohen Arazi (tcohen) 2025-02-11 18:53:33 UTC
Created attachment 177750 [details] [review]
Bug 38340: Make Standard ILL backend templates translatable

This patchset moves the `Standard` backend templates into the right
places so they are picked up for translation.

To test:
1. Apply this patches
2. Notice the `Standard` ILL backend works as expected
3. Run:
   $ ktd --shell
  k$ cd misc/translator/po
  k$ git fetch ; git reset --hard origin/main
  k$ cd ..
  k$ perl translate update es-ES
  k$ cd po
  k$ git diff
4. Inside the diff pager, make a search for 'backends/Standard'
=> SUCCESS: There are strings that come from the Standard backend!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 David Nind 2025-02-13 22:00:51 UTC
Created attachment 178057 [details] [review]
Bug 38340: Move templates

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 16 David Nind 2025-02-13 22:00:54 UTC
Created attachment 178058 [details] [review]
Bug 38340: Make Standard ILL backend templates translatable

This patchset moves the `Standard` backend templates into the right
places so they are picked up for translation.

To test:
1. Apply this patches
2. Notice the `Standard` ILL backend works as expected
3. Run:
   $ ktd --shell
  k$ cd misc/translator/po
  k$ git fetch ; git reset --hard origin/main
  k$ cd ..
  k$ perl translate update es-ES
  k$ cd po
  k$ git diff
4. Inside the diff pager, make a search for 'backends/Standard'
=> SUCCESS: There are strings that come from the Standard backend!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 17 Marcel de Rooy 2025-02-14 08:16:17 UTC
Created attachment 178079 [details] [review]
Bug 38340: Move templates

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 18 Marcel de Rooy 2025-02-14 08:16:20 UTC
Created attachment 178080 [details] [review]
Bug 38340: Make Standard ILL backend templates translatable

This patchset moves the `Standard` backend templates into the right
places so they are picked up for translation.

To test:
1. Apply this patches
2. Notice the `Standard` ILL backend works as expected
3. Run:
   $ ktd --shell
  k$ cd misc/translator/po
  k$ git fetch ; git reset --hard origin/main
  k$ cd ..
  k$ perl translate update es-ES
  k$ cd po
  k$ git diff
4. Inside the diff pager, make a search for 'backends/Standard'
=> SUCCESS: There are strings that come from the Standard backend!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 19 Jonathan Druart 2025-02-14 12:56:26 UTC
Please run the QA script.
Most of templates are no tidy here.

They have been ignored during the big tidy because they were in an unusual directory and because they are very wrong.

See "exclude" in get_js_files and get_tt_files from tidy.pl

Note that this exclusion list is rewritten on bug 39115.
Comment 20 Jonathan Druart 2025-02-14 12:59:48 UTC
> Most of templates are no tidy here.

Just some of them actually.
Comment 21 Jonathan Druart 2025-02-14 13:00:09 UTC
koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard
├── cancel.inc
├── confirm.inc
├── create.inc
├── edititem.inc
├── illview.inc
├── inc
│   ├── custom_fields.inc
│   ├── form_input_helpers.inc
│   ├── forms
│   │   ├── article.inc
│   │   ├── book.inc
│   │   ├── chapter.inc
│   │   ├── conference.inc
│   │   ├── dvd.inc
│   │   ├── journal.inc
│   │   ├── other.inc
│   │   ├── resource.inc
│   │   └── thesis.inc
│   └── shared.js
└── migrate.inc


Do we have inc of inc?
Comment 22 Tomás Cohen Arazi (tcohen) 2025-02-14 13:02:21 UTC
(In reply to Jonathan Druart from comment #21)
> koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard
> ├── cancel.inc
> ├── confirm.inc
> ├── create.inc
> ├── edititem.inc
> ├── illview.inc
> ├── inc
> │   ├── custom_fields.inc
> │   ├── form_input_helpers.inc
> │   ├── forms
> │   │   ├── article.inc
> │   │   ├── book.inc
> │   │   ├── chapter.inc
> │   │   ├── conference.inc
> │   │   ├── dvd.inc
> │   │   ├── journal.inc
> │   │   ├── other.inc
> │   │   ├── resource.inc
> │   │   └── thesis.inc
> │   └── shared.js
> └── migrate.inc
> 
> 
> Do we have inc of inc?

I didn't choose the top level `.inc` extension instead of just `.tt`. That was like 9 years ago and someone else. This is an ILL backend design thing. I'm not introducing it, just moving the files somewhere else to make them translatable.
Comment 23 Tomás Cohen Arazi (tcohen) 2025-02-20 13:39:23 UTC
Created attachment 178414 [details] [review]
Bug 38340: Move templates

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 24 Tomás Cohen Arazi (tcohen) 2025-02-20 13:39:26 UTC
Created attachment 178415 [details] [review]
Bug 38340: Make Standard ILL backend templates translatable

This patchset moves the `Standard` backend templates into the right
places so they are picked up for translation.

To test:
1. Apply this patches
2. Notice the `Standard` ILL backend works as expected
3. Run:
   $ ktd --shell
  k$ cd misc/translator/po
  k$ git fetch ; git reset --hard origin/main
  k$ cd ..
  k$ perl translate update es-ES
  k$ cd po
  k$ git diff
4. Inside the diff pager, make a search for 'backends/Standard'
=> SUCCESS: There are strings that come from the Standard backend!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 25 Tomás Cohen Arazi (tcohen) 2025-02-20 13:39:28 UTC Comment hidden (obsolete)
Comment 26 Tomás Cohen Arazi (tcohen) 2025-02-20 13:41:29 UTC Comment hidden (obsolete)
Comment 27 Tomás Cohen Arazi (tcohen) 2025-02-20 14:08:30 UTC
Created attachment 178418 [details] [review]
Bug 38340: (follow-up) Refactor and tidy template files

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 28 Tomás Cohen Arazi (tcohen) 2025-02-20 14:16:01 UTC
I moved some code around, so this deserves a new testing round.
Comment 29 Brendan Lawlor 2025-02-20 14:50:38 UTC
Created attachment 178420 [details] [review]
Bug 38340: Move templates

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 30 Brendan Lawlor 2025-02-20 14:50:40 UTC
Created attachment 178421 [details] [review]
Bug 38340: Make Standard ILL backend templates translatable

This patchset moves the `Standard` backend templates into the right
places so they are picked up for translation.

To test:
1. Apply this patches
2. Notice the `Standard` ILL backend works as expected
3. Run:
   $ ktd --shell
  k$ cd misc/translator/po
  k$ git fetch ; git reset --hard origin/main
  k$ cd ..
  k$ perl translate update es-ES
  k$ cd po
  k$ git diff
4. Inside the diff pager, make a search for 'backends/Standard'
=> SUCCESS: There are strings that come from the Standard backend!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 31 Brendan Lawlor 2025-02-20 14:50:42 UTC
Created attachment 178422 [details] [review]
Bug 38340: (follow-up) Refactor and tidy template files

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 32 Brendan Lawlor 2025-02-20 14:53:40 UTC
Followed the test plan and found in the diff many lines from backends/Standard that looked something like this:
 
+#: koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard/inc/forms/article.inc:13
+#: koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard/inc/forms/chapter.inc:12
+#: koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard/inc/forms/resource.inc:12
+#: koha-tmpl/intranet-tmpl/prog/en/modules/tools/additional-contents.tt:32
+#: koha-tmpl/intranet-tmpl/prog/en/modules/tools/additional-contents.tt:358
+#: koha-tmpl/opac-tmpl/bootstrap/en/includes/ill/backends/Standard/inc/forms/article.inc:13
+#: koha-tmpl/opac-tmpl/bootstrap/en/includes/ill/backends/Standard/inc/forms/chapter.inc:12
+#: koha-tmpl/opac-tmpl/bootstrap/en/includes/ill/backends/Standard/inc/forms/resource.inc:12
+msgid "Pages"
+msgstr "Páginas"
Comment 33 Marcel de Rooy 2025-02-21 09:20:08 UTC
From qa tools:

An error occurred : Cannot open /usr/share/koha/Koha/ILL/Backend/intra-includes/create.inc (No such file or directory) at /usr/share/koha/C4/TTParser.pm line 70.
Comment 34 Marcel de Rooy 2025-02-21 09:23:50 UTC
 FAIL   koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard/create.inc
   FAIL   filters
                missing_filter at line 117 (            var core = [ [% whole.core %] ];)

 FAIL   koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard/edititem.inc
   FAIL   filters
                missing_filter at line 87 (            var core = [ [% whole.core %] ];)

 FAIL   koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard/migrate.inc
   FAIL   filters
                missing_filter at line 110 (            var core = [ [% whole.core %] ];)

 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/includes/ill/backends/Standard/create.inc
   FAIL   filters
                missing_filter at line 111 (        var core = [ [% whole.core %] ];)