Reports run should be logged in action logs.
Created attachment 115162 [details] [review] Bug 27432: Add report run to action logs Reports run should be logged in action logs. This uses the exisiting action "Run" from cronjobs. Like for other actions, infos contains the report name ("Unknown report" is no report id provided) and executed SQL query. 1) Apply patch 2) Enable system preference "ReportsLog" 3) Run a report 4) Go to Tools > Log viewer 5) Select module "Reports" and submit 6) Check you see a log with report name and SQL query 7) Change SQL of report, re-run 8) Go to Tools > Log viewer 9) Select module "Reports" and submit 10) Select the 2 logs "Compare" and click "View comparison"
If accepted I'll create a patch to edit ReportsLog syspref description
Created attachment 115184 [details] [review] Bug 27432: Add report run to action logs Reports run should be logged in action logs. This uses the exisiting action "Run" from cronjobs. Like for other actions, infos contains the report name ("Unknown report" is no report id provided) and executed SQL query. 1) Apply patch 2) Enable system preference "ReportsLog" 3) Run a report 4) Go to Tools > Log viewer 5) Select module "Reports" and submit 6) Check you see a log with report name and SQL query 7) Change SQL of report, re-run 8) Go to Tools > Log viewer 9) Select module "Reports" and submit 10) Select the 2 logs "Compare" and click "View comparison" Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Hi Frido, we have a fail in the QA tools here: FAIL C4/Reports/Guided.pm FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). Also, can you please update the system preference description?
Created attachment 115926 [details] [review] Bug 27432: (QA follow-up) fix if report_id
Created attachment 115927 [details] [review] Bug 27432: Update ReportsLog system preference description
(In reply to Katrin Fischer from comment #4) > Hi Frido, > > we have a fail in the QA tools here: > > FAIL C4/Reports/Guided.pm > FAIL critic > # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). Done. I will be more careful in the future ;) > > Also, can you please update the system preference description? Done
Created attachment 117355 [details] [review] Bug 27432: (follow-up) Correct grammar of preference description
Created attachment 117356 [details] [review] Bug 27432: Add report run to action logs Reports run should be logged in action logs. This uses the exisiting action "Run" from cronjobs. Like for other actions, infos contains the report name ("Unknown report" is no report id provided) and executed SQL query. 1) Apply patch 2) Enable system preference "ReportsLog" 3) Run a report 4) Go to Tools > Log viewer 5) Select module "Reports" and submit 6) Check you see a log with report name and SQL query 7) Change SQL of report, re-run 8) Go to Tools > Log viewer 9) Select module "Reports" and submit 10) Select the 2 logs "Compare" and click "View comparison" Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Created attachment 117357 [details] [review] Bug 27432: (QA follow-up) fix if report_id
Created attachment 117358 [details] [review] Bug 27432: Update ReportsLog system preference description
I feel something is out of order here - patches don't apply in the current sequence. Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 27432: (follow-up) Correct grammar of preference description error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref). error: could not build fake ancestor Patch failed at 0001 Bug 27432: (follow-up) Correct grammar of preference description The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-27432-follow-up-Correct-grammar-of-preference--qT_FEt.patch
(In reply to Katrin Fischer from comment #12) > I feel something is out of order here - patches don't apply in the current > sequence. > > Apply? [(y)es, (n)o, (i)nteractive] y > Applying: Bug 27432: (follow-up) Correct grammar of preference description > error: sha1 information is lacking or useless > (koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref). > error: could not build fake ancestor > Patch failed at 0001 Bug 27432: (follow-up) Correct grammar of preference > description > The copy of the patch that failed is found in: .git/rebase-apply/patch > When you have resolved this problem run "git bz apply --continue". > If you would prefer to skip this patch, instead run "git bz apply --skip". > To restore the original branch and stop patching run "git bz apply --abort". > Patch left in > /tmp/Bug-27432-follow-up-Correct-grammar-of-preference--qT_FEt.patch Ah indeed, patch : Bug 27432: (follow-up) Correct grammar of preference description Is from 17:45 and other patches are from 17:54. I think this patch must be moved after main patch : Bug 27432: Add report run to action logs
I've tried to apply patches 2 3 4 as attached, but I still have a conflict that needs to be solved by the dev: Apply? [(y)es, (n)o, (i)nteractive] i Applying: Bug 27432: Add report run to action logs Applying: Bug 27432: (QA follow-up) fix if report_id Applying: Bug 27432: Update ReportsLog system preference description error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref). error: could not build fake ancestor Patch failed at 0001 Bug 27432: Update ReportsLog system preference description The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-27432-Update-ReportsLog-system-preference-desc-Ygc9OB.patch
Mhh strange rebase does not work. I simply fix my commit : Bug 27432: Update ReportsLog system preference description With patch (manually set to obsolete) : Bug 27432: (follow-up) Correct grammar of preference description
Created attachment 118953 [details] [review] Bug 27432: Add report run to action logs Reports run should be logged in action logs. This uses the exisiting action "Run" from cronjobs. Like for other actions, infos contains the report name ("Unknown report" is no report id provided) and executed SQL query. 1) Apply patch 2) Enable system preference "ReportsLog" 3) Run a report 4) Go to Tools > Log viewer 5) Select module "Reports" and submit 6) Check you see a log with report name and SQL query 7) Change SQL of report, re-run 8) Go to Tools > Log viewer 9) Select module "Reports" and submit 10) Select the 2 logs "Compare" and click "View comparison" Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Created attachment 118954 [details] [review] Bug 27432: (QA follow-up) fix if report_id
Created attachment 118955 [details] [review] Bug 27432: Update ReportsLog system preference description
Created attachment 119817 [details] [review] Bug 27432: Add report run to action logs Reports run should be logged in action logs. This uses the exisiting action "Run" from cronjobs. Like for other actions, infos contains the report name ("Unknown report" is no report id provided) and executed SQL query. 1) Apply patch 2) Enable system preference "ReportsLog" 3) Run a report 4) Go to Tools > Log viewer 5) Select module "Reports" and submit 6) Check you see a log with report name and SQL query 7) Change SQL of report, re-run 8) Go to Tools > Log viewer 9) Select module "Reports" and submit 10) Select the 2 logs "Compare" and click "View comparison" Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 119818 [details] [review] Bug 27432: (QA follow-up) fix if report_id Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 119819 [details] [review] Bug 27432: Update ReportsLog system preference description Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Also checked that these can be found with the log viewer tool.
Missing test coverage.
I would love to see this get through. What do we need to do to push this along?
We need a unit test. Maybe in t/db_dependent/Log.t. Actually there are no test for other reports actions right ?
(In reply to Fridolin Somers from comment #25) > We need a unit test. > Maybe in t/db_dependent/Log.t. > > Actually there are no test for other reports actions right ? That's what I was wondering too.
Could we get some help here with the unit tests? Would make a real nice addition.
There is no difficulty in writing the require tests. Let me know if you need guidance however.
Note that we should not fetch the report if ReportsLog is off.
(In reply to Jonathan Druart from comment #29) > Note that we should not fetch the report if ReportsLog is off. Correct. I see in execute_query() : my ( $is_sql_valid, $errors ) = Koha::Report->new({ savedsql => $sql })->is_sql_valid; return (undef, @{$errors}[0]) unless $is_sql_valid; Should we replace with : my $report = Koha::Reports->find($report_id) if $report_id; ... $report->is_sql_valid Plus : ====== I see an issue with this feature. Some public reports maybe run very often via JSON, like webservices. Maybe we should log only runs via the staff interface ?
Seeing when the report was run from the staff interface would solve one issue for a partner library. Would the running of cronned reports that email out the results also be viewable since they use runreport.pl?