[Bug 35518] New: Call to C4::Context->userenv happens before it's gets populated breaks code logic
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Bug ID: 35518 Summary: Call to C4::Context->userenv happens before it's gets populated breaks code logic Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: slavashishkin@gmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com, nugged@gmail.com Depends on: 17798 We getting results from `C4::Context->userenv` on line 72: my $userenv = C4::Context->userenv; but `C4::Context->userenv` gets populated only far below in `get_template_and_user` call, line 142, this means that everything that we getting further from `$userenv` is empty. And more, because autovivification occurs for the `$userenv` at line 73: my $branch = $userenv->{'branch'} // ''; that does not cause any visible errors, but it causes malfunctions at places where $branch, $desk_id is used, and also where $userenv is accessed, at line 153: if (!C4::Auth::haspermission( $userenv->{id} , { circulate => 'force_checkout' } ) ). To check that we have undef and it gets autovivificated, let's add some dumps into code when we open any borrower, e.g. /cgi-bin/koha/circ/circulation.pl?borrowernumber=19, Let's dump $userenv before and after `$branch = $userenv->{'branch'} // '';` line: 72 my $userenv = C4::Context->userenv; 73 use Data::Dumper; 74 warn __LINE__, ' ', Dumper($userenv); 75 my $branch = $userenv->{'branch'} // ''; 76 warn __LINE__, ' ', Dumper($userenv); this gives in server logs: 74 $VAR1 = undef; 76 $VAR1 = {}; and also to check when `C4::Context->userenv` gets its real values, let's dump before and after the `get_template_and_user()` call at line 142 (line 145 if to continue above dumps code): 145 warn __LINE__, ' ', Dumper([C4::Context->userenv]); 146 my ( $template, $loggedinuser, $cookie ) = get_template_and_user ( ... 153 ); 154 warn __LINE__, ' ', Dumper([C4::Context->userenv]); this gives in server logs: 145 $VAR1 = []; 154 $VAR1 = [ { ... 'id' => 'admin', ... } ]; what depicts the whole bug. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17798 [Bug 17798] Checking out an item on hold for another patron prints a slip but does not update hold -- 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=35518 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |critical CC| |jonathan.druart+koha@gmail. | |com, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com -- 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=35518 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=35518 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 161539 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161539&action=edit Bug 35518: Check authentication and set userenv before fetching userenv variables Currently we get the userenv before we have set it correctly for the session To test: 1 - Sign in as a user with fast cataloging permission 2 - Bring up a patron, type gibberish into barcode field to get a fast cataloging link 3 - Check the link, it should have your current signed in barcode 4 - Sign in to a different browser with a different user and at a different branch 5 - Bring up a aptron in circulation and type gibberish into barcode field to get a fast cataloging link 6 - It may have your branch, but it may also have the other user's branch from the other window 7 - Keep entering gibberish to get a link until one user has the correct branch 8 - Then switch to the other browser, and keep entering gibberish, watch the branchcode change 9 - Apply patch, restart all 10 - Test switching between browsers. generating fast cataloging links 11 - Users should now consistently have the correct branch -- 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=35518 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- 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=35518 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 David Nind <david@davidnind.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=35518 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161539|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 161707 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161707&action=edit Bug 35518: Check authentication and set userenv before fetching userenv variables Currently we get the userenv before we have set it correctly for the session To test: 1 - Sign in as a user with fast cataloging permission 2 - Bring up a patron, type gibberish into barcode field to get a fast cataloging link 3 - Check the link, it should have your current signed in barcode 4 - Sign in to a different browser with a different user and at a different branch 5 - Bring up a aptron in circulation and type gibberish into barcode field to get a fast cataloging link 6 - It may have your branch, but it may also have the other user's branch from the other window 7 - Keep entering gibberish to get a link until one user has the correct branch 8 - Then switch to the other browser, and keep entering gibberish, watch the branchcode change 9 - Apply patch, restart all 10 - Test switching between browsers. generating fast cataloging links 11 - Users should now consistently have the correct branch Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161707|0 |1 is obsolete| | --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 161929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161929&action=edit Bug 35518: Check authentication and set userenv before fetching userenv variables Currently we get the userenv before we have set it correctly for the session To test: 1 - Sign in as a user with fast cataloging permission 2 - Bring up a patron, type gibberish into barcode field to get a fast cataloging link 3 - Check the link, it should have your current signed in barcode 4 - Sign in to a different browser with a different user and at a different branch 5 - Bring up a aptron in circulation and type gibberish into barcode field to get a fast cataloging link 6 - It may have your branch, but it may also have the other user's branch from the other window 7 - Keep entering gibberish to get a link until one user has the correct branch 8 - Then switch to the other browser, and keep entering gibberish, watch the branchcode change 9 - Apply patch, restart all 10 - Test switching between browsers. generating fast cataloging links 11 - Users should now consistently have the correct branch Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 161930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161930&action=edit Bug 35518: Tidy the moved blocks This patch just tidies the moved blocks to get us past the QA script check. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com 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=35518 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- No regressions found, QA script happy. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #6 from Andrii Nugged <nugged@gmail.com> --- this comment is not related to the code/solution, but to the problem as before the ticket: we recently found that before this solution Koha brings the wrong returning branches when the item is returned on circ/circulation.pl page (moreover, it looks like (unsure) that it reads branch data of another user from the Plack session because the current wasn't populated properly) so this bug should have "increased" priority, IMO. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- It's already PQA, we're in a security freeze right now. It'll be pushed as soon as that freeze is unlocked. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Michael Adamyk <madamyk@ckls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |madamyk@ckls.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Call to |Call to |C4::Context->userenv |C4::Context->userenv |happens before it's gets |happens before it's gets |populated breaks code logic |populated breaks code logic | |in circulation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Call to |Call to |C4::Context->userenv |C4::Context->userenv |happens before it's gets |happens before it's gets |populated breaks code logic |populated breaks code logic |in circulation | Status|Passed QA |Pushed to master Version(s)| |24.05.00 released in| | --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to master for 24.05.00, thanks for the hard work everyone. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Call to |Call to |C4::Context->userenv |C4::Context->userenv |happens before it's gets |happens before it's gets |populated breaks code logic |populated breaks code logic | |in circulation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Version(s)|24.05.00 |24.05.00,23.11.03 released in| | CC| |fridolin.somers@biblibre.co | |m Status|Pushed to master |Pushed to stable --- Comment #9 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Michael Hafen <michael.hafen@washk12.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |michael.hafen@washk12.org --- Comment #10 from Michael Hafen <michael.hafen@washk12.org> --- I found a bug with this patch. Because the AutoSwitch check has been moved to after the @$barcodes variable is filled, if I scan a patron barcode into the item barcode field (as my librarians do all the time), the @$barcodes variable gets filled, but not emptied, and the 'Barcode not found' error surfaces. I'll be submitting a (trivial) patch to correct that in a moment. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #11 from Michael Hafen <michael.hafen@washk12.org> --- Created attachment 162216 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162216&action=edit Bug 35518: follow up - clear the barcode list in AutoSwitchPatron block The AutoSwitchPatron block got moved, and now the @$barcodes variable gets filled and not cleared. Leading to a 'Barcode not found' error when the patron is auto switched. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #12 from Michael Hafen <michael.hafen@washk12.org> --- Created attachment 162218 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162218&action=edit Bug 35518: Follow-up for AutoSwitchPatron - clear $patron variable too Another follow up: The AutoSwitchPatron clears the $borrowernumber variable to switch patrons. With the AuthSwitchPatron block moved, the $patron variable still gets set, and the patron doesn't get switched. The clears the $patron variable too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #13 from Fridolin Somers <fridolin.somers@biblibre.com> --- Hi Michael Thanks for that. You should create a new bug report with those patches since the previous ones have already been pushed. And beware of commit messages for follow-ups : https://wiki.koha-community.org/wiki/Commit_messages#Follow-ups -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Michael Hafen <michael.hafen@washk12.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36139 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 [Bug 36139] Bug 35518 follow-up: fix AutoSwitchPatron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Eric <eric@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |eric@bywatersolutions.com --- Comment #14 from Eric <eric@bywatersolutions.com> --- Would it be possible for this to be backported to 23.05? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Michelle Spinney <mspinney@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mspinney@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #15 from Michael Adamyk <madamyk@ckls.org> --- +1 to Eric's question. This is causing significant confusion in several different contexts in our consortium. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Version(s)|24.05.00,23.11.03 |24.05.00,23.11.03,23.05.09 released in| | Status|Pushed to stable |Pushed to oldstable --- Comment #16 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.09 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Michael Hafen <michael.hafen@washk12.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162216|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Michael Hafen <michael.hafen@washk12.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162218|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #17 from Michael Hafen <michael.hafen@washk12.org> --- (In reply to Fridolin Somers from comment #13)
Hi Michael Thanks for that.
You should create a new bug report with those patches since the previous ones have already been pushed. And beware of commit messages for follow-ups : https://wiki.koha-community.org/wiki/Commit_messages#Follow-ups
I was to late in marking those two patches obsolete, they were included in the push to 23.05.x. I made bug 36139 to track my patches. I foresee some confusion here. in 23.05.x those two extra patches should be reverted, or bug 36139 needs to Not be backported. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #18 from Aleisha Amohia <aleisha@catalyst.net.nz> --- (In reply to Michael Hafen from comment #17)
(In reply to Fridolin Somers from comment #13)
Hi Michael Thanks for that.
You should create a new bug report with those patches since the previous ones have already been pushed. And beware of commit messages for follow-ups : https://wiki.koha-community.org/wiki/Commit_messages#Follow-ups
I was to late in marking those two patches obsolete, they were included in the push to 23.05.x.
I made bug 36139 to track my patches.
I foresee some confusion here. in 23.05.x those two extra patches should be reverted, or bug 36139 needs to Not be backported.
Hey Lucas, do you understand what is required here for 23.05.x? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #19 from Lucas Gass <lucas@bywatersolutions.com> ---
Hey Lucas, do you understand what is required here for 23.05.x?
Do we need to revert Bug 36139 from 23.05.x? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #20 from Michael Hafen <michael.hafen@washk12.org> --- Don't need to revert Bug 36139 since it doesn't seem to have been back-ported at this point. Just need to make sure it doesn't get pulled into 23.05.x. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #21 from Aleisha Amohia <aleisha@catalyst.net.nz> --- (In reply to Michael Hafen from comment #20)
Don't need to revert Bug 36139 since it doesn't seem to have been back-ported at this point. Just need to make sure it doesn't get pulled into 23.05.x.
We are seeing this bug in 23.05 so just want to check that the next release will have this fixed. You mentioned earlier "in 23.05.x those two extra patches should be reverted" - are you referring to the patches on this report, Bug 35518 need to be reverted from 23.05? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #22 from Michael Hafen <michael.hafen@washk12.org> --- There are two extra patches on this bug that are now obsolete. This bug was backported to 23.05.x before those two patches were marked obsolete. Those two patches are now on Bug 36139. My advise was that either Bug 36139 should be skipped (not backported to 23.05.x) OR the two extra patches should be reverted. I'll leave which option is the more appropriate to your discretion, in my opinion you should leave those two patches as is on this bug (don't revert them) and skip Bug 36139. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 --- Comment #23 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Ah yes sorry that makes sense! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Bug 35518 depends on bug 17798, which changed state. Bug 17798 Summary: Checking out an item on hold for another patron prints a slip but does not update hold https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17798 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #24 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 unless requested -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org