I think it's high time we added a HoldsLog system preference to log creation, updating, and filling of holds. I would say the following is needed: 1) Hold creation ( staff/opac, bib, item, date, priority ) 2) All hold modifications ( changing of priorities except for reflowing priorities, trapping an item to a hold, deleting a hold, etc ) 3) Hold fulfillment
I'd add suspending/unsuspending to the list :)
I agree -- suspending/unsuspending. I think this is a really good start to help troubleshoot better when something with holds go awry, esp in high-hold and/or multi-branch environments. I'm sure there's more things to add, but this is a great start. Thanks Kyle!
I want this! I don't see it explicitly mentioned, will borrowernumbers for both the hold placer/modifier and the hold placee(who it's for) be recorded? I know logs usually track "Librarian"(user) and "Object"(borrower), so will this track the patron as both the "Librarian" and the "Object" if they originate the hold in the OPAC? Also, this sounds very similar to bug 6890.
*** Bug 15022 has been marked as a duplicate of this bug. ***
Created attachment 52824 [details] [review] Bug 14642: Add HoldsLog syspref This patch introduces a new syspref 'HoldsLog' which is to be used for controlling holds actions logs. Sponsored-by: NEKLS
Created attachment 52825 [details] [review] Bug 14642: Add logging for Holds This patch adds logging for several holds actions. Specifically for: - CREATE - CANCEL - SUSPEND - RESUME Sponsored-by: NEKLS
This is an initial submission to get some feedback from other people. It is still missing priority changes, fullfilment, etc. Please contact me if you have any advice or ideas of what to log.
Created attachment 52848 [details] [review] Bug 14642: Add logging for Holds This patch adds logging for several holds actions. Specifically for: - CREATE - CANCEL - SUSPEND - RESUME To test: - Enable the HoldsLog syspref - Add a hold on a record/item => SUCCESS: The log view shows the CREATE action - Click on the <Suspend> button => SUCCESS: The log view shows the SUSPEND action - Click on the <Unsuspend> button => SUCCESS: The log view shows the RESUME action - Click on the red cross, to delete the hold => SUCCESS: The log view shows the CANCEL action Note: This is WIP, as there are several other actions missing from the logs. Sponsored-by: NEKLS
One thing that would be useful is to log if its an opac hold or staff generated hold. In the UK public libraries standard reporting includes a requirement to report number of holds placed via opac
(In reply to Colin Campbell from comment #9) > One thing that would be useful is to log if its an opac hold or staff > generated hold. In the UK public libraries standard reporting includes a > requirement to report number of holds placed via opac Ack
Created attachment 52931 [details] [review] Bug 14642: Log on ::delete
(In reply to Colin Campbell from comment #9) > One thing that would be useful is to log if its an opac hold or staff > generated hold. In the UK public libraries standard reporting includes a > requirement to report number of holds placed via opac Colin, please take a look at bug 16829. And sign it :-D One thing I'd add (on another bugs, will try to get funding for it) is that SCO should be a separate interface from OPAC, and also we should make cli scripts explicitly set 'commandline' if they still don't.
Created attachment 53070 [details] [review] Bug 14642: Add logging for Holds This patch adds logging for several holds actions. Specifically for: - CREATE - CANCEL - DELETE - RESUME - SUSPEND To test: - Enable the HoldsLog syspref - Add a hold on a record/item => SUCCESS: The log view shows the CREATE action - Click on the <Suspend> button => SUCCESS: The log view shows the SUSPEND action - Click on the <Unsuspend> button => SUCCESS: The log view shows the RESUME action - Click on the red cross, to delete the hold => SUCCESS: The log view shows the CANCEL action Note: The DELETE action is logged when DelMember is called, with bug 16819 patches applied. Sponsored-by: NEKLS
Created attachment 53196 [details] [review] Bug 14642: Add logging for Holds This patch adds logging for several holds actions. Specifically for: - CREATE - CANCEL - DELETE - RESUME - SUSPEND - MODIFY To test: - Enable the HoldsLog syspref - Add a hold on a record/item => SUCCESS: The log view shows the CREATE action - Click on the <Suspend> button => SUCCESS: The log view shows the SUSPEND action - Click on the <Unsuspend> button => SUCCESS: The log view shows the RESUME action - Click on the red cross, to delete the hold => SUCCESS: The log view shows the CANCEL action Note: The DELETE action is logged when DelMember is called, with bug 16819 patches applied. Sponsored-by: NEKLS
Logging holds is enabled by default. Is that intended?
Created attachment 53466 [details] [review] [SIGNED-OFF] Bug 14642: Add logging for Holds This patch adds logging for several holds actions. Specifically for: - CREATE - CANCEL - DELETE - RESUME - SUSPEND - MODIFY To test: - Enable the HoldsLog syspref - Add a hold on a record/item => SUCCESS: The log view shows the CREATE action - Click on the <Suspend> button => SUCCESS: The log view shows the SUSPEND action - Click on the <Unsuspend> button => SUCCESS: The log view shows the RESUME action - Click on the red cross, to delete the hold => SUCCESS: The log view shows the CANCEL action Note: The DELETE action is logged when DelMember is called, with bug 16819 patches applied. Sponsored-by: NEKLS Signed-off-by: Liz Rea <liz@catalyst.net.nz> I also wonder about this going in defaulted on, but since the other logs are as well it seems ok to me.
Comment on attachment 52824 [details] [review] Bug 14642: Add HoldsLog syspref >From 133621157e8fe90cd4728970c23a4869d3212f43 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Fri, 24 Jun 2016 11:34:01 -0300 >Subject: [PATCH] Bug 14642: Add HoldsLog syspref > >This patch introduces a new syspref 'HoldsLog' which is to be used for controlling >holds actions logs. > >Sponsored-by: NEKLS >--- > .../data/mysql/atomicupdate/bug_14642_holds_logging.sql | 13 +++++++++++++ > installer/data/mysql/sysprefs.sql | 1 + > .../prog/en/modules/admin/preferences/logs.pref | 6 ++++++ > 3 files changed, 20 insertions(+) > create mode 100644 installer/data/mysql/atomicupdate/bug_14642_holds_logging.sql > >diff --git a/installer/data/mysql/atomicupdate/bug_14642_holds_logging.sql b/installer/data/mysql/atomicupdate/bug_14642_holds_logging.sql >new file mode 100644 >index 0000000..aca2ee3 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_14642_holds_logging.sql >@@ -0,0 +1,13 @@ >+INSERT IGNORE INTO systempreferences ( variable, value, options, explanation, type ) VALUES >+('HoldsLog','1',NULL,'If ON, log create/cancel/suspend/resume actions on holds.','YesNo'); >+ >+-- $DBversion = "16.06.00.XXX"; >+-- if ( CheckVersion($DBversion) ) { >+-- $dbh->do(q{ >+-- INSERT IGNORE INTO systempreferences ( variable, value, options, explanation, type ) VALUES >+-- ('HoldsLog','1',NULL,'If ON, log create/cancel/suspend/resume actions on holds.','YesNo'); >+-- }); >+ >+-- print "Upgrade to $DBversion done (Bug 14642: Add logging of hold modifications)\n"; >+-- SetVersion($DBversion); >+-- } >diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql >index cd0f8bf..099db56 100644 >--- a/installer/data/mysql/sysprefs.sql >+++ b/installer/data/mysql/sysprefs.sql >@@ -167,6 +167,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('HighlightOwnItemsOnOPAC','0','','If on, and a patron is logged into the OPAC, items from his or her home library will be emphasized and shown first in search results and item details.','YesNo'), > ('HighlightOwnItemsOnOPACWhich','PatronBranch','PatronBranch|OpacURLBranch','Decides which branch\'s items to emphasize. If PatronBranch, emphasize the logged in user\'s library\'s items. If OpacURLBranch, highlight the items of the Apache var BRANCHCODE defined in Koha\'s Apache configuration file.','Choice'), > ('HoldFeeMode','not_always','always|not_always','Set the hold fee mode','Choice'), >+('HoldsLog','1',NULL,'If ON, log create/cancel/suspend/resume actions on holds.','YesNo'), > ('HoldsQueueSkipClosed', '0', NULL, 'If enabled, any libraries that are closed when the holds queue is built will be ignored for the purpose of filling holds.', 'YesNo'), > ('HoldsToPullStartDate','2',NULL,'Set the default start date for the Holds to pull list to this many days ago','Integer'), > ('HomeOrHoldingBranch','holdingbranch','holdingbranch|homebranch','Used by Circulation to determine which branch of an item to check with independent branches on, and by search to determine which branch to choose for availability ','Choice'), >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref >index 8d6c4c8..4aa5790 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref >@@ -31,6 +31,12 @@ Logging: > off: "Don't log" > - when fines are charged, paid, or forgiven. > - >+ - pref: HoldsLog >+ choices: >+ on: Log >+ off: "Don't log" >+ - any actions on holds (create, cancel, suspend, resume, etc). >+ - > - pref: IssueLog > choices: > on: Log >-- >2.9.0
I'm not sure why the patch that introduces the syspref got obsoleted! Reverting it
Created attachment 53734 [details] [review] Bug 14642: (QA followup) Make HoldsLog default to 0 After a couple comments regarding the default value I picked for the new syspref, I've decided to set it 0, as libraries interested on the fetarue will just enable it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Tomas, regarding the Koha::Hold->delete method, I think you must provide tests. Have a look the first patch of bug 16907, I have overwritten Koha::Patron->delete which log as well and provided relevant tests.
(In reply to Jonathan Druart from comment #20) > Tomas, regarding the Koha::Hold->delete method, I think you must provide > tests. > Have a look the first patch of bug 16907, I have overwritten > Koha::Patron->delete which log as well and provided relevant tests. Will do!
Created attachment 54177 [details] [review] Bug 14642: (QA followup) Add tests for Koha::Hold->delete Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 54179 [details] [review] Bug 14642: Add HoldsLog syspref This patch introduces a new syspref 'HoldsLog' which is to be used for controlling holds actions logs. Sponsored-by: NEKLS Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 54180 [details] [review] Bug 14642: Add logging for Holds This patch adds logging for several holds actions. Specifically for: - CREATE - CANCEL - DELETE - RESUME - SUSPEND - MODIFY To test: - Enable the HoldsLog syspref - Add a hold on a record/item => SUCCESS: The log view shows the CREATE action - Click on the <Suspend> button => SUCCESS: The log view shows the SUSPEND action - Click on the <Unsuspend> button => SUCCESS: The log view shows the RESUME action - Click on the red cross, to delete the hold => SUCCESS: The log view shows the CANCEL action Note: The DELETE action is logged when DelMember is called, with bug 16819 patches applied. Sponsored-by: NEKLS Signed-off-by: Liz Rea <liz@catalyst.net.nz> I also wonder about this going in defaulted on, but since the other logs are as well it seems ok to me. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 54181 [details] [review] Bug 14642: (QA followup) Make HoldsLog default to 0 After a couple comments regarding the default value I picked for the new syspref, I've decided to set it 0, as libraries interested on the fetarue will just enable it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 54182 [details] [review] Bug 14642: (QA followup) Add tests for Koha::Hold->delete Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 54183 [details] [review] Bug 14642: Make sure the log will be written only if the hold has been deleted Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to master for 16.11, thanks Tomas, Jonathan!
*** Bug 6890 has been marked as a duplicate of this bug. ***
*** Bug 24472 has been marked as a duplicate of this bug. ***