Skip to content
New issue

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

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

Already on GitHub? # to your account

Add "order nulls last" global and per column options #279

Merged
merged 4 commits into from
May 5, 2018

Conversation

natebird
Copy link
Contributor

  • Allows for nulls to be ordered last using a global configuration option using config.nulls_last = true in the ajax_datatables_rails.rb file.
  • Allows for nulls to be ordered last on individual columns by adding nulls_last: true to the view_columns method in the datatable class.

@natebird
Copy link
Contributor Author

active_record_filter_records_spec.rb:265 is failing on the master branch for me without any changes. The changes in this PR shouldn't have any effect on that test.

@natebird
Copy link
Contributor Author

Any update on getting this merged?

@ikaul
Copy link

ikaul commented May 1, 2018

Will this be merged to the master branch any time soon?
I was looking forward to add this to my project.

@n-rodriguez
Copy link
Member

Well... I'd love to, but it seems it breaks tests for Rails prior to 5.x

@ikaul
Copy link

ikaul commented May 1, 2018

Ah! Bummer.

@natebird
Copy link
Contributor Author

natebird commented May 1, 2018

It's failing the Oracle tests after merging in some fixes from the master branch. @n-rodriguez any ideas on what is causing Oracle to fail? It looks like its just a missing dependency or something.

@n-rodriguez
Copy link
Member

Yeah it's normal. Oracle tests depends on my own login and password to download the archive.
The credentials are injected via environment variables but only on the master branch and on the main repo for security reasons.

@n-rodriguez
Copy link
Member

@@ -83,6 +83,9 @@ def get_raw_records
end
end

# class ComplexDatatableHash < ComplexDatatable
# end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can remove it? ;)

@@ -10,13 +10,17 @@
let(:column) { datatable.datatable.columns.first }

before do
datatable.params[:columns] = {'0'=>{'data'=>'username', 'name'=>'', 'searchable'=>'true', 'orderable'=>'true', 'search'=>{'value'=>'searchvalue', 'regex'=>'false'}}}
datatable.params[:columns] = {'0'=>{'data'=>'username', 'name'=>'', 'searchable'=>'true', 'orderable'=>'true', 'search'=>{'value'=>'searchvalue', 'regex'=>'false'}, 'nulls_last'=>'true'}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we really need this since it shoud be set in the Datatable class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is being used. I think that is simulating setting it on the global variable. It's been a while since I've looked at this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing that fails the test…

@n-rodriguez n-rodriguez merged commit 5d40427 into jbox-web:master May 5, 2018
@n-rodriguez
Copy link
Member

Thank you!

@ikaul
Copy link

ikaul commented May 9, 2018

Awesome, Thank you guys for working on this and getting this merged. This will help me make my datatables more presentable.

@natebird natebird deleted the order_nulls_last_new branch January 16, 2019 18:42
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants