|
Description
Phil Ringnalda
2024-06-21 21:26:06 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). 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! 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. Created attachment 168004 [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 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 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> 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> 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> Pushed for 24.11! Well done everyone, thank you! Backported to 24.05.x for upcoming 24.05.02 Full CSRF not in 23.11.x The manual doesn't document deleting tags at all, so it doesn't have to adjust any mention of the "Tag deleted" page. |