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

Initial dynamic element template value is not pre-populated. #3327

Closed
nikku opened this issue Nov 30, 2022 · 9 comments
Closed

Initial dynamic element template value is not pre-populated. #3327

nikku opened this issue Nov 30, 2022 · 9 comments
Assignees
Labels
Milestone

Comments

@nikku
Copy link
Member

nikku commented Nov 30, 2022

Describe the bug

Related to #3029 whenever I initially apply an element template the initial value is not applied.

Steps to reproduce

Consider following element template and video attached (works in web modeler and Desktop Modeler (nightly)):

Screen.Recording.2022-11-30.at.18.45.50.mov

Element template attached below ⬇️.

Expected Behavior

When initially applying an element template with a dynamic binding the valid effects (for which the condition is true) is applied.

Additional Context

Element template to reproduce this:

{
  "$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"
      }
    }
  ]
}

Originally posted by @igpetrov in #3029 (comment)


Depends on #3327.

@nikku

This comment was marked as outdated.

@nikku
Copy link
Member Author

nikku commented Dec 1, 2022

Nevermind #3327 (comment).


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

As no initial value is established there is nothing selected yet. Despite that we still render the first value in the dropdown. The obvious resolution is to not render a selection yet (#3029 (comment)).

This is a visual glitch only; in the screen capture you shared Set it as 1 is not established (yet), but can also not be chosen by the user.

@nikku
Copy link
Member Author

nikku commented Dec 1, 2022

It took me forever (unfortunately!) to reproduce the issue.

The bug is unrelated to dropdowns, but due to the fact that we establish an empty binding for zeebe:input. The scenario works fine for zeebe:property and other bindings:

{
  "$schema": "https://unpkg.com/@camunda/zeebe-element-templates-json-schema/resources/schema.json",
  "name": "Condition (dropdown, no default)",
  "id": "conditional-dropdown-no-default-value",
  "appliesTo": [
    "bpmn:Task"
  ],
  "properties": [
    {
      "id": "operation",
      "label": "operation",
      "description": "Operation to be done",
      "type": "Dropdown",
      "choices": [
        {
          "name": "Action 1",
          "value": "action1"
        },
        {
          "name": "Action 2",
          "value": "action2"
        }
      ],
      "binding": {
        "type": "zeebe:property",
        "name": "functionType"
      }
    }
  ]
}

@nikku nikku added the in progress Currently worked on label Dec 1, 2022 — with bpmn-io-tasks
@nikku
Copy link
Member Author

nikku commented Dec 1, 2022

As discussed @smbea can you take this one over?

I root caused it and from what it seems like we don't properly initialize zeebe:input bindings (#3327 (comment)).

@nikku nikku assigned smbea and nikku and unassigned nikku Dec 1, 2022
@smbea
Copy link
Contributor

smbea commented Dec 1, 2022

When there is no default value, would the expected behaviour for inputs be:

  1. Create an input with a source set to '' - we do this for zeebe:property
  2. or don't create the input at all - we do this for zeebe:taskHeaders

Both would fix this issue. From the tests, it leads me to believe the first option is what is expected.

@nikku
Copy link
Member Author

nikku commented Dec 1, 2022

We're actually inconsistent here? 🙈

I'd advocate for 2️⃣, and also align zeebe:property to it.

Rationale: Empty inputs are rejected by the engine. We should not allow that broken state to be created in the first palace.

Probably 1️⃣ is an oversight on our end. We should not create empty zeebe:property (for the same reason), too. At least that is how I see it.

@smbea
Copy link
Contributor

smbea commented Dec 1, 2022

We add it when the property is non-optional. We even have a test case for this (that ensures the problem we are fixing). But I guess we shouldn't add it ever if there is no value (it it's non optional)?

We actually have a test case for optional/non-optional inputs ensuring the issue we are fixing 🤪 https://github.com/bpmn-io/bpmn-js-properties-panel/blob/2a77249f751b2f8739115bf4298355bd50d8bbb0/test/spec/provider/cloud-element-templates/cmd/ChangeElementTemplateHandler.spec.js#L2040

@smbea
Copy link
Contributor

smbea commented Dec 1, 2022

I have a fix for this issue if this is the route we want to go bpmn-io/bpmn-js-properties-panel#824. It also aligns the behaviour with zeebe:property

smbea added a commit to bpmn-io/bpmn-js-properties-panel that referenced this issue Dec 1, 2022
nikku pushed a commit to bpmn-io/bpmn-js-properties-panel that referenced this issue Dec 2, 2022
@nikku nikku added this to the M60 milestone Dec 9, 2022
nikku added a commit to bpmn-io/bpmn-js-properties-panel that referenced this issue Dec 15, 2022
nikku added a commit to bpmn-io/bpmn-js-properties-panel that referenced this issue Dec 15, 2022
@nikku nikku added the fixed upstream Requires integration of upstream change label Dec 15, 2022 — with bpmn-io-tasks
@nikku nikku removed the in progress Currently worked on label Dec 15, 2022
@nikku
Copy link
Member Author

nikku commented Dec 15, 2022

Fixed via bpmn-io/bpmn-js-properties-panel@b3248fe in bpmn-js-properties-panel. Awaiting integration.

philippfromme added a commit that referenced this issue Jan 3, 2023
philippfromme added a commit that referenced this issue Jan 4, 2023
philippfromme added a commit that referenced this issue Jan 4, 2023
@bpmn-io-tasks bpmn-io-tasks bot removed the fixed upstream Requires integration of upstream change label Jan 4, 2023
lzgabel pushed a commit to lzgabel/camunda-modeler that referenced this issue Apr 4, 2023
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
Projects
None yet
Development

No branches or pull requests

2 participants