[Bug 30915] New: "Scalar" TT plugin no longer needed
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Bug ID: 30915 Summary: "Scalar" TT plugin no longer needed Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 23166, 29844 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 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 [Bug 23166] Simplify code related to orders in catalogue/*detail.pl https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29844 [Bug 29844] Remove uses of wantarray in Koha::Objects -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | Status|NEW |ASSIGNED CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 184468 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184468&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 184469 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184469&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Quoting Martin from bug 28883: I don't hugely like the use of Context.Scalar in templates.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184468|0 |1 is obsolete| | Attachment #184469|0 |1 is obsolete| | --- Comment #5 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 185108 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185108&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #6 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 185109 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185109&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 185117 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185117&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This patch set removes the release notes| |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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185108|0 |1 is obsolete| | Attachment #185109|0 |1 is obsolete| | Attachment #185117|0 |1 is obsolete| | --- Comment #10 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 186719 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186719&action=edit 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> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #11 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 186720 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186720&action=edit 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> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #12 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 186721 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186721&action=edit 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> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186719|0 |1 is obsolete| | Attachment #186720|0 |1 is obsolete| | Attachment #186721|0 |1 is obsolete| | --- Comment #13 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 186724 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186724&action=edit 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> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #14 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 186725 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186725&action=edit 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> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #15 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 186726 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186726&action=edit 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> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #16 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 186727 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186727&action=edit Bug 30915: Fix spelling WARN spelling nWe ==> new Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #17 from Jonathan Druart <jonathan.druart@gmail.com> --- We overlapped, Tomas. I don't understand the QA failure, I am investigating it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #18 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- The spelling check is not understanding \n is a escaped new line . -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #19 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Jonathan Druart from comment #17)
We overlapped, Tomas.
I don't understand the QA failure, I am investigating it.
Fixed, https://gitlab.com/koha-community/qa-test-tools/-/commit/9c800d3c54c3ff11861... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #20 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #18)
The spelling check is not understanding \n is a escaped new line .
I've fixed this one, see the last patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 --- Comment #21 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | CC| |Laura.escamilla@bywatersolu | |tions.com Status|Pushed to main |Needs documenting --- Comment #22 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- This is an enhancement and will not be backported to the 25.05.x branch. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org