Closed PaulFarault closed 1 year ago
Force pushed after rebase on master.
Hi, :question: :x: :white_check_mark: :heavy_check_mark: :question: 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. :question: When you try to modify the attribute name, another variable is created instead. :question: In Raw mode, when there's a syntax error, an unhandled error popup appears ; do we have to add a try/catch block ? :question: If there's a syntax error, there's only an error in the console when you click on the Validate button. :question: When you scroll down to the end of the text, only one line is displayed ; I prefer the behavior of a text editor :question: when you add an array variable in Raw mode, in View mode you have another display : :question: (maybe related to another issue) when you dislay an existing array, you have this : in Raw mode, then in View mode :
:heavy_check_mark: When you modify in Raw mode, the state is updated in View mode :heavy_check_mark: The syntax error are well displayed
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.
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:
I found a bug.
DO_NOT_USE_IN_PRODUCTION_DISABLE_TOKEN_CHECK=True
"skipAuth": true
in config.jsonUncaught 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.
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.
First we have to determine if null
is an object or a primitive variable.
Thanks @sergkudinov! I fixed it on my last commit.
Looks good to me, except https://github.com/TOSIT-IO/tdp-ui/pull/191#issuecomment-1499032850 is not improved.
True, it doesn't seem to be any settings for this with Monaco.
TODO :
I fixed the bug that I mentioned previously.
Forced pushed to squash commits.
I tested the add/remove of a variable :
null
is set
- 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?
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.
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:
PUT
and ignore deleted lines for now (as you suggested)POST
instead to send the whole config at each requestUse POST
for now.
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.
@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.
This PR should fix the PUT behavior to allow deletion : https://github.com/TOSIT-IO/tdp-server/pull/122
I'll switch from PATCH to PUT in the next commit to handle variables deletion.
It is fine from my side.
Which issue(s) this PR fixes
Fixes #96 Fixes #149 Fixes #164
Additional comments
react-hook-form
to optimize state and to simplify theuserInput
slice. The slice now only contains dirty variables across a single service and is emptied when switching service.Agreements