Bug 37161 - After deleting a tag in a MARC framework, confirmation page is blank
Summary: After deleting a tag in a MARC framework, confirmation page is blank
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: 36192
  Show dependency tree / graph
 
Reported: 2024-06-21 21:26 UTC by Phil Ringnalda
Modified: 2024-09-10 23:50 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This removes the "Tag deleted" page after deleting a MARC framework tag. After confirming the deletion of a tag, you are now returned to you where you were in the list of tags for the framework (this now matches the behavour when deleting tags for an authority type - see bug 37163). It also fixes the blank page that was displayed after confirming the tag deletion - this was related to the CSRF changes added in Koha 24.05 to improve form security.
Version(s) released in:
24.11.00,24.05.02
Circulation function:

Attachments
Bug 37161: After deleting a tag in a MARC framework, redirect back to the tag list rather than showing a 'Tag deleted' page first (4.67 KB, patch)
2024-06-22 22:49 UTC, Phil Ringnalda 		Details | Diff | Splinter Review
Bug 37161: After deleting a tag in a MARC framework, redirect back to the tag list rather than showing a 'Tag deleted' page first (4.68 KB, patch)
2024-06-23 00:30 UTC, Phil Ringnalda 		Details | Diff | Splinter Review
Bug 37161: After deleting a tag in a MARC framework, redirect back to the tag list rather than showing a 'Tag deleted' page first (4.73 KB, patch)
2024-06-24 16:51 UTC, Sam Lau 		Details | Diff | Splinter Review
Bug 37161: After deleting a tag in a MARC framework, redirect back to the tag list rather than showing a 'Tag deleted' page first (4.78 KB, patch)
2024-06-24 17:01 UTC, Eric Garcia 		Details | Diff | Splinter Review
Bug 37161: After deleting a tag in a MARC framework, redirect back to the tag list rather than showing a 'Tag deleted' page first (4.84 KB, patch)
2024-06-25 22:36 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Show Obsolete (4) 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 21:26:06 UTC 
In older versions (I tested in 23.05.11 since I had it handy), when you edit a MARC framework and delete a tag, you get a page saying "Tag deleted" with an OK button, and clicking the button reloads the framework you were in (though not exactly the way it was originally inteded to, since it's supposed to reload with a search for the tagnumber, so you wind up in the same place you were).

In main, you don't get the "Tag deleted", which is inside an [% IF ( delete_confirmed ) %].

I don't see what magic was ever setting delete_confirmed, unless it was set by that being the name of the $op back before it became cud-delete_confirmed. https://git.koha-community.org/Koha-community/Koha/src/commit/582d6916c9a4e021641030d8779466cb5f7f6273/acqui/basket.pl#L175 explicitly sets it, but I don't see where anything else does, and if the magic was coming from the $op, then there's a whole bunch more things that need to be fixed to set it.
Comment 1 Phil Ringnalda 2024-06-21 23:18:57 UTC 
FWIW, explicitly setting delete_confirmed => 1 in https://git.koha-community.org/Koha-community/Koha/src/commit/b0b168b6f84ef00babf566097933de7a338726a7/admin/marctagstructure.pl#L183 does make the "Tag deleted [OK]" -> redirect thing work again, but magic was nicer.

And since the "Tag deleted [OK]" page is really rather silly, it would probably be simpler to copy the authority technique of a redirect in https://git.koha-community.org/Koha-community/Koha/src/commit/36c4d315c8b4bac116377d7fac4d9e51414c7f80/admin/auth_tag_structure.pl#L160 (though maybe actually correctly setting the tag number with "my $tagfield = $input->param('searchfield');" so you go back to where you were, since that's currently setting it to $input->param('tagfield') which doesn't exist).
Comment 2 Phil Ringnalda 2024-06-22 04:04:37 UTC 
Fixed the authority tagfield thing in bug 37163
Comment 3 Phil Ringnalda 2024-06-22 21:11:15 UTC 
Oh, I see, not magic, just common practice to do something like https://git.koha-community.org/Koha-community/Koha/src/commit/b0b168b6f84ef00babf566097933de7a338726a7/admin/marctagstructure.pl#L79 where you add a template param of $op => 1, which worked fine when it was also common practice to name ops in a way compatible with TT variable names, which are alphanumeric plus underscore, which doesn't include the hyphen in cud-.

So every script with a cud- op needs to have its template checked for use of the former op name, and the script needs to be updated to set the non-cud- version of it if the template uses it? Fun!
Comment 4 Phil Ringnalda 2024-06-22 22:34:34 UTC 
Luckily, it's also common practice to just redirect to showing the result of a cud- op rather than depending on a template knowing what to do, which limits the damages.
Comment 5 Phil Ringnalda 2024-06-22 22:49:43 UTC Comment hidden (obsolete)
Comment 6 Phil Ringnalda 2024-06-23 00:30:18 UTC 
Created attachment 168007 [details] [review]
Bug 37161: After deleting a tag in a MARC framework, redirect back to the tag list rather than showing a 'Tag deleted' page first

Currently after you delete a tag from a MARC framework, Koha intends to show
you a page with the text "Tag deleted" and an OK button to click to go back to
where you were in the list of tags. But because that depends on the template
variable being set for the name of the op in the script, and the name of the
op is "cud-delete_confirmed" which isn't a legal TT variable name, nothing is
set to tell the template what to display, so it displays a mostly-blank page.

Far better to show, don't tell, and just redirect to the list of tags like
deleting a tag from an authority framework does.

Test plan:
1. Without the patch, Administration - MARC bibliographic frameworks -
   Binders row - Actions menu - MARC structure
2. Type 092 and press Enter in the Search for tag: input
3. 092 row - Actions menu - Delete - Yes, delete this tag
4. Stare blankly at the blank page with only a header
5. Apply patch, restart_all
6. Administration - MARC bibliographic frameworks - Kits row - Actions menu -
   MARC structure
7. Type 092 and press Enter in the Search for tag: input
8. 092 row - Actions menu - Delete - Yes, delete this tag
9. Enjoy the sight of the Kits framework showing a search for 092 that
   doesn't show one, because you just deleted it, and now you can delete
   the 096 tag next
Comment 7 Sam Lau 2024-06-24 16:51:53 UTC 
Created attachment 168032 [details] [review]
Bug 37161: After deleting a tag in a MARC framework, redirect back to the tag list rather than showing a 'Tag deleted' page first

Currently after you delete a tag from a MARC framework, Koha intends to show
you a page with the text "Tag deleted" and an OK button to click to go back to
where you were in the list of tags. But because that depends on the template
variable being set for the name of the op in the script, and the name of the
op is "cud-delete_confirmed" which isn't a legal TT variable name, nothing is
set to tell the template what to display, so it displays a mostly-blank page.

Far better to show, don't tell, and just redirect to the list of tags like
deleting a tag from an authority framework does.

Test plan:
1. Without the patch, Administration - MARC bibliographic frameworks -
   Binders row - Actions menu - MARC structure
2. Type 092 and press Enter in the Search for tag: input
3. 092 row - Actions menu - Delete - Yes, delete this tag
4. Stare blankly at the blank page with only a header
5. Apply patch, restart_all
6. Administration - MARC bibliographic frameworks - Kits row - Actions menu -
   MARC structure
7. Type 092 and press Enter in the Search for tag: input
8. 092 row - Actions menu - Delete - Yes, delete this tag
9. Enjoy the sight of the Kits framework showing a search for 092 that
   doesn't show one, because you just deleted it, and now you can delete
   the 096 tag next

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 8 Eric Garcia 2024-06-24 17:01:37 UTC 
Created attachment 168033 [details] [review]
Bug 37161: After deleting a tag in a MARC framework, redirect back to the tag list rather than showing a 'Tag deleted' page first

Currently after you delete a tag from a MARC framework, Koha intends to show
you a page with the text "Tag deleted" and an OK button to click to go back to
where you were in the list of tags. But because that depends on the template
variable being set for the name of the op in the script, and the name of the
op is "cud-delete_confirmed" which isn't a legal TT variable name, nothing is
set to tell the template what to display, so it displays a mostly-blank page.

Far better to show, don't tell, and just redirect to the list of tags like
deleting a tag from an authority framework does.

Test plan:
1. Without the patch, Administration - MARC bibliographic frameworks -
   Binders row - Actions menu - MARC structure
2. Type 092 and press Enter in the Search for tag: input
3. 092 row - Actions menu - Delete - Yes, delete this tag
4. Stare blankly at the blank page with only a header
5. Apply patch, restart_all
6. Administration - MARC bibliographic frameworks - Kits row - Actions menu -
   MARC structure
7. Type 092 and press Enter in the Search for tag: input
8. 092 row - Actions menu - Delete - Yes, delete this tag
9. Enjoy the sight of the Kits framework showing a search for 092 that
   doesn't show one, because you just deleted it, and now you can delete
   the 096 tag next

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Eric Garcia <cubingguy714@gmail.com>
Comment 9 Nick Clemens (kidclamp) 2024-06-25 22:36:35 UTC 
Created attachment 168108 [details] [review]
Bug 37161: After deleting a tag in a MARC framework, redirect back to the tag list rather than showing a 'Tag deleted' page first

Currently after you delete a tag from a MARC framework, Koha intends to show
you a page with the text "Tag deleted" and an OK button to click to go back to
where you were in the list of tags. But because that depends on the template
variable being set for the name of the op in the script, and the name of the
op is "cud-delete_confirmed" which isn't a legal TT variable name, nothing is
set to tell the template what to display, so it displays a mostly-blank page.

Far better to show, don't tell, and just redirect to the list of tags like
deleting a tag from an authority framework does.

Test plan:
1. Without the patch, Administration - MARC bibliographic frameworks -
   Binders row - Actions menu - MARC structure
2. Type 092 and press Enter in the Search for tag: input
3. 092 row - Actions menu - Delete - Yes, delete this tag
4. Stare blankly at the blank page with only a header
5. Apply patch, restart_all
6. Administration - MARC bibliographic frameworks - Kits row - Actions menu -
   MARC structure
7. Type 092 and press Enter in the Search for tag: input
8. 092 row - Actions menu - Delete - Yes, delete this tag
9. Enjoy the sight of the Kits framework showing a search for 092 that
   doesn't show one, because you just deleted it, and now you can delete
   the 096 tag next

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

Well done everyone, thank you!
Comment 11 Lucas Gass (lukeg) 2024-07-18 20:19:37 UTC 
Backported to 24.05.x for upcoming 24.05.02
Comment 12 Fridolin Somers 2024-07-19 09:32:34 UTC 
Full CSRF not in 23.11.x
Comment 13 Phil Ringnalda 2024-09-10 23:50:29 UTC 
The manual doesn't document deleting tags at all, so it doesn't have to adjust any mention of the "Tag deleted" page.