Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug that .order breaks the order of original records #197

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions lib/active_hash/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ def initialize(klass, all_records, query_hash = nil)
self.all_records = all_records
self.query_hash = query_hash
self.records_dirty = false
self.order_values = []
self
end

Expand Down Expand Up @@ -75,18 +76,23 @@ def pick(*column_names)
end

def reload
@records = filter_all_records_by_query_hash
filtered_records = filter_all_records_by_query_hash

if order_values.present?
filtered_records = filtered_records.dup if filtered_records.equal? all_records
order_by_args!(filtered_records)
end

@records = filtered_records
end

def order(*options)
check_if_method_has_arguments!(:order, options)
relation = where({})
return relation if options.blank?

processed_args = preprocess_order_args(options)
candidates = relation.dup

order_by_args!(candidates, processed_args)
candidates.order_values.concat preprocess_order_args(options)

candidates
end
Expand All @@ -95,12 +101,11 @@ def to_ary
records.dup
end


attr_reader :query_hash, :klass, :all_records, :records_dirty
attr_reader :query_hash, :klass, :all_records, :records_dirty, :order_values

private

attr_writer :query_hash, :klass, :all_records, :records_dirty
attr_writer :query_hash, :klass, :all_records, :records_dirty, :order_values

def records
if @records.nil? || records_dirty
Expand Down Expand Up @@ -164,8 +169,8 @@ def preprocess_order_args(order_args)
ary.map! { |e| e.split(/\W+/) }.reverse!
end

def order_by_args!(candidates, args)
args.each do |arg|
def order_by_args!(candidates)
order_values.each do |arg|
field, dir = if arg.is_a?(Hash)
arg.to_a.flatten.map(&:to_sym)
elsif arg.is_a?(Array)
Expand Down
11 changes: 11 additions & 0 deletions spec/active_hash/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,17 @@ class Region < ActiveHash::Base
expect(countries.first).to eq Country.find_by(name: "Canada")
expect(countries.second).to eq Country.find_by(name: "US")
end

it "doesn't break the order of original records" do
countries = Country.order(id: :desc)
expect(countries.first).to eq Country.find_by(name: "Mexico")
expect(countries.second).to eq Country.find_by(name: "Canada")
expect(countries.third).to eq Country.find_by(name: "US")

expect(Country.all.first).to eq Country.find_by(name: "US")
expect(Country.all.second).to eq Country.find_by(name: "Canada")
expect(Country.all.third).to eq Country.find_by(name: "Mexico")
end
end

describe "#method_missing" do
Expand Down