Bugzilla – Attachment 166104 Details for
Bug 36526
Remove circular dependency from Koha::Objects
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com>
Bug-36526-Remove-circular-dependency-from-KohaObje.patch (text/plain), 8.04 KB, created by
HKS3 Tadeusz Sośnierz
on 2024-05-03 09:35:32 UTC
(
hide
)
Description:
Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com>
Filename:
MIME Type:
Creator:
HKS3 Tadeusz Sośnierz
Created:
2024-05-03 09:35:32 UTC
Size:
8.04 KB
patch
obsolete
>From 5d7efa2e9a13c0756424802510ce4b943515a403 Mon Sep 17 00:00:00 2001 >From: Julian Maurice <julian.maurice@biblibre.com> >Date: Thu, 4 Apr 2024 16:26:00 +0200 >Subject: [PATCH] Bug 36526: Remove circular dependency from Koha::Objects >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >Koha::Objects depends on Koha::DateUtils, which depends on C4::Context, >which depends on Koha::Config::SysPrefs, which depends on Koha::Objects > >Apart from the circular dependency, the dependency on C4::Context alone >is problematic as it loads a bunch of modules that are not needed at all >in Koha::Objects (YAML::XS and ZOOM for instance). >As Koha::Objects is used as a base for a lot of modules, we should take >care to only load the minimum required. > >This patch removes uses of Koha::DateUtils from Koha::Objects. >It was only used in Koha::Objects::filter_by_last_update > >filter_by_last_update now requires that the 'from' and 'to' parameters >must be DateTime objects. Previously it would also allow date and >datetime strings. This possibility was only used in two places: >* misc/cronjobs/cleanup_database.pl >* tools/cleanborrowers.pl > >Now they call dt_from_string first and pass a DateTime object to >filter_by_last_update > >Test plan: >1. Run `perl -cw Koha/Objects.pm`. It should only say: > "Koha/Objects.pm syntax OK" without warnings >2. Run `prove t/db_dependent/Koha/Objects.t` >3. Verify that misc/cronjobs/cleanup_database.pl works as before, > especially with the options --pseudo-transactions, > --pseudo-transactions-from and --pseudo-transactions-to >4. Go to Tools » Batch patron deletion and anonymization, check "Verify > you want to anonymize patron checkout history" and enter a date in > the text input below. Then click Next and verify that the correct > count of borrowers is shown. Click on the "Finish" button and verify > that the circulation history has been correctly anonymized > >See also bug 36432 > >Signed-off-by: Tadeusz âtadzikâ SoÅnierz <tadeusz@sosnierz.com> >--- > Koha/Objects.pm | 28 +++++++++++++++------------- > misc/cronjobs/cleanup_database.pl | 3 +++ > t/db_dependent/Koha/Objects.t | 25 +++++-------------------- > tools/cleanborrowers.pl | 4 +++- > 4 files changed, 26 insertions(+), 34 deletions(-) > >diff --git a/Koha/Objects.pm b/Koha/Objects.pm >index dc3d044bd3..a59e5d5712 100644 >--- a/Koha/Objects.pm >+++ b/Koha/Objects.pm >@@ -25,7 +25,6 @@ use Class::Inspector; > > use Koha::Database; > use Koha::Exceptions::Object; >-use Koha::DateUtils qw( dt_from_string ); > > =head1 NAME > >@@ -232,10 +231,10 @@ sub update { > > =head3 filter_by_last_update > >-my $filtered_objects = $objects->filter_by_last_update({ >- from => $date1, to => $date2, >- days|older_than => $days, min_days => $days, younger_than => $days, >-}); >+ my $filtered_objects = $objects->filter_by_last_update({ >+ from => $from_datetime, to => $to_datetime, >+ days|older_than => $days, min_days => $days, younger_than => $days, >+ }); > > You should pass at least one of the parameters: from, to, days|older_than, > min_days or younger_than. Make sure that they do not conflict with each other >@@ -243,7 +242,7 @@ to get meaningful results. > Note: from, to and min_days are inclusive! And by nature days|older_than > and younger_than are exclusive. > >-The from and to parameters can be DateTime objects or date strings. >+The from and to parameters must be DateTime objects. > > =cut > >@@ -254,21 +253,24 @@ sub filter_by_last_update { > Koha::Exceptions::MissingParameter->throw("Please pass: days|from|to|older_than|younger_than") > unless grep { exists $params->{$_} } qw/days from to older_than younger_than min_days/; > >+ foreach my $key (qw(from to)) { >+ if (exists $params->{$key} and ref $params->{$key} ne 'DateTime') { >+ Koha::Exceptions::WrongParameter->throw("'$key' parameter must be a DateTime object"); >+ } >+ } >+ > my $dtf = Koha::Database->new->schema->storage->datetime_parser; > foreach my $p ( qw/days older_than younger_than min_days/ ) { > next if !exists $params->{$p}; >- my $dt = Koha::DateUtils::dt_from_string(); >+ my $days = $params->{$p}; > my $operator = { days => '<', older_than => '<', min_days => '<=' }->{$p} // '>'; >- $dt->subtract( days => $params->{$p} )->truncate( to => 'day' ); >- $conditions->{$operator} = $dtf->format_datetime( $dt ); >+ $conditions->{$operator} = \['DATE_SUB(CURDATE(), INTERVAL ? DAY)', $days]; > } > if ( exists $params->{from} ) { >- my $from = ref($params->{from}) ? $params->{from} : dt_from_string($params->{from}); >- $conditions->{'>='} = $dtf->format_datetime( $from ); >+ $conditions->{'>='} = $dtf->format_datetime( $params->{from} ); > } > if ( exists $params->{to} ) { >- my $to = ref($params->{to}) ? $params->{to} : dt_from_string($params->{to}); >- $conditions->{'<='} = $dtf->format_datetime( $to ); >+ $conditions->{'<='} = $dtf->format_datetime( $params->{to} ); > } > > return $self->search( >diff --git a/misc/cronjobs/cleanup_database.pl b/misc/cronjobs/cleanup_database.pl >index 04cc934b9c..0193018129 100755 >--- a/misc/cronjobs/cleanup_database.pl >+++ b/misc/cronjobs/cleanup_database.pl >@@ -701,6 +701,9 @@ if ( defined $pPseudoTransactions or $pPseudoTransactionsFrom or $pPseudoTransac > $pPseudoTransactionsTo->set( hour => 23, minute => 59, second => 59 ); > } > } >+ if ($pPseudoTransactionsFrom) { >+ $pPseudoTransactionsFrom = dt_from_string($pPseudoTransactionsFrom); >+ } > my $anonymized_transactions = Koha::PseudonymizedTransactions->filter_by_last_update( > { > timestamp_column_name => 'datetime', >diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t >index a868b5c3d0..a5444fc2a8 100755 >--- a/t/db_dependent/Koha/Objects.t >+++ b/t/db_dependent/Koha/Objects.t >@@ -1162,31 +1162,16 @@ subtest "filter_by_last_update" => sub { > )->count; > is( $count, 3, '3 patrons have been updated between D-4 and D-2' ); > >- t::lib::Mocks::mock_preference( 'dateformat', 'metric' ); >- try { >+ throws_ok { > $count = $patrons->filter_by_last_update( > { timestamp_column_name => 'updated_on', from => '1970-12-31' } ) > ->count; >- } >- catch { >- ok( >- $_->isa( >- 'No exception raised, from and to parameters can take an iso formatted date' >- ) >- ); >- }; >- try { >+ } 'Koha::Exceptions::WrongParameter', 'from parameter must be a DateTime object'; >+ throws_ok { > $count = $patrons->filter_by_last_update( >- { timestamp_column_name => 'updated_on', from => '31/12/1970' } ) >+ { timestamp_column_name => 'updated_on', to => '1970-12-31' } ) > ->count; >- } >- catch { >- ok( >- $_->isa( >- 'No exception raised, from and to parameters can take an metric formatted date (depending on dateformat syspref)' >- ) >- ); >- }; >+ } 'Koha::Exceptions::WrongParameter', 'to parameter must be a DateTime object'; > > subtest 'Parameters older_than, younger_than' => sub { > my $now = dt_from_string(); >diff --git a/tools/cleanborrowers.pl b/tools/cleanborrowers.pl >index 55d98689fa..2da6fbca0f 100755 >--- a/tools/cleanborrowers.pl >+++ b/tools/cleanborrowers.pl >@@ -37,6 +37,8 @@ use CGI qw ( -utf8 ); > use C4::Auth qw( get_template_and_user ); > use C4::Output qw( output_html_with_http_headers ); > use C4::Members qw( GetBorrowersToExpunge ); >+ >+use Koha::DateUtils qw( dt_from_string ); > use Koha::Old::Checkouts; > use Koha::Patron::Categories; > use Koha::Patrons; >@@ -149,7 +151,7 @@ elsif ( $op eq 'cud-delete' && $step == 3 ) { > my $rows = Koha::Old::Checkouts > ->filter_by_anonymizable > ->filter_by_last_update({ >- to => $last_issue_date, timestamp_column_name => 'returndate' }) >+ to => dt_from_string($last_issue_date), timestamp_column_name => 'returndate' }) > ->anonymize; > > $template->param( >-- >2.43.0
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 36526
:
164431
|
164432
|
166104
|
166108