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"