Bug 38329 - Remove orphan confirm_deletion() in supplier.tt
Summary: Remove orphan confirm_deletion() in supplier.tt
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Phil Ringnalda
QA Contact: Lucas Gass (lukeg)
URL:
Keywords:
Depends on: 28082
Blocks:
  Show dependency treegraph
 
Reported: 2024-11-01 19:29 UTC by Phil Ringnalda
Modified: 2025-02-03 16:26 UTC (History)
3 users (show)

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


Attachments
Bug 38329: Remove orphan confirm_deletion() in supplier.tt (1.73 KB, patch)
2024-11-01 19:53 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 38329: Remove orphan confirm_deletion() in supplier.tt (1.78 KB, patch)
2024-11-04 18:28 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 38329: Remove orphan confirm_deletion() in supplier.tt (1.83 KB, patch)
2024-11-05 20:36 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Phil Ringnalda 2024-11-01 19:29:28 UTC
A dozen years ago, we used YUI rather than Bootstrap, and https://git.koha-community.org/Koha-community/Koha/src/commit/f05a8c839f7bd2c3f1da070b81701f0c4dc9d997/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tt#L538 was actually used.

The switch to Bootstrap duplicated the function in koha-tmpl/intranet-tmpl/prog/en/includes/acquisitions-toolbar.inc, so we had two of them with slightly different strings (and I'm not going down the rabbit-hole of which one was actually used), until bug 28082 switched it from a link with href="javascript:confirm_deletion()" to a form a couple of years ago. Luckily, that was on 22.05, before every currently supported version, so it doesn't matter which one was used, because there's only one and it is unused on every current version.

Now, it's just a complete orphan which gives a false-positive for anyone looking for the ?op="delete" which will fail due to CSRF protection.
Comment 1 Phil Ringnalda 2024-11-01 19:53:32 UTC
Created attachment 173890 [details] [review]
Bug 38329: Remove orphan confirm_deletion() in supplier.tt

The JavaScript function confirm_deletion() in supplier.tt has either been an
unused orphan since bug 28082 two years ago, or since the switch from YUI to
Bootstrap 12 years ago. Doesn't matter which, nothing calls it, and if it was
called, it would fail by trying to GET with op="delete" rather than POST with
op="cud-delete".

Test plan:
 1. Acquisitions - New vendor, enter a name and Save
 2. Click the name of your new vendor, so your URL is at supplier.pl
 3. Click Delete vendor to verify that deleting still works without this
    unused code which would fail

Sponsored-by: Chetco Community Public Library
Comment 2 Owen Leonard 2024-11-04 18:28:40 UTC
Created attachment 173948 [details] [review]
Bug 38329: Remove orphan confirm_deletion() in supplier.tt

The JavaScript function confirm_deletion() in supplier.tt has either
been an unused orphan since bug 28082 two years ago, or since the switch
from YUI to Bootstrap 12 years ago. Doesn't matter which, nothing calls
it, and if it was called, it would fail by trying to GET with
op="delete" rather than POST with op="cud-delete".

Test plan:
 1. Acquisitions - New vendor, enter a name and Save
 2. Click the name of your new vendor, so your URL is at supplier.pl
 3. Click Delete vendor to verify that deleting still works without this
    unused code which would fail

Sponsored-by: Chetco Community Public Library
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 3 Lucas Gass (lukeg) 2024-11-05 20:36:50 UTC
Created attachment 174044 [details] [review]
Bug 38329: Remove orphan confirm_deletion() in supplier.tt

The JavaScript function confirm_deletion() in supplier.tt has either
been an unused orphan since bug 28082 two years ago, or since the switch
from YUI to Bootstrap 12 years ago. Doesn't matter which, nothing calls
it, and if it was called, it would fail by trying to GET with
op="delete" rather than POST with op="cud-delete".

Test plan:
 1. Acquisitions - New vendor, enter a name and Save
 2. Click the name of your new vendor, so your URL is at supplier.pl
 3. Click Delete vendor to verify that deleting still works without this
    unused code which would fail

Sponsored-by: Chetco Community Public Library
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 4 Katrin Fischer 2024-11-07 17:51:34 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 5 Lucas Gass (lukeg) 2024-12-05 22:03:54 UTC
BAckported to 24.05.x for upcoming 24.05.06
Comment 6 Fridolin Somers 2024-12-18 12:23:11 UTC
Pushed to 23.11.x for 23.11.11
Comment 7 Jesse Maseto 2025-02-03 16:26:36 UTC
Not pushed to LTS. Marked Resolved.

If you feel this should be in LTS please reply with your reason.