fix(planes): ensure we switch to existing plane

[marstamm]

With the ability to remove planes, we have to adjust the Behavior that switches planes during command executions. We should use names instead of plane object when switching planes.

This fixes a scenario that is caused during bpmn-js modeling:

• Create new Collapsed Subprocess
• Add an element to it
• undo everything --> this removes the plane and the root element
• redo creation of plane --> a new plane with the same root element, name and layer is created
• redo shape add --> we switch to a plane without a root element because we saved the plane element that was removed
• We are on an invalid plane

Because the behavior for adding/removing planes is part of bpmn-js, but switching to the correct plane is a diagram-js concern, I had to manually remove the plane+root element and could not use the command stack in the tests.

[nikku]

What we're doing here is essentially operating on two different layers: Doing a modeling operation on the one hand, and doing low level operations (testing exactly what?) on the other hand.

What I'd personally like here is to create a simple test command that executes a real world use-case, i.e. do/undo/redo of adding plane + switching to it, adding an element and so forth. Then we have something that works end-to-end and can be tested in an integration test manner.

[nikku]

Example:

• Implement a behavior that creates a plane any time an element is being created
• Drill into that plane, add a task
• Undo both things (and see how active planes change); re-do both things (and again, verify what is happening).

[nikku]

As a general remark, I do not understand why this PR would be necessary.

Re-doing a certain operation shall re-use what we already got (i.e. rootElements, planes). So it shall be safe to refer to them by reference, not ID (or name). If a plane gets added again, after redo, it shall be the exact same plane.

Referencing by name may cause other issues, i.e. if the name changes.

[marstamm]

I agree that this would be the desired behavior. Currently, Canvas does not offer an API to create a Plane from an element:

diagram-js/lib/core/Canvas.js

Lines 367 to 376 in 3244df4

	/**
	* Creates a plane that is used to draw elements on it. If no
	* root element is provided, an implicit root will be used.
	*
	* @param {string} name
	* @param {Object|djs.model.Root} [rootElement] optional root element
	*
	* @return {Object} plane descriptor with { layer, rootElement, name }
	*/

It will always create a new plane descriptor. So the fix for this Issue would be to expand the Canvas API so we can reuse Plane descriptors. e.g.: eecc65d

Should I adjust this PR or create a new one as this is another concern?

[nikku]

Should I adjust this PR or create a new one as this is another concern?

Fine with either.

eecc65d

Exactly.

[marstamm]

superseded by #599