Bug 26669 - Last Run column not updated when report is run publicly (via CoverFlow or elsewhere)
Summary: Last Run column not updated when report is run publicly (via CoverFlow or els...
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on: 14024
Blocks:
  Show dependency treegraph
 
Reported: 2020-10-13 17:12 UTC by Christopher Brannon
Modified: 2022-05-05 21:44 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.05


Attachments
Bug 26669: Avoid a warn in update_sql (1.05 KB, patch)
2021-01-14 15:50 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 26669: Last Run if report not always updated (9.80 KB, patch)
2021-01-14 15:50 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 26669: Last Run if report not always updated (10.72 KB, patch)
2021-01-20 09:11 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 26669: Last Run if report not always updated (10.36 KB, patch)
2022-03-10 06:52 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 26669: Last Run if report not always updated (10.50 KB, patch)
2022-03-30 09:40 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 26669: Avoid a warn in update_sql (1.26 KB, patch)
2022-03-30 09:44 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 26669: Last Run if report not always updated (10.56 KB, patch)
2022-03-30 09:44 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 26669: Avoid a warn in update_sql (1.32 KB, patch)
2022-03-30 12:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26669: Last Run if report not always updated (10.62 KB, patch)
2022-03-30 12:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26669: Adjust POD (4.67 KB, patch)
2022-03-30 12:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26669: (QA follow-up) Update last run when report run by name (1.75 KB, patch)
2022-03-30 12:28 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2020-10-13 17:12:03 UTC
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?
Comment 1 Fridolin Somers 2021-01-14 14:32:17 UTC
I think I know why.
Last run is only updated with execute_query() is called with report ID.
This id is sometimes missing.
Comment 2 Fridolin Somers 2021-01-14 15:50:33 UTC
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)
Comment 3 Fridolin Somers 2021-01-14 15:50:46 UTC
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...
Comment 4 Fridolin Somers 2021-01-14 15:52:48 UTC
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.
Comment 5 Christopher Brannon 2021-01-14 16:08:57 UTC
(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.
Comment 6 Fridolin Somers 2021-01-20 09:10:41 UTC
Looks good to me, thanks
Comment 7 Fridolin Somers 2021-01-20 09:11:39 UTC
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.
Comment 8 Fridolin Somers 2022-03-10 06:52:34 UTC
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.
Comment 9 Séverine Queune 2022-03-30 09:40:37 UTC
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>
Comment 10 Séverine Queune 2022-03-30 09:44:45 UTC
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>
Comment 11 Séverine Queune 2022-03-30 09:44:50 UTC
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>
Comment 12 Séverine Queune 2022-03-30 09:47:20 UTC
Seemed ok for me : timestamp in Last run column was updated after all the 3 mentionned actions.
Thanks Frido !
Comment 13 Nick Clemens 2022-03-30 12:28:45 UTC
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>
Comment 14 Nick Clemens 2022-03-30 12:28:49 UTC
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>
Comment 15 Nick Clemens 2022-03-30 12:28:53 UTC
Created attachment 132589 [details] [review]
Bug 26669: Adjust POD

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Nick Clemens 2022-03-30 12:28:57 UTC
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>
Comment 17 Nick Clemens 2022-03-30 12:32:56 UTC
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
Comment 18 Fridolin Somers 2022-04-05 15:14:59 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 19 Kyle M Hall 2022-04-08 14:48:18 UTC
Pushed to 21.11.x for 21.11.05
Comment 20 Chris Cormack 2022-05-04 21:19:47 UTC
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?
Comment 21 Nick Clemens 2022-05-05 11:55:56 UTC
(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
Comment 22 Chris Cormack 2022-05-05 21:44:02 UTC
Thanks Nick