Bug 28744 - Class with empty/no to_api_mapping should generate an empty from_api_mapping
Summary: Class with empty/no to_api_mapping should generate an empty from_api_mapping
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 25260
  Show dependency treegraph
 
Reported: 2021-07-23 03:34 UTC by Tomás Cohen Arazi
Modified: 2022-06-06 20:24 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.04,20.11.10


Attachments
Bug 28744: Regression tests (1.86 KB, patch)
2021-07-23 03:41 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28744: Better handling of undefined to_api_mapping (1.48 KB, patch)
2021-07-23 03:41 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28744: Regression tests (1.91 KB, patch)
2021-07-23 19:48 UTC, David Nind
Details | Diff | Splinter Review
Bug 28744: Better handling of undefined to_api_mapping (1.52 KB, patch)
2021-07-23 19:48 UTC, David Nind
Details | Diff | Splinter Review
Bug 28744: Regression tests (2.00 KB, patch)
2021-08-27 09:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28744: Better handling of undefined to_api_mapping (1.62 KB, patch)
2021-08-27 09:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28744: Better handling of undefined to_api_mapping (2.38 KB, patch)
2021-08-27 15:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28744: Regression tests (1.89 KB, patch)
2021-08-27 15:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28744: Regression tests (1.89 KB, patch)
2021-08-30 12:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28744: Better handling of undefined to_api_mapping (1.63 KB, patch)
2021-08-30 12:31 UTC, Jonathan Druart
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 2021-07-23 03:34:43 UTC
We always had some mapping because of the DB vs. API object definition discrepancies. But bug 25260 revamps the reserves table, and it is a perfect match. It highlights this edge case: if no mapping defined, then and undef from_api_mapping is generated (as opposed to an empty hashref) and this leads to errors in the query translation from the API.
Comment 1 Tomás Cohen Arazi 2021-07-23 03:41:47 UTC
Created attachment 123069 [details] [review]
Bug 28744: Regression tests

This patch adds tests for the following cases:
- to_api_mapping not defined
- to_api_mapping returning an empty hashref

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> FAIL: Tests fail :-(

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2021-07-23 03:41:51 UTC
Created attachment 123070 [details] [review]
Bug 28744: Better handling of undefined to_api_mapping

We always had some mapping because of the DB vs. API object definition discrepancies. But bug 25260 revamps the reserves table, and it is a perfect match. It highlights this edge case: if no mapping defined, then and undef from_api_mapping is generated (as opposed to an empty hashref) and this leads to errors in the query translation from the API.

This patch makes a small change so this method always returns an empty
hashref.

To test:
1. Apply the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> FAIL: Test fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 David Nind 2021-07-23 19:48:28 UTC
Created attachment 123152 [details] [review]
Bug 28744: Regression tests

This patch adds tests for the following cases:
- to_api_mapping not defined
- to_api_mapping returning an empty hashref

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> FAIL: Tests fail :-(

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2021-07-23 19:48:35 UTC
Created attachment 123153 [details] [review]
Bug 28744: Better handling of undefined to_api_mapping

We always had some mapping because of the DB vs. API object definition discrepancies. But bug 25260 revamps the reserves table, and it is a perfect match. It highlights this edge case: if no mapping defined, then and undef from_api_mapping is generated (as opposed to an empty hashref) and this leads to errors in the query translation from the API.

This patch makes a small change so this method always returns an empty
hashref.

To test:
1. Apply the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> FAIL: Test fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Marcel de Rooy 2021-08-27 09:54:30 UTC
Created attachment 124162 [details] [review]
Bug 28744: Regression tests

This patch adds tests for the following cases:
- to_api_mapping not defined
- to_api_mapping returning an empty hashref

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> FAIL: Tests fail :-(

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2021-08-27 09:54:35 UTC
Created attachment 124163 [details] [review]
Bug 28744: Better handling of undefined to_api_mapping

We always had some mapping because of the DB vs. API object definition discrepancies. But bug 25260 revamps the reserves table, and it is a perfect match. It highlights this edge case: if no mapping defined, then and undef from_api_mapping is generated (as opposed to an empty hashref) and this leads to errors in the query translation from the API.

This patch makes a small change so this method always returns an empty
hashref.

To test:
1. Apply the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> FAIL: Test fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Jonathan Druart 2021-08-27 15:28:12 UTC
Created attachment 124189 [details] [review]
Bug 28744: Better handling of undefined to_api_mapping

We always had some mapping because of the DB vs. API object definition discrepancies. But bug 25260 revamps the reserves table, and it is a perfect match. It highlights this edge case: if no mapping defined, then and undef from_api_mapping is generated (as opposed to an empty hashref) and this leads to errors in the query translation from the API.

This patch makes a small change so this method always returns an empty
hashref.

To test:
1. Apply the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> FAIL: Test fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

JD amended patch: remove uneeded commented lines
Comment 8 Jonathan Druart 2021-08-27 15:29:21 UTC
Created attachment 124190 [details] [review]
Bug 28744: Regression tests

This patch adds tests for the following cases:
- to_api_mapping not defined
- to_api_mapping returning an empty hashref

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> FAIL: Tests fail :-(

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

JD amended patch: remove uneeded commented lines
Comment 9 Jonathan Druart 2021-08-30 12:31:15 UTC
Created attachment 124238 [details] [review]
Bug 28744: Regression tests

This patch adds tests for the following cases:
- to_api_mapping not defined
- to_api_mapping returning an empty hashref

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> FAIL: Tests fail :-(

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

JD amended patch: remove uneeded commented lines
Comment 10 Jonathan Druart 2021-08-30 12:31:20 UTC
Created attachment 124239 [details] [review]
Bug 28744: Better handling of undefined to_api_mapping

We always had some mapping because of the DB vs. API object definition discrepancies. But bug 25260 revamps the reserves table, and it is a perfect match. It highlights this edge case: if no mapping defined, then and undef from_api_mapping is generated (as opposed to an empty hashref) and this leads to errors in the query translation from the API.

This patch makes a small change so this method always returns an empty
hashref.

To test:
1. Apply the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> FAIL: Test fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

JD amended patch: remove uneeded commented lines
Comment 11 Jonathan Druart 2021-08-30 15:12:01 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 12 Kyle M Hall 2021-09-03 14:53:24 UTC
Pushed to 21.05.x for 21.05.04
Comment 13 Fridolin Somers 2021-09-08 20:37:32 UTC
Pushed to 20.11.x for 20.11.10
Comment 14 Victor Grousset/tuxayo 2021-09-20 11:47:09 UTC
Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed.