Bug 29957 removed cookies upon logout and I think this is impacting catalogers choice of advanced vs basic cataloging editor. Before Bug 29957 a cataloger could set the editor to basic or advanced and that choice would be remembered after logout. Now catalogers who logout and log back in will always be defaulted to the basic editor.
On bug 22957 there was a comment: "I'd love to see a 'Private computer' checkbox on login screens next and have a list of cookies to keep should that checkbox be ticked at login vs a completely public computer where I still think we should remove all by default." I think this is not the worst ever solution for this. It happens to be how the bank, for example, often does it.
(In reply to Liz Rea from comment #1) > On bug 22957 there was a comment: > > "I'd love to see a 'Private computer' checkbox on login screens next and > have a list of cookies to keep should that checkbox be ticked at login vs a > completely public computer where I still think we should remove all by > default." > > I think this is not the worst ever solution for this. It happens to be how > the bank, for example, often does it. This does seem like the best solution for this, IMHO.
The referenced bug allows you to specify cookies not to be removed: See koha-conf.xml <!-- Uncomment lines like hereunder to not clear cookies at logout: The cookie name is case sensitive. <do_not_remove_cookie>KohaOpacLanguage</do_not_remove_cookie> --> The problem is that it sets a cookie for each user: Cookies.set( 'catalogue_editor_[% logged_in_user.borrowernumber | html %]', 'advanced', { expires: 365, path: '/', sameSite: 'Lax' } ); Adding a fix for that
Created attachment 138203 [details] [review] Bug 31250: Deny clearing cookies with numeric suffix This change allows us to add catalogue_editor_ to the deny list in koha-conf.xml and keep cookies like catalogue_editor_123. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 138204 [details] [review] Bug 31250: Add catalogue_editor_ cookie to koha-conf deny list Test plan: Add this change to your koha-conf.xml. Flush, restart. Test if the cookie is kept now in the interface.
Lucas, I could imagine that we call this a bug for backporting. Especially the first patch. Note that we do not touch koha-conf at upgrades, so it still needs koha-conf editing. Which might make backporting a bit less interesting..
knowing it *can* be fixed is a huge thing - I agree it's interesting for a backport. :D
Created attachment 138981 [details] [review] Bug 31250: Deny clearing cookies with numeric suffix This change allows us to add catalogue_editor_ to the deny list in koha-conf.xml and keep cookies like catalogue_editor_123. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> To test: 1 - Sign in to staff client 2 - Search for records and edit one 3 - Switch to advanced editor 4 - View cookies (F12/developer panel/storage tab) 5 - Note cookie like 'catalogue_editor_##' with value 'advanced' 6 - Log out 7 - note cookie value deleted 8 - Log in and search/edit a record 9 - Basic editor loads 10 - Apply patch 11 - Add line to koha-conf as described in second patch 12 - Restart all 13 - Switch to advanced editor 14 - Cookie value updated 15 - Logout 16 - Cookie value remains 17 - Log in and search/edit 18 - Confirm advanced editor loads Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 138982 [details] [review] Bug 31250: Add catalogue_editor_ cookie to koha-conf deny list Test plan: Add this change to your koha-conf.xml. Flush, restart. Test if the cookie is kept now in the interface. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
hum, why not supporting a wildcard then? catalogue_editor_* Seems more flexible.
(In reply to Jonathan Druart from comment #10) > hum, why not supporting a wildcard then? catalogue_editor_* > > Seems more flexible. We do not need it currently. Can be done on a later report imo. This is ready to go.
(In reply to Marcel de Rooy from comment #11) > (In reply to Jonathan Druart from comment #10) > > hum, why not supporting a wildcard then? catalogue_editor_* > > > > Seems more flexible. > > We do not need it currently. Can be done on a later report imo. This is > ready to go. How will you modified the config files? Once this is pushed you will have to keep the behaviour. If we propose a generic solution from the beginning, future will be easier ;)
(In reply to Jonathan Druart from comment #12) > How will you modified the config files? Once this is pushed you will have to > keep the behaviour. If we propose a generic solution from the beginning, > future will be easier ;) All improvements in the future :) Will think a bit about it.
Created attachment 139389 [details] [review] Bug 31250: (QA follow-up) The future will be easier As requested by Jonathan, we need more flexibility ;) Here it comes. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 139571 [details] [review] Bug 31250: (QA follow-up) The future will be easier As requested by Jonathan, we need more flexibility ;) Here it comes. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140163 [details] [review] Bug 31250: Deny clearing cookies with numeric suffix This change allows us to add catalogue_editor_ to the deny list in koha-conf.xml and keep cookies like catalogue_editor_123. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> To test: 1 - Sign in to staff client 2 - Search for records and edit one 3 - Switch to advanced editor 4 - View cookies (F12/developer panel/storage tab) 5 - Note cookie like 'catalogue_editor_##' with value 'advanced' 6 - Log out 7 - note cookie value deleted 8 - Log in and search/edit a record 9 - Basic editor loads 10 - Apply patch 11 - Add line to koha-conf as described in second patch 12 - Restart all 13 - Switch to advanced editor 14 - Cookie value updated 15 - Logout 16 - Cookie value remains 17 - Log in and search/edit 18 - Confirm advanced editor loads Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 140164 [details] [review] Bug 31250: Deny clearing cookies with numeric suffix This change allows us to add catalogue_editor_ to the deny list in koha-conf.xml and keep cookies like catalogue_editor_123. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> To test: 1 - Sign in to staff client 2 - Search for records and edit one 3 - Switch to advanced editor 4 - View cookies (F12/developer panel/storage tab) 5 - Note cookie like 'catalogue_editor_##' with value 'advanced' 6 - Log out 7 - note cookie value deleted 8 - Log in and search/edit a record 9 - Basic editor loads 10 - Apply patch 11 - Add line to koha-conf as described in second patch 12 - Restart all 13 - Switch to advanced editor 14 - Cookie value updated 15 - Logout 16 - Cookie value remains 17 - Log in and search/edit 18 - Confirm advanced editor loads Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 140165 [details] [review] Bug 31250: Add catalogue_editor_ cookie to koha-conf deny list Test plan: Add this change to your koha-conf.xml. Flush, restart. Test if the cookie is kept now in the interface. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 140166 [details] [review] Bug 31250: (QA follow-up) The future will be easier As requested by Jonathan, we need more flexibility ;) Here it comes. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(In reply to Marcel de Rooy from comment #13) > (In reply to Jonathan Druart from comment #12) > > How will you modified the config files? Once this is pushed you will have to > > keep the behaviour. If we propose a generic solution from the beginning, > > future will be easier ;) > > All improvements in the future :) > Will think a bit about it. Have you thougth about it?
(In reply to Tomás Cohen Arazi from comment #20) > (In reply to Marcel de Rooy from comment #13) > > (In reply to Jonathan Druart from comment #12) > > > How will you modified the config files? Once this is pushed you will have to > > > keep the behaviour. If we propose a generic solution from the beginning, > > > future will be easier ;) > > > > All improvements in the future :) > > Will think a bit about it. > > Have you thougth about it? Yes and I added a follow-up. Allowing the required flexibility via regexes.
(In reply to Marcel de Rooy from comment #21) > (In reply to Tomás Cohen Arazi from comment #20) > > (In reply to Marcel de Rooy from comment #13) > > > (In reply to Jonathan Druart from comment #12) > > > > How will you modified the config files? Once this is pushed you will have to > > > > keep the behaviour. If we propose a generic solution from the beginning, > > > > future will be easier ;) > > > > > > All improvements in the future :) > > > Will think a bit about it. > > > > Have you thougth about it? > > Yes and I added a follow-up. Allowing the required flexibility via regexes. Which is the generic solution mentioned.
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for upcoming 22.05.07 release
applied to 21.11.x for 21.11.14
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document it seems, marking resolved.