Bug 30915 - "Scalar" TT plugin no longer needed
Summary: "Scalar" TT plugin no longer needed
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords: release-notes-needed
Depends on: 23166 29844
Blocks:
  Show dependency treegraph
 
Reported: 2022-06-07 07:50 UTC by Jonathan Druart
Modified: 2025-08-05 12: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:
This patch set removes the Context TT plugin which only contained the Scalar method. This was only used a few times in the codebase. (We do not expect them to be used in notices on larger scale.) The db revision will warn users for (adjusted) notices that contain this construct if any. Occurrences like [% Context.Scalar(x,y) %] can be replaced by [% x.y %]. If x.y returns a list, change to x.y.size.
Version(s) released in:
Circulation function:


Attachments
Bug 30915: Remove Context.Scalar calls from templates (6.21 KB, patch)
2025-07-22 13:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30915: Remove the TT plugin itself (1.96 KB, patch)
2025-07-22 13:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30915: Remove Context.Scalar calls from templates (6.24 KB, patch)
2025-08-04 17:49 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 30915: Remove the TT plugin itself (1.98 KB, patch)
2025-08-04 17:49 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 30915: (QA follow-up) Add warning for notices with Context.Scalar (1.79 KB, patch)
2025-08-05 12:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2022-06-07 07:50:18 UTC
We needed it to workaround the list context issue we had with Koha::Objects methods using wantarray.
Since we remove it (bug 29844) we could remove Koha/Template/Plugin/Context.pm
Comment 1 Marcel de Rooy 2025-07-22 13:04:51 UTC
Created attachment 184468 [details] [review]
Bug 30915: Remove Context.Scalar calls from templates

Test plan:
git grep on "Context.Scalar"
Go to book with holdable items in staff catalogue detail. Check if
Place hold is seen in the horizontal toolbar above the title.
Place an article request with a patron. Check the article requests
tab on patron detail.
Same for the article requests tab on moremember (Patron details).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2025-07-22 13:04:53 UTC
Created attachment 184469 [details] [review]
Bug 30915: Remove the TT plugin itself

Test plan:
git grep Koha::Template::Plugin::Context
git grep "\[% USE Context"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2025-07-22 13:07:35 UTC
Quoting Martin from bug 28883:
I don't hugely like the use of Context.Scalar in templates..
Comment 4 Tomás Cohen Arazi (tcohen) 2025-08-04 17:48:10 UTC
I agree with the change and things look ok.

My only concern is:

* Should we deprecate it first? (e.g. people might be using it in notices)
* In the event we remove it (via deprecation window or not), shouldn't we provide an alternative approach at least in the documentation?

I noticed the feature doesn't seem to have tests implemented.
Comment 5 Tomás Cohen Arazi (tcohen) 2025-08-04 17:49:03 UTC
Created attachment 185108 [details] [review]
Bug 30915: Remove Context.Scalar calls from templates

Test plan:
git grep on "Context.Scalar"
Go to book with holdable items in staff catalogue detail. Check if
Place hold is seen in the horizontal toolbar above the title.
Place an article request with a patron. Check the article requests
tab on patron detail.
Same for the article requests tab on moremember (Patron details).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi (tcohen) 2025-08-04 17:49:06 UTC
Created attachment 185109 [details] [review]
Bug 30915: Remove the TT plugin itself

Test plan:
git grep Koha::Template::Plugin::Context
git grep "\[% USE Context"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Marcel de Rooy 2025-08-05 11:40:41 UTC
(In reply to Tomás Cohen Arazi (tcohen) from comment #4)
> I agree with the change and things look ok.
> 
> My only concern is:
> 
> * Should we deprecate it first? (e.g. people might be using it in notices)
> * In the event we remove it (via deprecation window or not), shouldn't we
> provide an alternative approach at least in the documentation?
> 
> I noticed the feature doesn't seem to have tests implemented.

Thx for signing off here. Legitimate questions!

I assume (..) that we will have not much people using this in notices based on the fact that we only used the construct in our codebase just a few times too.
But I think that we definitely should warn people who do use it in notices. Since this will probably be a small number and manually replacing them should not be so hard, I tend to think that a warning dbrev should be sufficient. Will add that now.

The refactoring pattern is
Replace Context.Scalar( $x, $y ) by just $x.$y if you know that method y (like search and friends) will be a scalar (resultset). This was the case in the templates.
If method y returns a list and was "Scalared" then you need to add a count to get at the same result. So in that case replace Context.Scalar( $x, $y ) by $x.$y.count. Note that this was probably not used like that, since using Scalar for that is unnecessary complexity for $x.$y.count already.
Comment 8 Marcel de Rooy 2025-08-05 11:42:05 UTC
(In reply to Marcel de Rooy from comment #7)
> If method y returns a list and was "Scalared" then you need to add a count
> to get at the same result. So in that case replace Context.Scalar( $x, $y )
> by $x.$y.count. Note that this was probably not used like that, since using
> Scalar for that is unnecessary complexity for $x.$y.count already.

Hmm. Within TT we should use size here.
Comment 9 Marcel de Rooy 2025-08-05 12:17:02 UTC
Created attachment 185117 [details] [review]
Bug 30915: (QA follow-up) Add warning for notices with Context.Scalar

As requested on comment4.

Test plan:
Run the dbrev.
Bonus: Add a Context.Scalar construct in a notice and rerun.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>