Bug 43241: Add tests for datetime/boolean fixup inside DBIC operators
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 <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
parent
537a64a01b
commit
811e57c441
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue