Bug 38678 - GET /deleted/biblios cannot be filtered on `deleted_on`
Summary: GET /deleted/biblios cannot be filtered on `deleted_on`
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Matt Blenkinsop
URL:
Keywords:
Depends on: 33960
Blocks:
  Show dependency treegraph
 
Reported: 2024-12-11 17:08 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-02-07 18:08 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00,24.11.02
Circulation function:


Attachments
Bug 38678: Regression tests (2.12 KB, patch)
2024-12-11 18:10 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38678: Fix wrong attribute mapping in Koha::Old::Biblio (1.24 KB, patch)
2024-12-11 18:10 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38678: Regression tests (2.16 KB, patch)
2024-12-18 19:23 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38678: Fix wrong attribute mapping in Koha::Old::Biblio (1.24 KB, patch)
2024-12-18 19:23 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38678: Regression tests (2.20 KB, patch)
2024-12-18 19:27 UTC, David Nind
Details | Diff | Splinter Review
Bug 38678: Fix wrong attribute mapping in Koha::Old::Biblio (1.28 KB, patch)
2024-12-18 19:27 UTC, David Nind
Details | Diff | Splinter Review
Bug 38678: Regression tests (2.28 KB, patch)
2024-12-30 10:55 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38678: Fix wrong attribute mapping in Koha::Old::Biblio (1.36 KB, patch)
2024-12-30 10:55 UTC, Matt Blenkinsop
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2024-12-11 17:08:53 UTC
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.
Comment 1 Tomás Cohen Arazi (tcohen) 2024-12-11 18:10:50 UTC
Created attachment 175376 [details] [review]
Bug 38678: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi (tcohen) 2024-12-11 18:10:52 UTC
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>
Comment 3 David Nind 2024-12-14 21:07:42 UTC
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
Comment 4 Tomás Cohen Arazi (tcohen) 2024-12-18 19:11:51 UTC
(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.
Comment 5 Tomás Cohen Arazi (tcohen) 2024-12-18 19:23:00 UTC
Created attachment 175669 [details] [review]
Bug 38678: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi (tcohen) 2024-12-18 19:23:02 UTC
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>
Comment 7 David Nind 2024-12-18 19:27:31 UTC
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>
Comment 8 David Nind 2024-12-18 19:27:34 UTC
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>
Comment 9 Matt Blenkinsop 2024-12-30 10:55:21 UTC
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>
Comment 10 Matt Blenkinsop 2024-12-30 10:55:24 UTC
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>
Comment 11 Katrin Fischer 2024-12-30 14:14:59 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 12 Nick Clemens (kidclamp) 2025-01-08 13:18:25 UTC
Please consider for backport
Comment 13 Paul Derscheid 2025-02-04 21:54:39 UTC
Nice work everyone!

Pushed to 24.11.x for 24.11.02