KaotoIO / kaoto-ui

Frontend for the Kaoto project to provide an easy-to-use integration framework based on Apache Camel.
https://www.kaoto.io
Apache License 2.0
89 stars 44 forks source link

Syncing the code loses the currently selected step if said step is nested #1475

Closed lordrip closed 1 year ago

lordrip commented 1 year ago

Describe the Bug

Having selected a nested step, if the code is synced, this value gets lost. This might be caused by how the IDs are calculated from this line

if (branchSteps) step.UUID = `${step.name}-${idx}-${getRandomArbitraryNumber()}`;

Steps to Reproduce the Bug or Issue

  1. Use the following yaml file as a starting point

    - from:
    uri: as2:null:null
    steps:
    - choice:
        when:
        - simple: 'This is a simple condition'
          steps: []
        otherwise:
          steps:
          - choice:
              when:
              - jq: 'This is a nested JQ condition'
                steps: []
  2. Click on the nested choice step

  3. Sync the code

  4. Notice how the selected step gets lost and the view is not right

Screenshots or Videos

Screencast from 2023-03-14 14-16-48.webm

Platform

apupier commented 1 year ago

I'm suspecting that it is causing lost data in VS Code: valueLostWhenWritingFastInVSCodeAndChanging focusInsideBranches

I was unable to reproduce outside of a branch. Neither inside branches with OperateFirst.

Increasing the priority then

kahboom commented 1 year ago

@lordrip - So just to confirm, the problem here is that the corresponding step extension does not get reloaded, right? So it falls back to the default config?

lordrip commented 1 year ago

Actually, I think that it's more related to the fact that we don't keep a consistent ID for nested steps, as opposite to regular steps that have the name and their array position.

This happens because we have a selected step, let's say "nested-127893612", now after a code sync, this ID not longer exist, therefore the step details doesn't know what to show.

I was thinking on creating an ID based on their parent step -> branch position and finally name and array, so this way even if we get a code sync, the IDs are consistent

Delawen commented 1 year ago

@lordrip consider that we will always have the case in which some previous branch or step is removed and therefore the ID does no longer fit

kahboom commented 1 year ago

Relates to https://github.com/KaotoIO/kaoto-ui/issues/1371, I guess if the ID is not matching or if there is some change to the selected step, maybe instead of us trying to find the new selected step and its views, we should just close the step detail view?

lordrip commented 1 year ago

@lordrip consider that we will always have the case in which some previous branch or step is removed and therefore the ID does no longer fit

Let me check if I'm getting this right, for this YAML:

- from:
    uri: activemq:queue:null
    steps:
    - choice:
        when:
        - simple: '{{header}}'
          steps:
          - to:
              uri: sql:null
        - simple: '{{body}}'
          steps:
          - to:
              uri: mongodb:null
        otherwise:
          steps:
          - to:
              uri: sql-stored:null

the IDs would be:

- activemq: activemq-0
- choice: choice-1
    - sql: choice-1-[branch-0]-sql-0
    - mongodb: choice-1-[branch-1]-mongodb-1
    - sql-stored: choice-1-[branch-2]-sqlstored-2

In that sense, if something gets deleted, it will be replaced but we still show the right values when looking for that ID

kahboom commented 1 year ago

In that sense, if something gets deleted, it will be replaced but we still show the right values when looking for that ID

@lordrip If I've understood the proposed solution correctly, when something gets deleted then that path will no longer be valid. For example, let's assume that, from your example, branch-1 has more than one step and the selected step is mongodb. If the step before it gets deleted, then mongo-db's path will now be different.

We can still use the old path of mongo-db (the selected step) and check the that the new step in its place has a same type/connector, and close the panel otherwise. Let's assume for a second that the step after mongodb is also a mongodb step, if we automatically select that step (because it occupies the same path now and has the same step connector/type), I anticipate people will report this as a bug since it is now selecting and showing a step different from the one originally selected and can be confusing. So, it's possible but is it a good option? Or did I misunderstand something?

I don't like the idea of closing the panel either though. 🤔 I guess it's a matter of choosing the most intuitive.

kahboom commented 1 year ago

I'm suspecting that it is causing lost data in VS Code:

@apupier I almost think that is a separate issue. I do wonder if this is reason enough to have the save button on all step extensions and configuration forms, since the debounce and throttle can sometimes cause more problems when users are trying to work very quickly.

lordrip commented 1 year ago

I do wonder if this is reason enough to have the save button on all step extensions and configuration forms.

Well, I think that it will fix the Step details tab -> Code editor, but wouldn't that interfere in the opposite direction :thinking: Edit: Maybe not, because we don't automatically update the Step extension while typing in the code editor.

I anticipate people will report this as a bug since it is now selecting and showing a step different from the one originally selected and can be confusing.

Yes, that's also true... that could possibly happen.