Bugzilla – Attachment 73729 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-b.patch (text/plain), 8.95 KB, created by
Marcel de Rooy
on 2018-04-06 06:10:44 UTC
(
hide
)
Description:
Bug 18816: Make CataloguingLog work in production by preventing circulation from spamming the log
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2018-04-06 06:10:44 UTC
Size:
8.95 KB
patch
obsolete
>From 853570afac8e2a6fefcfa7e869b00f0c882336f7 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 >Content-Type: text/plain; charset=utf-8 > >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) Log into staff client >2) Home -> Koha administration -> Global system preferences -> Logs >3) Set only CataloguingLog to 'Log', everything else to "Don't log" >4) Click 'Save all Logging preferences' >5) In MySQL, use your instance DB, and then type 'delete from action_logs;' >6) Have a person checkout and checkin anything. >7) In MySQL, 'select * from action_logs;' > -- there will be data. This is the floodiness that will be removed. >8) Apply this patch >9) Repeat steps 5-7 > -- there should be no data. >10) Edit any biblio or item. >11) In MySQL, 'select * from action_logs;' > -- there should be data reflecting the changes made. >12) run koha qa test tools > >NOTE: Improved clarity of test plan -- Mark Tompsett > >Signed-off-by: Mark Tompsett <mtompset@hotmail.com> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > C4/Circulation.pm | 18 ++++++++++-------- > C4/Items.pm | 17 +++++++++++------ > t/db_dependent/Items.t | 46 +++++++++++++++++++++++++++++++++++++++++++--- > 3 files changed, 64 insertions(+), 17 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 3e711cd..15c05fc 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1397,7 +1397,8 @@ sub AddIssue { > datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(), > }, > $item->{'biblionumber'}, >- $item->{'itemnumber'} >+ $item->{'itemnumber'}, >+ 0 > ); > ModDateLastSeen( $item->{'itemnumber'} ); > >@@ -1830,7 +1831,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) >@@ -1854,7 +1855,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; > } > } >@@ -1920,7 +1921,7 @@ sub AddReturn { > > } > >- ModItem({ onloan => undef }, $item->{biblionumber}, $item->{'itemnumber'}); >+ ModItem( { onloan => undef }, $item->{biblionumber}, $item->{itemnumber}, 0 ); > } > > # the holdingbranch is updated if the document is returned to another location. >@@ -2156,7 +2157,7 @@ sub MarkIssueReturned { > # And finally delete the issue > $issue->delete; > >- ModItem( { 'onloan' => undef }, undef, $itemnumber ); >+ ModItem( { 'onloan' => undef }, undef, $itemnumber, 0 ); > > if ( C4::Context->preference('StoreLastBorrower') ) { > my $item = Koha::Items->find( $itemnumber ); >@@ -2395,7 +2396,7 @@ sub _FixAccountForLostAndReturned { > } > ); > >- ModItem( { paidfor => '' }, undef, $itemnumber ); >+ ModItem( { paidfor => '' }, undef, $itemnumber, 0 ); > > return $credit_id; > } >@@ -2820,7 +2821,7 @@ sub AddRenewal { > > # Update the renewal count on the item, and tell zebra to reindex > $renews = $item->{renewals} + 1; >- ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber); >+ ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber, 0 ); > > # Charge a new rental fee, if applicable? > my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber ); >@@ -3699,7 +3700,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 a3ecd87..f2c7dc2 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -517,7 +517,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. >@@ -538,12 +538,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) { >@@ -552,8 +556,8 @@ sub ModItem { > > my $dbh = @_ ? shift : C4::Context->dbh; > my $frameworkcode = @_ ? shift : C4::Biblio::GetFrameworkCode( $biblionumber ); >- >- my $unlinked_item_subfields; >+ >+ my $unlinked_item_subfields; > if (@_) { > $unlinked_item_subfields = shift; > $item->{'more_subfields_xml'} = _get_unlinked_subfields_xml($unlinked_item_subfields); >@@ -602,7 +606,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 >@@ -646,9 +651,9 @@ C<$itemnum> is the item number > > sub ModDateLastSeen { > my ($itemnumber) = @_; >- >+ > my $today = output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }); >- ModItem({ itemlost => 0, datelastseen => $today }, undef, $itemnumber); >+ ModItem( { itemlost => 0, datelastseen => $today }, undef, $itemnumber, 0 ); > } > > =head2 DelItem >diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t >index f6cc891..da74ca3 100755 >--- a/t/db_dependent/Items.t >+++ b/t/db_dependent/Items.t >@@ -25,13 +25,12 @@ use Koha::Database; > use Koha::DateUtils qw( dt_from_string ); > use Koha::Library; > use Koha::DateUtils; >+use Koha::MarcSubfieldStructures; >+use Koha::Caches; > > use t::lib::Mocks; > use t::lib::TestBuilder; > >-use Koha::MarcSubfieldStructures; >-use Koha::Caches; >- > use Test::More tests => 13; > > use Test::Warn; >@@ -797,6 +796,47 @@ subtest 'get_hostitemnumbers_of' => sub { > is( @itemnumbers, 0, ); > }; > >+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