fix(api): keep an equivalent ramp when editing a non-preset weekly/monthly plan

internal/api/handler_plans.go

@@ -395,9 +395,11 @@ func mergeUpdatedPlan(existingPlan *config.PurchasePlan, req *PlanRequest, planI
 // carried over; a rebuild has no way to know either and would otherwise
 // wipe them. CurrentStep, StartDate, and NextExecutionDate are carried
 // over when the requested ramp schedule has the same shape as the existing
-// one (same type/percent/interval/step-count): an unrelated field edit
-// (rename, target coverage, ...) resubmits the same schedule and must not
-// restart it. A different shape is refused once a step has completed,
+// one (see rampScheduleShapeUnchanged): an unrelated field edit (rename,
+// target coverage, ...) resubmits the same schedule and must not restart it.
+// The stored type label is kept too, so resubmitting a non-preset weekly or
+// monthly ramp as the equal custom one leaves the plan as it was (issue
+// #1184). A different shape is refused once a step has completed,
 // because the rebuilt schedule would restart at step zero and forget the
 // percentage already bought (issue #386); before the first step it is a
 // plain schedule change.
@@ -406,6 +408,7 @@ func preserveRampProgress(existingPlan, plan *config.PurchasePlan) error {
 	plan.LastNotificationSent = existingPlan.LastNotificationSent
 
 	if rampScheduleShapeUnchanged(existingPlan.RampSchedule, plan.RampSchedule) {
+		plan.RampSchedule.Type = existingPlan.RampSchedule.Type
 		plan.RampSchedule.CurrentStep = existingPlan.RampSchedule.CurrentStep
 		plan.RampSchedule.StartDate = existingPlan.RampSchedule.StartDate
 		// Editor-owned markers the request cannot express; dropping them hands
@@ -428,13 +431,33 @@ func preserveRampProgress(existingPlan, plan *config.PurchasePlan) error {
 // excluding the progress fields CurrentStep and StartDate, to tell apart
 // "the update resubmitted the same schedule" from "the update picked a
 // genuinely different one".
+//
+// The type must match, except that weekly, monthly and custom are only labels
+// for the same step-based behavior (nothing downstream branches on them; only
+// "immediate" is special). The UI can send just the presets or "custom", so a
+// stored weekly 20%/7d ramp comes back as custom 20%/7d, and treating that as
+// a change refused every edit of such a plan once a step had run (issue #1184).
 func rampScheduleShapeUnchanged(a, b config.RampSchedule) bool {
-	return a.Type == b.Type &&
+	return rampTypesEquivalent(a.Type, b.Type) &&
 		a.PercentPerStep == b.PercentPerStep &&
 		a.StepIntervalDays == b.StepIntervalDays &&
 		a.TotalSteps == b.TotalSteps
 }
 
+// rampTypesEquivalent reports whether two ramp type labels describe the same
+// behavior: identical labels, or two of the step-based weekly/monthly/custom.
+// "immediate" and any unrecognized label must match exactly.
+func rampTypesEquivalent(a, b string) bool {
+	if a == b {
+		return true
+	}
+	return isStepBasedRampType(a) && isStepBasedRampType(b)
+}
+
+func isStepBasedRampType(t string) bool {
+	return t == "weekly" || t == "monthly" || t == "custom"
+}
+
 func (h *Handler) deletePlan(ctx context.Context, req *events.LambdaFunctionURLRequest, planID string) (any, error) {
 	// Validate UUID format to prevent injection attacks
 	if err := validateUUID(planID); err != nil {

internal/api/handler_plans_ramp_shape_test.go

@@ -3,6 +3,7 @@ package api
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
 	"github.com/aws/aws-lambda-go/events"
@@ -85,3 +86,98 @@ func TestUpdatePlan_RampShapeChange(t *testing.T) {
 		})
 	}
 }
+
+// TestUpdatePlan_EquivalentCustomRampKeepsStoredSchedule pins issue #1184.
+// The UI can send only the presets or "custom", so Edit resubmits a stored
+// non-preset weekly/monthly ramp as an equal custom one. The type label is
+// cosmetic for step-based ramps (only "immediate" changes behavior), so an
+// equal percent, interval and step count is the same schedule: an in-progress
+// plan keeps its progress, and the stored type, start date and next execution
+// date are kept rather than rebuilt, whether or not a step has completed.
+func TestUpdatePlan_EquivalentCustomRampKeepsStoredSchedule(t *testing.T) {
+	const planID = "12345678-1234-1234-1234-123456789abc"
+	start := time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC)
+	next := time.Date(2026, 9, 22, 0, 0, 0, 0, time.UTC)
+	weekly20 := config.RampSchedule{Type: "weekly", PercentPerStep: 20, StepIntervalDays: 7, TotalSteps: 5}
+	monthly5 := config.RampSchedule{Type: "monthly", PercentPerStep: 5, StepIntervalDays: 30, TotalSteps: 20}
+	custom25 := config.RampSchedule{Type: "custom", PercentPerStep: 25, StepIntervalDays: 7, TotalSteps: 4}
+
+	tests := []struct {
+		name   string
+		stored config.RampSchedule
+		step   int
+		body   string
+	}{
+		{"in-progress weekly 20%, renamed via custom", weekly20, 3,
+			`{"name":"renamed","ramp_schedule":"custom","custom_step_percent":20,"custom_interval_days":7}`},
+		{"in-progress monthly 5%, renamed via custom", monthly5, 1,
+			`{"name":"renamed","ramp_schedule":"custom","custom_step_percent":5,"custom_interval_days":30}`},
+		{"in-progress custom 25%/7d, renamed via the equal weekly preset", custom25, 2,
+			`{"name":"renamed","ramp_schedule":"weekly-25pct"}`},
+		{"not started weekly 20%, renamed via custom", weekly20, 0,
+			`{"name":"renamed","ramp_schedule":"custom","custom_step_percent":20,"custom_interval_days":7}`},
+		{"not started monthly 5%, renamed via custom", monthly5, 0,
+			`{"name":"renamed","ramp_schedule":"custom","custom_step_percent":5,"custom_interval_days":30}`},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			rs := tc.stored
+			rs.CurrentStep = tc.step
+			rs.StartDate = start
+			nextExec := next
+			existing := &config.PurchasePlan{ID: planID, Name: "p", Enabled: true, RampSchedule: rs, NextExecutionDate: &nextExec}
+
+			resp, saved := putPlan(t, existing, tc.body)
+
+			require.Equal(t, 200, resp.StatusCode, resp.Body)
+			require.NotNil(t, saved)
+			assert.Equal(t, "renamed", saved.Name)
+			assert.Equal(t, rs, saved.RampSchedule, "the stored schedule (type, progress, start date) must be kept")
+			require.NotNil(t, saved.NextExecutionDate)
+			assert.True(t, next.Equal(*saved.NextExecutionDate), "next execution date must be kept, got %v", saved.NextExecutionDate)
+		})
+	}
+}
+
+// TestUpdatePlan_RealRampChangeOnNonPresetStillRefused is the other half of
+// #1184: label-agnostic matching must not let a real change through on an
+// in-progress plan.
+func TestUpdatePlan_RealRampChangeOnNonPresetStillRefused(t *testing.T) {
+	const planID = "12345678-1234-1234-1234-123456789abc"
+	weekly20 := config.RampSchedule{Type: "weekly", PercentPerStep: 20, StepIntervalDays: 7, TotalSteps: 5, CurrentStep: 2}
+	// 30% and 33% both give 3 steps: only the percent differs.
+	weekly30 := config.RampSchedule{Type: "weekly", PercentPerStep: 30, StepIntervalDays: 7, TotalSteps: 3, CurrentStep: 1}
+	immediate := config.PresetRampSchedules[config.RampImmediate]
+	immediate.CurrentStep = 1
+	// Same percent, interval and step count as immediate: only the type
+	// differs, and "immediate" is never interchangeable with a step-based type.
+	custom100 := config.RampSchedule{Type: "custom", PercentPerStep: 100, StepIntervalDays: 0, TotalSteps: 1, CurrentStep: 1}
+
+	tests := []struct {
+		name   string
+		stored config.RampSchedule
+		body   string
+	}{
+		{"different percent and step count", weekly20,
+			`{"name":"p","ramp_schedule":"custom","custom_step_percent":25,"custom_interval_days":7}`},
+		{"different interval", weekly20,
+			`{"name":"p","ramp_schedule":"custom","custom_step_percent":20,"custom_interval_days":14}`},
+		{"different percent, same step count", weekly30,
+			`{"name":"p","ramp_schedule":"custom","custom_step_percent":33,"custom_interval_days":7}`},
+		{"weekly to immediate", weekly20, `{"name":"p","ramp_schedule":"immediate"}`},
+		{"immediate to custom 100%", immediate,
+			`{"name":"p","ramp_schedule":"custom","custom_step_percent":100,"custom_interval_days":1}`},
+		{"custom 100% to immediate", custom100, `{"name":"p","ramp_schedule":"immediate"}`},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			existing := &config.PurchasePlan{ID: planID, Name: "p", Enabled: true, RampSchedule: tc.stored}
+
+			resp, saved := putPlan(t, existing, tc.body)
+
+			assert.Equal(t, 409, resp.StatusCode, resp.Body)
+			assert.Nil(t, saved, "a refused shape change must not reach the store")
+			assert.Contains(t, resp.Body, "create a new plan")
+		})
+	}
+}