Skip to content

Commit

Permalink
fixes without_count last_page? behaviour (#1009)
Browse files Browse the repository at this point in the history
* fixes without_count last_page? behaviour
* attempt to handle older versions for arel
* typo
* should be checking limit class after super too
* stricter class checking
* fixes rails 4.{1,2}
* made adjustable? more readable
* regression tests for without_count
  • Loading branch information
montdidier authored and yuki24 committed Jan 17, 2020
1 parent 104d735 commit bfcf118
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 2 deletions.
Expand Up @@ -59,11 +59,11 @@ def load
else
@values[:limit] = limit_value + 1
# FIXME: this could be removed when we're dropping AR 4 support
@arel.limit = @values[:limit] if @arel && (Integer === @arel.limit)
@arel.limit = adjusted_limit(@values[:limit]) if @arel && adjustable?(@arel.limit)
super
@values[:limit] = limit_value - 1
# FIXME: this could be removed when we're dropping AR 4 support
@arel.limit = @values[:limit] if @arel && (Integer === @arel.limit)
@arel.limit = adjusted_limit(@values[:limit]) if @arel && adjustable?(@arel.limit)

if @records.any?
@records = @records.dup if (frozen = @records.frozen?)
Expand All @@ -75,6 +75,19 @@ def load
end
end

def adjustable?(obj)
obj.class == Integer || obj.class == Fixnum || (obj.class == Arel::Nodes::BindParam && obj.respond_to?(:value))
end

def adjusted_limit(limit)
case @arel.limit.class
when Integer, Fixnum
limit
when Arel::Nodes::BindParam
Arel::Nodes::BindParam.new(@arel.limit.value.with_cast_value(limit))
end
end

# The page wouldn't be the last page if there's "limit + 1" record
def last_page?
!out_of_range? && !@_has_next
Expand Down
Expand Up @@ -70,6 +70,22 @@ def self.shutdown
assert @users.out_of_range?
end

test 'regression: call arel first' do
@user = User.page(1).without_count
@user.arel

assert_equal false, @user.last_page?
end

test 'regression: call last page first' do
@user = User.page(1).without_count

@user.last_page?
@user.arel

assert_equal false, @user.last_page?
end

def assert_no_queries
subscriber = ActiveSupport::Notifications.subscribe 'sql.active_record' do
raise 'A SQL query is being made to the db:'
Expand Down

0 comments on commit bfcf118

Please sign in to comment.