From 0132d7b25130d8e2920029e9be0914934b124ffc Mon Sep 17 00:00:00 2001
From: Karina Kwiatek <kjk@kjk.dog>
Date: Sat, 4 Mar 2023 20:42:51 +0100
Subject: [PATCH] Remove usages of `is_active`

---
 .../ajax/subscription_controller.rb            |  8 ++++----
 app/models/comment.rb                          |  2 +-
 app/models/subscription.rb                     | 18 ++++++------------
 .../ajax/subscription_controller_spec.rb       | 10 +++++-----
 4 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/app/controllers/ajax/subscription_controller.rb b/app/controllers/ajax/subscription_controller.rb
index a04520e4..fd574e78 100644
--- a/app/controllers/ajax/subscription_controller.rb
+++ b/app/controllers/ajax/subscription_controller.rb
@@ -4,14 +4,14 @@ class Ajax::SubscriptionController < AjaxController
   def subscribe
     params.require :answer
     @response[:status] = :okay
-    state = Subscription.subscribe(current_user, Answer.find(params[:answer])).nil?
-    @response[:success] = state == false
+    result = Subscription.subscribe(current_user, Answer.find(params[:answer]))
+    @response[:success] = result.present?
   end
 
   def unsubscribe
     params.require :answer
     @response[:status] = :okay
-    state = Subscription.unsubscribe(current_user, Answer.find(params[:answer])).nil?
-    @response[:success] = state == false
+    result = Subscription.unsubscribe(current_user, Answer.find(params[:answer]))
+    @response[:success] = result&.destroyed? || false
   end
 end
diff --git a/app/models/comment.rb b/app/models/comment.rb
index 3b43999e..830a6f4e 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -8,7 +8,7 @@ class Comment < ApplicationRecord
   validates :content, length: { maximum: 512 }
 
   after_create do
-    Subscription.subscribe self.user, answer, false
+    Subscription.subscribe self.user, answer
     Subscription.notify self, answer
   end
 
diff --git a/app/models/subscription.rb b/app/models/subscription.rb
index e8704d8b..1eb4fd9e 100644
--- a/app/models/subscription.rb
+++ b/app/models/subscription.rb
@@ -3,17 +3,11 @@ class Subscription < ApplicationRecord
   belongs_to :answer
 
   class << self
-    def for(target)
-      Subscription.where(answer: target)
-    end
-
-    def subscribe(recipient, target, force = true)
+    def subscribe(recipient, target)
       existing = Subscription.find_by(user: recipient, answer: target)
-      if existing.nil?
-        Subscription.new(user: recipient, answer: target).save!
-      elsif force
-        existing.update(is_active: true)
-      end
+      return true if existing.present?
+
+      Subscription.create!(user: recipient, answer: target)
     end
 
     def unsubscribe(recipient, target)
@@ -22,7 +16,7 @@ class Subscription < ApplicationRecord
       end
 
       subs = Subscription.find_by(user: recipient, answer: target)
-      subs.update(is_active: false) unless subs.nil?
+      subs&.destroy
     end
 
     def destruct(target)
@@ -46,7 +40,7 @@ class Subscription < ApplicationRecord
         return nil
       end
 
-      Subscription.where(answer: target, is_active: true).each do |subs|
+      Subscription.where(answer: target).each do |subs|
         next unless not subs.user == source.user
         Notification.notify subs.user, source
       end
diff --git a/spec/controllers/ajax/subscription_controller_spec.rb b/spec/controllers/ajax/subscription_controller_spec.rb
index 6a14aeee..23499211 100644
--- a/spec/controllers/ajax/subscription_controller_spec.rb
+++ b/spec/controllers/ajax/subscription_controller_spec.rb
@@ -33,7 +33,7 @@ describe Ajax::SubscriptionController, :ajax_controller, type: :controller do
         context "when subscription does not exist" do
           it "creates a subscription on the answer" do
             expect { subject }.to(change { answer.subscriptions.count }.by(1))
-            expect(answer.subscriptions.where(is_active: true).map { |s| s.user.id }.sort).to eq([answer_user.id, user.id].sort)
+            expect(answer.subscriptions.map { |s| s.user.id }.sort).to eq([answer_user.id, user.id].sort)
           end
 
           include_examples "returns the expected response"
@@ -44,7 +44,7 @@ describe Ajax::SubscriptionController, :ajax_controller, type: :controller do
 
           it "does not modify the answer's subscriptions" do
             expect { subject }.to(change { answer.subscriptions.count }.by(0))
-            expect(answer.subscriptions.where(is_active: true).map { |s| s.user.id }.sort).to eq([answer_user.id, user.id].sort)
+            expect(answer.subscriptions.map { |s| s.user.id }.sort).to eq([answer_user.id, user.id].sort)
           end
 
           include_examples "returns the expected response"
@@ -105,8 +105,8 @@ describe Ajax::SubscriptionController, :ajax_controller, type: :controller do
           before(:each) { Subscription.subscribe(user, answer) }
 
           it "removes an active subscription from the answer" do
-            expect { subject }.to(change { answer.subscriptions.where(is_active: true).count }.by(-1))
-            expect(answer.subscriptions.where(is_active: true).map { |s| s.user.id }.sort).to eq([answer_user.id].sort)
+            expect { subject }.to(change { answer.subscriptions.count }.by(-1))
+            expect(answer.subscriptions.map { |s| s.user.id }.sort).to eq([answer_user.id].sort)
           end
 
           include_examples "returns the expected response"
@@ -123,7 +123,7 @@ describe Ajax::SubscriptionController, :ajax_controller, type: :controller do
 
           it "does not modify the answer's subscriptions" do
             expect { subject }.to(change { answer.subscriptions.count }.by(0))
-            expect(answer.subscriptions.where(is_active: true).map { |s| s.user.id }.sort).to eq([answer_user.id].sort)
+            expect(answer.subscriptions.map { |s| s.user.id }.sort).to eq([answer_user.id].sort)
           end
 
           include_examples "returns the expected response"