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

4634 - Add the Delete button on the Partner Group Page #4649

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion app/controllers/partner_groups_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
class PartnerGroupsController < ApplicationController
before_action :set_partner_group, only: %i[edit destroy]

def new
@partner_group = current_organization.partner_groups.new
@item_categories = current_organization.item_categories
Expand All @@ -16,7 +18,6 @@ def create
end

def edit
@partner_group = current_organization.partner_groups.find(params[:id])
@item_categories = current_organization.item_categories
end

Expand All @@ -30,8 +31,23 @@ def update
end
end

def destroy
if @partner_group.partners.any?
redirect_to partners_path + "#nav-partner-groups", alert: "Partner Group cannot be deleted."
else
@partner_group.destroy
respond_to do |format|
format.html { redirect_to partners_path + "#nav-partner-groups", notice: "Partner Group was successfully deleted." }
end
end
end

private

def set_partner_group
@partner_group = current_organization.partner_groups.find(params[:id])
end

def partner_group_params
params.require(:partner_group).permit(:name, :send_reminders, :deadline_day, :reminder_day, item_category_ids: [])
end
Expand Down
3 changes: 2 additions & 1 deletion app/views/partners/_partner_groups_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@
<span class='text-gray-600 text-bold font-italic'>No</span>
<% end %>
</td>
<td class="text-right">
<td class="text-left">
<%= edit_button_to edit_partner_group_path(pg) %>
<%= delete_button_to(partner_group_path(pg),{confirm: confirm_delete_msg(pg.name)}) if pg.partners.none? %>
</td>
</tr>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ def set_up_flipper
end
end

resources :partner_groups, only: [:new, :create, :edit, :update]
resources :partner_groups, only: %i(new create edit update destroy)

resources :product_drives

Expand Down
46 changes: 46 additions & 0 deletions spec/requests/partner_group_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
RSpec.describe "PartnerGroups", type: :request do
let(:user) { create(:user) }
let(:partner_group) { create(:partner_group) }

before do
sign_in(user)
end

describe "DELETE #destroy" do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a spec that checks that the delete button is only shown if there are no partners. This can still be a request spec since you can check the response HTML.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also - please don't force push your branch once a review has started - it becomes harder to see what has changed since the last review.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hii @dorner,
I've updated the spec based on your suggestion. I had to force-push the changes since I couldn't see my updates in the branch. Please ignore this for now. I'll work on improving it in future PRs

context "when partner group has no partners" do
let!(:partner_group) { create(:partner_group) }
before { get partners_path + "#nav-partner-groups" }
it "destroys the partner group" do
within "#nav-partner-groups" do
expect(response.body).to have_link("Delete")
end
expect {
delete partner_group_path(partner_group)
}.to change(PartnerGroup, :count).by(-1)

expect(flash[:notice]).to eq("Partner Group was successfully deleted.")
expect(response).to redirect_to(partners_path + "#nav-partner-groups")
end
end

context "when partner group has partners" do
let!(:partner_group) { create(:partner_group) }

before do
create(:partner, partner_group: partner_group)
get partners_path + "#nav-partner-groups"
end
it "does not destroy the partner group" do
within "#nav-partner-groups" do
expect(reponse.body).not_to have_link("Delete")
end
expect {
delete partner_group_path(partner_group)
}.not_to change(PartnerGroup, :count)

expect(flash[:alert]).to eq("Partner Group cannot be deleted.")
expect(response).to redirect_to(partners_path + "#nav-partner-groups")
end
end
end
end