| Summary: | Refactor C4::Reports.Guided - remove get_saved_report | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
| Component: | Reports | Assignee: | Nick Clemens (kidclamp) <nick> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | jonathan.druart, martin.renvoize, mtompset |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 20052 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 20495: Remove get_saved_report
Bug 20495: Follow up to refactor tools/scheduler Bug 20495: Remove get_saved_report Bug 20495: Remove get_saved_report Bug 20495: Remove get_saved_report Bug 20495: (follow-up) Correct search for report by name Bug 20495: Remove get_saved_report Bug 20495: (follow-up) Correct search for report by name |
||
|
Description
Nick Clemens (kidclamp)
2018-03-29 13:33:19 UTC
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. |