Bug 37162 - Remove dead confirmation code when deleting tags from authority frameworks
Summary: Remove dead confirmation code when deleting tags from authority frameworks
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Phil Ringnalda
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-06-21 23:36 UTC by Phil Ringnalda
Modified: 2024-06-27 09:53 UTC (History)
0 users

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


Attachments
Bug 37162: Remove the [% IF ( delete_confirmed ) %] stuff in auth_tag_structure.tt (3.36 KB, patch)
2024-06-22 23:04 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 37162: Remove the [% IF ( delete_confirmed ) %] stuff in auth_tag_structure.tt (3.40 KB, patch)
2024-06-24 17:10 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 37162: Remove the [% IF ( delete_confirmed ) %] stuff in auth_tag_structure.tt (3.46 KB, patch)
2024-06-25 22:40 UTC, Nick Clemens (kidclamp)
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-06-21 23:36:12 UTC
auth_tag_structure.tt has multiple sections of [% IF ( delete_confirmed ) %] like https://git.koha-community.org/Koha-community/Koha/src/commit/36c4d315c8b4bac116377d7fac4d9e51414c7f80/koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_tag_structure.tt#L212 but in the script, if the $op is cud-delete_confirmed, https://git.koha-community.org/Koha-community/Koha/src/commit/36c4d315c8b4bac116377d7fac4d9e51414c7f80/admin/auth_tag_structure.pl#L160 just redirects and exits, it doesn't ever load the template.

Bug 37161, which made me notice this, might just adopt the redirect too, but if it instead actually makes delete_confirmed work we ought to make auth_tag_structure actually use it, so we have them both doing the same thing and don't just have dead code that pretends they do the same thing.
Comment 1 Phil Ringnalda 2024-06-22 23:04:04 UTC
Created attachment 168005 [details] [review]
Bug 37162: Remove the [% IF ( delete_confirmed ) %] stuff in auth_tag_structure.tt

Since auth_tag_structure.pl redirects after deleting when the op is
delete_confirmed (which is actually cud-delete_confirmed) rather than loading
the template, there's no reason to have dead code for IF ( delete_confirmed )
which is both never set, and is never called when anything like it is set.

Test plan:
1. With the patch applied, Administration - Authority types - Chronological
   Term row - Actions menu - MARC Structure
2. You've already verified that auth_tag_structure.tt isn't malformed, but for
   extra fun choose a tag, Actions menu - Delete - Yes, delete and verify that
   it was deleted
Comment 2 Phil Ringnalda 2024-06-22 23:14:10 UTC
Wups, doesn't block bug 36192, unlike marctagstructure this has been dead code since 2009.
Comment 3 Sam Lau 2024-06-24 17:10:01 UTC
Created attachment 168034 [details] [review]
Bug 37162: Remove the [% IF ( delete_confirmed ) %] stuff in auth_tag_structure.tt

Since auth_tag_structure.pl redirects after deleting when the op is
delete_confirmed (which is actually cud-delete_confirmed) rather than loading
the template, there's no reason to have dead code for IF ( delete_confirmed )
which is both never set, and is never called when anything like it is set.

Test plan:
1. With the patch applied, Administration - Authority types - Chronological
   Term row - Actions menu - MARC Structure
2. You've already verified that auth_tag_structure.tt isn't malformed, but for
   extra fun choose a tag, Actions menu - Delete - Yes, delete and verify that
   it was deleted

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 4 Nick Clemens (kidclamp) 2024-06-25 22:40:36 UTC
Created attachment 168109 [details] [review]
Bug 37162: Remove the [% IF ( delete_confirmed ) %] stuff in auth_tag_structure.tt

Since auth_tag_structure.pl redirects after deleting when the op is
delete_confirmed (which is actually cud-delete_confirmed) rather than loading
the template, there's no reason to have dead code for IF ( delete_confirmed )
which is both never set, and is never called when anything like it is set.

Test plan:
1. With the patch applied, Administration - Authority types - Chronological
   Term row - Actions menu - MARC Structure
2. You've already verified that auth_tag_structure.tt isn't malformed, but for
   extra fun choose a tag, Actions menu - Delete - Yes, delete and verify that
   it was deleted

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Katrin Fischer 2024-06-27 09:53:03 UTC
Pushed for 24.11!

Well done everyone, thank you!