Bug 21056

Summary: Changing the logged in library can fail sporadically
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Architecture, internals, and plumbingAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: fridolin.somers, george, jonathan.druart, martin.renvoize, nick
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Patch
Bug 21056: Changing the logged in library can fail sporadically
Bug 21056: Add another flush later

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