Bug 40939 - Cardnumber not found when performing batch actions from report results
Summary: Cardnumber not found when performing batch actions from report results
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 38664
Blocks:
  Show dependency treegraph
 
Reported: 2025-10-02 15:51 UTC by Andrew Fuerste-Henry
Modified: 2025-10-06 14:45 UTC (History)
4 users (show)

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


Attachments
Bug 40939: Fix batch actions from report results (2.83 KB, patch)
2025-10-06 07:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40939: Fix batch actions from report results (2.88 KB, patch)
2025-10-06 14:45 UTC, Brendan Lawlor
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-10-02 15:51:28 UTC
Koha is failing to find *any* cardnumbers when sending patron accounts to batch actions from report results.

Note that batch actions via borrowernumber do work, providing an easy workaround.

To recreate:
- save and run "SELECT cardnumber FROM borrowers"
- use the Batch Operations button to send all visible cardnumbers to batch patron modification
- see that none of your cardnumbers have been found
Comment 1 Phil Ringnalda 2025-10-03 21:14:54 UTC
It does find the first one, which is a vague clue.

It's actually a Tidy regression: in https://git.koha-community.org/Koha-community/Koha/src/commit/e0ebd409917c6c017234211db8409ae05ed2b787/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt#L1089 Jonathan thought the comment would keep people from indenting the line, but then his autotidy code ignored his comment.

Seems odd that Koha::Patrons->find ignores leading whitespace for borrowernumber but treats it as significant for cardnumber. And if it does have to do that for some historical reason, then the first cardnumber will be broken when it has significant leading whitespace, since we | trim the output of PROCESS batch_list (which is the reason it currently does find the first cardnumber).
Comment 2 Jonathan Druart 2025-10-06 07:36:02 UTC
Created attachment 187437 [details] [review]
Bug 40939: Fix batch actions from report results

Batch patron operations are broken when using cardnumbers

From
  commit d659526b5ae3363de6dc7691a782a45aaf781a46
  Bug 38664: Tidy the whole codebase

-                                                [%- IF header_row.$place.cell == batch_type || header_types.item(header_row.$place.cell) == batch_type %]
-[%# We must not add whitespace to the cardnumbers %][% cells.cell | html %]
+                                            [%- IF header_row.$place.cell == batch_type || header_types.item(header_row.$place.cell) == batch_type %]
+                                                [%# We must not add whitespace to the cardnumbers %][% cells.cell | html %]

The auto tidy added the whitespaces back

This patch suggests to use TT to add the new lines.

+                                                [%- cells.cell | html -%][% "\n" | $raw %][%# We want 1 item per line, without whitespace %]

Test plan:
- save and run "SELECT cardnumber FROM borrowers"
- use the Batch Operations button to send all visible cardnumbers to batch patron modification
- see that all of your cardnumbers have been found
Comment 3 Brendan Lawlor 2025-10-06 14:45:31 UTC
Created attachment 187480 [details] [review]
Bug 40939: Fix batch actions from report results

Batch patron operations are broken when using cardnumbers

From
  commit d659526b5ae3363de6dc7691a782a45aaf781a46
  Bug 38664: Tidy the whole codebase

-                                                [%- IF header_row.$place.cell == batch_type || header_types.item(header_row.$place.cell) == batch_type %]
-[%# We must not add whitespace to the cardnumbers %][% cells.cell | html %]
+                                            [%- IF header_row.$place.cell == batch_type || header_types.item(header_row.$place.cell) == batch_type %]
+                                                [%# We must not add whitespace to the cardnumbers %][% cells.cell | html %]

The auto tidy added the whitespaces back

This patch suggests to use TT to add the new lines.

+                                                [%- cells.cell | html -%][% "\n" | $raw %][%# We want 1 item per line, without whitespace %]

Test plan:
- save and run "SELECT cardnumber FROM borrowers"
- use the Batch Operations button to send all visible cardnumbers to batch patron modification
- see that all of your cardnumbers have been found

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>