Bug 14470

Summary: Renewals should be disabled in OPAC if are referred to an on-site checkout
Product: Koha Reporter: Claudio Costales <costalc>
Component: CirculationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: chris, gmcharlt, jonathan.druart, katrin.fischer, kyle.m.hall, liz, mirko, tomascohen
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14884
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 10860    
Bug Blocks:    
Attachments: step-by-step sample
Bug 14470: Do not allow renew for on-site checkouts
Missing data display in checkout patron view
Legends suggestions
Bug 14470: Do not allow renew for on-site checkouts
Bug 14470: Do not allow renew for on-site checkouts
[PASSED QA] Bug 14470: Do not allow renew for on-site checkouts
[PASSED QA] Bug 14470: Do not allow renew for on-site checkouts

Description Claudio Costales 2015-06-28 17:15:29 UTC
Performing an on-site checkout should disable renewals in OPAC as happens in staff client side.

Attached is an step-by-step sample.
Comment 1 Claudio Costales 2015-06-28 17:16:03 UTC
Created attachment 40706 [details]
step-by-step sample
Comment 2 Jonathan Druart 2015-07-14 16:38:33 UTC
Katrin, do you agree this is a bug?
Comment 3 Katrin Fischer 2015-07-19 21:51:34 UTC
I think if we block renewals in the staff client, we should do the same in OPAC - normally staff client allows more than OPAC, the opposite behaviour seems unexpected.
Comment 4 Claudio Costales 2015-07-19 21:57:01 UTC
(In reply to Katrin Fischer from comment #3)
> I think if we block renewals in the staff client, we should do the same in
> OPAC - normally staff client allows more than OPAC, the opposite behaviour
> seems unexpected.

Katrin, despite of your comment (that I agree), the behaviour in staff, as I understand, is the correct behaviour.
Comment 5 Katrin Fischer 2015-07-19 22:01:05 UTC
I think we are not disagreeing - I think the OPAC should also block renewals in this case if it was designed to do so in staff. It doesn't make sense to treat both separately. It would be nice if there was also a way to define the number of renewals possible for on-site-checkouts in the future.
Comment 6 Claudio Costales 2015-07-19 22:04:41 UTC
(In reply to Katrin Fischer from comment #5)
> I think we are not disagreeing - I think the OPAC should also block renewals
> in this case if it was designed to do so in staff. It doesn't make sense to
> treat both separately. It would be nice if there was also a way to define
> the number of renewals possible for on-site-checkouts in the future.

That was the meaning of my comment. I agree with you. Sorry if there was a misunderstanding.
Comment 7 Jonathan Druart 2015-08-04 10:29:40 UTC Comment hidden (obsolete)
Comment 8 Claudio Costales 2015-08-07 17:27:54 UTC
Jonathan,

I was testing in a local installation. All works fine if I discard checkout.js modifications. If I consider checkout.js modifications all checkouts in the checkout patron view are missing. Please see attached PDF.
Comment 9 Claudio Costales 2015-08-07 17:29:17 UTC
Created attachment 41430 [details]
Missing data display in checkout patron view
Comment 10 Jonathan Druart 2015-08-10 08:37:43 UTC
It seems to work here.
Are you sure you have correctly refreshed the page (to take into account the JS changes)?
You certainly got a JS error, please report it here.
Comment 11 Claudio Costales 2015-08-10 13:08:57 UTC
You are wright Jonathan, I'm soooooo sorry. All work fine, great job!

I've attached a PDF file with some legends suggestions.

Thank you very much Jonathan!
Comment 12 Claudio Costales 2015-08-10 13:10:36 UTC
Created attachment 41440 [details]
Legends suggestions
Comment 13 Mirko Tietgen 2015-08-11 12:12:32 UTC Comment hidden (obsolete)
Comment 14 Mirko Tietgen 2015-08-11 12:18:33 UTC
The display is not consistent. I observed:

OPAC (opac-user.pl, column »Renew«)
before patch (if renewal allowed according to circ matrix): »[checkbox] Renew (x of x renewals remaining)«
before patch (if renewal not allowed): »[no checkbox] Not renewable«
after patch: [empty]

Staff (Checkouts, tab »Check out«, column »Renew«)
before patch (if renewal allowed): »0«
before patch (if renewal not allowed): »0 Not renewable«
after patch: »0«

OPAC should display no checkbox, but the message »Not renewable«, as it does if renewal is not allowed for other reasons.

Staff should display »Not renewable«
Comment 15 Claudio Costales 2015-08-26 18:30:32 UTC
It's fine for me.
Comment 16 Katrin Fischer 2015-09-01 22:15:07 UTC
Patch doesn't apply due to a conflict in Circulation.t - can you please fix?
Comment 17 Jonathan Druart 2015-09-02 08:56:35 UTC Comment hidden (obsolete)
Comment 18 Katrin Fischer 2015-09-03 22:22:55 UTC Comment hidden (obsolete)
Comment 19 Katrin Fischer 2015-09-03 22:24:47 UTC
This is good - but I agree with bondiurbano - we should really mark the on-site checkouts in the checkouts list, maybe similar to how it is in the staff client. It's visible in the reading history only.
Comment 20 Katrin Fischer 2015-09-03 22:26:03 UTC
Created attachment 42366 [details] [review]
[PASSED QA] Bug 14470: Do not allow renew for on-site checkouts

At the opac, the renew checkbox should not be displayed if it's an
on-site checkout (same on the intranet).

On the way, this patch adds a specific message to the intranet if the
librarian try to renew an on-site checkout.
Indeed before this patch a renew was allowed if the barcode was scanned.

Test plan:
1/ Create an on-site checkout for a patron
2/ Confirm that the checkbox 'renew' is not displayed on the checkout
list tables
3/ At the OPAC, the renew should not be allowed (no checkbox)
4/ Try to check the item out to the same patron, confirm that you get a
specifig message to inform you the renew is not allowed for on-site
checkouts.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Changed 'issue' to 'item' in the error message.
Comment 21 Tomás Cohen Arazi 2015-09-07 15:13:30 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 22 Claudio Costales 2015-09-07 16:55:07 UTC
Thanks to all!
Comment 23 Chris Cormack 2015-09-22 16:50:20 UTC
Pushed to 3.20.x will be in 3.20.4
Comment 24 Liz Rea 2015-09-22 22:32:57 UTC
Pushed to 3.18.x will be in 3.18.11