Bug 37089

Summary: Cannot delete a fund or a currency
Product: Koha Reporter: Emily Lamancusa (emlam) <emily.lamancusa>
Component: AcquisitionsAssignee: Emily Lamancusa (emlam) <emily.lamancusa>
Status: RESOLVED FIXED QA Contact: Lucas Gass (lukeg) <lucas>
Severity: major    
Priority: P5 - low CC: david, fridolin.somers, lucas
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37080
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37090
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37091
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes the acquisitions funds and currencies pages so that they can be deleted. After the CSRF changes in 24.05 to improve form security, the 'Delete' actions no longer worked. The deletion confirmation message was displayed with '' for the name and no values for the fund or currency, then didn't delete them.
Version(s) released in:
24.11.00,24.05.02
Circulation function:
Bug Depends on:    
Bug Blocks: 36192    
Attachments: Bug 37089: cud-delete_confirm should not be cud
Bug 37089: cud-delete_confirm should not be cud
Bug 37089: cud-delete_confirm should not be cud

Description Emily Lamancusa (emlam) 2024-06-14 14:02:01 UTC
aqbudgets.pl and currency.pl are looking for op=cud-delete_confirm, but delete_confirm isn't CUD and the page is just a GET displaying the information.

To reproduce:
Part 1 - Delete a fund
1. Go to Acquisitions > Funds
2. Try to delete a fund
--> Confirmation page shows fund name as '' and fund amount as 0.00
3. Click "Yes, delete this fund"
--> The fund is still there

Part 2 - Delete a currency
1. Go to Acquisitions > Currencies
2. Try to delete a currency
--> Confirmation shows empty values for currency and rate
3. Click, "Yes, delete this currency"
--> An error message appears and the currency is not deleted
Comment 1 Emily Lamancusa (emlam) 2024-06-14 14:08:13 UTC
Created attachment 167715 [details] [review]
Bug 37089: cud-delete_confirm should not be cud

Adjust aqbudgets.pl and currency.pl to check for the correct op value

To test:
Part 1 - Delete a fund
1. Go to Acquisitions > Funds
2. Try to delete a fund
--> Confirmation page shows fund name as '' and fund amount as 0.00
3. Click "Yes, delete this fund"
--> The fund is still there

Part 2 - Delete a currency
3. Go to Acquisitions > Currencies
4. Try to delete a currency
--> Confirmation shows empty values for currency and rate
5. Click, "Yes, delete this currency"
--> An error message appears and the currency is not deleted

6. Apply patch and restart_all
7. Repeat steps 1-5
--> Confirmation pages display correct information, and deletions
    occur successfully
Comment 2 Owen Leonard 2024-06-14 14:27:50 UTC
Created attachment 167717 [details] [review]
Bug 37089: cud-delete_confirm should not be cud

Adjust aqbudgets.pl and currency.pl to check for the correct op value

To test:
Part 1 - Delete a fund
1. Go to Acquisitions > Funds
2. Try to delete a fund
   --> Confirmation page shows fund name as '' and fund amount as 0.00
3. Click "Yes, delete this fund"
   --> The fund is still there

Part 2 - Delete a currency
3. Go to Acquisitions > Currencies
4. Try to delete a currency
   --> Confirmation shows empty values for currency and rate
5. Click, "Yes, delete this currency"
   --> An error message appears and the currency is not deleted

6. Apply patch and restart_all
7. Repeat steps 1-5
   --> Confirmation pages display correct information, and deletions
       occur successfully

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 3 Lucas Gass (lukeg) 2024-06-14 17:07:53 UTC
Created attachment 167751 [details] [review]
Bug 37089: cud-delete_confirm should not be cud

Adjust aqbudgets.pl and currency.pl to check for the correct op value

To test:
Part 1 - Delete a fund
1. Go to Acquisitions > Funds
2. Try to delete a fund
   --> Confirmation page shows fund name as '' and fund amount as 0.00
3. Click "Yes, delete this fund"
   --> The fund is still there

Part 2 - Delete a currency
3. Go to Acquisitions > Currencies
4. Try to delete a currency
   --> Confirmation shows empty values for currency and rate
5. Click, "Yes, delete this currency"
   --> An error message appears and the currency is not deleted

6. Apply patch and restart_all
7. Repeat steps 1-5
   --> Confirmation pages display correct information, and deletions
       occur successfully

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 4 Katrin Fischer 2024-06-21 12:04:16 UTC
It looks like we still got quite a few of "cud-delete-confirm" - should we check those systematically?
Comment 5 Katrin Fischer 2024-06-21 13:04:41 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 6 Emily Lamancusa (emlam) 2024-06-21 13:57:26 UTC
(In reply to Katrin Fischer from comment #4)
> It looks like we still got quite a few of "cud-delete-confirm" - should we
> check those systematically?

I agree! That was my goal here as well. :) I actually do have a list that I've been working through (pulled by git grep). The others will take a bit more attention because they *don't* seem to break functionality. I just filed bug 37152 with the list and what I've tested so far.
Comment 7 Katrin Fischer 2024-06-21 14:31:07 UTC
(In reply to Emily Lamancusa from comment #6)
> (In reply to Katrin Fischer from comment #4)
> > It looks like we still got quite a few of "cud-delete-confirm" - should we
> > check those systematically?
> 
> I agree! That was my goal here as well. :) I actually do have a list that
> I've been working through (pulled by git grep). The others will take a bit
> more attention because they *don't* seem to break functionality. I just
> filed bug 37152 with the list and what I've tested so far.

Awesome, thanks Emily!
Comment 8 Lucas Gass (lukeg) 2024-07-18 19:23:29 UTC
Backported to 24.05.x for upcoming 24.05.02
Comment 9 Fridolin Somers 2024-07-19 08:34:52 UTC
Full CSRF not in 23.11.x
Comment 10 David Nind 2024-07-26 20:42:50 UTC
No changes to the manual required - this fixes something that was not working.