Optimize variable edition and use of Monaco editor in raw mode

[PaulFarault]

Which issue(s) this PR fixes

Fixes #96
Fixes #149
Fixes #164

Additional comments

• Add react-hook-form to optimize state and to simplify the userInput slice. The slice now only contains dirty variables across a single service and is emptied when switching service.
• Sync variable edition in raw and view modes, with Monaco as the raw code editor.
• Persist user settings across pages (raw/visual mode, show unused tabs).
• New variables can be added using raw mode. (Variable creation #65)
• Variable deletion in raw mode.

Agreements

• I hereby declare this contribution to be licensed under the Apache License Version 2.0, January 2004.
• I hereby agree to grant TOSIT a copyright license to use my contributions.

[PaulFarault]

Force pushed after rebase on master.

[mdrutel]

Hi,
❓ ❌ ✅ ✔️
❓ When you select the Knox service, the tdpldap variable from the group gateway_topology is empty in View mode.
But it is related to the issue #149.
❓ When you try to modify the attribute name, another variable is created instead.
❓ In Raw mode, when there's a syntax error, an unhandled error popup appears ; do we have to add a try/catch block ?
❓ If there's a syntax error, there's only an error in the console when you click on the Validate button.
❓ When you scroll down to the end of the text, only one line is displayed ; I prefer the behavior of a text editor
❓ when you add an array variable in Raw mode, in View mode you have another display :


❓ (maybe related to another issue) when you dislay an existing array, you have this : in Raw mode, then in View mode :

✔️ When you modify in Raw mode, the state is updated in View mode
✔️ The syntax error are well displayed

[PaulFarault]

Overall, the raw view seems to be fixed now and the sync between view / raw is working. View mode has some problems but those are hard to solve without any JSON schema. The default mode is now the raw one. I'll add a mention on the view page that this is still a work in progress.

When you try to modify the attribute name, another variable is created instead.

Yes that normal. Renaming a variable is not possible. Hence, it is the same as deleting/creating a new one.

In Raw mode, when there's a syntax error, an unhandled error popup appears ; do we have to add a try/catch block ?

It seems to be fixed now. The problem now appear on visual mode.

If there's a syntax error, there's only an error in the console when you click on the Validate button.

It seems to be fixed now.

When you scroll down to the end of the text, only one line is displayed ; I prefer the behavior of a text editor

I'm not sure to understand this one.

when you add an array variable in Raw mode, in View mode you have another display / (maybe related to another issue) when you dislay an existing array, you have this : in Raw mode, then in View mode

Arrays are causing some troubles. I decided to treat them as object until we make use of the json schemas.

[sergkudinov]

When you scroll down to the end of the text, only one line is displayed ; I prefer the behavior of a text editor

It is this:

[sergkudinov]

I found a bug.

To reproduce

1. Run tdp-server with docker, enabling DO_NOT_USE_IN_PRODUCTION_DISABLE_TOKEN_CHECK=True
2. Run tdp-ui with "skipAuth": true in config.json
3. Go to Spark service - http://localhost:3000/services/spark
4. Click on the "View" button of the "spark" tab
5. It outputs errors:

Uncaught TypeError: can't convert null to object
...
The above error occurred in the <VisualEditor> component:
VisualEditor@webpack-internal:///./src/components/Services/ServiceVariables/Editor/VisualEditor/index.tsx:98:51
...

It happens also on "Spark3" (http://localhost:3000/services/spark3) and nowhere else.

Reason

The reason is it defines "spark_env_client": null as an Object variable and put it to objectVariables, later on after destructuring the object it fails with on a type check.

Solution

First we have to determine if null is an object or a primitive variable.

[PaulFarault]

Thanks @sergkudinov! I fixed it on my last commit.

[sergkudinov]

Looks good to me, except this is not improved.

[PaulFarault]

Looks good to me, except #191 (comment) is not improved.

True, it doesn't seem to be any settings for this with Monaco.

[PaulFarault]

TODO :

• If there is no click outside Monaco, the value is not taken into account when validating
• Errors are display inside the console when JSON can't be parsed

[PaulFarault]

I fixed the bug that I mentioned previously.

Forced pushed to squash commits.

[mdrutel]

I tested the add/remove of a variable :

• Adding a variable : ✔️
  I get my variable
• Removing a variable : ❌
  The variable is kept, and null is set

[PaulFarault]

• Removing a variable : x
  The variable is kept, and null is set

I did it on purpose. I understood from @rpignolet that deleted variables should be set as null. Does it seems good for you Romain or am I wrong?

[rpignolet]

Does the server support variable deletion by setting it to null ? If not, in the UI, removing variable should not be possible until the server support it.

[PaulFarault]

Does the server support variable deletion by setting it to null ?

In my understanding, it is not possible to delete a variable using a PUT. The only way is to POST a new config.

If not, in the UI, removing variable should not be possible until the server support it.

It's a bit weird to prevent deleting in a raw mode. I can either:

• Continue to use PUT and ignore deleted lines for now (as you suggested)
• Use POST instead to send the whole config at each request

[rpignolet]

Use POST for now.

[sergkudinov]

I ran and tested the edditor, it seams it works without issues descussed above. However, I see different errors in the browser console. I propose to merge to master as is, because existing errors must disappear after my refactoring to RTK Query this week.

[PaulFarault]

Use POST for now.

@rpignolet I was mistaken, there is no POST on the server (only PUT, to add new variables, and PATH, to edit existing variables). Deleting variable is not possible currently. IMO the server lacks both POST and DELETE methods.

What should I do for this PR then? Leaving the deleted variable as null as it is now?

---

Thanks for your test and your input @sergkudinov! I agree, some of the errors are coming from the server answer / toast which aren't directly related to this PR.

[mdrutel]

test of the editore : OK

[rpignolet]

@PaulFarault
If you put null for a variable, I don't know if it will actually write null to the YAML file for Ansible. Also how will Ansible interpret this null, will it still merge dicts?

Currently the server does not support variable deletion, setting a variable to null doesn't necessarily make sense so I would say ignore the deletion for now and display a message above the editor stating that it is not supported at this time.

When the server will officially support it we can do the implementation on the UI side. The UI must use existing features of the server and not seek to circumvent non-existent features of the server.

[PaulFarault]

This PR should fix the PUT behavior to allow deletion : TOSIT-IO/tdp-server#122

I'll switch from PATCH to PUT in the next commit to handle variables deletion.

[sergkudinov]

It is fine from my side.