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

Conversation

ytakhs
Copy link

@ytakhs ytakhs commented Feb 9, 2020

This PR fixes #193

class Country < ActiveHash::Base
  field :name
  field :language
  self.data = [
    {:id => 1, :name => "US", :language => 'English'},
    {:id => 2, :name => "Canada", :language => 'English'},
    {:id => 3, :name => "Mexico", :language => 'Spanish'}
  ]
end

p Country.first.name
#=> "US"
Country.order(id: :desc)
p Country.first.name
#=> "Mexico"

#filter_all_records_by_query_hash returns all_records without copy when query_hash is empty, so Array#sort! breaks the order of original records.

Thanks.

@ytakhs ytakhs requested review from zilkey and syguer and removed request for zilkey February 9, 2020 13:50
@kbrock
Copy link
Collaborator

kbrock commented Apr 6, 2022

With this implementation, I wonder if we need reorder as well.

aside: this implementation is getting closer to active record's implementation of sorting. that is good

@littleforest
Copy link

I'm wondering what the status of this PR is? I've been affected by #193, and have kept using version 2.3 to avoid this bug, but I am now needing to upgrade to Ruby 3. Could this PR be moved along (or closed if it is not the correct way to solve the issue?)

@castlese
Copy link

castlese commented Feb 7, 2023

@littleforest how did you end up addressing your problem? We are in the same situation right now, reliant on an old branch but need to upgrade ruby

@littleforest
Copy link

littleforest commented Feb 7, 2023

@castlese I just redefined the order method in my model:

    def self.order(field)
      all.sort_by { |hash| hash[field] }
    end

Also, I do seem to still be using version 2.3 in one of my Ruby 3 apps, so you don't necessarily need to upgrade I don't think.

@kbrock
Copy link
Collaborator

kbrock commented Mar 27, 2023

Thank you for helping us with this solution. Going with #268 instead

@kbrock kbrock closed this Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants