Reports show an inaccurate last run date for reports used by CoverFlow. Shouldn't it show that activity, or would coverflow report timestamps bog the system down?
I think I know why. Last run is only updated with execute_query() is called with report ID. This id is sometimes missing.
Created attachment 115165 [details] [review] Bug 26669: Avoid a warn in update_sql In C4/Reports/Guided.pm update_sql() called by test suite return warn : Use of uninitialized value $sql in substitution (s///) Test plan : Run prove t/db_dependent/Reports/Guided.t and see warning disapearing (whouchhhh)
Created attachment 115166 [details] [review] Bug 26669: Last Run if report not always updated The last run of a report is updated only if method execute_query() is called with report_id. This whas missing for : - when report is run publicly - when report is sent by email - when report is exported Patch changes the method signature to use a hash of params, in order to easily avoid some params. Test plan is coming...
Here is a fix. But it needs a big test plan. Christopher, do you think you can create the test plan for where patch has an impact ? I'm actually short on time.
(In reply to Fridolin Somers from comment #4) > Here is a fix. > But it needs a big test plan. > > Christopher, do you think you can create the test plan for where patch has > an impact ? > I'm actually short on time. Not sure how well I can. I was only able to observe that it wasn't updating for our coverflow reports. I can't even test the ability to send report in e-mail, as I don't have that functionality on a test server. And I am not sure which method of e-mailing you are referring to. Are you talking about using the scheduling feature, or e-mailing from the backend like we do through ByWater via a cron job (I think)? If I had to guess, I would say: 1) Create a report. 2) Run report. 3) Check the report listing. Confirm that the last run info on the report is updated. 4) Make report public. 5) Run report via public url. 6) Check the report listing. Confirm that the last run info on the report IS NOT updated. 7) Schedule the report to run at a given time and e-mailed to an address. 8) After the report runs at the scheduled time, check the report listing. Confirm that the last run info on the report IS NOT updated. 9) Run report. 10) Export results. 11) Check the report listing. Confirm that the last run info on the report IS NOT updated AT THE TIME OF THE EXPORT. Questionable (I don't know if this is addressed): 12) Run report on backend through a cron job and send results via e-mail. 13) Check the report listing. Confirm that the last run info on the report IS NOT updated. 14) Apply patch. 15) Rerun steps 2-13. Confirm that steps 3, 6, 8, 11, and 13 DO UPDATE the last run info.
Looks good to me, thanks
Created attachment 115414 [details] [review] Bug 26669: Last Run if report not always updated The last run of a report is updated only if method execute_query() is called with report_id. This whas missing for : - when report is run publicly - when report is sent by email - when report is exported Patch changes the method signature to use a hash of params, in order to easily avoid some params. Test plan : 1) Create a report. 2) Run report. 3) Check the report listing. Confirm that the last run info on the report is updated. 4) Make report public. 5) Run report via public url. 6) Check the report listing. Confirm that the last run info on the report IS NOT updated. 7) Schedule the report to run at a given time and e-mailed to an address. 8) After the report runs at the scheduled time, check the report listing. Confirm that the last run info on the report IS NOT updated. 9) Run report. 10) Export results. 11) Check the report listing. Confirm that the last run info on the report IS NOT updated AT THE TIME OF THE EXPORT. Questionable (I don't know if this is addressed): 12) Run report on backend through a cron job and send results via e-mail. 13) Check the report listing. Confirm that the last run info on the report IS NOT updated. 14) Apply patch. 15) Rerun steps 2-13. Confirm that steps 3, 6, 8, 11, and 13 DO UPDATE the last run info.
Created attachment 131539 [details] [review] Bug 26669: Last Run if report not always updated The last run of a report is updated only if method execute_query() is called with report_id. This whas missing for : - when report is run publicly - when report is sent by email - when report is exported Patch changes the method signature to use a hash of params, in order to easily avoid some params. Test plan : 1) Create a report. 2) Run report. 3) Check the report listing. Confirm that the last run info on the report is updated. 4) Make report public. 5) Run report via public url. 6) Check the report listing. Confirm that the last run info on the report IS NOT updated. 7) Schedule the report to run at a given time and e-mailed to an address. 8) After the report runs at the scheduled time, check the report listing. Confirm that the last run info on the report IS NOT updated. 9) Run report. 10) Export results. 11) Check the report listing. Confirm that the last run info on the report IS NOT updated AT THE TIME OF THE EXPORT. Questionable (I don't know if this is addressed): 12) Run report on backend through a cron job and send results via e-mail. 13) Check the report listing. Confirm that the last run info on the report IS NOT updated. 14) Apply patch. 15) Rerun steps 2-13. Confirm that steps 3, 6, 8, 11, and 13 DO UPDATE the last run info.
Created attachment 132547 [details] [review] Bug 26669: Last Run if report not always updated The last run of a report is updated only if method execute_query() is called with report_id. This whas missing for : - when report is run publicly - when report is sent by email - when report is exported Patch changes the method signature to use a hash of params, in order to easily avoid some params. Test plan : 1) Create a report. 2) Run report. 3) Check the report listing. Confirm that the last run info on the report is updated. 4) Make report public. 5) Run report via public url. 6) Check the report listing. Confirm that the last run info on the report IS NOT updated. 7) Schedule the report to run at a given time and e-mailed to an address. 8) After the report runs at the scheduled time, check the report listing. Confirm that the last run info on the report IS NOT updated. 9) Run report. 10) Export results. 11) Check the report listing. Confirm that the last run info on the report IS NOT updated AT THE TIME OF THE EXPORT. Questionable (I don't know if this is addressed): 12) Run report on backend through a cron job and send results via e-mail. 13) Check the report listing. Confirm that the last run info on the report IS NOT updated. 14) Apply patch. 15) Rerun steps 2-13. Confirm that steps 3, 6, 8, 11, and 13 DO UPDATE the last run info. Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Created attachment 132548 [details] [review] Bug 26669: Avoid a warn in update_sql In C4/Reports/Guided.pm update_sql() called by test suite return warn : Use of uninitialized value $sql in substitution (s///) Test plan : Run prove t/db_dependent/Reports/Guided.t and see warning disapearing (whouchhhh) Signed-off-by: Séverine Queune <severine.queune@bulac.fr> Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Created attachment 132549 [details] [review] Bug 26669: Last Run if report not always updated The last run of a report is updated only if method execute_query() is called with report_id. This whas missing for : - when report is run publicly - when report is sent by email - when report is exported Patch changes the method signature to use a hash of params, in order to easily avoid some params. Test plan : 1) Create a report. 2) Run report. 3) Check the report listing. Confirm that the last run info on the report is updated. 4) Make report public. 5) Run report via public url. 6) Check the report listing. Confirm that the last run info on the report IS NOT updated. 7) Schedule the report to run at a given time and e-mailed to an address. 8) After the report runs at the scheduled time, check the report listing. Confirm that the last run info on the report IS NOT updated. 9) Run report. 10) Export results. 11) Check the report listing. Confirm that the last run info on the report IS NOT updated AT THE TIME OF THE EXPORT. Questionable (I don't know if this is addressed): 12) Run report on backend through a cron job and send results via e-mail. 13) Check the report listing. Confirm that the last run info on the report IS NOT updated. 14) Apply patch. 15) Rerun steps 2-13. Confirm that steps 3, 6, 8, 11, and 13 DO UPDATE the last run info. Signed-off-by: Séverine Queune <severine.queune@bulac.fr> Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Seemed ok for me : timestamp in Last run column was updated after all the 3 mentionned actions. Thanks Frido !
Created attachment 132587 [details] [review] Bug 26669: Avoid a warn in update_sql In C4/Reports/Guided.pm update_sql() called by test suite return warn : Use of uninitialized value $sql in substitution (s///) Test plan : Run prove t/db_dependent/Reports/Guided.t and see warning disapearing (whouchhhh) Signed-off-by: Séverine Queune <severine.queune@bulac.fr> Signed-off-by: Séverine Queune <severine.queune@bulac.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 132588 [details] [review] Bug 26669: Last Run if report not always updated The last run of a report is updated only if method execute_query() is called with report_id. This whas missing for : - when report is run publicly - when report is sent by email - when report is exported Patch changes the method signature to use a hash of params, in order to easily avoid some params. Test plan : 1) Create a report. 2) Run report. 3) Check the report listing. Confirm that the last run info on the report is updated. 4) Make report public. 5) Run report via public url. 6) Check the report listing. Confirm that the last run info on the report IS NOT updated. 7) Schedule the report to run at a given time and e-mailed to an address. 8) After the report runs at the scheduled time, check the report listing. Confirm that the last run info on the report IS NOT updated. 9) Run report. 10) Export results. 11) Check the report listing. Confirm that the last run info on the report IS NOT updated AT THE TIME OF THE EXPORT. Questionable (I don't know if this is addressed): 12) Run report on backend through a cron job and send results via e-mail. 13) Check the report listing. Confirm that the last run info on the report IS NOT updated. 14) Apply patch. 15) Rerun steps 2-13. Confirm that steps 3, 6, 8, 11, and 13 DO UPDATE the last run info. Signed-off-by: Séverine Queune <severine.queune@bulac.fr> Signed-off-by: Séverine Queune <severine.queune@bulac.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 132589 [details] [review] Bug 26669: Adjust POD Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 132590 [details] [review] Bug 26669: (QA follow-up) Update last run when report run by name This alters the svc scripts to set the report id after fetchign the report object to ensure it is passed to exectue query Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
The only ones that I saw failing before were: Running report by name: http://kohadev.mydnsname.org:8080/cgi-bin/koha/svc/report?name=Test patron_emailer: perl misc/cronjobs/patron_emailer.pl --report 19 --commit --notice BREAD --module catalogue --verbose All works after and is nice to pass params I added a fix for name, fixed up the POD
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
Pushed to 21.11.x for 21.11.05
I note now that the coverflow plugin is broken {"error":"Unhandled exception (Can't use string (\"SELECT DISTINCT biblio.title, bi\"...) as a HASH ref while \"strict refs\" in use at \/usr\/share\/koha\/lib\/C4\/Reports\/Guided.pm line 559.\n)"} Is there a plan to release a fixed coverflow plugin soon?
(In reply to Chris Cormack from comment #20) > I note now that the coverflow plugin is broken > > {"error":"Unhandled exception (Can't use string (\"SELECT DISTINCT > biblio.title, bi\"...) as a HASH ref while \"strict refs\" in use at > \/usr\/share\/koha\/lib\/C4\/Reports\/Guided.pm line 559.\n)"} > > Is there a plan to release a fixed coverflow plugin soon? New release this morning
Thanks Nick