Bug 27683 - Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref
Summary: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on: 20702 22140
Blocks:
  Show dependency treegraph
 
Reported: 2021-02-11 13:41 UTC by Fridolin Somers
Modified: 2023-06-08 22:26 UTC (History)
5 users (show)

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


Attachments
Bug 27683: Unit test for GetAnalyticsCount (1.87 KB, patch)
2021-02-11 15:34 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref (1.31 KB, patch)
2021-02-11 15:34 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 27683: Impact on catalogue/detail.pl (1.31 KB, patch)
2021-02-11 15:34 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 27683: Unit test for GetAnalyticsCount (1.87 KB, patch)
2021-11-19 08:07 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref (1.31 KB, patch)
2021-11-19 08:07 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 27683: Impact on catalogue/detail.pl (1.31 KB, patch)
2021-11-19 08:07 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 27683: Unit test for GetAnalyticsCount (1.92 KB, patch)
2022-07-09 19:49 UTC, David Nind
Details | Diff | Splinter Review
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref (1.36 KB, patch)
2022-07-09 19:49 UTC, David Nind
Details | Diff | Splinter Review
Bug 27683: Impact on catalogue/detail.pl (1.36 KB, patch)
2022-07-09 19:49 UTC, David Nind
Details | Diff | Splinter Review
Bug 27683: Unit test for GetAnalyticsCount (1.98 KB, patch)
2022-07-14 09:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref (1.42 KB, patch)
2022-07-14 09:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27683: Impact on catalogue/detail.pl (1.42 KB, patch)
2022-07-14 09:36 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2021-02-11 13:41:45 UTC
C4::Items::GetAnalyticsCount is part of the easy analytics feature.
Like Bug 20702 make early return when the system preference EasyAnalyticalRecords is disabled.
Actually it may block an item deletion for wrong reason.
Comment 1 Fridolin Somers 2021-02-11 15:34:09 UTC
Created attachment 116767 [details] [review]
Bug 27683: Unit test for GetAnalyticsCount

Run prove t/db_dependent/Items/GetAnalyticsCount.t
Comment 2 Fridolin Somers 2021-02-11 15:34:26 UTC
Created attachment 116768 [details] [review]
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref

C4::Items::GetAnalyticsCount is part of the easy analytics feature.
Like Bug 20702 make early return when the system preference EasyAnalyticalRecords is disabled.
Actually it may block an item deletion for wrong reason.

Test plan :
1) Dont apply patch
2) Build an item and a linked analytical record with 773$0 and $9
3) Enable EasyAnalyticalRecords
4) Try to delete the item
5) You have an alert because linked to analytics
6) Disable EasyAnalyticalRecords
7) Try to delete the item
8) You have an alert because linked to analytics
9) Apply patch
10) Try to delete the item
11) No alert, it works :D
Comment 3 Fridolin Somers 2021-02-11 15:34:38 UTC
Created attachment 116769 [details] [review]
Bug 27683: Impact on catalogue/detail.pl

Since EasyAnalyticalRecords pref is now in GetAnalyticsCount, remove
test in catalogue/detail.pl.

Test plan :
1) Build an item and a linked analytical record with 773$0 and $9
2) Go to record detail page
3) Enable EasyAnalyticalRecords
4) You see analytics tab and analytical count on item
5) Disable EasyAnalyticalRecords
6) You dont see analytics tab
Comment 4 Fridolin Somers 2021-11-19 08:07:21 UTC
Created attachment 127839 [details] [review]
Bug 27683: Unit test for GetAnalyticsCount

Run prove t/db_dependent/Items/GetAnalyticsCount.t
Comment 5 Fridolin Somers 2021-11-19 08:07:34 UTC
Created attachment 127840 [details] [review]
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref

C4::Items::GetAnalyticsCount is part of the easy analytics feature.
Like Bug 20702 make early return when the system preference EasyAnalyticalRecords is disabled.
Actually it may block an item deletion for wrong reason.

Test plan :
1) Dont apply patch
2) Build an item and a linked analytical record with 773$0 and $9
3) Enable EasyAnalyticalRecords
4) Try to delete the item
5) You have an alert because linked to analytics
6) Disable EasyAnalyticalRecords
7) Try to delete the item
8) You have an alert because linked to analytics
9) Apply patch
10) Try to delete the item
11) No alert, it works :D
Comment 6 Fridolin Somers 2021-11-19 08:07:48 UTC
Created attachment 127841 [details] [review]
Bug 27683: Impact on catalogue/detail.pl

Since EasyAnalyticalRecords pref is now in GetAnalyticsCount, remove
test in catalogue/detail.pl.

Test plan :
1) Build an item and a linked analytical record with 773$0 and $9
2) Go to record detail page
3) Enable EasyAnalyticalRecords
4) You see analytics tab and analytical count on item
5) Disable EasyAnalyticalRecords
6) You dont see analytics tab
Comment 7 David Nind 2022-07-09 19:49:24 UTC
Created attachment 137447 [details] [review]
Bug 27683: Unit test for GetAnalyticsCount

Run prove t/db_dependent/Items/GetAnalyticsCount.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2022-07-09 19:49:28 UTC
Created attachment 137448 [details] [review]
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref

C4::Items::GetAnalyticsCount is part of the easy analytics feature.
Like Bug 20702 make early return when the system preference EasyAnalyticalRecords is disabled.
Actually it may block an item deletion for wrong reason.

Test plan :
1) Dont apply patch
2) Build an item and a linked analytical record with 773$0 and $9
3) Enable EasyAnalyticalRecords
4) Try to delete the item
5) You have an alert because linked to analytics
6) Disable EasyAnalyticalRecords
7) Try to delete the item
8) You have an alert because linked to analytics
9) Apply patch
10) Try to delete the item
11) No alert, it works :D

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2022-07-09 19:49:32 UTC
Created attachment 137449 [details] [review]
Bug 27683: Impact on catalogue/detail.pl

Since EasyAnalyticalRecords pref is now in GetAnalyticsCount, remove
test in catalogue/detail.pl.

Test plan :
1) Build an item and a linked analytical record with 773$0 and $9
2) Go to record detail page
3) Enable EasyAnalyticalRecords
4) You see analytics tab and analytical count on item
5) Disable EasyAnalyticalRecords
6) You dont see analytics tab

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Martin Renvoize 2022-07-14 09:36:42 UTC
Created attachment 137700 [details] [review]
Bug 27683: Unit test for GetAnalyticsCount

Run prove t/db_dependent/Items/GetAnalyticsCount.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2022-07-14 09:36:46 UTC
Created attachment 137701 [details] [review]
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref

C4::Items::GetAnalyticsCount is part of the easy analytics feature.
Like Bug 20702 make early return when the system preference EasyAnalyticalRecords is disabled.
Actually it may block an item deletion for wrong reason.

Test plan :
1) Dont apply patch
2) Build an item and a linked analytical record with 773$0 and $9
3) Enable EasyAnalyticalRecords
4) Try to delete the item
5) You have an alert because linked to analytics
6) Disable EasyAnalyticalRecords
7) Try to delete the item
8) You have an alert because linked to analytics
9) Apply patch
10) Try to delete the item
11) No alert, it works :D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2022-07-14 09:36:49 UTC
Created attachment 137702 [details] [review]
Bug 27683: Impact on catalogue/detail.pl

Since EasyAnalyticalRecords pref is now in GetAnalyticsCount, remove
test in catalogue/detail.pl.

Test plan :
1) Build an item and a linked analytical record with 773$0 and $9
2) Go to record detail page
3) Enable EasyAnalyticalRecords
4) You see analytics tab and analytical count on item
5) Disable EasyAnalyticalRecords
6) You dont see analytics tab

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2022-07-14 09:37:42 UTC
Great to see these old routines getting a little love and cleanup, thanks Fridolin..

Tests added, tidy up works as expected.. history confirmed.

Passing QA
Comment 14 Tomás Cohen Arazi 2022-07-18 21:54:11 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 15 Fridolin Somers 2022-07-22 08:38:17 UTC
Greaaaaaate thanks to all.
This was a huge performance issue for some cases.
Comment 16 Fridolin Somers 2022-07-22 08:38:51 UTC
Pleaaaaaaaaaase backport to 22.05 and 21.11.
Comment 17 Lucas Gass 2022-08-23 19:40:15 UTC
Backported to 22.05.x for 22.05.05
Comment 18 Arthur Suzuki 2022-08-30 09:27:46 UTC
thx pushed to 21.11.x for 21.11.12
Comment 19 Victor Grousset/tuxayo 2022-09-14 19:25:53 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document, marking resolved.