Bug 37826

Summary: Remove the use of the script_name variable where it is unnecessary
Product: Koha Reporter: Phil Ringnalda <phil>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: Pushed to main --- QA Contact: Katrin Fischer <katrin.fischer>
Severity: enhancement    
Priority: P5 - low CC: david
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement removes the $script_name variable from several pages where it is unnecessary, and updates the corresponding template with the URL itself. (Most of the places where a $script_name variable was used was not strictly necessary. It was also used inconsistently.)
Version(s) released in:
25.05.00
Circulation function:
Attachments: Bug 37826: Remove the use of the script_name variable where it is unnecessary
Bug 37826: Remove the use of the script_name variable where it is unnecessary

Description Phil Ringnalda 2024-09-04 01:46:08 UTC
We have templates that use the variable script_name for the path and filename of the script that called them, particularly as the action attribute of forms that call back to it.

We have scripts that set the template->param script_name to their path and filename.

What we don't have is a 1:1 match between templates and scripts, as the failure mode in bug 37797 comment 1 shows. If you set action="[% script_name %]" when it isn't defined, you get action="", which may work for years if every form submitted to that script is a POST, but once someone changes one to a get, action="" will POST to aqbudgetperiods.pl?op=delete_confirm&budget_period_id=2 and both the things in the POST body and also things in the query string will wind up being sent to the script.

What I don't have is any good idea how to find templates using unset script_name, short of just brute force looking at every one.
Comment 1 Owen Leonard 2024-12-06 19:27:42 UTC
Created attachment 175271 [details] [review]
Bug 37826: Remove the use of the script_name variable where it is unnecessary

Most of the places where a "script_name" variable used it is not
strictly necessary. Because it is also used inconsistently, I think it's
better to simply remove the use of the variable where it isn't strictly
necessary.

This patch removes creation of the template variable from several
scripts and updates the corresponding template with the URL itself.

To test, apply the patch and restart services. Test the following pages,
including the various permutations where they are present, e.g. New,
Edit, Delete.

- Administration -> Cities and towns -> Search via the header search
  form
- Administratoin -> Currencies and exchange rates -> Search via the
  header search form
- Administration -> Desks
- Administration -> Budgets
- Administration -> Authority types
- Administration -> Authority types -> MARC structure -> Subfields
- Administration -> Classification configuration
- Administration -> MARC bibliographic framework
- Administration -> MARC bibliographic framework -> MARC structure ->
  Subfields
- Administration -> Record matching rules
- Administration -> OAI repositories
- Administration -> Patron attribute types
- Administration -> System preferences -> Local use
- Administration -> Z39.50/SRU servers
- Acquisitions -> Vendor -> Contracts
- Acquisitions -> Vendor -> Basket -> Export as CSV
- Acquisitions -> Vendor -> Basket -> Edit basket
- Acquisitions -> Vendor -> Basket groups
- Tools -> Import patrons
- Tools -> Notices and slips

Sponsored-by: Athens County Public Libraries
Comment 2 Phil Ringnalda 2024-12-13 14:36:03 UTC
Created attachment 175448 [details] [review]
Bug 37826: Remove the use of the script_name variable where it is unnecessary

Most of the places where a "script_name" variable used it is not
strictly necessary. Because it is also used inconsistently, I think it's
better to simply remove the use of the variable where it isn't strictly
necessary.

This patch removes creation of the template variable from several
scripts and updates the corresponding template with the URL itself.

To test, apply the patch and restart services. Test the following pages,
including the various permutations where they are present, e.g. New,
Edit, Delete.

- Administration -> Cities and towns -> Search via the header search
  form
- Administratoin -> Currencies and exchange rates -> Search via the
  header search form
- Administration -> Desks
- Administration -> Budgets
- Administration -> Authority types
- Administration -> Authority types -> MARC structure -> Subfields
- Administration -> Classification configuration
- Administration -> MARC bibliographic framework
- Administration -> MARC bibliographic framework -> MARC structure ->
  Subfields
- Administration -> Record matching rules
- Administration -> OAI repositories
- Administration -> Patron attribute types
- Administration -> System preferences -> Local use
- Administration -> Z39.50/SRU servers
- Acquisitions -> Vendor -> Contracts
- Acquisitions -> Vendor -> Basket -> Export as CSV
- Acquisitions -> Vendor -> Basket -> Edit basket
- Acquisitions -> Vendor -> Basket groups
- Tools -> Import patrons
- Tools -> Notices and slips

Sponsored-by: Athens County Public Libraries
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 3 Katrin Fischer 2024-12-27 16:18:53 UTC
QA by RM.
Comment 4 Katrin Fischer 2024-12-27 16:36:20 UTC
Pushed for 25.05!

Well done everyone, thank you!