From c3bfb6c251b53060211096d3c4b53a96cd635f9c Mon Sep 17 00:00:00 2001 From: Parag Jain Date: Mon, 7 Sep 2026 22:02:25 +0530 Subject: [PATCH 1/2] fix(rilltime): accept legacy DAX comparison offsets in ParseLegacy Alerts and reports created before the legacy time range refactor store comparison time ranges with an iso_offset of rill-PD/PW/PM/PQ/PY, which ParseLegacy rejected with "offset cannot have DAX notation". Map those tokens to the equivalent ISO offsets and treat rill-PP as a previous period offset, matching the pre-refactor behaviour. Co-Authored-By: Claude Fable 5.1 --- cli/cmd/sudo/project/edit_test.go | 59 +++++++++++++++++++++++++++ runtime/pkg/rilltime/rilltime.go | 24 ++++++++++- runtime/pkg/rilltime/rilltime_test.go | 13 ++++++ 3 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 cli/cmd/sudo/project/edit_test.go diff --git a/cli/cmd/sudo/project/edit_test.go b/cli/cmd/sudo/project/edit_test.go new file mode 100644 index 000000000000..0e22874e8f47 --- /dev/null +++ b/cli/cmd/sudo/project/edit_test.go @@ -0,0 +1,59 @@ +package project + +import ( + "context" + "io" + "net" + "testing" + + "github.com/rilldata/rill/cli/pkg/cmdutil" + "github.com/rilldata/rill/cli/pkg/version" + adminv1 "github.com/rilldata/rill/proto/gen/rill/admin/v1" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" +) + +func TestEditCloudEditingUsesDedicatedUpdate(t *testing.T) { + for _, disabled := range []bool{true, false} { + flag := "--cloud-editing-disabled=true" + if !disabled { + flag = "--cloud-editing-disabled=false" + } + t.Run(flag, func(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + server := grpc.NewServer() + requests := make(chan *adminv1.SudoUpdateProjectCloudEditingRequest, 1) + // Every RPC except the dedicated update is unimplemented, including GetProject + // and the full-map replacement API. + adminv1.RegisterAdminServiceServer(server, &cloudEditingServer{requests: requests}) + t.Cleanup(server.Stop) + go func() { _ = server.Serve(listener) }() + + ch, err := cmdutil.NewHelper(version.Version{}, t.TempDir()) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, ch.Close()) }) + ch.AdminURLOverride = "http://" + listener.Addr().String() + ch.Printer.OverrideHumanOutput(io.Discard) + ch.Printer.OverrideDataOutput(io.Discard) + cmd := EditCmd(ch) + cmd.SetArgs([]string{"org", "project", flag}) + require.NoError(t, cmd.ExecuteContext(t.Context())) + + req := <-requests + require.Equal(t, "org", req.Org) + require.Equal(t, "project", req.Project) + require.Equal(t, disabled, req.Disabled) + }) + } +} + +type cloudEditingServer struct { + adminv1.UnimplementedAdminServiceServer + requests chan *adminv1.SudoUpdateProjectCloudEditingRequest +} + +func (s *cloudEditingServer) SudoUpdateProjectCloudEditing(_ context.Context, req *adminv1.SudoUpdateProjectCloudEditingRequest) (*adminv1.SudoUpdateProjectCloudEditingResponse, error) { + s.requests <- req + return &adminv1.SudoUpdateProjectCloudEditingResponse{Project: &adminv1.Project{Name: req.Project, OrgName: req.Org}}, nil +} diff --git a/runtime/pkg/rilltime/rilltime.go b/runtime/pkg/rilltime/rilltime.go index 37999eb8a36e..8ce5cbb06601 100644 --- a/runtime/pkg/rilltime/rilltime.go +++ b/runtime/pkg/rilltime/rilltime.go @@ -76,6 +76,16 @@ var ( "PQ": "-1Q/Q to ref/Q", "PY": "-1Y/Y to ref/Y", } + // Mapping for our old rill- comparison offsets to ISO durations. + // Older reports/alerts may send these as the offset of a legacy comparison time range. + // "PP" (previous period) is handled separately since it depends on the main interval. + daxOffsetNotations = map[string]string{ + "PD": "P1D", + "PW": "P1W", + "PM": "P1M", + "PQ": "P3M", + "PY": "P1Y", + } grainMap = map[string]timeutil.TimeGrain{ "s": timeutil.TimeGrainSecond, "S": timeutil.TimeGrainSecond, @@ -352,7 +362,19 @@ func ParseLegacy(duration, offset string, roundToGrain timeutil.TimeGrain, parse if offset != "" { if strings.HasPrefix(offset, "rill-") { - return nil, fmt.Errorf("offset cannot have DAX notation") + dax := strings.TrimPrefix(offset, "rill-") + if dax == "PP" { + rt.Offset = &Offset{ + PreviousPeriod: &PreviousPeriod{Prefix: "-", Num: 1}, + } + return rt, nil + } + + iso, ok := daxOffsetNotations[dax] + if !ok { + return nil, fmt.Errorf("invalid DAX offset %q", offset) + } + offset = iso } offsetGrainPart, err := parseISODuration(offset) diff --git a/runtime/pkg/rilltime/rilltime_test.go b/runtime/pkg/rilltime/rilltime_test.go index bae3ff6a59e0..5860cc8fbe1d 100644 --- a/runtime/pkg/rilltime/rilltime_test.go +++ b/runtime/pkg/rilltime/rilltime_test.go @@ -495,6 +495,14 @@ func TestParseISO(t *testing.T) { {"With duration and offset no round to grain", "P7D", "P2D", timeutil.TimeGrainUnspecified, "2025-05-04T06:32:36Z", "2025-05-11T06:32:36Z", timeutil.TimeGrainUnspecified}, {"With duration, offset and round to grain", "P7D", "P2D", timeutil.TimeGrainDay, "2025-05-04T00:00:00Z", "2025-05-11T00:00:00Z", timeutil.TimeGrainUnspecified}, {"With DAX duration, offset and round to grain", "rill-PW", "P2D", timeutil.TimeGrainDay, "2025-05-03T00:00:00Z", "2025-05-10T00:00:00Z", timeutil.TimeGrainUnspecified}, + // Legacy DAX comparison offsets sent by older clients + {"With duration and DAX previous period offset", "P7D", "rill-PP", timeutil.TimeGrainUnspecified, "2025-04-29T06:32:36Z", "2025-05-06T06:32:36Z", timeutil.TimeGrainUnspecified}, + {"With duration and DAX previous day offset", "P7D", "rill-PD", timeutil.TimeGrainUnspecified, "2025-05-05T06:32:36Z", "2025-05-12T06:32:36Z", timeutil.TimeGrainUnspecified}, + {"With duration and DAX previous week offset", "P7D", "rill-PW", timeutil.TimeGrainUnspecified, "2025-04-29T06:32:36Z", "2025-05-06T06:32:36Z", timeutil.TimeGrainUnspecified}, + {"With duration and DAX previous month offset", "P7D", "rill-PM", timeutil.TimeGrainUnspecified, "2025-04-06T06:32:36Z", "2025-04-13T06:32:36Z", timeutil.TimeGrainUnspecified}, + {"With duration and DAX previous quarter offset", "P7D", "rill-PQ", timeutil.TimeGrainUnspecified, "2025-02-06T06:32:36Z", "2025-02-13T06:32:36Z", timeutil.TimeGrainUnspecified}, + {"With duration and DAX previous year offset", "P7D", "rill-PY", timeutil.TimeGrainUnspecified, "2024-05-06T06:32:36Z", "2024-05-13T06:32:36Z", timeutil.TimeGrainUnspecified}, + {"With duration, DAX previous week offset and round to grain", "P7D", "rill-PW", timeutil.TimeGrainDay, "2025-04-29T00:00:00Z", "2025-05-06T00:00:00Z", timeutil.TimeGrainUnspecified}, } nowTm := parseTestTime(t, now) @@ -522,6 +530,11 @@ func TestParseISO(t *testing.T) { } } +func TestParseISO_InvalidOffset(t *testing.T) { + _, err := ParseLegacy("P7D", "rill-PX", timeutil.TimeGrainUnspecified, ParseOptions{}) + require.ErrorContains(t, err, `invalid DAX offset "rill-PX"`) +} + func TestEval_SyntaxErrors(t *testing.T) { testCases := []struct { timeRange string From a0b6db7d7f89a701273a7f1f472c40ea55113712 Mon Sep 17 00:00:00 2001 From: Parag Jain Date: Mon, 7 Sep 2026 22:23:21 +0530 Subject: [PATCH 2/2] remove wrong test --- cli/cmd/sudo/project/edit_test.go | 59 ------------------------------- 1 file changed, 59 deletions(-) delete mode 100644 cli/cmd/sudo/project/edit_test.go diff --git a/cli/cmd/sudo/project/edit_test.go b/cli/cmd/sudo/project/edit_test.go deleted file mode 100644 index 0e22874e8f47..000000000000 --- a/cli/cmd/sudo/project/edit_test.go +++ /dev/null @@ -1,59 +0,0 @@ -package project - -import ( - "context" - "io" - "net" - "testing" - - "github.com/rilldata/rill/cli/pkg/cmdutil" - "github.com/rilldata/rill/cli/pkg/version" - adminv1 "github.com/rilldata/rill/proto/gen/rill/admin/v1" - "github.com/stretchr/testify/require" - "google.golang.org/grpc" -) - -func TestEditCloudEditingUsesDedicatedUpdate(t *testing.T) { - for _, disabled := range []bool{true, false} { - flag := "--cloud-editing-disabled=true" - if !disabled { - flag = "--cloud-editing-disabled=false" - } - t.Run(flag, func(t *testing.T) { - listener, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - server := grpc.NewServer() - requests := make(chan *adminv1.SudoUpdateProjectCloudEditingRequest, 1) - // Every RPC except the dedicated update is unimplemented, including GetProject - // and the full-map replacement API. - adminv1.RegisterAdminServiceServer(server, &cloudEditingServer{requests: requests}) - t.Cleanup(server.Stop) - go func() { _ = server.Serve(listener) }() - - ch, err := cmdutil.NewHelper(version.Version{}, t.TempDir()) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, ch.Close()) }) - ch.AdminURLOverride = "http://" + listener.Addr().String() - ch.Printer.OverrideHumanOutput(io.Discard) - ch.Printer.OverrideDataOutput(io.Discard) - cmd := EditCmd(ch) - cmd.SetArgs([]string{"org", "project", flag}) - require.NoError(t, cmd.ExecuteContext(t.Context())) - - req := <-requests - require.Equal(t, "org", req.Org) - require.Equal(t, "project", req.Project) - require.Equal(t, disabled, req.Disabled) - }) - } -} - -type cloudEditingServer struct { - adminv1.UnimplementedAdminServiceServer - requests chan *adminv1.SudoUpdateProjectCloudEditingRequest -} - -func (s *cloudEditingServer) SudoUpdateProjectCloudEditing(_ context.Context, req *adminv1.SudoUpdateProjectCloudEditingRequest) (*adminv1.SudoUpdateProjectCloudEditingResponse, error) { - s.requests <- req - return &adminv1.SudoUpdateProjectCloudEditingResponse{Project: &adminv1.Project{Name: req.Project, OrgName: req.Org}}, nil -}