Bug 14470 - Renewals should be disabled in OPAC if are referred to an on-site checkout
Summary: Renewals should be disabled in OPAC if are referred to an on-site checkout
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 10860
Blocks:
  Show dependency treegraph
 
Reported: 2015-06-28 17:15 UTC by Claudio Costales
Modified: 2016-06-21 21:38 UTC (History)
8 users (show)

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


Attachments
step-by-step sample (144.91 KB, application/pdf)
2015-06-28 17:16 UTC, Claudio Costales
Details
Bug 14470: Do not allow renew for on-site checkouts (7.53 KB, patch)
2015-08-04 10:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Missing data display in checkout patron view (86.52 KB, application/pdf)
2015-08-07 17:29 UTC, Claudio Costales
Details
Legends suggestions (168.02 KB, application/pdf)
2015-08-10 13:10 UTC, Claudio Costales
Details
Bug 14470: Do not allow renew for on-site checkouts (7.59 KB, patch)
2015-08-11 12:12 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 14470: Do not allow renew for on-site checkouts (7.67 KB, patch)
2015-09-02 08:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 14470: Do not allow renew for on-site checkouts (7.78 KB, patch)
2015-09-03 22:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14470: Do not allow renew for on-site checkouts (7.78 KB, patch)
2015-09-03 22:26 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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