Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4949-Delete vendors if no purchases #5051

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
12 changes: 12 additions & 0 deletions app/controllers/vendors_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,18 @@ def reactivate
redirect_to vendors_path, notice: "#{vendor.business_name} has been reactivated."
end

def destroy
vendor = current_organization.vendors.find(params[:id])
vendor.destroy
if vendor.errors.any?
errors = vendor.errors.full_messages.join("\n")
redirect_to vendors_path, error: "#{vendor.business_name} could not be removed. \n#{errors}"
return
end

redirect_to vendors_path, notice: "#{vendor.business_name} has been removed."
end

private

def vendor_params
Expand Down
11 changes: 11 additions & 0 deletions app/models/vendor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class Vendor < ApplicationRecord
.group(:id)
}

before_destroy :check_for_purchases, prepend: true

def volume
LineItem.where(
itemizable_type: "Purchase",
Expand All @@ -49,4 +51,13 @@ def deactivate!
def reactivate!
update!(active: true)
end

private

def check_for_purchases
return if purchases.empty?

errors.add(:base, "Vendor has purchase items")
throw(:abort)
end
end
12 changes: 9 additions & 3 deletions app/views/vendors/_vendor_row.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@
<%= view_button_to vendor_row %>
<%= edit_button_to edit_vendor_path(vendor_row) %>
<% if vendor_row.active? %>
<%= deactivate_button_to deactivate_vendor_path(vendor_row),
text: 'Deactivate',
confirm: confirm_deactivate_msg(vendor_row.business_name) %>
<% if vendor_row&.purchases&.count.to_i.zero? %>
<%= delete_button_to vendor_path(vendor_row),
text: 'Delete',
confirm: confirm_delete_msg(vendor_row.business_name) %>
<% else %>
<%= deactivate_button_to deactivate_vendor_path(vendor_row),
text: 'Deactivate',
confirm: confirm_deactivate_msg(vendor_row.business_name) %>
<% end %>
<% else %>
<%= reactivate_button_to reactivate_vendor_path(vendor_row),
text: 'Reactivate',
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def set_up_flipper
end
end

resources :vendors, except: [:destroy] do
resources :vendors do
collection do
post :import_csv
end
Expand Down
30 changes: 22 additions & 8 deletions spec/requests/vendors_requests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@
expect(response.body).to include(first_vendor.business_name)
expect(response.body).not_to include(deactivated_vendor.business_name)
end

it "should have a deactivate button for each active vendor" do
expect(subject.body.scan("Deactivate").count).to eq(2)
end
end

context "csv" do
Expand Down Expand Up @@ -115,10 +111,28 @@
end

describe "DELETE #destroy" do
subject { delete vendor_path(id: create(:vendor)) }
it "does not have a route for this" do
subject
expect(response.code).to eq('404')
let!(:vendor) { create(:vendor, organization: organization) }

subject { delete vendor_path(id: vendor.id) }

context 'when vendor does not have purchase items' do
it 'shoud delete the vendor' do
expect { subject }.to change(Vendor, :count)
expect(response).to redirect_to(vendors_path)
follow_redirect!
expect(response.body).to include("#{vendor.business_name} has been removed.")
end
end

context 'when vendor has purchase items' do
before do
create(:purchase, :with_items, vendor: vendor)
end

it 'shoud not delete the vendor' do
expect { subject }.not_to change(Vendor, :count)
expect(response).to have_error(/ could not be removed/)
end
end
end

Expand Down
14 changes: 13 additions & 1 deletion spec/system/vendor_system_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,20 @@
context "When a user views the index page" do
before(:each) do
@second = create(:vendor, business_name: "Bcd")
create(:purchase, :with_items, vendor: @second)
@first = create(:vendor, business_name: "Abc")
create(:purchase, :with_items, vendor: @first)

@third = create(:vendor, business_name: "Cde")
create(:purchase, :with_items, vendor: @third)

@last = create(:vendor, business_name: "Zzz")

visit vendors_path
end

it "should have the vendor names in alphabetical order" do
expect(page).to have_xpath("//table//tr", count: 4)
expect(page).to have_xpath("//table//tr", count: 5)
expect(page.find(:xpath, "//table/tbody/tr[1]/td[1]")).to have_content(@first.business_name)
expect(page.find(:xpath, "//table/tbody/tr[3]/td[1]")).to have_content(@third.business_name)
end
Expand All @@ -33,6 +40,11 @@
expect { click_link "Reactivate", match: :first }.to change { @first.reload.active }.to(true)
end

it "should delete when vendor does not have purchases items" do
expect { click_link "Delete" }.to change(Vendor, :count).by(-1)
expect(page).to have_content("#{@last.business_name} has been removed.")
end

context "When using the include_inactive_vendors filter" do
before(:each) do
@active_vendor = create(:vendor, business_name: "Active Vendor", active: true)
Expand Down