Bug 23893 - Add ->new_from_api and ->set_from_api methods to Koha::Object
Summary: Add ->new_from_api and ->set_from_api methods to Koha::Object
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Kyle M Hall
URL:
Keywords: release-notes-needed
Depends on: 23843
Blocks: 24225 24228 24321 24432
  Show dependency treegraph
 
Reported: 2019-10-24 17:51 UTC by Tomás Cohen Arazi
Modified: 2020-02-05 21:21 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
This development introduces generic methods to deal with API-to-DB attribute names translations, and some data transformations (dates and booleans). With this design we can overload this methods to handle specific cases without repeating the code as we did on initial implementations of API controllers. Testing becomes easier as well.
Version(s) released in:
20.05.00, 19.11.02


Attachments
Bug 23893: Unit tests (4.42 KB, patch)
2019-11-11 19:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Implement Koha::Object->from_api_mapping (4.24 KB, patch)
2019-11-11 19:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Add new_from_api and set_from_api to Koha::Object (1.96 KB, patch)
2019-11-11 19:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Unit tests (4.42 KB, patch)
2019-11-12 14:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Implement Koha::Object->from_api_mapping (4.24 KB, patch)
2019-11-12 14:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Add new_from_api and set_from_api to Koha::Object (2.84 KB, patch)
2019-11-12 14:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Use in /cities (1.33 KB, patch)
2019-11-12 14:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Use in /patrons (2.50 KB, patch)
2019-11-12 14:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Unit tests (4.48 KB, patch)
2019-12-10 06:33 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 23893: Implement Koha::Object->from_api_mapping (4.30 KB, patch)
2019-12-10 06:33 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 23893: Add new_from_api and set_from_api to Koha::Object (2.90 KB, patch)
2019-12-10 06:33 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 23893: Use in /cities (1.39 KB, patch)
2019-12-10 06:33 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 23893: Use in /patrons (2.56 KB, patch)
2019-12-10 06:33 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 23893: No special care for booleans (1.52 KB, patch)
2019-12-27 14:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23893: Unit tests (4.53 KB, patch)
2019-12-27 15:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Implement Koha::Object->from_api_mapping (4.35 KB, patch)
2019-12-27 15:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Add new_from_api and set_from_api to Koha::Object (3.39 KB, patch)
2019-12-27 15:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Use in /cities (1.44 KB, patch)
2019-12-27 15:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Use in /patrons (2.61 KB, patch)
2019-12-27 15:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: No special care for booleans (1.58 KB, patch)
2019-12-27 15:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: (QA follow-up) Explicit date format (1.38 KB, patch)
2019-12-27 15:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Remove Overloaded method from Koha::Patron (2.27 KB, patch)
2019-12-27 16:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23893: Remove Overloaded method from Koha::Patron (2.27 KB, patch)
2019-12-27 18:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Catch dt_from_string exceptions (3.47 KB, patch)
2019-12-27 18:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23893: Unit tests (4.59 KB, patch)
2020-01-07 18:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Implement Koha::Object->from_api_mapping (4.41 KB, patch)
2020-01-07 18:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Add new_from_api and set_from_api to Koha::Object (3.45 KB, patch)
2020-01-07 18:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Use in /cities (1.50 KB, patch)
2020-01-07 18:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Use in /patrons (2.67 KB, patch)
2020-01-07 18:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: No special care for booleans (1.64 KB, patch)
2020-01-07 18:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: (QA follow-up) Explicit date format (1.44 KB, patch)
2020-01-07 18:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Remove Overloaded method from Koha::Patron (2.33 KB, patch)
2020-01-07 18:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Catch dt_from_string exceptions (3.53 KB, patch)
2020-01-07 18:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Unit tests (4.60 KB, patch)
2020-01-07 18:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Implement Koha::Object->from_api_mapping (4.42 KB, patch)
2020-01-07 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Add new_from_api and set_from_api to Koha::Object (3.46 KB, patch)
2020-01-07 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Use in /cities (1.51 KB, patch)
2020-01-07 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Use in /patrons (2.68 KB, patch)
2020-01-07 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: No special care for booleans (1.65 KB, patch)
2020-01-07 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: (QA follow-up) Explicit date format (1.45 KB, patch)
2020-01-07 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Remove Overloaded method from Koha::Patron (2.34 KB, patch)
2020-01-07 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Catch dt_from_string exceptions (3.55 KB, patch)
2020-01-07 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23893: Special care for booleans (2.92 KB, patch)
2020-01-15 12:55 UTC, Tomás Cohen Arazi
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 2019-10-24 17:51:56 UTC
Now we have ->to_api, we need to add the opposite operation, so:

POST
->
my $object = Koha::Class->new_from_api( $c->validation->param('body') )->store;

PUT/PATCH
->
my $object = Koha::Class->find( $c->validation->param('object_id') );
$object->set_from_api( $c->validation->param('body') )->store;
Comment 1 Tomás Cohen Arazi 2019-11-11 19:15:23 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2019-11-11 19:15:27 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2019-11-11 19:15:30 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2019-11-11 19:31:56 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2019-11-12 14:01:16 UTC
Created attachment 95311 [details] [review]
Bug 23893: Unit tests

This patch introduces tests for the implemented methods.
Comment 6 Tomás Cohen Arazi 2019-11-12 14:01:19 UTC
Created attachment 95312 [details] [review]
Bug 23893: Implement Koha::Object->from_api_mapping

This patch implements the from_api_mapping method, that calculates (and
caches) the reserve mapping from the to_api_mapping method.

A generic to_api_mapping is added to simplify things a bit in the generic
to_api method and so the mappings are reusable in the way they are in
from_api_mapping.

To test:
1. Apply this patches
2. Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 7 Tomás Cohen Arazi 2019-11-12 14:01:23 UTC
Created attachment 95313 [details] [review]
Bug 23893: Add new_from_api and set_from_api to Koha::Object

This patch introduces the following methods to the Koha::Object class:
- set_from_api
- new_from_api

This methods are going to be used when writing API controllers that map
to the attributes to the DB schema ones.

To test:
1. Apply this patchset
2. Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 8 Tomás Cohen Arazi 2019-11-12 14:01:27 UTC
Created attachment 95314 [details] [review]
Bug 23893: Use in /cities

This patch makes the cities controller use the new methods from
Koha::Object.

To test:
1. Apply this patch
2. Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 9 Tomás Cohen Arazi 2019-11-12 14:01:31 UTC
Created attachment 95315 [details] [review]
Bug 23893: Use in /patrons

This patch makes the patrons endpoint use the new methods from
Koha::Object.

To test:
1. Apply this patch
2. Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 10 Josef Moravec 2019-12-10 06:33:12 UTC
Created attachment 96126 [details] [review]
Bug 23893: Unit tests

This patch introduces tests for the implemented methods.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 11 Josef Moravec 2019-12-10 06:33:16 UTC
Created attachment 96127 [details] [review]
Bug 23893: Implement Koha::Object->from_api_mapping

This patch implements the from_api_mapping method, that calculates (and
caches) the reserve mapping from the to_api_mapping method.

A generic to_api_mapping is added to simplify things a bit in the generic
to_api method and so the mappings are reusable in the way they are in
from_api_mapping.

To test:
1. Apply this patches
2. Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 12 Josef Moravec 2019-12-10 06:33:20 UTC
Created attachment 96128 [details] [review]
Bug 23893: Add new_from_api and set_from_api to Koha::Object

This patch introduces the following methods to the Koha::Object class:
- set_from_api
- new_from_api

This methods are going to be used when writing API controllers that map
to the attributes to the DB schema ones.

To test:
1. Apply this patchset
2. Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 13 Josef Moravec 2019-12-10 06:33:24 UTC
Created attachment 96129 [details] [review]
Bug 23893: Use in /cities

This patch makes the cities controller use the new methods from
Koha::Object.

To test:
1. Apply this patch
2. Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 14 Josef Moravec 2019-12-10 06:33:27 UTC
Created attachment 96130 [details] [review]
Bug 23893: Use in /patrons

This patch makes the patrons endpoint use the new methods from
Koha::Object.

To test:
1. Apply this patch
2. Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 15 Jonathan Druart 2019-12-27 14:21:23 UTC
Please move attributes_from_api to a better please (not after AUTOLOAD). Before  internals methods seems better (please amend the patch)
Comment 16 Jonathan Druart 2019-12-27 14:23:23 UTC
Created attachment 96652 [details] [review]
Bug 23893: No special care for booleans

Tests pass without that changes. If we need them they should be at
Koha::Object anyway.

About the date, I also think that that should be moved to Koha::Object
(we can guess it, like we do in Koha::Object->store, retrieve the
datatype and adjust). Also we can send a DT object to DBIC, no need for
an sql formatted date
Comment 17 Jonathan Druart 2019-12-27 14:27:35 UTC
Do we really need the cache mechanism (_from_api_mapping)?
Comment 18 Tomás Cohen Arazi 2019-12-27 14:52:21 UTC
(In reply to Jonathan Druart from comment #17)
> Do we really need the cache mechanism (_from_api_mapping)?

I'm not sure. My way of thinking was if someone needs it, we need to calculate it. And if for some reason the objects are passed around, it might be reused. But I'm not really sure. I would keep it.
Comment 19 Tomás Cohen Arazi 2019-12-27 15:21:58 UTC
Created attachment 96655 [details] [review]
Bug 23893: Unit tests

This patch introduces tests for the implemented methods.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Tomás Cohen Arazi 2019-12-27 15:22:02 UTC
Created attachment 96656 [details] [review]
Bug 23893: Implement Koha::Object->from_api_mapping

This patch implements the from_api_mapping method, that calculates (and
caches) the reserve mapping from the to_api_mapping method.

A generic to_api_mapping is added to simplify things a bit in the generic
to_api method and so the mappings are reusable in the way they are in
from_api_mapping.

To test:
1. Apply this patches
2. Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Tomás Cohen Arazi 2019-12-27 15:22:05 UTC
Created attachment 96657 [details] [review]
Bug 23893: Add new_from_api and set_from_api to Koha::Object

This patch introduces the following methods to the Koha::Object class:
- set_from_api
- new_from_api

This methods are going to be used when writing API controllers that map
to the attributes to the DB schema ones.

To test:
1. Apply this patchset
2. Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 22 Tomás Cohen Arazi 2019-12-27 15:22:09 UTC
Created attachment 96658 [details] [review]
Bug 23893: Use in /cities

This patch makes the cities controller use the new methods from
Koha::Object.

To test:
1. Apply this patch
2. Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 23 Tomás Cohen Arazi 2019-12-27 15:22:13 UTC
Created attachment 96659 [details] [review]
Bug 23893: Use in /patrons

This patch makes the patrons endpoint use the new methods from
Koha::Object.

To test:
1. Apply this patch
2. Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 24 Tomás Cohen Arazi 2019-12-27 15:22:17 UTC
Created attachment 96660 [details] [review]
Bug 23893: No special care for booleans

Tests pass without that changes. If we need them they should be at
Koha::Object anyway.

About the date, I also think that that should be moved to Koha::Object
(we can guess it, like we do in Koha::Object->store, retrieve the
datatype and adjust). Also we can send a DT object to DBIC, no need for
an sql formatted date

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 25 Tomás Cohen Arazi 2019-12-27 15:22:21 UTC
Created attachment 96661 [details] [review]
Bug 23893: (QA follow-up) Explicit date format

This patch makes the input date format explicit. It also adds a missing
POD.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 26 Jonathan Druart 2019-12-27 16:11:49 UTC
Created attachment 96662 [details] [review]
Bug 23893: Remove Overloaded method from Koha::Patron

That way we are dealing with any date fields for any Koha::Object
Comment 27 Tomás Cohen Arazi 2019-12-27 18:29:05 UTC
Created attachment 96665 [details] [review]
Bug 23893: Remove Overloaded method from Koha::Patron

That way we are dealing with any date fields for any Koha::Object
Comment 28 Tomás Cohen Arazi 2019-12-27 18:29:09 UTC
Created attachment 96666 [details] [review]
Bug 23893: Catch dt_from_string exceptions

This patch adds exception handling to the attributes_from_api() method.
This can happen with invalid date/datetimes, for example.

Tests are added:

1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 29 Kyle M Hall 2020-01-07 18:02:45 UTC
Created attachment 96920 [details] [review]
Bug 23893: Unit tests

This patch introduces tests for the implemented methods.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: John Doe <you@example.com>
Comment 30 Kyle M Hall 2020-01-07 18:03:01 UTC
Created attachment 96921 [details] [review]
Bug 23893: Implement Koha::Object->from_api_mapping

This patch implements the from_api_mapping method, that calculates (and
caches) the reserve mapping from the to_api_mapping method.

A generic to_api_mapping is added to simplify things a bit in the generic
to_api method and so the mappings are reusable in the way they are in
from_api_mapping.

To test:
1. Apply this patches
2. Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: John Doe <you@example.com>
Comment 31 Kyle M Hall 2020-01-07 18:03:04 UTC
Created attachment 96922 [details] [review]
Bug 23893: Add new_from_api and set_from_api to Koha::Object

This patch introduces the following methods to the Koha::Object class:
- set_from_api
- new_from_api

This methods are going to be used when writing API controllers that map
to the attributes to the DB schema ones.

To test:
1. Apply this patchset
2. Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: John Doe <you@example.com>
Comment 32 Kyle M Hall 2020-01-07 18:03:07 UTC
Created attachment 96923 [details] [review]
Bug 23893: Use in /cities

This patch makes the cities controller use the new methods from
Koha::Object.

To test:
1. Apply this patch
2. Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: John Doe <you@example.com>
Comment 33 Kyle M Hall 2020-01-07 18:03:10 UTC
Created attachment 96924 [details] [review]
Bug 23893: Use in /patrons

This patch makes the patrons endpoint use the new methods from
Koha::Object.

To test:
1. Apply this patch
2. Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: John Doe <you@example.com>
Comment 34 Kyle M Hall 2020-01-07 18:03:14 UTC
Created attachment 96925 [details] [review]
Bug 23893: No special care for booleans

Tests pass without that changes. If we need them they should be at
Koha::Object anyway.

About the date, I also think that that should be moved to Koha::Object
(we can guess it, like we do in Koha::Object->store, retrieve the
datatype and adjust). Also we can send a DT object to DBIC, no need for
an sql formatted date

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

Signed-off-by: John Doe <you@example.com>
Comment 35 Kyle M Hall 2020-01-07 18:03:18 UTC
Created attachment 96926 [details] [review]
Bug 23893: (QA follow-up) Explicit date format

This patch makes the input date format explicit. It also adds a missing
POD.

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

Signed-off-by: John Doe <you@example.com>
Comment 36 Kyle M Hall 2020-01-07 18:03:21 UTC
Created attachment 96927 [details] [review]
Bug 23893: Remove Overloaded method from Koha::Patron

That way we are dealing with any date fields for any Koha::Object

Signed-off-by: John Doe <you@example.com>
Comment 37 Kyle M Hall 2020-01-07 18:03:24 UTC
Created attachment 96928 [details] [review]
Bug 23893: Catch dt_from_string exceptions

This patch adds exception handling to the attributes_from_api() method.
This can happen with invalid date/datetimes, for example.

Tests are added:

1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

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

Signed-off-by: John Doe <you@example.com>
Comment 38 Kyle M Hall 2020-01-07 18:19:56 UTC
Created attachment 96929 [details] [review]
Bug 23893: Unit tests

This patch introduces tests for the implemented methods.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 39 Kyle M Hall 2020-01-07 18:20:07 UTC
Created attachment 96930 [details] [review]
Bug 23893: Implement Koha::Object->from_api_mapping

This patch implements the from_api_mapping method, that calculates (and
caches) the reserve mapping from the to_api_mapping method.

A generic to_api_mapping is added to simplify things a bit in the generic
to_api method and so the mappings are reusable in the way they are in
from_api_mapping.

To test:
1. Apply this patches
2. Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 40 Kyle M Hall 2020-01-07 18:20:10 UTC
Created attachment 96931 [details] [review]
Bug 23893: Add new_from_api and set_from_api to Koha::Object

This patch introduces the following methods to the Koha::Object class:
- set_from_api
- new_from_api

This methods are going to be used when writing API controllers that map
to the attributes to the DB schema ones.

To test:
1. Apply this patchset
2. Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 41 Kyle M Hall 2020-01-07 18:20:14 UTC
Created attachment 96932 [details] [review]
Bug 23893: Use in /cities

This patch makes the cities controller use the new methods from
Koha::Object.

To test:
1. Apply this patch
2. Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 42 Kyle M Hall 2020-01-07 18:20:17 UTC
Created attachment 96933 [details] [review]
Bug 23893: Use in /patrons

This patch makes the patrons endpoint use the new methods from
Koha::Object.

To test:
1. Apply this patch
2. Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 43 Kyle M Hall 2020-01-07 18:20:21 UTC
Created attachment 96934 [details] [review]
Bug 23893: No special care for booleans

Tests pass without that changes. If we need them they should be at
Koha::Object anyway.

About the date, I also think that that should be moved to Koha::Object
(we can guess it, like we do in Koha::Object->store, retrieve the
datatype and adjust). Also we can send a DT object to DBIC, no need for
an sql formatted date

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 44 Kyle M Hall 2020-01-07 18:20:25 UTC
Created attachment 96935 [details] [review]
Bug 23893: (QA follow-up) Explicit date format

This patch makes the input date format explicit. It also adds a missing
POD.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 45 Kyle M Hall 2020-01-07 18:20:29 UTC
Created attachment 96936 [details] [review]
Bug 23893: Remove Overloaded method from Koha::Patron

That way we are dealing with any date fields for any Koha::Object

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 46 Kyle M Hall 2020-01-07 18:20:32 UTC
Created attachment 96937 [details] [review]
Bug 23893: Catch dt_from_string exceptions

This patch adds exception handling to the attributes_from_api() method.
This can happen with invalid date/datetimes, for example.

Tests are added:

1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 47 Martin Renvoize 2020-01-08 14:43:42 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 48 Tomás Cohen Arazi 2020-01-08 14:57:51 UTC
Thanks, Martin.
Comment 49 Tomás Cohen Arazi 2020-01-15 12:55:37 UTC
Created attachment 97391 [details] [review]
Bug 23893: Special care for booleans

This patch acknowledges the fact that in D8 the Mojo::JSON->true and
Mojo::JSON->false values don't translate into integers when passed to
DBIC. It works correctly on D9 onwards, but we haven't formally
deprecated Jessie. This is adding back this translation, in the right
place now that all mappings code has been integrated into
Koha::Object(s) directly.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 50 Martin Renvoize 2020-01-15 13:36:35 UTC
Followup pushed
Comment 51 Joy Nelson 2020-01-18 16:03:26 UTC
Pushed to 19.11.x branch for 19.11.02
Comment 52 Lucas Gass 2020-02-05 21:21:48 UTC
enhancement will not be backported to 19.05.x series