Bug 37823

Summary: Remove unreachable code in aqcontract.tt
Product: Koha Reporter: Emily Lamancusa (emlam) <emily.lamancusa>
Component: Architecture, internals, and plumbingAssignee: Emily Lamancusa (emlam) <emily.lamancusa>
Status: Needs documenting --- QA Contact: Julian Maurice <julian.maurice>
Severity: minor    
Priority: P5 - low CC: fridolin.somers, julian.maurice, lucas, phil
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37785
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.06
Circulation function:
Bug Depends on:    
Bug Blocks: 37728    
Attachments: Bug 37823: Remove unreachable code in vendor contract page
Bug 37823: Remove unreachable code in vendor contract page
Bug 37823: Remove unreachable code in vendor contract page

Description Emily Lamancusa (emlam) 2024-09-03 14:54:50 UTC
aqcontract.tt has a few sections of code that were meant to display minimal confirmation messages (e.g. "Data recorded") after completing an action. However, this code is never reached, as aqcontract.pl simply redirects the page after completing the requested action.

These unreachable sections of code use empty forms that are POSTed with no op or inputs, simply to display an "OK" button. This now violates coding guidelines, as submitting a form with method="post" and no "op" variable is no longer allowed.

Since this code does nothing, it should simply be removed.
Comment 1 Emily Lamancusa (emlam) 2024-09-03 15:24:16 UTC
Created attachment 170990 [details] [review]
Bug 37823: Remove unreachable code in vendor contract page

The vendor contract management page has a few sections of code that are
meant to display a minimal confirmation message ("Data recorded") after
saving a new or edited vendor contract.

However, aqcontract.pl redirects back to the vendor details page after
saving, so this code is never reached and should be removed.

Test plan:
1. Apply patch
2. Go to the Acquisitions module and find a vendor
3. Click + New > Contract
4. Fill in the information and click Save
5. Click the "Contracts" link in the navigation menu on the left
--> Confirm that the contract was saved correctly
6. Click the Edit button and make changes to all fields
7. Save the information and click "Contracts" again
--> Confirm that the new information saved correctly
Comment 2 Emily Lamancusa (emlam) 2024-09-03 15:39:37 UTC
Testing note: I found an unrelated bug that the input validation isn't working correctly if the start date and/or end date is missing. This bug is present with and without my patch - I filed bug 37825 for it.
Comment 3 Phil Ringnalda 2024-09-04 01:54:27 UTC
Created attachment 171007 [details] [review]
Bug 37823: Remove unreachable code in vendor contract page

The vendor contract management page has a few sections of code that are
meant to display a minimal confirmation message ("Data recorded") after
saving a new or edited vendor contract.

However, aqcontract.pl redirects back to the vendor details page after
saving, so this code is never reached and should be removed.

Test plan:
1. Apply patch
2. Go to the Acquisitions module and find a vendor
3. Click + New > Contract
4. Fill in the information and click Save
5. Click the "Contracts" link in the navigation menu on the left
--> Confirm that the contract was saved correctly
6. Click the Edit button and make changes to all fields
7. Save the information and click "Contracts" again
--> Confirm that the new information saved correctly

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 4 Phil Ringnalda 2024-09-04 05:06:50 UTC
Did you mean to handle the other one in the [% IF ( delete_confirmed ) %] block in this bug, too?
Comment 5 Phil Ringnalda 2024-09-04 21:26:56 UTC
Wups, meant to ask that as a question, not to take away my signoff.
Comment 6 Julian Maurice 2024-09-06 08:14:53 UTC
Created attachment 171128 [details] [review]
Bug 37823: Remove unreachable code in vendor contract page

The vendor contract management page has a few sections of code that are
meant to display a minimal confirmation message ("Data recorded") after
saving a new or edited vendor contract.

However, aqcontract.pl redirects back to the vendor details page after
saving, so this code is never reached and should be removed.

Test plan:
1. Apply patch
2. Go to the Acquisitions module and find a vendor
3. Click + New > Contract
4. Fill in the information and click Save
5. Click the "Contracts" link in the navigation menu on the left
--> Confirm that the contract was saved correctly
6. Click the Edit button and make changes to all fields
7. Save the information and click "Contracts" again
--> Confirm that the new information saved correctly

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 7 Julian Maurice 2024-09-06 08:16:46 UTC
It's always nice to see useless code removed, thanks Emily
Comment 8 Phil Ringnalda 2024-09-12 22:21:06 UTC
Filed bug 37913 for the other one.
Comment 9 Katrin Fischer 2024-09-13 11:58:38 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 10 Lucas Gass (lukeg) 2024-11-18 16:01:38 UTC
Backported to 24.05.x for upcoming 24.05.06
Comment 11 Fridolin Somers 2024-12-02 09:26:50 UTC
I prefer not impact 23.11.x for stability :)