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

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This removes redundant code that is no longer used when deleting authority tags. A previous change removed the extra page displayed after confirming the deletion an authority tag - this required you to click OK, and then you were returned to the list of tags.
Version(s) released in:
24.11.00,24.05.02,23.11.07
Circulation function:

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
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

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!
Comment 6 Lucas Gass (lukeg) 2024-07-18 20:24:21 UTC 
Backported to 24.05.x for upcoming 24.05.02
Comment 7 Fridolin Somers 2024-07-19 09:41:15 UTC 
Pushed to 23.11.x for 23.11.07
Comment 8 Wainui Witika-Park 2024-09-17 03:10:21 UTC 
Not backporting to 23.05.x unless requested
Comment 9 Phil Ringnalda 2024-09-17 03:12:31 UTC 
No real need, and nothing to document here.