On pages that have the check out and check in tabs on the quick search box at the top, it would be nice to also have a renew tab.
Created attachment 46847 [details] [review] Bug 12051: Adds a Renew tab to the staff client To test: 1) Apply patch 2) Confirm that there is now a Renew tab on every page 3) Attempt to renew a book using this, confirm it works as expected 4) Check if I have missed any pages
Created attachment 47276 [details] [review] Bug 12051: Adds a Renew tab to the staff client To test: 1) Apply patch 2) Confirm that there is now a Renew tab on every page 3) Attempt to renew a book using this, confirm it works as expected 4) Check if I have missed any pages Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
I am not sure about adding the same tabs everywhere - it might get a bit crowded up there. Renew doesn't seem to be used very often as it requires you to have the items on hand to scan the barcodes. Hm, adding Owen for another opinion.
(In reply to Katrin Fischer from comment #3) > I am not sure about adding the same tabs everywhere - it might get a bit > crowded up there. Renew doesn't seem to be used very often as it requires > you to have the items on hand to scan the barcodes. > > Hm, adding Owen for another opinion. I see what you mean. Perhaps only on the pages where there is also a check-in tab? Since they also need the barcode for that?
(In reply to Aleisha Amohia from comment #4) > I see what you mean. Perhaps only on the pages where there is also a > check-in tab? Since they also need the barcode for that? I agree with this. I think the presence of the check-in tab is a good indicator that the Renew tab fits.
+1 :)
Created attachment 47361 [details] [review] Bug 12051: Adds a Renew tab to the staff client To test: 1) Apply patch 2) Confirm that there is now a Renew tab on the pages where there is also a Check-in tab only 3) Attempt to renew a book using this, confirm it works as expected 4) Check if I have missed any pages
Created attachment 47362 [details] [review] Bug 12051: Adds a Renew tab to the staff client To test: 1) Apply patch 2) Confirm that there is now a Renew tab on the pages where there is also a Check-in tab only 3) Attempt to renew a book using this, confirm it works as expected 4) Check if I have missed any pages Had to fix some numbering
I think there is one more place where the renew tab would make sense: /cgi-bin/koha/circ/returns.pl Use case: A patron who wants to return some books and to renew some others at the same time.
Created attachment 47406 [details] [review] Bug 12051: Adds a Renew tab to the staff client UPDATE: Added to checkin-search.inc to show up on returns.pl To test: 1) Apply patch 2) Confirm that there is now a Renew tab on the pages where there is also a Check-in tab only 3) Attempt to renew a book using this, confirm it works as expected 4) Check if I have missed any pages
Created attachment 47412 [details] [review] Bug 12051: Adds a Renew tab to the staff client UPDATE: Added to checkin-search.inc to show up on returns.pl To test: 1) Apply patch 2) Confirm that there is now a Renew tab on the pages where there is also a Check-in tab only 3) Attempt to renew a book using this, confirm it works as expected 4) Check if I have missed any pages Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 47454 [details] [review] Bug 12051: Adds a Renew tab to the staff client UPDATE: Added to checkin-search.inc to show up on returns.pl To test: 1) Apply patch 2) Confirm that there is now a Renew tab on the pages where there is also a Check-in tab only 3) Attempt to renew a book using this, confirm it works as expected 4) Check if I have missed any pages Signed-off-by: Marc Véron <veron@veron.ch>
Aleisha, I have amended your patch to remove files where only 1 line where added/removed. I have also fixed a 3 vs 2: +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/suggestions-add-search.inc @@ -23,10 +23,11 @@ </form> </div> [% END %] + <ul> <li><a href="#suggestions_search">Search suggestions</a></li> [% IF ( CAN_user_circulate ) %]<li><a onclick="keep_text(0)" href="#circ_search">Check out</a></li>[% END %] - [% IF ( CAN_user_catalogue ) %]<li><a onclick="keep_text(1)" href="#catalog_search">Search the catalog</a></li>[% END %] + [% IF ( CAN_user_catalogue ) %]<li><a onclick="keep_text(2)" href="#catalog_search">Search the catalog</a></li>[% END %] </ul> </div> I was going to pass QA on this patch, but I am wondering if the permission should not be CAN_user_circulate_circulate_remaining_permissions instead of CAN_user_circulate. Indeed the circ/renew.pl script needs only this subperm.
> I was going to pass QA on this patch, but I am wondering if the permission > should not be CAN_user_circulate_circulate_remaining_permissions instead of > CAN_user_circulate. Indeed the circ/renew.pl script needs only this subperm. I think the issue at hand is that this is really just repeating an existing bug. For instance, in cat-search.inc, the checkin box is wrapped with [% IF ( CAN_user_circulate ) %] yet a librarian with only CAN_user_circulate_circulate_remaining_permissions should be able to see the box, as checkins only require circulate_remaining_permissions. I would suggest we leave it the way it is for now for the sake of consistency and file a new bug to fix the issue globally.
Created attachment 47476 [details] [review] Bug 12051: Adds a Renew tab to the staff client UPDATE: Added to checkin-search.inc to show up on returns.pl To test: 1) Apply patch 2) Confirm that there is now a Renew tab on the pages where there is also a Check-in tab only 3) Attempt to renew a book using this, confirm it works as expected 4) Check if I have missed any pages Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
I have filed bug 15706 for the more systemic issue brought up by Jonathan.
Pushed to Master - Should be in the May 2016 Release. Thanks!