|
Description
Tomás Cohen Arazi (tcohen)
2019-12-09 14:23:56 UTC
Created attachment 96114 [details] [review] Bug 24191: Regression tests This patch adds missing tests for calling objects.search with non-existent column names for sorting, that should be mapped using to_model for that matter. Tests should fail as there's no current use of to_model for building the order_by portion of the query. Sponsored-by: ByWater Solutions Created attachment 96115 [details] [review] Bug 24191: Add to_model param to _build_order_atom and dbic_merge_sorting This patch adds a to_model parameter to dbic_merge_sorting so it is passed when used (for example from objects.search). The to_model param is passed along to the _build_order_atom method where it is finally used. In the process I wrote tests that reflected some problems in the current code: - Mojolicious automatically returns a scalar if a query parameter only happens once on a request. The code expected an arrayref in every case. - There's a design issue that forced me to use some hacky code in _build_order_atom. The first issue is dealth with, by using Scalar::Util::reftype as the Perl docs recommend. The second issue, I don't plan to clean it here, as there's ongoing work on a Koha::Objects->search_from_api method that will obsolete this code most probably (see bug 23893 for a better picture of where the mappings will be living soon). To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass!! 3. Sign off :-D Sponsored-by: ByWater Solutions Created attachment 96116 [details] [review] Bug 24191: Make objects.search pass to_model to dbic_merge_sorting This patch makes objects.search pass the to_mode method reference so it can be used to map API attributes to column names when building the sorting portion of the query. To test: 1. Apply the regression tests on this patchset 2. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => FAIL: Tests fail! 3. Apply the rest of the patchset 4. Run: k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! changes to the methods work as expected! 5. Run: k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 6. Sign off :-D Sponsored-by: ByWater Solutions @RM I set this to major as it is a blocker for serious uses of the API pagination and sorting. Please feel free to set it back to normal! Created attachment 96117 [details] [review] Bug 24191: Regression tests This patch adds missing tests for calling objects.search with non-existent column names for sorting, that should be mapped using to_model for that matter. Tests should fail as there's no current use of to_model for building the order_by portion of the query. Sponsored-by: ByWater Solutions Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 96118 [details] [review] Bug 24191: Add to_model param to _build_order_atom and dbic_merge_sorting This patch adds a to_model parameter to dbic_merge_sorting so it is passed when used (for example from objects.search). The to_model param is passed along to the _build_order_atom method where it is finally used. In the process I wrote tests that reflected some problems in the current code: - Mojolicious automatically returns a scalar if a query parameter only happens once on a request. The code expected an arrayref in every case. - There's a design issue that forced me to use some hacky code in _build_order_atom. The first issue is dealth with, by using Scalar::Util::reftype as the Perl docs recommend. The second issue, I don't plan to clean it here, as there's ongoing work on a Koha::Objects->search_from_api method that will obsolete this code most probably (see bug 23893 for a better picture of where the mappings will be living soon). To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass!! 3. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 96119 [details] [review] Bug 24191: Make objects.search pass to_model to dbic_merge_sorting This patch makes objects.search pass the to_mode method reference so it can be used to map API attributes to column names when building the sorting portion of the query. To test: 1. Apply the regression tests on this patchset 2. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => FAIL: Tests fail! 3. Apply the rest of the patchset 4. Run: k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! changes to the methods work as expected! 5. Run: k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 6. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 96133 [details] [review] Bug 24191: Regression tests This patch adds missing tests for calling objects.search with non-existent column names for sorting, that should be mapped using to_model for that matter. Tests should fail as there's no current use of to_model for building the order_by portion of the query. Sponsored-by: ByWater Solutions Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 96134 [details] [review] Bug 24191: Add to_model param to _build_order_atom and dbic_merge_sorting This patch adds a to_model parameter to dbic_merge_sorting so it is passed when used (for example from objects.search). The to_model param is passed along to the _build_order_atom method where it is finally used. In the process I wrote tests that reflected some problems in the current code: - Mojolicious automatically returns a scalar if a query parameter only happens once on a request. The code expected an arrayref in every case. - There's a design issue that forced me to use some hacky code in _build_order_atom. The first issue is dealth with, by using Scalar::Util::reftype as the Perl docs recommend. The second issue, I don't plan to clean it here, as there's ongoing work on a Koha::Objects->search_from_api method that will obsolete this code most probably (see bug 23893 for a better picture of where the mappings will be living soon). To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass!! 3. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 96135 [details] [review] Bug 24191: Make objects.search pass to_model to dbic_merge_sorting This patch makes objects.search pass the to_mode method reference so it can be used to map API attributes to column names when building the sorting portion of the query. To test: 1. Apply the regression tests on this patchset 2. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => FAIL: Tests fail! 3. Apply the rest of the patchset 4. Run: k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! changes to the methods work as expected! 5. Run: k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 6. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Nice work everyone! Pushed to master for 20.05 Pushed to 19.11.01 backported to 19.05.x for 19.05.07 Backported to 18.11.x for 18.11.13 |