Bug 40164 - Add Template::Plugin::JSON to handle JSON in Template Toolkit
Summary: Add Template::Plugin::JSON to handle JSON in Template Toolkit
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: David Cook
URL:
Keywords: dependency
Depends on:
Blocks:
 
Reported: 2025-06-18 14:38 UTC by Andrew Fuerste-Henry
Modified: 2025-06-22 23:32 UTC (History)
4 users (show)

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


Attachments
Bug 40164: Add Template::Plugin::JSON dependency (777 bytes, patch)
2025-06-18 14:46 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 40164: Add Template::Plugin::JSON dependency (828 bytes, patch)
2025-06-19 00:21 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2025-06-18 14:38:54 UTC
Prior to Bug 39184, some libraries have been using PERL in their notices to parse JSON data. With PERL disallowed in notices, we need a way to handle JSON using Template Toolkit. 

https://metacpan.org/pod/Template::Plugin::JSON
Comment 1 Tomás Cohen Arazi (tcohen) 2025-06-18 14:46:48 UTC
Created attachment 183334 [details] [review]
Bug 40164: Add Template::Plugin::JSON dependency

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 David Cook 2025-06-19 00:20:13 UTC
I've taken a look at Template::Plugin::JSON, JSON, and JSON::XS and I think overall this should be OK.

There are some security considerations mentioned in JSON::XS, but I don't think it's anything that isn't already a potential problem.

I thought about creating a Template::Plugin::XML at one point, but it had too many security issues, so I ended up raising bug 39296 to add a Template plugin that lets you take MARCXML and manipulate it with MARC::Record. You might find that one interesting too.
Comment 3 David Cook 2025-06-19 00:21:14 UTC
I added bug 39184 as a dependency but technically it's not.
Comment 4 David Cook 2025-06-19 00:21:37 UTC
Created attachment 183362 [details] [review]
Bug 40164: Add Template::Plugin::JSON dependency

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 5 Jonathan Druart 2025-06-19 06:55:54 UTC
I think we should have a test for this.
Comment 6 David Cook 2025-06-19 23:31:33 UTC
(In reply to Jonathan Druart from comment #5)
> I think we should have a test for this.

Not a bad idea. What do you have in mind?
Comment 7 Tomás Cohen Arazi (tcohen) 2025-06-20 13:30:52 UTC
(In reply to Jonathan Druart from comment #5)
> I think we should have a test for this.

Do we actually have any test for notices using *any* TT plugin?
Comment 8 David Cook 2025-06-22 23:29:35 UTC
(In reply to Tomás Cohen Arazi (tcohen) from comment #7)
> (In reply to Jonathan Druart from comment #5)
> > I think we should have a test for this.
> 
> Do we actually have any test for notices using *any* TT plugin?

Touché! I don't know specifically about tests for notices using TT plugins, but we do have some unit tests for home-made TT plugins like t/Koha/Plugins/SafeURL.t, 

It's not clear to me what we'd be trying to test when bringing in a third-party plugin though. I suppose we could test that the plugin does what we're hoping/wanting/expecting it to do, so that if there is an upstream change we catch it early. But I don't know.
Comment 9 Tomás Cohen Arazi (tcohen) 2025-06-22 23:32:05 UTC
(In reply to Tomás Cohen Arazi (tcohen) from comment #7)
> (In reply to Jonathan Druart from comment #5)
> > I think we should have a test for this.
> 
> Do we actually have any test for notices using *any* TT plugin?

I agree it would be useful to have a test, using this TT plugin. And document how to use it for end users. The test should help us detect breaking changes in the plugin. I agree with adding them. Maybe in a separate bug. Will try tomorrow at the office