Bug 14803 - Rental and fine fees are not updating on circulation screen
Summary: Rental and fine fees are not updating on circulation screen
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Srdjan Jankovic
QA Contact: Testopia
URL:
Whiteboard:
Keywords: Academy
Depends on: 11703
Blocks:
  Show dependency treegraph
 
Reported: 2015-09-10 23:19 UTC by Debbie
Modified: 2017-06-22 04:55 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Fix test data in issue.t and defaults for stats (4.19 KB, patch)
2016-06-10 00:58 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Test rental charges properly (3.90 KB, patch)
2016-06-10 00:58 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 14803: Reload checkouts table on renewal (3.26 KB, patch)
2016-06-10 00:59 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 14803: Reload checkouts table on renewal (3.69 KB, patch)
2016-06-17 02:13 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug 14803: KOHA.Checkouts js object (17.44 KB, patch)
2016-06-17 02:13 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug 14803: KOHA.Checkouts js object (18.54 KB, patch)
2016-07-15 01:54 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug 14803: KOHA.Checkouts js object (18.96 KB, patch)
2016-07-19 00:33 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 14803: Reload checkouts table on renewal (3.69 KB, patch)
2017-02-24 04:56 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bugfix: added missing use Koha::Account (591 bytes, patch)
2017-02-24 04:56 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug 14803: KOHA.Checkouts js object (19.17 KB, patch)
2017-02-24 04:57 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Screenshot showing a problem with the checkout box (15.22 KB, image/png)
2017-04-18 13:59 UTC, Owen Leonard
Details
bug 14803: KOHA.Checkouts js object (19.22 KB, patch)
2017-04-20 03:06 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Checking Out (20.40 KB, image/png)
2017-05-04 07:02 UTC, Josef Moravec
Details
bug 14803: KOHA.Checkouts js object (20.82 KB, patch)
2017-05-05 01:25 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug 14803: KOHA.Checkouts js object (21.00 KB, patch)
2017-05-08 00:47 UTC, Srdjan Jankovic
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Debbie 2015-09-10 23:19:10 UTC
To replicate:
 - From circulation screen attempt to renew an item with a rental fee
 - Note that fees and charges aren't updating: e.g. 
  the circulation screen should read under 
Attention:
Fees & Charges: Patron has Outstanding fees & charges of "x" amount. Make payment.
- Note "x" amount has not increase
- All fees/fines appear not to be updating
Comment 1 Jonathan Druart 2015-09-15 10:09:50 UTC
Kyle,
Have you any idea how we could fix that? Looks tricky.
Comment 2 Liz Rea 2015-10-02 02:34:26 UTC
Similar to Bug 14798
Comment 3 Srdjan Jankovic 2016-06-10 00:58:09 UTC Comment hidden (obsolete)
Comment 4 Srdjan Jankovic 2016-06-10 00:58:25 UTC Comment hidden (obsolete)
Comment 5 Srdjan Jankovic 2016-06-10 00:59:29 UTC Comment hidden (obsolete)
Comment 6 Srdjan Jankovic 2016-06-10 01:03:49 UTC
To test - Librarian interface - Circulation:

Have an item with rental charges. Checkout. Renew. Checkouts table should reload with rental charges added.
Comment 7 Srdjan Jankovic 2016-06-17 02:13:18 UTC Comment hidden (obsolete)
Comment 8 Srdjan Jankovic 2016-06-17 02:13:31 UTC Comment hidden (obsolete)
Comment 9 Srdjan Jankovic 2016-06-17 02:18:35 UTC
This is a bit bigger than expected. Requires testing issue blocking with fines over limit.

Also "Patron details" top " Checkouts are BLOCKED" message needs checking because it uses same component.
Comment 10 Owen Leonard 2016-07-12 18:16:55 UTC
(In reply to Srdjan Jankovic from comment #6)
> To test - Librarian interface - Circulation:
> 
> Have an item with rental charges. Checkout. Renew. Checkouts table should
> reload with rental charges added.

Is this the whole test plan for all patches?
Comment 11 Srdjan Jankovic 2016-07-13 00:03:44 UTC
Yes. And run t/db_dependent/Circulation/issue.t, first two patches is the test fix.
Comment 12 Owen Leonard 2016-07-13 13:17:50 UTC
This breaks the list of current checkouts. On the circulation page it says "0 checkouts" even though I have many. On the patron detail page it shows the correct count in the checkouts tab but lists no checkouts.

I strongly object to the delay this patch introduces to the appearance of the checkout form. In my test system there is a 5-second delay between the time the rest of the page renders and the time the checkout form appears.

The more AJAX we introduce on the checkout page the slower it gets. I don't think that's what we intend, but that's the real consequence.
Comment 13 Srdjan Jankovic 2016-07-15 01:54:45 UTC Comment hidden (obsolete)
Comment 14 Srdjan Jankovic 2016-07-15 02:07:50 UTC
Owen, I'm not sure why you are getting the delay. In my case once page has loaded form comes up instantaneously. Admittedly, my database is not a large one. Nevertheless, the only overhead introduced here is a http request/response, so if the connection is decent (and it should be for the librarian interface) everything should be fine.
The change moved one database query from the main handler to a service. As a matter of fact it replaced two calls to the same function with only one, which should only be faster. So when comparing, a fair comparison would be full time from the circulation.pl request inception to the point where the checkout form is shown.
Can you please give it another go with timing pre and post patch from the point patron card number is entered to the appearance of the item checkout form. In my env I can detect no difference.
Comment 15 Srdjan Jankovic 2016-07-19 00:33:05 UTC Comment hidden (obsolete)
Comment 16 Christopher Brannon 2016-08-07 07:20:13 UTC
From what I can tell, when the patches are applied, it breaks the form #mainform, and is not displayed.  Cannot follow test plan.
Comment 17 Srdjan Jankovic 2017-02-24 04:56:27 UTC
Created attachment 60629 [details] [review]
Bug 14803: Reload checkouts table on renewal

svc/checkouts: get real rental charges for patron, rather than itemtype
indication
Comment 18 Srdjan Jankovic 2017-02-24 04:56:40 UTC
Created attachment 60630 [details] [review]
Bugfix: added missing use Koha::Account
Comment 19 Srdjan Jankovic 2017-02-24 04:57:38 UTC
Created attachment 60631 [details] [review]
bug 14803: KOHA.Checkouts js object

* Use javascript to:
- update fines/checkouts info
- show/hide checkouts form and messages
* Created svc/patron/fines
Comment 20 Srdjan Jankovic 2017-02-24 05:00:38 UTC
Things have moved on meanwhile, and it seems to me that the page is being force reloaded now, so I'm not sure how useful this patch is. Maybe just a start of a full ajax solution.
Comment 21 Marc Véron 2017-04-06 08:04:12 UTC
(In reply to Srdjan Jankovic from comment #20)
> Things have moved on meanwhile, and it seems to me that the page is being
> force reloaded now, so I'm not sure how useful this patch is. Maybe just a
> start of a full ajax solution.

Srdjan, do I get it right: No sign-off needed at the moment? Can status be switched e.g. to "In Discussion"?
Comment 22 Srdjan Jankovic 2017-04-06 23:51:06 UTC
Ideally someone should try to replicate the original report in the top note. I could not. But in discussion is fine with me.
Comment 23 Owen Leonard 2017-04-18 13:59:13 UTC
Created attachment 62294 [details]
Screenshot showing a problem with the checkout box

(In reply to Srdjan Jankovic from comment #20)
> Things have moved on meanwhile, and it seems to me that the page is being
> force reloaded now, so I'm not sure how useful this patch is.

I don't find that this page is being reloaded when renewing from the checkboxes. This looks like it's working correctly to update the fee numbers, but something weird happens when the patron's fines are past the noissuescharge limit. The checkout form is hidden from all users, whether they have permission to override or not.
Comment 24 Srdjan Jankovic 2017-04-20 03:06:48 UTC
Created attachment 62432 [details] [review]
bug 14803: KOHA.Checkouts js object

* Use javascript to:
- update fines/checkouts info
- show/hide checkouts form and messages
* Created svc/patron/fines
Comment 25 Srdjan Jankovic 2017-04-20 03:10:53 UTC
(In reply to Owen Leonard from comment #23)
> Created attachment 62294 [details]
> Screenshot showing a problem with the checkout box
> 
> something weird happens when the patron's fines are past the
> noissuescharge limit. The checkout form is hidden from all users, whether
> they have permission to override or not.

That may be an existing condition. I've made some changes to take in account AllowFineOverride pref.
Comment 26 Josef Moravec 2017-05-04 07:02:06 UTC
Created attachment 63047 [details]
Checking Out

(In reply to Srdjan Jankovic from comment #25)
> (In reply to Owen Leonard from comment #23)
> > Created attachment 62294 [details]
> > Screenshot showing a problem with the checkout box
> > 
> > something weird happens when the patron's fines are past the
> > noissuescharge limit. The checkout form is hidden from all users, whether
> > they have permission to override or not.
> 
> That may be an existing condition. I've made some changes to take in account
> AllowFineOverride pref.

But now, when I have no fines owed, I can't see the checkout form and even any current checkouts...

See attached screenshot
Comment 27 Srdjan Jankovic 2017-05-05 01:25:53 UTC
Created attachment 63153 [details] [review]
bug 14803: KOHA.Checkouts js object

* Use javascript to:
- update fines/checkouts info
- show/hide checkouts form and messages
* Created svc/patron/fines
Comment 28 Srdjan Jankovic 2017-05-05 01:27:44 UTC
> But now, when I have no fines owed, I can't see the checkout form and even
> any current checkouts...

Oops. Corrected
Comment 29 Josef Moravec 2017-05-05 09:41:43 UTC
I have to more concerns about it:
1) The barcode field doesn't have focus after page is loaded - probably easy to fix, but important
2) Looks like it waits than issues are loaded and then show the barcode input field, maybe I am wrong and it just looks like, but in any case it is much slower then current master...
3) Message "loading, you can continue scanning" is badly placed (in the left top corner)
Comment 30 Srdjan Jankovic 2017-05-08 00:47:01 UTC
Created attachment 63230 [details] [review]
bug 14803: KOHA.Checkouts js object

* Use javascript to:
- update fines/checkouts info
- show/hide checkouts form and messages
* Created svc/patron/fines
Comment 31 Srdjan Jankovic 2017-05-08 00:55:50 UTC
(In reply to Josef Moravec from comment #29)

Hi Josef, thanks for testing this.

> I have to more concerns about it:
> 1) The barcode field doesn't have focus after page is loaded - probably easy
> to fix, but important
> 2) Looks like it waits than issues are loaded and then show the barcode
> input field, maybe I am wrong and it just looks like, but in any case it is
> much slower then current master...

Ok, now it should be behaving like master. I tried to add some accuracy, but drawbacks are obviously too big...

> 3) Message "loading, you can continue scanning" is badly placed (in the left
> top corner)

For me it is placed center of the table. At any rate I believe my patch did not change the table, it just makes a issuesTable.api().ajax.reload() call. Could you please a) check it in another browser b) compare to master. Thanks a lot.
Comment 32 Josef Moravec 2017-05-09 14:52:12 UTC
(In reply to Srdjan Jankovic from comment #31)
> (In reply to Josef Moravec from comment #29)
> 
> Hi Josef, thanks for testing this.
> 
> > I have to more concerns about it:
> > 1) The barcode field doesn't have focus after page is loaded - probably easy
> > to fix, but important
> > 2) Looks like it waits than issues are loaded and then show the barcode
> > input field, maybe I am wrong and it just looks like, but in any case it is
> > much slower then current master...
> 
> Ok, now it should be behaving like master. I tried to add some accuracy, but
> drawbacks are obviously too big...

It is Ok now, thanks

> > 3) Message "loading, you can continue scanning" is badly placed (in the left
> > top corner)
> 
> For me it is placed center of the table. At any rate I believe my patch did
> not change the table, it just makes a issuesTable.api().ajax.reload() call.
> Could you please a) check it in another browser b) compare to master. Thanks
> a lot.

Hm, strange... On master it's ok for me, on this patchset still bad... but I can't find out where's the problem...