Replace removes valid dropdown property

[nikku]

Describe the Bug

As I upgrade a task from one template to the other a valid property gets removed unintentionally, despite the fact it should be kept according to our good practices (unlink + apply new template shall always be equivalent to replace (with new template)).

Steps to Reproduce

1. Go to connectors extension demo
2. Open REST Task diagram
3. Replace legacy REST Task with REST Connector Template
4. See that username and password entered are gone and authentication type is reset to No Auth (despite being set to basic in the original template)

Expected Behavior

• Valid template data is kept: username, password, and authentication type are kept

Environment

• Host (Browser/Node version), if applicable: Any
• OS: Arch
• Library version: latest (2022-09-19)

[nikku]

Reproduction for this one available on feature branch.

[nikku]

Removed from ready; made spring cleaning candiate.

[marstamm]

Root Cause

This behavior is actually consistent to the rules we defined:

• New default values override old (unchanged) default values

As opposed to the unlink-apply behavior which does not know about default values and only applies:

• Existing values will be kept, if they are valid in the new template

Because the source field is of type "hidden", it will always be the default and will be overwritten by the new default during the upgrade.

Solution Sketch

Here are my Ideas on how to solve this:

1️⃣ The new Template does not define a default

This allows us to keep our rules consistent, but is not necessarily intuitive for template designers. This also requires the user to manually select a type on a new template, which is not desirable.
[Note: this is also currently broken and will replace the unchanged value with 'undefined'. I would see this as a bug]

2️⃣ Ditch the "default value changed" check

This solves the issue, but makes upgrade paths for all Templates more tedious for the user, as they have to check if values they never touched changed.

3️⃣ Add edge case for hidden values.

Basically the same we already have for always override with hidden value, we can have always keep value from hidden field when we upgrade

---

Currently, my favorites are 1️⃣ and 3️⃣ , with slight preference towards 1️⃣. It keeps a consistent behavior no matter what Input type you have.
Would love to hear your input @nikku , as you have better insights in the requirements for connectors

[nikku]

I'm gravitating heavily towards 3️⃣. It will enable the use case described without any negative impact on existing user journeys. We could in the future fine tune this, i.e. not take new defaults in any case if the binding type changes (I'm not certain on this at this stage though).

Option 1️⃣ has the grave disadvantage you mentioned:

This also requires the user to manually select a type on a new template, which is not desirable.

Let's discuss in sync in a few minutes..

[marstamm]

Sync Summary:

We want to do a modified 2️⃣

• When the target is a Dropdown, we always keep the existing value (no changed check)

This helps us keep the "no surprises" design principle while Upgrading, as discussed with 2 examples:

Hidden -> Dropdown

• Previous values: basicAuth
• new values: [noAuth (default), basicAuth, oAuth]
• Expected Outcome: basicAuth

Dropdown -> Dropdown

• Previous values: [basicAuth (default), oAtuh], basicAuth selected
• new values: [noAuth (default), basicAuth, oAuth]
• Expected Outcome: basicAuth

[marstamm]

I added the implementation on top of your branch here: #796

[PreetiNirwal]

Hi @marstamm and @nikku, with modified 2️⃣, could you confirm that all data in fields is preserved for the user?

For example, they were using the older "REST Bearer Token Auth" template. Now we are asking them to unlink older template and replace with new REST template. After they replace, all their data in the fields remains the same and they are not required to make any edits/adjustments.

Thanks.

[nikku]

#767 (comment)

This is exactly what should be the case. Everything else is a bug that we have to fix.

Replace should always upgrade gracefully, no surprises.

[PreetiNirwal]

Ok perfect, thank you for clarifying @nikku!

[MaxTru]

Bumped WebModeler to camunda-bpmn-js@0.20.0, but the bug still exists:

Do I miss something?

[nikku]

Not sure what is happening in the app.

---

All dependencies upgraded this works end-to-end in the create-append anything extension demo:

It also worked during a local integration test.

[nikku]

Let's assume this is fixed and continue our investigation on https://github.com/camunda/web-modeler/pull/2598. Happy to check that out tomorrow.

[nikku]

Example showing bearer token to work end-to-end: