From 811e57c44165adc16152c756e14d0300d8db1104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= Date: Thu, 6 Aug 2026 10:45:13 -0300 Subject: [PATCH] Bug 43241: Add tests for datetime/boolean fixup inside DBIC operators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds regression tests proving that RFC3339 datetime values and boolean values nested inside DBIC operators (-and, -or, -between) in q= queries are not converted to their DB-native format. Unit tests (Query.t): - Datetime: >=, <=, range, -or, -between, nested -and/-or, me. prefix - Boolean: true/false inside -and, -or, nested -and within -or - Non-datetime values pass through unchanged Integration tests (Objects.t): - Full API stack tests using action_logs with controlled data - Same operator/nesting coverage as unit tests Test plan: 1. Apply patch 2. Run: $ prove t/Koha/REST/Plugin/Query.t $ prove t/db_dependent/Koha/REST/Plugin/Objects.t => FAIL: Tests fail! Values inside -and are not converted 3. Sign off :-D Signed-off-by: Tomás Cohen Arazi Signed-off-by: David Nind Signed-off-by: Jonathan Druart Signed-off-by: Pedro Amorim --- t/Koha/REST/Plugin/Query.t | 166 +++++++++++++++++++++- t/db_dependent/Koha/REST/Plugin/Objects.t | 146 ++++++++++++++++++- 2 files changed, 306 insertions(+), 6 deletions(-) diff --git a/t/Koha/REST/Plugin/Query.t b/t/Koha/REST/Plugin/Query.t index 88e60447f8..53190460e4 100755 --- a/t/Koha/REST/Plugin/Query.t +++ b/t/Koha/REST/Plugin/Query.t @@ -21,6 +21,7 @@ use Modern::Perl; use Mojolicious::Lite; use Try::Tiny; +use Koha::ActionLogs; use Koha::Cities; use Koha::Holds; use Koha::Biblios; @@ -349,6 +350,24 @@ get '/merge_q_params' => sub { $c->render( json => $filtered_params, status => 200 ); }; +get '/merge_q_params_datetime' => sub { + my $c = shift; + my $filtered_params = {}; + my $result_set = Koha::ActionLogs->new; + $filtered_params = $c->merge_q_params( $filtered_params, $c->req->json->{q}, $result_set ); + + $c->render( json => $filtered_params, status => 200 ); +}; + +get '/merge_q_params_patrons' => sub { + my $c = shift; + my $filtered_params = {}; + my $result_set = Koha::Patrons->new; + $filtered_params = $c->merge_q_params( $filtered_params, $c->req->json->{q}, $result_set ); + + $c->render( json => $filtered_params, status => 200 ); +}; + get '/build_query' => sub { my $c = shift; my ( $filtered_params, $reserved_params ) = $c->extract_reserved_params( $c->req->params->to_hash ); @@ -561,7 +580,7 @@ get '/ill_requests_count_sort_correctness' => sub { # The tests use Test::NoWarnings; -use Test::More tests => 13; +use Test::More tests => 15; use Test::Mojo; subtest 'extract_reserved_params() tests' => sub { @@ -720,7 +739,7 @@ subtest '/merge_q_params' => sub { q => { "-not_bool" => "suggestions.suggester.patron_card_lost", "-or" => [ - { "creation_date" => { "!=" => [ "fff", "zzz", "xxx" ] } }, + { "creation_date" => { "!=" => [ "2024-01-01", "2024-06-15", "2025-01-01" ] } }, { "suggestions.suggester.housebound_profile.frequency" => "123" }, { "suggestions.suggester.library_id" => { "like" => "%CPL%" } } ] @@ -734,9 +753,9 @@ subtest '/merge_q_params' => sub { { "datecreated" => { "!=" => [ - "fff", - "zzz", - "xxx" + "2024-01-01", + "2024-06-15", + "2025-01-01" ] } }, @@ -749,6 +768,143 @@ subtest '/merge_q_params' => sub { ); }; +subtest '_parse_dbic_query datetime fixup' => sub { + plan tests => 21; + + my $t = Test::Mojo->new; + + # Test 1: datetime with >= inside -and + $t->get_ok( '/merge_q_params_datetime' => json => + { q => { '-and' => [ { timestamp => { '>=' => '2026-08-01T00:00:00Z' }, module => 'MEMBERS' } ] } } ) + ->status_is(200) + ->json_is( '' => { '-and' => [ { timestamp => { '>=' => '2026-08-01 00:00:00' }, module => 'MEMBERS' } ] } ); + + # Test 2: datetime range (>= and <=) inside -and + $t->get_ok( + '/merge_q_params_datetime' => json => { + q => { '-and' => [ { timestamp => { '>=' => '2026-08-01T00:00:00Z', '<=' => '2026-08-05T23:59:59Z' } } ] } + } + ) + ->status_is(200) + ->json_is( + '' => { '-and' => [ { timestamp => { '>=' => '2026-08-01 00:00:00', '<=' => '2026-08-05 23:59:59' } } ] } ); + + # Test 3: datetime inside -or + $t->get_ok( + '/merge_q_params_datetime' => json => { + q => { + '-or' => [ + { timestamp => { '>=' => '2026-08-01T00:00:00Z' } }, + { timestamp => { '<=' => '2026-07-01T00:00:00Z' } }, + ] + } + } + )->status_is(200)->json_is( + '' => { + '-or' => [ + { timestamp => { '>=' => '2026-08-01 00:00:00' } }, + { timestamp => { '<=' => '2026-07-01 00:00:00' } }, + ] + } + ); + + # Test 4: datetime with -between inside -and + $t->get_ok( + '/merge_q_params_datetime' => json => { + q => { '-and' => [ { timestamp => { '-between' => [ '2026-08-01T00:00:00Z', '2026-08-05T23:59:59Z' ] } } ] } + } + ) + ->status_is(200) + ->json_is( + '' => { '-and' => [ { timestamp => { '-between' => [ '2026-08-01 00:00:00', '2026-08-05 23:59:59' ] } } ] } ); + + # Test 5: nested -and inside -or + $t->get_ok( + '/merge_q_params_datetime' => json => { + q => { + '-or' => [ + { '-and' => [ { timestamp => { '>=' => '2026-08-01T00:00:00Z' } } ] }, + { '-and' => [ { timestamp => { '<=' => '2026-07-01T00:00:00Z' } } ] }, + ] + } + } + )->status_is(200)->json_is( + '' => { + '-or' => [ + { '-and' => [ { timestamp => { '>=' => '2026-08-01 00:00:00' } } ] }, + { '-and' => [ { timestamp => { '<=' => '2026-07-01 00:00:00' } } ] }, + ] + } + ); + + # Test 6: me. prefix + $t->get_ok( '/merge_q_params_datetime' => json => + { q => { '-and' => [ { 'me.timestamp' => { '>=' => '2026-08-01T00:00:00Z' } } ] } } ) + ->status_is(200) + ->json_is( '' => { '-and' => [ { 'me.timestamp' => { '>=' => '2026-08-01 00:00:00' } } ] } ); + + # Test 7: non-datetime values pass through unchanged + $t->get_ok( '/merge_q_params_datetime' => json => + { q => { '-and' => [ { module => 'CRONJOBS', action => { '-in' => [ 'ADD', 'MODIFY' ] } } ] } } ) + ->status_is(200) + ->json_is( '' => { '-and' => [ { module => 'CRONJOBS', action => { '-in' => [ 'ADD', 'MODIFY' ] } } ] } ); +}; + +subtest '_parse_dbic_query boolean fixup' => sub { + plan tests => 12; + + my $t = Test::Mojo->new; + + # Test 1: boolean true inside -and + $t->get_ok( + '/merge_q_params_patrons' => json => { q => { '-and' => [ { patron_card_lost => 1, library_id => 'MPL' } ] } } ) + ->status_is(200) + ->json_is( '' => { '-and' => [ { lost => 1, branchcode => 'MPL' } ] } ); + + # Test 2: boolean false inside -and + $t->get_ok( '/merge_q_params_patrons' => json => { q => { '-and' => [ { patron_card_lost => 0 } ] } } ) + ->status_is(200) + ->json_is( '' => { '-and' => [ { lost => 0 } ] } ); + + # Test 3: boolean inside -or + $t->get_ok( + '/merge_q_params_patrons' => json => { + q => { + '-or' => [ + { patron_card_lost => 1 }, + { incorrect_address => 1 }, + ] + } + } + )->status_is(200)->json_is( + '' => { + '-or' => [ + { lost => 1 }, + { gonenoaddress => 1 }, + ] + } + ); + + # Test 4: boolean inside nested -and within -or + $t->get_ok( + '/merge_q_params_patrons' => json => { + q => { + '-or' => [ + { '-and' => [ { patron_card_lost => 1, library_id => 'MPL' } ] }, + { '-and' => [ { incorrect_address => 1, library_id => 'CPL' } ] }, + ] + } + } + )->status_is(200)->json_is( + '' => { + '-or' => [ + { '-and' => [ { lost => 1, branchcode => 'MPL' } ] }, + { '-and' => [ { gonenoaddress => 1, branchcode => 'CPL' } ] }, + ] + } + ); +}; + subtest '_build_query_params_from_api' => sub { plan tests => 16; diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t index d643575da7..3ebcdbc29f 100755 --- a/t/db_dependent/Koha/REST/Plugin/Objects.t +++ b/t/db_dependent/Koha/REST/Plugin/Objects.t @@ -18,6 +18,7 @@ use Modern::Perl; use Koha::Acquisition::Orders; +use Koha::ActionLogs; use Koha::AuthorisedValueCategories; use Koha::AuthorisedValues; use Koha::Cities; @@ -145,7 +146,7 @@ get '/cities/:city_id/rs' => sub { # The tests use Test::NoWarnings; -use Test::More tests => 20; +use Test::More tests => 21; use Test::Mojo; use t::lib::Mocks; @@ -1088,6 +1089,149 @@ subtest 'date handling' => sub { $schema->storage->txn_rollback; }; +subtest 'date handling inside DBIC operators' => sub { + plan tests => 28; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); + + my $patron = + $builder->build_object( { class => 'Koha::Patrons', value => { flags => 1 } } ); + my $password = 'thePassword123'; + $patron->set_password( { password => $password, skip_validation => 1 } ); + my $userid = $patron->userid; + t::lib::Mocks::mock_userenv( { patron => $patron } ); + + # Clear action_logs to avoid interference + Koha::ActionLogs->delete; + + my $now = dt_from_string(); + my $one_hour_ago = $now->clone->subtract( hours => 1 ); + my $two_hours_ago = $now->clone->subtract( hours => 2 ); + my $one_day_ago = $now->clone->subtract( days => 1 ); + + my $log_recent = $builder->build_object( + { + class => 'Koha::ActionLogs', + value => { + user => $patron->borrowernumber, + module => 'MEMBERS', + action => 'MODIFY', + object => $patron->borrowernumber, + timestamp => $one_hour_ago, + } + } + ); + + my $log_older = $builder->build_object( + { + class => 'Koha::ActionLogs', + value => { + user => $patron->borrowernumber, + module => 'MEMBERS', + action => 'ADD', + object => $patron->borrowernumber, + timestamp => $one_day_ago, + } + } + ); + + my $t = Test::Mojo->new('Koha::REST::V1'); + + # Test 1: datetime with >= inside -and + my $q = encode_json( { '-and' => [ { timestamp => { '>=' => $two_hours_ago->rfc3339 }, module => 'MEMBERS' } ] } ); + $t->get_ok( "//$userid:$password@/api/v1/action_logs?q=" . $q ) + ->status_is(200) + ->json_is( '/0/action_id' => $log_recent->action_id ); + is( scalar @{ $t->tx->res->json }, 1, '-and with >= filters correctly' ); + + # Test 2: datetime with <= inside -and + $q = encode_json( { '-and' => [ { timestamp => { '<=' => $two_hours_ago->rfc3339 }, module => 'MEMBERS' } ] } ); + $t->get_ok( "//$userid:$password@/api/v1/action_logs?q=" . $q ) + ->status_is(200) + ->json_is( '/0/action_id' => $log_older->action_id ); + is( scalar @{ $t->tx->res->json }, 1, '-and with <= filters correctly' ); + + # Test 3: datetime range (>= and <=) inside -and + $q = encode_json( + { + '-and' => [ + { + timestamp => { '>=' => $two_hours_ago->rfc3339, '<=' => $now->rfc3339 }, + module => 'MEMBERS' + } + ] + } + ); + $t->get_ok( "//$userid:$password@/api/v1/action_logs?q=" . $q ) + ->status_is(200) + ->json_is( '/0/action_id' => $log_recent->action_id ); + is( scalar @{ $t->tx->res->json }, 1, '-and with >= and <= range filters correctly' ); + + # Test 4: datetime inside -or + $q = encode_json( + { + '-or' => [ + { timestamp => { '>=' => $two_hours_ago->rfc3339 } }, + { timestamp => { '<=' => $one_day_ago->clone->subtract( hours => 1 )->rfc3339 } }, + ], + module => 'MEMBERS' + } + ); + $t->get_ok( "//$userid:$password@/api/v1/action_logs?q=" . $q ) + ->status_is(200) + ->json_is( '/0/action_id' => $log_recent->action_id ); + is( scalar @{ $t->tx->res->json }, 1, '-or with datetime filters correctly' ); + + # Test 5: datetime with -between inside -and + $q = encode_json( + { + '-and' => [ + { + timestamp => { '-between' => [ $two_hours_ago->rfc3339, $now->rfc3339 ] }, + module => 'MEMBERS' + } + ] + } + ); + $t->get_ok( "//$userid:$password@/api/v1/action_logs?q=" . $q ) + ->status_is(200) + ->json_is( '/0/action_id' => $log_recent->action_id ); + is( scalar @{ $t->tx->res->json }, 1, '-and with -between filters correctly' ); + + # Test 6: nested -and inside -or with datetime + $q = encode_json( + { + '-or' => [ + { '-and' => [ { timestamp => { '>=' => $two_hours_ago->rfc3339 }, module => 'MEMBERS' } ] }, + { + '-and' => [ + { + timestamp => { '<=' => $one_day_ago->clone->subtract( hours => 1 )->rfc3339 }, + module => 'MEMBERS' + } + ] + }, + ] + } + ); + $t->get_ok( "//$userid:$password@/api/v1/action_logs?q=" . $q ) + ->status_is(200) + ->json_is( '/0/action_id' => $log_recent->action_id ); + is( scalar @{ $t->tx->res->json }, 1, 'nested -and inside -or with datetime filters correctly' ); + + # Test 7: datetime with me. prefix inside -and + $q = + encode_json( { '-and' => [ { 'me.timestamp' => { '>=' => $two_hours_ago->rfc3339 }, module => 'MEMBERS' } ] } ); + $t->get_ok( "//$userid:$password@/api/v1/action_logs?q=" . $q ) + ->status_is(200) + ->json_is( '/0/action_id' => $log_recent->action_id ); + is( scalar @{ $t->tx->res->json }, 1, '-and with me. prefixed datetime filters correctly' ); + + $schema->storage->txn_rollback; +}; + subtest 'Regression test - search_rs should not reinstate removed query parameters' => sub { plan tests => 3;