Refactor notifiable_available? method

This method was calling `check_availability`, which returned a boolean
value and caused a `Naming/PredicateMethod` when upgrading rubocop.

So we're changing the logic a little bit to remove the
`check_availability` method and merge the tests of `check_availability`
and `notifiable_available?` (which were almost identical) together.
This commit is contained in:
Javi Martín
2025-10-31 15:11:22 +01:00
parent 2fdfefe55d
commit 15f7632f3d
4 changed files with 23 additions and 41 deletions

View File

@@ -17,20 +17,20 @@ module Notifiable
end end
def notifiable_available? def notifiable_available?
case self.class.name notifiable_resource.present? &&
when "ProposalNotification" !(notifiable_resource.respond_to?(:hidden?) && notifiable_resource.hidden?) &&
check_availability(proposal) !(notifiable_resource.respond_to?(:retired?) && notifiable_resource.retired?)
when "Comment"
check_availability(commentable)
else
check_availability(self)
end
end end
def check_availability(resource) def notifiable_resource
resource.present? && case self.class.name
!(resource.respond_to?(:hidden?) && resource.hidden?) && when "ProposalNotification"
!(resource.respond_to?(:retired?) && resource.retired?) proposal
when "Comment"
commentable
else
self
end
end end
def linkable_resource def linkable_resource

View File

@@ -10,8 +10,7 @@ class Notification < ApplicationRecord
scope :recent, -> { order(id: :desc) } scope :recent, -> { order(id: :desc) }
scope :for_render, -> { includes(:notifiable) } scope :for_render, -> { includes(:notifiable) }
delegate :notifiable_title, :notifiable_body, :notifiable_available?, delegate :notifiable_title, :notifiable_body, :notifiable_available?, :linkable_resource,
:check_availability, :linkable_resource,
to: :notifiable, allow_nil: true to: :notifiable, allow_nil: true
def mark_as_read def mark_as_read

View File

@@ -126,27 +126,21 @@ describe ProposalNotification do
expect(notification.notifiable_available?).to be false expect(notification.notifiable_available?).to be false
end end
end
describe "check_availability" do
it "returns true if the resource is present, not hidden, nor retired" do
notification = create(:notification, notifiable: notifiable)
expect(notification.check_availability(proposal)).to be true
end
it "returns false if the resource is not present" do it "returns false if the resource is not present" do
notification = create(:notification, notifiable: notifiable) notification = create(:notification, notifiable: notifiable)
notifiable.proposal.really_destroy! notifiable.proposal.really_destroy!
expect(notification.check_availability(proposal)).to be false
expect(notification.notifiable_available?).to be false
end end
it "returns false if the resource is hidden" do it "returns false if the resource is hidden" do
notification = create(:notification, notifiable: notifiable) notification = create(:notification, notifiable: notifiable)
notifiable.proposal.hide notifiable.proposal.hide
expect(notification.check_availability(proposal)).to be false
expect(notification.notifiable_available?).to be false
end end
it "returns false if the resource is retired" do it "returns false if the resource is retired" do
@@ -155,7 +149,8 @@ describe ProposalNotification do
notifiable.proposal.update!(retired_at: Time.current, notifiable.proposal.update!(retired_at: Time.current,
retired_explanation: "Unfeasible reason explanation", retired_explanation: "Unfeasible reason explanation",
retired_reason: "unfeasible") retired_reason: "unfeasible")
expect(notification.check_availability(proposal)).to be false
expect(notification.notifiable_available?).to be false
end end
end end

View File

@@ -17,7 +17,7 @@ shared_examples "notifiable" do
end end
describe "notifiable_available?" do describe "notifiable_available?" do
it "returns true when it's a root comment and the notifiable is available" do it "returns true if the resource is present, not hidden, nor retired" do
notification = create(:notification, notifiable: notifiable) notification = create(:notification, notifiable: notifiable)
expect(notification.notifiable_available?).to be true expect(notification.notifiable_available?).to be true
@@ -30,7 +30,7 @@ shared_examples "notifiable" do
expect(notification.notifiable_available?).to be true expect(notification.notifiable_available?).to be true
end end
it "returns false when it's a root comment and the notifiable has been hidden" do it "returns false if the resource is hidden" do
notification = create(:notification, notifiable: notifiable) notification = create(:notification, notifiable: notifiable)
notifiable.hide notifiable.hide
@@ -48,24 +48,12 @@ shared_examples "notifiable" do
expect(notification.notifiable_available?).to be false expect(notification.notifiable_available?).to be false
end end
end
describe "check_availability" do
it "returns true if the resource is present, not hidden, nor retired" do
notification = create(:notification, notifiable: notifiable)
expect(notification.check_availability(notifiable)).to be true
end
it "returns false if the resource is not present" do it "returns false if the resource is not present" do
notification = create(:notification, notifiable: notifiable) notification = create(:notification, notifiable: notifiable)
notifiable.really_destroy! notifiable.really_destroy!
expect(notification.check_availability(notifiable)).to be false
end
it "returns false if the resource is not hidden" do expect(notification.notifiable_available?).to be false
notification = create(:notification, notifiable: notifiable)
notifiable.hide
expect(notification.check_availability(notifiable)).to be false
end end
it "returns false if the resource is retired" do it "returns false if the resource is retired" do
@@ -74,7 +62,7 @@ shared_examples "notifiable" do
if notifiable.respond_to?(:retired_at) if notifiable.respond_to?(:retired_at)
notifiable.update!(retired_at: Time.current, retired_reason: "unfeasible", notifiable.update!(retired_at: Time.current, retired_reason: "unfeasible",
retired_explanation: "Unfeasibility explanation ...") retired_explanation: "Unfeasibility explanation ...")
expect(notification.check_availability(notifiable)).to be false expect(notification.notifiable_available?).to be false
end end
end end
end end