TSK-1143C mismatch between forward and backward calculation between planned and due
This commit is contained in:
parent
a6e007ef68
commit
7b409451dc
|
@ -16,6 +16,7 @@ import java.util.stream.LongStream;
|
|||
import java.util.stream.Stream;
|
||||
|
||||
import pro.taskana.common.api.exceptions.InvalidArgumentException;
|
||||
import pro.taskana.common.api.exceptions.SystemException;
|
||||
|
||||
/**
|
||||
* The DaysToWorkingDaysConverter provides a method to convert an age in days into an age in working
|
||||
|
@ -74,10 +75,17 @@ public final class DaysToWorkingDaysConverter {
|
|||
}
|
||||
|
||||
public long convertWorkingDaysToDays(Instant startTime, long numberOfDays) {
|
||||
if (startTime == null) {
|
||||
throw new SystemException(
|
||||
"Internal Error: convertWorkingDasToDays was called with a null startTime");
|
||||
} else if (!startTime.equals(referenceDate)) {
|
||||
refreshReferenceDate(referenceDate);
|
||||
}
|
||||
int direction = numberOfDays >= 0 ? 1 : -1;
|
||||
long limit = Math.abs(numberOfDays);
|
||||
final Instant finalStartTime = startTime;
|
||||
return LongStream.iterate(0, i -> i + direction)
|
||||
.filter(day -> isWorkingDay(day, startTime))
|
||||
.filter(day -> isWorkingDay(day, finalStartTime))
|
||||
.skip(limit)
|
||||
.findFirst()
|
||||
.orElse(0);
|
||||
|
@ -161,6 +169,17 @@ public final class DaysToWorkingDaysConverter {
|
|||
return LocalDate.of(year, 3, 22).plusDays(d + e);
|
||||
}
|
||||
|
||||
private void refreshReferenceDate(Instant newReferenceDate) {
|
||||
int yearOfReferenceDate =
|
||||
LocalDateTime.ofInstant(referenceDate, ZoneId.systemDefault()).getYear();
|
||||
int yearOfNewReferenceDate =
|
||||
LocalDateTime.ofInstant(newReferenceDate, ZoneId.systemDefault()).getYear();
|
||||
if (yearOfReferenceDate != yearOfNewReferenceDate) {
|
||||
easterSunday = getEasterSunday(yearOfNewReferenceDate);
|
||||
}
|
||||
this.referenceDate = newReferenceDate;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return "DaysToWorkingDaysConverter{"
|
||||
|
|
|
@ -432,6 +432,7 @@ public interface TaskService {
|
|||
* Retrieves a task comment for a given taskCommentId.
|
||||
*
|
||||
* @param taskCommentId The id of the task comment which should be retrieved
|
||||
* @return the task comment identified by taskCommentId
|
||||
* @throws TaskCommentNotFoundException If the given taskCommentId in the TaskComment does not
|
||||
* refer to an existing taskComment.
|
||||
* @throws NotAuthorizedException If the current user has no authorization to retrieve a
|
||||
|
@ -448,6 +449,7 @@ public interface TaskService {
|
|||
* Retrieves a list of task comments for a given taskId.
|
||||
*
|
||||
* @param taskId The id of the task for which all task comments should be retrieved
|
||||
* @return the list of task comments attached to task with id taskId
|
||||
* @throws NotAuthorizedException If the current user has no authorization to retrieve a
|
||||
* taskComment from a certain task or is not authorized to access the task.
|
||||
* @throws TaskNotFoundException If the given taskId in the TaskComment does not refer to an
|
||||
|
|
|
@ -36,6 +36,7 @@ public interface TaskComment {
|
|||
/**
|
||||
* Sets the text field of the task comment.
|
||||
*
|
||||
* @param textField the text field
|
||||
*/
|
||||
void setTextField(String textField);
|
||||
|
||||
|
|
|
@ -47,6 +47,8 @@ class ServiceLevelHandler {
|
|||
this.taskanaEngine = taskanaEngine;
|
||||
this.taskMapper = taskMapper;
|
||||
this.attachmentMapper = attachmentMapper;
|
||||
DaysToWorkingDaysConverter.setGermanPublicHolidaysEnabled(
|
||||
taskanaEngine.getEngine().getConfiguration().isGermanPublicHolidaysEnabled());
|
||||
try {
|
||||
this.converter = DaysToWorkingDaysConverter.initialize();
|
||||
} catch (InvalidArgumentException e) {
|
||||
|
@ -84,6 +86,7 @@ class ServiceLevelHandler {
|
|||
boolean onlyPriority = false;
|
||||
if (newTaskImpl.getClassificationSummary() == null
|
||||
|| newTaskImpl.getClassificationSummary().getServiceLevel() == null) {
|
||||
newTaskImpl = setPlannedDueOnMissingServiceLevel(newTaskImpl);
|
||||
onlyPriority = true;
|
||||
}
|
||||
|
||||
|
@ -132,6 +135,19 @@ class ServiceLevelHandler {
|
|||
return getFinalPrioDurationOfTask(resolvedClassifications, onlyPriority);
|
||||
}
|
||||
|
||||
private TaskImpl setPlannedDueOnMissingServiceLevel(TaskImpl task) {
|
||||
Instant now = Instant.now();
|
||||
if (task.getDue() == null && task.getPlanned() == null) {
|
||||
task.setDue(now);
|
||||
task.setPlanned(now);
|
||||
} else if (task.getDue() == null) {
|
||||
task.setDue(task.getPlanned());
|
||||
} else {
|
||||
task.setPlanned(task.getDue());
|
||||
}
|
||||
return task;
|
||||
}
|
||||
|
||||
private TaskImpl updatePlannedDueOnTaskUpdate(
|
||||
TaskImpl newTaskImpl, TaskImpl oldTaskImpl, DurationPrioHolder durationPrioHolder)
|
||||
throws InvalidArgumentException {
|
||||
|
@ -150,26 +166,97 @@ class ServiceLevelHandler {
|
|||
newTaskImpl.setDue(
|
||||
converter.addWorkingDaysToInstant(
|
||||
newTaskImpl.getPlanned(), durationPrioHolder.getDuration()));
|
||||
} else { // case 3: due was changed
|
||||
if (newTaskImpl.getDue() == null) {
|
||||
newTaskImpl.setDue(
|
||||
converter.addWorkingDaysToInstant(
|
||||
newTaskImpl.getPlanned(), durationPrioHolder.getDuration()));
|
||||
} else {
|
||||
Instant planned =
|
||||
(converter.subtractWorkingDaysFromInstant(
|
||||
newTaskImpl.getDue(), durationPrioHolder.getDuration()));
|
||||
if (newTaskImpl.getPlanned() != null && !planned.equals(newTaskImpl.getPlanned())) {
|
||||
throw new InvalidArgumentException(
|
||||
"Cannot update a task with given planned "
|
||||
+ "and due date not matching the service level");
|
||||
}
|
||||
newTaskImpl.setPlanned(planned);
|
||||
if (!converter.isWorkingDay(0, newTaskImpl.getPlanned())) {
|
||||
newTaskImpl.setPlanned(getFirstFollowingWorkingDay(newTaskImpl.getPlanned()));
|
||||
}
|
||||
} else {
|
||||
updatePlannedDueOnTaskUpdateWhenDueWasChanged(newTaskImpl, durationPrioHolder);
|
||||
}
|
||||
return newTaskImpl;
|
||||
}
|
||||
|
||||
private void updatePlannedDueOnTaskUpdateWhenDueWasChanged(TaskImpl newTaskImpl,
|
||||
DurationPrioHolder durationPrioHolder) throws InvalidArgumentException {
|
||||
if (newTaskImpl.getDue() == null) {
|
||||
newTaskImpl.setDue(
|
||||
converter.addWorkingDaysToInstant(
|
||||
newTaskImpl.getPlanned(), durationPrioHolder.getDuration()));
|
||||
if (!converter.isWorkingDay(0, newTaskImpl.getPlanned())) {
|
||||
newTaskImpl.setPlanned(getFirstFollowingWorkingDay(newTaskImpl.getPlanned()));
|
||||
}
|
||||
} else {
|
||||
Instant planned =
|
||||
(converter.subtractWorkingDaysFromInstant(
|
||||
newTaskImpl.getDue(), durationPrioHolder.getDuration()));
|
||||
ensureServiceLevelIsNotViolated(newTaskImpl, durationPrioHolder.getDuration(), planned);
|
||||
newTaskImpl.setPlanned(planned);
|
||||
if (!converter.isWorkingDay(0, newTaskImpl.getDue())) {
|
||||
newTaskImpl.setDue(getFirstPreceedingWorkingDay(newTaskImpl.getDue()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private Instant getFirstFollowingWorkingDay(Instant planned) {
|
||||
long days = 0;
|
||||
while (!converter.isWorkingDay(days, planned)) {
|
||||
days++;
|
||||
}
|
||||
return planned.plus(Duration.ofDays(days));
|
||||
}
|
||||
|
||||
private Instant getFirstPreceedingWorkingDay(Instant due) {
|
||||
long days = 0;
|
||||
while (!converter.isWorkingDay(days, due)) {
|
||||
days--;
|
||||
}
|
||||
return due.minus(Duration.ofDays(Math.abs(days)));
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure that planned and due of task comply with the associated service level. The 'planned'
|
||||
* timestamp was calculated by subtracting the service level duration from task.due. It may not be
|
||||
* the same as task.planned and the request may nevertheless be correct. The following Scenario
|
||||
* illustrates this: If task.planned is on a Saturday morning, and duration is 1 working day, then
|
||||
* calculating forward from planned to due will give Tuesday morning as due date, because sunday
|
||||
* is skipped. On the other hand, calculating from due (Tuesday morning) 1 day backwards will
|
||||
* result in a planned date of monday morning which differs from task.planned. Therefore, if
|
||||
* task.getPlanned is not equal to planned, the service level is not violated and we still must
|
||||
* grant the request if the following conditions are fulfilled: - planned is after task.planned -
|
||||
* task.planned is not a working day, - there is no working day between task.planned and planned.
|
||||
*
|
||||
* @param task the task for the difference between planned and due must be duration
|
||||
* @param duration the serviceLevel for the task
|
||||
* @param planned the planned Timestamp thas was calculated based on due and duration
|
||||
* @throws InvalidArgumentException if service level is violated.
|
||||
*/
|
||||
private void ensureServiceLevelIsNotViolated(TaskImpl task, Duration duration, Instant planned)
|
||||
throws InvalidArgumentException {
|
||||
if (task.getPlanned() != null && !task.getPlanned().equals(planned)) {
|
||||
boolean isServiceLevelViolated = false;
|
||||
Instant taskPlanned = task.getPlanned();
|
||||
if (converter.isWorkingDay(0, taskPlanned)) {
|
||||
isServiceLevelViolated = true;
|
||||
} else if (taskPlanned.isAfter(planned)) {
|
||||
isServiceLevelViolated = true;
|
||||
} else {
|
||||
long days = Duration.between(taskPlanned, planned).toDays();
|
||||
for (long day = 0; day < days; day++) {
|
||||
if (converter.isWorkingDay(day, taskPlanned)) {
|
||||
isServiceLevelViolated = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (isServiceLevelViolated) {
|
||||
throw new InvalidArgumentException(
|
||||
String.format(
|
||||
"Cannot update a task with given planned %s "
|
||||
+ "and due date %s not matching the service level %s.",
|
||||
task.getPlanned(), task.getDue(), duration));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private TaskImpl updatePlannedDueOnCreationOfNewTask(
|
||||
TaskImpl newTaskImpl, DurationPrioHolder durationPrioHolder) throws InvalidArgumentException {
|
||||
if (newTaskImpl.getDue() != null) { // due is specified: calculate back and check correctnes
|
||||
|
|
|
@ -43,6 +43,7 @@ public abstract class AbstractAccTest {
|
|||
}
|
||||
dataSource = TaskanaEngineTestConfiguration.getDataSource();
|
||||
taskanaEngineConfiguration = new TaskanaEngineConfiguration(dataSource, false, schemaName);
|
||||
taskanaEngineConfiguration.setGermanPublicHolidaysEnabled(true);
|
||||
taskanaEngine = taskanaEngineConfiguration.buildTaskanaEngine();
|
||||
taskanaEngine.setConnectionManagementMode(ConnectionManagementMode.AUTOCOMMIT);
|
||||
sampleDataGenerator.clearDb();
|
||||
|
|
|
@ -10,6 +10,7 @@ import java.time.temporal.ChronoUnit;
|
|||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
|
||||
|
@ -307,11 +308,11 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest {
|
|||
// test update of due that fails
|
||||
task.setDue(planned.plus(Duration.ofDays(8)));
|
||||
Task finalTask = task;
|
||||
assertThatThrownBy(
|
||||
ThrowingCallable taskanaCall =
|
||||
() -> {
|
||||
taskService.updateTask(finalTask);
|
||||
})
|
||||
.isInstanceOf(InvalidArgumentException.class);
|
||||
};
|
||||
assertThatThrownBy(taskanaCall).isInstanceOf(InvalidArgumentException.class);
|
||||
|
||||
// update due and planned as expected.
|
||||
task = taskService.getTask(taskId);
|
||||
|
@ -361,12 +362,12 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest {
|
|||
days = converter.convertWorkingDaysToDays(task.getPlanned(), 1);
|
||||
assertThat(task.getDue()).isEqualTo(task.getPlanned().plus(Duration.ofDays(days)));
|
||||
|
||||
task.setDue(planned.plus(Duration.ofDays(13)));
|
||||
task.setDue(planned.plus(Duration.ofDays(13))); // due = 2020-05-16, i.e. saturday
|
||||
task.setPlanned(null);
|
||||
task = taskService.updateTask(task);
|
||||
days = converter.convertWorkingDaysToDays(task.getDue(), -1);
|
||||
assertThat(task.getDue()).isEqualTo(planned.plus(Duration.ofDays(13)));
|
||||
assertThat(task.getPlanned()).isEqualTo(task.getDue().plus(Duration.ofDays(days)));
|
||||
assertThat(task.getPlanned()).isEqualTo(getInstant("2020-05-14T07:00:00"));
|
||||
assertThat(task.getDue()).isEqualTo(getInstant("2020-05-15T07:00:00"));
|
||||
}
|
||||
|
||||
@WithAccessId(
|
||||
|
@ -380,4 +381,37 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest {
|
|||
assertThatThrownBy(() -> taskService.updateTask(task))
|
||||
.isInstanceOf(InvalidArgumentException.class);
|
||||
}
|
||||
|
||||
@WithAccessId(
|
||||
userName = "user_1_2",
|
||||
groupNames = {"group_1"})
|
||||
@Test
|
||||
void testUpdateTaskSetPlannedOrDueToWeekend()
|
||||
throws NotAuthorizedException, TaskNotFoundException, ClassificationNotFoundException,
|
||||
InvalidArgumentException, InvalidStateException, ConcurrencyException,
|
||||
AttachmentPersistenceException {
|
||||
Task task = taskService.getTask("TKI:000000000000000000000000000000000030"); // SL=P13D
|
||||
task.setPlanned(getInstant("2020-03-21T07:00:00")); // planned = saturday
|
||||
task = taskService.updateTask(task);
|
||||
assertThat(task.getDue()).isEqualTo(getInstant("2020-04-09T07:00:00"));
|
||||
|
||||
task.setDue(getInstant("2020-04-11T07:00:00")); // due = saturday
|
||||
task.setPlanned(null);
|
||||
task = taskService.updateTask(task);
|
||||
assertThat(task.getPlanned()).isEqualTo(getInstant("2020-03-23T07:00:00"));
|
||||
|
||||
task.setDue(getInstant("2020-04-12T07:00:00")); // due = sunday
|
||||
task = taskService.updateTask(task);
|
||||
assertThat(task.getPlanned()).isEqualTo(getInstant("2020-03-23T07:00:00"));
|
||||
|
||||
task.setPlanned(getInstant("2020-03-21T07:00:00")); // planned = saturday
|
||||
task.setDue(getInstant("2020-04-09T07:00:00")); // thursday
|
||||
task = taskService.updateTask(task);
|
||||
assertThat(task.getPlanned()).isEqualTo(getInstant("2020-03-23T07:00:00"));
|
||||
|
||||
task.setPlanned(getInstant("2020-03-03T07:00:00")); // planned on tuesday
|
||||
task.setDue(getInstant("2020-03-22T07:00:00")); // due = sunday
|
||||
task = taskService.updateTask(task);
|
||||
assertThat(task.getDue()).isEqualTo(getInstant("2020-03-20T07:00:00")); // friday
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue