Bug 41324

Summary: Tidy kohaTable block
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: BLOCKED --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: lucas, martin.renvoize, nick, oleonard, paul.derscheid, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
URL: https://gitlab.com/joubu/Koha/-/commits/bug_41324
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 39715, 41563, 41564, 41565, 41566, 41567, 41568, 41569, 41570, 41571, 41572, 41573, 41574, 41575, 41576, 41577, 41578, 41579, 41580, 41581, 41582    
Bug Blocks: 41323    
Attachments: Bug 41324: Add a xt test
Bug 41324: Tidy some kohaTable blocks - examples
Bug 41324: Tidy modified files
Bug 41324: Add a xt test

Description Jonathan Druart 2025-11-27 14:20:54 UTC
Bug 39715 removed quotes around DataTables options. We should enforce it with a test to prevent further occurrences.
Comment 1 Jonathan Druart 2025-11-27 14:32:12 UTC
Created attachment 189994 [details] [review]
Bug 41324: Add a xt test

Patch from commit a307855
Comment 2 Jonathan Druart 2025-11-27 14:32:13 UTC
Created attachment 189995 [details] [review]
Bug 41324: Tidy some kohaTable blocks - examples

Commit using --no-verify to ease rebase
Comment 3 Jonathan Druart 2025-11-27 14:32:15 UTC
Created attachment 189996 [details] [review]
Bug 41324: Tidy modified files

Commit the tidiness

perl misc/devel/tidy.pl $(git diff --name-only HEAD~1)
Comment 4 Jonathan Druart 2025-11-27 14:42:17 UTC
Patches for discussion.

One last thing that is not tidy is the script tags from the .tt files.

If script tags have TT tags, the prettier plugin cannot tidy them.

I was trying to provide a solution to prevent regressions for bug 39715. I think this is great, but it will introduce conflicts.

I am happy to continue, but I first need to know if people want that.

The idea is to use JS variables instead of TT variables, and have the assignment in a first script block.

example:

```
<script>
    // This cannot be tidy
    [% IF foo %]
        # do something
        [% ELSE %]
        let foo = 42;
    [% END %]
</script>
```

=>

```
<script>
    // This still cannot be tidy
    let foo = [% foo ? 1 : 0 %];
</script>

<script>
    // But now this will be tidy automatically!
    if ( foo ) {
        # do something
    } else {
        let foo = 42;
    }
</script>
```

On this bug report I will mainly focus on kohaTable, but it may actually impact whole template file. See also bug 41323 for the "remaining ones".
Comment 5 Owen Leonard 2026-01-05 13:03:45 UTC
I think this looks great, and is very much in line with what I was trying to do with bugs like bug 23885.
Comment 6 Jonathan Druart 2026-01-09 14:55:02 UTC
To achieve this, one child bug report has been created per module.

Here is the test plan for all of them.

1. Variable serialization
In most of the scripts we need to use Template::Toolkit variable (Perl) for JavaScript logic. We are simply going to serialize them.
eg.
  my js_var = [% my_tt_var | html %];

That might generate
  my js_var = ;
if the TT variable is not defined. To prevent invalid JS code we are going to surround it with quotes:
  my js_var = "[% my_tt_var | html %]";

This might be a problem when we compare strings and integer (keep in mind when testing).

When we want to generate a boolean we will use:
  const js_var = [% tt_var ? 1 : 0 | html %];
No need to quote.

2. Use of permissions and sysprefs
See 41562. For permissions and sysprefs we are going to use a global variable that will be shared between the different script tags. That will avoid situations where a JS variable is erased later in a separate script tag.

3. "table_settings" variable
There may be a problem with the "table_settings" variable. In some scripts we have several variables named "table_settings", that causes problem in DT init.
To prevent that we are simply going to rename the variable to make it more specific.
Comment 7 Jonathan Druart 2026-01-09 14:56:06 UTC
Created attachment 191189 [details] [review]
Bug 41324: Add a xt test

Patch from commit 170433c
Comment 8 Jonathan Druart 2026-01-09 14:57:47 UTC
With all related bugs applied there is only one failure: 
koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt

it's a tricky one, I will clean it later.
Comment 9 Jonathan Druart 2026-01-09 14:57:52 UTC
All this is more than just about kohaTable: it tidy all script tags in the templates with kohaTable.
Comment 10 Jonathan Druart 2026-01-09 14:58:30 UTC
The whole tree is available at https://gitlab.com/joubu/Koha/-/commits/bug_41324
Comment 11 Jonathan Druart 2026-01-09 15:14:47 UTC
Why do we tidy script tags?


1. Consistency
The entire codebase is already tidy, except script tags.

2. Detect error early
Running Prettier on JS code helps catch syntax errors and potential issues earlier


3. Clear separation of concerns
Isolate TT and JS variables make code more robust and maintainable. This separation also paves the way for migrating JS logic to dedicated .js files

4. Predictable refactoring
Cleaning up script tags in bulk prevents unexpected large diffs later. Without this the tidy script might suddenly reformat a script tag when the last TT tag is removed, confusing the author of the change