Skip to content

Commit 2839acf

Browse files
committed
Fix regression introduced with 235eae3. Closes #701.
See the comments in the code for more information. TODO: Add test coverage for this.
1 parent 3c26986 commit 2839acf

File tree

1 file changed

+20
-1
lines changed

1 file changed

+20
-1
lines changed

lib/ransack/nodes/condition.rb

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,29 @@ def combinator=(val)
127127
alias :m= :combinator=
128128
alias :m :combinator
129129

130+
131+
# == build_attribute
132+
#
133+
# This method was originally called from Nodes::Grouping#new_condition
134+
# only, without arguments, without #valid? checking, to build a new
135+
# grouping condition.
136+
#
137+
# After refactoring in 235eae3, it is now called from 2 places:
138+
#
139+
# 1. Nodes::Condition#attributes=, with +name+ argument passed or +name+
140+
# and +ransacker_args+. Attributes are included only if #valid?.
141+
#
142+
# 2. Nodes::Grouping#new_condition without arguments. In this case, the
143+
# #valid? conditional needs to be bypassed, otherwise nothing is
144+
# built. The `name.nil?` conditional below currently does this.
145+
#
146+
# TODO: Add test coverage for this behavior and ensure that `name.nil?`
147+
# isn't fixing issue #702 by introducing untested regressions.
148+
#
130149
def build_attribute(name = nil, ransacker_args = [])
131150
Attribute.new(@context, name, ransacker_args).tap do |attribute|
132151
@context.bind(attribute, attribute.name)
133-
self.attributes << attribute if attribute.valid?
152+
self.attributes << attribute if name.nil? || attribute.valid?
134153
if predicate && !negative?
135154
@context.lock_association(attribute.parent)
136155
end

0 commit comments

Comments
 (0)