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;
Created attachment 95276 [details] [review] Bug 23893: Unit tests This patch introduces tests for the implemented methods.
Created attachment 95277 [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
Created attachment 95278 [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
I want to hear your opinions, I'm not feeling comfortable with this, looking at cleaning Koha::REST::V1::Patrons with this makes me wonder if it is falling short.
Created attachment 95311 [details] [review] Bug 23893: Unit tests This patch introduces tests for the implemented methods.
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
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
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
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
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>
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>
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>
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>
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>
Please move attributes_from_api to a better please (not after AUTOLOAD). Before internals methods seems better (please amend the patch)
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
Do we really need the cache mechanism (_from_api_mapping)?
(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.
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>
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>
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>
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>
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>
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>
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>
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
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
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Nice work everyone! Pushed to master for 20.05
Thanks, Martin.
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>
Followup pushed
Pushed to 19.11.x branch for 19.11.02
enhancement will not be backported to 19.05.x series