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