Created attachment 73423 [details] [review] Bug 20495: Remove get_saved_report To test: 1 - prove t/db_dependent/Reports/Guided.t 2 - grep "get_saved_report" - ensure there are no occurences of the singular form 3 - create, save, edit, and convert a report 4 - access a public report and report json from opac and staff client 5 - Ensure all function as expected
Created attachment 74868 [details] [review] Bug 20495: Follow up to refactor tools/scheduler TEST PLAN --------- This doesn't require the other patch applied. -- Make sure you have at least 2 saved reports. If you create them from an empty DB, they are 1 and 2 otherwise, you'll need to know their ID numbers. Home -> Tools -> Task Scheduler -- The Report Dropdown has content! localhost:8081/cgi-bin/koha/tools/scheduler.pl?id=1 -- The Report Dropdown is non-empty and correctly selected -- Attempt with id=2, different report selected. apply this patch restart_all Home -> Tools -> Task Scheduler -- Report dropdown has same content! localhost:8081/cgi-bin/koha/tools/scheduler.pl?id=1 -- The Report Dropdown is non-empty and correctly selected -- Attempt with id=2, different report selected. -- effectively no change. run koha qa test tools
Created attachment 74869 [details] [review] Bug 20495: Remove get_saved_report To test: 1 - prove t/db_dependent/Reports/Guided.t 2 - grep "get_saved_report" - ensure there are no occurences of the singular form 3 - create, save, edit, and convert a report 4 - access a public report and report json from opac and staff client 5 - Ensure all function as expected NOTE: rebased and fixed typo "fund" vs. "find"
Nick, could you confirm that compound reports are not possible? I think one of your changes is valid, but pointless.
(In reply to M. Tompsett from comment #4) > Nick, could you confirm that compound reports are not possible? I think one > of your changes is valid, but pointless. I jumped back to 3.0.x and: git grep "Create Compound Report" only displays in guided reports once! If you can't create compound reports, then you can't save them. I think I'll create a bug to remove those parts.
Created attachment 74870 [details] [review] Bug 20495: Remove get_saved_report To test: 1 - prove t/db_dependent/Reports/Guided.t 2 - grep "get_saved_report" - ensure there are no occurences of the singular form 3 - create, save, edit, and convert a report 4 - access a public report and report json from opac and staff client 5 - Ensure all function as expected Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
+my $report_rec = Koha::Reports->find( $report_name ? { 'name' => $report_name } : { 'id' => $report_id } ); This is wrong: there is no saved_sql.name column, and saved_sql.report_name is not a uniq key (so you should ->search instead of find).
Created attachment 75653 [details] [review] Bug 20495: Remove get_saved_report To test: 1 - prove t/db_dependent/Reports/Guided.t 2 - grep "get_saved_report" - ensure there are no occurences of the singular form 3 - create, save, edit, and convert a report 4 - access a public report and report json from opac and staff client 5 - Ensure all function as expected Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 75654 [details] [review] Bug 20495: (follow-up) Correct search for report by name Ultimately we should probably remove name access as it is not a unique id, but this should preserve existing behaviour To test: Create a report Use the service link to confirm the report runs Replace id=# parameter with name=XXXXXX Confirm URL works
Created attachment 75716 [details] [review] Bug 20495: Remove get_saved_report To test: 1 - prove t/db_dependent/Reports/Guided.t 2 - grep "get_saved_report" - ensure there are no occurences of the singular form 3 - create, save, edit, and convert a report 4 - access a public report and report json from opac and staff client 5 - Ensure all function as expected Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 75717 [details] [review] Bug 20495: (follow-up) Correct search for report by name Ultimately we should probably remove name access as it is not a unique id, but this should preserve existing behaviour To test: Create a report Use the service link to confirm the report runs Replace id=# parameter with name=XXXXXX Confirm URL works Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Note: We should not remove the param "name", it's a feature, see bug 8256.
Awesome work all! Pushed to master for 18.11
Enhancement, not back porting for 18.05.x series.