Merge b501bb9 into 30bf0ca

app/models/caffeinate/campaign_subscription.rb

@@ -99,6 +99,7 @@ def ended?
     # Updates `ended_at` and runs `on_complete` callbacks
     def end!(reason = ::Caffeinate.config.default_ended_reason)
       raise ::Caffeinate::InvalidState, 'CampaignSubscription is already unsubscribed.' if unsubscribed?
+      return true if ended?
 
       update!(ended_at: ::Caffeinate.config.time_now, ended_reason: reason)
 
@@ -109,6 +110,7 @@ def end!(reason = ::Caffeinate.config.default_ended_reason)
     # Updates `ended_at` and runs `on_complete` callbacks
     def end(reason = ::Caffeinate.config.default_ended_reason)
       return false if unsubscribed?
+      return true if ended?
 
       result = update(ended_at: ::Caffeinate.config.time_now, ended_reason: reason)
 

spec/models/caffeinate/campaign_subscription_spec.rb

@@ -34,6 +34,15 @@
   end
 
   describe '#end!' do
+    context 'when already ended' do
+      it 'no-ops on subsequent #end! calls' do
+        subscription.end!
+        first_ended_at = subscription.ended_at
+        expect(subscription).to_not receive(:update!)
+        subscription.end!
+        expect(first_ended_at).to eq subscription.ended_at
+      end
+    end
     context 'without argument' do
       it 'is not ended' do
         expect(subscription).not_to be_ended