Stafftools delete groups and groupings

[nwoodthorpe]

Closes #813

Staff can now delete groups and groupings from stafftools.

Also fixed a small link bug similar to #837

I made Grouping#destroy remove all group-assignments under it because some views we're assuming all group-assignments had a valid grouping, which seems valid. Instead of having this in the controller, should we move it to the model and give GroupAssignments a dependant: :destroy relationship with Grouping?

@tarebyte

[tarebyte]

Overall this is looking good, left a bit of feedback.

Thanks for tackling this!

[tarebyte]

So this could actually be:

GroupAssignment.where(grouping: @grouping).destroy_all

[tarebyte]

I think we'd like to handle the case where the group actually didn't get destroyed. Maybe something like?

if @group.destroy
  flash[:success] = 'Group was destroyed'
  redirect_to stafftools_grouping_path(grouping.id)
else
  flash[:error] = 'Group was not destroyed'
  render :show
end

[tarebyte]

Nice 👍

[tarebyte]

Is there a reason we need the ! here?

Also we can clean this up a bit:

let(:group_assignment) do
  create(:group_assignment, creator: user, organization: organization, grouping: grouping)
end

[nwoodthorpe]

Did let! for eager-evaluation, so that the assignment is created before the delete request runs in the test. Is there a better way to do this? For some reason the test is failing, but passing on my machine. Maybe your tidied version will work

[tarebyte]

Same comment as https://github.com/education/classroom/pull/839/files#r99513000

[nwoodthorpe]

Thanks for the review, will make another commit in a bit

[nwoodthorpe]

@tarebyte Any clue why this test is failing? ./script/cibuild and rspec spec/controllers/stafftools/groupings_controller_spec.rb both pass locally, I can't figure out why Travis is having this error 😕

[tarebyte]

Any clue why this test is failing? ./script/cibuild and rspec spec/controllers/stafftools/groupings_controller_spec.rb both pass locally, I can't figure out why Travis is having this error 😕

My best guess would be to try recreating the VCR cassettes and try pushing that up.

[nwoodthorpe]

Weird, pushed new cassettes and it still fails. I'll look at it when I have a bit more time

[nwoodthorpe]

@tarebyte finally got this sorted, sorry for the huuuuuge delay. I was still on a forked remote for some reason, so my changes were based off of a different repo and making weird things happen.

All good now 👍

[nwoodthorpe]

Ping @tarebyte for when you have time

[nwoodthorpe]

Tests fixed.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.