Add is_deleted to Group and soft_destroy!

groupに紐づいたcollaborationsは全削除する

groups/editにdeleteボタンを設置

Remove authorize_resource
代わりにforbiddenを追加
- 権限がないユーザーはアクセス拒否

Add Group#deletable?
- Not delete a group when it is owner of project

app/assets/stylesheets/groups/_form.scss

@@ -17,6 +17,11 @@
     margin-bottom: 20px;
   }
 
+  #alert {
+    color: $common-error-text-color;
+    margin-bottom: 20px;
+  }
+
   .page-title {
     padding: 20px 0;
     font-weight: normal;

app/assets/stylesheets/groups/edit.scss

@@ -84,8 +84,11 @@
       }
     }
 
-    #new-membership {
+    #new-membership, #delete-group {
       margin-top: 55px;
+    }
+
+    #new-membership {
       #s2id_member_name {
         margin: 20px 0px;
         font-family: $common-novelsans-semibold;
@@ -110,6 +113,17 @@
         border: none;
       }
     }
+
+    #delete-group {
+      #delete-btn {
+        @include clickable;
+        margin: 60px 0 20px;
+        padding: 5px 68px;
+        font-size: 18px;
+        color: $common-btn-text-color;
+        border: none;
+      }
+    }
   }
 }
 

app/controllers/groups_controller.rb

@@ -1,20 +1,19 @@
 class GroupsController < ApplicationController
   layout 'groups'
 
-  before_action :build_group, only: [:new, :create]
   before_action :load_group, only: [:edit, :update, :destroy]
-
-  authorize_resource
+  before_action :forbidden, only: [:edit, :update, :destroy]
 
   def index
     @groups = current_user.groups
   end
 
   def new
+    @group = Group.new
   end
 
   def create
-    @group = Group.new group_params
+    @group = Group.new(group_params)
     Group.transaction do
       @group.save!
       membership = current_user.join_to @group
@@ -30,16 +29,23 @@ def edit
   end
 
   def update
-    if @group.update_attributes group_params
+    if @group.update_attributes(group_params)
       redirect_to [:edit, @group], notice: 'Group profile was successfully updated.'
     else
       render :edit
     end
   end
 
   def destroy
-    @group.destroy
+    unless @group.deletable?
+      redirect_to [:edit, @group], alert: 'This group still has projects.'
+      return
+    end
+
+    @group.soft_destroy!
     redirect_to :groups
+  rescue
+    redirect_to [:edit, @group], alert: 'Group was not deleted.'
   end
 
   private
@@ -54,11 +60,13 @@ def additional_params
       [:id, member_ids: []]
     end
 
-    def build_group
-      @group = Group.new(group_params)
+    def load_group
+      @group = current_user.groups.friendly.find params[:id]
     end
 
-    def load_group
-      @group = Group.friendly.find params[:id]
+    def forbidden
+      unless current_user.is_admin_of?(@group)
+        render file: "public/403.html", status: :forbidden
+      end
     end
 end

app/models/group.rb

@@ -4,6 +4,7 @@
 #
 #  id             :integer          not null, primary key
 #  avatar         :string(255)
+#  is_deleted     :boolean          default(FALSE), not null
 #  location       :string(255)
 #  name           :string(255)
 #  projects_count :integer          default(0), not null
@@ -46,6 +47,17 @@ def generate_draft
     end
   end
 
+  def deletable?
+    projects.none? || projects.pluck(:is_deleted).all?
+  end
+
+  def soft_destroy!
+    transaction do
+      update!(is_deleted: true)
+      collaborations.destroy_all
+    end
+  end
+
   private
 
     def should_generate_new_friendly_id?

app/views/groups/_form.html.slim

@@ -1,5 +1,6 @@
 = form_for @group, class: "group-form" do |f|
-  p#notice = notice
+  - flash.each do |key, value|
+    = content_tag(:p, value, id: key)
 
   - if @group.errors.any?
     #error_explanation

app/views/groups/edit.html.slim

@@ -9,7 +9,7 @@
   #edit-members
     .inner
       h1.title
-       'Members
+        'Members
       ul#members
         - @group.members.each do |member|
           = render "membership", membership: member.membership_in(@group)
@@ -23,7 +23,12 @@
           = select_tag :role, options_for_select(Membership::ROLE)
           = submit_tag "add", id: "add-btn", class: "btn"
 
-- content_for :bottom
+        #delete-group
+          h2.title
+            'Delete this group
+          = link_to "Delete", group_path(@group), method: :delete, class: "btn", id: "delete-btn", data: { confirm: "Are you sure to delete this group?" }
+
+        - content_for :bottom
 
   coffee:
     $(document).on "click", "#group-avatar", (event) ->

db/migrate/20180912105807_add_is_deleted_to_group.rb

@@ -0,0 +1,5 @@
+class AddIsDeletedToGroup < ActiveRecord::Migration[5.2]
+  def change
+    add_column :groups, :is_deleted, :boolean, null: false, default: false
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2018_09_04_033951) do
+ActiveRecord::Schema.define(version: 2018_09_12_105807) do
 
   create_table "attachments", id: :integer, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 ROW_FORMAT=DYNAMIC", force: :cascade do |t|
     t.string "content"
@@ -106,6 +106,7 @@
     t.integer "projects_count", default: 0, null: false
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.boolean "is_deleted", default: false, null: false
     t.index ["name"], name: "index_users_on_name", unique: true
     t.index ["slug"], name: "index_users_on_slug", unique: true
   end

spec/controllers/groups_controller_spec.rb

@@ -60,7 +60,7 @@
         other.memberships.create(group: group, role: 'editor')
       end
 
-      it { is_expected.to have_http_status :unauthorized }
+      it { is_expected.to have_http_status :forbidden }
     end
   end
 
@@ -106,34 +106,76 @@
         expect { subject }.not_to change { group.reload.name }
       end
 
-      it { is_expected.to have_http_status :unauthorized }
+      it { is_expected.to have_http_status :forbidden }
     end
   end
 
   describe 'DELETE destroy' do
     subject { delete :destroy, params: { id: group } }
+    before { user.memberships.create(group_id: group.id, role: 'admin') }
 
     describe 'as an admin' do
       let!(:group) { FactoryBot.create :group }
 
-      before do
-        sign_in user
-        user.memberships.create(group_id: group.id, role: 'admin')
-      end
+      before { sign_in user }
 
       it { is_expected.to have_http_status :redirect }
-      it { expect{ subject }.to change{ Group.count }.by(-1) }
+      it { expect { subject }.to change { group.reload.is_deleted }.from(false).to(true) }
     end
 
     describe 'as an editor' do
       before do
         sign_in other
-        user.memberships.create(group_id: group.id, role: 'admin')
         other.memberships.create(group_id: group.id, role: 'editor')
       end
 
-      it { is_expected.to have_http_status :unauthorized }
-      it { expect{ subject }.to change{ Group.count }.by(0) }
+      it { is_expected.to have_http_status :forbidden }
+      it { expect { subject }.not_to change { group.reload.is_deleted } }
+    end
+
+    describe 'collaborations' do
+      before do
+        sign_in user
+        FactoryBot.create_list(:collaboration, 2, owner: group)
+      end
+
+      it { expect { subject }.to change { Collaboration.count }.to(0) }
+    end
+
+    describe 'projects' do
+      before do
+        sign_in user
+        FactoryBot.create_list(:project, 2, owner: group)
+      end
+
+      context 'when a group has projects' do
+        it { is_expected.to redirect_to [:edit, group] }
+        it { expect { subject }.not_to change { group.reload.is_deleted } }
+      end
+
+      context 'when a group has NO projects' do
+        before { group.projects.destroy_all }
+        it { is_expected.to redirect_to :groups }
+        it { expect { subject }.to change { group.reload.is_deleted }.from(false).to(true) }
+      end
+
+      context 'when projects are soft deleted' do
+        before { group.projects.update_all(is_deleted: true) }
+        it { is_expected.to redirect_to :groups }
+        it { expect { subject }.to change { group.reload.is_deleted }.from(false).to(true) }
+      end
+    end
+
+    describe 'raise on soft_destroy!' do
+      before do
+        sign_in user
+        FactoryBot.create_list(:collaboration, 2, owner: group)
+        allow_any_instance_of(Group).to receive(:soft_destroy!).and_raise(StandardError)
+      end
+
+      it { is_expected.to redirect_to [:edit, group] }
+      it { expect { subject }.not_to change { group.reload.is_deleted } }
+      it { expect { subject }.not_to change { Collaboration.count } }
     end
   end
 end

spec/factories/groups.rb

@@ -5,6 +5,7 @@
 #
 #  id             :integer          not null, primary key
 #  avatar         :string(255)
+#  is_deleted     :boolean          default(FALSE), not null
 #  location       :string(255)
 #  name           :string(255)
 #  projects_count :integer          default(0), not null

spec/models/group_spec.rb

@@ -30,4 +30,58 @@
     end
     it { is_expected.to be true }
   end
+
+  describe '#deletable?' do
+    subject { group.deletable? }
+    let(:group) { FactoryBot.create :group }
+
+    it { is_expected.to eq true }
+
+    context 'when group has projects' do
+      before { FactoryBot.create(:project, owner: group) }
+
+      it { is_expected.to eq false }
+    end
+
+    context 'when projects are all deleted' do
+      before { FactoryBot.create_list(:project, 2, owner: group, is_deleted: true) }
+
+      it { is_expected.to eq true }
+    end
+  end
+
+  describe '#soft_destroy!' do
+    subject { group.soft_destroy! }
+    let(:group) { FactoryBot.create(:group) }
+    let(:collaborations) { FactoryBot.create_list(:collaboration, 2, owner: group) }
+
+    it { expect { subject }.to change { group.is_deleted }.from(false).to(true) }
+    it { expect { subject }.to change { group.collaborations.count }.by(0) }
+
+    context 'when failing to update group' do
+      before { allow(group).to receive(:update!).and_raise(ActiveRecord::RecordNotSaved) }
+
+      it 'does NOT delete group' do
+        expect do
+          begin
+            subject
+          rescue ActiveRecord::RecordNotSaved => _
+          end
+        end.not_to change { group.is_deleted }
+      end
+    end
+
+    context 'when failing to delete all collaborations' do
+      before { allow(Collaboration).to receive(:destroy!).and_raise(ActiveRecord::RecordNotDestroyed) }
+
+      it 'does NOT delete collaborations' do
+        expect do
+          begin
+            subject
+          rescue ActiveRecord::RecordNotDestroyed => _
+          end
+        end.not_to change { group.collaborations.count }
+      end
+    end
+  end
 end