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

DataTable: multi-sort broken #4557

Closed
fanste opened this issue Feb 19, 2019 · 7 comments

Comments

Projects
None yet
4 participants
@fanste
Copy link
Contributor

commented Feb 19, 2019

1) Environment

  • PrimeFaces version: 7.0.RC3
  • Does it work on the newest released PrimeFaces version? Version? No (as i use the newest)
  • Does it work on the newest sources in GitHub? (Build by source -> https://github.com/primefaces/primefaces/wiki/Building-From-Source) No (as i use the newest)
  • Application server + version: WF10.1.Final, JSF 2.2
  • Affected browsers: All

2) Expected behavior

When I provide an initial list of SortMeta it is correctly applied in my lazy data model. My SQL query contains the ORDER BY-clause with my desired columns in the correct order.
This order must also be reflected client side to ensure correct sorting when you e.g. sort by an additional column.

3) Actual behavior

The sortMeta on client side is not provided by the server but created during bindSortEvents in JS. This simply adds all sorted columns to a list but in the order of the columns as they appear in the HTML. Not the real order used for sorting on the client side.

Page load:

  • SortMeta on server side: b ASC, a ASC
  • SortMeta on client side: a ASC, b ASC
    => does not match

When I now additional sort by c:

  • SortMeta on client AND server side: a ASC, b ASC, c ASC
    => a and b got swapped
    => Correct would be: b ASC, a ASC, c ASC

4) Steps to reproduce

5) Sample XHTML

<p:dataTable widgetVar="tblTest" value="#{myBean.list}" var="entry" sortMode="multiple" lazy="true">
    <f:event type="postAddToView" listener="#{myBean.doPopulateLayout}" />

    <p:column headerText="A" sortBy="#{entry.a}">#{entry.a}</p:column>
    <p:column headerText="B" sortBy="#{entry.b}">#{entry.b}</p:column>
    <p:column headerText="C" sortBy="#{entry.c}">#{entry.c}</p:column>
    <p:column headerText="D" sortBy="#{entry.d}">#{entry.d}</p:column>
</p:dataTable>

6) Sample bean

@Named
@ViewScoped
public class MyBean implements Serialzable {
    public void doPopulateLayout(ComponentSystemEvent e) {
        final DataTable dt = (DataTable) e.getComponent();
        final List<SortMeta> multiSortMeta = new ArrayList<>();

        SortMeta sortMeta = new SortMeta();
        sortMeta.setSortField("b");
        sortMeta.setSortOrder(SortOrder.ASCENDING);
        sortMeta.setSortBy(dt.getColumns().get(1));
        multiSortMeta.add(sortMeta);

        sortMeta = new SortMeta();
        sortMeta.setSortField("a");
        sortMeta.setSortOrder(SortOrder.ASCENDING);
        sortMeta.setSortBy(dt.getColumns().get(0));
        multiSortMeta.add(sortMeta);

        dt.setMultiSortMeta(multiSortMeta);
    }
}
@melloware

This comment has been minimized.

Copy link
Contributor

commented Feb 19, 2019

What was the previous version you tried before 7.0.RC3? I think this has been broken for a while because I noticed this behavior with 6.2 also.

@tandraschko tandraschko added the defect label Feb 19, 2019

@fanste

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2019

I've implemented the multi-sorting in my app using 6.3-SNAPSHOT. So I can't give you information based on earlier versions - I can't even test version prior 6.2 due to required code changes.

But the JS code looks (nearly) identical in 6.0 to 7.0.

A possible fix is on the way..

fanste added a commit to fanste/primefaces that referenced this issue Feb 21, 2019

fanste added a commit to fanste/primefaces that referenced this issue Feb 25, 2019

mertsincan added a commit that referenced this issue Apr 29, 2019

@mertsincan mertsincan added this to the 7.1 milestone Apr 29, 2019

mertsincan added a commit that referenced this issue Apr 29, 2019

@mertsincan

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

Refactoring JS codes ;) Could you please try it with your sample project?

@melloware

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

Looks like this might have introduced a new issue" #4819

@fanste

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

@mertsincan your refactored fix does not work. getSortMetaOrder only returns those columns that are sorted. Your for-loop will resolve columnHeaderId and columnHeader to undefined. Therefore no sortorder (~line 250) will be set which results in an error as soon as you try to sort by a not already sorted column (sortOrder=NaN).

My fix used the original sortableColumns and only resolved the index of the meta data against sortMetaOrder. That's why I did not use addSortMeta but added the items directly to the array.

If addSortMeta should be used only two (three) practical solution come into my mind:

  • sort sortableColumns using sortMetaOrder to bring the already sorted columns into the correct order.
  • sort sortMeta using sortMetaOrder after all sortableColumns have been processed
  • add a new parameter to addSortMeta that will insert the item at a specific position and resolve that position again using indexOf.

@melloware it seems that mertsincan already fixed that problem.

@mertsincan

This comment has been minimized.

Copy link
Member

commented May 20, 2019

It works fine for me. Please see;

image

Test code;

<p:dataTable value="#{myBean.list}" var="entry" sortMode="multiple" ...>
            <f:event type="postAddToView" listener="#{myBean.doPopulateLayout}" />

            <p:column id="col_A" headerText="A" sortBy="#{entry.a}">#{entry.a}</p:column>
            <p:column id="col_B" headerText="B" sortBy="#{entry.b}">#{entry.b}</p:column>
            <p:column headerText="C" sortBy="#{entry.c}">#{entry.c}</p:column>
            <p:column headerText="D" sortBy="#{entry.d}">#{entry.d}</p:column>
        </p:dataTable>

mertsincan added a commit that referenced this issue May 20, 2019

@mertsincan

This comment has been minimized.

Copy link
Member

commented May 20, 2019

Ahhh, you're right! Please see my changes; 211f177

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.