Skip to content
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

[Bug] Disable delete a specific workflow version when exist workflow instance under this version which is not finish #15730

Conversation

ruanwenjun
Copy link
Member

Purpose of the pull request

When we delete a specific workflow definition version that contains running workflow instances, then the workflow instance might get an unexcepted status and we cannot open the workflow instance detail page since missing metadaba.

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

* @param taskDefinitionJson taskDefinitionJson
* @param otherParamsJson otherParamsJson handle other params
* @param otherParamsJson otherParamsJson handle other params

Check notice

Code scanning / CodeQL

Spurious Javadoc @param tags Note

@param tag "otherParamsJson" does not match any actual parameter of method "updateProcessDefinition()".
* @param projectCode project code
* @param code the process definition code
* @param version the process definition version user want to delete
* @param code the process definition code

Check notice

Code scanning / CodeQL

Spurious Javadoc @param tags Note

@param tag "code" does not match any actual parameter of method "deleteProcessDefinitionVersion()".
* @param code the process definition code
* @param version the process definition version user want to delete
* @param code the process definition code
* @param version the process definition version user want to delete

Check notice

Code scanning / CodeQL

Spurious Javadoc @param tags Note

@param tag "version" does not match any actual parameter of method "deleteProcessDefinitionVersion()".
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_cannotDeleteWorkflowVersionWhenExistRunningInstance branch from 9e9e421 to d8bb8ff Compare March 18, 2024 05:53
@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2024

Codecov Report

Attention: Patch coverage is 10.34483% with 26 lines in your changes are missing coverage. Please review.

Project coverage is 39.09%. Comparing base (a16a15c) to head (91afa3d).

❗ Current head 91afa3d differs from pull request most recent head ca29e1e. Consider uploading reports for the commit ca29e1e to get more accurate results

Files Patch % Lines
...api/service/impl/ProcessDefinitionServiceImpl.java 0.00% 15 Missing ⚠️
...cheduler/common/enums/WorkflowExecutionStatus.java 0.00% 10 Missing ⚠️
...r/api/service/impl/ProcessInstanceServiceImpl.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev   #15730      +/-   ##
============================================
- Coverage     39.11%   39.09%   -0.03%     
+ Complexity     4855     4852       -3     
============================================
  Files          1316     1316              
  Lines         44963    44967       +4     
  Branches       4810     4810              
============================================
- Hits          17589    17579      -10     
- Misses        25474    25489      +15     
+ Partials       1900     1899       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@EricGao888
Copy link
Member

I have a question not directly related to this PR. Since currently we encapsulate the whole query stuff in Mappers, do we still need DAOs? It seems our DAOs have already lost the function of decoupling.

@ruanwenjun
Copy link
Member Author

I have a question not directly related to this PR. Since currently we encapsulate the whole query stuff in Mappers, do we still need DAOs? It seems our DAOs have already lost the function of decoupling.

it's better to return DO in the Mapper, and return DTO in DAO. So the DAO might contain some merge or transform function.

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_cannotDeleteWorkflowVersionWhenExistRunningInstance branch from 3f3a407 to 818d91f Compare March 18, 2024 15:23
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_cannotDeleteWorkflowVersionWhenExistRunningInstance branch from 818d91f to 1854968 Compare March 19, 2024 02:46
@ruanwenjun ruanwenjun added the bug Something isn't working label Mar 19, 2024
@ruanwenjun ruanwenjun added this to the 3.2.2 milestone Mar 19, 2024
@ruanwenjun ruanwenjun added 3.2.1 and removed 3.2.2 labels Mar 19, 2024
@ruanwenjun ruanwenjun closed this Mar 20, 2024
@ruanwenjun ruanwenjun reopened this Mar 20, 2024
@ruanwenjun ruanwenjun self-assigned this Mar 21, 2024
…instance under this version which is not finish
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_cannotDeleteWorkflowVersionWhenExistRunningInstance branch from ca29e1e to e3d8979 Compare March 21, 2024 08:03
Copy link

sonarcloud bot commented Mar 21, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
11.5% Coverage on New Code (required ≥ 60%)

See analysis details on SonarCloud

@ruanwenjun ruanwenjun merged commit 56a834b into apache:dev Mar 21, 2024
59 of 60 checks passed
@ruanwenjun ruanwenjun deleted the dev_wenjun_cannotDeleteWorkflowVersionWhenExistRunningInstance branch March 21, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants