This new feature will add a link (on the top-left of the site) to the last patron consulted in the staff site. The feature currently uses cookies to save this information. When a staff site user accesses a patron record, the ID, name and card number of the patron will be saved in the cookie. (I'll add a patch and a test plan shortly)
Created attachment 72294 [details] [review] Bug 20312 - Add a link to last consulted patron TEST PLAN 1 - apply patch 2 - Go to a patron record 3 - Go to a different module 4 - Click on "Last patron" (top-left) 5 - It should go the last patron record you consulted
Patch tested with a sandbox, by Axelle <axelle.clarisse@univ-amu.fr>
Created attachment 72638 [details] [review] Bug 20312 - Add a link to last consulted patron TEST PLAN 1 - apply patch 2 - Go to a patron record 3 - Go to a different module 4 - Click on "Last patron" (top-left) 5 - It should go the last patron record you consulted Signed-off-by: Axelle <axelle.clarisse@univ-amu.fr>
Hi Maksim, the patch contains a po file that should not be there, can you please fix? I am also having doubts on the the placmeent of the link. This is not a link to a module, so I tihnk placing it in the top navigation is not a good spot. Asking Owen for additional feedback.
Created attachment 75672 [details] Screenshot of interface idea Something like this might work.
Created attachment 77272 [details] [review] Bug 20312: Add a link to last consulted patron This is an update to Maksim's patch. The link "Last patron" is now on the top-right corner of the screen and has the same style as attachment 75672 [details]. Moreover, the "X" button is added to remove the link if clicked on. To test: 1- Apply the patch 2- Go to a patron details' page 3- Verify that the link "Last patron" appears on the top-right corner of the screen 4- Go to another page 5- The link should still be here 6- Click on the link 7- Verify that you are at the patron's checkout page 8- Click on the "X" button next to the "Last patron" link 9- The link should be gone 10- Go to another page 11- The link shouldn't appear (until the next patron detail's page)
Hi Owen, Katrin, I created the file last-borrower.inc that contains the main code for the feature. It has a <style> tag for the link to appear like in attachment 75672 [details]. I would like to know if this tag is in the right place of it should be in another file. Also, is the link's position appropriate? Thank you.
Hi Pierre-Luc, taking a quick look. The QA script notes some issues: FAIL koha-tmpl/intranet-tmpl/prog/en/includes/last-borrower.inc FAIL forbidden patterns forbidden pattern: tab char (line 44) forbidden pattern: intranet-tmpl should certainly replaced with [% interface %] (line 3) forbidden pattern: tab char (line 41) forbidden pattern: tab char (line 48) forbidden pattern: tab char (line 38) forbidden pattern: tab char (line 40) forbidden pattern: tab char (line 37) Please note, that once this is pushed, the new cookie should be documented here: https://wiki.koha-community.org/wiki/Use_of_Cookies I will leave the question of where the Javascript should go to Owen, but feel like it might be better in a separate .js file. I wonder if libraries would like to have this optional for privacy reasons. It might be safer to add a pref.
Some suggestions: - Use <i class="fa fa-remove"></i> instead of an image for X. - Put the CSS in staff-global.scss - Put the JavaScript in staff-global.js - Add the JS to the existing $(document).ready() so you don't need 'addEventListener('DOMContentLoaded')'; - Be consistent about whether you use single or double quotes (In reply to Katrin Fischer from comment #8) > I wonder if libraries would like to have this optional for privacy reasons. > It might be safer to add a pref. A lot of this already rests in the hands of the librarian and their browser. If I have my browser configured to restore my last session, my back button will take me back to the last patron without a handy link. The way this patch does it, the cookie should disappear when the browser is closed. I would think that is adequate.
Created attachment 77769 [details] [review] Bug 20312: Add a link to last consulted patron This new feature adds a link to the last patron consulted in the staff site. It uses cookies to save this information. When a staff site user accesses a patron record, the ID, name and card number of the patron will be saved in the cookie. To test: 1- Apply the patch 2- Go to a patron details' page 3- Verify that the link "Last patron" appears on the top-right corner of the screen 4- Go to another page 5- The link should still be here 6- Click on the link 7- Verify that you are at the patron's checkout page 8- Click on the "X" button next to the "Last patron" link 9- The link should be gone 10- Go to another page 11- The link shouldn't appear (until the next patron detail's page)
Created attachment 77796 [details] [review] Bug 20312: (follow-up) Minor CSS corrections This patch makes a couple of minor CSS corrections to match linter style and to correctly color the "X" link.
I noticed a problem when testing this today: The first time the "last patron" link appears, it doesn't contain any parameters (i.e. /cgi-bin/koha/circ/circulation.pl instead of /cgi-bin/koha/circ/circulation.pl?borrowernumber=XXX). It's only after I open a second patron record that the "last patron" link takes me to a valid patron record.
Created attachment 77835 [details] [review] Bug 20312: Add a link to last consulted patron This new feature adds a link to the last patron consulted in the staff site. It uses cookies to save this information. When a staff site user accesses a patron record, the ID, name and card number of the patron will be saved in the cookie. To test: 1- Apply the patch 2- Go to a patron details' page 3- Verify that the link "Last patron" appears on the top-right corner of the screen 4- Go to another page 5- The link should still be here 6- Click on the link 7- Verify that you are at the patron's checkout page 8- Click on the "X" button next to the "Last patron" link 9- The link should be gone 10- Go to another page 11- The link shouldn't appear (until the next patron detail's page)
This seems to be working well now. The only thing which seems unexpected to me was that the "Last patron" link changed immediately upon opening a new checkout screen. - Check out to Patron A - "Last patron" = Patron A. - Check out to Patron B - "Last patron" = Patron B. ...Isn't the "last patron" technically Patron A? I think it's only when switching between patrons at checkout when this is potentially confusing. I'm not sure if that justifies failing this. I'm going to defer to QA.
Created attachment 77977 [details] [review] Bug 20312: Add a link to last consulted patron This new feature adds a link to the last patron consulted in the staff site. It uses cookies to save this information. When a staff site user accesses a patron record, the ID, name and card number of the patron will be saved in the cookie. To test: 1- Apply the patch 2- Go to a patron details' page 3- Verify that the link "Last patron" appears on the top-right corner of the screen 4- Go to another page 5- The link should still be here 6- Click on the link 7- Verify that you are at the patron's checkout page 8- Click on the "X" button next to the "Last patron" link 9- The link should be gone 10- Go to another page 11- The link shouldn't appear (until the next patron detail's page) Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 77979 [details] [review] Bug 20312: Add a link to last consulted patron This new feature adds a link to the last patron consulted in the staff site. It uses cookies to save this information. When a staff site user accesses a patron record, the ID, name and card number of the patron will be saved in the cookie. To test: 1- Apply the patch 2- Go to a patron details' page 3- Verify that the link "Last patron" appears on the top-right corner of the screen 4- Go to another page 5- The link should still be here 6- Click on the link 7- Verify that you are at the patron's checkout page 8- Click on the "X" button next to the "Last patron" link 9- The link should be gone 10- Go to another page 11- The link shouldn't appear (until the next patron detail's page) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice little feature addition here.. code looks reasonable and the feature works as expected for me.. I can't find any regressions and the qa scripts all pass. Passing QA.
Minor note.. https://wiki.koha-community.org/wiki/Use_of_Cookies will need updating to reflect the additional cookie data added here.
Hi, Now it looks like that: https://snag.gy/6vbjNS.jpg (a bit different that what is on the attached screenshot) I am sorry to tell that, but I do not like it. I will list some things that are wrong and enhancement suggestions: * #lastborrower-window ("window"?) is shown by default, then hidden when the page is loaded, it should be the reverse. * A general pref should control it, you click 'x' then it displays again. It can be frustrating (yes it could be hidden with js, but here I think we need a pref (or better would have been a config at per user, but out of the scope). * If you are on patron A detail view, you see the "last patron" that link to ... patron A. It is useless in that case. Either it points to the last patron, or is not displayed. * I think it would have been more useless with the last X (5?) instead of the last one (not required) You can wait for other QAers opinions before implementing these changes.
(In reply to Jonathan Druart from comment #19) > * I think it would have been more useless with the last X (5?) instead of > the last one (not required) Ofc this was ^useless^useful^
Further comment.. it would be good to set the 'last patron' upon navigating away from a patron interaction rather than to it if that's possible.. else 'last patron' and 'current patron' end up matching immediately so if you want to return to the last patron you're stuck again at starting from scratch.
Created attachment 78002 [details] [review] Bug 20312: Add a link to last consulted patron This new feature adds a link to the last patron consulted in the staff site. It uses cookies to save this information. When a staff site user accesses a patron record, the ID, name and card number of the patron will be saved in the cookie. To test: 1- Apply the patch 2- Go to a patron details' page 4- Go to another page 4- Verify that the link "Last patron" appears on the top-right corner of the screen 5- Click on the link 6- Verify that you are at the patron's checkout page 7- Click on the "X" button next to the "Last patron" link 8- The link should be gone 9- Go to another page 10- The link shouldn't appear (until the next patron detail's page)
Created attachment 78005 [details] [review] Bug 20312: Add a link to last consulted patron This new feature adds a link to the last patron consulted in the staff site. It uses cookies to save this information. When a staff site user accesses a patron record, the ID, name and card number of the patron will be saved in the cookie. To test: 1- Apply the patch 2- Go to a patron details' page 4- Go to another page 4- Verify that the link "Last patron" appears on the top-right corner of the screen 5- Click on the link 6- Verify that you are at the patron's checkout page 7- Click on the "X" button next to the "Last patron" link 8- The link should be gone 9- Go to another page 10- The link shouldn't appear (until the next patron detail's page) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Second times a charm. Thankyou for making the requested changes, this looks very good to me now. Passing QA
(In reply to Jonathan Druart from comment #19) > * A general pref should control it, you click 'x' then it displays again. It > can be frustrating (yes it could be hidden with js, but here I think we need > a pref I agree with Jonathan here, I asked around at Kohacon, many libraries would like this feature, but a few had concerns about privacy/security and would like to have the option to disable. Can you please add a syspref to disable the feature?
Created attachment 81726 [details] [review] Bug 20312: Add a link to last consulted patron This new feature adds a link to the last patron consulted in the staff site. It uses cookies to save this information. When a staff site user accesses a patron record, the ID, name and card number of the patron will be saved in the cookie. To test: 1- Apply the patch 2- Go to a patron details' page 4- Go to another page 4- Verify that the link "Last patron" appears on the top-right corner of the screen 5- Click on the link 6- Verify that you are at the patron's checkout page 7- Click on the "X" button next to the "Last patron" link 8- The link should be gone 9- Go to another page 10- The link shouldn't appear (until the next patron detail's page) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 81727 [details] [review] Bug 20312: (QA follow-up) Make display syspref driven Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Quick rescue job done.
*** Bug 21244 has been marked as a duplicate of this bug. ***
Created attachment 81728 [details] [review] Bug 20312: (QA follow-up) Make display syspref driven Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Just reverse my last patch so the feature default to 'off' for now.. I want to get the refinements in before it's widely used ;)
Awesome work all! Pushed to master for 18.11
Enhancement will not be backported.