primefaces-extensions / primefaces-extensions.github.com

Organization repo, only for homepage, wiki and issue tracker
https://primefaces-extensions.github.io/
68 stars 22 forks source link

Sheet: Filtering and dynamic columns not working together #713

Closed blutorange closed 5 years ago

blutorange commented 5 years ago

PrimeFaces Extensions: 7.0.2

Problem

The showcase gives an example for how to do sorting and another examples illustrating dynamic columns. When I try to use both features together, it does not work properly anymore:

Say the the user want sort sort by the 4th column. Clicking on the 4th column however results the rows getting sorted by the the first column. This is because internally, the column to sort by is searched by the literal value of the sortBy-attribute.

A basic example to illustrate how this can be reproduced:

<pe:sheet id="catalog" value="#{model.sheetRows}"
    filteredValue="#{myBean.model.filteredSheetRows}"
    var="row" rowKey="#{row.rowId}">
    <c:forEach items="#{myBean.model.headers}" var="header" varStatus="status">
        <pe:sheetcolumn colType="text" id="col-#{status.index}"
            value="#{row.cells[status.index].value}"
            sortBy="#{row.cells[status.index].value}" />
    </c:forEach>
</pe:sheet>

The same issue can probably be reproduced when you activate sorting for the example with dynamic columns in the showcase.

The user clicks on the second column:

image

But it sorts by the first column:

image

Cause

This is caused by how the column to sort by is searched for on the server. In Sheet#getSortColRenderIndex we find the following (simplified):

        final ValueExpression veSortBy = getValueExpression(PropertyKeys.sortBy.name());
        final String sortByExp = veSortBy.getExpressionString();
        int colIdx = 0;
        for (final SheetColumn column : getColumns()) {
            final ValueExpression veCol = column.getValueExpression(PropertyKeys.sortBy.name());
           if (veCol.getExpressionString().equals(sortByExp)) {
                return colIdx;
            }
            colIdx++;
        }
        return -1;

It returns the first column with a sortBy attribute that matches the stored selection. In the case of dynamic columns, this attribute is the same for all columns, namely:

#{row.cells[status.index].value}

So it always returns the first column, which results in the observed behavior.

Solution

I think this could be solved by storing the ID of the column instead of the value expression. When searching for the column, the value expression should still be tried first to preserve compatibility with older versions. I'm trying this now and if it works out, I'll submit a PR.

melloware commented 5 years ago

Great deep dive! A PR would be much appreciated.

blutorange commented 5 years ago

I made a PR here: https://github.com/primefaces-extensions/core/pull/84 Preserving backwards compatibility made it a bit harder, but it should work now.

melloware commented 5 years ago

Much appreciated and great work.