Bug 28744

Summary: Class with empty/no to_api_mapping should generate an empty from_api_mapping
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: david, fridolin.somers, jonathan.druart, kyle, m.de.rooy, martin.renvoize, nick, victor
Version: master   
Hardware: All   
OS: All   
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
Bug Depends on:    
Bug Blocks: 25260    
Attachments: Bug 28744: Regression tests
Bug 28744: Better handling of undefined to_api_mapping
Bug 28744: Regression tests
Bug 28744: Better handling of undefined to_api_mapping
Bug 28744: Regression tests
Bug 28744: Better handling of undefined to_api_mapping
Bug 28744: Better handling of undefined to_api_mapping
Bug 28744: Regression tests
Bug 28744: Regression tests
Bug 28744: Better handling of undefined to_api_mapping

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.