Fix GridStaticSection communication to use column ids (#13334)

Change-Id: Ic5174543cab912ea8647b92445f33ec3d9fca366

5 files changed, 77 insertions, 77 deletions

diff --git a/client/src/com/vaadin/client/ui/grid/GridConnector.java b/client/src/com/vaadin/client/ui/grid/GridConnector.java
index 26649e6111..d34a57a4b3 100644
--- a/client/src/com/vaadin/client/ui/grid/GridConnector.java
+++ b/client/src/com/vaadin/client/ui/grid/GridConnector.java
@@ -45,7 +45,6 @@ import com.vaadin.client.ui.grid.renderers.AbstractRendererConnector;
 import com.vaadin.client.ui.grid.selection.AbstractRowHandleSelectionModel;
 import com.vaadin.client.ui.grid.selection.SelectionChangeEvent;
 import com.vaadin.client.ui.grid.selection.SelectionChangeHandler;
-import com.vaadin.client.ui.grid.selection.SelectionModel;
 import com.vaadin.client.ui.grid.selection.SelectionModelMulti;
 import com.vaadin.client.ui.grid.selection.SelectionModelNone;
 import com.vaadin.client.ui.grid.selection.SelectionModelSingle;
@@ -422,18 +421,10 @@ public class GridConnector extends AbstractHasComponentsConnector implements
         for (RowState rowState : state.rows) {
             GridStaticSection.StaticRow<?> row = section.appendRow();
 
-            int selectionOffset = 1;
-            if (getWidget().getSelectionModel() instanceof SelectionModel.None) {
-                selectionOffset = 0;
-            }
-
-            assert rowState.cells.size() == getWidget().getColumnCount()
-                    - selectionOffset;
-
-            int i = 0 + selectionOffset;
             for (CellState cellState : rowState.cells) {
-                GridStaticSection.StaticCell cell = row.getCell(getWidget()
-                        .getColumn(i++));
+                CustomGridColumn column = columnIdToColumn
+                        .get(cellState.columnId);
+                GridStaticSection.StaticCell cell = row.getCell(column);
                 switch (cellState.type) {
                 case TEXT:
                     cell.setText(cellState.text);
@@ -451,12 +442,10 @@ public class GridConnector extends AbstractHasComponentsConnector implements
                 }
             }
 
-            for (List<Integer> group : rowState.cellGroups) {
+            for (List<String> group : rowState.cellGroups) {
                 GridColumn<?, ?>[] columns = new GridColumn<?, ?>[group.size()];
-                i = 0;
-                for (Integer colIndex : group) {
-                    columns[i++] = getWidget().getColumn(
-                            selectionOffset + colIndex);
+                for (int i = 0; i < group.size(); ++i) {
+                    columns[i] = columnIdToColumn.get(group.get(i));
                 }
                 row.join(columns);
             }
diff --git a/server/src/com/vaadin/ui/components/grid/Grid.java b/server/src/com/vaadin/ui/components/grid/Grid.java
index 81a4598097..e29b1cf196 100644
--- a/server/src/com/vaadin/ui/components/grid/Grid.java
+++ b/server/src/com/vaadin/ui/components/grid/Grid.java
@@ -590,6 +590,10 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
 
         GridColumnState columnState = new GridColumnState();
         columnState.id = columnKeys.key(datasourcePropertyId);
+
+        GridColumn column = new GridColumn(this, columnState);
+        columns.put(datasourcePropertyId, column);
+
         getState().columns.add(columnState);
 
         for (int i = 0; i < getHeader().getRowCount(); ++i) {
@@ -600,8 +604,6 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
             getFooter().getRow(i).addCell(datasourcePropertyId);
         }
 
-        GridColumn column = new GridColumn(this, columnState);
-        columns.put(datasourcePropertyId, column);
         column.setHeaderCaption(String.valueOf(datasourcePropertyId));
 
         return column;
diff --git a/server/src/com/vaadin/ui/components/grid/GridStaticSection.java b/server/src/com/vaadin/ui/components/grid/GridStaticSection.java
index 80a39022e9..398ee6630e 100644
--- a/server/src/com/vaadin/ui/components/grid/GridStaticSection.java
+++ b/server/src/com/vaadin/ui/components/grid/GridStaticSection.java
@@ -24,6 +24,7 @@ import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 
 import com.vaadin.data.Container.Indexed;
 import com.vaadin.shared.ui.grid.GridStaticCellType;
@@ -63,6 +64,7 @@ abstract class GridStaticSection<ROWTYPE extends GridStaticSection.StaticRow<?>>
 
         protected void addCell(Object propertyId) {
             CELLTYPE cell = createCell();
+            cell.setColumnId(section.grid.getColumn(propertyId).getState().id);
             cells.put(propertyId, cell);
             rowState.cells.add(cell.getCellState());
         }
@@ -90,62 +92,6 @@ abstract class GridStaticSection<ROWTYPE extends GridStaticSection.StaticRow<?>>
         }
 
         /**
-         * Merges cells in a row
-         * 
-         * @param cells
-         *            The cells to be merged
-         * @return The first cell of the merged cells
-         */
-        protected CELLTYPE join(List<CELLTYPE> cells) {
-            assert cells.size() > 1 : "You cannot merge less than 2 cells together";
-
-            // Ensure no cell is already grouped
-            for (CELLTYPE cell : cells) {
-                if (getCellGroupForCell(cell) != null) {
-                    throw new IllegalStateException("Cell " + cell.getText()
-                            + " is already grouped.");
-                }
-            }
-
-            // Ensure continuous range
-            Iterator<CELLTYPE> cellIterator = this.cells.values().iterator();
-            CELLTYPE current = null;
-            int firstIndex = 0;
-
-            while (cellIterator.hasNext()) {
-                current = cellIterator.next();
-                if (current == cells.get(0)) {
-                    break;
-                }
-                firstIndex++;
-            }
-
-            for (int i = 1; i < cells.size(); ++i) {
-                current = cellIterator.next();
-
-                if (current != cells.get(i)) {
-                    throw new IllegalStateException(
-                            "Cell range must be a continous range");
-                }
-            }
-
-            // Create a new group
-            final ArrayList<CELLTYPE> cellGroup = new ArrayList<CELLTYPE>(cells);
-            cellGroups.add(cellGroup);
-
-            // Add group to state
-            List<Integer> stateGroup = new ArrayList<Integer>();
-            for (int i = 0; i < cells.size(); ++i) {
-                stateGroup.add(firstIndex + i);
-            }
-            rowState.cellGroups.add(stateGroup);
-            section.markAsDirty();
-
-            // Returns first cell of group
-            return cells.get(0);
-        }
-
-        /**
          * Merges columns cells in a row
          * 
          * @param properties
@@ -153,6 +99,8 @@ abstract class GridStaticSection<ROWTYPE extends GridStaticSection.StaticRow<?>>
          * @return The remaining visible cell after the merge
          */
         public CELLTYPE join(Object... properties) {
+            assert properties.length > 1 : "You need to merge at least 2 properties";
+
             List<CELLTYPE> cells = new ArrayList<CELLTYPE>();
             for (int i = 0; i < properties.length; ++i) {
                 cells.add(getCell(properties[i]));
@@ -169,9 +117,60 @@ abstract class GridStaticSection<ROWTYPE extends GridStaticSection.StaticRow<?>>
          * @return The remaining visible cell after the merge
          */
         public CELLTYPE join(CELLTYPE... cells) {
+            assert cells.length > 1 : "You need to merge at least 2 cells";
+
             return join(Arrays.asList(cells));
         }
 
+        protected CELLTYPE join(List<CELLTYPE> cells) {
+            for (CELLTYPE cell : cells) {
+                if (getCellGroupForCell(cell) != null) {
+                    throw new IllegalArgumentException("Cell already merged");
+                } else if (!this.cells.containsValue(cell)) {
+                    throw new IllegalArgumentException(
+                            "Cell does not exist on this row");
+                }
+            }
+
+            if (cellsInContinuousRange(cells)) {
+                List<String> columnGroup = new ArrayList<String>();
+                for (CELLTYPE cell : cells) {
+                    columnGroup.add(cell.getColumnId());
+                }
+                rowState.cellGroups.add(columnGroup);
+                cellGroups.add(cells);
+                return cells.get(0);
+            } else {
+                throw new IllegalArgumentException(
+                        "Cells are in invalid order or not in a contiunous range");
+            }
+        }
+
+        private boolean cellsInContinuousRange(List<CELLTYPE> mergeCells) {
+            Iterator<CELLTYPE> mergeCellIterator = mergeCells.iterator();
+            CELLTYPE mergeCell = mergeCellIterator.next();
+            boolean firstFound = false;
+            for (Entry<Object, CELLTYPE> entry : cells.entrySet()) {
+                // Go through all the cells until first to be merged is found
+                CELLTYPE currentCell = entry.getValue();
+                if (currentCell == mergeCell) {
+                    if (!mergeCellIterator.hasNext()) {
+                        // All the cells to be merged are found and they
+                        // were in continuous range
+                        return true;
+                    }
+                    mergeCell = mergeCellIterator.next();
+                    firstFound = true;
+                } else if (firstFound) {
+                    // We found the first cell already, but at least one cell
+                    // was not in a continuous range.
+                    return false;
+                }
+            }
+
+            return false;
+        }
+
         private List<CELLTYPE> getCellGroupForCell(CELLTYPE cell) {
             for (List<CELLTYPE> group : cellGroups) {
                 if (group.contains(cell)) {
@@ -194,6 +193,14 @@ abstract class GridStaticSection<ROWTYPE extends GridStaticSection.StaticRow<?>>
             this.row = row;
         }
 
+        private void setColumnId(String id) {
+            cellState.columnId = id;
+        }
+
+        private String getColumnId() {
+            return cellState.columnId;
+        }
+
         /**
          * Gets the row where this cell is.
          * 
diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/GridStaticSection.java b/server/tests/src/com/vaadin/tests/server/component/grid/GridStaticSection.java
index e89f6a8c6e..3b00867257 100644
--- a/server/tests/src/com/vaadin/tests/server/component/grid/GridStaticSection.java
+++ b/server/tests/src/com/vaadin/tests/server/component/grid/GridStaticSection.java
@@ -88,7 +88,7 @@ public class GridStaticSection {
                 mergeRow.getCell("zipCode"));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testJoinHeaderCellsIncorrectly() {
         final GridHeader section = grid.getHeader();
         HeaderRow mergeRow = section.prependRow();
diff --git a/shared/src/com/vaadin/shared/ui/grid/GridStaticSectionState.java b/shared/src/com/vaadin/shared/ui/grid/GridStaticSectionState.java
index c3c373b5af..3dde4989b8 100644
--- a/shared/src/com/vaadin/shared/ui/grid/GridStaticSectionState.java
+++ b/shared/src/com/vaadin/shared/ui/grid/GridStaticSectionState.java
@@ -37,6 +37,8 @@ public class GridStaticSectionState implements Serializable {
         public Connector connector = null;
 
         public GridStaticCellType type = GridStaticCellType.TEXT;
+
+        public String columnId;
     }
 
     public static class RowState implements Serializable {
@@ -44,7 +46,7 @@ public class GridStaticSectionState implements Serializable {
 
         public boolean defaultRow = false;
 
-        public List<List<Integer>> cellGroups = new ArrayList<List<Integer>>();
+        public List<List<String>> cellGroups = new ArrayList<List<String>>();
     }
 
     public List<RowState> rows = new ArrayList<RowState>();