Fix for 11,12,13 and add tests

web-admin/src/features/scheduled-reports/metadata/ReportMetadata.svelte

@@ -45,7 +45,9 @@
   $: reportSpec = $reportQuery.data?.resource?.report?.spec;
 
   // Get human-readable frequency
-  $: humanReadableFrequency = formatRefreshSchedule(reportSpec);
+  $: humanReadableFrequency = reportSpec?.refreshSchedule?.cron
+    ? formatRefreshSchedule(reportSpec.refreshSchedule.cron)
+    : "";
 
   $: emailNotifier = extractNotifier(reportSpec?.notifiers, "email");
   $: slackNotifier = extractNotifier(reportSpec?.notifiers, "slack");

web-admin/src/features/scheduled-reports/metadata/metadata.spec.ts

@@ -0,0 +1,60 @@
+import { formatRefreshSchedule } from "@rilldata/web-admin/features/scheduled-reports/metadata/utils";
+import {
+  convertFormValuesToCronExpression,
+  getFrequencyFromCronExpression,
+  ReportFrequency,
+} from "@rilldata/web-common/features/scheduled-reports/time-utils";
+import { describe, it, expect } from "vitest";
+
+describe("Reports metadata", () => {
+  describe("formatRefreshSchedule", () => {
+    const TestCase: [string, string][] = [
+      [
+        "* * * 21 * *",
+        "Every second, every minute, every hour, on the 21st of each month",
+      ],
+      // TODO: we need a follow up to better match what is expected.
+      //       perhaps we should replace cronstrue
+      ["0 0 * * 1", "At 12:00 AM, only on Monday"],
+    ];
+    for (const [cron, formattedSchedule] of TestCase) {
+      it(`format(${cron})=${formattedSchedule}`, () => {
+        expect(formatRefreshSchedule(cron)).toEqual(formattedSchedule);
+      });
+    }
+  });
+
+  describe("frequency extraction and conversion", () => {
+    const TestCase: [
+      frequency: ReportFrequency,
+      dayOfWeek: string,
+      timeOfDay: string,
+      dayOfMonth: number,
+      expectedCron: string,
+    ][] = [
+      [ReportFrequency.Daily, "Monday", "11:20", 1, "20 11 * * *"],
+      [ReportFrequency.Weekly, "Tuesday", "11:20", 1, "20 11 * * 2"],
+      [ReportFrequency.Weekdays, "Tuesday", "11:20", 1, "20 11 * * 1-5"],
+      [ReportFrequency.Monthly, "Tuesday", "11:20", 11, "20 11 11 * *"],
+    ];
+
+    for (const [
+      frequency,
+      dayOfWeek,
+      timeOfDay,
+      dayOfMonth,
+      expectedCron,
+    ] of TestCase) {
+      it(expectedCron, () => {
+        const actualCron = convertFormValuesToCronExpression(
+          frequency,
+          dayOfWeek,
+          timeOfDay,
+          dayOfMonth,
+        );
+        expect(actualCron).toEqual(expectedCron);
+        expect(getFrequencyFromCronExpression(actualCron)).toEqual(frequency);
+      });
+    }
+  });
+});

web-admin/src/features/scheduled-reports/metadata/utils.ts

@@ -1,3 +1,4 @@
+import { ordinal } from "@rilldata/web-common/lib/ordinal";
 import type { V1ReportSpec } from "@rilldata/web-common/runtime-client";
 import cronstrue from "cronstrue";
 import { DateTime } from "luxon";
@@ -28,25 +29,13 @@ export function formatNextRunOn(nextRunOn: string, timeZone: string): string {
     .toLocaleString(DateTime.DATETIME_FULL);
 }
 
-const suffixMap = {
-  1: "st",
-  2: "nd",
-  3: "rd",
-};
-export function formatRefreshSchedule(reportSpec: V1ReportSpec) {
-  if (!reportSpec.refreshSchedule?.cron) return "";
-  let formattedRefreshSchedule = cronstrue.toString(
-    reportSpec.refreshSchedule.cron,
-    {
-      verbose: true,
-    },
-  );
+export function formatRefreshSchedule(cron: string) {
+  let formattedRefreshSchedule = cronstrue.toString(cron, {
+    verbose: true,
+  });
   formattedRefreshSchedule = formattedRefreshSchedule.replace(
     /on day (\d*) of the month/,
-    (_, day: number) => {
-      const suffix = suffixMap[day % 10] ?? "th";
-      return `on the ${day}${suffix} of each month`;
-    },
+    (_, day: number) => `on the ${ordinal(day)} of each month`,
   );
 
   return formattedRefreshSchedule;

web-common/src/features/scheduled-reports/time-utils.ts

@@ -31,6 +31,15 @@ export function getTimeIn24FormatFromDateTime(dateTime: DateTime): string {
   return dateTime.toFormat("HH:mm");
 }
 
+const weekDayMap: Record<string, number> = {
+  Sunday: 0,
+  Monday: 1,
+  Tuesday: 2,
+  Wednesday: 3,
+  Thursday: 4,
+  Friday: 5,
+  Saturday: 6,
+};
 export function convertFormValuesToCronExpression(
   frequency: ReportFrequency,
   dayOfWeek: string,
@@ -48,15 +57,6 @@ export function convertFormValuesToCronExpression(
       cronExpr += "* * 1-5";
       break;
     case ReportFrequency.Weekly: {
-      const weekDayMap: Record<string, number> = {
-        Sunday: 0,
-        Monday: 1,
-        Tuesday: 2,
-        Wednesday: 3,
-        Thursday: 4,
-        Friday: 5,
-        Saturday: 6,
-      };
       cronExpr += `* * ${weekDayMap[dayOfWeek]}`;
       break;
     }

web-common/src/lib/ordinal.spec.ts

@@ -0,0 +1,34 @@
+import { ordinal } from "@rilldata/web-common/lib/ordinal";
+import { describe, it, expect } from "vitest";
+
+describe("ordinal", () => {
+  const TestCases: [number, string][] = [
+    [0, "0th"],
+    [1, "1st"],
+    [2, "2nd"],
+    [3, "3rd"],
+    [4, "4th"],
+    [10, "10th"],
+    [11, "11th"],
+    [12, "12th"],
+    [13, "13th"],
+    [20, "20th"],
+    [21, "21st"],
+    [22, "22nd"],
+    [23, "23rd"],
+    [110, "110th"],
+    [111, "111th"],
+    [112, "112th"],
+    [113, "113th"],
+    [120, "120th"],
+    [121, "121st"],
+    [122, "122nd"],
+    [123, "123rd"],
+  ];
+
+  for (const [num, expected] of TestCases) {
+    it(`ordinal(${num})=${expected}`, () => {
+      expect(ordinal(num)).toEqual(expected);
+    });
+  }
+});

web-common/src/lib/ordinal.ts

@@ -0,0 +1,14 @@
+function indicator(num: number): string {
+  num = Math.abs(num);
+  const cent = num % 100;
+  if (cent >= 10 && cent <= 20) return "th";
+  const dec = num % 10;
+  if (dec === 1) return "st";
+  if (dec === 2) return "nd";
+  if (dec === 3) return "rd";
+  return "th";
+}
+
+export function ordinal(num: number): string {
+  return `${num}${indicator(num)}`;
+}