Bug 37162

Summary: Remove dead confirmation code when deleting tags from authority frameworks
Product: Koha Reporter: Phil Ringnalda <phil>
Component: TemplatesAssignee: Phil Ringnalda <phil>
Status: Pushed to main --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low    
Version: Main   
Hardware: All   
OS: All   
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
Bug 37162: Remove the [% IF ( delete_confirmed ) %] stuff in auth_tag_structure.tt
Bug 37162: Remove the [% IF ( delete_confirmed ) %] stuff in auth_tag_structure.tt

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!