Bug 37270 - Deleting a report from the actions menu on a list of saved reports does not work
Summary: Deleting a report from the actions menu on a list of saved reports does not work
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Owen Leonard
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 36192
Blocks:
  Show dependency treegraph
 
Reported: 2024-07-07 22:36 UTC by Phil Ringnalda (back in August)
Modified: 2024-07-17 15:20 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 37270: Fix deletion of saved reports (7.00 KB, patch)
2024-07-09 13:53 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 37270: Fix deletion of saved reports (7.05 KB, patch)
2024-07-09 15:19 UTC, Phil Ringnalda (back in August)
Details | Diff | Splinter Review
Bug 37270: Fix deletion of saved reports (7.12 KB, patch)
2024-07-17 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Phil Ringnalda (back in August) 2024-07-07 22:36:31 UTC
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.
Comment 1 Owen Leonard 2024-07-09 13:53:27 UTC
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
Comment 2 Owen Leonard 2024-07-09 13:54:31 UTC
"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.
Comment 3 Phil Ringnalda (back in August) 2024-07-09 15:05:18 UTC
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 :)
Comment 4 Phil Ringnalda (back in August) 2024-07-09 15:19:50 UTC
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>
Comment 5 Martin Renvoize 2024-07-17 15:19:35 UTC
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>
Comment 6 Martin Renvoize 2024-07-17 15:19:58 UTC
Nice bit of cleanup alongside the fix here.

Passing QA