diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/planner/entities/Task.java b/navalplanner-business/src/main/java/org/navalplanner/business/planner/entities/Task.java index 7e9aeebf9..dfd437639 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/planner/entities/Task.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/planner/entities/Task.java @@ -29,6 +29,8 @@ import java.util.List; import java.util.Set; import org.apache.commons.lang.Validate; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.hibernate.validator.AssertTrue; import org.joda.time.DateTime; import org.joda.time.Days; @@ -48,6 +50,8 @@ import org.navalplanner.business.resources.entities.Worker; */ public class Task extends TaskElement { + private static final Log LOG = LogFactory.getLog(Task.class); + public static Task createTask(TaskSource taskSource) { Task task = new Task(); OrderElement orderElement = taskSource.getOrderElement(); @@ -104,7 +108,21 @@ public class Task extends TaskElement { } public Set> getResourceAllocations() { - return Collections.unmodifiableSet(resourceAllocations); + return Collections.unmodifiableSet(filterEmpty(resourceAllocations)); + } + + private Set> filterEmpty( + Set> allocations) { + Set> result = new HashSet>(); + for (ResourceAllocation each : allocations) { + if (each.hasAssignments()) { + result.add(each); + } else { + LOG.warn("there is an empty resource allocation: " + + each.toString() + " ,on task: " + this); + } + } + return result; } public void addResourceAllocation(ResourceAllocation resourceAllocation) { @@ -112,8 +130,13 @@ public class Task extends TaskElement { throw new IllegalArgumentException( "the resourceAllocation's task must be this task"); } - resourceAllocations.add(resourceAllocation); - resourceAllocation.associateAssignmentsToResource(); + if (resourceAllocation.hasAssignments()) { + resourceAllocations.add(resourceAllocation); + resourceAllocation.associateAssignmentsToResource(); + } else { + LOG.warn("adding a resource allocation without assignments: " + + resourceAllocation); + } } public void removeResourceAllocation( @@ -289,7 +312,7 @@ public class Task extends TaskElement { @Override protected void moveAllocations() { List copied = ModifiedAllocation - .copy(resourceAllocations); + .copy(getResourceAllocations()); List allocations = ResourcesPerDayModification .fromExistent(ModifiedAllocation.modified(copied)); if (allocations.isEmpty()) { diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/planner/entities/allocationalgorithms/ResourcesPerDayModification.java b/navalplanner-business/src/main/java/org/navalplanner/business/planner/entities/allocationalgorithms/ResourcesPerDayModification.java index d968b788c..2f8a18ac9 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/planner/entities/allocationalgorithms/ResourcesPerDayModification.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/planner/entities/allocationalgorithms/ResourcesPerDayModification.java @@ -119,6 +119,7 @@ public abstract class ResourcesPerDayModification extends Collection> allocations) { List result = new ArrayList(); for (ResourceAllocation resourceAllocation : allocations) { + Validate.isTrue(resourceAllocation.hasAssignments()); ResourcesPerDay perDay = resourceAllocation .getResourcesPerDay(); Validate.notNull(perDay); diff --git a/navalplanner-business/src/test/java/org/navalplanner/business/test/planner/entities/TaskTest.java b/navalplanner-business/src/test/java/org/navalplanner/business/test/planner/entities/TaskTest.java index 328a8e93b..284d69634 100644 --- a/navalplanner-business/src/test/java/org/navalplanner/business/test/planner/entities/TaskTest.java +++ b/navalplanner-business/src/test/java/org/navalplanner/business/test/planner/entities/TaskTest.java @@ -34,7 +34,6 @@ import org.junit.Test; import org.navalplanner.business.orders.entities.HoursGroup; import org.navalplanner.business.orders.entities.OrderLine; import org.navalplanner.business.orders.entities.TaskSource; -import org.navalplanner.business.planner.entities.ResourceAllocation; import org.navalplanner.business.planner.entities.SpecificResourceAllocation; import org.navalplanner.business.planner.entities.Task; import org.navalplanner.business.planner.entities.TaskElement; @@ -80,23 +79,31 @@ public class TaskTest { } @Test - public void taskAddResourceAllocation() { + public void addingEmptyResourceAllocationDoesntAddIt() { assertThat(task.getResourceAllocations().size(), equalTo(0)); SpecificResourceAllocation resourceAllocation = SpecificResourceAllocation.create(task); task.addResourceAllocation(resourceAllocation); + assertThat(task.getResourceAllocations().size(), equalTo(0)); + } + + @Test + public void addingNoEmptyResourceAllocationAddsIt() { + assertThat(task.getResourceAllocations().size(), equalTo(0)); + + SpecificResourceAllocation resourceAllocation = stubResourceAllocationWithAssignedHours( + task, 500); + task.addResourceAllocation(resourceAllocation); assertThat(task.getResourceAllocations().size(), equalTo(1)); - assertThat( - resourceAllocation.getTask().getResourceAllocations().size(), - equalTo(1)); } @Test public void taskRemoveResourceAllocation() { assertThat(task.getResourceAllocations().size(), equalTo(0)); - SpecificResourceAllocation resourceAllocation = SpecificResourceAllocation.create(task); + SpecificResourceAllocation resourceAllocation = stubResourceAllocationWithAssignedHours( + task, 500); task.addResourceAllocation(resourceAllocation); assertThat(task.getResourceAllocations().size(), equalTo(1)); @@ -112,17 +119,21 @@ public class TaskTest { @Test public void aTaskWithAllocationsReturnsTheSumOfItsAllocations() { - task.addResourceAllocation(stubResourceAllocationWithAssignedHours(5)); - task.addResourceAllocation(stubResourceAllocationWithAssignedHours(3)); + task.addResourceAllocation(stubResourceAllocationWithAssignedHours( + task, 5)); + task.addResourceAllocation(stubResourceAllocationWithAssignedHours( + task, 3)); assertThat(task.getAssignedHours(), equalTo(8)); } - private ResourceAllocation stubResourceAllocationWithAssignedHours( + private SpecificResourceAllocation stubResourceAllocationWithAssignedHours( + Task task, int hours) { - ResourceAllocation resourceAllocation = createNiceMock(ResourceAllocation.class); + SpecificResourceAllocation resourceAllocation = createNiceMock(SpecificResourceAllocation.class); expect(resourceAllocation.getAssignedHours()).andReturn(hours) .anyTimes(); expect(resourceAllocation.getTask()).andReturn(task).anyTimes(); + expect(resourceAllocation.hasAssignments()).andReturn(true).anyTimes(); replay(resourceAllocation); return resourceAllocation; }