Bug 6776 - Add Checkin tab to patron search tool
Summary: Add Checkin tab to patron search tool
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) enhancement (vote)
Assignee: shinoy m
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-25 04:34 UTC by Savitra Sirohi
Modified: 2013-12-05 19:53 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Add Checkin tab to patron search tool (2.07 KB, patch)
2011-09-09 07:57 UTC, Amit Gupta
Details | Diff | Splinter Review
Bug 6776 - Add Checkin tab to patron search tool (2.12 KB, patch)
2011-10-06 16:30 UTC, Paul Poulain
Details | Diff | Splinter Review
Add Checkin tab to patron search tool (1.91 KB, patch)
2011-10-07 06:27 UTC, Amit Gupta
Details | Diff | Splinter Review
Add Checkin tab to patron search tool (1.76 KB, patch)
2011-10-07 12:10 UTC, Amit Gupta
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Savitra Sirohi 2011-08-25 04:34:53 UTC
Add check-in as an option in patron-search.inc, thus allowing a checkin tab to be available in several patron and circulation pages
Comment 1 Amit Gupta 2011-09-09 07:57:22 UTC Comment hidden (obsolete)
Comment 2 Paul Poulain 2011-10-06 16:30:16 UTC Comment hidden (obsolete)
Comment 3 Paul Poulain 2011-10-06 16:31:39 UTC
QA comment:

trivial patch, correctly spelled, usefull addition even if all other search tabs have usually 3 tabs, marking passed QA
Comment 4 Ian Walls 2011-10-06 19:51:52 UTC
Does not respect checkin permissions like circulation/circ-search.inc does.  Needs to be wrapped in [% IF ( CAN_user_circulate ) %]
Comment 5 Amit Gupta 2011-10-07 06:27:10 UTC Comment hidden (obsolete)
Comment 6 Paul Poulain 2011-10-07 09:15:50 UTC
QA comment, sorry amit, another comment:
please don't use tabs, but 4 spaces.

It would also be better to have:
+      </div>
+   [% END %]

instead of 
+      </div>[% END %]

I almost missed the END and thought it would not work !
Comment 7 Ian Walls 2011-10-07 11:31:13 UTC
Thank you for resubmitting with the permissions change!

One other small bit, not a blocker to QA but something I noticed; since Checkout and Checkin require the same permissions, you can wrap the tabs in the same [% if %] block, instead of having conditionals individually for both tabs.  This is just a shortcut, not important, and your way does allow for us to more easily differentiate checkin and checkout permissions in the future.

The change from tab to 4 spaces is requested; putting [% END %] on it's own line is optional.
Comment 8 Amit Gupta 2011-10-07 12:10:49 UTC
Created attachment 5766 [details] [review]
Add Checkin tab to patron search tool

Using space instead of tab.
Comment 9 Ian Walls 2011-10-07 12:14:13 UTC
Thank you for your diligence on this, Amit.  I count the previous signoffs as still good, and include my own.  Passed QA.
Comment 10 Owen Leonard 2011-10-07 12:16:59 UTC
I would also like to comment that we don't have a strict rule about tabs versus spaces in the templates, although maybe we should. There is also no style rule about Template:Toolkit tag placement. These could be subjects of future template style discussions, but shouldn't be a barrier to QA until rules are in place.
Comment 11 Chris Cormack 2011-10-09 21:08:33 UTC
Pushed please test
Comment 12 Jared Camins-Esakov 2012-05-23 12:51:30 UTC
Seems to be working. Closing.