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.

Sign up for GitHub

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

Migrate base automation config to plurals #22053

Merged
merged 5 commits into from
Sep 24, 2024
Merged

Migrate base automation config to plurals #22053

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
22f0392
Migrate base automation config to plurals
bramkragten Sep 22, 2024
7be80d3
revert
bramkragten Sep 22, 2024
6c0815e
Update hat-script-graph.ts
bramkragten Sep 22, 2024
2b11f2f
Make traces work with both new and old config
bramkragten Sep 24, 2024
35c3624
Adjust validateConfig
bramkragten Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions gallery/src/data/traces/basic_trace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,20 +217,20 @@ export const basicTrace: DemoTrace = {
id: "1615419646544",
alias: "Ensure Party mode",
description: "",
trigger: [
triggers: [
{
platform: "state",
entity_id: "input_boolean.toggle_1",
},
],
condition: [
conditions: [
{
condition: "template",
alias: "Test if Paulus is home",
value_template: "{{ true }}",
},
],
action: [
actions: [
{
action: "input_boolean.toggle",
target: {
Expand Down
4 changes: 2 additions & 2 deletions gallery/src/data/traces/mock-demo-trace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ export const mockDemoTrace = (
],
},
config: {
trigger: [],
action: [],
triggers: [],
actions: [],
},
context: {
id: "abcd",
Expand Down
4 changes: 2 additions & 2 deletions gallery/src/data/traces/motion-light-trace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,15 @@ export const motionLightTrace: DemoTrace = {
config: {
mode: "restart",
max_exceeded: "silent",
trigger: [
triggers: [
{
platform: "state",
entity_id: "binary_sensor.pauluss_macbook_pro_camera_in_use",
from: "off",
to: "on",
},
],
action: [
actions: [
{
action: "light.turn_on",
target: {
Expand Down
2 changes: 1 addition & 1 deletion gallery/src/pages/automation/describe-action.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ const ACTIONS = [
];

const initialAction: Action = {
service: "light.turn_on",
action: "light.turn_on",
target: {
entity_id: "light.kitchen",
},
Expand Down
19 changes: 12 additions & 7 deletions src/components/trace/hat-script-graph.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,10 +569,15 @@ export class HatScriptGraph extends LitElement {
}

protected render() {
const triggerKey = "triggers" in this.trace.config ? "triggers" : "trigger";
const conditionKey =
"conditions" in this.trace.config ? "conditions" : "condition";
const actionKey = "actions" in this.trace.config ? "actions" : "action";

const paths = Object.keys(this.trackedNodes);
const trigger_nodes =
"trigger" in this.trace.config
? flattenTriggers(ensureArray(this.trace.config.trigger)).map(
triggerKey in this.trace.config
? flattenTriggers(ensureArray(this.trace.config[triggerKey])).map(
(trigger, i) => this.render_trigger(trigger, i)
)
: undefined;
Expand All @@ -584,14 +589,14 @@ export class HatScriptGraph extends LitElement {
${trigger_nodes}
</hat-graph-branch>`
: ""}
${"condition" in this.trace.config
? html`${ensureArray(this.trace.config.condition)?.map(
${conditionKey in this.trace.config
? html`${ensureArray(this.trace.config[conditionKey])?.map(
(condition, i) => this.render_condition(condition, i)
)}`
: ""}
${"action" in this.trace.config
? html`${ensureArray(this.trace.config.action).map((action, i) =>
this.render_action_node(action, `action/${i}`)
${actionKey in this.trace.config
? html`${ensureArray(this.trace.config[actionKey]).map(
(action, i) => this.render_action_node(action, `action/${i}`)
)}`
: ""}
${"sequence" in this.trace.config
Expand Down
42 changes: 38 additions & 4 deletions src/data/automation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,14 @@ export interface ManualAutomationConfig {
id?: string;
alias?: string;
description?: string;
trigger: Trigger | Trigger[];
triggers: Trigger | Trigger[];
/** @deprecated Use `triggers` instead */
trigger?: Trigger | Trigger[];
conditions?: Condition | Condition[];
/** @deprecated Use `conditions` instead */
condition?: Condition | Condition[];
actions: Action | Action[];
/** @deprecated Use `actions` instead */
action?: Action | Action[];
mode?: (typeof MODES)[number];
max?: number;
Expand Down Expand Up @@ -362,22 +368,50 @@ export const normalizeAutomationConfig = <
>(
config: T
): T => {
config = migrateAutomationConfig(config);

// Normalize data: ensure triggers, actions and conditions are lists
// Happens when people copy paste their automations into the config
for (const key of ["trigger", "condition", "action"]) {
for (const key of ["triggers", "conditions", "actions"]) {
const value = config[key];
if (value && !Array.isArray(value)) {
config[key] = [value];
}
}

if (config.action) {
config.action = migrateAutomationAction(config.action);
if (config.actions) {
config.actions = migrateAutomationAction(config.actions);
}

return config;
};

export const migrateAutomationConfig = <
T extends Partial<AutomationConfig> | AutomationConfig,
>(
config: T
) => {
if ("trigger" in config) {
if (!("triggers" in config)) {
config.triggers = config.trigger;
}
delete config.trigger;
}
if ("condition" in config) {
if (!("conditions" in config)) {
config.conditions = config.condition;
}
delete config.condition;
}
if ("action" in config) {
if (!("actions" in config)) {
config.actions = config.action;
}
delete config.action;
}
return config;
};
Comment on lines +389 to +413
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using the delete operator to improve performance

Using the delete operator on objects can degrade performance because it alters the object's shape, leading to deoptimizations in JavaScript engines. Consider setting the properties to undefined instead, or refactoring the code to avoid removing properties.

Apply this diff to replace delete operations with assignments to undefined:

      if ("trigger" in config) {
        if (!("triggers" in config)) {
          config.triggers = config.trigger;
        }
-       delete config.trigger;
+       config.trigger = undefined;
      }
      if ("condition" in config) {
        if (!("conditions" in config)) {
          config.conditions = config.condition;
        }
-       delete config.condition;
+       config.condition = undefined;
      }
      if ("action" in config) {
        if (!("actions" in config)) {
          config.actions = config.action;
        }
-       delete config.action;
+       config.action = undefined;
      }

Alternatively, consider restructuring the code to avoid the need to remove properties, such as creating a new object to store the migrated configuration.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const migrateAutomationConfig = <
T extends Partial<AutomationConfig> | AutomationConfig,
>(
config: T
) => {
if ("trigger" in config) {
if (!("triggers" in config)) {
config.triggers = config.trigger;
}
delete config.trigger;
}
if ("condition" in config) {
if (!("conditions" in config)) {
config.conditions = config.condition;
}
delete config.condition;
}
if ("action" in config) {
if (!("actions" in config)) {
config.actions = config.action;
}
delete config.action;
}
return config;
};
export const migrateAutomationConfig = <
T extends Partial<AutomationConfig> | AutomationConfig,
>(
config: T
) => {
if ("trigger" in config) {
if (!("triggers" in config)) {
config.triggers = config.trigger;
}
config.trigger = undefined;
}
if ("condition" in config) {
if (!("conditions" in config)) {
config.conditions = config.condition;
}
config.condition = undefined;
}
if ("action" in config) {
if (!("actions" in config)) {
config.actions = config.action;
}
config.action = undefined;
}
return config;
};
Tools
Biome

[error] 398-398: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

[error] 404-404: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

[error] 410-410: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


export const flattenTriggers = (
triggers: undefined | (Trigger | TriggerList)[]
): Trigger[] => {
Expand Down
2 changes: 1 addition & 1 deletion src/data/script.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ export const getActionType = (action: Action): ActionType => {
if ("set_conversation_response" in action) {
return "set_conversation_response";
}
if ("action" in action) {
if ("action" in action || "service" in action) {
if ("metadata" in action) {
if (is(action, activateSceneActionStruct)) {
return "activate_scene";
Expand Down
13 changes: 12 additions & 1 deletion src/data/trace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,21 @@ export const getDataFromPath = (
const asNumber = Number(raw);

if (isNaN(asNumber)) {
const tempResult = result[raw];
let tempResult = result[raw];
if (!tempResult && raw === "sequence") {
continue;
}

if (!tempResult && raw === "trigger") {
tempResult = result.triggers;
}
if (!tempResult && raw === "condition") {
tempResult = result.conditions;
}
if (!tempResult && raw === "action") {
tempResult = result.actions;
}

if (raw === "trigger") {
result = flattenTriggers(tempResult);
} else {
Expand Down
12 changes: 8 additions & 4 deletions src/panels/config/automation/ha-automation-editor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
fetchAutomationFileConfig,
getAutomationEditorInitData,
getAutomationStateConfig,
migrateAutomationConfig,
normalizeAutomationConfig,
saveAutomationConfig,
showAutomationEditor,
Expand Down Expand Up @@ -520,9 +521,9 @@ export class HaAutomationEditor extends KeyboardShortcutMixin(LitElement) {
return;
}
const validation = await validateConfig(this.hass, {
trigger: this._config.trigger,
condition: this._config.condition,
action: this._config.action,
trigger: this._config.triggers,
condition: this._config.conditions,
action: this._config.actions,
});
this._validationErrors = (
Object.entries(validation) as Entries<typeof validation>
Expand Down Expand Up @@ -637,7 +638,10 @@ export class HaAutomationEditor extends KeyboardShortcutMixin(LitElement) {
if (!ev.detail.isValid) {
return;
}
this._config = { id: this._config?.id, ...ev.detail.value };
this._config = {
id: this._config?.id,
...migrateAutomationConfig(ev.detail.value),
};
this._errors = undefined;
this._dirty = true;
}
Expand Down
16 changes: 8 additions & 8 deletions src/panels/config/automation/manual-automation-editor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class HaManualAutomationEditor extends LitElement {
<ha-automation-trigger
role="region"
aria-labelledby="triggers-heading"
.triggers=${this.config.trigger || []}
.triggers=${this.config.triggers || []}
.path=${["trigger"]}
@value-changed=${this._triggerChanged}
@item-moved=${this._itemMoved}
Expand Down Expand Up @@ -119,7 +119,7 @@ export class HaManualAutomationEditor extends LitElement {
></ha-icon-button>
</a>
</div>
${!ensureArray(this.config.condition)?.length
${!ensureArray(this.config.conditions)?.length
? html`<p>
${this.hass.localize(
"ui.panel.config.automation.editor.conditions.description",
Expand All @@ -131,7 +131,7 @@ export class HaManualAutomationEditor extends LitElement {
<ha-automation-condition
role="region"
aria-labelledby="conditions-heading"
.conditions=${this.config.condition || []}
.conditions=${this.config.conditions || []}
.path=${["condition"]}
@value-changed=${this._conditionChanged}
@item-moved=${this._itemMoved}
Expand Down Expand Up @@ -160,7 +160,7 @@ export class HaManualAutomationEditor extends LitElement {
</a>
</div>
</div>
${!ensureArray(this.config.action)?.length
${!ensureArray(this.config.actions)?.length
? html`<p>
${this.hass.localize(
"ui.panel.config.automation.editor.actions.description"
Expand All @@ -171,7 +171,7 @@ export class HaManualAutomationEditor extends LitElement {
<ha-automation-action
role="region"
aria-labelledby="actions-heading"
.actions=${this.config.action}
.actions=${this.config.actions || []}
.path=${["action"]}
@value-changed=${this._actionChanged}
@item-moved=${this._itemMoved}
Expand All @@ -185,7 +185,7 @@ export class HaManualAutomationEditor extends LitElement {
private _triggerChanged(ev: CustomEvent): void {
ev.stopPropagation();
fireEvent(this, "value-changed", {
value: { ...this.config!, trigger: ev.detail.value as Trigger[] },
value: { ...this.config!, triggers: ev.detail.value as Trigger[] },
});
}

Expand All @@ -194,15 +194,15 @@ export class HaManualAutomationEditor extends LitElement {
fireEvent(this, "value-changed", {
value: {
...this.config!,
condition: ev.detail.value as Condition[],
conditions: ev.detail.value as Condition[],
},
});
}

private _actionChanged(ev: CustomEvent): void {
ev.stopPropagation();
fireEvent(this, "value-changed", {
value: { ...this.config!, action: ev.detail.value as Action[] },
value: { ...this.config!, actions: ev.detail.value as Action[] },
});
}

Expand Down
Loading