Remove usages of `is_active`

This commit is contained in:
Karina Kwiatek 2023-03-04 20:42:51 +01:00
parent 51638d2eb2
commit 0132d7b251
4 changed files with 16 additions and 22 deletions

View File

@ -4,14 +4,14 @@ class Ajax::SubscriptionController < AjaxController
def subscribe def subscribe
params.require :answer params.require :answer
@response[:status] = :okay @response[:status] = :okay
state = Subscription.subscribe(current_user, Answer.find(params[:answer])).nil? result = Subscription.subscribe(current_user, Answer.find(params[:answer]))
@response[:success] = state == false @response[:success] = result.present?
end end
def unsubscribe def unsubscribe
params.require :answer params.require :answer
@response[:status] = :okay @response[:status] = :okay
state = Subscription.unsubscribe(current_user, Answer.find(params[:answer])).nil? result = Subscription.unsubscribe(current_user, Answer.find(params[:answer]))
@response[:success] = state == false @response[:success] = result&.destroyed? || false
end end
end end

View File

@ -8,7 +8,7 @@ class Comment < ApplicationRecord
validates :content, length: { maximum: 512 } validates :content, length: { maximum: 512 }
after_create do after_create do
Subscription.subscribe self.user, answer, false Subscription.subscribe self.user, answer
Subscription.notify self, answer Subscription.notify self, answer
end end

View File

@ -3,17 +3,11 @@ class Subscription < ApplicationRecord
belongs_to :answer belongs_to :answer
class << self class << self
def for(target) def subscribe(recipient, target)
Subscription.where(answer: target)
end
def subscribe(recipient, target, force = true)
existing = Subscription.find_by(user: recipient, answer: target) existing = Subscription.find_by(user: recipient, answer: target)
if existing.nil? return true if existing.present?
Subscription.new(user: recipient, answer: target).save!
elsif force Subscription.create!(user: recipient, answer: target)
existing.update(is_active: true)
end
end end
def unsubscribe(recipient, target) def unsubscribe(recipient, target)
@ -22,7 +16,7 @@ class Subscription < ApplicationRecord
end end
subs = Subscription.find_by(user: recipient, answer: target) subs = Subscription.find_by(user: recipient, answer: target)
subs.update(is_active: false) unless subs.nil? subs&.destroy
end end
def destruct(target) def destruct(target)
@ -46,7 +40,7 @@ class Subscription < ApplicationRecord
return nil return nil
end 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 next unless not subs.user == source.user
Notification.notify subs.user, source Notification.notify subs.user, source
end end

View File

@ -33,7 +33,7 @@ describe Ajax::SubscriptionController, :ajax_controller, type: :controller do
context "when subscription does not exist" do context "when subscription does not exist" do
it "creates a subscription on the answer" do it "creates a subscription on the answer" do
expect { subject }.to(change { answer.subscriptions.count }.by(1)) 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 end
include_examples "returns the expected response" 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 it "does not modify the answer's subscriptions" do
expect { subject }.to(change { answer.subscriptions.count }.by(0)) 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 end
include_examples "returns the expected response" include_examples "returns the expected response"
@ -105,8 +105,8 @@ describe Ajax::SubscriptionController, :ajax_controller, type: :controller do
before(:each) { Subscription.subscribe(user, answer) } before(:each) { Subscription.subscribe(user, answer) }
it "removes an active subscription from the answer" do it "removes an active subscription from the answer" do
expect { subject }.to(change { answer.subscriptions.where(is_active: true).count }.by(-1)) 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].sort) expect(answer.subscriptions.map { |s| s.user.id }.sort).to eq([answer_user.id].sort)
end end
include_examples "returns the expected response" 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 it "does not modify the answer's subscriptions" do
expect { subject }.to(change { answer.subscriptions.count }.by(0)) 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 end
include_examples "returns the expected response" include_examples "returns the expected response"