Git issue 4046 : Persist taskState/lifeCycleState & return in get task and get history task api

[jyotisahu9]

Git issue : https://github.com/camunda/camunda-bpm-platform/issues/4046 Camunda forum issue : https://forum.camunda.io/t/task-state-in-camunda-7-vs-camunda-8/48746/3 Persist task state & update Get task & Get Tasks (Historic) api's to return task state.

[CLAassistant]

All committers have signed the CLA.

[yanavasileva]

Hi @jyotisahu9 ,

Thank you for raising this. We will have a look at it and get back to you.

Best, Yana

[jyotisahu9]

Hi @yanavasileva ,

Did you get a chance to review the code changes.

Thanks, Jyoti

[yanavasileva]

Hi @jyotisahu9, I will need more time to review the PR as the change is not trivial. Please stay tuned and thank you for your patience in advance.

[jyotisahu9]

👍 Overall you covered a lot of the changes in the first run, that's great! Nice work. 🔧 Could you please remove the GIT Issue line from the comments and the javadoc. With the modern IDE and github features, tracking the commit, the PR, and the issue is easier so I think it's redundant. I left comments for what's missing or can be improved. Please have a look.

Thank you for your patience for the review.

Thanks @yanavasileva for your review comments. I am going through them and would do the needful.

[jyotisahu9]

Hi @yanavasileva,

Latest code changes for review comments have been pushed. I have resolved few of the review conversation, for few I have added my comments and it can be closed once you are good with them.

Please review the code whenever you get a chance and let me know in case of any issues.

Thanks, Jyoti

[yanavasileva]

@jyotisahu9, thank you for considering my input. I will have a look at made changes and get back to you next week.

[yanavasileva]

👍 Overall the changes look good, I want to have a look again at the tests, then I will merge the PR.

[jyotisahu9]

Hi @yanavasileva,

Sql script corrections are banked. Request you to review them and merge the PR.

Thanks, Jyoti

[jyotisahu9]

The tests need to be modified. Please note, that I am not able to commit to your branch/fork, so I can't do the changes on my own.

Hi @yanavasileva ,

I went through the review comment. Do you want me to just remove the assertion from HistoricTaskInstanceTest file ?

Thanks, Jyoti

[yanavasileva]

Hi Jyoti,

Do you want me to just remove the assertion from HistoricTaskInstanceTest file ?

No. Beside the suggested removal of the assertion, I expect to add similar test assertion for runtime tasks where the state is created and updated. I suggest to add the assertions to TaskServiceTest.

Best, Yana

[jyotisahu9]

Hi @yanavasileva ,

Pushed the test case review comments fixes. Please review and merge the PR

Thanks

[jyotisahu9]

Hi @yanavasileva

Are the changes good to be merged?

Thanks, Jyoti

[yanavasileva]

I want to bring a detail to your attention and write it down for future reference. When a task is completed, a delete reason is added along the way and the task state is considered as a "Completed". Code reference: https://github.com/camunda/camunda-bpm-platform/blob/e543c70d4f4afb4f6faad20e25f9a4634d084fe6/engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/TaskEntity.java#L360-L366 So once a task is completed the historic task entity will look like as follows:

{
    "id": "8c6c12d3-288c-11ef-9ca5-00ff18b69f72",
    "processDefinitionKey": "invoice",
    "processDefinitionId": "invoice:2:8a96b6d5-288c-11ef-9ca5-00ff18b69f72",
    "processInstanceId": "8c684224-288c-11ef-9ca5-00ff18b69f72",
    "executionId": "8c684224-288c-11ef-9ca5-00ff18b69f72",
    "caseDefinitionKey": null,
    "caseDefinitionId": null,
    "caseInstanceId": null,
    "caseExecutionId": null,
    "activityInstanceId": "approveInvoice:8c6c12d2-288c-11ef-9ca5-00ff18b69f72",
    "name": "Approve Invoice",
    "description": "Approve the invoice (or not).",
    "deleteReason": "completed",
    "owner": null,
    "assignee": null,
    "startTime": "2024-06-07T09:22:38.792+0200",
    "endTime": "2024-06-12T09:22:38.792+0200",
    "duration": 432000000,
    "taskDefinitionKey": "approveInvoice",
    "priority": 50,
    "due": "2024-06-14T09:22:38.792+0200",
    "parentTaskId": null,
    "followUp": null,
    "tenantId": null,
    "removalTime": null,
    "rootProcessInstanceId": "8c684224-288c-11ef-9ca5-00ff18b69f72",
    "taskState": "Completed"
  }

"Deleted" state will be assigned only if a task delete is executed. I don't suggest any changes on this, I just wanted to highlight the behaviour in case it's important for you.

[jyotisahu9]

Thanks @yanavasileva for merging the pull request.