Bug 27683

Summary: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: CatalogingAssignee: Fridolin Somers <fridolin.somers>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: arthur.suzuki, lucas, m.de.rooy, martin.renvoize, victor
Version: Main   
Hardware: All   
OS: All   
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
Bug Depends on: 20702, 22140    
Bug Blocks:    
Attachments: Bug 27683: Unit test for GetAnalyticsCount
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref
Bug 27683: Impact on catalogue/detail.pl
Bug 27683: Unit test for GetAnalyticsCount
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref
Bug 27683: Impact on catalogue/detail.pl
Bug 27683: Unit test for GetAnalyticsCount
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref
Bug 27683: Impact on catalogue/detail.pl
Bug 27683: Unit test for GetAnalyticsCount
Bug 27683: Bind results of GetAnalyticsCount to the EasyAnalyticalRecords pref
Bug 27683: Impact on catalogue/detail.pl

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.