Bug 24625 - Phase out jquery.cookie.js: showLastPatron
Summary: Phase out jquery.cookie.js: showLastPatron
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 12410
  Show dependency treegraph
 
Reported: 2020-02-10 19:52 UTC by Owen Leonard
Modified: 2020-08-03 22:56 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.03


Attachments
Bug 24625: Store showLastPatron information using localStorage (5.63 KB, patch)
2020-04-01 19:39 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 24625: Store showLastPatron information using localStorage (5.68 KB, patch)
2020-04-02 01:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 24625: Store showLastPatron information using localStorage (6.34 KB, patch)
2020-06-16 12:40 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 24625: Store showLastPatron information using localStorage (6.38 KB, patch)
2020-06-17 11:27 UTC, David Nind
Details | Diff | Splinter Review
Bug 24625: Store showLastPatron information using localStorage (6.38 KB, patch)
2020-06-22 12:40 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 24625: Store showLastPatron information using localStorage (6.58 KB, patch)
2020-07-03 13:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 24625: Store showLastPatron information using localStorage (6.63 KB, patch)
2020-07-03 14:15 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2020-02-10 19:52:12 UTC
The showLastPatron feature uses cookies to store information about last searched patron in the staff client.

This process should use the newer cookie library, js-cookie.
Comment 1 Owen Leonard 2020-04-01 18:56:24 UTC
This one will be converted to use sessionStorage instead of cookies.
Comment 2 Owen Leonard 2020-04-01 19:39:13 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2020-04-01 19:40:18 UTC
(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.
Comment 4 David Nind 2020-04-02 01:50:24 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2020-04-02 08:47:54 UTC
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
Comment 6 Owen Leonard 2020-06-16 12:40:11 UTC Comment hidden (obsolete)
Comment 7 David Nind 2020-06-17 11:27:52 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2020-06-21 14:19:04 UTC
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
Comment 9 Owen Leonard 2020-06-22 12:40:21 UTC
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>
Comment 10 Julian Maurice 2020-07-03 08:35:14 UTC
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.
Comment 11 Owen Leonard 2020-07-03 13:59:06 UTC
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>
Comment 12 Julian Maurice 2020-07-03 14:15:57 UTC
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>
Comment 13 Jonathan Druart 2020-07-20 15:48:32 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 14 Lucas Gass 2020-07-27 16:54:09 UTC
backported to 20.05.x for 20.05.03
Comment 15 Aleisha Amohia 2020-08-03 22:56:43 UTC
enhancement, not backported to 19.11.x