Bug 31921 - No confirmation alert when deleting a vendor
Summary: No confirmation alert when deleting a vendor
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Sam Lau
QA Contact: Testopia
URL:
Keywords:
Depends on: 28082
Blocks:
  Show dependency treegraph
 
Reported: 2022-10-21 15:47 UTC by Jonathan Druart
Modified: 2024-07-11 14:40 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 31921: Add confirmation modal when deleting a vendor (7.66 KB, patch)
2024-07-09 15:30 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 31921: Add confirmation modal when deleting a vendor (7.73 KB, patch)
2024-07-09 17:26 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 31921: Add confirmation modal to contracts and uncertain prices (1.83 KB, patch)
2024-07-10 15:53 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 31921: Include the modal code from acquisitions-toolbar.inc (2.06 KB, patch)
2024-07-11 14:37 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 Jonathan Druart 2022-10-21 15:47:47 UTC
First vendor of the page has a confirmation alert, but the answer is ignored!

I guess it's coming from
Comment 1 Martin Renvoize 2024-06-10 16:11:21 UTC
Can we confirm if this is still the case?
Comment 2 Jonathan Druart 2024-06-13 10:48:41 UTC
Yes, still valid.
Comment 3 Sam Lau 2024-07-09 15:30:16 UTC
Created attachment 168668 [details] [review]
Bug 31921: Add confirmation modal when deleting a vendor

This patch adds a new modal for confirming vendor deletion. It fixes the previous functionality, showing a confirmation modal for each vendor and appropriately handling the response.

To test:
1) Visit Acquisitions home. Under where it says 'Manage orders', click the search bar to bring up the list of vendors.
2) Add a few vendors by clicking 'New' and 'Vendor'. After this, go back to the vendor search so that all the vendors are showing.
3) For each vendor, click on 'Delete vendor' and ensure the modal shows properly.
4) Attempt to delete one of the vendors. Go back to the vendor search and ensure they are deleted correctly.
5) Click on a vendor's name to go their supplier page. Make sure modal works properly and attempt to delete a vendor from here.
Comment 4 Owen Leonard 2024-07-09 17:26:02 UTC
Created attachment 168674 [details] [review]
Bug 31921: Add confirmation modal when deleting a vendor

This patch adds a new modal for confirming vendor deletion. It fixes
the previous functionality, showing a confirmation modal for each
vendor and appropriately handling the response.

To test:
1) Visit Acquisitions home. Under where it says 'Manage orders', click
   the search bar to bring up the list of vendors.
2) Add a few vendors by clicking 'New' and 'Vendor'. After this, go back
   to the vendor search so that all the vendors are showing.
3) For each vendor, click on 'Delete vendor' and ensure the modal shows
   properly.
4) Attempt to delete one of the vendors. Go back to the vendor search
   and ensure they are deleted correctly.
5) Click on a vendor's name to go their supplier page. Make sure modal
   works properly and attempt to delete a vendor from here.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Jonathan Druart 2024-07-10 08:17:23 UTC
It will only works from supplier.pl and booksellers.pl

For instance it does not work from aqcontract.pl

I think you should include the modal code from acquisitions-toolbar.inc to avoid this kind of mistakes.
Comment 6 Sam Lau 2024-07-10 14:43:14 UTC Comment hidden (obsolete)
Comment 7 Sam Lau 2024-07-10 15:53:19 UTC
Created attachment 168749 [details] [review]
Bug 31921: Add confirmation modal to contracts and uncertain prices

To test:
1) Have 2 deletable vendors
2) Visit the first vendor's "Contracts" tab
3) Attempt to delete from here, modal shows and deltion is successful
4) Visit the second vendor's "Uncertain prices" tab.
5) Attempt to delete from here, modal shows and deletion is successful
Comment 8 Jonathan Druart 2024-07-11 09:07:45 UTC
(In reply to Jonathan Druart from comment #5)
> I think you should include the modal code from acquisitions-toolbar.inc to
> avoid this kind of mistakes.

Did you try this?
Comment 9 Sam Lau 2024-07-11 14:13:07 UTC
(In reply to Jonathan Druart from comment #8)
> (In reply to Jonathan Druart from comment #5)
> > I think you should include the modal code from acquisitions-toolbar.inc to
> > avoid this kind of mistakes.
> 
> Did you try this?

There is no modal code in acquisitions-toolbar.inc so I'm not sure what you mean. Unless you are saying to include the new 'modals/delete_vendor.inc' inside of the acquisitions toolbar? In which case, no I did not.
Comment 10 Jonathan Druart 2024-07-11 14:37:37 UTC
Created attachment 168832 [details] [review]
Bug 31921: Include the modal code from acquisitions-toolbar.inc
Comment 11 Jonathan Druart 2024-07-11 14:39:35 UTC
This patch is what I meant. It works on all pages where the modal exists and avoid the code duplication.
Comment 12 Sam Lau 2024-07-11 14:40:58 UTC
(In reply to Jonathan Druart from comment #11)
> This patch is what I meant. It works on all pages where the modal exists and
> avoid the code duplication.

Got it, that makes sense. Thanks for the help