search

tlaukkan / vaadin-lazyquerycontainer

Lazy Query Container is a lazy loading container implementation for accessing data from business services.
http://vaadin.com/directory#addon/117
39 stars 73 forks source link

Sorting Grid when backed by the LazyQueryContainer does nothing #71

Open mvysny opened 9 years ago

mvysny commented 9 years ago

Hi, thank you for your wonderful extension. My problem is that when using the LQC with the new Vaadin 7.4 Grid, when clicking the Grid column to sort by a column, the LQC's sort() method only invalidates stuff but the data is not polled to the Grid and the Grid is not refreshed with the new data. I am not sure where the problem lies (whether it is in the Grid or in LQC) but I will keep looking. I can also attach a small sample project which illustrates this issue. The intriguing part is that BeanContainer works correctly. The only difference to me is that BeanContainer fires the ItemSetChange event, while the LQC does not. However, Grid does not attach to the Container as a ItemSetChange listener AFAIK. I will try to explore this path though.

mvysny commented 9 years ago

It seems that the ItemSetChange event actually is used, not by the Grid directly, but by the RpcDataProviderExtension class. There are actually two events, item set change event and property set change notifier, but I think that only the ItemSetChange is actually important. I will try to patch the LQC sources and I will let you know.

mvysny commented 9 years ago

Patching LazyQueryContainer by adding notifyItemSetChanged(); to the sort() function helped. From a quick glance on the filter-related methods I would say that the filters will suffer from identical issue.

mvysny commented 9 years ago

I have verified that LazyQueryContainer.addContainerFilter(), LazyQueryContainer.removeContainerFilter() and LazyQueryContainer.removeAllContainerFilters() do not work properly with Grid unless notifyItemSetChanged(); is called.

tlaukkan commented 9 years ago

Hi

Nice work! It would be of great help if you could write a fix. Would you have time to make pull request?

Best regards, Tommi

mvysny commented 9 years ago

Sure, please let me clean up the code and study how to make a pull request :-) Also I will try the fix with the "old" Table, to check if nothing is broken by the fix. Best regards, Martin

mvysny commented 9 years ago

Hi, created a pull request here: https://github.com/tlaukkan/vaadin-lazyquerycontainer/pull/72 It's my first pull request so I hope I did everything correctly ;)

tlaukkan commented 8 years ago

A patch introduces behavior that refreshes container after each filter change which can run multiple unnecessary queries for example when multiple filters are removed. This behavior was introduced in a pull request and I missed the fact when accepting it. It is probably good time to think about adding functionality for mass removing filters in LazyQueryContainer API so that it disabled the filters at that point.