From 43ed441daf60d22a579d93fde840861770b4ede5 Mon Sep 17 00:00:00 2001 From: Yakup Ensar Evli <54323073+ensarevlideveloper@users.noreply.github.com> Date: Tue, 31 May 2022 12:46:35 +0200 Subject: [PATCH] TSK-1812: Added details in task-history for claim/cancel Detect changes and pass them in the specific event in TaskServiceImpl class. For saving the old state of the specified task, I had to use copy() since getTask returns the same object reference, thus no changes can be detected. In the specified events details are accepted as parameters. Extended Tests for Claim and Cancel Events to check correct details. --- ...eHistoryEventOnTaskCancelClaimAccTest.java | 48 +++++++++++++++++-- .../CreateHistoryEventOnTaskClaimAccTest.java | 46 ++++++++++++++++-- .../events/task/TaskClaimCancelledEvent.java | 4 +- .../api/events/task/TaskClaimedEvent.java | 4 +- .../task/internal/TaskServiceImpl.java | 35 +++++++++++--- 5 files changed, 118 insertions(+), 19 deletions(-) diff --git a/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskCancelClaimAccTest.java b/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskCancelClaimAccTest.java index 7ce381dff..1493beb47 100644 --- a/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskCancelClaimAccTest.java +++ b/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskCancelClaimAccTest.java @@ -3,7 +3,10 @@ package acceptance.events.task; import static org.assertj.core.api.Assertions.assertThat; import acceptance.AbstractAccTest; +import java.time.Instant; import java.util.List; +import org.json.JSONArray; +import org.json.JSONObject; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -29,17 +32,19 @@ class CreateHistoryEventOnTaskCancelClaimAccTest extends AbstractAccTest { void should_CreateCancelClaimedHistoryEvent_When_TaskIsCancelClaimed() throws Exception { final String taskId = "TKI:000000000000000000000000000000000043"; + Task task = taskService.getTask(taskId); + final Instant oldModified = task.getModified(); + final Instant oldClaimed = task.getClaimed(); TaskHistoryQueryMapper taskHistoryQueryMapper = getHistoryQueryMapper(); - List events = taskHistoryQueryMapper.queryHistoryEvents( (TaskHistoryQueryImpl) historyService.createTaskHistoryQuery().taskIdIn(taskId)); assertThat(events).isEmpty(); - assertThat(taskService.getTask(taskId).getState()).isEqualTo(TaskState.CLAIMED); - Task task = taskService.forceCancelClaim(taskId); + assertThat(task.getState()).isEqualTo(TaskState.CLAIMED); + task = taskService.forceCancelClaim(taskId); assertThat(task.getState()).isEqualTo(TaskState.READY); events = @@ -48,8 +53,41 @@ class CreateHistoryEventOnTaskCancelClaimAccTest extends AbstractAccTest { assertThat(events).hasSize(1); - String eventType = events.get(0).getEventType(); + TaskHistoryEvent event = events.get(0); - assertThat(eventType).isEqualTo(TaskHistoryEventType.CLAIM_CANCELLED.getName()); + assertThat(event.getEventType()).isEqualTo(TaskHistoryEventType.CLAIM_CANCELLED.getName()); + + event = historyService.getTaskHistoryEvent(event.getId()); + + assertThat(event.getDetails()).isNotNull(); + + JSONArray changes = new JSONObject(event.getDetails()).getJSONArray("changes"); + + JSONObject expectedClaimed = + new JSONObject() + .put("newValue", "") + .put("fieldName", "claimed") + .put("oldValue", oldClaimed.toString()); + JSONObject expectedModified = + new JSONObject() + .put("newValue", task.getModified().toString()) + .put("fieldName", "modified") + .put("oldValue", oldModified.toString()); + JSONObject expectedState = + new JSONObject() + .put("newValue", "READY") + .put("fieldName", "state") + .put("oldValue", "CLAIMED"); + JSONObject expectedOwner = + new JSONObject().put("newValue", "").put("fieldName", "owner").put("oldValue", "user-b-1"); + + JSONArray expectedChanges = + new JSONArray() + .put(expectedClaimed) + .put(expectedModified) + .put(expectedState) + .put(expectedOwner); + + assertThat(changes.similar(expectedChanges)).isTrue(); } } diff --git a/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskClaimAccTest.java b/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskClaimAccTest.java index 940e769e6..e3db329f4 100644 --- a/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskClaimAccTest.java +++ b/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskClaimAccTest.java @@ -3,7 +3,10 @@ package acceptance.events.task; import static org.assertj.core.api.Assertions.assertThat; import acceptance.AbstractAccTest; +import java.time.Instant; import java.util.List; +import org.json.JSONArray; +import org.json.JSONObject; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -27,8 +30,8 @@ class CreateHistoryEventOnTaskClaimAccTest extends AbstractAccTest { @WithAccessId(user = "admin") @Test void should_CreateClaimedHistoryEvent_When_TaskIsClaimed() throws Exception { - final String taskId = "TKI:000000000000000000000000000000000047"; + final Instant oldModified = taskService.getTask(taskId).getModified(); TaskHistoryQueryMapper taskHistoryQueryMapper = getHistoryQueryMapper(); @@ -40,14 +43,51 @@ class CreateHistoryEventOnTaskClaimAccTest extends AbstractAccTest { assertThat(taskService.getTask(taskId).getState()).isEqualTo(TaskState.READY); Task task = taskService.claim(taskId); + assertThat(task.getState()).isEqualTo(TaskState.CLAIMED); events = taskHistoryQueryMapper.queryHistoryEvents( (TaskHistoryQueryImpl) historyService.createTaskHistoryQuery().taskIdIn(taskId)); - String eventType = events.get(0).getEventType(); + TaskHistoryEvent event = events.get(0); - assertThat(eventType).isEqualTo(TaskHistoryEventType.CLAIMED.getName()); + assertThat(event.getEventType()).isEqualTo(TaskHistoryEventType.CLAIMED.getName()); + + event = historyService.getTaskHistoryEvent(event.getId()); + + assertThat(event.getDetails()).isNotNull(); + + JSONArray changes = new JSONObject(event.getDetails()).getJSONArray("changes"); + + JSONObject expectedClaimed = + new JSONObject() + .put("newValue", task.getModified().toString()) + .put("fieldName", "claimed") + .put("oldValue", ""); + JSONObject expectedModified = + new JSONObject() + .put("newValue", task.getModified().toString()) + .put("fieldName", "modified") + .put("oldValue", oldModified.toString()); + JSONObject expectedState = + new JSONObject() + .put("newValue", "CLAIMED") + .put("fieldName", "state") + .put("oldValue", "READY"); + JSONObject expectedOwner = + new JSONObject().put("newValue", "admin").put("fieldName", "owner").put("oldValue", ""); + JSONObject expectedIsRead = + new JSONObject().put("newValue", true).put("fieldName", "isRead").put("oldValue", false); + + JSONArray expectedChanges = + new JSONArray() + .put(expectedClaimed) + .put(expectedModified) + .put(expectedState) + .put(expectedOwner) + .put(expectedIsRead); + + assertThat(changes.similar(expectedChanges)).isTrue(); } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/events/task/TaskClaimCancelledEvent.java b/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/events/task/TaskClaimCancelledEvent.java index 5067ddafe..c920659d1 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/events/task/TaskClaimCancelledEvent.java +++ b/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/events/task/TaskClaimCancelledEvent.java @@ -5,8 +5,8 @@ import pro.taskana.task.api.models.Task; /** Event fired if a task is cancelled to be claimed. */ public class TaskClaimCancelledEvent extends TaskHistoryEvent { - public TaskClaimCancelledEvent(String id, Task task, String userId) { - super(id, task, userId, null); + public TaskClaimCancelledEvent(String id, Task task, String userId, String details) { + super(id, task, userId, details); eventType = TaskHistoryEventType.CLAIM_CANCELLED.getName(); created = task.getModified(); } diff --git a/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/events/task/TaskClaimedEvent.java b/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/events/task/TaskClaimedEvent.java index 9a426fe86..25827c73f 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/events/task/TaskClaimedEvent.java +++ b/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/events/task/TaskClaimedEvent.java @@ -5,8 +5,8 @@ import pro.taskana.task.api.models.Task; /** Event fired if a task is claimed. */ public class TaskClaimedEvent extends TaskHistoryEvent { - public TaskClaimedEvent(String id, Task task, String userId) { - super(id, task, userId, null); + public TaskClaimedEvent(String id, Task task, String userId, String details) { + super(id, task, userId, details); eventType = (TaskHistoryEventType.CLAIMED.getName()); created = task.getClaimed(); } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskServiceImpl.java index 442cb6993..96da61dd7 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskServiceImpl.java @@ -1162,6 +1162,10 @@ public class TaskServiceImpl implements TaskService { try { taskanaEngine.openConnection(); task = (TaskImpl) getTask(taskId); + + TaskImpl oldTask = task.copy(); + oldTask.setId(taskId); + oldTask.setExternalId(task.getExternalId()); Instant now = Instant.now(); checkPreconditionsForClaimTask(task, forceClaim); @@ -1171,11 +1175,15 @@ public class TaskServiceImpl implements TaskService { LOGGER.debug("Task '{}' claimed by user '{}'.", taskId, userId); } if (historyEventManager.isEnabled()) { + String changeDetails = + ObjectAttributeChangeDetector.determineChangesInAttributes(oldTask, task); + historyEventManager.createEvent( new TaskClaimedEvent( IdGenerator.generateWithPrefix(IdGenerator.ID_PREFIX_TASK_HISTORY_EVENT), task, - taskanaEngine.getEngine().getCurrentUserContext().getUserid())); + taskanaEngine.getEngine().getCurrentUserContext().getUserid(), + changeDetails)); } } finally { taskanaEngine.returnConnection(); @@ -1191,6 +1199,14 @@ public class TaskServiceImpl implements TaskService { task.setState(TaskState.CLAIMED); } + private static void cancelClaimActionsOnTask(TaskSummaryImpl task, Instant now) { + task.setOwner(null); + task.setModified(now); + task.setClaimed(null); + task.setRead(true); + task.setState(TaskState.READY); + } + private static void completeActionsOnTask(TaskSummaryImpl task, String userId, Instant now) { task.setCompleted(now); task.setModified(now); @@ -1249,6 +1265,11 @@ public class TaskServiceImpl implements TaskService { try { taskanaEngine.openConnection(); task = (TaskImpl) getTask(taskId); + + TaskImpl oldTask = task.copy(); + oldTask.setId(taskId); + oldTask.setExternalId(task.getExternalId()); + TaskState state = task.getState(); if (state.isEndState()) { throw new InvalidTaskStateException( @@ -1258,21 +1279,21 @@ public class TaskServiceImpl implements TaskService { throw new InvalidOwnerException(userId, taskId); } Instant now = Instant.now(); - task.setOwner(null); - task.setModified(now); - task.setClaimed(null); - task.setRead(true); - task.setState(TaskState.READY); + cancelClaimActionsOnTask(task, now); taskMapper.update(task); if (LOGGER.isDebugEnabled()) { LOGGER.debug("Task '{}' unclaimed by user '{}'.", taskId, userId); } if (historyEventManager.isEnabled()) { + String changeDetails = + ObjectAttributeChangeDetector.determineChangesInAttributes(oldTask, task); + historyEventManager.createEvent( new TaskClaimCancelledEvent( IdGenerator.generateWithPrefix(IdGenerator.ID_PREFIX_TASK_HISTORY_EVENT), task, - taskanaEngine.getEngine().getCurrentUserContext().getUserid())); + taskanaEngine.getEngine().getCurrentUserContext().getUserid(), + changeDetails)); } } finally { taskanaEngine.returnConnection();