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

Tree: filter on a property of node values #2861

Closed
f-r-a-n-s opened this issue Oct 24, 2017 · 8 comments

Comments

Projects
None yet
7 participants
@f-r-a-n-s
Copy link

commented Oct 24, 2017

1) Environment

  • PrimeFaces version: 6.1.7
  • Does it work on the newest released PrimeFaces version? No
  • Does it work on the newest sources in GitHub? No
  • Application server + version: Tomcat 8.5.20
  • Affected browsers: Firefox, Chrome, Edge

2) Expected behavior

When building a <p:tree> of nodes whose data is a bean, not a String, I expect to be able to apply filtering, say startsWith, on a property of these beans, as documented in the user manual:

<p:tree value="#{treeBean.root}" var="node" filterBy="#{node.name}">
    <p:treeNode>
        <h:outputText value="#{node}"/>
    </p:treeNode>
</p:tree>

3) Actual behavior

Filtering only works as expected when node values are String. When using a bean property, it returns no result.

4) Steps to reproduce

See example.

5) Sample XHTML

<p:tree value="#{treeBean.root}" var="node" filterBy="#{node.name}">
    <p:treeNode>
        <h:outputText value="#{node.name}"/>
    </p:treeNode>
</p:tree>

6) Sample bean

public class TreeBean {

    private TreeNode root;

    public TreeBean() {
        this.root = new DefaultTreeNode();
        this.root.getChildren().add(new DefaultTreeNode(new Fruit("Lemon")));
        this.root.getChildren().add(new DefaultTreeNode(new Fruit("Apple")));
        this.root.getChildren().add(new DefaultTreeNode(new Fruit("Cherry")));
        this.root.getChildren().add(new DefaultTreeNode(new Fruit("Banana")));
    }

    public TreeNode getRoot() {
        return root;
    }

    public class Fruit {

        private String name;

        public Fruit(final String name) {
            this.name = name;
        }

        public String getName() {
            return name;
        }

    }
}

7) Analysis

The expression passed in filterBy attribute seems to be ignored, or more precisely, it seems to be only used to trigger filtering or not, whatever the value of the expression is.
Then org.primefaces.model.filter.StartsWithFilterConstraint.applies() does an explicit toString() conversion of the node value before applying the filter (line 34):

return value.toString().toLowerCase(locale).startsWith(filterText);

This is true for several implementations of FilterConstraint.

And indeed, if I add a custom toString() method to my Fruit class, it works as expected:

        @Override
        public String toString() {
            return this.name;
        }
@melloware

This comment has been minimized.

Copy link
Contributor

commented Oct 31, 2017

Yes everything is expected for filtering to be on Strings or Objects that implement a toString method. How would you expect it to behave differently? Or should I say what was your original expected result before you added the toString() method?

Or are you saying that..
filterBy="#{node.name}"

Is actually only filtering on node and ignoring name like...
filterBy="#{node}"

@f-r-a-n-s

This comment has been minimized.

Copy link
Author

commented Nov 2, 2017

I expect that it evaluates the expression passed in filterBy attribute and applies the filter on that value (by calling toString() on that value if necessary).

What happens is that the expression in filterBy is just ignored, only its presence is checked.
Whether I use filterBy="#{node.name}", filterBy="#{node}", filterBy="#{primeFaces.rocks}" or filterBy="#{someRandomString}", I get the same result: the filter is applied on node data's toString() value (see org.primefaces.component.tree.TreeRenderer.encodeFilteredNodes()).

@melloware

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2017

Understood. then I definitely agree that is a defect.

@zek72

This comment has been minimized.

Copy link

commented Dec 11, 2017

Hi, I am facing the same issue. May I learn what is the status of this issue?

@melloware

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2017

It has been acknowledged as a defect but has not been fixed yet nor has any user submitted a PR to fix it yet.

@f-r-a-n-s f-r-a-n-s changed the title Tree: possibility to filter on a property of node values Tree: filter on a property of node values Jan 16, 2018

@kukel

This comment has been minimized.

Copy link

commented Jan 17, 2018

@melloware: Same is true for datable afaik. And it was by design I think. Better ask @cagataycivici since this has been changed a couple of times and it should NOT break backwards compatibility!!!

@Rapster

This comment has been minimized.

Copy link
Member

commented Feb 23, 2018

@mertsincan is the author of that feature, he may have a clue 😉 (I'm facing the same problem actually)

@Rapster

This comment has been minimized.

Copy link
Member

commented Feb 23, 2018

IMO, the problem is around here:

protected void encodeFilteredNodes(FacesContext context, Tree tree, TreeNode node, String filteredValue, Locale filterLocale)
            throws IOException {
        
        int childCount = node.getChildCount();
        for (int i = 0; i < childCount; i++) {
            TreeNode childNode = node.getChildren().get(i);
            FilterConstraint filterConstraint = this.getFilterConstraint(tree);
            if (filterConstraint.applies(childNode.getData(), filteredValue, filterLocale)) {
                tree.getFilteredRowKeys().add(childNode.getRowKey());
            }
            encodeFilteredNodes(context, tree, childNode, filteredValue, filterLocale);
        }
    }

Instead of childNode.getData(), it should be the ValueExpression filterBy used for the filter constraint

Rapster added a commit to Rapster/primefaces that referenced this issue Feb 23, 2018

tandraschko added a commit that referenced this issue Mar 12, 2018

Merge pull request #3365 from Rapster/fix-2861
Fix #2861 - Tree: filter on a property of node values

@tandraschko tandraschko added this to the 6.3 milestone Mar 12, 2018

@mertsincan mertsincan added the 6.2.20 label Apr 22, 2019

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.