Bugzilla – Attachment 64393 Details for
Bug 18816
Make CataloguingLog work in production by preventing circulation from spamming the log
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18816 - Make CataloguingLog work in production by preventing circulation from spamming the log
Bug-18816---Make-CataloguingLog-work-in-production.patch (text/plain), 7.40 KB, created by
Kyle M Hall (khall)
on 2017-06-16 14:24:37 UTC
(
hide
)
Description:
Bug 18816 - Make CataloguingLog work in production by preventing circulation from spamming the log
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2017-06-16 14:24:37 UTC
Size:
7.40 KB
patch
obsolete
>From 203f59af616d86e7c7e823b21225d65a753a4efc Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Fri, 16 Jun 2017 08:27:29 -0400 >Subject: [PATCH] Bug 18816 - Make CataloguingLog work in production by > preventing circulation from spamming the log > >The system preference CataloguingLog is not recommended for use in >production. This is do to the fact that every checkin and checkout >generates one or more log entires. This seems to be not only bad >behavior, but unnecessary and outside the needs of CataloguingLog as we >have CirculationLog. > >Test Plan: >1) Enable CataloguingLog >2) Note the checkins and checkouts cause log entries >3) Apply this patch >4) Note that checkins and checkouts no longer cause log entries, > but editing the item will continue to cause log entries. >--- > C4/Circulation.pm | 18 ++++++++++-------- > C4/Items.pm | 9 +++++++-- > t/db_dependent/Items.t | 43 ++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 59 insertions(+), 11 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index ca41dd6..3fe78e0 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1421,7 +1421,8 @@ sub AddIssue { > datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(), > }, > $item->{'biblionumber'}, >- $item->{'itemnumber'} >+ $item->{'itemnumber'}, >+ 0 > ); > ModDateLastSeen( $item->{'itemnumber'} ); > >@@ -1863,7 +1864,7 @@ sub AddReturn { > $item->{location} = $item->{permanent_location}; > } > >- ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'} ); >+ ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'}, 0 ); > } > > # full item data, but no borrowernumber or checkout info (no issue) >@@ -1887,7 +1888,7 @@ sub AddReturn { > foreach my $key ( keys %$rules ) { > if ( $item->{notforloan} eq $key ) { > $messages->{'NotForLoanStatusUpdated'} = { from => $item->{notforloan}, to => $rules->{$key} }; >- ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber ); >+ ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber, 0 ); > last; > } > } >@@ -1959,7 +1960,7 @@ sub AddReturn { > > } > >- ModItem({ onloan => undef }, $issue->{'biblionumber'}, $item->{'itemnumber'}); >+ ModItem({ onloan => undef }, $issue->{'biblionumber'}, $item->{'itemnumber'}, 0 ); > } > > # the holdingbranch is updated if the document is returned to another location. >@@ -2200,7 +2201,7 @@ sub MarkIssueReturned { > > $dbh->do(q|DELETE FROM issues WHERE issue_id = ?|, undef, $issue_id); > >- ModItem( { 'onloan' => undef }, undef, $itemnumber ); >+ ModItem( { 'onloan' => undef }, undef, $itemnumber, 0 ); > > if ( C4::Context->preference('StoreLastBorrower') ) { > my $item = Koha::Items->find( $itemnumber ); >@@ -2437,7 +2438,7 @@ sub _FixAccountForLostAndReturned { > } > ); > >- ModItem( { paidfor => '' }, undef, $itemnumber ); >+ ModItem( { paidfor => '' }, undef, $itemnumber, 0 ); > > return $credit_id; > } >@@ -2894,7 +2895,7 @@ sub AddRenewal { > > # Update the renewal count on the item, and tell zebra to reindex > $renews = $biblio->{'renewals'} + 1; >- ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $biblio->{'biblionumber'}, $itemnumber); >+ ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $biblio->{'biblionumber'}, $itemnumber, 0); > > # Charge a new rental fee, if applicable? > my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber ); >@@ -3760,7 +3761,8 @@ sub ProcessOfflineReturn { > ModItem( > { renewals => 0, onloan => undef }, > $issue->{'biblionumber'}, >- $itemnumber >+ $itemnumber, >+ 0 > ); > return "Success."; > } else { >diff --git a/C4/Items.pm b/C4/Items.pm >index 26c14a1..d378a2c 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -518,7 +518,7 @@ sub ModItemFromMarc { > > =head2 ModItem > >- ModItem({ column => $newvalue }, $biblionumber, $itemnumber); >+ ModItem({ column => $newvalue }, $biblionumber, $itemnumber, $log_action ); > > Change one or more columns in an item record and update > the MARC representation of the item. >@@ -539,12 +539,16 @@ the derived value of a column such as C<items.cn_sort>, > this routine will perform the necessary calculation > and set the value. > >+If log_action is set to false, the action will not be logged. >+If log_action is true or undefined, the action will be logged. >+ > =cut > > sub ModItem { > my $item = shift; > my $biblionumber = shift; > my $itemnumber = shift; >+ my $log_action = shift // 1; > > # if $biblionumber is undefined, get it from the current item > unless (defined $biblionumber) { >@@ -603,7 +607,8 @@ sub ModItem { > # item status is possible > ModZebra( $biblionumber, "specialUpdate", "biblioserver" ); > >- logaction("CATALOGUING", "MODIFY", $itemnumber, "item ".Dumper($item)) if C4::Context->preference("CataloguingLog"); >+ logaction( "CATALOGUING", "MODIFY", $itemnumber, "item " . Dumper($item) ) >+ if $log_action && C4::Context->preference("CataloguingLog"); > } > > =head2 ModItemTransfer >diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t >index f85d48a..da9ba90 100755 >--- a/t/db_dependent/Items.t >+++ b/t/db_dependent/Items.t >@@ -26,7 +26,7 @@ use Koha::Library; > use t::lib::Mocks; > use t::lib::TestBuilder; > >-use Test::More tests => 11; >+use Test::More tests => 12; > > use Test::Warn; > >@@ -782,6 +782,47 @@ subtest '_mod_item_dates' => sub { > 'yetanotherdatetime is ok' ); > }; > >+subtest 'Test logging for AddItem' => sub { >+ >+ plan tests => 3; >+ >+ t::lib::Mocks::mock_preference('CataloguingLog', 1); >+ >+ $schema->storage->txn_begin; >+ >+ my $builder = t::lib::TestBuilder->new; >+ my $library = $builder->build({ >+ source => 'Branch', >+ }); >+ my $itemtype = $builder->build({ >+ source => 'Itemtype', >+ }); >+ >+ # Create a biblio instance for testing >+ t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); >+ my ($bibnum, $bibitemnum) = get_biblio(); >+ >+ # Add an item. >+ my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, location => $location, itype => $itemtype->{itemtype} } , $bibnum); >+ >+ # False means no logging >+ $schema->resultset('ActionLog')->search()->delete(); >+ ModItem({ location => $location }, $bibnum, $itemnumber, 0); >+ is( $schema->resultset('ActionLog')->count(), 0, 'False value does not trigger logging' ); >+ >+ # True means logging >+ $schema->resultset('ActionLog')->search()->delete(); >+ ModItem({ location => $location }, $bibnum, $itemnumber, 1, 'True value does trigger logging'); >+ is( $schema->resultset('ActionLog')->count(), 1 ); >+ >+ # Undefined defaults to true >+ $schema->resultset('ActionLog')->search()->delete(); >+ ModItem({ location => $location }, $bibnum, $itemnumber); >+ is( $schema->resultset('ActionLog')->count(), 1, 'Undefined value defaults to true, triggers logging' ); >+ >+ $schema->storage->txn_rollback; >+}; >+ > # Helper method to set up a Biblio. > sub get_biblio { > my ( $frameworkcode ) = @_; >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18816
:
64383
|
64393
|
65711
|
65712
|
68600
|
68647
|
73154
|
73155
|
73169
|
73170
|
73171
|
73729
|
73730
|
73731
|
73732
|
73733