Skip to content

Check if config is compatible with UI editor#2137

Merged
bramkragten merged 9 commits intodevfrom
check-config-ui-editor
Nov 29, 2018
Merged

Check if config is compatible with UI editor#2137
bramkragten merged 9 commits intodevfrom
check-config-ui-editor

Conversation

@bramkragten
Copy link
Copy Markdown
Member

@bramkragten bramkragten commented Nov 28, 2018

Just for the entities editor now.
Extra keys are not allowed:
image
Some config is not supported by the UI editor:
image
And missing required elements:
image

Fixes: #2124

@ghost ghost assigned bramkragten Nov 28, 2018
@ghost ghost added the in progress label Nov 28, 2018
@bramkragten bramkragten force-pushed the check-config-ui-editor branch from bcb1c2c to 68a36f5 Compare November 28, 2018 12:38
const optionalEntKeys = ["name", "icon"];
const allEntKeys = optionalEntKeys.concat(requiredEntKeys);

requiredKeys.forEach((key) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's create a helper so we can write code like this:

verify(config, reqKeys, optKeys);

for (const entity of config.entities) {
  verify(entity, reqEntKeys, optEntKeys);
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah was thinking the same

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


const entitiesConfigStruct = struct.union([
{
entity: "string",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a superstruct that validates entity ID and icon to be valid format?

  • entity ID: should contain a .
  • icon: should contain :

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we should! 👍

"string",
]);

const cardConfigStruct = struct({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So nice!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering, this is something we should probably export from the hui-entities-card so it can validate the config there too ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nevermind, the config editor only supports a subset so needs their own.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, but the struct of the editor is a little bit simpler as it doesn't support special rows etc.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yet 😉

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can have our special struct register types that validate objects.

So we can have something like

const struct = superstruct({
  'divider-row': validateDividerRow,
  'entity-row': validateEntityRow,
})

const validate = struct({
  entities: ['divider-row|entity-row']
})

Copy link
Copy Markdown
Member Author

@bramkragten bramkragten Nov 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And use a struct inside validateDividerRow? Or just use a struct instead of superstruct, will make code a little bit more ugly though...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah use a struct inside the validator, if that can work. Was just thinking out loud.

@balloob
Copy link
Copy Markdown
Member

balloob commented Nov 29, 2018

Very nice!

@balloob
Copy link
Copy Markdown
Member

balloob commented Nov 29, 2018

This can be merged and the other things can wait for a future PR

@bramkragten bramkragten merged commit 90cea56 into dev Nov 29, 2018
@delete-merged-branch delete-merged-branch bot deleted the check-config-ui-editor branch November 29, 2018 14:00
@ghost ghost removed the in progress label Nov 29, 2018
@balloob balloob mentioned this pull request Dec 5, 2018
@github-actions github-actions bot locked and limited conversation to collaborators Jul 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When card type is changed, reload editor element

3 participants