Bug 41237

Summary: OPAC created requests ignore library selection, always default to patron's library
Product: Koha Reporter: Pedro Amorim <pedro.amorim>
Component: ILLAssignee: Pedro Amorim <pedro.amorim>
Status: Signed Off --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: katrin.fischer, lisette, martin.renvoize, oleonard, pedro.amorim, s.graham4, tomascohen
Version: Main   
Hardware: All   
OS: All   
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:
Circulation function:
Bug Depends on: 35151    
Bug Blocks:    
Attachments: Bug 41237: Update fetch of patron's branchcode to search additional_contents by
Bug 41237: Add tests
Bug 41237: Update fetch of patron's branchcode to search additional_contents by
Bug 41237: Add tests
[24.11.x] Bug 41237: Update fetch of patron's branchcode to search additional_contents by

Description Pedro Amorim 2025-11-11 15:02:32 UTC

    
Comment 1 Pedro Amorim 2025-11-11 15:09:59 UTC
I believe this regression was introduced by bug 35151, by setting the $params->{branchcode} to the patron's branchcode, overwriting the form selection.
The intent of that code was so that the patron's branchcode would then be used as a search parameter for the respective's library ILLModuleCopyrightClearance additional contents, but caused this unintended side effect. 

To reproduce:
1) Enable ILLModule
2) Logged in as koha:koha, create an OPAC request, visit:
<opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
3) Pick any library not Centerville (the default library of the 'koha' user)
4) Notice the created request's library is CPL (Centerville). It ignored the library selection from the form.
Comment 2 Pedro Amorim 2025-11-11 15:27:52 UTC
Created attachment 189491 [details] [review]
Bug 41237: Update fetch of patron's branchcode to search additional_contents by
Comment 3 Pedro Amorim 2025-11-11 15:27:55 UTC
Created attachment 189492 [details] [review]
Bug 41237: Add tests

Test plan, k-t-d:
1) Apply tests patch, run:
prove t/db_dependent/Koha/ILL/Requests.t
2) Notice it fails. Apply the second patch. Repeat. Notice it passes.

Test failure explanation (before fix patch):
    #   Failed test 'Cardnumber not supplied and couldnt find additional_contents for all libraries. Dont show copyrightclearance'
    #   at t/db_dependent/Koha/ILL/Requests.t line 1067.
    #          got: 'copyrightclearance'
    #     expected: 'commit'

Its searching the copyrightclearance additional_contents for the provided branchcode param (illrq->patron->branchcode) (and finding one) when it shouldn't. That parameter is only to be utilized for the newly created ILL request's library. The branchcode filter of the additional_contents must the be logged in user's cardnumber, not params->{branchcode}. If no logged in user exists and this stage is reached (ILLOpacUnauthenticatedRequest), then only 'All libraries' ILLModuleCopyrightClearance are to be fetched, as expected.

Human test plan, before applying patches:
1) Enable ILLModule
2) Logged in as koha:koha, create an OPAC request, visit:
<opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
3) Pick any library not Centerville (the default library of the 'koha' user)
4) Notice the created request's library is CPL (Centerville). It ignored the library selection from the form.
5) Apply patches. Repeat. Notice the request's library is now the one that was selected on the form.
Comment 4 Pedro Amorim 2025-11-11 17:04:07 UTC
Created attachment 189493 [details] [review]
Bug 41237: Update fetch of patron's branchcode to search additional_contents by

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 5 Pedro Amorim 2025-11-11 17:04:10 UTC
Created attachment 189494 [details] [review]
Bug 41237: Add tests

Test plan, k-t-d:
1) Apply tests patch, run:
prove t/db_dependent/Koha/ILL/Requests.t
2) Notice it fails. Apply the second patch. Repeat. Notice it passes.

Test failure explanation (before fix patch):
    #   Failed test 'Cardnumber not supplied and couldnt find additional_contents for all libraries. Dont show copyrightclearance'
    #   at t/db_dependent/Koha/ILL/Requests.t line 1067.
    #          got: 'copyrightclearance'
    #     expected: 'commit'

Its searching the copyrightclearance additional_contents for the provided branchcode param (illrq->patron->branchcode) (and finding one) when it shouldn't. That parameter is only to be utilized for the newly created ILL request's library. The branchcode filter of the additional_contents must the be logged in user's cardnumber, not params->{branchcode}. If no logged in user exists and this stage is reached (ILLOpacUnauthenticatedRequest), then only 'All libraries' ILLModuleCopyrightClearance are to be fetched, as expected.

Human test plan, before applying patches:
1) Enable ILLModule
2) Logged in as koha:koha, create an OPAC request, visit:
<opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
3) Pick any library not Centerville (the default library of the 'koha' user)
4) Notice the created request's library is CPL (Centerville). It ignored the library selection from the form.
5) Apply patches. Repeat. Notice the request's library is now the one that was selected on the form.

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 6 Pedro Amorim 2025-11-12 14:15:05 UTC
Created attachment 189514 [details] [review]
[24.11.x] Bug 41237: Update fetch of patron's branchcode to search additional_contents by

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>