Bug 37823 - Remove unreachable code in aqcontract.tt
Summary: Remove unreachable code in aqcontract.tt
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Emily Lamancusa (emlam)
QA Contact: Julian Maurice
URL:
Keywords:
Depends on:
Blocks: 37728
  Show dependency treegraph
 
Reported: 2024-09-03 14:54 UTC by Emily Lamancusa (emlam)
Modified: 2024-11-18 16:01 UTC (History)
3 users (show)

See Also:
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:


Attachments
Bug 37823: Remove unreachable code in vendor contract page (3.28 KB, patch)
2024-09-03 15:24 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 37823: Remove unreachable code in vendor contract page (3.34 KB, patch)
2024-09-04 01:54 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 37823: Remove unreachable code in vendor contract page (3.40 KB, patch)
2024-09-06 08:14 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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