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

Unassign guiders from groups #52

Merged
merged 2 commits into from Oct 5, 2016
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 12 additions & 3 deletions app/controllers/groups_controller.rb
Expand Up @@ -10,15 +10,24 @@ def index

def create
CreateGroupAssignments.new(user_ids, group_params).call
go_back_with_success(:assigned)
Copy link
Contributor

@dwhenry dwhenry Oct 5, 2016

Choose a reason for hiding this comment

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

"The users were assigned from the specified groups"
should this be changed to:
go_back_with_success('assigned to')

end

def destroy
DestroyGroupAssignments.new(user_ids, params[:id]).call

go_back_with_success(:removed)
end

private

def go_back_with_success(action)
redirect_back(
fallback_location: users_path,
success: 'The users were assigned to the specified groups'
success: "The users were #{action} from the specified groups"
)
end

private

def group_params
params.require(:group).permit(:name)
end
Expand Down
17 changes: 17 additions & 0 deletions app/forms/destroy_group_assignments.rb
@@ -0,0 +1,17 @@
class DestroyGroupAssignments
def initialize(user_ids, group_id)
@user_ids = user_ids
@group_id = group_id
end

def call
GroupAssignment
.where(user_id: user_ids, group_id: group_id)
.delete_all
end

private

attr_reader :user_ids
attr_reader :group_id
Copy link
Contributor

Choose a reason for hiding this comment

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

are these needed?
cleaner to just used @user_ids and @group_id

end
11 changes: 7 additions & 4 deletions app/views/groups/index.html.erb
Expand Up @@ -34,7 +34,7 @@
<div class="col-lg-12">
<%= form_for Group.new, url: groups_path(user_ids: params[:user_ids]), class: 'js-form' do |f| %>
<div class="input-group">
<%= f.text_field :name, class: 'form-control js-group-input t-group', placeholder: 'Add a new group' %>
<%= f.text_field :name, class: 'form-control js-group-input t-name', placeholder: 'Add a new group' %>
<span class="input-group-btn">
<%= f.button class: 'btn btn-primary js-button t-add-group' do %>
<span class="glyphicon glyphicon-plus"></span>
Expand All @@ -48,14 +48,17 @@
</td>
</tr>
<% @groups.each do |group| %>
<tr>
<tr class="t-group">
<td class="lead">
<span class="label label-info"><%= group.name %></span>
</td>
<td class="lead">
<a href="#" class="js-remove pull-right">
<%= link_to group_path(group, user_ids: params[:user_ids]),
class: 'js-remove pull-right t-remove',
method: :delete,
data: { confirm: "Are you sure you want to remove the group #{group.name} from these guiders?" } do %>
<span class="glyphicon glyphicon-remove"></span>
</a>
<% end %>
</td>
</tr>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Expand Up @@ -7,5 +7,5 @@
resources :schedules
end

resources :groups, only: %i(index create)
resources :groups, only: %i(index create destroy)
end
24 changes: 24 additions & 0 deletions spec/features/resource_manager_manages_guiders_spec.rb
Expand Up @@ -23,6 +23,30 @@
end
end

scenario 'Unassigning a group from multiple guiders', js: true do
given_the_user_is_a_resource_manager do
and_there_are_guiders_assigned_to_groups
when_they_visit_the_guiders_page
and_they_choose_to_remove_groups_from_multiple_guiders
then_they_see_the_guiders_they_will_affect
when_they_remove_a_group
then_the_group_is_unassigned_from_those_guiders
end
end

def and_they_choose_to_remove_groups_from_multiple_guiders
and_they_choose_to_add_groups_to_multiple_guiders
end

def when_they_remove_a_group
@page.groups.first.remove.click
end

def then_the_group_is_unassigned_from_those_guiders
expect(@page).to have_flash_of_success
expect(@page).to have_no_text(@team_one.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tempting to check the groups are removed in the database too

Copy link
Member Author

Choose a reason for hiding this comment

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

This behaviour is covered in the spec I added for DestroyGroupAssignments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I read the description now. Why is that?

Copy link
Member Author

Choose a reason for hiding this comment

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

They could be reassigned in future as providing the name of an existing group would use that particular instance. Also it cuts out on the need for weird logic checking that the group doesn't have associated assignments and deleting on that basis.

I'm adding a typeahead on the group text field so this would expose the existing groups too.

end

def and_they_choose_to_add_groups_to_multiple_guiders
@page.guiders.map(&:checkbox).each { |c| c.set(true) }
@page.wait_for_action_panel
Expand Down
19 changes: 19 additions & 0 deletions spec/forms/destroy_group_assignments_spec.rb
@@ -0,0 +1,19 @@
require 'rails_helper'

RSpec.describe DestroyGroupAssignments, '#call' do
let(:group) { create(:group) }
let(:users) { create_list(:guider, 2) }
let(:user_ids) { users.map(&:id) }

before do
users.each { |u| u.groups << group }
end

subject { described_class.new(user_ids, group.id) }

context 'for the given users' do
it 'unassigns the given group' do
expect { subject.call }.to change { GroupAssignment.count }.by(-2)
end
end
end
6 changes: 5 additions & 1 deletion spec/support/pages/manage_groups.rb
Expand Up @@ -3,8 +3,12 @@ class ManageGroups < SitePrism::Page
set_url '/groups'

element :affected, '.t-affected'
element :group, '.t-group'
element :group, '.t-name'
element :add_group, '.t-add-group'
element :flash_of_success, '.alert-success'

sections :groups, '.t-group' do
element :remove, '.t-remove'
end
end
end