Normalize offsets in sun and calendar trigger descriptions

src/data/automation.ts

@@ -5,7 +5,10 @@ import type {
   HassServiceTarget,
 } from "home-assistant-js-websocket";
 import { ensureArray } from "../common/array/ensure-array";
+import { createDurationData } from "../common/datetime/create_duration_data";
+import { applyDurationSign } from "../common/datetime/normalize_duration";
 import type { WeekdayShort } from "../common/datetime/weekday";
+import { computeDomain } from "../common/entity/compute_domain";
 import { navigate } from "../common/navigate";
 import type { LocalizeKeys } from "../common/translations/localize";
 import { createSearchParam } from "../common/url/search-params";
@@ -518,6 +521,26 @@ export const pickRowConfig = <T extends object>(
   return config as Partial<T>;
 };
 
+// Older sun and calendar triggers set a positive offset with an offset type,
+// which the backend folds into the sign of the offset.
+const OFFSET_TYPE_TRIGGER_DOMAINS = ["sun", "calendar"];
+
+const foldOffsetType = (
+  options: Record<string, unknown>
+): Record<string, unknown> => {
+  const { offset_type: offsetType, ...folded } = options;
+  const offset = createDurationData(
+    folded.offset as string | number | ForDict | undefined
+  );
+  if (offset && !Object.values(offset).every(Number.isFinite)) {
+    return options;
+  }
+  if (offsetType === "before" && offset) {
+    folded.offset = applyDurationSign(offset, true);
+  }
+  return folded;
+};
+
 export const migrateAutomationTrigger = (
   trigger: Trigger | Trigger[],
   report?: AutomationMigrationReport
@@ -558,6 +581,13 @@ export const migrateAutomationTrigger = (
         }
       }
     }
+    if (
+      trigger.options &&
+      "offset_type" in trigger.options &&
+      OFFSET_TYPE_TRIGGER_DOMAINS.includes(computeDomain(trigger.trigger))
+    ) {
+      trigger.options = foldOffsetType(trigger.options);
+    }
   }
 
   return trigger;

src/data/automation_i18n.ts

@@ -1,6 +1,8 @@
 import { TZDate } from "@date-fns/tz";
 import type { HassConfig, HassEntity } from "home-assistant-js-websocket";
 import { ensureArray } from "../common/array/ensure-array";
+import { createDurationData } from "../common/datetime/create_duration_data";
+import { durationDataToSeconds } from "../common/datetime/duration_to_seconds";
 import {
   formatDurationDigital,
   formatDurationLong,
@@ -10,6 +12,7 @@ import {
   formatTime,
   formatTimeWithSeconds,
 } from "../common/datetime/format_time";
+import { normalizeDuration } from "../common/datetime/normalize_duration";
 import secondsToDuration from "../common/datetime/seconds_to_duration";
 import { sortWeekdays } from "../common/datetime/sort_weekdays";
 import { computeAttributeNameDisplay } from "../common/entity/compute_attribute_display";
@@ -413,16 +416,7 @@ const describeLegacyTrigger = (
 
   // Sun Trigger
   if (trigger.trigger === "sun" && trigger.event) {
-    let duration = "";
-    if (trigger.offset) {
-      if (typeof trigger.offset === "number") {
-        duration = secondsToDuration(trigger.offset)!;
-      } else if (typeof trigger.offset === "string") {
-        duration = trigger.offset;
-      } else {
-        duration = JSON.stringify(trigger.offset);
-      }
-    }
+    const duration = formatSunOffset(hass, trigger.offset);
 
     return hass.localize(
       trigger.event === "sunset"
@@ -831,19 +825,16 @@ const describeLegacyTrigger = (
 
     let offsetChoice = "other";
     let offset = "";
-    if (typeof trigger.offset === "string" && trigger.offset) {
-      offsetChoice = trigger.offset.startsWith("-") ? "before" : "after";
-      const parts = trigger.offset.startsWith("-")
-        ? trigger.offset.substring(1).split(":")
-        : trigger.offset.split(":");
-      const duration = {
-        hours: parts.length > 0 ? +parts[0] : 0,
-        minutes: parts.length > 1 ? +parts[1] : 0,
-        seconds: parts.length > 2 ? +parts[2] : 0,
-      };
-      offset = formatDurationLong(hass.locale, duration);
-      if (offset === "") {
-        offsetChoice = "other";
+    const duration = createDurationData(trigger.offset);
+    if (duration) {
+      const normalized = normalizeDuration(duration, {
+        enableDay: true,
+        enableSecond: true,
+        enableMillisecond: true,
+      });
+      offset = formatDurationLong(hass.locale, normalized.duration);
+      if (offset) {
+        offsetChoice = normalized.negative ? "before" : "after";
       }
     }
 
@@ -874,9 +865,17 @@ const formatSunOffset = (
   if (typeof offset === "string") {
     return offset;
   }
+  if (!durationDataToSeconds(offset)) {
+    return "";
+  }
   try {
-    const formatted = formatDurationDigital(hass.locale, offset);
-    return formatted.startsWith("-") ? formatted : `+${formatted}`;
+    const normalized = normalizeDuration(offset, {
+      enableDay: false,
+      enableSecond: true,
+      enableMillisecond: true,
+    });
+    const formatted = formatDurationDigital(hass.locale, normalized.duration);
+    return `${normalized.negative ? "-" : "+"}${formatted}`;
   } catch (_e) {
     return JSON.stringify(offset);
   }

src/panels/config/automation/condition/types/ha-automation-condition-sun.ts

@@ -65,7 +65,7 @@ export class HaSunCondition extends LitElement implements ConditionElement {
                 name: "after_offset",
                 selector: {
                   duration: {
-                    allow_negative: true,
+                    mode: "offset",
                   },
                 },
               },
@@ -96,7 +96,7 @@ export class HaSunCondition extends LitElement implements ConditionElement {
                 name: "before_offset",
                 selector: {
                   duration: {
-                    allow_negative: true,
+                    mode: "offset",
                   },
                 },
               },

src/panels/config/automation/ha-automation-row-options.ts

@@ -5,16 +5,22 @@ import memoizeOne from "memoize-one";
 import { consume } from "../../../common/decorators/consume";
 import { createDurationData } from "../../../common/datetime/create_duration_data";
 import { formatDurationNarrow } from "../../../common/datetime/format_duration";
+import { normalizeDuration } from "../../../common/datetime/normalize_duration";
 import { hasTemplate } from "../../../common/string/has-template";
 import type { ForDict } from "../../../data/automation";
 import { internationalizationContext } from "../../../data/context";
 import { rowSummaryStyles } from "./styles";
 
+const SUMMARY_UNITS = {
+  enableDay: true,
+  enableSecond: true,
+  enableMillisecond: true,
+};
+
 interface HaAutomationRowOptionsConfig {
   options?: {
     for?: string | number | ForDict;
     offset?: string | number | ForDict;
-    offset_type?: "before" | "after";
   };
   timeout?: string | number | ForDict;
 }
@@ -63,20 +69,18 @@ export class HaAutomationRowOptions extends LitElement {
             this._i18n.localize(
               "ui.panel.config.automation.editor.row_options.for",
               {
-                duration: forDuration,
+                duration: forDuration.text,
               }
             )
           );
         }
 
-        const offsetDuration = this._duration(options.offset);
-        if (offsetDuration) {
-          const offsetType =
-            options.offset_type === "before" ? "before" : "after";
+        const offset = this._duration(options.offset);
+        if (offset) {
           parts.push(
             this._i18n.localize(
-              `ui.panel.config.automation.editor.row_options.offset_${offsetType}`,
-              { offset: offsetDuration }
+              `ui.panel.config.automation.editor.row_options.offset_${offset.negative ? "before" : "after"}`,
+              { offset: offset.text }
             )
           );
         }
@@ -88,7 +92,7 @@ export class HaAutomationRowOptions extends LitElement {
           parts.push(
             this._i18n.localize(
               "ui.panel.config.automation.editor.row_options.timeout",
-              { duration: timeoutDuration }
+              { duration: timeoutDuration.text }
             )
           );
         }
@@ -98,7 +102,9 @@ export class HaAutomationRowOptions extends LitElement {
     }
   );
 
-  private _duration(value: unknown): string | undefined {
+  private _duration(
+    value: unknown
+  ): { negative: boolean; text: string } | undefined {
     if (value === undefined || hasTemplate(value)) {
       return undefined;
     }
@@ -113,7 +119,9 @@ export class HaAutomationRowOptions extends LitElement {
     ) {
       return undefined;
     }
-    return formatDurationNarrow(this._i18n.locale, duration);
+    const normalized = normalizeDuration(duration, SUMMARY_UNITS);
+    const text = formatDurationNarrow(this._i18n.locale, normalized.duration);
+    return text ? { negative: normalized.negative, text } : undefined;
   }
 
   static styles = rowSummaryStyles;

src/panels/config/automation/trigger/types/ha-automation-trigger-calendar.ts

@@ -5,10 +5,7 @@ import { fireEvent } from "../../../../../common/dom/fire_event";
 import type { CalendarTrigger } from "../../../../../data/automation";
 import type { HomeAssistant } from "../../../../../types";
 import type { TriggerElement } from "../ha-automation-trigger-row";
-import type { HaDurationData } from "../../../../../components/ha-duration-input";
 import "../../../../../components/ha-form/ha-form";
-import { createDurationData } from "../../../../../common/datetime/create_duration_data";
-import { durationDataToSeconds } from "../../../../../common/datetime/duration_to_seconds";
 import type { LocalizeFunc } from "../../../../../common/translations/localize";
 import type { SchemaUnion } from "../../../../../components/ha-form/types";
 
@@ -47,25 +44,10 @@ export class HaCalendarTrigger extends LitElement implements TriggerElement {
             ],
           ],
         },
-        { name: "offset", required: true, selector: { duration: {} } },
         {
-          name: "offset_type",
-          type: "select",
+          name: "offset",
           required: true,
-          options: [
-            [
-              "before",
-              localize(
-                "ui.panel.config.automation.editor.triggers.type.calendar.before"
-              ),
-            ],
-            [
-              "after",
-              localize(
-                "ui.panel.config.automation.editor.triggers.type.calendar.after"
-              ),
-            ],
-          ],
+          selector: { duration: { enable_day: true, mode: "offset" } },
         },
       ] as const
   );
@@ -81,34 +63,10 @@ export class HaCalendarTrigger extends LitElement implements TriggerElement {
 
   protected render() {
     const schema = this._schema(this.hass.localize);
-    // Convert from string representation to ha form duration representation
-    const trigger_offset = this.trigger.offset;
-    // Copy, `createDurationData` returns the input object as-is for dict values.
-    const duration: HaDurationData = { ...createDurationData(trigger_offset)! };
-    let offset_type = "after";
-    if (durationDataToSeconds(duration) < 0) {
-      // A negative offset negates the whole period, and the sign is shown by
-      // the separate before/after select instead.
-      if (duration.days) {
-        duration.days = Math.abs(duration.days);
-      }
-      duration.hours = Math.abs(duration.hours ?? 0);
-      duration.minutes = Math.abs(duration.minutes ?? 0);
-      duration.seconds = Math.abs(duration.seconds ?? 0);
-      if (duration.milliseconds) {
-        duration.milliseconds = Math.abs(duration.milliseconds);
-      }
-      offset_type = "before";
-    }
-    const data = {
-      ...this.trigger,
-      offset: duration,
-      offset_type: offset_type,
-    };
     return html`
       <ha-form
         .schema=${schema}
-        .data=${data}
+        .data=${this.trigger}
         .hass=${this.hass}
         .disabled=${this.disabled}
         .computeLabel=${this._computeLabelCallback}
@@ -119,18 +77,7 @@ export class HaCalendarTrigger extends LitElement implements TriggerElement {
 
   private _valueChanged(ev: CustomEvent): void {
     ev.stopPropagation();
-    // Convert back to duration string representation
-    const duration = ev.detail.value.offset;
-    const offsetType = ev.detail.value.offset_type === "before" ? "-" : "";
-    const h = (duration.days ?? 0) * 24 + (duration.hours ?? 0);
-    const m = duration.minutes ?? 0;
-    const s = (duration.seconds ?? 0) + (duration.milliseconds ?? 0) / 1000;
-    const newTrigger = {
-      ...ev.detail.value,
-      offset: `${offsetType}${h}:${m}:${s}`,
-    };
-    delete newTrigger.offset_type;
-    fireEvent(this, "value-changed", { value: newTrigger });
+    fireEvent(this, "value-changed", { value: ev.detail.value });
   }
 
   private _computeLabelCallback = (
@@ -143,6 +90,10 @@ export class HaCalendarTrigger extends LitElement implements TriggerElement {
         return this.hass.localize(
           "ui.panel.config.automation.editor.triggers.type.calendar.event"
         );
+      case "offset":
+        return this.hass.localize(
+          "ui.panel.config.automation.editor.triggers.type.calendar.offset"
+        );
     }
     return "";
   };

src/panels/config/automation/trigger/types/ha-automation-trigger-sun.ts

@@ -39,7 +39,7 @@ export class HaSunTrigger extends LitElement implements TriggerElement {
             ],
           ],
         },
-        { name: "offset", selector: { text: {} } },
+        { name: "offset", selector: { duration: { mode: "offset" } } },
       ] as const
   );
 

src/translations/en.json

@@ -5533,9 +5533,7 @@
                   "event": "[%key:ui::panel::config::automation::editor::triggers::type::homeassistant::event%]",
                   "start": "Event start",
                   "end": "Event end",
-                  "offset": "Offset (optional)",
-                  "before": "Before",
-                  "after": "After",
+                  "offset": "Offset",
                   "description": {
                     "picker": "Triggers when a calendar event starts or ends.",
                     "full": "When{offsetChoice, select, \n before { it's {offset} before}\n after { it's {offset} after}\n other {}\n} a calendar event{eventChoice, select, \n start { starts}\n end { ends}\n other { starts or ends}\n}{hasCalendar, select, \n true { in {calendar}}\n other {}\n}"
@@ -5658,7 +5656,7 @@
                   "event": "[%key:ui::panel::config::automation::editor::triggers::type::homeassistant::event%]",
                   "sunrise": "Sunrise",
                   "sunset": "Sunset",
-                  "offset": "Offset in seconds or HH:MM:SS (optional)",
+                  "offset": "Offset",
                   "description": {
                     "picker": "Triggers when the sun sets or rises.",
                     "sets": "When the sun sets{hasDuration, select, \n true { offset by {duration}} \n other {}\n }",

test/data/automation-migration.test.ts

@@ -108,3 +108,83 @@ describe("normalizeAutomationConfig deprecated option reporting", () => {
     ).not.toThrow();
   });
 });
+
+describe("migrateAutomationConfig offset type", () => {
+  const migrateTrigger = (trigger: Record<string, unknown>) =>
+    (
+      migrateAutomationConfig({ triggers: [trigger as any] }).triggers as any
+    )[0];
+
+  it.each([
+    ["sun.sunrise", { hours: 1 }, { hours: -1 }],
+    [
+      "calendar.event_started",
+      { hours: 1, minutes: 30 },
+      { hours: -1, minutes: -30 },
+    ],
+    ["sun.sunset", { hours: -1 }, { hours: 1 }],
+    [
+      "sun.dawn",
+      "00:30:00",
+      { hours: 0, minutes: -30, seconds: 0, milliseconds: 0 },
+    ],
+  ])(
+    "folds a `before` offset type into the sign for %s",
+    (trigger, offset, expected) => {
+      expect(
+        migrateTrigger({ trigger, options: { offset, offset_type: "before" } })
+      ).toEqual({ trigger, options: { offset: expected } });
+    }
+  );
+
+  it("drops an `after` offset type and keeps the offset", () => {
+    expect(
+      migrateTrigger({
+        trigger: "calendar.event_ended",
+        options: { offset: { hours: 1 }, offset_type: "after" },
+      })
+    ).toEqual({
+      trigger: "calendar.event_ended",
+      options: { offset: { hours: 1 } },
+    });
+  });
+
+  it("drops the offset type when there is no offset", () => {
+    expect(
+      migrateTrigger({
+        trigger: "sun.sunrise",
+        options: { offset_type: "before" },
+      })
+    ).toEqual({ trigger: "sun.sunrise", options: {} });
+  });
+
+  it("leaves an offset it cannot read untouched", () => {
+    const options = { offset: "{{ offset }}", offset_type: "before" };
+    expect(
+      migrateTrigger({ trigger: "sun.sunrise", options: { ...options } })
+    ).toEqual({ trigger: "sun.sunrise", options });
+  });
+
+  it("leaves other integrations untouched", () => {
+    const options = { offset: { hours: 1 }, offset_type: "before" };
+    expect(
+      migrateTrigger({ trigger: "custom.event", options: { ...options } })
+    ).toEqual({ trigger: "custom.event", options });
+  });
+
+  it("does not flag the migration as deprecated", () => {
+    const report: AutomationMigrationReport = { deprecated: false };
+    migrateAutomationConfig(
+      {
+        triggers: [
+          {
+            trigger: "sun.sunrise",
+            options: { offset: { hours: 1 }, offset_type: "before" },
+          } as any,
+        ],
+      },
+      report
+    );
+    expect(report.deprecated).toBe(false);
+  });
+});

test/data/sun-calendar-offset-description.test.ts

@@ -0,0 +1,85 @@
+import { IntlMessageFormat } from "intl-messageformat";
+import { describe, expect, it } from "vitest";
+import type { ForDict } from "../../src/data/automation";
+import { describeTrigger } from "../../src/data/automation_i18n";
+import {
+  DateFormat,
+  FirstWeekday,
+  NumberFormat,
+  TimeFormat,
+  TimeZone,
+} from "../../src/data/translation";
+import en from "../../src/translations/en.json";
+import type { HomeAssistant } from "../../src/types";
+
+type TranslationNode = string | { [key: string]: TranslationNode };
+
+const localize = (key: string, values?: Record<string, unknown>) => {
+  const message = key
+    .split(".")
+    .reduce<TranslationNode | undefined>(
+      (translations, part) =>
+        typeof translations === "object" ? translations[part] : undefined,
+      en as TranslationNode
+    );
+  return typeof message === "string"
+    ? (new IntlMessageFormat(message, "en").format(values) as string)
+    : "";
+};
+
+const hass = {
+  localize,
+  locale: {
+    language: "en",
+    number_format: NumberFormat.language,
+    time_format: TimeFormat.twenty_four,
+    date_format: DateFormat.language,
+    first_weekday: FirstWeekday.language,
+    time_zone: TimeZone.local,
+  },
+  config: { time_zone: "Etc/UTC" },
+  states: {},
+} as unknown as HomeAssistant;
+
+describe("legacy calendar trigger offset description", () => {
+  it.each([
+    [
+      "-01:30:00",
+      "When it's 1 hour, 30 minutes before a calendar event starts",
+    ],
+    [
+      { hours: 1, minutes: -30 },
+      "When it's 30 minutes after a calendar event starts",
+    ],
+    [
+      { seconds: 1.5 },
+      "When it's 1 second, 500 milliseconds after a calendar event starts",
+    ],
+    [{ hours: 0, minutes: 0, seconds: 0 }, "When a calendar event starts"],
+  ])("describes %j", (offset, expected) => {
+    expect(
+      describeTrigger(
+        {
+          trigger: "calendar",
+          event: "start",
+          entity_id: "",
+          offset: offset as string | ForDict,
+        },
+        hass,
+        []
+      )
+    ).toBe(expected);
+  });
+});
+
+describe("legacy sun trigger offset description", () => {
+  it.each([
+    [{ minutes: -30 }, "When the sun sets offset by -30:00"],
+    [{ hours: 1, minutes: -30 }, "When the sun sets offset by +30:00"],
+    [{ hours: 0, minutes: 0, seconds: 0 }, "When the sun sets"],
+  ])("describes %j", (offset, expected) => {
+    expect(
+      describeTrigger({ trigger: "sun", event: "sunset", offset }, hass, [])
+    ).toBe(expected);
+  });
+});