While testing bug 22323 I noticed an issue with the reports table in combination with stored results. Stealing a part of the test plan: 1) Create a new item with itemnotes 'accentué' 2) Create a report and note the report number ... 5) Run from command line (replace X by report number) : misc/cronjobs/runreport.pl X --format=csv --csv-header --store-results ... 7) Go to saved reports table 8) Look at the table, each run of the cronjob will create a new row in the table instead of just updating the saved results column.
I suspect a problem with the group by somewhere.
This gets out of hand pretty fast.
Created attachment 87598 [details] [review] Bug 22357: Fix saved reports table when runreport.pl with --store-results is used At the moment every time you run runreport.pl with the --store-results option another line will appear for your report in the saved reports table. This is not a data, but a display problem as the report is still only stored once. 1) Create a report and note the report number 2) Run from command line (replace X by report number) : misc/cronjobs/runreport.pl X --format=csv --csv-header --store-results 3) Go to saved reports table 4) Look at the table, each run of the cronjob will create a new row in the table instead of just updating the saved results column. 5) Apply patch 6) Veriy the table displays correctly again and there are no regressions
I think I found a fix and couldn't find any regressions. But I have to admit I am not sure why the line removed from the SQL was there in the first place... please test thoroughly.
This patch breaks keyword searching again - as Josef pointed out the line I removed was added by bug 17535. It looks like we want to search on the stored results with the keyword search, but in that case I am not sure how to fix the display issue.
So am I to understand that we can have many saved results for one report, and that the interface isn't currently supporting that well?
Each 'line' in the saved reports table shows all dates in the saved results column. So the entries for one report are identical and complete... there are just too many. But when we want to search in the saved results, we can't change the SQL used for search... but still don't want to show them duplicated. I am stuck on how to do this.
Created attachment 88992 [details] [review] Bug 22357: Do not duplicate report rows if several reports exist At the moment every time you run runreport.pl with the --store-results option another line will appear for your report in the saved reports table. This is not a data, but a display problem as the report is still only stored once. 1) Create a report and note the report number 2) Run from command line (replace X by report number) : misc/cronjobs/runreport.pl X --format=csv --csv-header --store-results 3) Go to saved reports table 4) Look at the table, each run of the cronjob will create a new row in the table instead of just updating the saved results column. 5) Apply patch 6) Veriy the table displays correctly again and there are no regressions QA: Run t/db_dependent/Reports/Guided.t
This could be a fix (yes, ugly). We could also remove the s.* from the fields selection, but this will need to know which one is used (how, without making sure we will not introduce regressions?) Or we could have 1 query to fetch the reports, then 1 query per report to fetch the results (O(n), bad)
Created attachment 89036 [details] [review] Bug 22357: Do not duplicate report rows if several reports exist At the moment every time you run runreport.pl with the --store-results option another line will appear for your report in the saved reports table. This is not a data, but a display problem as the report is still only stored once. 1) Create a report and note the report number 2) Run from command line (replace X by report number) : misc/cronjobs/runreport.pl X --format=csv --csv-header --store-results 3) Go to saved reports table 4) Look at the table, each run of the cronjob will create a new row in the table instead of just updating the saved results column. 5) Apply patch 6) Veriy the table displays correctly again and there are no regressions QA: Run t/db_dependent/Reports/Guided.t Signed-off-by: Liz Rea <wizzyrea@gmail.com> I didn't assess this for "code ugliness" but the functionality does seem good, and fixes the issue.
Created attachment 89055 [details] [review] Bug 22357: Do not duplicate report rows if several reports exist At the moment every time you run runreport.pl with the --store-results option another line will appear for your report in the saved reports table. This is not a data, but a display problem as the report is still only stored once. 1) Create a report and note the report number 2) Run from command line (replace X by report number) : misc/cronjobs/runreport.pl X --format=csv --csv-header --store-results 3) Go to saved reports table 4) Look at the table, each run of the cronjob will create a new row in the table instead of just updating the saved results column. 5) Apply patch 6) Veriy the table displays correctly again and there are no regressions QA: Run t/db_dependent/Reports/Guided.t Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Works for me, no regressions found (keyword search still works too!). Thx for the fix!
Awesome work all! Pushed to master for 19.05
(In reply to Jonathan Druart from comment #9) > Or we could have 1 query to fetch the reports, then 1 query per report to > fetch the results (O(n), bad) I think this is not such a bad idea, but we would move the listing of saved reports to the specific report page (instead of listing them for all the reports on the main page) If you are saving a report regularly it will be taking a lot of screen eventually
Pushed to 18.11.x for 18.11.06
Depends on bug 17047 only in so much as it adds the mana_id field to the group by clause. RMaint note: I removed that field to get my tests to pass again as mana is not in the 18.11.x series.
missing dependencies, wont backport to 18.05.x