You need at least one saved report to see this. Reports - Use saved - for any listed report in the Actions column click the up-arrow next to Run and choose Delete. Confirm that you want to delete, then nothing happens. Our JS thinks that the DOM is <form> </form> <a href="#" class="delete" title="Delete this saved report"><i class="fa fa-trash-can"></i> Delete</a> And attaches a click handler to .delete at https://git.koha-community.org/Koha-community/Koha/src/commit/10853d1edc9c31bcef096ffa514bb3025e0a760e/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt#L2252 which does `return $(this).siblings('form').submit()`. That fails because there's already a form open above that form, and you can't nest forms, so the form we want to submit doesn't exist because the browser has ignored that tag. Now that we have form-submit.js, we should just use it instead.
Created attachment 168653 [details] [review] Bug 37270: Fix deletion of saved reports This patch fixes deletion of saved reports in two instances which were broken by the CSRF changes: From the dropdown menu in the table of saved reports, and in the modal window which appears if you click the "Preview" dropdown menu in the table of saved reports. The patch also makes a minor change to form-submit.js so that the event handler will attach to dynamically-generated elements (as is the case with the preview modal). To test, apply the patch and go to Reports -> Use saved. - Add one or more reports if necessary. - In the table of reports, click the secondary dropdown link in the "Run" button. - Click "Delete." You should be asked to confirm, and confirming should correctly delete the report. - Now test the "Preview SQL" link in the menu. - A modal window should appear showing you the SQL of the report. - In the footer of the modal, test the "Delete" button. Sponsored-by: Athens County Public Libraries
"FAIL koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt FAIL forbidden patterns forbidden pattern: simple-quote string (line 2461)" I think this isn't correct in this situation.
That failure made me look at what that code was trying to do (since it's certainly obscure that the start of that single quote is 15 lines above), which made me discover that SQL preview actually has actions and is thus what I want to do first when I can't remember whether a forgotten report named 'tempfoo' is actually junk, so correct or not it was certainly useful to me :)
Created attachment 168667 [details] [review] Bug 37270: Fix deletion of saved reports This patch fixes deletion of saved reports in two instances which were broken by the CSRF changes: From the dropdown menu in the table of saved reports, and in the modal window which appears if you click the "Preview" dropdown menu in the table of saved reports. The patch also makes a minor change to form-submit.js so that the event handler will attach to dynamically-generated elements (as is the case with the preview modal). To test, apply the patch and go to Reports -> Use saved. - Add one or more reports if necessary. - In the table of reports, click the secondary dropdown link in the "Run" button. - Click "Delete." You should be asked to confirm, and confirming should correctly delete the report. - Now test the "Preview SQL" link in the menu. - A modal window should appear showing you the SQL of the report. - In the footer of the modal, test the "Delete" button. Sponsored-by: Athens County Public Libraries Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Created attachment 169091 [details] [review] Bug 37270: Fix deletion of saved reports This patch fixes deletion of saved reports in two instances which were broken by the CSRF changes: From the dropdown menu in the table of saved reports, and in the modal window which appears if you click the "Preview" dropdown menu in the table of saved reports. The patch also makes a minor change to form-submit.js so that the event handler will attach to dynamically-generated elements (as is the case with the preview modal). To test, apply the patch and go to Reports -> Use saved. - Add one or more reports if necessary. - In the table of reports, click the secondary dropdown link in the "Run" button. - Click "Delete." You should be asked to confirm, and confirming should correctly delete the report. - Now test the "Preview SQL" link in the menu. - A modal window should appear showing you the SQL of the report. - In the footer of the modal, test the "Delete" button. Sponsored-by: Athens County Public Libraries Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice bit of cleanup alongside the fix here. Passing QA
Can you please fix this QA script complaint: FAIL koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt FAIL forbidden patterns forbidden pattern: simple-quote string (line 2461) It's in a big block of JS so switching the quotes is quite confusing.
This actually breaks the test, so waiting for the follow-up before pushing it. # Looks like you failed 1 test of 1. xt/single_quotes.t ............... Dubious, test returned 1 (wstat 256, 0x100) Failed 1/1 subtests xt/tt_valid.t .....
Created attachment 169155 [details] [review] Bug 37270: (follow-up) Fix mixed quotes in preview modal JS
I was not able to get the translation tool to run, so I don't know if this really works in translated templates. If you can, please try it!
Thanks for all the hard work! Pushed to main for the next 24.11.00 release as RM Assistant
*** Bug 38160 has been marked as a duplicate of this bug. ***
Would this be able to be backported?
Backported to 24.05.x for upcoming 24.05.05
Lucas, it appears backporting this patch on 24.05.x caused a regression, breaking the Delete button in the toolbar drop-down menu when running or editing reports. It looks like removal of lines 2252-2258 in guided_reports_start.tt (in the first patch) broke the drop-down delete option. This only affects the 24.05.x branch, not main.
Depends on Bug 36192 not in 23.11.x