ItEr15S11CUConfiguracionDeOrganizacionsDeTraballoConUnidadesTraballoItEr14S11: Implemented new specification using just FIXED_PERCENTAGE instead of all the other options.

Needed some reattchments that should be reviewed, highlighted with FIXME marks.

navalplanner-business/src/main/java/org/navalplanner/business/orders/entities/HoursGroup.java

@@ -4,7 +4,6 @@ import java.math.BigDecimal;
 import java.util.HashSet;
 import java.util.Set;
 
-import org.apache.commons.lang.builder.ToStringBuilder;
 import org.hibernate.validator.NotNull;
 import org.navalplanner.business.resources.entities.Criterion;
 import org.navalplanner.business.resources.entities.ICriterionType;
@@ -22,17 +21,32 @@ public class HoursGroup implements Cloneable {
     @NotNull
     private Integer workingHours = 0;
 
-    private BigDecimal percentage;
+    private BigDecimal percentage = new BigDecimal(0).setScale(2);
 
-    public enum HoursPolicies {
-        NO_FIXED, FIXED_HOURS, FIXED_PERCENTAGE
-    };
-
-    private HoursPolicies hoursPolicy = HoursPolicies.NO_FIXED;
+    private Boolean fixedPercentage = false;
 
     private Set<Criterion> criterions = new HashSet<Criterion>();
 
-    public void setWorkingHours(Integer workingHours) {
+    @NotNull
+    private OrderLine parentOrderLine;
+
+    /**
+     * Constructor for hibernate. Do not use!
+     */
+    public HoursGroup() {
+    }
+
+    public HoursGroup(OrderLine parentOrderLine) {
+        this.parentOrderLine = parentOrderLine;
+    }
+
+    public void setWorkingHours(Integer workingHours)
+            throws IllegalArgumentException {
+        if (workingHours < 0) {
+            throw new IllegalArgumentException(
+                    "Working hours shouldn't be neagtive");
+        }
+
         this.workingHours = workingHours;
     }
 
@@ -40,20 +54,29 @@ public class HoursGroup implements Cloneable {
         return workingHours;
     }
 
-    public void setPercentage(BigDecimal percentage) {
+    public void setPercentage(BigDecimal percentage)
+            throws IllegalArgumentException {
+        BigDecimal oldPercentage = this.percentage;
+
         this.percentage = percentage;
+
+        if (!parentOrderLine.isPercentageValid()) {
+            this.percentage = oldPercentage;
+            throw new IllegalArgumentException(
+                    "Total percentage should be less than 100%");
+        }
     }
 
     public BigDecimal getPercentage() {
         return percentage;
     }
 
-    public void setHoursPolicy(HoursPolicies hoursPolicy) {
-        this.hoursPolicy = hoursPolicy;
+    public void setFixedPercentage(Boolean fixedPercentage) {
+        this.fixedPercentage = fixedPercentage;
     }
 
-    public HoursPolicies getHoursPolicy() {
-        return hoursPolicy;
+    public Boolean isFixedPercentage() {
+        return this.fixedPercentage;
     }
 
     public void setCriterions(Set<Criterion> criterions) {
@@ -115,14 +138,23 @@ public class HoursGroup implements Cloneable {
         criterions.size();
     }
 
+    public void setParentOrderLine(OrderLine parentOrderLine) {
+        this.parentOrderLine = parentOrderLine;
+    }
+
+    public OrderLine getParentOrderLine() {
+        return parentOrderLine;
+    }
+
     public void makeTransientAgain() {
         // FIXME Review reattachment
         id = null;
         version = null;
     }
 
-    @Override
-    public String toString() {
-        return ToStringBuilder.reflectionToString(this);
+    public boolean isTransient() {
+     // FIXME Review reattachment
+        return id == null;
     }
+
 }

navalplanner-business/src/main/java/org/navalplanner/business/orders/entities/OrderElement.java

@@ -1,7 +1,9 @@
 package org.navalplanner.business.orders.entities;
 
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.hibernate.validator.NotNull;
 
@@ -111,4 +113,17 @@ public abstract class OrderElement {
         return id == null;
     }
 
+    public Set<HoursGroup> getTransientHoursGroups() {
+        // FIXME Review reattachment
+        Set<HoursGroup> transientHoursGroups = new HashSet<HoursGroup>();
+
+        for (HoursGroup hoursGroup : getHoursGroups()) {
+            if (hoursGroup.isTransient()) {
+                transientHoursGroups.add(hoursGroup);
+            }
+        }
+
+        return transientHoursGroups;
+    }
+
 }

navalplanner-business/src/main/java/org/navalplanner/business/orders/entities/OrderLine.java

@@ -6,8 +6,6 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
-import org.navalplanner.business.orders.entities.HoursGroup.HoursPolicies;
-
 public class OrderLine extends OrderElement {
 
     private Set<HoursGroup> hoursGroups = new HashSet<HoursGroup>();
@@ -44,13 +42,14 @@ public class OrderLine extends OrderElement {
     }
 
     public void addHoursGroup(HoursGroup hoursGroup) {
+        hoursGroup.setParentOrderLine(this);
         hoursGroups.add(hoursGroup);
-        recalculatePercentages(hoursGroups);
+        recalculateHoursGroups();
     }
 
     public void deleteHoursGroup(HoursGroup hoursGroup) {
         hoursGroups.remove(hoursGroup);
-        recalculatePercentages(hoursGroups);
+        recalculateHoursGroups();
     }
 
     /**
@@ -71,7 +70,7 @@ public class OrderLine extends OrderElement {
         }
 
         if (hoursGroups.isEmpty()) {
-            HoursGroup hoursGroup = new HoursGroup();
+            HoursGroup hoursGroup = new HoursGroup(this);
             hoursGroup.setWorkingHours(workHours);
             hoursGroup.setPercentage((new BigDecimal(1).setScale(2)));
 
@@ -94,8 +93,6 @@ public class OrderLine extends OrderElement {
      *
      * This method takes into account the different {@link HoursGroup} policies:
      *
-     * {@link HoursGroup} with FIXED_HOURS policy don't change.
-     *
      * If policy is FIXED_PERCENTAGE the new value is calculated for each
      * {@link HoursGroup} with this policy. Using round down in order to avoid
      * problems.
@@ -114,28 +111,17 @@ public class OrderLine extends OrderElement {
         Set<HoursGroup> newHoursGroups = new HashSet<HoursGroup>();
 
         // Divide HourGroup depending on policy
-        Set<HoursGroup> fixedHoursGroups = new HashSet<HoursGroup>();
         Set<HoursGroup> fixedPercentageGroups = new HashSet<HoursGroup>();
         Set<HoursGroup> noFixedGroups = new HashSet<HoursGroup>();
 
         for (HoursGroup hoursGroup : hoursGroups) {
-            switch (hoursGroup.getHoursPolicy()) {
-            case FIXED_HOURS:
-                fixedHoursGroups.add(hoursGroup);
-                break;
-            case FIXED_PERCENTAGE:
+            if (hoursGroup.isFixedPercentage()) {
                 fixedPercentageGroups.add(hoursGroup);
-                break;
-            case NO_FIXED:
-            default:
+            } else {
                 noFixedGroups.add(hoursGroup);
-                break;
             }
         }
 
-        // All the HourGroup with FIXED_HOURS will be kept without changes
-        newHoursGroups.addAll(fixedHoursGroups);
-
         // For every HourGroup with FIXED_PERCENTAGE, workingHours will be
         // calculated
         for (HoursGroup hoursGroup : fixedPercentageGroups) {
@@ -173,20 +159,21 @@ public class OrderLine extends OrderElement {
         newTotal = calculateTotalHours(newHoursGroups);
         if (newTotal.compareTo(workHours) < 0) {
             // Add a new HourGroup with the remaining hours
-            HoursGroup hoursGroup = new HoursGroup();
+            HoursGroup hoursGroup = new HoursGroup(this);
             hoursGroup.setWorkingHours(workHours - newTotal);
+
             newHoursGroups.add(hoursGroup);
         }
 
-        // Re-calculate percentages
-        recalculatePercentages(newHoursGroups);
-
         // Set the attribute with the new hours group calculated
         hoursGroups = newHoursGroups;
+
+        // Re-calculate percentages
+        recalculateHoursGroups();
     }
 
     /**
-     * Check if the desired total number of hours is valid taking into account
+     * Checks if the desired total number of hours is valid taking into account
      * {@link HoursGroup} policy restrictions.
      *
      * @param total
@@ -198,18 +185,10 @@ public class OrderLine extends OrderElement {
         Integer newTotal = 0;
 
         for (HoursGroup hoursGroup : hoursGroups) {
-            switch (hoursGroup.getHoursPolicy()) {
-            case FIXED_HOURS:
-                newTotal += hoursGroup.getWorkingHours();
-                break;
-            case FIXED_PERCENTAGE:
+            if (hoursGroup.isFixedPercentage()) {
                 newTotal += hoursGroup.getPercentage().multiply(
                         new BigDecimal(total).setScale(2)).toBigInteger()
                         .intValue();
-                break;
-            case NO_FIXED:
-            default:
-                break;
             }
         }
 
@@ -220,6 +199,31 @@ public class OrderLine extends OrderElement {
         return true;
     }
 
+    /**
+     * Checks if the percentage is or not valid. That means, if the pertentage
+     * of all {@link HoursGroup} with FIXED_PERCENTAGE isn't more than 100%.
+     *
+     * This method is called from setPercentage at {@link HoursGroup} class.
+     *
+     * @return true if the percentage is valid
+     */
+    public boolean isPercentageValid() {
+
+        BigDecimal newPercentage = new BigDecimal(0).setScale(2);
+
+        for (HoursGroup hoursGroup : hoursGroups) {
+            if (hoursGroup.isFixedPercentage()) {
+                newPercentage = newPercentage.add(hoursGroup.getPercentage());
+            }
+        }
+
+        if (newPercentage.compareTo(new BigDecimal(1).setScale(2)) > 0) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * Calculates the total number of working hours in a set of
      * {@link HoursGroup}.
@@ -248,7 +252,7 @@ public class OrderLine extends OrderElement {
     private Integer calculateTotalHoursNoFixed(Set<HoursGroup> hoursGroups) {
         Integer result = 0;
         for (HoursGroup hoursGroup : hoursGroups) {
-            if (hoursGroup.getHoursPolicy() == HoursPolicies.NO_FIXED) {
+            if (!hoursGroup.isFixedPercentage()) {
                 result += hoursGroup.getWorkingHours();
             }
         }
@@ -256,18 +260,55 @@ public class OrderLine extends OrderElement {
     }
 
     /**
-     * Re-calculates the percentages in a {@link HoursGroup} set, without modify
-     * the {@link HoursGroup} with policy FIXED_PERCENTAGE.
+     * Re-calculates the working hours and percentages in the {@link HoursGroup}
+     * set of the current {@link OrderLine}, taking into account the policy of
+     * each {@link HoursGroup}.
      *
-     * @param hoursGroups
-     *            A {@link HoursGroup} set
      */
-    private void recalculatePercentages(Set<HoursGroup> hoursGroups) {
+    public void recalculateHoursGroups() {
         Integer total = calculateTotalHours(hoursGroups);
         BigDecimal totalBigDecimal = new BigDecimal(total).setScale(2);
 
+        // For each HoursGroup with FIXED_PERCENTAGE, the workingHours are
+        // calculated
         for (HoursGroup hoursGroup : hoursGroups) {
-            if (hoursGroup.getHoursPolicy() != HoursPolicies.FIXED_PERCENTAGE) {
+            if (hoursGroup.isFixedPercentage()) {
+                Integer workingHours = hoursGroup.getPercentage().multiply(
+                        totalBigDecimal).toBigInteger().intValue();
+
+                hoursGroup.setWorkingHours(workingHours);
+            }
+        }
+
+        Integer newTotal = calculateTotalHours(hoursGroups);
+        // If the total was modified
+        if (!newTotal.equals(total)) {
+            Integer totalNoFixed = calculateTotalHoursNoFixed(hoursGroups);
+
+            // For each HoursGroup without FIXED_PERCENTAGE, the hours are
+            // proportionally shared
+            for (HoursGroup hoursGroup : hoursGroups) {
+                if (!hoursGroup.isFixedPercentage()) {
+                    Integer hours = hoursGroup.getWorkingHours();
+                    Integer newHours = (int) (((float) hours / totalNoFixed) * (total - (newTotal - totalNoFixed)));
+                    hoursGroup.setWorkingHours(newHours);
+                }
+            }
+        }
+
+        newTotal = calculateTotalHours(hoursGroups);
+        // If there's still some remaining hours
+        if (newTotal.compareTo(total) < 0) {
+            // Add a new HourGroup with the remaining hours
+            HoursGroup hoursGroup = new HoursGroup(this);
+            hoursGroup.setWorkingHours(total - newTotal);
+            hoursGroups.add(hoursGroup);
+        }
+
+        // Then the percentages for the HoursGroup without FIXED_PERCENTAGE are
+        // recalculated.
+        for (HoursGroup hoursGroup : hoursGroups) {
+            if (!hoursGroup.isFixedPercentage()) {
                 if (totalBigDecimal.equals(new BigDecimal(0).setScale(2))) {
                     hoursGroup.setPercentage(new BigDecimal(0).setScale(2));
                 } else {
@@ -281,16 +322,6 @@ public class OrderLine extends OrderElement {
         }
     }
 
-    /**
-     * Re-calculates the percentages in the {@link HoursGroup} set of the
-     * current {@link OrderLine}, without modify the {@link HoursGroup} with
-     * policy FIXED_PERCENTAGE.
-     *
-     */
-    public void recalculatePercentages() {
-        recalculatePercentages(hoursGroups);
-    }
-
     @Override
     public void forceLoadHourGroups() {
         for (HoursGroup hoursGroup : hoursGroups) {

navalplanner-business/src/main/resources/org/navalplanner/business/orders/entities/Orders.hbm.xml

@@ -44,8 +44,8 @@
         <joined-subclass name="OrderLine">
             <key column="ORDERELEMENTID"></key>
 
-            <set name="hoursGroups" access="field" cascade="all">
-                <key column="PARENT_ORDER_ELEMENT" not-null="false"></key>
+            <set name="hoursGroups" access="field" cascade="all" inverse="true">
+                <key column="PARENT_ORDER_LINE" not-null="false"></key>
                 <one-to-many class="HoursGroup" />
             </set>
         </joined-subclass>
@@ -59,7 +59,7 @@
 
         <property name="workingHours" access="field" />
         <property name="percentage" access="field" />
-        <property name="hoursPolicy" access="field" />
+        <property name="fixedPercentage" access="field" />
 
         <set name="criterions" table="CriterionHoursGroup"
             access="field" inverse="false">
@@ -68,5 +68,9 @@
                 class="org.navalplanner.business.resources.entities.Criterion" />
         </set>
 
+        <many-to-one name="parentOrderLine" column="PARENT_ORDER_LINE"
+            not-null="true"/>
+
     </class>
+
 </hibernate-mapping>

navalplanner-business/src/test/java/org/navalplanner/business/test/orders/services/OrderServiceTest.java

@@ -171,7 +171,7 @@ public class OrderServiceTest {
         leaf.setName("leaf");
         container.add(leaf);
         order.add(container);
-        HoursGroup hoursGroup = new HoursGroup();
+        HoursGroup hoursGroup = new HoursGroup(leaf);
         hoursGroup.setWorkingHours(3);
         leaf.addHoursGroup(hoursGroup);
         orderService.save(order);
@@ -206,17 +206,17 @@ public class OrderServiceTest {
     public void testManyToManyHoursGroupCriterionMapping() throws Exception {
         final Order order = createValidOrder();
 
-        OrderElement orderElement = new OrderLine();
-        orderElement.setName("Order element");
-        order.add(orderElement);
+        OrderLine orderLine = new OrderLine();
+        orderLine.setName("Order element");
+        order.add(orderLine);
 
-        HoursGroup hoursGroup = new HoursGroup();
+        HoursGroup hoursGroup = new HoursGroup(orderLine);
         hoursGroup.setWorkingHours(10);
-        HoursGroup hoursGroup2 = new HoursGroup();
+        HoursGroup hoursGroup2 = new HoursGroup(orderLine);
         hoursGroup2.setWorkingHours(5);
 
-        ((OrderLine) orderElement).addHoursGroup(hoursGroup);
-        ((OrderLine) orderElement).addHoursGroup(hoursGroup2);
+        orderLine.addHoursGroup(hoursGroup);
+        orderLine.addHoursGroup(hoursGroup2);
 
         Criterion criterion = Criterion.withNameAndType("Test"
                 + UUID.randomUUID().toString(), "test");

navalplanner-webapp/src/main/java/org/navalplanner/web/common/Util.java

@@ -6,6 +6,7 @@ import java.util.Date;
 import org.zkoss.zk.ui.Component;
 import org.zkoss.zk.ui.event.Event;
 import org.zkoss.zk.ui.event.EventListener;
+import org.zkoss.zk.ui.event.Events;
 import org.zkoss.zk.ui.event.InputEvent;
 import org.zkoss.zkplus.databind.DataBinder;
 import org.zkoss.zul.Checkbox;
@@ -288,7 +289,7 @@ public class Util {
     public static Checkbox bind(final Checkbox checkBox,
             final Getter<Boolean> getter, final Setter<Boolean> setter) {
         checkBox.setChecked(getter.get());
-        checkBox.addEventListener("onChange", new EventListener() {
+        checkBox.addEventListener(Events.ON_CHECK, new EventListener() {
 
             @Override
             public void onEvent(Event event) throws Exception {

navalplanner-webapp/src/main/java/org/navalplanner/web/orders/OrderCRUDController.java

@@ -2,6 +2,7 @@ package org.navalplanner.web.orders;
 
 import java.util.List;
 
+import org.apache.commons.logging.LogFactory;
 import org.navalplanner.business.common.exceptions.ValidationException;
 import org.navalplanner.business.orders.entities.IOrderLineGroup;
 import org.navalplanner.business.orders.entities.Order;
@@ -16,10 +17,14 @@ import org.zkoss.zul.api.Window;
 
 /**
  * Controller for CRUD actions <br />
+ *
  * @author Óscar González Fernández <ogonzalez@igalia.com>
  */
 public class OrderCRUDController extends GenericForwardComposer {
 
+    private static final org.apache.commons.logging.Log LOG = LogFactory
+            .getLog(OrderCRUDController.class);
+
     private IOrderModel orderModel;
 
     private IMessagesForUser messagesForUser;
@@ -113,8 +118,7 @@ public class OrderCRUDController extends GenericForwardComposer {
         orderModel.remove(order);
         hideConfirmingWindow();
         Util.reloadBindings(listWindow);
-        messagesForUser.showMessage(Level.INFO, "removed "
-                + order.getName());
+        messagesForUser.showMessage(Level.INFO, "removed " + order.getName());
     }
 
     public void goToCreateForm() {
@@ -134,17 +138,17 @@ public class OrderCRUDController extends GenericForwardComposer {
         orderElementController.doAfterCompose(comp
                 .getFellow("editOrderElementPopup"));
 
-        setupOrderElementTreeController(comp, "editWindow", orderElementController);
-        setupOrderElementTreeController(comp, "createWindow", orderElementController);
+        setupOrderElementTreeController(comp, "editWindow",
+                orderElementController);
+        setupOrderElementTreeController(comp, "createWindow",
+                orderElementController);
     }
 
     private void setupOrderElementTreeController(Component comp, String window,
-            OrderElementController orderElementController)
-            throws Exception {
+            OrderElementController orderElementController) throws Exception {
         OrderElementTreeController controller = new OrderElementTreeController(
                 orderModel, orderElementController);
-        controller
-                .doAfterCompose(comp.getFellow(window).getFellow(
+        controller.doAfterCompose(comp.getFellow(window).getFellow(
                 "orderElementTree"));
     }
 

navalplanner-webapp/src/main/java/org/navalplanner/web/orders/OrderElementController.java

@@ -11,7 +11,6 @@ import org.navalplanner.business.orders.entities.Order;
 import org.navalplanner.business.orders.entities.OrderElement;
 import org.navalplanner.business.orders.entities.OrderLine;
 import org.navalplanner.business.orders.entities.OrderLineGroup;
-import org.navalplanner.business.orders.entities.HoursGroup.HoursPolicies;
 import org.navalplanner.business.resources.entities.Criterion;
 import org.navalplanner.business.resources.entities.ICriterionType;
 import org.navalplanner.web.common.Util;
@@ -21,6 +20,7 @@ import org.zkoss.zk.ui.event.Event;
 import org.zkoss.zk.ui.event.EventListener;
 import org.zkoss.zk.ui.event.Events;
 import org.zkoss.zk.ui.util.GenericForwardComposer;
+import org.zkoss.zul.Checkbox;
 import org.zkoss.zul.Constraint;
 import org.zkoss.zul.Decimalbox;
 import org.zkoss.zul.Intbox;
@@ -30,6 +30,7 @@ import org.zkoss.zul.Listheader;
 import org.zkoss.zul.Listitem;
 import org.zkoss.zul.ListitemRenderer;
 import org.zkoss.zul.Popup;
+import org.zkoss.zul.Vbox;
 import org.zkoss.zul.api.Listhead;
 
 /**
@@ -39,6 +40,9 @@ import org.zkoss.zul.api.Listhead;
  */
 public class OrderElementController extends GenericForwardComposer {
 
+    /**
+     * {@link IOrderElementModel} with the data needed for this controller
+     */
     private IOrderElementModel model;
 
     /**
@@ -46,11 +50,6 @@ public class OrderElementController extends GenericForwardComposer {
      */
     private Popup popup;
 
-    /**
-     * Model of the {@link HoursGroup} list
-     */
-    private List<HoursGroup> hoursGroupsModel;
-
     /**
      * {@link Listitem} for every {@link HoursGroup}
      */
@@ -61,6 +60,9 @@ public class OrderElementController extends GenericForwardComposer {
      */
     private Listbox hoursGroupsListbox;
 
+    /**
+     * Set of selected {@link ICriterionType} just used in the controller
+     */
     private Set<ICriterionType<?>> selectedCriterionTypes = new HashSet<ICriterionType<?>>();
 
     public OrderElement getOrderElement() {
@@ -71,14 +73,17 @@ public class OrderElementController extends GenericForwardComposer {
         return model.getOrderElement();
     }
 
-    public List<HoursGroup> getHoursGroupsModel() {
-        return hoursGroupsModel;
-    }
-
     public HoursGroupListitemRender getRenderer() {
         return renderer;
     }
 
+    public List<HoursGroup> getHoursGroups() {
+        if (model == null) {
+            return new ArrayList<HoursGroup>();
+        }
+
+        return model.getOrderElement().getHoursGroups();
+    }
 
     @Override
     public void doAfterCompose(Component comp) throws Exception {
@@ -101,8 +106,6 @@ public class OrderElementController extends GenericForwardComposer {
 
         final OrderElement orderElement = model.getOrderElement();
 
-        this.hoursGroupsModel = orderElement.getHoursGroups();
-
         // If is a container
         if (orderElement instanceof OrderLineGroup) {
             // Disable fields just used in the OrderLine
@@ -175,13 +178,12 @@ public class OrderElementController extends GenericForwardComposer {
      * The {@link OrderElement} should be a {@link OrderLine}
      */
     public void addHoursGroup() {
-        HoursGroup hoursGroup = new HoursGroup();
+        OrderLine orderLine = (OrderLine) getOrderElement();
 
-        OrderElement orderElement = getOrderElement();
+        HoursGroup hoursGroup = new HoursGroup(orderLine);
 
-        ((OrderLine) orderElement).addHoursGroup(hoursGroup);
+        orderLine.addHoursGroup(hoursGroup);
 
-        this.hoursGroupsModel = orderElement.getHoursGroups();
         Util.reloadBindings(popup);
     }
 
@@ -201,10 +203,12 @@ public class OrderElementController extends GenericForwardComposer {
                     .getValue());
         }
 
-        this.hoursGroupsModel = orderElement.getHoursGroups();
         Util.reloadBindings(popup);
     }
 
+    /**
+     * Toggle visibility of the selectCriterions {@link Vbox}
+     */
     public void manageCriterions() {
         Component selectCriterions = popup.getFellow("selectCriterions");
 
@@ -217,6 +221,11 @@ public class OrderElementController extends GenericForwardComposer {
         Util.reloadBindings(selectCriterions);
     }
 
+    /**
+     * Gets the list of possible {@link ICriterionType}.
+     *
+     * @return A {@link List} of {@link ICriterionType}
+     */
     public List<ICriterionType<?>> getCriterionTypes() {
         if (model == null) {
             return new ArrayList<ICriterionType<?>>();
@@ -227,11 +236,20 @@ public class OrderElementController extends GenericForwardComposer {
         return list;
     }
 
+    /**
+     * Returns the selected {@link ICriterionType}.
+     *
+     * @return A {@link Set} of {@link ICriterionType}
+     */
     public Set<ICriterionType<?>> getSelectedCriterionTypes() {
         return selectedCriterionTypes;
     }
 
-    public void reloadSelectedCriterionTypes() {
+    /**
+     * Reloads the selected {@link ICriterionType}, depending on the
+     * {@link Criterion} related with the {@link HoursGroup}
+     */
+    private void reloadSelectedCriterionTypes() {
         OrderElement orderElement = getOrderElement();
 
         if (orderElement == null) {
@@ -251,6 +269,14 @@ public class OrderElementController extends GenericForwardComposer {
         }
     }
 
+    /**
+     * Adds the selected {@link ICriterionType} to the selectedCriterionTypes
+     * attribute.
+     *
+     * @param selectedItems
+     *            {@link Set} of {@link Listitem} with the selected
+     *            {@link ICriterionType}
+     */
     public void assignCriterions(Set<Listitem> selectedItems) {
         for (Listitem listitem : selectedItems) {
             ICriterionType<?> value = (ICriterionType<?>) listitem.getValue();
@@ -259,6 +285,14 @@ public class OrderElementController extends GenericForwardComposer {
         Util.reloadBindings(popup);
     }
 
+    /**
+     * Removes the selected {@link ICriterionType} from the
+     * selectedCriterionTypes attribute.
+     *
+     * @param selectedItems
+     *            {@link Set} of {@link Listitem} with the selected
+     *            {@link ICriterionType}
+     */
     public void unassignCriterions(Set<Listitem> selectedItems) {
         for (Listitem listitem : selectedItems) {
             ICriterionType<?> value = (ICriterionType<?>) listitem.getValue();
@@ -268,6 +302,13 @@ public class OrderElementController extends GenericForwardComposer {
         Util.reloadBindings(popup);
     }
 
+    /**
+     * Removes the {@link Criterion} which matches with this type for all the
+     * {@link HoursGroup}
+     *
+     * @param type
+     *            The type of the {@link Criterion} that should be removed
+     */
     private void removeCriterionsFromHoursGroup(ICriterionType<?> type) {
         OrderElement orderElement = getOrderElement();
         for (HoursGroup hoursGroup : orderElement.getHoursGroups()) {
@@ -294,20 +335,8 @@ public class OrderElementController extends GenericForwardComposer {
             cellWorkingHours.setParent(item);
             Listcell cellPercentage = new Listcell();
             cellPercentage.setParent(item);
-            Listcell cellHoursPolicy = new Listcell();
-            cellHoursPolicy.setParent(item);
-
-            // Generate hours policy Listbox
-            final Listbox hoursPolicyListBox = new Listbox();
-            hoursPolicyListBox.setRows(1);
-            hoursPolicyListBox.setMold("select");
-
-            for (HoursPolicies hourPolicy : HoursPolicies.values()) {
-                Listitem listitem = new Listitem();
-                listitem.setValue(hourPolicy);
-                listitem.setLabel(hourPolicy.toString());
-                listitem.setParent(hoursPolicyListBox);
-            }
+            Listcell cellFixedPercentage = new Listcell();
+            cellFixedPercentage.setParent(item);
 
             Decimalbox decimalBox = new Decimalbox();
             decimalBox.setScale(2);
@@ -336,15 +365,19 @@ public class OrderElementController extends GenericForwardComposer {
                             }
                         }));
 
-                // Hours policy
-                hoursPolicyListBox.setSelectedIndex(hoursGroup.getHoursPolicy()
-                        .ordinal());
-                hoursPolicyListBox.setDisabled(true);
-                cellHoursPolicy.appendChild(hoursPolicyListBox);
+                // Fixed percentage
+                cellFixedPercentage.appendChild(Util.bind(new Checkbox(),
+                        new Util.Getter<Boolean>() {
+
+                            @Override
+                            public Boolean get() {
+                                return hoursGroup.isFixedPercentage();
+                            }
+                        }));
 
             } else { // If is a leaf
 
-                final Intbox workingHours = Util.bind(new Intbox(),
+                Intbox workingHours = Util.bind(new Intbox(),
                         new Util.Getter<Integer>() {
 
                             @Override
@@ -365,13 +398,13 @@ public class OrderElementController extends GenericForwardComposer {
 
                             @Override
                             public void onEvent(Event event) throws Exception {
-                                Util.reloadBindings(popup);
                                 ((OrderLine) getOrderElement())
-                                        .recalculatePercentages();
+                                        .recalculateHoursGroups();
+                                Util.reloadBindings(popup);
                             }
                         });
 
-                final Decimalbox percentage = Util.bind(decimalBox,
+                Decimalbox percentage = Util.bind(decimalBox,
                         new Util.Getter<BigDecimal>() {
 
                             @Override
@@ -392,38 +425,45 @@ public class OrderElementController extends GenericForwardComposer {
 
                             @Override
                             public void onEvent(Event event) throws Exception {
-                                Util.reloadBindings(popup);
                                 ((OrderLine) getOrderElement())
-                                        .recalculatePercentages();
+                                        .recalculateHoursGroups();
+                                Util.reloadBindings(popup);
                             }
                         });
 
-                // Hours policy
-                hoursPolicyListBox.setSelectedIndex(hoursGroup.getHoursPolicy()
-                        .ordinal());
-                hoursPolicyListBox.addEventListener(Events.ON_SELECT,
+                // Fixed percentage
+                Checkbox fixedPercentage = Util.bind(new Checkbox(),
+                        new Util.Getter<Boolean>() {
+
+                            @Override
+                            public Boolean get() {
+                                return hoursGroup.isFixedPercentage();
+                            }
+                        }, new Util.Setter<Boolean>() {
+
+                            @Override
+                            public void set(Boolean value) {
+                                hoursGroup.setFixedPercentage(value);
+                            }
+                        });
+                fixedPercentage.addEventListener(Events.ON_CHECK,
                         new EventListener() {
 
                             @Override
                             public void onEvent(Event event) throws Exception {
-                                HoursPolicies policy = (HoursPolicies) hoursPolicyListBox
-                                        .getSelectedItem().getValue();
-                                hoursGroup.setHoursPolicy(policy);
-
-                                // Disable components depending on the policy
-                                disableComponents(workingHours, percentage,
-                                        policy);
+                                ((OrderLine) getOrderElement())
+                                        .recalculateHoursGroups();
+                                Util.reloadBindings(popup);
                             }
                         });
 
                 // Disable components depending on the policy
                 disableComponents(workingHours, percentage,
-                        (HoursPolicies) hoursPolicyListBox.getSelectedItem()
-                                .getValue());
+                        fixedPercentage.isChecked());
 
                 cellWorkingHours.appendChild(workingHours);
                 cellPercentage.appendChild(percentage);
-                cellHoursPolicy.appendChild(hoursPolicyListBox);
+                cellFixedPercentage.appendChild(fixedPercentage);
 
                 // For each ICriterionType selected
                 for (ICriterionType<?> criterionType : getSelectedCriterionTypes()) {
@@ -500,28 +540,22 @@ public class OrderElementController extends GenericForwardComposer {
          *            An {@link Intbox} for the workingHours
          * @param percentage
          *            A {@link Decimalbox} for the percentage
-         * @param policy
-         *            A {@link HoursPolicies} value
+         * @param fixedPercentage
+         *            If FIXED_PERCENTAGE policy is set or not
          */
         public void disableComponents(Intbox workingHours,
-                Decimalbox percentage, HoursPolicies policy) {
+                Decimalbox percentage, Boolean fixedPercentage) {
 
-            switch (policy) {
-            case FIXED_PERCENTAGE:
+            if (fixedPercentage) {
                 // Working hours not editable
                 workingHours.setDisabled(true);
                 // Percentage editable
                 percentage.setDisabled(false);
-                break;
-
-            case NO_FIXED:
-            case FIXED_HOURS:
-            default:
+            } else {
                 // Working hours editable
                 workingHours.setDisabled(false);
                 // Percentage not editable
                 percentage.setDisabled(true);
-                break;
             }
         }
     }

navalplanner-webapp/src/main/java/org/navalplanner/web/orders/OrderElementModel.java

@@ -3,8 +3,10 @@ package org.navalplanner.web.orders;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.navalplanner.business.orders.daos.IOrderElementDao;
+import org.navalplanner.business.orders.entities.HoursGroup;
 import org.navalplanner.business.orders.entities.OrderElement;
 import org.navalplanner.business.resources.bootstrap.ICriterionsBootstrap;
 import org.navalplanner.business.resources.entities.Criterion;
@@ -43,11 +45,16 @@ public class OrderElementModel implements IOrderElementModel {
     public void setCurrent(OrderElement orderElement) {
         // FIXME Review reattachment
         boolean wasTransient = orderElement.isTransient();
+        Set<HoursGroup> transientHoursGroups = orderElement
+                .getTransientHoursGroups();
         orderElementDao.save(orderElement);
         orderElement.forceLoadHourGroupsCriterions();
         if (wasTransient) {
             orderElement.makeTransientAgain();
         }
+        for (HoursGroup hoursGroup : transientHoursGroups) {
+            hoursGroup.makeTransientAgain();
+        }
         this.orderElement = orderElement;
     }
 

navalplanner-webapp/src/main/webapp/orders/_editOrderElement.zul

@@ -40,12 +40,12 @@
         </hbox>
 
         <listbox id="hoursGroupsListbox" multiple="true"
-            model="@{orderElementController.hoursGroupsModel}"
+            model="@{orderElementController.hoursGroups}"
             itemRenderer="@{orderElementController.renderer}">
              <listhead sizable="true">
                  <listheader label="Hours" />
                  <listheader label="%" />
-                 <listheader label="Fixed" />
+                 <listheader label="Fixed percentage" />
              </listhead>
         </listbox>