Bug 37769 - Fix forms that POST without an op in currency administration
Summary: Fix forms that POST without an op in currency administration
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P3 normal
Assignee: Phil Ringnalda
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 36192
Blocks: 37728
  Show dependency treegraph
 
Reported: 2024-08-29 04:01 UTC by Phil Ringnalda
Modified: 2024-09-17 08:50 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 37769: Fix forms that POST without an op in currency administration (2.98 KB, patch)
2024-08-29 22:46 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 37769: Fix forms that POST without an op in currency administration (3.05 KB, patch)
2024-09-09 18:25 UTC, Sukhmandeep
Details | Diff | Splinter Review
Bug 37769: Fix forms that POST without an op in currency administration (3.12 KB, patch)
2024-09-16 12:53 UTC, Jonathan Druart
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-08-29 04:01:51 UTC
We intend not to have forms with method="post" without an op variable (so we
can check that the op starts with "cud-" as part of the CSRF protection), but
because of bug 37728 some were missed.

In Currency and exchange rate administration, that's the "OK" button that takes you back to the list when you try to delete a currency in use by a vendor, which doesn't need to POST, and the OK button in the confirmation page after you delete a currency, which currently doesn't actually show at all, but when it does doesn't need to POST since it just takes you back to the list of currencies.
Comment 1 Phil Ringnalda 2024-08-29 22:27:22 UTC
Too many near-identical bugs, the second instance is not in a confirmation page after deleting, currencies don't have one, it is in the "No, do not delete" cancel button when you could confirm deletion but instead decide not to delete.
Comment 2 Phil Ringnalda 2024-08-29 22:46:18 UTC
Created attachment 170890 [details] [review]
Bug 37769: Fix forms that POST without an op in currency administration

We intend not to have forms with method="post" without an op variable (so we
can check that the op starts with "cud-" as part of the CSRF protection), but
because of bug 37728 some were missed.

This patch changes the form around the OK button when you are told you can't
delete a currency which is in use, and the No, do not delete button when you
could delete a currency and decide not to, from a POST to a GET because all
they need to do is show the list of currencies again.

The only visible change from the patch is that the URL will end with a "?"
from having done a GET without any params. Someone who wants to decide
which of our link-as-cancel-button styles to use is welcome to switch them
to links, in a bug not blocking an RM_priority bug.

Test plan:
1. No changes to see, so apply the patch first
2. Administration - Currencies and exchange rates
3. You need one currency in use and one not in use. Luckily, ktd gave you
   USD for in use, and GBP for not in use. For USD, click the Deleete button
4. On the page telling you that you can't delete it because it's in use,
   click the OK button and verify that you are back at the list of currencies
5. Click the Delete button for GBP, then the No, do not delete button
6. Verify that you are back at the list of currencies

Sponsored-by: Chetco Community Public Library
Comment 3 Sukhmandeep 2024-09-09 18:25:29 UTC
Created attachment 171228 [details] [review]
Bug 37769: Fix forms that POST without an op in currency administration

We intend not to have forms with method="post" without an op variable (so we
can check that the op starts with "cud-" as part of the CSRF protection), but
because of bug 37728 some were missed.

This patch changes the form around the OK button when you are told you can't
delete a currency which is in use, and the No, do not delete button when you
could delete a currency and decide not to, from a POST to a GET because all
they need to do is show the list of currencies again.

The only visible change from the patch is that the URL will end with a "?"
from having done a GET without any params. Someone who wants to decide
which of our link-as-cancel-button styles to use is welcome to switch them
to links, in a bug not blocking an RM_priority bug.

Test plan:
1. No changes to see, so apply the patch first
2. Administration - Currencies and exchange rates
3. You need one currency in use and one not in use. Luckily, ktd gave you
   USD for in use, and GBP for not in use. For USD, click the Deleete button
4. On the page telling you that you can't delete it because it's in use,
   click the OK button and verify that you are back at the list of currencies
5. Click the Delete button for GBP, then the No, do not delete button
6. Verify that you are back at the list of currencies

Sponsored-by: Chetco Community Public Library
Signed-off-by: Sukhmandeep Benipal <sukhmandeep.benipal@inLibro.com>
Comment 4 Jonathan Druart 2024-09-16 12:53:26 UTC
Created attachment 171539 [details] [review]
Bug 37769: Fix forms that POST without an op in currency administration

We intend not to have forms with method="post" without an op variable (so we
can check that the op starts with "cud-" as part of the CSRF protection), but
because of bug 37728 some were missed.

This patch changes the form around the OK button when you are told you can't
delete a currency which is in use, and the No, do not delete button when you
could delete a currency and decide not to, from a POST to a GET because all
they need to do is show the list of currencies again.

The only visible change from the patch is that the URL will end with a "?"
from having done a GET without any params. Someone who wants to decide
which of our link-as-cancel-button styles to use is welcome to switch them
to links, in a bug not blocking an RM_priority bug.

Test plan:
1. No changes to see, so apply the patch first
2. Administration - Currencies and exchange rates
3. You need one currency in use and one not in use. Luckily, ktd gave you
   USD for in use, and GBP for not in use. For USD, click the Deleete button
4. On the page telling you that you can't delete it because it's in use,
   click the OK button and verify that you are back at the list of currencies
5. Click the Delete button for GBP, then the No, do not delete button
6. Verify that you are back at the list of currencies

Sponsored-by: Chetco Community Public Library
Signed-off-by: Sukhmandeep Benipal <sukhmandeep.benipal@inLibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Katrin Fischer 2024-09-17 08:50:03 UTC
Pushed for 24.11!

Well done everyone, thank you!