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

Update README.md #431

Closed
wants to merge 1 commit into from
Closed

Update README.md #431

wants to merge 1 commit into from

Conversation

lorenzk
Copy link

@lorenzk lorenzk commented Sep 17, 2014

The documentation for sort_link is outdated. Separating associations' fields' with . does not work. Using _ does, at least in ransack 1.3.0.

The documentation for sort_link is outdated. Separating associations' fields' with `.` does not work. Using `_` does, at least in ransack 1.3.0.
@jonatack
Copy link
Contributor

@lorenzk Thank you, but please use [skip ci] for doc changes to avoid running the travis-ci test suite needlessly.

Could you please elaborate on "does not work"? I use sort_link as per the README in production and they should be working. If there is a case where they do not, we'll need a test and a fix.

Thanks!

@lorenzk
Copy link
Author

lorenzk commented Sep 17, 2014

Sorry, didn't know about [skip ci].

For me, using the 'table.column' notation to order by an associations columns results in silently having no ORDER BY clause at all. Using :table_column works as expected. Local columns work with both the :field and the 'field' notations.

@jonatack
Copy link
Contributor

Can you please provide the info in the Contributing Guide so I can try to reproduce your issue?

@jonatack
Copy link
Contributor

I just tried sort_link with :table_column (using Rails 4.2, postgresql 9.3,5, Ransack rails-4.2 branch) and see the following type of error message:

(PG::UndefinedColumn: ERROR:  column "employees_last_name"

Whereas with 'table.column' the ORDER BY is "employees.last_name" as it should be.

Tests are passing on Ransack 1.3.0. Seems like time to have a look at the Ransack test suite.

@lorenzk
Copy link
Author

lorenzk commented Sep 17, 2014

Interesting. I'll write up a detailed report after work.

@lorenzk
Copy link
Author

lorenzk commented Sep 17, 2014

Ok, here's a more detailed report from the rails console:

# These are my models
> Reservation.last.booking_option.title
=> "asdf"

# GOOD
> Reservation.search("s" => "booking_option_title").result.to_sql
=> "SELECT \"reservations\".* FROM \"reservations\" LEFT OUTER JOIN \"booking_options\" ON \"booking_options\".\"id\" = \"reservations\".\"booking_option_id\"  ORDER BY \"booking_options\".\"title\" ASC"

# BAD
> Reservation.search("s" => "booking_options.title").result.to_sql
=> "SELECT \"reservations\".* FROM \"reservations\""

The same thing happens when I use sort_link in a view. Adding .joins(:booking_option) or .includes(:booking_option) does not help.

I am using rails 4.0.9, ransack 1.3.0, postgresql 9.3.5, ruby 2.1.1p76

@avit
Copy link
Contributor

avit commented Sep 17, 2014

I'm thinking that the sort params would better match the format of the condition params ("association_attribute") and not as SQL-like ("table.column"), so @lorenzk's current behaviour seems right to me when used within the flat-hash ransack query format.

This seems more consistent to me:

Reservation.search(
  "booking_option_something_eq" => "abc",
  "s" => "booking_option_title"
)

Not sure what is intended to be correct. Is this related to ransortable_attributes?

@jonatack
Copy link
Contributor

@avit which way is working for your sort_links, and with which Ransack?

@jonatack
Copy link
Contributor

@lorenzk, @avit, agreed, in the rails console it works for me the same as you described.

But using the sort_link helper in a view works for me with 'table.column' and not with :table_column.

@lorenzk
Copy link
Author

lorenzk commented Sep 17, 2014

I just double checked the view helper while grepping the log (comments added later):

$ tail -f log/development.log | grep -o 'SELECT "reservations".*'

# no sort, as a baseline
SELECT "reservations".* FROM "reservations" LIMIT 10 OFFSET 0

# sort_link(@q, :booking_option_title)
SELECT "reservations".* FROM "reservations" LEFT OUTER JOIN "booking_options" ON "booking_options"."id" = "reservations"."booking_option_id" ORDER BY "booking_options"."title" ASC LIMIT 10 OFFSET 0

# sort_link(@q, 'booking_options.title')
SELECT "reservations".* FROM "reservations" LIMIT 10 OFFSET 0

Sorry for confusing you :-) Can I give you any other debugging infos that could help?

@jonatack
Copy link
Contributor

@avit ransortable_attributes returns an array of all the parent table attributes as strings and ransackable_associations an array of all the associations as strings.

@jonatack jonatack closed this Sep 17, 2014
@jonatack jonatack reopened this Sep 17, 2014
@jonatack
Copy link
Contributor

Sorry, didn't mean to close. Wrong button.

@jonatack jonatack closed this Sep 17, 2014
@jonatack jonatack reopened this Sep 17, 2014
@jonatack
Copy link
Contributor

@lorenzk could you provide your controller#index method and view search form code?

@avit
Copy link
Contributor

avit commented Sep 17, 2014

Ah yeah, I'm not using the ransack form helpers but rather ransack_ui gem... I'd have to look at what's coming in through there.

@lorenzk
Copy link
Author

lorenzk commented Sep 17, 2014

# reservations_controller.rb
class ReservationsController < ApplicationController
  def index
    @q = Reservation.search(params[:q])
    @reservations = @q.result.page(params[:page])
  end
end

There is no search form, I am just using the sort_link helper.

@jonatack
Copy link
Contributor

With Ransack 1.4 I am still unable to reproduce this issue with the sort_link view helper. Still reach the same conclusions I stated above.

@jonatack jonatack closed this Oct 2, 2014
@jonatack jonatack mentioned this pull request Nov 10, 2014
jonatack added a commit that referenced this pull request Aug 21, 2015
and provide the 'SQL string' version as an alternative in case of
difficulty.

This commit is in response to some users over time stating that the
docs were incorrect. I haven't been able to reproduce the issue. Let's
see what user feedback comes in. Closes #468, #450 and #431.
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.

3 participants