|
Description
Owen Leonard
2020-02-10 19:52:12 UTC
This one will be converted to use sessionStorage instead of cookies. Created attachment 102249 [details] [review] Bug 24625: Store showLastPatron information using localStorage This patch removes the use of jquery.cookie to store "last patron" information, using localStorage instead. Because there is no "session only" option with localStorage, additional handling of the showLastPatron data is added to the login page. That takes care of "stale" last patron information if user didn't log out but the session expired for some reason. To test apply the patch and enable the showLastPatron system preference. 1. Load a patron's account for checkout 2. Load another patron's account for checkout - There should now be a "Last patron" link in the breadcrumbs bar which links to the patron in step 1. Hovering your mouse over the link should display a tooltip containing the patron's name and card number. - Click the "X" to clear the last patron information. The last patron link should go away. 3. Log out and log back in. The last patron information should be gone. (In reply to Owen Leonard from comment #1) > This one will be converted to use sessionStorage instead of cookies. Or maybe localStorage :) sessionStorage's definition of "session" being linked to the window or tab means that the information wouldn't persist in the same way that a session cookie would. Created attachment 102256 [details] [review] Bug 24625: Store showLastPatron information using localStorage This patch removes the use of jquery.cookie to store "last patron" information, using localStorage instead. Because there is no "session only" option with localStorage, additional handling of the showLastPatron data is added to the login page. That takes care of "stale" last patron information if user didn't log out but the session expired for some reason. To test apply the patch and enable the showLastPatron system preference. 1. Load a patron's account for checkout 2. Load another patron's account for checkout - There should now be a "Last patron" link in the breadcrumbs bar which links to the patron in step 1. Hovering your mouse over the link should display a tooltip containing the patron's name and card number. - Click the "X" to clear the last patron information. The last patron link should go away. 3. Log out and log back in. The last patron information should be gone. Signed-off-by: David Nind <david@davidnind.com> It does not work: - Search for patrons - Select Edna - Header, search the catalog - Last patron is OK here - Click a record - Last patron is "undefined" KO Created attachment 105911 [details] [review] Bug 24625: Store showLastPatron information using localStorage This patch removes the use of jquery.cookie to store "last patron" information, using localStorage instead. Because there is no "session only" option with localStorage, additional handling of the showLastPatron data is added to the login page. That takes care of "stale" last patron information if user didn't log out but the session expired for some reason. To test apply the patch and enable the showLastPatron system preference. 1. Load a patron's account for checkout 2. Navigate away from patron-related pages: Perform a catalog search from the search header form and open the detail page from the search results. Confirm that the correct last patron information still shows. 3. Load another patron's account for checkout - There should now be a "Last patron" link in the breadcrumbs bar which links to the patron in step 1. Hovering your mouse over the link should display a tooltip containing the patron's name and card number. - Click the "X" to clear the last patron information. The last patron link should go away. 4. Log out and log back in. The last patron information should be gone. Created attachment 105946 [details] [review] Bug 24625: Store showLastPatron information using localStorage This patch removes the use of jquery.cookie to store "last patron" information, using localStorage instead. Because there is no "session only" option with localStorage, additional handling of the showLastPatron data is added to the login page. That takes care of "stale" last patron information if user didn't log out but the session expired for some reason. To test apply the patch and enable the showLastPatron system preference. 1. Load a patron's account for checkout 2. Navigate away from patron-related pages: Perform a catalog search from the search header form and open the detail page from the search results. Confirm that the correct last patron information still shows. 3. Load another patron's account for checkout - There should now be a "Last patron" link in the breadcrumbs bar which links to the patron in step 1. Hovering your mouse over the link should display a tooltip containing the patron's name and card number. - Click the "X" to clear the last patron information. The last patron link should go away. 4. Log out and log back in. The last patron information should be gone. Signed-off-by: David Nind <david@davidnind.com> Can you please rebase? Bug 24625 - Phase out jquery.cookie.js: showLastPatron 105946 - Bug 24625: Store showLastPatron information using localStorage Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 24625: Store showLastPatron information using localStorage Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/js/staff-global.js Falling back to patching base and 3-way merge... ^[[AAuto-merging koha-tmpl/intranet-tmpl/prog/js/staff-global.js CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/js/staff-global.js Created attachment 106164 [details] [review] Bug 24625: Store showLastPatron information using localStorage This patch removes the use of jquery.cookie to store "last patron" information, using localStorage instead. Because there is no "session only" option with localStorage, additional handling of the showLastPatron data is added to the login page. That takes care of "stale" last patron information if user didn't log out but the session expired for some reason. To test apply the patch and enable the showLastPatron system preference. 1. Load a patron's account for checkout 2. Navigate away from patron-related pages: Perform a catalog search from the search header form and open the detail page from the search results. Confirm that the correct last patron information still shows. 3. Load another patron's account for checkout - There should now be a "Last patron" link in the breadcrumbs bar which links to the patron in step 1. Hovering your mouse over the link should display a tooltip containing the patron's name and card number. - Click the "X" to clear the last patron information. The last patron link should go away. 4. Log out and log back in. The last patron information should be gone. Signed-off-by: David Nind <david@davidnind.com> Hi Owen, Patch looks good and works as expected. But something is missing in my opinion : the motivation behind "phasing out" jquery.cookie.js. Is it deprecated ? Has it been decided in an IRC meeting ? Does localStorage has any benefit over using a cookie ? It would be great if you could add a short explanation in the commit message. Created attachment 106514 [details] [review] Bug 24625: Store showLastPatron information using localStorage This patch removes the use of jquery.cookie to store "last patron" information, using localStorage instead. jquery.cookie.js is obsolete. See Bug 24624. localStorage has been chosen as an alternative in this situation because it does not require transmission between the client and the server. See Bug 12410. Because there is no "session only" option with localStorage, additional handling of the showLastPatron data is added to the login page. That takes care of "stale" last patron information if user didn't log out but the session expired for some reason. To test apply the patch and enable the showLastPatron system preference. 1. Load a patron's account for checkout 2. Navigate away from patron-related pages: Perform a catalog search from the search header form and open the detail page from the search results. Confirm that the correct last patron information still shows. 3. Load another patron's account for checkout - There should now be a "Last patron" link in the breadcrumbs bar which links to the patron in step 1. Hovering your mouse over the link should display a tooltip containing the patron's name and card number. - Click the "X" to clear the last patron information. The last patron link should go away. 4. Log out and log back in. The last patron information should be gone. Signed-off-by: David Nind <david@davidnind.com> Created attachment 106519 [details] [review] Bug 24625: Store showLastPatron information using localStorage This patch removes the use of jquery.cookie to store "last patron" information, using localStorage instead. jquery.cookie.js is obsolete. See Bug 24624. localStorage has been chosen as an alternative in this situation because it does not require transmission between the client and the server. See Bug 12410. Because there is no "session only" option with localStorage, additional handling of the showLastPatron data is added to the login page. That takes care of "stale" last patron information if user didn't log out but the session expired for some reason. To test apply the patch and enable the showLastPatron system preference. 1. Load a patron's account for checkout 2. Navigate away from patron-related pages: Perform a catalog search from the search header form and open the detail page from the search results. Confirm that the correct last patron information still shows. 3. Load another patron's account for checkout - There should now be a "Last patron" link in the breadcrumbs bar which links to the patron in step 1. Hovering your mouse over the link should display a tooltip containing the patron's name and card number. - Click the "X" to clear the last patron information. The last patron link should go away. 4. Log out and log back in. The last patron information should be gone. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Pushed to master for 20.11, thanks to everybody involved! backported to 20.05.x for 20.05.03 enhancement, not backported to 19.11.x |