Bug 4969

Summary: Vendors can not be deleted / show only active vendors
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: AcquisitionsAssignee: Aleisha Amohia <aleisha>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: enhancement    
Priority: P5 - low CC: adrien.saurat, aleisha, brendan, colin.campbell, koha.sekjal, kyle, liz, marjorie.barry-vila, paul.poulain, veron, ztajoli
Version: master   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This patch provides the functionality to hide inactive vendors from the vendor search.
Version(s) released in:
Attachments: proposed patch
proposed patch #2
proposed patch #3
proposed patch #4
Bug 4969: allows deletion of booksellers
[SIGNED-OFF] Bug 4969: deletion of suppliers
Bug 4969: Toggle showing of all/active vendors
Bug 4969: Toggle showing of all/active vendors
Bug 4969: Toggle showing of all/active vendors
Bug 4969: Cursor should be pointer for links
Bug 4969: Do not jump to the top on click
Bug 4969: Do not display the new link if we are seeing all baskets of a given vendor

Description Katrin Fischer 2010-07-08 05:41:56 UTC
You can delete almost everything in acq, but not vendors. Vendors can only be deactivated but still show up in search results.

Although almost everything else in acq can be deleted (budgets, funds, orders (reopen + delete), im not sure it should be possible to delete vendors. 

An option to filter search results to show only active vendors would be a nice feature.
Comment 1 Katrin Fischer 2010-08-10 12:34:41 UTC
*** Bug 5111 has been marked as a duplicate of this bug. ***
Comment 2 Nicole C. Engard 2010-08-10 12:35:44 UTC
upping from enhancement, since it seems like an oversight to me, something that needs to be there (we can delete everything else ... why not vendors?).
Comment 3 Nicole C. Engard 2010-08-10 12:48:29 UTC
--- Comment #1 from Owen Leonard <oleonard@myacpl.org>  ---
This came up in an IRC discussion at one point, and it was pointed out that you
shouldn't be able to delete a vendor from which orders have been placed because
that would mess up information in other places. If a vendor delete option is
added it should have a method of checking to see if the vendor ID is in use
anywhere else.

----

i agree with owen's comment on bug 5111 but that's the same deal with bibs and items - you can't delete them if they're used - but you can delete them
Comment 4 Colin Campbell 2010-08-10 15:06:10 UTC
These code in supplier.pl to do the deletion. What's needed is a button to call it and some code to check we are not deleting a supplier with orders.

It strikes me you might want to flag a bookseller as inactive, i.e. retained for historic info but flagged as not to use for new orders
Comment 5 Adrien SAURAT 2012-01-31 10:32:03 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2012-02-09 14:23:00 UTC
This looks like it's working well, but I think we need to add, at the very least, a JavaScript prompt asking the user to confirm the deletion. Even better would be to have that JS prompt backed up by a server-side prompt, but I realize we don't always stick to that standard.
Comment 7 Adrien SAURAT 2012-02-09 15:23:21 UTC Comment hidden (obsolete)
Comment 8 Owen Leonard 2012-02-10 20:11:26 UTC
This no longer applies.
Comment 9 Katrin Fischer 2012-02-12 20:20:07 UTC
Hi Adrien, 

the patch applies fine for me, but I see a problem.

My test cases were:

1) vendor without orders > Ok, can be deleted
2) vendor with orders > Ok, can NOT be deleted
3) vendor with subscription > Not ok, can be deleted, but this should not be possible.

Serials and acquisitions share vendors, so we should check for orders and subscriptions here.
Comment 10 Adrien SAURAT 2012-02-23 14:22:55 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2012-02-24 06:16:53 UTC
Hi Adrien, 
sorry, your patch does not apply any longer - I think it's because of bug 7113. Could you fix your patch to use booksellerid and resubmit please?
Comment 12 Adrien SAURAT 2012-03-02 13:26:12 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2012-03-08 06:37:40 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2012-03-08 06:39:14 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2012-03-08 06:40:49 UTC
Only a small note: I was a bit irritated that deleting the vendor redirected to the acq home page, because I had expected it to redirect to my vendor list. 
This is probably debatable - the list does not make much sense when you had only one result for yoru search. In my case I was going through the list deleting vendors so did a wider search.
Comment 16 Ian Walls 2012-03-09 20:28:23 UTC
New template param safely added, as well as a conditional check for basketcount before deletion.  Template <li> element has id.  Marking as Passed QA.
Comment 17 Paul Poulain 2012-03-13 14:15:19 UTC
this patch can also be considered as an ENH, but it's worth having it in 3.6

I also checked the JS message was translatable
Comment 18 Jared Camins-Esakov 2012-05-03 22:48:21 UTC
The patches for this bug do not apply to 3.6. If you want this included in 3.6.x, please rebase on 3.6.x and reset the version to rel_3_6.
Comment 19 Owen Leonard 2012-11-27 15:15:01 UTC
This bug seems to cover the "vendors cannot be deleted" aspect of the bug but not the "show only active vendors" part. Does it need a follow-up in order to be considered fixed?
Comment 20 Katrin Fischer 2012-11-27 15:37:10 UTC
I still think it would be a nice enhancement. And a way to shorten your search result list for special libraries. 

I think we can leave the bug open, but obsolete the patch that will not go into 3.6.x now probably.
Comment 21 Liz Rea 2014-07-18 01:32:32 UTC
We still need a "hide inactive vendors" function. If vendors cannot be deleted because they have associated basket/order/invoices, then they need to be able to be hidden when they are no longer in use. 

Another option would be to allow deletion of vendors but only with a superlibrarian account and 4x levels of confirmation (i.e. confirm you want to delete all invoices for X vendor (show them). Now confirm you want to delete all orders for this vendor (show them). Now confirm you want to delete all baskets for this vendor (show them). (accept all three) Delete this vendor?) Would probably also need a warning that doing this will upset acquisitions history. 

All of that seems like a lot of work to simply achieve "my list of vendors is too long and I only use 4 of these 150 vendors I have in my system" so probably better to just do something useful with the inactive flag.
Comment 22 Aleisha Amohia 2017-03-03 01:35:16 UTC
Created attachment 60803 [details] [review]
Bug 4969: Toggle showing of all/active vendors

This patch provides the functionality to hide inactive vendors from the
vendor search.

To test:
1) Make sure at least one vendor is inactive
2) Do a vendor search
3) Confirm the link to show only active vendors works, and is replaced
with a link to show all vendors when clicked
4) Confirm the link to show all vendors works, and is replaced with a
link to show only active vendors when clicked

Sponsored-by: Catalyst IT
Comment 23 Marc Véron 2017-03-03 12:08:28 UTC
Created attachment 60807 [details] [review]
Bug 4969: Toggle showing of all/active vendors

This patch provides the functionality to hide inactive vendors from the
vendor search.

To test:
1) Make sure at least one vendor is inactive
2) Do a vendor search
3) Confirm the link to show only active vendors works, and is replaced
with a link to show all vendors when clicked
4) Confirm the link to show all vendors works, and is replaced with a
link to show only active vendors when clicked

Sponsored-by: Catalyst IT

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 24 Jonathan Druart 2017-03-07 18:52:44 UTC
Created attachment 60893 [details] [review]
Bug 4969: Toggle showing of all/active vendors

This patch provides the functionality to hide inactive vendors from the
vendor search.

To test:
1) Make sure at least one vendor is inactive
2) Do a vendor search
3) Confirm the link to show only active vendors works, and is replaced
with a link to show all vendors when clicked
4) Confirm the link to show all vendors works, and is replaced with a
link to show only active vendors when clicked

Sponsored-by: Catalyst IT

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Jonathan Druart 2017-03-07 18:52:55 UTC
Created attachment 60894 [details] [review]
Bug 4969: Cursor should be pointer for links

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 26 Jonathan Druart 2017-03-07 18:52:59 UTC
Created attachment 60895 [details] [review]
Bug 4969: Do not jump to the top on click

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 27 Jonathan Druart 2017-03-07 18:53:04 UTC
Created attachment 60896 [details] [review]
Bug 4969: Do not display the new link if we are seeing all baskets of a given vendor

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 28 Kyle M Hall 2017-03-31 14:32:42 UTC
Pushed to master for 17.05, thanks Aleisha, Jonathan!
Comment 29 Katrin Fischer 2017-04-02 17:42:02 UTC
Like!

This won't get ported back to 16.11.x as it is an enhancement.