Bug 36347 - Return claims table is loaded twice
Summary: Return claims table is loaded twice
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Julian Maurice
QA Contact: Lucas Gass
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-03-18 15:09 UTC by Julian Maurice
Modified: 2024-05-28 16:16 UTC (History)
6 users (show)

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


Attachments
Bug 36347: Avoid return claims table being loaded twice (2.46 KB, patch)
2024-03-18 15:09 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 36347: Avoid return claims table being loaded twice (2.51 KB, patch)
2024-03-22 22:13 UTC, David Nind
Details | Diff | Splinter Review
Bug 36347: Avoid return claims table being loaded twice (2.57 KB, patch)
2024-03-25 18:01 UTC, Lucas Gass
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2024-03-18 15:09:11 UTC
The issue is caused by these lines in circulation.js:

$("#finesholdsissues a[data-toggle='tab']").on("shown.bs.tab", function(e){
    $(this).click();
});

When the user clicks on a hidden tab, 'shown.bs.tab' is triggered and then another click event is triggered. And because the table is loaded on tab click, it is loaded twice.
Visually it can cause the rows to be duplicated (it is random, probably due to timing, network latency, ...)
Comment 1 Julian Maurice 2024-03-18 15:09:59 UTC
Created attachment 163340 [details] [review]
Bug 36347: Avoid return claims table being loaded twice

The issue is caused by these lines in circulation.js:

$("#finesholdsissues a[data-toggle='tab']").on("shown.bs.tab", function(e){
    $(this).click();
});

When the user clicks on a hidden tab, 'shown.bs.tab' is triggered and
then another click event is triggered. And because the table was loaded
on tab click, it was loaded twice.
Visually it could have caused the rows to be duplicated (it was random,
probably due to timing, network latency, ...)

I don't know why we need to trigger a click in this case, but removing
this will probably break something else, so the fix here is to load the
table on 'shown.bs.tab' event instead of 'click'

This patch also contains a small fix in refreshReturnClaimsTable which
was reloading the table right after the table initialization.

Test plan:
1. Check out an item, and in the checkouts table, click "Claim returned"
2. Open the network tab of your browser's developer tools
3. Click on the "Claims" tab, verify that only one call is made to
   svc/return_claims
4. Verify that the claims table is displayed correctly
Comment 2 David Nind 2024-03-22 22:13:11 UTC
Created attachment 163747 [details] [review]
Bug 36347: Avoid return claims table being loaded twice

The issue is caused by these lines in circulation.js:

$("#finesholdsissues a[data-toggle='tab']").on("shown.bs.tab", function(e){
    $(this).click();
});

When the user clicks on a hidden tab, 'shown.bs.tab' is triggered and
then another click event is triggered. And because the table was loaded
on tab click, it was loaded twice.
Visually it could have caused the rows to be duplicated (it was random,
probably due to timing, network latency, ...)

I don't know why we need to trigger a click in this case, but removing
this will probably break something else, so the fix here is to load the
table on 'shown.bs.tab' event instead of 'click'

This patch also contains a small fix in refreshReturnClaimsTable which
was reloading the table right after the table initialization.

Test plan:
1. Check out an item, and in the checkouts table, click "Claim returned"
2. Open the network tab of your browser's developer tools
3. Click on the "Claims" tab, verify that only one call is made to
   svc/return_claims
4. Verify that the claims table is displayed correctly

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2024-03-22 22:17:52 UTC
Testing notes (using KTD):

1. I had to play with the browser developer tools in the network section to display the right information:
   . Firefox: right-click on the columns > Reset Columns + display the URL column
   . Chromium: reset columns shown, information shows in the Path column

2. Before the patch, there were three requests shown for me.
Comment 4 Lucas Gass 2024-03-25 18:00:27 UTC
Changes make sense, works as intended, QA scripts are happy.
Comment 5 Lucas Gass 2024-03-25 18:01:08 UTC
Created attachment 163849 [details] [review]
Bug 36347: Avoid return claims table being loaded twice

The issue is caused by these lines in circulation.js:

$("#finesholdsissues a[data-toggle='tab']").on("shown.bs.tab", function(e){
    $(this).click();
});

When the user clicks on a hidden tab, 'shown.bs.tab' is triggered and
then another click event is triggered. And because the table was loaded
on tab click, it was loaded twice.
Visually it could have caused the rows to be duplicated (it was random,
probably due to timing, network latency, ...)

I don't know why we need to trigger a click in this case, but removing
this will probably break something else, so the fix here is to load the
table on 'shown.bs.tab' event instead of 'click'

This patch also contains a small fix in refreshReturnClaimsTable which
was reloading the table right after the table initialization.

Test plan:
1. Check out an item, and in the checkouts table, click "Claim returned"
2. Open the network tab of your browser's developer tools
3. Click on the "Claims" tab, verify that only one call is made to
   svc/return_claims
4. Verify that the claims table is displayed correctly

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 6 Katrin Fischer 2024-03-26 10:22:14 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 7 Fridolin Somers 2024-05-23 08:45:22 UTC
Pushed to 23.11.x for 23.11.06
Comment 8 Lucas Gass 2024-05-28 16:16:29 UTC
Backported to 23.05.x for upcoming 23.05.12