camunda / camunda-modeler

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

Add element Id refresh/rebuild button #384

Open StephenOTT opened 8 years ago

StephenOTT commented 8 years ago

Based on conversation from #380

The element ID is set on the initial creation of the element on the canvas.

When a user adds a Task element, the ID is set to something like: Task_1u62ryy Currently, when a user changes the task to a User Task, the ID remains the same.

Proposal:

Allow the user to manually refresh/rebuild the element ID based on automated naming convention such as Task_1u62ryy becomes UserTask_CallCustomer_1i87o9i.

Current proposal for for naming convention: [element type]_[element name]_[generated short UUID] The short UUID ensures that if there are two elements with the same element type and name, the UUID will still be unique.

If we wanted to get really fancy 😸, this could be a configurable property of the modeler allowing someone to create a "pattern" to follow based on Tokens/Slugs. But I think that is just "flashy" rather than having a strong use case for a v1.

The user will have to manually refresh the IDs and will not be automatic when the element type is changed.

Could see use cases for an ID refresh on an individual element and in Bulk. Bulk Scenario: user creates the BPMN and after the overall "design" is established, the user "refreshes" all ID values so they correspond with the proper naming convention.

Example scenario where being able to easily refresh the ID is helpful:

screen shot 2016-07-28 at 12 14 29 pm

screen shot 2016-07-28 at 12 14 48 pm

Where the End event was changed to a Intermediate event, but the ID is still indicates it is a End Event.

Or:

screen shot 2016-07-28 at 12 13 41 pm

screen shot 2016-07-28 at 12 13 51 pm

Where the Task was changed to a User Task, but the ID still indicates it is a generic task.

ingorichtsmeier commented 8 years ago

The short UUID could be 1, 2, ... if the name is still used in the diagram.

StephenOTT commented 8 years ago

Why does the ID generator currently use the short uuid? Does the generator check if the generated Id already exists?

@ingorichtsmeier is there use cases for having the increment format?

nikku commented 8 years ago

Why does the ID generator currently use the short uuid? Does the generator check if the generated Id already exists?

Simply to avoid collisions. For users numbers vs. short UUIDs probably do not matter anyway.

I do not like to add labels, too. The idea behind IDs is that they are stable, kind of.

If you change ids with every label change you'll not benefit from stuff like DIFFING at all.

StephenOTT commented 8 years ago

I do not like to add labels, too. The idea behind IDs is that they are stable, kind of.

If you change ids with every label change you'll not benefit from stuff like DIFFING at all.

Good point. I see value in having a user defined /user understood value such as the label, but the chnaging labels is a problem. Having a additional field which is the "id name" field seems like a problematic solution as well.

Thoughts of something such as?:

UserLabel_activityType_incrementNumber

The trick being that when name field value is chnages the userLabel is unchanged, but when you chnage activity type, everything after the userLabel is chnaged? You would use the underscore or another character as the delimiter.

berndruecker commented 8 years ago

The use case for real labels is that you will see this ID in the engine database, testcases, etc. If it is human readable that helps a lot.

If it is only done on the press of a special button you are aware that you change the ID now - so breaking DIFFING is no show stopper here.

But I could imagine that this needs to be a configurable strategy.

StephenOTT commented 8 years ago

@nikku do you see issues with the label changes causing a change of the ID if the ID only gets updated through a manual user action (such as a "button" or menu command) ?

ingorichtsmeier commented 8 years ago

@StephenOTT: My use case is:

  1. Build a process definition by business analyst: Use the names as ids, the types (User, Service, Send, ...) doesn't matter. To make the ids unique, append a seqence number if a second activity with the same name appears.
  2. Automate the process definition by a process engineer. Readable Ids helps writing tests: assertThat(processInstance).isWaitingAt("receive_payment").hasPassed("send_invoice"). The ids should be stable while morphing the task and be automatically changed if the name of the task is changed.

This keeps names and ids in sync and makes it easy to follow the test cases.

berndruecker commented 8 years ago

Hey Ingo.

Please not that it should be assertThat(processInstance).isWaitingAt("receiveTaskReceivePayment").hasPassed("sendTaskSendInvoice").

According to our best practices: https://camunda.com/best-practices/naming-technically-relevant-ids/

Cheers Bernd

StephenOTT commented 7 years ago

@berndruecker is that best-practices link supposed to be password protected?

berndruecker commented 7 years ago

Yes - you need a Camunda BPM Enterprise Subscription to access the best practices. The Company credentials used for downloading are valid here too.

StephenOTT commented 7 years ago

@berndruecker k. thanks!

StephenOTT commented 7 years ago

@nikku here is another scenario where this would be helpful.

You convert a Task into a Sub-Process. The ID remains with the original Task name.

screen shot 2016-11-13 at 2 23 53 pm

screen shot 2016-11-13 at 2 24 16 pm

StephenOTT commented 4 years ago

@nikku any movement on this? With Zeebe Modeler coming into usage, this is now becoming a more common issue. Generate a Intermediate Event, and then converting to end event, or start event, and the element ID is still the old one. Or gateways that always have IDs of "ExclusiveGateway.."

If the IDs are not going to be updated, then Why even have Element specific naming? Why not just have all of the tools in the tools pane default with generics like "gateway.." and "task..", "event_..", etc.

nikku commented 4 years ago

If the IDs are not going to be updated, then Why even have Element specific naming? Why not just have all of the tools in the tools pane default with generics like "gateway.." and "task..", "event_..", etc.

I like this suggestion. We'll discuss this internally.

nikku commented 4 years ago

We've discussed more generic IDs as a feature request and are actively considering them for implementation at the moment: https://github.com/camunda/camunda-modeler/issues/1654.


My personal opinion on changing IDs as I develop is: This stuff is and remains dangerous, especially in the presence of multiple stakeholders and systems involved. Changing the ID means that you are not going to be able to track that element across different versions of the diagram and so on.

I understand that testing should be easy. In this case, considering the Camunda assert library it would be way easier to add an "element named" helper or the like and use the (unique) names to reference elements:

assertThat(processInstance).isWaitingAt(taskNamed("FOO BAR"));

This way developers do not need to care about IDs during testing.

ingorichtsmeier commented 4 years ago

Hi @nikku ,

in camunda-bpm-assert this is nearly done. I've already added a findId("My task name") and it should be published as a new minor version soon.

The commit is already pushed: https://github.com/camunda/camunda-bpm-assert/commit/15cc63d8386589c2954c0f899ab37426dddaef7f

Cheers, Ingo

nikku commented 4 years ago

That is great news and a good first step towards making it a "good practice".

falko commented 3 years ago

This modeler plugin can be used as a workaround to generate human-readable technical ids: https://github.com/camunda-consulting/code/tree/master/snippets/camunda-modeler-plugins/bpmn-js-plugin-rename-technical-ids

However, it always renames all ids. That's okay for the first time. It would be good to only regenerate ids for parts of the model that changed significantly maybe with an extra button in the properties panel or a dialog triggered by saving or by being able to select the ids to change from a list of all ids. That would give control over the diffing problem.