Bug 20495 - Refactor C4::Reports.Guided - remove get_saved_report
Summary: Refactor C4::Reports.Guided - remove get_saved_report
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 20052
Blocks:
  Show dependency treegraph
 
Reported: 2018-03-29 13:33 UTC by Nick Clemens
Modified: 2019-10-14 19:56 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20495: Remove get_saved_report (13.04 KB, patch)
2018-03-29 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20495: Follow up to refactor tools/scheduler (2.14 KB, patch)
2018-04-26 00:02 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 20495: Remove get_saved_report (13.04 KB, patch)
2018-04-26 00:22 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 20495: Remove get_saved_report (13.09 KB, patch)
2018-04-26 03:44 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 20495: Remove get_saved_report (12.34 KB, patch)
2018-05-30 00:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20495: (follow-up) Correct search for report by name (2.46 KB, patch)
2018-05-30 00:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20495: Remove get_saved_report (12.45 KB, patch)
2018-05-31 19:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20495: (follow-up) Correct search for report by name (2.62 KB, patch)
2018-05-31 19:34 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2018-03-29 13:33:19 UTC

    
Comment 1 Nick Clemens 2018-03-29 13:41:52 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
Comment 2 Mark Tompsett 2018-04-26 00:02:59 UTC Comment hidden (obsolete)
Comment 3 Mark Tompsett 2018-04-26 00:22:47 UTC
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"
Comment 4 Mark Tompsett 2018-04-26 02:18:19 UTC
Nick, could you confirm that compound reports are not possible? I think one of your changes is valid, but pointless.
Comment 5 Mark Tompsett 2018-04-26 03:24:02 UTC
(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.
Comment 6 Mark Tompsett 2018-04-26 03:44:39 UTC
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>
Comment 7 Jonathan Druart 2018-05-28 20:06:06 UTC
+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).
Comment 8 Nick Clemens 2018-05-30 00:35:17 UTC
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>
Comment 9 Nick Clemens 2018-05-30 00:35:20 UTC
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
Comment 10 Jonathan Druart 2018-05-31 19:34:41 UTC
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>
Comment 11 Jonathan Druart 2018-05-31 19:34:45 UTC
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.
Comment 12 Nick Clemens 2018-07-02 13:16:48 UTC
Awesome work all!

Pushed to master for 18.11
Comment 13 Martin Renvoize 2018-07-02 17:17:50 UTC
Enhancement, not back porting for 18.05.x series.