Because of a wrong mapping, filtering on the `deleted_on` attribute is not possible. To reproduce: 1. Perform this search: GET /api/v1/deleted/biblios { "-and": [ { "me.deleted_on": { ">=": "2024-10-01T08:35:24-05:00" } }, { "me.deleted_on": { "<=": "2024-11-19T08:35:25-07:00" } } ] } => FAIL: You get a 500. The logs say `me.deleted_on` doesn't exist.
Created attachment 175376 [details] [review] Bug 38678: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 175377 [details] [review] Bug 38678: Fix wrong attribute mapping in Koha::Old::Biblio Because the mapping was defined in the wrong direction, it is not possible to filter results in `GET /deleted/biblios` neither on the `deleted_on` attribute, or even the fallback to DB fields when mappings don't exist, on `timestamp`. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/deleted_biblios.t => FAIL: A 200 returns a 500 instead 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass, behavior is correct 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I had a go at testing, but am getting odd results: 1. With the regression patch applied, I get: prove t/db_dependent/api/v1/deleted_biblios.t t/db_dependent/api/v1/deleted_biblios.t .. 1/2 DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Incorrect datetime value: '2024-12-12T17:33:57+00:00' for column `koha_kohadev`.`deletedbiblio`.`timestamp` at row 1 at /kohadevbox/koha/Koha/Object.pm line 172 # Looks like you planned 21 tests but ran 15. # Failed test 'list() tests' # at t/db_dependent/api/v1/deleted_biblios.t line 262. Invalid value passed, deletedbiblio.timestamp=2024-12-12T17:33:57+00:00 expected type is datetime# Looks like your test exited with 2 just after 2. t/db_dependent/api/v1/deleted_biblios.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 1/2 subtests Test Summary Report ------------------- t/db_dependent/api/v1/deleted_biblios.t (Wstat: 512 (exited 2) Tests: 2 Failed: 1) Failed test: 2 Non-zero exit status: 2 Files=1, Tests=2, 5 wallclock secs ( 0.01 usr 0.00 sys + 4.08 cusr 0.42 csys = 4.51 CPU) Result: FAIL 2. With both patches applied, I get: prove t/db_dependent/api/v1/deleted_biblios.t t/db_dependent/api/v1/deleted_biblios.t .. 1/2 DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Incorrect datetime value: '2024-12-12T17:33:57+00:00' for column `koha_kohadev`.`deletedbiblio`.`timestamp` at row 1 at /kohadevbox/koha/Koha/Object.pm line 172 # Looks like you planned 21 tests but ran 15. t/db_dependent/api/v1/deleted_biblios.t .. 2/2 # Failed test 'list() tests' # at t/db_dependent/api/v1/deleted_biblios.t line 262. Invalid value passed, deletedbiblio.timestamp=2024-12-12T17:33:57+00:00 expected type is datetime# Looks like your test exited with 2 just after 2. t/db_dependent/api/v1/deleted_biblios.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 1/2 subtests Test Summary Report ------------------- t/db_dependent/api/v1/deleted_biblios.t (Wstat: 512 (exited 2) Tests: 2 Failed: 1) Failed test: 2 Non-zero exit status: 2 Files=1, Tests=2, 5 wallclock secs ( 0.01 usr 0.00 sys + 4.04 cusr 0.31 csys = 4.36 CPU) Result: FAIL
(In reply to David Nind from comment #3) > > prove t/db_dependent/api/v1/deleted_biblios.t > t/db_dependent/api/v1/deleted_biblios.t .. 1/2 > DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st > execute failed: Incorrect datetime value: '2024-12-12T17:33:57+00:00' for > column `koha_kohadev`.`deletedbiblio`.`timestamp` at row 1 at > /kohadevbox/koha/Koha/Object.pm line 172 Interesting! It doesn't fail for me, but I'm using `DB_IMAGE=mysql:8.0`. Will compare with `mariadb:10.5` and provide a new patchset.
Created attachment 175669 [details] [review] Bug 38678: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 175670 [details] [review] Bug 38678: Fix wrong attribute mapping in Koha::Old::Biblio Because the mapping was defined in the wrong direction, it is not possible to filter results in `GET /deleted/biblios` neither on the `deleted_on` attribute, or even the fallback to DB fields when mappings don't exist, on `timestamp`. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/deleted_biblios.t => FAIL: A 200 returns a 500 instead 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass, behavior is correct 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 175671 [details] [review] Bug 38678: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 175672 [details] [review] Bug 38678: Fix wrong attribute mapping in Koha::Old::Biblio Because the mapping was defined in the wrong direction, it is not possible to filter results in `GET /deleted/biblios` neither on the `deleted_on` attribute, or even the fallback to DB fields when mappings don't exist, on `timestamp`. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/deleted_biblios.t => FAIL: A 200 returns a 500 instead 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass, behavior is correct 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 175988 [details] [review] Bug 38678: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Created attachment 175989 [details] [review] Bug 38678: Fix wrong attribute mapping in Koha::Old::Biblio Because the mapping was defined in the wrong direction, it is not possible to filter results in `GET /deleted/biblios` neither on the `deleted_on` attribute, or even the fallback to DB fields when mappings don't exist, on `timestamp`. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/deleted_biblios.t => FAIL: A 200 returns a 500 instead 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass, behavior is correct 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Pushed for 25.05! Well done everyone, thank you!
Please consider for backport
Nice work everyone! Pushed to 24.11.x for 24.11.02
Nice work everyone! Pushed to 24.05.x for 24.05.08
Depends on Bug 33960 not in 23.11.x
Filtering on the timestamp is really slow. Should it be added to index?