DataTable: Global FilterFunction Broken

[melloware]

Describe the defect
Working on a showcase example to put back the old GlobalFilterFunction and the functionality has changed.

                <p:dataTable var="customer" value="#{dtFilterView.customers2}" widgetVar="customersTable2"
                    emptyMessage="No customers found with given criteria"
                    filteredValue="#{dtFilterView.filteredCustomers2}" filterBy="#{dtFilterView.filterBy}"
                    globalFilterFunction="#{dtFilterView.globalFilterFunction}">

                    <f:facet name="header">
                        <div class="p-text-left">
                            <h:outputText value="Search all fields using globalFilterFunction:" />
                            <p:inputText id="globalFilter" onkeyup="PF('customersTable2').filter()" style="width:150px"
                                placeholder="Enter keyword" />
                        </div>
                    </f:facet>

 public boolean globalFilterFunction(Object value, Object filter, Locale locale) {
        String filterText = (filter == null) ? null : filter.toString().trim().toLowerCase();
        if (LangUtils.isValueBlank(filterText)) {
            return true;
        }
        int filterInt = getInteger(filterText);

        Customer customer = (Customer) value;
        return customer.getName().toLowerCase().contains(filterText)
                || customer.getCountry().getName().toLowerCase().contains(filterText)
                || customer.getRepresentative().getName().toLowerCase().contains(filterText)
                || customer.getStatus().name().toLowerCase().contains(filterText)
                || customer.getActivity() < filterInt;
    }

its expecting the value to be the Customer POJO and right now it is just passing it each individual column value as a String which causes java.lang.ClassCastException: java.lang.String cannot be cast to org.primefaces.showcase.domain.Customer.

The old code was passing the table.RowData();

if (hasGlobalFilter && globalFilterFunction != null) {
                globalMatch = (Boolean) globalFilterFunction.invoke(elContext, new Object[]{table.getRowData(), globalFilterValue, filterLocale});
            }

But the new code is passing each individual column.

[Rapster]

😢

[christophs78]

We should add a integration-test for this ... whatever the expected correct behaviour is.

[melloware]

I am waiting to hear back on this review then will write one.

[melloware]

Integration Tests...

Global Filter: https://github.com/primefaces-extensions/primefaces-integration-tests/commit/64d6e5c246ab3735889142a77060e8d3ec6f4451

Global Filter Function: https://github.com/primefaces-extensions/primefaces-integration-tests/commit/18d79fca8a43d45aee64bdc0547c90ab4cec3dee

[tandraschko]

@melloware @sqores please dont merge such PRs without review before.
We must at least clarify what should be the correct behavior and refactor the code for it, not just hackin this behavior, without removing the old behavior.

[tandraschko]

this just leads to dead and bad code now.

[melloware]

Thomas this use case was wholesale broken and was missed in the refactor. Feel free to review but I believe the fixes are the correct one and I have integration tests to verify.

[tandraschko]

thats ok but we have deadcode now as FunctionFilterConstraint is actually unused

[melloware]

Are you sure? FunctionFilterConstraint is still used when a column has a filter like...

<p:column filterFunction=""...

Right?

[tandraschko]

not sure but the codepath in FilterMeta ln122 is now dead

[melloware]

Ok let me add some more tests and verify and then I can remove the dead code.

[melloware]

@tandraschko When you say ln122 I think you mean this line:

        FilterConstraint constraint = globalFilterFunction == null
                ? new GlobalFilterConstraint()
                : new FunctionFilterConstraint(globalFilterFunction);

If so I have Integration tests that prove both paths are followed.

GlobalFilterConstraint is used when you have a generic GloblalFilter like in my DataTable001Test.

<p:inputText id="globalFilter" onchange="PF('wgtTable').filter()" placeholder="Enter keyword"/>

FunctionFilterConstraint is used when you have either a custom GloblalFilter or a Column FilterFunction like in my DataTable019Test.

<p:dataTable id="datatable" globalFilterFunction="#{dataTable019.globalFilterFunction}">

<p:column field="type" filterFunction="#{dataTable019.columnFilterFunction}"/>

So I am still not sure there is any dead code and this wasn't the correct change to make. If you have more insight I am happy to write more Integration Tests

[tandraschko]

The method of ln122 creates the global FilterMeta and reuses globalFilterFunction.
If a globalFilterFunction is defined, you completely skip the global FilterMeta in FilterFeature

[melloware]

I see what you mean now. Removed this code and all Integration Tests are still passing.

[melloware]

Added new Integration test for column filter function as well:
https://github.com/primefaces-extensions/primefaces-integration-tests/commit/f7457b6f6d5f6cd32c7d70d08c4df107411fc72d#diff-25a6634263c1b1f6fc4697a04e2b9904ea4b042a89af59dc93ec1f5d44848a26