Attachment 153950 Details for Bug 33837 – Bug 33837: Replace days_inclusive by min_days

[patch] Bug 33837: Replace days_inclusive by min_days

Bug-33837-Replace-daysinclusive-by-mindays.patch (text/plain), 6.26 KB, created by Jonathan Druart on 2023-07-27 09:10:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2023-07-27 09:10:48 UTC

Size: 6.26 KB

patch

obsolete

>From 947da14c2c138d0a4c616d8aae9a5804df463f70 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Fri, 26 May 2023 07:26:15 +0000
>Subject: [PATCH] Bug 33837: Replace days_inclusive by min_days
>
>Instead of passing days and days_inclusive, this patch adds
>min_days as replacement. Since days_inclusive is not widely
>used, this can be done easily. It removes the confusion
>whether days_inclusive impacted other parameters or not.
>
>Test plan:
>Run t/db_dependent/Koha/Objects.t
>Run t/db_dependent/Koha/Old/Checkouts.t
>Run t/db_dependent/Koha/Patrons.t (verifying the change in the
>Koha::Patrons module for filtering by expiration date).
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> Koha/Objects.pm                     | 26 ++++++++++----------------
> Koha/Patrons.pm                     |  3 +--
> t/db_dependent/Koha/Objects.t       |  6 +++---
> t/db_dependent/Koha/Old/Checkouts.t |  2 +-
> 4 files changed, 15 insertions(+), 22 deletions(-)
>
>diff --git a/Koha/Objects.pm b/Koha/Objects.pm
>index 9e8c90f2249..70f5be29b3d 100644
>--- a/Koha/Objects.pm
>+++ b/Koha/Objects.pm
>@@ -233,17 +233,16 @@ sub update {
> =head3 filter_by_last_update
> 
> my $filtered_objects = $objects->filter_by_last_update({
>-    days => $days, from => $date1, to => $date2, days_inclusive => 1,
>-    older_than => $days, younger_than => $days,
>+    from => $date1, to => $date2,
>+    days|older_than => $days, min_days => $days, younger_than => $days,
>     datetime => 1,
> });
> 
>-You should pass at least one of the parameters: days, from, to, older_than,
>-younger_than. Make sure that they do not conflict with each other to get
>-meaningful results.
>-By default, from and to are inclusive and days is exclusive (unless you
>-passed the optional days_inclusive parameter).
>-By nature older_than and younger_than are exclusive. This cannot be changed.
>+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
>+to get meaningful results.
>+Note: from, to and min_days are inclusive! And by nature days|older_than
>+and younger_than are exclusive.
> The optional parameter datetime allows datetime comparison.
> 
> The from and to parameters can be DateTime objects or date strings.
>@@ -253,21 +252,16 @@ The from and to parameters can be DateTime objects or date strings.
> sub filter_by_last_update {
>     my ( $self, $params ) = @_;
>     my $timestamp_column_name = $params->{timestamp_column_name} || 'timestamp';
>-    my $days_inclusive = $params->{days_inclusive} || 0;
>     my $conditions;
>     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/;
>+        unless grep { exists $params->{$_} } qw/days from to older_than younger_than min_days/;
> 
>     my $dtf = Koha::Database->new->schema->storage->datetime_parser;
>     my $method =  $params->{datetime} ? 'format_datetime' : 'format_date';
>-    foreach my $p ( qw/days older_than younger_than/  ) {
>+    foreach my $p ( qw/days older_than younger_than min_days/  ) {
>         next if !exists $params->{$p};
>         my $dt = Koha::DateUtils::dt_from_string();
>-        my $operator = $p eq 'days' && $days_inclusive
>-            ? '<='
>-            : $p eq 'younger_than'
>-            ? '>'
>-            : '<';
>+        my $operator = { days => '<', older_than => '<', min_days => '<=' }->{$p} // '>';
>         $conditions->{$operator} = $dtf->$method( $dt->subtract( days => $params->{$p} ) );
>     }
>     if ( exists $params->{from} ) {
>diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm
>index c210f53f4bc..92d3918363a 100644
>--- a/Koha/Patrons.pm
>+++ b/Koha/Patrons.pm
>@@ -205,8 +205,7 @@ sub filter_by_expiration_date {
>     return $class->filter_by_last_update(
>         {
>             timestamp_column_name => 'dateexpiry',
>-            days                  => $params->{days} || 0,
>-            days_inclusive        => 1,
>+            min_days              => $params->{days} || 0,
>         }
>     );
> }
>diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t
>index e62e802b9e9..09082df574d 100755
>--- a/t/db_dependent/Koha/Objects.t
>+++ b/t/db_dependent/Koha/Objects.t
>@@ -1126,7 +1126,7 @@ subtest "filter_by_last_update" => sub {
>     is( $count, 3, '3 patrons have been updated before the last 2 days (exclusive)' );
> 
>     $count = $patrons->filter_by_last_update(
>-        { timestamp_column_name => 'updated_on', days => 2, days_inclusive => 1 } )->count;
>+        { timestamp_column_name => 'updated_on', min_days => 2 } )->count;
>     is( $count, 4, '4 patrons have been updated before the last 2 days (inclusive)' );
> 
>     $count = $patrons->filter_by_last_update(
>@@ -1134,7 +1134,7 @@ subtest "filter_by_last_update" => sub {
>     is( $count, 4, '4 patrons have been updated before yesterday (exclusive)' );
> 
>     $count = $patrons->filter_by_last_update(
>-        { timestamp_column_name => 'updated_on', days => 1, days_inclusive => 1 } )->count;
>+        { timestamp_column_name => 'updated_on', min_days => 1 } )->count;
>     is( $count, 5, '5 patrons have been updated before yesterday (inclusive)' );
> 
>     $count = $patrons->filter_by_last_update(
>@@ -1142,7 +1142,7 @@ subtest "filter_by_last_update" => sub {
>     is( $count, 5, '5 patrons have been updated before today (exclusive)' );
> 
>     $count = $patrons->filter_by_last_update(
>-        { timestamp_column_name => 'updated_on', days => 0, days_inclusive => 1 } )->count;
>+        { timestamp_column_name => 'updated_on', min_days => 0 } )->count;
>     is( $count, 6, '6 patrons have been updated before today (inclusive)' );
> 
>     $count = $patrons->filter_by_last_update(
>diff --git a/t/db_dependent/Koha/Old/Checkouts.t b/t/db_dependent/Koha/Old/Checkouts.t
>index 2c7c52ad620..250b258a6e1 100755
>--- a/t/db_dependent/Koha/Old/Checkouts.t
>+++ b/t/db_dependent/Koha/Old/Checkouts.t
>@@ -94,7 +94,7 @@ subtest 'anonymize() tests' => sub {
> 
>     # filter them so only the older two are part of the resultset
>     my $checkouts = $patron->old_checkouts->filter_by_last_update(
>-        { days => 1, days_inclusive => 1 } );
>+        { min_days => 1 } );
> 
>     t::lib::Mocks::mock_preference( 'AnonymousPatron', undef );
>     throws_ok
>-- 
>2.25.1

Actions: View | Diff | Splinter Review

Attachments on bug 33837: 151720 | 151721 | 151722 | 151723 | 153778 | 153898 | 153900 | 153915 | 153916 | 153917 | 153918 | 153919 | 153920 | 153947 | 153948 | 153949 | 153950 | 153951