Bug 30928 - Add interface to statistics table
Summary: Add interface to statistics table
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement with 20 votes (vote)
Assignee: Lucas Gass
QA Contact: Aleisha Amohia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-06-08 21:02 UTC by hebah
Modified: 2023-06-14 12:48 UTC (History)
12 users (show)

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


Attachments
Bug 30928: Add interface column: DB update, schema changes, and UpdateStats (6.69 KB, patch)
2023-02-02 01:39 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30928: Update Accounts.pm and Recalls.pm (1.34 KB, patch)
2023-02-03 00:11 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30928: Add interface column: DB update, schema changes, and UpdateStats (8.84 KB, patch)
2023-02-03 22:57 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30928: Add tests (3.59 KB, patch)
2023-02-03 23:25 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30642: (follow-up) Remove use of deleted file (1.07 KB, text/plain)
2023-02-14 21:54 UTC, Emily Lamancusa
Details
Bug 30928: Add interface column: DB update, schema changes, and UpdateStats (8.89 KB, patch)
2023-02-14 21:58 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 30928: Add tests (3.58 KB, patch)
2023-03-07 13:19 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30928: Add interface column: DB update, schema changes, and UpdateStats (8.89 KB, patch)
2023-03-07 13:19 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30928: Add interface to statistics (4.99 KB, patch)
2023-04-28 14:32 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30928: DB and Schema updates (2.91 KB, patch)
2023-04-28 14:33 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30928: Add tests (3.65 KB, patch)
2023-04-30 21:34 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30928: Add interface to statistics (5.06 KB, patch)
2023-04-30 21:34 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30928: (QA follow-up) DB updates (2.14 KB, patch)
2023-04-30 21:34 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30928: (QA follow-up) Schema updates (1.55 KB, patch)
2023-04-30 21:34 UTC, Aleisha Amohia
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description hebah 2022-06-08 21:02:53 UTC
Many libraries are interested in seeing stats on the interfaces where transactions took place (staff intranet vs sip checkout, for example). Currently the only way to find that information is via action_logs, which means that data drops off eventually. It would be nice to have this information as part of statistics for ease of reporting.
Comment 1 raymond.bryson 2022-07-14 17:58:40 UTC
It would be incredibly useful to have the interface added to the statistics table. Our use case is this - 

We need to calculate self-checkout percentage, by library location. 

At the system-wide level (all branches), we can do this using the action_logs table. However, if we need granularity at the library branch level, we currently cannot provide accurate data for reporting/comparisons.  

For our use case, we would use the interface to compare Intranet (staff) vs SIP (self-checkouts), at the branch level.

There are other use cases where the interface (context) at the branch level might be useful too.
Comment 2 Sara Brown 2022-10-14 14:29:52 UTC
This would be great! Once it's added to statistics, it would be helpful to have it added to the pseudonymization options too.
Comment 3 Lucas Gass 2023-02-02 01:39:38 UTC
Created attachment 145982 [details] [review]
Bug 30928: Add interface column: DB update, schema changes, and UpdateStats
Comment 4 Lucas Gass 2023-02-03 00:11:17 UTC
Created attachment 146042 [details] [review]
Bug 30928: Update Accounts.pm and Recalls.pm
Comment 5 Lucas Gass 2023-02-03 22:57:08 UTC
Created attachment 146171 [details] [review]
Bug 30928: Add interface column: DB update, schema changes, and UpdateStats

To test:
Apply patch, updatedatabase, and restart_all

Do each of the following actions and check the statistics table making sure the interface is properly being recorded:

RENEWALS:
-Renewal from staff client
-Renew from OPAC
-Renew from API. You could add something like this to IntranetUSerJS:
$.ajax({
    url: '/api/v1/checkouts/338/renewal',
    method: 'POST',
    contentType: 'application/json',
    success: function(result) {
        console.log('renewed')
    },
});
-Renewal from automatic_renewals
-Renewal from SIP ( can use sip_cli_emulator.pl )

CHECKOUTS:
-Checkout from staff client
-Checkout from SCO
-Checkout from API
-Checkout from SIP ( can use sip_cli_emulator.pl )

CHECKIN:
-Checkin from staff client
-Check in from batch
-Check in from API
-Check in from SCI
-Check in from SIP ( can use sip_cli_emulator.pl )

RECALLS:
-OPAC recall
-Staff client recall

ACCOUNTING:
-Make a payment
-Make a payemnt from SIP ( can use sip_cli_emulator.pl )

prove -v /kohadevbox/koha/t/db_dependent/Circulation.t
Tests should all pass
Comment 6 Lucas Gass 2023-02-03 23:25:41 UTC
Created attachment 146172 [details] [review]
Bug 30928: Add tests

prove -v t/db_dependent/Koha/Statistics.t
prove -v t/db_dependent/Stats.t
Comment 7 Emily Lamancusa 2023-02-13 22:13:53 UTC
Testing Notes (KTD):

1 error:
ACCOUNTING
- When a payment was made from the staff interface, the Statistics table incorrectly logged that the transaction was made through the OPAC

2 unable to test:
- I'm not sure how to test CHECKIN and CHECKOUT via REST API. Any chance you can include JS snippets for that in your test plan, like you did for a renewal?

- Everything else, including prove commands, worked as expected :)
Comment 8 Lucas Gass 2023-02-14 13:21:10 UTC
> 1 error:
> ACCOUNTING
> - When a payment was made from the staff interface, the Statistics table
> incorrectly logged that the transaction was made through the OPAC

Hi, Emily. I am not recreating this. Can you provide some specific steps you use to log the transaction as OPAC via the staff client?
Comment 9 Emily Lamancusa 2023-02-14 21:44:14 UTC
Suddenly I wasn't able to reproduce it either...it turned out to be a very specific interaction between the intranet, OPAC, and SCO logins. It also looks like a separate bug that has nothing to do with your patch. (It didn't occur to me yesterday to compare with the system logs - but having checked them, the logs show the same error, both with and without the patch. Sorry for missing that yesterday!) Moved it to bug 32965.
Comment 10 Emily Lamancusa 2023-02-14 21:54:10 UTC Comment hidden (obsolete)
Comment 11 Emily Lamancusa 2023-02-14 21:58:24 UTC
Created attachment 146654 [details] [review]
Bug 30928: Add interface column: DB update, schema changes, and UpdateStats

To test:
Apply patch, updatedatabase, and restart_all

Do each of the following actions and check the statistics table making sure the interface is properly being recorded:

RENEWALS:
-Renewal from staff client
-Renew from OPAC
-Renew from API. You could add something like this to IntranetUSerJS:
$.ajax({
    url: '/api/v1/checkouts/338/renewal',
    method: 'POST',
    contentType: 'application/json',
    success: function(result) {
        console.log('renewed')
    },
});
-Renewal from automatic_renewals
-Renewal from SIP ( can use sip_cli_emulator.pl )

CHECKOUTS:
-Checkout from staff client
-Checkout from SCO
-Checkout from API
-Checkout from SIP ( can use sip_cli_emulator.pl )

CHECKIN:
-Checkin from staff client
-Check in from batch
-Check in from API
-Check in from SCI
-Check in from SIP ( can use sip_cli_emulator.pl )

RECALLS:
-OPAC recall
-Staff client recall

ACCOUNTING:
-Make a payment
-Make a payemnt from SIP ( can use sip_cli_emulator.pl )

prove -v /kohadevbox/koha/t/db_dependent/Circulation.t
Tests should all pass

Signed-off-by: emlam <emily.lamancusa@montgomerycountymd.gov>
Comment 12 Emily Lamancusa 2023-02-14 22:08:59 UTC
Testing Notes:

Couldn't find any routes for checkin/checkout in the API documentation.
All other use cases behave as expected, and tests pass.
Comment 13 Emily Lamancusa 2023-03-06 21:16:50 UTC
Patch no longer applies - merge conflict in C4/Circulation.pm

Please rebase
Comment 14 Lucas Gass 2023-03-07 13:19:01 UTC
Created attachment 147838 [details] [review]
Bug 30928: Add tests

prove -v t/db_dependent/Koha/Statistics.t
prove -v t/db_dependent/Stats.t
Comment 15 Lucas Gass 2023-03-07 13:19:04 UTC
Created attachment 147839 [details] [review]
Bug 30928: Add interface column: DB update, schema changes, and UpdateStats

To test:
Apply patch, updatedatabase, and restart_all

Do each of the following actions and check the statistics table making sure the interface is properly being recorded:

RENEWALS:
-Renewal from staff client
-Renew from OPAC
-Renew from API. You could add something like this to IntranetUSerJS:
$.ajax({
    url: '/api/v1/checkouts/338/renewal',
    method: 'POST',
    contentType: 'application/json',
    success: function(result) {
        console.log('renewed')
    },
});
-Renewal from automatic_renewals
-Renewal from SIP ( can use sip_cli_emulator.pl )

CHECKOUTS:
-Checkout from staff client
-Checkout from SCO
-Checkout from API
-Checkout from SIP ( can use sip_cli_emulator.pl )

CHECKIN:
-Checkin from staff client
-Check in from batch
-Check in from API
-Check in from SCI
-Check in from SIP ( can use sip_cli_emulator.pl )

RECALLS:
-OPAC recall
-Staff client recall

ACCOUNTING:
-Make a payment
-Make a payemnt from SIP ( can use sip_cli_emulator.pl )

prove -v /kohadevbox/koha/t/db_dependent/Circulation.t
Tests should all pass

Signed-off-by: emlam <emily.lamancusa@montgomerycountymd.gov>
Comment 16 Lucas Gass 2023-03-07 13:19:39 UTC
Rebased for current master
Comment 17 Aleisha Amohia 2023-04-28 03:52:38 UTC
Please move schema update to a separate patch. Once fixed I'll be happy to pass QA.
Comment 18 Lucas Gass 2023-04-28 14:32:57 UTC
Created attachment 150386 [details] [review]
Bug 30928: Add interface to statistics
Comment 19 Lucas Gass 2023-04-28 14:33:00 UTC
Created attachment 150387 [details] [review]
Bug 30928: DB and Schema updates
Comment 20 Lucas Gass 2023-04-28 14:34:13 UTC
(In reply to Aleisha Amohia from comment #17)
> Please move schema update to a separate patch. Once fixed I'll be happy to
> pass QA.

Thanks Aleisha! I have separated them.
Comment 21 Aleisha Amohia 2023-04-30 21:34:29 UTC
Created attachment 150433 [details] [review]
Bug 30928: Add tests

prove -v t/db_dependent/Koha/Statistics.t
prove -v t/db_dependent/Stats.t

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 22 Aleisha Amohia 2023-04-30 21:34:32 UTC
Created attachment 150434 [details] [review]
Bug 30928: Add interface to statistics

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 23 Aleisha Amohia 2023-04-30 21:34:35 UTC
Created attachment 150435 [details] [review]
Bug 30928: (QA follow-up) DB updates

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 24 Aleisha Amohia 2023-04-30 21:34:39 UTC
Created attachment 150436 [details] [review]
Bug 30928: (QA follow-up) Schema updates

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 25 Tomás Cohen Arazi 2023-05-10 17:19:10 UTC
Gentle message from the RM: the QA script tells the atomicupdate doesn't have the right permissions. I'm pushing this already, but keep in mind for future submissions.
Comment 26 Tomás Cohen Arazi 2023-05-10 17:19:47 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 27 Marcel de Rooy 2023-05-11 06:07:11 UTC
I am rebasing now bug 33608 on top of this one.
We have a failure after this patch set on Koha/Pseudonymization.t
Will add a follow-up here
Comment 28 Marcel de Rooy 2023-05-11 06:13:56 UTC
Hmm. It may come from another pushed patch set..
Comment 29 Marcel de Rooy 2023-05-11 06:47:11 UTC
(In reply to Marcel de Rooy from comment #28)
> Hmm. It may come from another pushed patch set..

It must be the authority relinking stuff that was pushed too. That makes sample_biblio crash in Pseudonymization.t.
So not caused here definitely.
Comment 30 Marcel de Rooy 2023-05-11 08:44:47 UTC
(In reply to Marcel de Rooy from comment #29)
> (In reply to Marcel de Rooy from comment #28)
> > Hmm. It may come from another pushed patch set..
> 
> It must be the authority relinking stuff that was pushed too. That makes
> sample_biblio crash in Pseudonymization.t.
> So not caused here definitely.

Opened bug 33718 for these issues. Disabling AutoLinkBiblios helps to get it passing again.
Comment 31 Matt Blenkinsop 2023-05-15 10:37:46 UTC
Enhancement - not backporting to 22.11.x
Comment 32 Caroline Cyr La Rose 2023-05-15 20:28:44 UTC
It looks like only backend changes. Nothing to add/change in the manual. Closing, but feel free to include more details if this needs to be documented in the manual.
Comment 33 Marcel de Rooy 2023-06-01 11:36:03 UTC
Will add a follow-up for this development on 33608. It would be good to default to C4::Context->interface instead of NULL, or always needing to pass it.
Comment 34 Katrin Fischer 2023-06-08 13:17:49 UTC
(In reply to Marcel de Rooy from comment #33)
> Will add a follow-up for this development on 33608. It would be good to
> default to C4::Context->interface instead of NULL, or always needing to pass
> it.

But much harder to spot missed/wrong values this way :)
Comment 35 Marcel de Rooy 2023-06-14 12:48:41 UTC
(In reply to Katrin Fischer from comment #34)
> (In reply to Marcel de Rooy from comment #33)
> > Will add a follow-up for this development on 33608. It would be good to
> > default to C4::Context->interface instead of NULL, or always needing to pass
> > it.
> 
> But much harder to spot missed/wrong values this way :)

Please explain what you mean. NULL would be a missing value, I suppose, that we fix there. If C4::Context->interface would provide a wrong value, we should address that elsewhere. But that should not be possible, since it already contains a list of valid options.