Bug 37913 - Remove more unreachable code in aqcontract.tt
Summary: Remove more unreachable code in aqcontract.tt
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P3 normal
Assignee: Phil Ringnalda
QA Contact: Paul Derscheid
URL:
Keywords:
Depends on:
Blocks: 37728
  Show dependency treegraph
 
Reported: 2024-09-12 22:05 UTC by Phil Ringnalda
Modified: 2024-09-16 08:54 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
Circulation function:


Attachments
Bug 37913: Remove more unreachable code in aqcontract.tt (3.47 KB, patch)
2024-09-12 22:20 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 37913: Remove more unreachable code in aqcontract.tt (3.54 KB, patch)
2024-09-13 12:59 UTC, Sukhmandeep
Details | Diff | Splinter Review
Bug 37913: Remove more unreachable code in aqcontract.tt (3.60 KB, patch)
2024-09-13 15:00 UTC, Paul Derscheid
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-09-12 22:05:07 UTC
Bug 37823 removes one set of unreachable code from aqcontract.tt, a block that tried to tell you that you had created a new contract rather than just showing you the new contract, but there is another:

When you delete a contract, what actually happens is that you are shown the details page for the vendor, which lists the contracts for that vendor at the bottom, thus showing you that the contract has been deleted, but the template has a block which thinks it is going to tell you "<h1>Contract deleted</h1>" and then have you click an OK button to POST a form with no op param starting with cud-, which now fails the CSRF protection. Luckily, it's unreachable (and has been since aqcontract.pl first landed, it has always redirected after deleting, and has never shown that block), but having it there still fails a test once bug 37728 lands and lets the test see it.
Comment 1 Phil Ringnalda 2024-09-12 22:20:18 UTC
Created attachment 171430 [details] [review]
Bug 37913: Remove more unreachable code in aqcontract.tt

When you successfully delete an acquisitions contract, you are redirected to
the page with the vendor details, which shows contracts for that vendor, which
shows you that the contract you deleted was deleted.

The aqcontract.tt template has a block in [% IF ( delete_confirmed ) %] (which
is never set) that thinks it will show you a '<h1>Contract deleted</h1>' and
make you click an OK button. That block has never been shown, but the button
is in a form which POSTs without an 'op' param, which is now forbidden by the
CSRF code, and will be caught by a test once bug 37728 updates the test.

There's nothing visible to test, so the test plan just makes sure nothing
blows up and you can still delete a contract.

1. Nothing will change, so start with the patch applied
2. Acquistions - click the Search button to search for vendors
3. New -> Contract
4. Give it a name, a start date, and an end date, and Save
5. In the list of contracts for My Vendor, click the Delete button for
   the contract you created
6. In the confirmation page asking if you wanted to delete, click 'Yes,
   delete contract'
7. Note that you are at acqui/supplier.pl?booksellerid=1, the details page
   for the vendor, and your contract isn't listed at the bottom of the page,
   and especially that you weren't shown a page that just said "Contract
   deleted."

Sponsored-by: Chetco Community Public Library
Comment 2 Sukhmandeep 2024-09-13 12:59:31 UTC
Created attachment 171456 [details] [review]
Bug 37913: Remove more unreachable code in aqcontract.tt

When you successfully delete an acquisitions contract, you are redirected to
the page with the vendor details, which shows contracts for that vendor, which
shows you that the contract you deleted was deleted.

The aqcontract.tt template has a block in [% IF ( delete_confirmed ) %] (which
is never set) that thinks it will show you a '<h1>Contract deleted</h1>' and
make you click an OK button. That block has never been shown, but the button
is in a form which POSTs without an 'op' param, which is now forbidden by the
CSRF code, and will be caught by a test once bug 37728 updates the test.

There's nothing visible to test, so the test plan just makes sure nothing
blows up and you can still delete a contract.

1. Nothing will change, so start with the patch applied
2. Acquistions - click the Search button to search for vendors
3. New -> Contract
4. Give it a name, a start date, and an end date, and Save
5. In the list of contracts for My Vendor, click the Delete button for
   the contract you created
6. In the confirmation page asking if you wanted to delete, click 'Yes,
   delete contract'
7. Note that you are at acqui/supplier.pl?booksellerid=1, the details page
   for the vendor, and your contract isn't listed at the bottom of the page,
   and especially that you weren't shown a page that just said "Contract
   deleted."

Sponsored-by: Chetco Community Public Library
Signed-off-by: Sukhmandeep Benipal <sukhmandeep.benipal@inLibro.com>
Comment 3 Paul Derscheid 2024-09-13 15:00:21 UTC
Created attachment 171471 [details] [review]
Bug 37913: Remove more unreachable code in aqcontract.tt

When you successfully delete an acquisitions contract, you are redirected to
the page with the vendor details, which shows contracts for that vendor, which
shows you that the contract you deleted was deleted.

The aqcontract.tt template has a block in [% IF ( delete_confirmed ) %] (which
is never set) that thinks it will show you a '<h1>Contract deleted</h1>' and
make you click an OK button. That block has never been shown, but the button
is in a form which POSTs without an 'op' param, which is now forbidden by the
CSRF code, and will be caught by a test once bug 37728 updates the test.

There's nothing visible to test, so the test plan just makes sure nothing
blows up and you can still delete a contract.

1. Nothing will change, so start with the patch applied
2. Acquistions - click the Search button to search for vendors
3. New -> Contract
4. Give it a name, a start date, and an end date, and Save
5. In the list of contracts for My Vendor, click the Delete button for
   the contract you created
6. In the confirmation page asking if you wanted to delete, click 'Yes,
   delete contract'
7. Note that you are at acqui/supplier.pl?booksellerid=1, the details page
   for the vendor, and your contract isn't listed at the bottom of the page,
   and especially that you weren't shown a page that just said "Contract
   deleted."

Sponsored-by: Chetco Community Public Library
Signed-off-by: Sukhmandeep Benipal <sukhmandeep.benipal@inLibro.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 4 Paul Derscheid 2024-09-13 15:00:41 UTC
Looks good, thanks!
Comment 5 Katrin Fischer 2024-09-16 08:54:19 UTC
Pushed for 24.11!

Well done everyone, thank you!