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: 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: 36192
  Show dependency treegraph
 
Reported: 2024-06-21 21:26 UTC by Phil Ringnalda
Modified: 2024-06-27 09:53 UTC (History)
2 users (show)

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 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

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!