-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get Process Instance API to return Process Definition Key value in the API response #4049
Comments
I will be involved in incorporating this feature.As such kindly request you to assign this ticket to me. |
Hi @Nandanrshenoy, Thank you for raising this feature.
Great to hear that, please don't forget to link the ticket to your pull request.
The tickets are being processed by the team, no need for an action here. Best, |
Hi @yanavasileva , Regards, |
@yanavasileva ,Could you kindly assign this feature to me, as I have already started working on it. Thanks and Regards, |
@Nandanrshenoy |
@yanavasileva ,The Initial analysis for the proposed code changes to accommodate Process definition key in Get API Process instance call has been summarized below. Kindly have a look and let me know if I am missing any detail. Request Flow : ![]() Implementation Class : ProcessInstanceRestServiceImpl ![]() Code Changes : No code changes would be required at RestController and Service Implementation end. Resource and ResourceImpl : ![]() Code changes: The ProcessInstance POJO class which is returned from the Data access layer , needs to have a place holder for the definitionKey.This in turn needs to be mapped to the ProcessInstanceDto for end user to retrieve this value. DAO layer changes: MyBatis Entity and Data Mapper The ProcessInstanceQuery interface class allows programmatic querying of Process Instances. In case of Get Process Instance API call based on instance ID, the ProcessInstanceQueryImpl class is only set with process instance ID as a query criteria. The Mybatis Data mapper file which deals with various operations on Process instances and executions employs the ExecutionEntity class to respond to Get Process Instance API call. Code changes: ![]() |
@Nandanrshenoy, thank you for the detailed summary. On first read it the analysis look correct. |
Hi @Nandanrshenoy, Before continuing, I want to make sure you considered using the historic data. Let me know if that will work for you, in that case we can avoid introducing the key in the runtime tables. Best, |
Hi @yanavasileva, Thanks for your inputs. On the other hand ,with the feature that we are implementing, we are no-where planning to introduce Process Definition Key field in the ACT_RU_EXEC table. The database schema doesn’t change. There is a join that already is happening between the ACT_RU_EXEC table and ACT_RE_PROCDEF when Get Process definition based on Process ID is being executed(Get Process List/Post process List uses the same sql) and we are planning to utilize this feature to just get that addition value of PDK programmatically. Kindly let us know your thoughts. Thanks and Regards, |
Hi @Nandanrshenoy, I agree with the performance concerns when querying the historic tables. I wanted only to lay out all of the possibilities to rule out the need of code change. The change now is fully argumented, please continue with the contribution as planned. Please check the contribution guidelines: https://github.com/camunda/camunda-bpm-platform/blob/master/CONTRIBUTING.md Best regards, |
I am closing the ticket due to inactivity. |
@yanavasileva, Regards, |
@yanavasileva , Requesting your support to kindly review the PR. Thanks and Regards, |
@yanavasileva , Thanks and Regards, |
TODO:
|
Thanks @yanavasileva for taking your time out and reviewing my code. On your second comment : #getProcessDefinitionKey() can be exposed to DelegateExecution .Is this the only change that you would need on my PR ? Do you want me to look in to this logic end to end . Please share me any other comments so that I can take things along with this suggested change.Thanks. |
@Nandanrshenoy, I haven't completed the review yet and there might be more findings, I just started a list to mark potential improvements. I found a potential blocker for the PR so please wait with any further changes related to it. I will share more when I finish the code review.
I see this as nice to have separate feature if there's interest. |
related to #4049 Signed-off-by: Shenoy, Nandan <[email protected]> Co-authored-by: <[email protected]>
Dev2QA
Docs |
User Story
As a developer I would like to have the process definition key associated with a process instance API response
In the current day scenario to achieve this , I would need to first make a call to the Get Process Instance API(https://docs.camunda.org/rest/camunda-bpm-platform/7.21-SNAPSHOT/{url}/process-instance/{id}) through which the Definition ID can be retrieved and then subsequently make a call to the Get Process Definition API(https://docs.camunda.org/rest/camunda-bpm-platform/7.21-SNAPSHOT/{url}/process-definition/{id}) to retrieve the Process Definition Key.
Adding the PDK value in the Get Process Instance API response will avert making the additional call to Get Process Definition API
Functional Requirements (Required before implementation)
Get Process Instance API response is updated so that it returns the Process Definition Key.
Technical Requirements (Required before implementation)
Update the camunda rest API for Get Process Instance to accommodate the Process Definition key in its response https://docs.camunda.org/rest/camunda-bpm-platform/7.21-SNAPSHOT/{url}/process-instance/{id}
Limitations of Scope
Hints
Links
Breakdown
Pull requests
Dev2QA handover
The text was updated successfully, but these errors were encountered: