When you request an endpoint in the API with a date as orderby, it throws Koha::Exceptions::BadParameter. Koha::REST::Plugin::Query in _build_order_atom, in order to get the model parameter for the order by, it requests {instance of Koha::Objects}->attributes_from_api({ <api parameter name> => 1 }); to get that hash translated, and use the resulting key as the model column to do the order by. This works ok for all datatypes except date types, because it's trying to transate the value of "1" as a date, throwig the mentioned exception. In my opinion, _build_order_atom should not use attributes_from_api, but should use the mapping in Koha::Object to get the column name.
Created attachment 97421 [details] [review] Bug 24432: Regresion test To test: 1. apply this patch 2. prove t/Koha/REST/Plugin/Query.t Test should fail at this point.
Created attachment 97424 [details] [review] Bug 24432: Regresion test To test: 1. apply this patch 2. prove t/Koha/REST/Plugin/Query.t Test should fail at this point.
Created attachment 97425 [details] [review] Bug 24432: add Koha::Objects->from_api_mapping This patch adds from_api_mapping to Koha::Objects, in order to be able to get the mapping from a result set. To test: 1. apply this patch 2. prove t/db_dependent/Koha/Objects.t 3. sign off
Created attachment 97426 [details] [review] Bug 24432: Use Koha::Objects->from_api_mapping instead of attributes_from_api to get the column name in Koha::REST::Plugins::Query->_build_order_atom Use the mapping instead of building a fake hash only to get the key name. To test: 1. apply this patch 2. prove t/Koha/REST/Plugin/Query.t 3. sign off
Great catch!
Created attachment 97528 [details] [review] Bug 24432: Regresion test To test: 1. apply this patch 2. prove t/Koha/REST/Plugin/Query.t Test should fail at this point. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 97529 [details] [review] Bug 24432: add Koha::Objects->from_api_mapping This patch adds from_api_mapping to Koha::Objects, in order to be able to get the mapping from a result set. To test: 1. apply this patch 2. prove t/db_dependent/Koha/Objects.t 3. sign off Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 97530 [details] [review] Bug 24432: Use from_api_mapping to translate column name in _build_order_atom Use the mapping instead of building a fake hash only to get the key name. To test: 1. apply this patch 2. prove t/Koha/REST/Plugin/Query.t 3. sign off Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 97660 [details] [review] Bug 24432: Regresion test To test: 1. apply this patch 2. prove t/Koha/REST/Plugin/Query.t Test should fail at this point. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 97661 [details] [review] Bug 24432: add Koha::Objects->from_api_mapping This patch adds from_api_mapping to Koha::Objects, in order to be able to get the mapping from a result set. To test: 1. apply this patch 2. prove t/db_dependent/Koha/Objects.t 3. sign off Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 97662 [details] [review] Bug 24432: Use from_api_mapping to translate column name in _build_order_atom Use the mapping instead of building a fake hash only to get the key name. To test: 1. apply this patch 2. prove t/Koha/REST/Plugin/Query.t 3. sign off Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Nice work everyone! Pushed to master for 20.05
does not apply to 19.11.x not backported