Unassign guiders from groups #52
Conversation
|
|
||
| private | ||
|
|
||
| def go_back_with_success(action = :assigned) |
There was a problem hiding this comment.
I would make this parameter not optional so it's more obvious when you use it in create.
| @@ -0,0 +1,15 @@ | |||
| class DestroyGroupAssignments | |||
| attr_reader :user_ids | |||
| attr_reader :group_id | |||
There was a problem hiding this comment.
Not sure you need to expose these
|
|
||
| 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) |
There was a problem hiding this comment.
Tempting to check the groups are removed in the database too
There was a problem hiding this comment.
This behaviour is covered in the spec I added for DestroyGroupAssignments.
There was a problem hiding this comment.
Oh I read the description now. Why is that?
There was a problem hiding this comment.
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.
benlovell
force-pushed
the
delete-groups-assignments
branch
from
ff673d5
to
c3296c0
Compare
|
Cool cool merge it |
For the moment this leaves the actual group instances around.
benlovell
force-pushed
the
delete-groups-assignments
branch
from
c3296c0
to
fd1814a
Compare
| private | ||
|
|
||
| attr_reader :user_ids | ||
| attr_reader :group_id |
There was a problem hiding this comment.
are these needed?
cleaner to just used @user_ids and @group_id
| @@ -10,15 +10,24 @@ def index | |||
|
|
|||
| def create | |||
| CreateGroupAssignments.new(user_ids, group_params).call | |||
| go_back_with_success(:assigned) | |||
There was a problem hiding this comment.
"The users were assigned from the specified groups"
should this be changed to:
go_back_with_success('assigned to')
For the moment this leaves the actual group instances around.