chore(history): populate incident treepath#1046
Conversation
venetrius
left a comment
venetrius
left a comment
There was a problem hiding this comment.
I had a few questions / suggestions.
...tor/core/src/main/java/io/camunda/migration/data/impl/history/migrator/IncidentMigrator.java
Outdated
Show resolved
Hide resolved
...n-tests/src/test/java/io/camunda/migration/data/qa/history/HistoryMigrationAbstractTest.java
Outdated
Show resolved
Hide resolved
...ion-tests/src/test/java/io/camunda/migration/data/qa/history/entity/HistoryIncidentTest.java
Outdated
Show resolved
Hide resolved
...tor/core/src/main/java/io/camunda/migration/data/impl/history/migrator/IncidentMigrator.java
Outdated
Show resolved
Hide resolved
| if (!c7Client.hasWaitingExecution(c7Incident.getProcessInstanceId(), c7Incident.getActivityId())) { // Activities on async before waiting state will not have a flow node instance key, but should not be skipped | ||
| throw new EntitySkippedException(c7Incident, SKIP_REASON_MISSING_FLOW_NODE); | ||
| if (dbModel.flowNodeInstanceKey() == null) { | ||
| if (!c7Client.hasWaitingExecution(c7Incident.getProcessInstanceId(), c7Incident.getActivityId())) { |
There was a problem hiding this comment.
❓ If the incident is not skipped. Then the C7 execution is completed the incident will have an inconsistent tree patch in C8. Is that a trade off we accepted?
There was a problem hiding this comment.
Yes the treePath is not a reason to skip the incidentMigration, I will add this to the documentation
| .isEqualTo("PI_" + processInstanceKey + "/FNI_" + flownodeInstanceKey); | ||
| } | ||
|
|
||
| protected void assertOnIncidentBasicFields(IncidentEntity c8Incident, HistoricIncident c7Incident, ProcessInstance c7ChildInstance, ProcessInstance c7ParentInstance) { |
There was a problem hiding this comment.
🙃 Would it make sense to assert treePath here?
There was a problem hiding this comment.
Hm I felt the treepath is not one of the basic/important fields, if we wanted to assert it here we would also need to pass the IncidentDbModel as the treepath is not included in the IncidentEntity - I feel that its not worth this added effort
HeleneW-dot
force-pushed
the
431-populate-treepath-incident
branch
from
d284ad3 to
b95f3e8
Compare
2 participants
related to #431
Pull Request Template
Description
Populates incident treePath.
Note: in operate UI, cancelled instances do not show any incident UI parts so migrated incident history will not be shown in the UI regardless of treepath. Since implementation effort was low, I still added the truncated treepath for consistency in the same truncation manner as for process and flownode instances (only keeping root and leaf).
Type of Change
Testing Checklist
Black-Box Testing Requirements
DbClient,IdKeyMapper,..impl..packages except logging constants)ArchitectureTestvalidates these rules)Test Coverage
Architecture Compliance
Run architecture tests to ensure compliance:
mvn test -Dtest=ArchitectureTestIf architecture tests fail, refactor your tests to use:
LogCapturerfor log assertionscamundaClient.new*SearchRequest()for C8 queriesDocumentation
Checklist
Related Issues
#431