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: more than one dataTable within p:repeat (or ui:repeat) breaks selection #3812

Closed
lagar84 opened this issue Jun 22, 2018 · 19 comments
Labels
workaround A workaround has been provided

Comments

@lagar84
Copy link

lagar84 commented Jun 22, 2018

1) Environment

  • PrimeFaces version: 6.2.5
  • Does it work on the newest released PrimeFaces version? No
  • Does it work on the newest sources in GitHub? No
  • Application server + version: jetty from primefaces-test (not important)
  • Affected browsers: all (server side bug)
  • Related issue: Using datatable within ui:repeat breaks selection #929

2) Expected behavior

In a scenario having more than one dataTable inside p:repeat (or ui:repeat), selected itens on dataTable should work for all selected records on all tables.

3) Actual behavior

Only the values from the last table are processed

4) Steps to reproduce

Use this reproducer: https://github.com/lagar84/primefaces-test
In the reproducer:
Check all records on both tables (click on all checkboxes)
Click on Submit ALL button
BUG: Only the last table values are shown

5) Sample XHTML

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml"
      xmlns:ui="http://java.sun.com/jsf/facelets"
      xmlns:f="http://java.sun.com/jsf/core"
      xmlns:p="http://primefaces.org/ui"
      xmlns:h="http://java.sun.com/jsf/html">

    <h:head>
        <title>PrimeFaces Test</title>
    </h:head>
    <h:body>

        <h3>Steps to reproduce the problem:</h3>
        <ol>
        	<li>Check all records on both tables (click on all checkboxes)</li>
        	<li>Click on Submit ALL button</li>
        	<li>BUG: Only the last table values are shown</li>        
        </ol>
        
        <h:form>
        	<p:repeat var="par" value="#{testView.parents}">
	        	<p:dataTable id="parTable" value="#{par.children}" var="rec" rowKey="#{rec.id}"
			 		 selection="#{par.childrenSelected}" rowSelectMode="checkbox">
					 
					 <p:column selectionMode="multiple" style="width: 50px; text-align:center;" />
				     
				     <p:column headerText="ID" style="width: 100px;">
				     	<h:outputText value="#{rec.id}" />
				     </p:column>
				     <p:column headerText="Name" >
				     	<h:outputText value="#{rec.name}" />
				     </p:column>
					 
				</p:dataTable>
				<br/>
			</p:repeat>
			<br/>
			<br/>
			
			<p:commandButton value="Submit ALL" process="@form" update="@form"/>
			
			<br/>
			<br/>
			<h3>You selected:</h3>
			<p:repeat var="parSel" value="#{testView.parents}">
				<p:repeat var="chSel" value="#{parSel.childrenSelected}">
				
					<h:outputText value="#{chSel.id}" />
					<h:outputText value=" - " />
					<h:outputText value="#{chSel.name}" />
					<br/>
				
				</p:repeat>
			</p:repeat>
        
        </h:form>

    </h:body>
</html>

6) Sample bean

package org.primefaces.test;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;

import javax.annotation.PostConstruct;
import javax.faces.bean.ManagedBean;
import javax.faces.bean.ViewScoped;

@ManagedBean(name = "testView")
@ViewScoped
public class TestView implements Serializable {
       
    private List<RecordParent> parents = new ArrayList<>();
    
    @PostConstruct  
    public void init() {
                
        List<Record> children1 = new ArrayList<>();
				     children1.add(new Record(1, "Mojarra"));
				     children1.add(new Record(2, "Myfaces"));
        
		List<Record> children2 = new ArrayList<>();
			         children2.add(new Record(3, "Eclipselink"));
			         children2.add(new Record(4, "Hibernate"));
			         
		RecordParent parent1 = new RecordParent(children1);
		RecordParent parent2 = new RecordParent(children2);
		
		parents.add(parent1);
		parents.add(parent2);
        
    }

	public List<RecordParent> getParents() {
		return parents;
	}

	public void setParents(List<RecordParent> parents) {
		this.parents = parents;
	}

   
	
}
package org.primefaces.test;

import java.util.ArrayList;
import java.util.List;

public class RecordParent {

	private List<Record> children = new ArrayList<>();
	private List<Record> childrenSelected;
	
	public RecordParent(List<Record> children) {
		this.children = children;
	}

	public List<Record> getChildren() {
		return children;
	}

	public void setChildren(List<Record> children) {
		this.children = children;
	}

	public List<Record> getChildrenSelected() {
		return childrenSelected;
	}

	public void setChildrenSelected(List<Record> childrenSelected) {
		this.childrenSelected = childrenSelected;
	}
}
package org.primefaces.test;

public class Record {

	private Integer id;
	private String name;
	
	public Record() {
	}

	public Record(Integer id, String name) {
		super();
		this.id = id;
		this.name = name;
	}
	

	public Integer getId() {
		return id;
	}

	public void setId(Integer id) {
		this.id = id;
	}

	public String getName() {
		return name;
	}

	public void setName(String name) {
		this.name = name;
	}
	
	
}

datatable_selection_repeat_bug_reproducer

@tandraschko
Copy link
Member

Please try MyFaces and it's ui:repeat, if it works there, the p:repeat (copy of ui:repeat of mojarra, with same adjustments) is still buggy.

@lagar84
Copy link
Author

lagar84 commented Jun 22, 2018

I changed p:repeat to ui:repeat and ran the reproducer using both myfaces22 and myfaces23 profiles.
It did not make difference. (shows the same behavior using mojarra / p:repeat / ui:repeat)

@tandraschko tandraschko added the 🐞 defect Bug...Something isn't working label Jun 23, 2018
@tandraschko
Copy link
Member

all right, a deeper anylsis or PR is welcome then.

@Rapster
Copy link
Member

Rapster commented Jun 23, 2018

Looks like a state saving issue. Selection is not saved at that point. What exactly solved p:repeat? It does not say much on the documentation

@tandraschko
Copy link
Member

can't remember.
mojarra's ui:repeat has something like a check "if thisUIData/Repeat inside another thisUIData/Repeat"
and this check failed with PF data components, therefore we have forked

MyFaces ui:repeat also works without such a if statement and therefore there was never a problem

@lagar84
Copy link
Author

lagar84 commented Jun 23, 2018

Looking at related issue #929, user sheldon901 posted this: https://forum.primefaces.org/viewtopic.php?p=116259&sid=2729695e2c7da3bb130627966bd45034#p116259

In that topic he suggests a fix. I tried his fix by creating a DataTable subclass:

package org.primefaces.test;

import org.primefaces.component.datatable.DataTable;

public class DataTableSelectionFix extends DataTable {

	public java.lang.Object getSelection() {
	    Object o = getLocalSelection();
	    if (o == null)
	        return (java.lang.Object) getStateHelper().eval(PropertyKeys.selection, null);
	    else
	        return o;
	}

	public void setSelection(java.lang.Object _selection) {
	    getStateHelper().put(PropertyKeys.selection + this.getClientId(), _selection);
	}

	public Object getLocalSelection() {
	    return getStateHelper().get(PropertyKeys.selection + this.getClientId());
	}
}

and then I put it on faces-config.xml:

<?xml version='1.0' encoding='UTF-8'?>
<faces-config version="2.2"
              xmlns="http://xmlns.jcp.org/xml/ns/javaee"
              xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
              xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-facesconfig_2_2.xsd">
              
          
<component>
	<component-type>org.primefaces.component.DataTable</component-type>
	<component-class>org.primefaces.test.DataTableSelectionFix</component-class>
</component> 
              
</faces-config>

Well, it worked! But the problem is, I don't know why it worked. He just appended this.getClientId() on PropertyKeys.selection. Please, could someone with proper knowledge check if this is a good fix?

@Rapster
Copy link
Member

Rapster commented Jun 23, 2018

According to @cagataycivici p:repeat should fix the problem (but apparently it does not), we should ask him what makes him think that? It would be great as I think the same problem exists with p:tree

@tandraschko
Copy link
Member

That was the reason for introducing p:repeat: https://code.google.com/p/primefaces/issues/detail?id=7190

But if it doesn't work in MyFaces, i think the bug is somewhere in our UIData/TabPanel/something base.

@kukel
Copy link
Contributor

kukel commented Jun 29, 2018

Due to this, I personally always use the index from the varStatus and use value="#{testView.parents[statusVar.index].children}" instead of value="#{par.children} and the same for selection

<p:repeat var="par" value="#{testView.parents}" varStatus="statusVar">
    <p:dataTable id="parTable"value="#{testView.parents[statusVar.index].children}" 
        var="rec" rowKey="#{rec.id}" 
        selection="#{testView.parents[statusVar.index].childrenSelected}" rowSelectMode="checkbox">

And also use the same construct for selection. That does seem to work all the time.

@lagar84
Copy link
Author

lagar84 commented Jun 30, 2018

@kukel Thank's for sharing a workaround, but unfortunately it did not work on the reproducer. The result is the same as the original code. I tried this:

<p:repeat value="#{testView.parents}" varStatus="idx">
	        <p:dataTable id="parTable" value="#{testView.parents[idx.index].children}" 
                               var="rec" rowKey="#{rec.id}"
                               selection="#{testView.parents[idx.index].childrenSelected}"
                               rowSelectMode="checkbox">
...
					 

It fails in mojarra and myfaces.

@fattanasio
Copy link

Hi,
I have same problem. I have a datatable with row expansion and into expansion there is a selectionable datatable with checkbox.
On all expansion there is a command button
Selection is always null.

How fix this ?
Thank you
Senza titolo-1

@chovyy
Copy link

chovyy commented Aug 17, 2020

We had the same issue with PrimeFaces 7 and fixed it by setting a unique widgetVar for every table. Applied to the reproducer code from the initial post, it would be something like:

<p:repeat var="par" value="#{testView.parents}">
    <p:dataTable id="parTable" value="#{par.children}" var="rec" rowKey="#{rec.id}"
            selection="#{par.childrenSelected}" rowSelectMode="checkbox"
            widgetVar="table-#{par.id}">
[...]

You can see from the browser console that the AJAX requests already contain the wrong table ids when you don't do that. So, any fixes on the server side won't help here.

@melloware
Copy link
Member

@chovyy I just tried yours with the reproducer and only the 2nd table still gets submitted.
pf-3812.zip

Am I doing something different than you?

@chovyy
Copy link

chovyy commented Aug 19, 2020

I'm sorry, I wasn't able to get your project to run quickly and at the moment I don't have the time to dive further into it. From the code, it looks good. I just can tell, that setting the widgetVar fixed it for us.

@melloware
Copy link
Member

We have run into this issue in an application we are writing. I can confirm @lagar84 patch works with this reproducer:
pf-3812.zip

melloware added a commit to melloware/primefaces that referenced this issue Sep 30, 2020
@melloware melloware self-assigned this Sep 30, 2020
@melloware melloware added this to the 9.0 milestone Sep 30, 2020
@melloware melloware linked a pull request Sep 30, 2020 that will close this issue
@Rapster
Copy link
Member

Rapster commented Sep 30, 2020

I would have picture the fix in UIData#restoreDescendantState just like it is done for UIForm/EditableValueHolder no?

@melloware
Copy link
Member

You might be right there is a better way to fix it but I must admit i can't understand much that is going on in that UIData class.

@melloware
Copy link
Member

OK i was able to take UI Repeat out of the equation and reproduce the problem with dynamic TabView.
Both examples are included in this reproducer:
pf-3812.zip

UIRepeat: http://localhost:8080/primefaces-test/repeat.xhtml

TabView: http://localhost:8080/primefaces-test/tabview.xhtml

Shows the same behavior so its not related to UI Repeat the bug is definitely in the Datatable Selection.

@melloware melloware removed this from the 9.0 milestone Oct 14, 2020
@melloware
Copy link
Member

From PrimeTek: "We cannot use such repeating components with *:repeat. This requires massive changes in components. That's why JSTL forEach is the ideal solution for this kind of work"

The recommendation for c:forEach and the working example is attached here:
primefaces-test.zip

<c:forEach var="par" items="#{testView.parents}">
   //datatable
</c:forEach>
<p:tabView>
   <c:forEach var="par" items="#{testView.parents}">
      <p:tab title="Parent - #{par.id}">
         //datatable
      </p:tab>
   </c:forEach>
</p:tabView>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
workaround A workaround has been provided
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants