search

camunda / camunda-modeler

An integrated modeling solution for BPMN, DMN and Forms based on bpmn.io.
https://camunda.com/products/modeler
MIT License
1.5k stars 481 forks source link

DMN: Diagram marked as dirty when switching from DRD to Literal Expression and vice versa #1228

Open mschoe opened 5 years ago

mschoe commented 5 years ago

Describe the Bug

Diagram marked as dirty when switching from DRD to Literal Expression and vice versa

dirty_state_drd

Steps to Reproduce

Expected Behavior

As long I make no changes to a diagram it should not be marked as dirty

Environment

Please complete the following information:

Status after first debugging

pinussilvestrus commented 5 years ago

Was able to reproduce it on MacOS 10.14.3. For some reasons, I also experienced the complete opposite behavior. When the diagram is marked as dirty, then switching to the literal expression or dmn table, the dirty state gets lost.

feb-14-2019 11-12-38

pinussilvestrus commented 5 years ago

Was able to reproduce it in the latest stable version v2.2.4, also seems to be a duplicate of https://github.com/camunda/camunda-modeler/issues/706. However, it's worth it to tackle this bug in this milestone.

In the current version, it seems that the dirty state isn't correctly set inside the cache of the DMNEditor which leads to a wrong dirty checking when switching the sheet.

pinussilvestrus commented 5 years ago

After several runs to fix the bugs described inside this issue, I make some interesting findings. To summarize we currently run inside two different bugs regarding the dirty state management inside the DMNEditor.

First bug: Dirty state got lost when switching to another view (e.g. Literal Expression)

This misleading behavior is caused by the fact we do not update the dirty state inside the cache when switching to another view. One solution would be to simply set it (cf. https://github.com/camunda/camunda-modeler/commit/e0d79d1ed3bb0c65db0f2d232526f5f269780443). This would fix the problem, but I'm not sure whether we can run into trouble because of the fact we throw the views.Changed event on every single modification (cf. https://github.com/camunda/camunda-modeler/issues/1115).

Second bug: Dirty state is set again when switching to another view (e.g. Literal Expression)

This is caused by a wrong stackIdx inside the cache:

 isDirty = () => {
    let {
      dirty,
      modeler,
      stackIdx
    } = this.getCached();

    return dirty || modeler.getStackIdx() !== stackIdx;
  }

When making changes inside the drd, saving the diagram and then switching to another view, e.g. Literal Expression, the commandStack of the new view is different from the old view and the dirty state got set again. The current stackIdx only got set right after the broken dirty state check is made. This leads me to the opinion that the stackIdx is maybe not the correct indicator of the dirty state inside the DmnEditor. We should think about this; maybe we can do another run to overthink the dirty state generation (cf. https://github.com/camunda/camunda-modeler/issues/1192#issuecomment-466440609)

Solving the second issue and finding a better way of dirty state indication would also fix the first bug.

pinussilvestrus commented 5 years ago

Created a PR for the first problem (cf. #1273). The second bug will need more work.