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

Use built-in methods for nulls_first, nulls_last #1376

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stereobooster
Copy link
Contributor

@stereobooster stereobooster commented Nov 12, 2022

PR for #1373. PR not ready for merge - let's discuss. I didn't rename options or tests for now

Summary:

  • Works for all databases except MySQL in Rails 7
  • Works for PostgreSQL in Rails 6

@scarroll32
Copy link
Member

Related to #1184

@deivid-rodriguez
Copy link
Contributor

@stereobooster Makes sense to me to use built-in Rails methods, better than the other two options you presented at #1373. Can you do the necessary renamings/test fixes?

@stereobooster
Copy link
Contributor Author

@deivid-rodriguez yes I can. The problem is that it is kind of braking change. What shall I do about it?

@deivid-rodriguez
Copy link
Contributor

Can you explain the breaking change more? Is it because some supported Rails versions don't yet have these?

@stereobooster
Copy link
Contributor Author

stereobooster commented Jan 31, 2023

Option is called postgres_fields_sort_option, but it would work for other databases as well. Shall I leave it as is?

@deivid-rodriguez
Copy link
Contributor

Oh, I didn't notice the option name. We'll be releasing Ransack 3 soon, we I'd rename the option since otherwise it's going to be very confusing.

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.

None yet

3 participants