Bug 21056 - Changing the logged in library can fail sporadically
Summary: Changing the logged in library can fail sporadically
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-07-09 18:04 UTC by Kyle M Hall
Modified: 2019-10-14 19:55 UTC (History)
5 users (show)

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


Attachments
Patch (1.36 KB, patch)
2018-07-09 18:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21056: Changing the logged in library can fail sporadically (1.41 KB, patch)
2018-07-31 15:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21056: Add another flush later (974 bytes, patch)
2018-07-31 15:16 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2018-07-09 18:04:53 UTC
We are getting reports that sometimes changing the logged in library via selectbranchprinter.pl sometimes appears to fail. Flushing the session after updating the branch appears to fix this.
Comment 1 Kyle M Hall 2018-07-09 18:07:56 UTC
Created attachment 76800 [details] [review]
Patch
Comment 2 Jonathan Druart 2018-07-11 21:40:31 UTC
What is the value of SessionStorage?

From CGI::Session:
"""
Always explicitly calling flush() on the session before the program exits is recommended. For extra safety, call it immediately after every important session update.
"""

Maybe we should add this line below (there is another ->param call later).

It sounds like we should put some flush call to C4::Auth as well.
Comment 3 Kyle M Hall 2018-07-12 11:19:40 UTC
(In reply to Jonathan Druart from comment #2)
> What is the value of SessionStorage?
> 
> From CGI::Session:
> """
> Always explicitly calling flush() on the session before the program exits is
> recommended. For extra safety, call it immediately after every important
> session update.
> """
> 
> Maybe we should add this line below (there is another ->param call later).
> 
> It sounds like we should put some flush call to C4::Auth as well.

I certainly wouldn't hurt! Do you want to just add that as a followup here, or do you want to file a separate bug?
Comment 4 George Williams (NEKLS) 2018-07-25 21:08:37 UTC
Tested on the NEKLS test server - works as expected
Comment 5 Jonathan Druart 2018-07-31 15:16:20 UTC
Created attachment 77336 [details] [review]
Bug 21056: Changing the logged in library can fail sporadically

We are getting reports that sometimes changing the logged in library via selectbranchprinter.pl sometimes appears to fail. Flushing the session after updating the branch appears to fix this.

Test Plan:
1) Change your logged in library repeatedly. Hopefully ( sort of ) it will fail to change.
   You may want to try various browsers.
2) Apply this patch
3) Note the change no longer fails

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Jonathan Druart 2018-07-31 15:16:24 UTC
Created attachment 77337 [details] [review]
Bug 21056: Add another flush later

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2018-07-31 15:16:57 UTC
(In reply to Jonathan Druart from comment #2)
> It sounds like we should put some flush call to C4::Auth as well.

No bug reported, so no fix for now.
Comment 8 Nick Clemens 2018-08-09 12:27:37 UTC
Awesome work all!

Pushed to master for 18.11
Comment 9 Martin Renvoize 2018-08-10 07:44:33 UTC
Pushed to 18.05.x for 18.05.03
Comment 10 Fridolin Somers 2018-09-21 10:36:24 UTC
Pushed to 17.11.x for 17.11.10