Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conditional properties are not reliably rendered #3029

Closed
christian-konrad opened this issue Jul 14, 2022 · 12 comments
Closed

Conditional properties are not reliably rendered #3029

christian-konrad opened this issue Jul 14, 2022 · 12 comments
Assignees
Labels
bug Something isn't working channel:customer element templates spring cleaning Could be cleaned up one day
Milestone

Comments

@christian-konrad
Copy link
Contributor

christian-konrad commented Jul 14, 2022

Describe the bug

Property conditions are not evaluated right when applying a template where the referenced dropdown value has no explicit default. Also, the value of a binding is not changed to the specified default value when using multiple but mutually exclusive properties with different default values (i.e. to simulate conditional default values). The latter is a behavior requested by a partner that implements a whole connector ecosystem.

Steps to reproduce

  1. Create a template with a dropdown with no default option specified (the first option will be rendered)
  2. Create 2 dependent properties, but with same binding, and both depending on different dropdown option. The first is dependent on the first dropdown option. Both have different default values.
  3. When applying the template, even if the first dropdown option is visually selected, the dependent property isn't shown
  4. Switching the dropdown value does not change the default value of the field/binding
  5. The field must be rendered invisible first (by invalidating all conditions), before the default value can change

See the video and template (next comment)

Bildschirmaufnahme.2022-07-14.um.17.12.52.mov

Expected behavior

  • Either the dropdown renders no value if no default is set, or if the first value is rendered, all conditions to this must be evaluated.
  • Default values of same bindings should change when conditions apply, as this is explicitly defined in the template (otherwhise, the template should prohibit multiple properties with same binding)

Environment

  • Camunda Modeler Version: 5.1.0
  • Execution Platform: C8
  • Installed plug-ins: None
@christian-konrad christian-konrad added the bug Something isn't working label Jul 14, 2022
@christian-konrad
Copy link
Contributor Author

  {
    "$schema": "https://unpkg.com/browse/@camunda/zeebe-element-templates-json-schema/resources/schema.json",
    "name": "Same Binding Test",
    "id": "same-binding-test",    
    "appliesTo": [
      "bpmn:ServiceTask"
    ],
    "properties": [
      {
        "id": "default-changer",
        "label": "Change the default",
        "type": "Dropdown",
        "choices": [
          {
            "name": "Set it as 1",
            "value": "set-1"
          },
          {
            "name": "Set it as 2",
            "value": "set-2"
          },
          {
            "name": "Reset",
            "value": ""
          }
        ],
        "binding": {
          "type": "zeebe:taskHeader",
          "key": "ignore"
        }
      },
      {
        "label": "Test",
        "type": "String",
        "value": "1",      
        "binding": {
          "type": "zeebe:input",
          "name": "test"
        },
        "condition": {
          "property": "default-changer",
          "equals": "set-1"
        }
      },

      {
        "label": "Test",
        "type": "String",      
        "value": "2",
        "binding": {
          "type": "zeebe:input",
          "name": "test"
        },
        "condition": {
          "property": "default-changer",
          "equals": "set-2"
        }
      }
    ]
  }
]

@pinussilvestrus
Copy link
Contributor

I believe it should work the same as for any other templated "Dropdown" property. When no value is given, it should display empty.

image

{
  "name": "Dropdown",
  "id": "Dropdown",
  "appliesTo": [  "bpmn:ServiceTask"  ],
  "properties": [
    {
      "label": "Task Priority",
      "type": "Dropdown",
      "choices": [
        { "name": "low", "value": "20" },
        { "name": "medium", "value": "50" },
        { "name": "height", "value": "100" }
      ],
      "binding": {
        "type": "property",
        "name": "camunda:priority"
      }
    }
  ]
}

@pinussilvestrus pinussilvestrus added the spring cleaning Could be cleaned up one day label Jul 19, 2022
@pinussilvestrus
Copy link
Contributor

Moving this to backlog as it was not picked for the next iteration. Feel free to disagree.

@pinussilvestrus pinussilvestrus added ready Ready to be worked on backlog Queued in backlog and removed ready Ready to be worked on labels Jul 19, 2022
@christian-konrad
Copy link
Contributor Author

christian-konrad commented Aug 1, 2022

Hi, a forum user just ran into this issue, too:

https://forum.camunda.io/t/help-use-expression-in-element-templates/39039

The problem became visible earlier as I thought. Since now at least two users reported this and the feature will become more powerful if we fix this, please put it into ready and pick it up on the next planning.

@christian-konrad christian-konrad added ready Ready to be worked on and removed backlog Queued in backlog labels Aug 1, 2022
@nikku nikku removed the ready Ready to be worked on label Nov 22, 2022
@nikku nikku added the backlog Queued in backlog label Nov 22, 2022 — with bpmn-io-tasks
@nikku
Copy link
Member

nikku commented Nov 22, 2022

Also reported from the connectors side of things: https://github.com/camunda/web-modeler/issues/3075.

Moving back to ready and hoping to be able to pick this up.

@nikku nikku added ready Ready to be worked on and removed backlog Queued in backlog labels Nov 22, 2022
@smbea
Copy link
Contributor

smbea commented Nov 22, 2022

I had a look at this and here are my discoveries:

Regarding rendering without a default values:

Regarding switching between values:

  • When: This happens when both dependant options have the same binding even though they have different conditions.

  • Why: This is because we reuse a helper function (findOldProperty) to compare a property against a template that is used in ChangeElementTemplateHandler. And the use case is not the same: ChangeElementTemplateHandler cares about maintaining/reusing XML properties, finding a property that fits fulfils that binding and doesn't compare the property label, for example.

EDIT: updated root cause here bpmn-io/bpmn-js-properties-panel#815

smbea added a commit to bpmn-io/bpmn-js-properties-panel that referenced this issue Nov 23, 2022
smbea added a commit to bpmn-io/bpmn-js-properties-panel that referenced this issue Nov 23, 2022
smbea added a commit to bpmn-io/bpmn-js-properties-panel that referenced this issue Nov 23, 2022
@nikku nikku added channel:customer in progress Currently worked on and removed ready Ready to be worked on labels Nov 24, 2022
nikku pushed a commit to bpmn-io/bpmn-js-properties-panel that referenced this issue Nov 25, 2022
@nikku nikku added the needs review Review pending label Nov 26, 2022 — with bpmn-io-tasks
@nikku nikku removed the in progress Currently worked on label Nov 26, 2022
nikku pushed a commit to bpmn-io/bpmn-js-properties-panel that referenced this issue Nov 28, 2022
@nikku nikku added the fixed upstream Requires integration of upstream change label Nov 28, 2022 — with bpmn-io-tasks
@nikku nikku removed the needs review Review pending label Nov 28, 2022
smbea added a commit to bpmn-io/bpmn-js-properties-panel that referenced this issue Nov 28, 2022
@nikku
Copy link
Member

nikku commented Nov 30, 2022

Closed via dependency bump.

@nikku
Copy link
Member

nikku commented Nov 30, 2022

capture m4q8B8_optimized

@nikku nikku closed this as completed Nov 30, 2022
@bpmn-io-tasks bpmn-io-tasks bot removed the fixed upstream Requires integration of upstream change label Nov 30, 2022
@igpetrov
Copy link

igpetrov commented Nov 30, 2022

UPD, TLDR: initial dynamic value is not pre-populated.

Hi @nikku, sorry for re-opening, just wanted to double-check if this is released (or is it even the same issue?) because this is an issue raised by one of external users.

I currently still see an issue on dev. Consider following element template and video attached.

{
  "$schema": "https://unpkg.com/@camunda/zeebe-element-templates-json-schema/resources/schema.json",
  "name": "dynamic-values",
  "id": "2b9dc4da-6acc-478c-a774-ece1b04ff988",
  "appliesTo": [
    "bpmn:ServiceTask"
  ],
  "properties": [
    {
      "id": "default-changer",
      "label": "Change the default",
      "type": "Dropdown",
      "choices": [
        {
          "name": "Set it as 1",
          "value": "set-1"
        },
        {
          "name": "Set it as 2",
          "value": "set-2"
        },
        {
          "name": "Set it as 3",
          "value": "set-3"
        }
      ],
      "binding": {
        "type": "zeebe:input",
        "name": "ignore"
      },
      "constraints": {
        "notEmpty": true
      }
    },
    {
      "label": "Test",
      "type": "String",
      "value": "1",
      "binding": {
        "type": "zeebe:input",
        "name": "test"
      },
      "condition": {
        "property": "default-changer",
        "equals": "set-1"
      }
    },
    {
      "label": "Test",
      "type": "String",
      "value": "2",
      "binding": {
        "type": "zeebe:input",
        "name": "test"
      },
      "condition": {
        "property": "default-changer",
        "equals": "set-2"
      }
    },
    {
      "label": "Test",
      "type": "String",
      "value": "3",
      "binding": {
        "type": "zeebe:input",
        "name": "test"
      },
      "condition": {
        "property": "default-changer",
        "equals": "set-3"
      }
    }
  ]
}
Screen.Recording.2022-11-30.at.18.45.50.mov

@igpetrov igpetrov reopened this Nov 30, 2022
@nikku
Copy link
Member

nikku commented Nov 30, 2022

Thanks for raising. I propose to (and will) move this to a separate issue.

@nikku
Copy link
Member

nikku commented Nov 30, 2022

Opened #3327 to track the bug you reported as a follow up.

@nikku nikku closed this as completed Nov 30, 2022
@nikku
Copy link
Member

nikku commented Dec 1, 2022

FYI @igpetrov #3327 (comment).

@nikku nikku added this to the M58 milestone Dec 7, 2022
marstamm pushed a commit to bpmn-io/bpmn-js-element-templates that referenced this issue Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working channel:customer element templates spring cleaning Could be cleaned up one day
Projects
None yet
Development

No branches or pull requests

5 participants