mbrn / material-table

Datatable for React based on material-ui's table with additional features
https://material-table.com
MIT License
3.5k stars 1.02k forks source link

Ordering remote data #1323

Closed ayrtongomes closed 4 years ago

ayrtongomes commented 5 years ago

Is there a way to order data by server side?

e.g: I'm using OData and i need the name of the field and if the ordernation action is asc or desc to make a new data request and return the values ordered based on the ordering action.

How can i get this info using remote material table?

ogenodisho commented 5 years ago

Material Table will call your remote data handler with an object that contains searching, filtering, paging, and sorting information.

https://material-table.com/#/docs/features/remote-data

Open the source code of the first example and add a console.log(query); statement inside the handler to see all of the information that is provided to your remote data handler. You will see that there are two properties named orderBy and orderDirection, this is the sorting information that you can use in your data request.

OnkelTem commented 5 years ago

There is one caveat with the Remote Sort.

For some reason MT sorts data one more time, despite they're comming from the backend as a sorted set. And if your data ain't meant to be sorted alphabetically, you you have to write your customSort hook to sort already sorted date. I would personally call it a bug.

/summon @mbrn

gabriel-kohen-by commented 4 years ago

@OnkelTem and @mbrn is there any news on this inconsistency ? Can you give guidelines so folks can contribute a fix? Thanks for a great component!

Domino987 commented 4 years ago

Hi I looked also into this. In Datamanager, getRenderState sorts the data if the data changed, regardless of where it came from. So yes, if you sort the data in backend, it will be sorted again.

But you cannot remove it just like that, because it does a lot more than just sorting the data. If you look at the implementation, it also groups/treefies the data, if these features are on.

So we would need to look at every possibility, check if the data could be already sorted (e.g. backend) and only skip the sorting, if it is backend and already appropriately sorted.

I think to just sort a date column, that would be unnecessary, because the sorting is not expensive.

We should just improve the sort to include type date and datetime and sort accordingly.

What do you think.

sandeepkumar-vedam-by commented 4 years ago

@Domino987 Thanks for the comments :).

So my suggestion we can pass new property remoteSorting:true/false. We can skip the sorting if it is already sorted from backend by passing remoteSorting:true. Is it better approach?

Please suggest.

cc: @gabriel-kohen-jdas @OnkelTem

gabriel-kohen-by commented 4 years ago

Hi @Domino987 , getting large datasets in today's reality is a pretty common use case. You have to pass the proper filter/sorting/grouping/paging parameters for it work. Can you give guidance on the places @sandeepkumar-vedam-jdas would have to check on if remoteSorting=true?. I think we can come up with quality PR once we have a good idea.

sandeepkumar-vedam-by commented 4 years ago

Hi @Domino987

I made changes as per our discussion. I am validating promise and updating remoteSorting status in data-manager.js. I am skipping sorting based on remoteSorting status.

CC: @OnkelTem @mbrn @gabriel-kohen-jdas

Below are the changes. Please suggest if we need any changes.

material-table.js

material-table

data-manager.js

data-manager
Domino987 commented 4 years ago

Hi I do not think , that this will fix the issue. Let me think about a good approach. Can't you just check if this.props.data is a function or an array? To determine the status?

sandeepkumar-vedam-by commented 4 years ago

Hi I do not think , that this will fix the issue. Let me think about a good approach. Can't you just check if this.props.data is a function or an array? To determine the status?

Yes. We can check this.props.data. We can skip sorting if this.props.data is a function but checking promise is more generic way as we discussed before. It is skipping the sorting as per my observation.

Domino987 commented 4 years ago

Can you put that into a PR so me can load it and try it out? I think calling this.props.data(query) twice for the status and the data, will call the api endpoint twice. But I want to try it out, so make a PR and we can talk then.

sandeepkumar-vedam-by commented 4 years ago

Hi @Domino987 Raised PR at https://github.com/mbrn/material-table/pull/1844 as per our discussion. Please review the PR.

CC: @gabriel-kohen-jdas @jason-vail-jdas

sandeepkumar-vedam-by commented 4 years ago

@Domino987 @mbrn Could you please review PR https://github.com/mbrn/material-table/pull/1844 ?

CC: @gabriel-kohen-jdas

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. You can reopen it if it required.