Transform Flag.flag! & unflag! into non-raising methods

They now return false and do nothing instead

app/controllers/comments_controller.rb

@@ -23,12 +23,12 @@ class CommentsController < ApplicationController
   end
 
   def flag
-    Flag.flag!(current_user, @comment)
+    Flag.flag(current_user, @comment)
     respond_with @comment, template: 'comments/_refresh_flag_actions'
   end
 
   def unflag
-    Flag.unflag!(current_user, @comment)
+    Flag.unflag(current_user, @comment)
     respond_with @comment, template: 'comments/_refresh_flag_actions'
   end
 

app/controllers/debates_controller.rb

@@ -55,12 +55,12 @@ class DebatesController < ApplicationController
   end
 
   def flag
-    Flag.flag!(current_user, @debate)
+    Flag.flag(current_user, @debate)
     respond_with @debate, template: 'debates/_refresh_flag_actions'
   end
 
   def unflag
-    Flag.unflag!(current_user, @debate)
+    Flag.unflag(current_user, @debate)
     respond_with @debate, template: 'debates/_refresh_flag_actions'
   end
 

app/models/flag.rb

@@ -9,28 +9,14 @@ class Flag < ActiveRecord::Base
           flaggable_id: flaggable.id)
   end)
 
-
+  def self.flag(user, flaggable)
-  class AlreadyFlaggedError < StandardError
+    return false if flagged?(user, flaggable)
-    def initialize
-      super "The flaggable was already flagged by this user"
-    end
-  end
-
-  class NotFlaggedError < StandardError
-    def initialize
-      super "The flaggable was not flagged by this user"
-    end
-  end
-
-
-  def self.flag!(user, flaggable)
-    raise AlreadyFlaggedError if flagged?(user, flaggable)
     create(user: user, flaggable: flaggable)
   end
 
-  def self.unflag!(user, flaggable)
+  def self.unflag(user, flaggable)
     flags = by_user_and_flaggable(user, flaggable)
-    raise NotFlaggedError if flags.empty?
+    return false if flags.empty?
     flags.destroy_all
   end
 

spec/factories.rb

@@ -68,7 +68,7 @@ FactoryGirl.define do
 
     trait :flagged do
       after :create do |debate|
-        Flag.flag!(FactoryGirl.create(:user), debate)
+        Flag.flag(FactoryGirl.create(:user), debate)
       end
     end
   end
@@ -101,7 +101,7 @@ FactoryGirl.define do
 
     trait :flagged do
       after :create do |debate|
-        Flag.flag!(FactoryGirl.create(:user), debate)
+        Flag.flag(FactoryGirl.create(:user), debate)
       end
     end
   end

spec/features/comments_spec.rb

@@ -155,7 +155,7 @@ feature 'Comments' do
     user = create(:user)
     debate = create(:debate)
     comment = create(:comment, commentable: debate)
-    Flag.flag!(user, comment)
+    Flag.flag(user, comment)
 
     login_as(user)
     visit debate_path(debate)

spec/features/debates_spec.rb

@@ -335,7 +335,7 @@ feature 'Debates' do
   scenario "Unflagging", :js do
     user = create(:user)
     debate = create(:debate)
-    Flag.flag!(user, debate)
+    Flag.flag(user, debate)
 
     login_as(user)
     visit debate_path(debate)

spec/models/ability_spec.rb

@@ -52,14 +52,14 @@ describe Ability do
       end
 
       describe "already-flagged comments" do
-        before(:each) { Flag.flag!(user, comment) }
+        before(:each) { Flag.flag(user, comment) }
 
         it { should_not be_able_to(:flag, comment) }
         it { should be_able_to(:unflag, comment) }
       end
 
       describe "already-flagged debates" do
-        before(:each) { Flag.flag!(user, debate) }
+        before(:each) { Flag.flag(user, debate) }
 
         it { should_not be_able_to(:flag, debate) }
         it { should be_able_to(:unflag, debate) }

spec/models/flag_spec.rb

@@ -5,41 +5,41 @@ describe Flag do
   let(:user) { create(:user) }
   let(:comment) { create(:comment) }
 
-  describe '.flag!' do
+  describe '.flag' do
 
     it 'creates a flag when there is none' do
-      expect { described_class.flag!(user, comment) }.to change{ Flag.count }.by(1)
+      expect { described_class.flag(user, comment) }.to change{ Flag.count }.by(1)
       expect(Flag.last.user).to eq(user)
       expect(Flag.last.flaggable).to eq(comment)
     end
 
-    it 'raises an error if the flag has already been created' do
+    it 'does nothing if the flag already exists' do
-      described_class.flag!(user, comment)
+      described_class.flag(user, comment)
-      expect { described_class.flag!(user, comment) }.to raise_error(Flag::AlreadyFlaggedError)
+      expect(described_class.flag(user, comment)).to eq(false)
+      expect(Flag.by_user_and_flaggable(user, comment).count).to eq(1)
     end
 
     it 'increases the flag count' do
-      expect { described_class.flag!(user, comment) }.to change{ comment.reload.flags_count }.by(1)
+      expect { described_class.flag(user, comment) }.to change{ comment.reload.flags_count }.by(1)
     end
   end
 
-  describe '.unflag!' do
+  describe '.unflag' do
     it 'raises an error if the flag does not exist' do
-      expect { described_class.unflag!(user, comment) }.to raise_error(Flag::NotFlaggedError)
+      expect(described_class.unflag(user, comment)).to eq(false)
     end
 
     describe 'when the flag already exists' do
-      before(:each) { described_class.flag!(user, comment) }
+      before(:each) { described_class.flag(user, comment) }
 
       it 'removes an existing flag' do
-        expect { described_class.unflag!(user, comment) }.to change{ Flag.count }.by(-1)
+        expect { described_class.unflag(user, comment) }.to change{ Flag.count }.by(-1)
       end
 
       it 'decreases the flag count' do
-        expect { described_class.unflag!(user, comment) }.to change{ comment.reload.flags_count }.by(-1)
+        expect { described_class.unflag(user, comment) }.to change{ comment.reload.flags_count }.by(-1)
       end
     end
-
   end
 
   describe '.flagged?' do
@@ -48,7 +48,7 @@ describe Flag do
     end
 
     it 'returns true when the user has flagged the comment' do
-      described_class.flag!(user, comment)
+      described_class.flag(user, comment)
       expect(described_class.flagged?(user, comment)).to be
     end
   end