Bug 36752 - Remove TODO about missing summary info in the SIP2 code
Summary: Remove TODO about missing summary info in the SIP2 code
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-05-02 11:32 UTC by Marcel de Rooy
Modified: 2026-02-26 16:06 UTC (History)
1 user (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
26.05.00
Circulation function:


Attachments
Bug 36752: Remove confusing TODOs (1.46 KB, patch)
2026-02-20 10:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 36752: Remove confusing TODOs (1.47 KB, patch)
2026-02-20 13:16 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2024-05-02 11:32:59 UTC
If you set Y (at the right position) in the 63 summary request, the 64 should contain "charged items", i.e. the items that you have on loan.

The code for handle_patron_info just contains a TODO here :)
Comment 1 Jan Kissig 2026-02-17 11:57:21 UTC
(In reply to Marcel de Rooy from comment #0)
> If you set Y (at the right position) in the 63 summary request, the 64
> should contain "charged items", i.e. the items that you have on loan.
> 
> The code for handle_patron_info just contains a TODO here :)

Can you still confirm the issue?

I checked on main via summary '  Y' (charged items)

perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l CPL --patron 23529000035676 -m patron_information -s '  Y     '
...
SEND: 6300120260217    114817  Y         AOCPL|AA23529000035676|ACterm1|
READ: 64              00120260217    114817000000000002000000000000AOCPL|AA23529000035676|AEHenry Acevedo|BLY|BHUSD|BV0|CC5|AU39999000006117|AU39999000006094|BD4345 Library Rd. Springfield, MA 44224|BF(212) 555-1212|PB19580530|PCS|PIY|AFGreetings from Koha. |

The response included the items in repeating AU fields
Comment 2 Marcel de Rooy 2026-02-20 10:00:11 UTC
(In reply to Jan Kissig from comment #1)
> (In reply to Marcel de Rooy from comment #0)
> > If you set Y (at the right position) in the 63 summary request, the 64
> > should contain "charged items", i.e. the items that you have on loan.
> > 
> > The code for handle_patron_info just contains a TODO here :)
> 
> Can you still confirm the issue?
> 
> I checked on main via summary '  Y' (charged items)
> 
> perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l
> CPL --patron 23529000035676 -m patron_information -s '  Y     '
> ...
> SEND: 6300120260217    114817  Y         AOCPL|AA23529000035676|ACterm1|
> READ: 64              00120260217   
> 114817000000000002000000000000AOCPL|AA23529000035676|AEHenry
> Acevedo|BLY|BHUSD|BV0|CC5|AU39999000006117|AU39999000006094|BD4345 Library
> Rd. Springfield, MA 44224|BF(212) 555-1212|PB19580530|PCS|PIY|AFGreetings
> from Koha. |
> 
> The response included the items in repeating AU fields

Ah. Thanks. You are right. They are included via summary_info. Will remove the misleading TODOs in the code.
Comment 3 Marcel de Rooy 2026-02-20 10:11:15 UTC
Created attachment 193506 [details] [review]
Bug 36752: Remove confusing TODOs

As Jan Kissig correctly mentioned on the BZ report, the various
summary info on charged items etc. are included.
This is done via the summary_info sub.

Note that the TODOs about
    BZ FID_HOLD_ITEMS_LMT
    CA FID_OVERDUE_ITEMS_LMT
    CB FID_CHARGED_ITEMS_LMT
are still valid. This is not included yet.

Test plan.
Since we are just removing comments, read the code.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Marcel de Rooy 2026-02-20 10:11:54 UTC
Trivial change (comments only). Self SO
Comment 5 Katrin Fischer 2026-02-20 13:16:40 UTC
Created attachment 193542 [details] [review]
Bug 36752: Remove confusing TODOs

As Jan Kissig correctly mentioned on the BZ report, the various
summary info on charged items etc. are included.
This is done via the summary_info sub.

Note that the TODOs about
    BZ FID_HOLD_ITEMS_LMT
    CA FID_OVERDUE_ITEMS_LMT
    CB FID_CHARGED_ITEMS_LMT
are still valid. This is not included yet.

Test plan.
Since we are just removing comments, read the code.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 6 Lucas Gass (lukeg) 2026-02-26 16:06:15 UTC
Nice work everyone!

Pushed to main for 26.05