From 9e298b9a1bb18c3c952bc25fe41d7e2538c6afae Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Fri, 25 Sep 2026 09:25:46 -0500 Subject: [PATCH 01/16] feat(targeting): manage rules and guard activation --- docs/previews/targeting-list.svg | 18 + docs/specs/cli-coverage.yaml | 12 + internal/api/client.go | 2 + internal/api/paths_gen.go | 8 + internal/api/targeting_rules.go | 91 +++++ internal/api/targeting_rules_test.go | 67 ++++ internal/cli/root.go | 1 + internal/cli/snapshot_test.go | 3 + internal/cli/targeting.go | 340 ++++++++++++++++++ internal/cli/targeting_test.go | 59 +++ .../testdata/snapshots/targeting-list.golden | 3 + 11 files changed, 604 insertions(+) create mode 100644 docs/previews/targeting-list.svg create mode 100644 internal/api/targeting_rules.go create mode 100644 internal/api/targeting_rules_test.go create mode 100644 internal/cli/targeting.go create mode 100644 internal/cli/targeting_test.go create mode 100644 internal/cli/testdata/snapshots/targeting-list.golden diff --git a/docs/previews/targeting-list.svg b/docs/previews/targeting-list.svg new file mode 100644 index 00000000..4deb9abf --- /dev/null +++ b/docs/previews/targeting-list.svg @@ -0,0 +1,18 @@ + + + + + +$ rc targeting list --no-input --project-id proj_snap --api-key sk_snap +ID         NAME               TYPE    STATE   SERVES   +trle_snap  US annual paywall  legacy  active  ofrng_us + + + diff --git a/docs/specs/cli-coverage.yaml b/docs/specs/cli-coverage.yaml index 9b2a28d8..be9df0c4 100644 --- a/docs/specs/cli-coverage.yaml +++ b/docs/specs/cli-coverage.yaml @@ -244,6 +244,18 @@ endpoints: - method: GET path: /projects/{project_id}/experiments/{experiment_id}/results + # Targeting rules (development-status; provided by the beta overlay) + - method: GET + path: /projects/{project_id}/targeting_rules + - method: POST + path: /projects/{project_id}/targeting_rules + - method: GET + path: /projects/{project_id}/targeting_rules/{targeting_rule_id} + - method: POST + path: /projects/{project_id}/targeting_rules/{targeting_rule_id} + - method: DELETE + path: /projects/{project_id}/targeting_rules/{targeting_rule_id} + # Audit - method: GET path: /projects/{project_id}/audit_logs diff --git a/internal/api/client.go b/internal/api/client.go index ce0cb70d..c3708ddd 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -55,6 +55,7 @@ type Client struct { Entitlements *EntitlementsService Offerings *OfferingsService Experiments *ExperimentsService + TargetingRules *TargetingRulesService Packages *PackagesService Products *ProductsService Subscriptions *SubscriptionsService @@ -99,6 +100,7 @@ func NewClient(opts Options) *Client { c.Entitlements = &EntitlementsService{c: c} c.Offerings = &OfferingsService{c: c} c.Experiments = &ExperimentsService{c: c} + c.TargetingRules = &TargetingRulesService{c: c} c.Packages = &PackagesService{c: c} c.Products = &ProductsService{c: c} c.Subscriptions = &SubscriptionsService{c: c} diff --git a/internal/api/paths_gen.go b/internal/api/paths_gen.go index 4519d14b..b6fc94cf 100644 --- a/internal/api/paths_gen.go +++ b/internal/api/paths_gen.go @@ -313,3 +313,11 @@ func pathSubscriptionEntitlements(projectID string, subscriptionID string) strin func pathSubscriptionTransactions(projectID string, subscriptionID string) string { return encodePath("projects", projectID, "subscriptions", subscriptionID, "transactions") } + +func pathTargetingRule(projectID string, targetingRuleID string) string { + return encodePath("projects", projectID, "targeting_rules", targetingRuleID) +} + +func pathTargetingRules(projectID string) string { + return encodePath("projects", projectID, "targeting_rules") +} diff --git a/internal/api/targeting_rules.go b/internal/api/targeting_rules.go new file mode 100644 index 00000000..e2bb4c3e --- /dev/null +++ b/internal/api/targeting_rules.go @@ -0,0 +1,91 @@ +package api + +import ( + "context" + "encoding/json" + "net/http" + "net/url" + "strconv" +) + +type TargetingRulesService struct{ c *Client } + +type TargetingRule struct { + Object string `json:"object"` + ID string `json:"id"` + RuleType string `json:"rule_type"` + State string `json:"state"` + DisplayName string `json:"display_name"` + OfferingID string `json:"offering_id,omitempty"` + AudienceID *string `json:"audience_id,omitempty"` + Conditions []any `json:"conditions,omitempty"` + Schedule any `json:"schedule,omitempty"` + Placements any `json:"placements,omitempty"` + FlowID string `json:"flow_id,omitempty"` + Checkpoints []any `json:"checkpoints,omitempty"` +} + +type ListTargetingRulesOptions struct { + State string + Limit int + StartingAfter string +} + +func (s *TargetingRulesService) List(ctx context.Context, projectID string, opts ListTargetingRulesOptions) (*Page[TargetingRule], error) { + q := url.Values{} + if opts.State != "" { + q.Set("state", opts.State) + } + if opts.Limit > 0 { + q.Set("limit", strconv.Itoa(opts.Limit)) + } + if opts.StartingAfter != "" { + q.Set("starting_after", opts.StartingAfter) + } + path := pathTargetingRules(projectID) + if len(q) > 0 { + path += "?" + q.Encode() + } + var out Page[TargetingRule] + err := s.c.do(ctx, http.MethodGet, path, nil, &out) + return &out, err +} + +func (s *TargetingRulesService) Get(ctx context.Context, projectID, id string) (*TargetingRule, error) { + var out TargetingRule + err := s.c.do(ctx, http.MethodGet, pathTargetingRule(projectID, id), nil, &out) + return &out, err +} + +type TargetingRuleCreate struct { + RuleType string `json:"rule_type,omitempty"` + Position *int `json:"position,omitempty"` + ID string `json:"id,omitempty"` + State string `json:"state,omitempty"` + DisplayName string `json:"display_name"` + OfferingID string `json:"offering_id,omitempty"` + AudienceID string `json:"audience_id,omitempty"` + Conditions json.RawMessage `json:"conditions,omitempty"` + Schedule json.RawMessage `json:"schedule,omitempty"` + Placements json.RawMessage `json:"placements,omitempty"` + FlowID string `json:"flow_id,omitempty"` + Checkpoints json.RawMessage `json:"checkpoints,omitempty"` +} + +type TargetingRuleUpdate map[string]json.RawMessage + +func (s *TargetingRulesService) Create(ctx context.Context, projectID string, body TargetingRuleCreate) (*TargetingRule, error) { + var out TargetingRule + err := s.c.do(ctx, http.MethodPost, pathTargetingRules(projectID), body, &out) + return &out, err +} + +func (s *TargetingRulesService) Update(ctx context.Context, projectID, id string, body TargetingRuleUpdate) (*TargetingRule, error) { + var out TargetingRule + err := s.c.do(ctx, http.MethodPost, pathTargetingRule(projectID, id), body, &out) + return &out, err +} + +func (s *TargetingRulesService) Delete(ctx context.Context, projectID, id string) error { + return s.c.do(ctx, http.MethodDelete, pathTargetingRule(projectID, id), nil, nil) +} diff --git a/internal/api/targeting_rules_test.go b/internal/api/targeting_rules_test.go new file mode 100644 index 00000000..acd3692a --- /dev/null +++ b/internal/api/targeting_rules_test.go @@ -0,0 +1,67 @@ +package api_test + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/revenuecat/cli/internal/api" +) + +func TestTargetingRuleRoutes(t *testing.T) { + requests := []string{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests = append(requests, r.Method+" "+r.URL.RequestURI()) + w.Header().Set("Content-Type", "application/json") + if r.Method == http.MethodPost && r.URL.Path == "/projects/proj/targeting_rules" { + var body api.TargetingRuleCreate + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatal(err) + } + if body.DisplayName != "US paywall" || body.OfferingID != "ofrng_us" || body.State != "inactive" { + t.Fatalf("unexpected create body: %+v", body) + } + } + if r.Method == http.MethodDelete { + fmt.Fprint(w, `{"object":"deleted_object","id":"trle1"}`) + return + } + if r.URL.Path == "/projects/proj/targeting_rules" && r.Method == http.MethodGet { + fmt.Fprint(w, `{"object":"list","items":[{"object":"targeting_rule","id":"trle1","rule_type":"legacy","state":"inactive","display_name":"US paywall","offering_id":"ofrng_us"}],"next_page":null,"url":"/projects/proj/targeting_rules"}`) + return + } + fmt.Fprint(w, `{"object":"targeting_rule","id":"trle1","rule_type":"legacy","state":"inactive","display_name":"US paywall","offering_id":"ofrng_us"}`) + })) + t.Cleanup(srv.Close) + client := api.NewClient(api.Options{APIKey: "sk_test", BaseURL: srv.URL}) + ctx := context.Background() + page, err := client.TargetingRules.List(ctx, "proj", api.ListTargetingRulesOptions{State: "inactive", Limit: 5, StartingAfter: "trle0"}) + if err != nil || len(page.Items) != 1 || page.Items[0].OfferingID != "ofrng_us" { + t.Fatalf("page=%+v err=%v", page, err) + } + if _, err := client.TargetingRules.Get(ctx, "proj", "trle1"); err != nil { + t.Fatal(err) + } + if _, err := client.TargetingRules.Create(ctx, "proj", api.TargetingRuleCreate{DisplayName: "US paywall", OfferingID: "ofrng_us", State: "inactive"}); err != nil { + t.Fatal(err) + } + if _, err := client.TargetingRules.Update(ctx, "proj", "trle1", api.TargetingRuleUpdate{"state": json.RawMessage(`"active"`)}); err != nil { + t.Fatal(err) + } + if err := client.TargetingRules.Delete(ctx, "proj", "trle1"); err != nil { + t.Fatal(err) + } + want := []string{ + "GET /projects/proj/targeting_rules?limit=5&starting_after=trle0&state=inactive", + "GET /projects/proj/targeting_rules/trle1", + "POST /projects/proj/targeting_rules", + "POST /projects/proj/targeting_rules/trle1", + "DELETE /projects/proj/targeting_rules/trle1", + } + if fmt.Sprint(requests) != fmt.Sprint(want) { + t.Fatalf("requests = %v, want %v", requests, want) + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 096fa9ec..9e3119ab 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -155,6 +155,7 @@ Agent-friendly entrypoints: {newEntitlementsCmd(), "catalog"}, {newOfferingsCmd(), "catalog"}, {newExperimentsCmd(), "revenue"}, + {newTargetingCmd(), "catalog"}, {newProductsCmd(), "catalog"}, {newSubscriptionsCmd(), "revenue"}, {newPurchasesCmd(), "revenue"}, diff --git a/internal/cli/snapshot_test.go b/internal/cli/snapshot_test.go index cac7104e..3c670cf5 100644 --- a/internal/cli/snapshot_test.go +++ b/internal/cli/snapshot_test.go @@ -28,6 +28,8 @@ func snapshotServer(t *testing.T) *httptest.Server { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") switch { + case strings.HasSuffix(r.URL.Path, "/targeting_rules"): + io.WriteString(w, `{"object":"list","items":[{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"US annual paywall","offering_id":"ofrng_us"}],"next_page":null,"url":"/projects/proj_snap/targeting_rules"}`) case strings.HasSuffix(r.URL.Path, "/experiments/exp_snap/actions/start"): io.WriteString(w, `{"object":"experiment","id":"exp_snap","display_name":"New paywall","status":"running","created_at":1784297950368,"updated_at":1784297950368}`) case strings.HasSuffix(r.URL.Path, "/experiments/exp_snap"): @@ -67,6 +69,7 @@ func TestOutputSnapshots(t *testing.T) { {"experiments-show", []string{"experiments", "show", "exp_snap", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"experiments-results", []string{"experiments", "results", "exp_snap", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"experiments-start", []string{"experiments", "start", "exp_snap", "--yes", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, + {"targeting-list", []string{"targeting", "list", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"apps-list", []string{"apps", "list", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"apps-list-all-projects", []string{"apps", "list", "--all-projects", "--bundle-id", "com.example.moodly", "--no-input", "--api-key", "sk_snap"}}, {"error-not-found", []string{"offerings", "show", "ofrng_missing", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go new file mode 100644 index 00000000..3ab2d710 --- /dev/null +++ b/internal/cli/targeting.go @@ -0,0 +1,340 @@ +package cli + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/charmbracelet/huh" + "github.com/spf13/cobra" + + "github.com/revenuecat/cli/internal/api" + "github.com/revenuecat/cli/internal/output" + "github.com/revenuecat/cli/internal/tui" +) + +func newTargetingCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "targeting", + Short: "Manage targeting rules for Offerings and Flows", + Long: "List, inspect, and manage targeting rules. Active rules are evaluated in priority order; the first match wins.", + } + cmd.AddCommand(newTargetingListCmd(), newTargetingShowCmd(), newTargetingCreateCmd(), newTargetingUpdateCmd(), newTargetingDeleteCmd()) + return cmd +} + +func newTargetingListCmd() *cobra.Command { + var state, startingAfter string + var limit int + cmd := &cobra.Command{ + Use: "list", + Short: "List targeting rules", + Example: " rc targeting list --state active\n rc targeting list --json", + RunE: func(cmd *cobra.Command, _ []string) error { + rt := RuntimeFrom(cmd.Context()) + projectID, err := requireProject(rt) + if err != nil { + return err + } + client, err := rt.API() + if err != nil { + return err + } + page, err := client.TargetingRules.List(cmd.Context(), projectID, api.ListTargetingRulesOptions{State: state, Limit: limit, StartingAfter: startingAfter}) + if err != nil { + return err + } + rows := make([][]string, 0, len(page.Items)) + for _, rule := range page.Items { + serves := rule.OfferingID + if rule.RuleType == "checkpoint" { + serves = rule.FlowID + } + rows = append(rows, []string{rule.ID, rule.DisplayName, rule.RuleType, rule.State, serves}) + } + return rt.Out.RenderTable(output.Table{Columns: []string{"ID", "NAME", "TYPE", "STATE", "SERVES"}, Rows: rows, Raw: page}) + }, + } + cmd.Flags().StringVar(&state, "state", "", "filter by active, scheduled, or inactive") + cmd.Flags().IntVar(&limit, "limit", 20, "maximum rules to return (1–100)") + cmd.Flags().StringVar(&startingAfter, "starting-after", "", "pagination cursor from the previous page") + return cmd +} + +func targetingPickerItems(cmd *cobra.Command, client *api.Client, projectID string) ([]PickerItem, error) { + page, err := client.TargetingRules.List(cmd.Context(), projectID, api.ListTargetingRulesOptions{}) + if err != nil { + return nil, err + } + items := make([]PickerItem, len(page.Items)) + for i, rule := range page.Items { + items[i] = PickerItem{ID: rule.ID, Label: fmt.Sprintf("%s (%s)", rule.DisplayName, rule.State)} + } + return items, nil +} + +func newTargetingShowCmd() *cobra.Command { + return &cobra.Command{ + Use: "show [id]", + Short: "Show a targeting rule", + Example: " rc targeting show trle123 --json", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + rt := RuntimeFrom(cmd.Context()) + projectID, err := requireProject(rt) + if err != nil { + return err + } + client, err := rt.API() + if err != nil { + return err + } + id, err := requireID(rt, argAt(args, 0), "targeting rule", func() ([]PickerItem, error) { + return targetingPickerItems(cmd, client, projectID) + }) + if err != nil { + return err + } + rule, err := client.TargetingRules.Get(cmd.Context(), projectID, id) + if err != nil { + return err + } + return rt.Out.Render(rule) + }, + } +} + +func newTargetingCreateCmd() *cobra.Command { + var name, offering, state, config string + cmd := &cobra.Command{ + Use: "create", + Short: "Create a targeting rule", + Long: "Creates an inactive Offering rule by default. Use --config for audiences, conditions, schedules, placements, or a checkpoint rule. Activating a rule requires confirmation.", + Example: ` rc targeting create --name "US paywall" --offering ofrng_us + rc targeting create --config rule.json --yes --json --no-input`, + RunE: func(cmd *cobra.Command, _ []string) error { + rt := RuntimeFrom(cmd.Context()) + projectID, err := requireProject(rt) + if err != nil { + return err + } + body := api.TargetingRuleCreate{} + if config != "" { + if err := readJSONConfig(config, &body); err != nil { + return err + } + } + if cmd.Flags().Changed("name") { + body.DisplayName = name + } + if cmd.Flags().Changed("offering") { + body.OfferingID = offering + } + if cmd.Flags().Changed("state") { + body.State = state + } + if body.RuleType == "" { + body.RuleType = "legacy" + } + if body.State == "" { + body.State = "inactive" + } + if err := gatherTargetingCreateInput(rt, &body); err != nil { + return err + } + if err := validateTargetingCreate(body); err != nil { + return err + } + if body.State != "inactive" { + rt.Out.Notice("This rule will affect which Offering or Flow matching customers receive.") + if err := confirmOrAbort(rt, "Activate targeting rule now?"); err != nil { + return err + } + } + client, err := rt.API() + if err != nil { + return err + } + rule, err := client.TargetingRules.Create(cmd.Context(), projectID, body) + if err != nil { + return err + } + rt.Out.Success("Created targeting rule " + rule.ID) + return rt.Out.Render(rule) + }, + } + cmd.Flags().StringVar(&name, "name", "", "rule display name") + cmd.Flags().StringVar(&offering, "offering", "", "Offering ID served by a legacy rule") + cmd.Flags().StringVar(&state, "state", "", "inactive (default) or active") + cmd.Flags().StringVar(&config, "config", "", "JSON config file; use - for stdin") + return cmd +} + +func gatherTargetingCreateInput(rt *Runtime, body *api.TargetingRuleCreate) error { + if body.RuleType == "checkpoint" { + var missing []string + if body.DisplayName == "" { + missing = append(missing, "display_name") + } + if body.AudienceID == "" { + missing = append(missing, "audience_id") + } + if body.FlowID == "" { + missing = append(missing, "flow_id") + } + if len(body.Checkpoints) == 0 { + missing = append(missing, "checkpoints") + } + if len(missing) > 0 { + return fmt.Errorf("checkpoint rule requires %s in --config", strings.Join(missing, ", ")) + } + return nil + } + if !rt.CanPrompt() { + var missing []string + if body.DisplayName == "" { + missing = append(missing, "--name") + } + if body.OfferingID == "" { + missing = append(missing, "--offering") + } + if len(missing) > 0 { + return fmt.Errorf("targeting input required: pass %s or --config ", strings.Join(missing, ", ")) + } + return nil + } + form := tui.Form(false) + if body.DisplayName == "" { + form.Field(huh.NewInput().Title("Rule name").Value(&body.DisplayName).Validate(tui.Required("rule name"))) + } + if body.OfferingID == "" { + form.Field(huh.NewInput().Title("Offering ID").Value(&body.OfferingID).Validate(tui.Required("offering ID"))) + } + return form.Run() +} + +func validateTargetingCreate(body api.TargetingRuleCreate) error { + if body.RuleType != "legacy" && body.RuleType != "checkpoint" { + return fmt.Errorf("rule_type must be legacy or checkpoint") + } + if body.State != "inactive" && body.State != "active" && !(body.RuleType == "checkpoint" && body.State == "scheduled") { + return fmt.Errorf("state must be inactive or active (checkpoint rules also support scheduled)") + } + if body.RuleType == "legacy" && body.AudienceID != "" && len(body.Conditions) > 0 && string(body.Conditions) != "[]" { + return fmt.Errorf("audience_id and conditions cannot both be set") + } + return nil +} + +func newTargetingUpdateCmd() *cobra.Command { + var config string + cmd := &cobra.Command{ + Use: "update [id]", + Short: "Update an Offering targeting rule", + Long: "Partially updates a legacy targeting rule from a JSON object. An active rule or an activation requires confirmation. Checkpoint rule updates are not exposed by this endpoint.", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if config == "" { + return fmt.Errorf("pass --config or --config - for stdin") + } + body := api.TargetingRuleUpdate{} + if err := readJSONConfig(config, &body); err != nil { + return err + } + if err := validTargetingUpdate(body); err != nil { + return err + } + rt := RuntimeFrom(cmd.Context()) + projectID, err := requireProject(rt) + if err != nil { + return err + } + client, err := rt.API() + if err != nil { + return err + } + id, err := requireID(rt, argAt(args, 0), "targeting rule", func() ([]PickerItem, error) { + return targetingPickerItems(cmd, client, projectID) + }) + if err != nil { + return err + } + current, err := client.TargetingRules.Get(cmd.Context(), projectID, id) + if err != nil { + return err + } + if current.RuleType == "checkpoint" { + return fmt.Errorf("checkpoint rule updates are not supported by this endpoint") + } + var newState string + if value, ok := body["state"]; ok { + if err := json.Unmarshal(value, &newState); err != nil { + return fmt.Errorf("state must be active or inactive") + } + if newState != "active" && newState != "inactive" { + return fmt.Errorf("state must be active or inactive") + } + } + if current.State == "active" || newState == "active" { + rt.Out.Notice("This change may affect which Offering matching customers receive.") + if err := confirmOrAbort(rt, "Update targeting rule now?"); err != nil { + return err + } + } + rule, err := client.TargetingRules.Update(cmd.Context(), projectID, id, body) + if err != nil { + return err + } + rt.Out.Success("Updated targeting rule " + rule.ID) + return rt.Out.Render(rule) + }, + } + cmd.Flags().StringVar(&config, "config", "", "JSON object of fields to update; use - for stdin") + return cmd +} + +func validTargetingUpdate(body api.TargetingRuleUpdate) error { + if len(body) == 0 { + return fmt.Errorf("config must contain at least one field") + } + allowed := map[string]bool{"position": true, "state": true, "display_name": true, "offering_id": true, "audience_id": true, "conditions": true, "schedule": true, "placements": true} + for key := range body { + if !allowed[key] { + return fmt.Errorf("unknown targeting rule field %q", key) + } + } + return nil +} + +func newTargetingDeleteCmd() *cobra.Command { + return &cobra.Command{ + Use: "delete [id]", + Short: "Delete a targeting rule", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + rt := RuntimeFrom(cmd.Context()) + projectID, err := requireProject(rt) + if err != nil { + return err + } + client, err := rt.API() + if err != nil { + return err + } + id, err := requireID(rt, argAt(args, 0), "targeting rule", func() ([]PickerItem, error) { + return targetingPickerItems(cmd, client, projectID) + }) + if err != nil { + return err + } + if err := confirmOrAbort(rt, "Delete targeting rule "+id+"?"); err != nil { + return err + } + if err := client.TargetingRules.Delete(cmd.Context(), projectID, id); err != nil { + return err + } + rt.Out.Success("Deleted targeting rule " + id) + return rt.Out.Render(map[string]any{"id": id, "deleted": true}) + }, + } +} diff --git a/internal/cli/targeting_test.go b/internal/cli/targeting_test.go new file mode 100644 index 00000000..d2f1ed76 --- /dev/null +++ b/internal/cli/targeting_test.go @@ -0,0 +1,59 @@ +package cli_test + +import ( + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestTargetingActivationRequiresApproval(t *testing.T) { + mutations := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + mutations++ + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"object":"targeting_rule","id":"trle1","rule_type":"legacy","state":"active","display_name":"US paywall","offering_id":"ofrng_us"}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + configPath := filepath.Join(t.TempDir(), "rule.json") + if err := os.WriteFile(configPath, []byte(`{"display_name":"US paywall","offering_id":"ofrng_us","state":"active"}`), 0o600); err != nil { + t.Fatal(err) + } + args := []string{"targeting", "create", "--config", configPath, "--project-id", "proj", "--api-key", "sk_test", "--no-input"} + _, _, err := runAgentCmd(t, args...) + if err == nil || !strings.Contains(err.Error(), "--yes") || mutations != 0 { + t.Fatalf("expected approval error before mutation: err=%v mutations=%d", err, mutations) + } + args = append(args, "--yes", "--json") + out, stderr, err := runAgentCmd(t, args...) + if err != nil || stderr != "" || mutations != 1 || !strings.Contains(out, `"state": "active"`) { + t.Fatalf("approved create failed: err=%v stderr=%q mutations=%d out=%s", err, stderr, mutations, out) + } +} + +func TestTargetingUpdateActiveRuleRequiresApproval(t *testing.T) { + mutations := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + mutations++ + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"object":"targeting_rule","id":"trle1","rule_type":"legacy","state":"active","display_name":"US paywall","offering_id":"ofrng_us"}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + configPath := filepath.Join(t.TempDir(), "change.json") + if err := os.WriteFile(configPath, []byte(`{"display_name":"US annual paywall"}`), 0o600); err != nil { + t.Fatal(err) + } + _, _, err := runAgentCmd(t, "targeting", "update", "trle1", "--config", configPath, "--project-id", "proj", "--api-key", "sk_test", "--no-input") + if err == nil || !strings.Contains(err.Error(), "--yes") || mutations != 0 { + t.Fatalf("expected approval error before mutation: err=%v mutations=%d", err, mutations) + } +} diff --git a/internal/cli/testdata/snapshots/targeting-list.golden b/internal/cli/testdata/snapshots/targeting-list.golden new file mode 100644 index 00000000..31020b38 --- /dev/null +++ b/internal/cli/testdata/snapshots/targeting-list.golden @@ -0,0 +1,3 @@ +$ rc targeting list --no-input --project-id proj_snap --api-key sk_snap +ID NAME TYPE STATE SERVES +trle_snap US annual paywall legacy active ofrng_us From d59f7904950b140da3a6ddb72911f32e137c4a47 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Fri, 25 Sep 2026 09:51:31 -0500 Subject: [PATCH 02/16] fix(targeting): show rule impact before activation --- docs/command-surface.md | 7 ++ docs/previews/targeting-create-active.svg | 36 ++++++ docs/specs/v2-beta-overlay.yaml | 69 +++++++++++ internal/cli/snapshot_test.go | 7 +- internal/cli/targeting.go | 115 +++++++++++++++--- .../snapshots/targeting-create-active.golden | 21 ++++ scripts/seed-beta-overlay.py | 1 + 7 files changed, 235 insertions(+), 21 deletions(-) create mode 100644 docs/previews/targeting-create-active.svg create mode 100644 internal/cli/testdata/snapshots/targeting-create-active.golden diff --git a/docs/command-surface.md b/docs/command-surface.md index 55992699..08c9bcb8 100644 --- a/docs/command-surface.md +++ b/docs/command-surface.md @@ -203,6 +203,13 @@ rc experiments pause # pauses enrollment; conf rc experiments resume # resumes enrollment; configuration review + confirmation/--yes rc experiments stop # permanent; confirmation/--yes +# Targeting rules — development v2 endpoints +rc targeting list [--state ] +rc targeting show +rc targeting create --name --offering [--state inactive|active] [--config ] # initially inactive +rc targeting update --config # partial update +rc targeting delete # confirmation/--yes + # Rico (AI assistant) rc rico [message] # streaming chat window in a TTY (--plain for a line loop) rc rico --continue # continue the most recent conversation diff --git a/docs/previews/targeting-create-active.svg b/docs/previews/targeting-create-active.svg new file mode 100644 index 00000000..3ef7620f --- /dev/null +++ b/docs/previews/targeting-create-active.svg @@ -0,0 +1,36 @@ + + + + + +$ rc targeting create --name Default paywall --offering ofrng_default --state active --yes --no-input --project-id proj_snap --api-key sk_snap + +▍ Targeting rule — Default paywall +  Apply this rule in priority order when it becomes active. + +  Type                        legacy +  State                       active +  Offering                    ofrng_default + +▐ This rule matches everyone. Active rules use the first match. + +  Position                    Append to the end + +▍ Plan +· 1. Create the rule and apply it to matching customers +✓ Created targeting rule trle_snap +id            trle_snap +display_name  Default paywall +offering_id   ofrng_default +rule_type     legacy +state         active + + + diff --git a/docs/specs/v2-beta-overlay.yaml b/docs/specs/v2-beta-overlay.yaml index bca33fd1..92365d15 100644 --- a/docs/specs/v2-beta-overlay.yaml +++ b/docs/specs/v2-beta-overlay.yaml @@ -1098,6 +1098,75 @@ paths: responses: '200': description: OK + /projects/{project_id}/targeting_rules: + parameters: + - name: project_id + in: path + required: true + schema: + type: string + get: + summary: Get a list of targeting rules + operationId: list-targeting-rules + x-release-status: development + x-revenuecat-release-gatekeeping: false + x-scopes: + - project_configuration:targeting_rules:read + responses: + '200': + description: OK + post: + summary: Create a targeting rule + operationId: create-targeting-rule + x-release-status: development + x-revenuecat-release-gatekeeping: false + x-scopes: + - project_configuration:targeting_rules:read_write + responses: + '200': + description: OK + /projects/{project_id}/targeting_rules/{targeting_rule_id}: + parameters: + - name: project_id + in: path + required: true + schema: + type: string + - name: targeting_rule_id + in: path + required: true + schema: + type: string + delete: + summary: Delete a targeting rule + operationId: delete-targeting-rule + x-release-status: development + x-revenuecat-release-gatekeeping: false + x-scopes: + - project_configuration:targeting_rules:read_write + responses: + '200': + description: OK + get: + summary: Get a targeting rule + operationId: get-targeting-rule + x-release-status: development + x-revenuecat-release-gatekeeping: false + x-scopes: + - project_configuration:targeting_rules:read + responses: + '200': + description: OK + post: + summary: Update a targeting rule + operationId: update-targeting-rule + x-release-status: development + x-revenuecat-release-gatekeeping: false + x-scopes: + - project_configuration:targeting_rules:read_write + responses: + '200': + description: OK components: schemas: Currency: diff --git a/internal/cli/snapshot_test.go b/internal/cli/snapshot_test.go index 3c670cf5..68d5e545 100644 --- a/internal/cli/snapshot_test.go +++ b/internal/cli/snapshot_test.go @@ -29,7 +29,11 @@ func snapshotServer(t *testing.T) *httptest.Server { w.Header().Set("Content-Type", "application/json") switch { case strings.HasSuffix(r.URL.Path, "/targeting_rules"): - io.WriteString(w, `{"object":"list","items":[{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"US annual paywall","offering_id":"ofrng_us"}],"next_page":null,"url":"/projects/proj_snap/targeting_rules"}`) + if r.Method == http.MethodPost { + io.WriteString(w, `{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"Default paywall","offering_id":"ofrng_default"}`) + } else { + io.WriteString(w, `{"object":"list","items":[{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"US annual paywall","offering_id":"ofrng_us"}],"next_page":null,"url":"/projects/proj_snap/targeting_rules"}`) + } case strings.HasSuffix(r.URL.Path, "/experiments/exp_snap/actions/start"): io.WriteString(w, `{"object":"experiment","id":"exp_snap","display_name":"New paywall","status":"running","created_at":1784297950368,"updated_at":1784297950368}`) case strings.HasSuffix(r.URL.Path, "/experiments/exp_snap"): @@ -70,6 +74,7 @@ func TestOutputSnapshots(t *testing.T) { {"experiments-results", []string{"experiments", "results", "exp_snap", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"experiments-start", []string{"experiments", "start", "exp_snap", "--yes", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"targeting-list", []string{"targeting", "list", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, + {"targeting-create-active", []string{"targeting", "create", "--name", "Default paywall", "--offering", "ofrng_default", "--state", "active", "--yes", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"apps-list", []string{"apps", "list", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"apps-list-all-projects", []string{"apps", "list", "--all-projects", "--bundle-id", "com.example.moodly", "--no-input", "--api-key", "sk_snap"}}, {"error-not-found", []string{"offerings", "show", "ofrng_missing", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index 3ab2d710..8c089382 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -24,7 +24,7 @@ func newTargetingCmd() *cobra.Command { } func newTargetingListCmd() *cobra.Command { - var state, startingAfter string + var state, cursor string var limit int cmd := &cobra.Command{ Use: "list", @@ -40,7 +40,7 @@ func newTargetingListCmd() *cobra.Command { if err != nil { return err } - page, err := client.TargetingRules.List(cmd.Context(), projectID, api.ListTargetingRulesOptions{State: state, Limit: limit, StartingAfter: startingAfter}) + page, err := client.TargetingRules.List(cmd.Context(), projectID, api.ListTargetingRulesOptions{State: state, Limit: limit, StartingAfter: cursor}) if err != nil { return err } @@ -52,12 +52,16 @@ func newTargetingListCmd() *cobra.Command { } rows = append(rows, []string{rule.ID, rule.DisplayName, rule.RuleType, rule.State, serves}) } - return rt.Out.RenderTable(output.Table{Columns: []string{"ID", "NAME", "TYPE", "STATE", "SERVES"}, Rows: rows, Raw: page}) + if err := rt.Out.RenderTable(output.Table{Columns: []string{"ID", "NAME", "TYPE", "STATE", "SERVES"}, Rows: rows, Raw: page}); err != nil { + return err + } + hintMoreResults(rt, page) + return nil }, } cmd.Flags().StringVar(&state, "state", "", "filter by active, scheduled, or inactive") cmd.Flags().IntVar(&limit, "limit", 20, "maximum rules to return (1–100)") - cmd.Flags().StringVar(&startingAfter, "starting-after", "", "pagination cursor from the previous page") + cmd.Flags().StringVar(&cursor, "cursor", "", "item ID to start after (pagination)") return cmd } @@ -99,7 +103,13 @@ func newTargetingShowCmd() *cobra.Command { if err != nil { return err } - return rt.Out.Render(rule) + if err := rt.Out.Render(rule); err != nil { + return err + } + if !rt.Globals.JSON { + rt.Out.Hint("Use --json to inspect conditions, placements, schedule, and checkpoints.") + } + return nil }, } } @@ -109,9 +119,14 @@ func newTargetingCreateCmd() *cobra.Command { cmd := &cobra.Command{ Use: "create", Short: "Create a targeting rule", - Long: "Creates an inactive Offering rule by default. Use --config for audiences, conditions, schedules, placements, or a checkpoint rule. Activating a rule requires confirmation.", - Example: ` rc targeting create --name "US paywall" --offering ofrng_us - rc targeting create --config rule.json --yes --json --no-input`, + Long: "Creates an inactive Offering rule by default. Without audience_id or conditions, a legacy rule matches everyone. Use --config for audience_id, conditions, schedule, placements, position, or a checkpoint rule with flow_id and checkpoints. Active or scheduled rules require confirmation.", + Example: ` rc targeting create --name "Default paywall" --offering ofrng_default + rc targeting create --config - --no-input <<'JSON' + {"rule_type":"legacy","display_name":"US paywall","offering_id":"ofrng_us","conditions":[{"field":"country","operator":"in","value":["US"]}]} + JSON + rc targeting create --config - --no-input <<'JSON' + {"rule_type":"checkpoint","display_name":"After onboarding","audience_id":"aud_123","flow_id":"wf_123","checkpoints":[{"checkpoint_id":"chkpt_123"}]} + JSON`, RunE: func(cmd *cobra.Command, _ []string) error { rt := RuntimeFrom(cmd.Context()) projectID, err := requireProject(rt) @@ -139,15 +154,19 @@ func newTargetingCreateCmd() *cobra.Command { if body.State == "" { body.State = "inactive" } - if err := gatherTargetingCreateInput(rt, &body); err != nil { + if err := gatherTargetingCreateInput(cmd, rt, projectID, &body); err != nil { return err } if err := validateTargetingCreate(body); err != nil { return err } if body.State != "inactive" { - rt.Out.Notice("This rule will affect which Offering or Flow matching customers receive.") - if err := confirmOrAbort(rt, "Activate targeting rule now?"); err != nil { + showTargetingCreatePlan(rt, body) + prompt := "Activate targeting rule now?" + if body.State == "scheduled" { + prompt = "Schedule targeting rule now?" + } + if err := confirmOrAbort(rt, prompt); err != nil { return err } } @@ -165,12 +184,12 @@ func newTargetingCreateCmd() *cobra.Command { } cmd.Flags().StringVar(&name, "name", "", "rule display name") cmd.Flags().StringVar(&offering, "offering", "", "Offering ID served by a legacy rule") - cmd.Flags().StringVar(&state, "state", "", "inactive (default) or active") + cmd.Flags().StringVar(&state, "state", "", "inactive (default), active, or scheduled (checkpoint only)") cmd.Flags().StringVar(&config, "config", "", "JSON config file; use - for stdin") return cmd } -func gatherTargetingCreateInput(rt *Runtime, body *api.TargetingRuleCreate) error { +func gatherTargetingCreateInput(cmd *cobra.Command, rt *Runtime, projectID string, body *api.TargetingRuleCreate) error { if body.RuleType == "checkpoint" { var missing []string if body.DisplayName == "" { @@ -207,10 +226,53 @@ func gatherTargetingCreateInput(rt *Runtime, body *api.TargetingRuleCreate) erro if body.DisplayName == "" { form.Field(huh.NewInput().Title("Rule name").Value(&body.DisplayName).Validate(tui.Required("rule name"))) } + if err := form.Run(); err != nil { + return err + } if body.OfferingID == "" { - form.Field(huh.NewInput().Title("Offering ID").Value(&body.OfferingID).Validate(tui.Required("offering ID"))) + client, err := rt.API() + if err != nil { + return err + } + body.OfferingID, err = requireID(rt, "", "offering", func() ([]PickerItem, error) { + return offeringPickerItems(cmd.Context(), client, projectID) + }) + return err + } + return nil +} + +func showTargetingCreatePlan(rt *Runtime, body api.TargetingRuleCreate) { + rt.Out.Title("Targeting rule — " + body.DisplayName) + rt.Out.Lead("Apply this rule in priority order when it becomes active.") + rt.Out.Field("Type", body.RuleType) + rt.Out.Field("State", body.State) + if body.RuleType == "legacy" { + rt.Out.Field("Offering", body.OfferingID) + if body.AudienceID != "" { + rt.Out.Field("Audience", body.AudienceID) + } else if len(body.Conditions) > 0 && string(body.Conditions) != "[]" { + rt.Out.Field("Conditions", string(body.Conditions)) + } else { + rt.Out.Notice("This rule matches everyone. Active rules use the first match.") + } + if body.Position != nil { + rt.Out.Field("Position", fmt.Sprint(*body.Position)) + } else { + rt.Out.Field("Position", "Append to the end") + } + if len(body.Placements) > 0 { + rt.Out.Field("Placements", string(body.Placements)) + } + } else { + rt.Out.Field("Flow", body.FlowID) + rt.Out.Field("Audience", body.AudienceID) + rt.Out.Field("Checkpoints", string(body.Checkpoints)) + } + if len(body.Schedule) > 0 { + rt.Out.Field("Schedule", string(body.Schedule)) } - return form.Run() + rt.Out.Plan([]string{"Create the rule and apply it to matching customers"}) } func validateTargetingCreate(body api.TargetingRuleCreate) error { @@ -229,10 +291,11 @@ func validateTargetingCreate(body api.TargetingRuleCreate) error { func newTargetingUpdateCmd() *cobra.Command { var config string cmd := &cobra.Command{ - Use: "update [id]", - Short: "Update an Offering targeting rule", - Long: "Partially updates a legacy targeting rule from a JSON object. An active rule or an activation requires confirmation. Checkpoint rule updates are not exposed by this endpoint.", - Args: cobra.MaximumNArgs(1), + Use: "update [id]", + Short: "Update an Offering targeting rule", + Long: "Partially updates a legacy targeting rule from a JSON object with position, state, display_name, offering_id, audience_id, conditions, schedule, or placements. An active rule or an activation requires confirmation. Checkpoint rule updates are not exposed by this endpoint.", + Example: ` echo '{"state":"active","position":1}' | rc targeting update trle_123 --config - --yes --no-input`, + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if config == "" { return fmt.Errorf("pass --config or --config - for stdin") @@ -276,7 +339,19 @@ func newTargetingUpdateCmd() *cobra.Command { } } if current.State == "active" || newState == "active" { - rt.Out.Notice("This change may affect which Offering matching customers receive.") + rt.Out.Title("Targeting rule — " + current.DisplayName) + rt.Out.Lead("Change the rule used to choose an Offering for matching customers.") + rt.Out.Field("Current state", current.State) + rt.Out.Field("Current offering", current.OfferingID) + if current.AudienceID != nil { + rt.Out.Field("Current audience", *current.AudienceID) + } else if len(current.Conditions) > 0 { + rt.Out.Field("Current conditions", compactJSON(current.Conditions)) + } else { + rt.Out.Notice("Current rule matches everyone. Active rules use the first match.") + } + rt.Out.Field("Changes", compactJSON(body)) + rt.Out.Plan([]string{"Update the targeting rule"}) if err := confirmOrAbort(rt, "Update targeting rule now?"); err != nil { return err } diff --git a/internal/cli/testdata/snapshots/targeting-create-active.golden b/internal/cli/testdata/snapshots/targeting-create-active.golden new file mode 100644 index 00000000..1dafecf7 --- /dev/null +++ b/internal/cli/testdata/snapshots/targeting-create-active.golden @@ -0,0 +1,21 @@ +$ rc targeting create --name Default paywall --offering ofrng_default --state active --yes --no-input --project-id proj_snap --api-key sk_snap + +▍ Targeting rule — Default paywall + Apply this rule in priority order when it becomes active. + + Type legacy + State active + Offering ofrng_default + +▐ This rule matches everyone. Active rules use the first match. + + Position Append to the end + +▍ Plan +· 1. Create the rule and apply it to matching customers +✓ Created targeting rule trle_snap +id trle_snap +display_name Default paywall +offering_id ofrng_default +rule_type legacy +state active diff --git a/scripts/seed-beta-overlay.py b/scripts/seed-beta-overlay.py index 02c4ad23..d6b84138 100644 --- a/scripts/seed-beta-overlay.py +++ b/scripts/seed-beta-overlay.py @@ -33,6 +33,7 @@ "/products/{product_id}/prices", "/products/{product_id}/test_store_prices", "/experiments", + "/targeting_rules", ] REF_RE = re.compile(r"#/components/([A-Za-z0-9]+)/([A-Za-z0-9_.-]+)") From f3a8a56a3b6b881257155aec80edca0938257dac Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Fri, 25 Sep 2026 09:55:03 -0500 Subject: [PATCH 03/16] fix(targeting): show resulting audience on updates --- docs/command-surface.md | 2 +- internal/cli/targeting.go | 44 ++++++++++++++++++++++++++++------ internal/cli/targeting_test.go | 17 +++++++++++++ 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/docs/command-surface.md b/docs/command-surface.md index 08c9bcb8..80e78396 100644 --- a/docs/command-surface.md +++ b/docs/command-surface.md @@ -206,7 +206,7 @@ rc experiments stop # permanent; confirmatio # Targeting rules — development v2 endpoints rc targeting list [--state ] rc targeting show -rc targeting create --name --offering [--state inactive|active] [--config ] # initially inactive +rc targeting create (--name --offering [--state inactive|active] | --config ) # initially inactive rc targeting update --config # partial update rc targeting delete # confirmation/--yes diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index 8c089382..336f26bf 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -121,12 +121,8 @@ func newTargetingCreateCmd() *cobra.Command { Short: "Create a targeting rule", Long: "Creates an inactive Offering rule by default. Without audience_id or conditions, a legacy rule matches everyone. Use --config for audience_id, conditions, schedule, placements, position, or a checkpoint rule with flow_id and checkpoints. Active or scheduled rules require confirmation.", Example: ` rc targeting create --name "Default paywall" --offering ofrng_default - rc targeting create --config - --no-input <<'JSON' - {"rule_type":"legacy","display_name":"US paywall","offering_id":"ofrng_us","conditions":[{"field":"country","operator":"in","value":["US"]}]} - JSON - rc targeting create --config - --no-input <<'JSON' - {"rule_type":"checkpoint","display_name":"After onboarding","audience_id":"aud_123","flow_id":"wf_123","checkpoints":[{"checkpoint_id":"chkpt_123"}]} - JSON`, + echo '{"rule_type":"legacy","display_name":"US paywall","offering_id":"ofrng_us","conditions":[{"field":"country","operator":"in","value":["US"]}]}' | rc targeting create --config - --no-input + echo '{"rule_type":"checkpoint","display_name":"After onboarding","audience_id":"aud_123","flow_id":"wf_123","checkpoints":[{"checkpoint_id":"chkpt_123"}]}' | rc targeting create --config - --no-input`, RunE: func(cmd *cobra.Command, _ []string) error { rt := RuntimeFrom(cmd.Context()) projectID, err := requireProject(rt) @@ -348,9 +344,17 @@ func newTargetingUpdateCmd() *cobra.Command { } else if len(current.Conditions) > 0 { rt.Out.Field("Current conditions", compactJSON(current.Conditions)) } else { - rt.Out.Notice("Current rule matches everyone. Active rules use the first match.") + rt.Out.Field("Current audience", "Everyone") } rt.Out.Field("Changes", compactJSON(body)) + resultingAudience, everyone, err := targetingAudienceAfterUpdate(current, body) + if err != nil { + return err + } + rt.Out.Field("Resulting audience", resultingAudience) + if everyone { + rt.Out.Notice("Resulting rule matches everyone. Active rules use the first match.") + } rt.Out.Plan([]string{"Update the targeting rule"}) if err := confirmOrAbort(rt, "Update targeting rule now?"); err != nil { return err @@ -381,6 +385,32 @@ func validTargetingUpdate(body api.TargetingRuleUpdate) error { return nil } +func targetingAudienceAfterUpdate(current *api.TargetingRule, body api.TargetingRuleUpdate) (string, bool, error) { + audienceID := "" + if current.AudienceID != nil { + audienceID = *current.AudienceID + } + conditions := current.Conditions + if value, ok := body["audience_id"]; ok { + audienceID = "" + if err := json.Unmarshal(value, &audienceID); err != nil { + return "", false, fmt.Errorf("audience_id must be a string or null") + } + } + if value, ok := body["conditions"]; ok { + if err := json.Unmarshal(value, &conditions); err != nil { + return "", false, fmt.Errorf("conditions must be an array") + } + } + if audienceID != "" { + return audienceID, false, nil + } + if len(conditions) > 0 { + return compactJSON(conditions), false, nil + } + return "Everyone", true, nil +} + func newTargetingDeleteCmd() *cobra.Command { return &cobra.Command{ Use: "delete [id]", diff --git a/internal/cli/targeting_test.go b/internal/cli/targeting_test.go index d2f1ed76..10f92ade 100644 --- a/internal/cli/targeting_test.go +++ b/internal/cli/targeting_test.go @@ -57,3 +57,20 @@ func TestTargetingUpdateActiveRuleRequiresApproval(t *testing.T) { t.Fatalf("expected approval error before mutation: err=%v mutations=%d", err, mutations) } } + +func TestTargetingUpdateWarnsWhenAudienceIsCleared(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"object":"targeting_rule","id":"trle1","rule_type":"legacy","state":"active","display_name":"US paywall","offering_id":"ofrng_us","audience_id":"aud_us","conditions":[]}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + configPath := filepath.Join(t.TempDir(), "change.json") + if err := os.WriteFile(configPath, []byte(`{"audience_id":null}`), 0o600); err != nil { + t.Fatal(err) + } + _, stderr, err := runAgentCmd(t, "targeting", "update", "trle1", "--config", configPath, "--project-id", "proj", "--api-key", "sk_test", "--no-input") + if err == nil || !strings.Contains(err.Error(), "--yes") || !strings.Contains(stderr, "Resulting audience") || !strings.Contains(stderr, "matches everyone") { + t.Fatalf("expected global-scope warning before approval; err=%v stderr=%q", err, stderr) + } +} From ea3fb0bb3617a39dd4206fc50a85da1935e72a48 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Fri, 25 Sep 2026 14:13:54 -0500 Subject: [PATCH 04/16] fix(targeting): show rule details in human output --- docs/command-surface.md | 2 +- docs/previews/targeting-create-active.svg | 21 ++-- docs/previews/targeting-show.svg | 32 +++++ internal/cli/snapshot_test.go | 3 + internal/cli/targeting.go | 12 +- internal/cli/targeting_format.go | 117 ++++++++++++++++++ internal/cli/targeting_test.go | 23 ++++ .../snapshots/targeting-create-active.golden | 17 ++- .../testdata/snapshots/targeting-show.golden | 17 +++ 9 files changed, 222 insertions(+), 22 deletions(-) create mode 100644 docs/previews/targeting-show.svg create mode 100644 internal/cli/targeting_format.go create mode 100644 internal/cli/testdata/snapshots/targeting-show.golden diff --git a/docs/command-surface.md b/docs/command-surface.md index 80e78396..9bba4274 100644 --- a/docs/command-surface.md +++ b/docs/command-surface.md @@ -205,7 +205,7 @@ rc experiments stop # permanent; confirmatio # Targeting rules — development v2 endpoints rc targeting list [--state ] -rc targeting show +rc targeting show # human detail view shows audience, placements, schedule, and checkpoints rc targeting create (--name --offering [--state inactive|active] | --config ) # initially inactive rc targeting update --config # partial update rc targeting delete # confirmation/--yes diff --git a/docs/previews/targeting-create-active.svg b/docs/previews/targeting-create-active.svg index 3ef7620f..9d5d934f 100644 --- a/docs/previews/targeting-create-active.svg +++ b/docs/previews/targeting-create-active.svg @@ -1,6 +1,6 @@ - + + $ rc targeting create --name Default paywall --offering ofrng_default --state active --yes --no-input --project-id proj_snap --api-key sk_snap @@ -26,11 +26,18 @@ ▍ Plan · 1. Create the rule and apply it to matching customers ✓ Created targeting rule trle_snap -id            trle_snap -display_name  Default paywall -offering_id   ofrng_default -rule_type     legacy -state         active +  Use --json for the complete API response. +▍ Default paywall +  trle_snap · legacy · active + +Serves +  Offering:  ofrng_default + +Audience +  Audience:  All eligible customers + +Placements +  No placement overrides diff --git a/docs/previews/targeting-show.svg b/docs/previews/targeting-show.svg new file mode 100644 index 00000000..44f86c07 --- /dev/null +++ b/docs/previews/targeting-show.svg @@ -0,0 +1,32 @@ + + + + + +$ rc targeting show trle_snap --no-input --project-id proj_snap --api-key sk_snap +  Use --json for the complete API response. +▍ US annual paywall +  trle_snap · legacy · active + +Serves +  Offering:  ofrng_us + +Audience +  Condition 1:  platform in ios + +Placements +  Fallback:    ofrng_default +  onboarding:  ofrng_us + +Schedule +  Start:  2026-09-25T12:00:00Z + + + diff --git a/internal/cli/snapshot_test.go b/internal/cli/snapshot_test.go index 68d5e545..c62c674e 100644 --- a/internal/cli/snapshot_test.go +++ b/internal/cli/snapshot_test.go @@ -28,6 +28,8 @@ func snapshotServer(t *testing.T) *httptest.Server { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") switch { + case strings.HasSuffix(r.URL.Path, "/targeting_rules/trle_snap"): + io.WriteString(w, `{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"US annual paywall","offering_id":"ofrng_us","conditions":[{"field":"platform","operator":"in","value":["ios"]}],"placements":{"fallback_offering_id":"ofrng_default","placement_offerings":[{"placement_identifier":"onboarding","offering_id":"ofrng_us"}]},"schedule":{"start_date":"2026-09-25T12:00:00Z","end_date":null}}`) case strings.HasSuffix(r.URL.Path, "/targeting_rules"): if r.Method == http.MethodPost { io.WriteString(w, `{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"Default paywall","offering_id":"ofrng_default"}`) @@ -74,6 +76,7 @@ func TestOutputSnapshots(t *testing.T) { {"experiments-results", []string{"experiments", "results", "exp_snap", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"experiments-start", []string{"experiments", "start", "exp_snap", "--yes", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"targeting-list", []string{"targeting", "list", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, + {"targeting-show", []string{"targeting", "show", "trle_snap", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"targeting-create-active", []string{"targeting", "create", "--name", "Default paywall", "--offering", "ofrng_default", "--state", "active", "--yes", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"apps-list", []string{"apps", "list", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"apps-list-all-projects", []string{"apps", "list", "--all-projects", "--bundle-id", "com.example.moodly", "--no-input", "--api-key", "sk_snap"}}, diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index 336f26bf..def744e8 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -103,13 +103,7 @@ func newTargetingShowCmd() *cobra.Command { if err != nil { return err } - if err := rt.Out.Render(rule); err != nil { - return err - } - if !rt.Globals.JSON { - rt.Out.Hint("Use --json to inspect conditions, placements, schedule, and checkpoints.") - } - return nil + return renderTargetingShow(rt, rule) }, } } @@ -175,7 +169,7 @@ func newTargetingCreateCmd() *cobra.Command { return err } rt.Out.Success("Created targeting rule " + rule.ID) - return rt.Out.Render(rule) + return renderTargetingShow(rt, rule) }, } cmd.Flags().StringVar(&name, "name", "", "rule display name") @@ -365,7 +359,7 @@ func newTargetingUpdateCmd() *cobra.Command { return err } rt.Out.Success("Updated targeting rule " + rule.ID) - return rt.Out.Render(rule) + return renderTargetingShow(rt, rule) }, } cmd.Flags().StringVar(&config, "config", "", "JSON object of fields to update; use - for stdin") diff --git a/internal/cli/targeting_format.go b/internal/cli/targeting_format.go new file mode 100644 index 00000000..8c703e11 --- /dev/null +++ b/internal/cli/targeting_format.go @@ -0,0 +1,117 @@ +package cli + +import ( + "encoding/json" + "fmt" + + "github.com/revenuecat/cli/internal/api" + "github.com/revenuecat/cli/internal/output" +) + +type targetingDisplay struct { + Conditions []api.ExperimentCondition `json:"conditions"` + Schedule *struct { + StartDate *string `json:"start_date"` + EndDate *string `json:"end_date"` + } `json:"schedule"` + Placements *struct { + FallbackOfferingID *string `json:"fallback_offering_id"` + PlacementOfferings []struct { + PlacementIdentifier string `json:"placement_identifier"` + OfferingID *string `json:"offering_id"` + } `json:"placement_offerings"` + } `json:"placements"` + Checkpoints []struct { + CheckpointID string `json:"checkpoint_id"` + Position int `json:"position"` + Checkpoint *struct { + Identifier string `json:"identifier"` + } `json:"checkpoint"` + } `json:"checkpoints"` +} + +func renderTargetingShow(rt *Runtime, rule *api.TargetingRule) error { + if rt.Globals.JSON { + return rt.Out.Render(rule) + } + + data, err := json.Marshal(rule) + if err != nil { + return err + } + var view targetingDisplay + if err := json.Unmarshal(data, &view); err != nil { + return err + } + + serves := []output.CardLine{} + if rule.OfferingID != "" { + serves = append(serves, output.CardLine{Key: "Offering", Value: rule.OfferingID}) + } + if rule.FlowID != "" { + serves = append(serves, output.CardLine{Key: "Flow", Value: rule.FlowID}) + } + + audience := []output.CardLine{} + if rule.AudienceID != nil && *rule.AudienceID != "" { + audience = append(audience, output.CardLine{Key: "Audience", Value: *rule.AudienceID}) + } else if len(view.Conditions) > 0 { + for i, condition := range view.Conditions { + audience = append(audience, output.CardLine{Key: fmt.Sprintf("Condition %d", i+1), Value: experimentConditionLabel(condition)}) + } + } else { + audience = append(audience, output.CardLine{Key: "Audience", Value: "All eligible customers"}) + } + + sections := []output.CardSection{ + {Heading: "Serves", Lines: serves}, + {Heading: "Audience", Lines: audience}, + } + if rule.RuleType == "legacy" { + placements := []output.CardLine{} + if view.Placements != nil { + if view.Placements.FallbackOfferingID != nil { + placements = append(placements, output.CardLine{Key: "Fallback", Value: *view.Placements.FallbackOfferingID}) + } + for _, placement := range view.Placements.PlacementOfferings { + offering := "Use fallback" + if placement.OfferingID != nil { + offering = *placement.OfferingID + } + placements = append(placements, output.CardLine{Key: placement.PlacementIdentifier, Value: offering}) + } + } + sections = append(sections, output.CardSection{Heading: "Placements", Lines: placements, Empty: "No placement overrides"}) + } + if view.Schedule != nil { + schedule := []output.CardLine{} + if view.Schedule.StartDate != nil { + schedule = append(schedule, output.CardLine{Key: "Start", Value: *view.Schedule.StartDate}) + } + if view.Schedule.EndDate != nil { + schedule = append(schedule, output.CardLine{Key: "End", Value: *view.Schedule.EndDate}) + } + sections = append(sections, output.CardSection{Heading: "Schedule", Lines: schedule}) + } + if rule.RuleType == "checkpoint" { + checkpoints := []output.CardLine{} + for _, checkpoint := range view.Checkpoints { + name := checkpoint.CheckpointID + if checkpoint.Checkpoint != nil && checkpoint.Checkpoint.Identifier != "" { + name = checkpoint.Checkpoint.Identifier + " (" + checkpoint.CheckpointID + ")" + } + checkpoints = append(checkpoints, output.CardLine{Key: name, Value: fmt.Sprintf("Position %d", checkpoint.Position)}) + } + sections = append(sections, output.CardSection{Heading: "Checkpoints", Lines: checkpoints}) + } + if err := rt.Out.RenderCard(output.Card{ + Title: rule.DisplayName, + Subtitle: rule.ID + " · " + rule.RuleType + " · " + rule.State, + Sections: sections, + Raw: rule, + }); err != nil { + return err + } + rt.Out.Hint("Use --json for the complete API response.") + return nil +} diff --git a/internal/cli/targeting_test.go b/internal/cli/targeting_test.go index 10f92ade..b8a73f53 100644 --- a/internal/cli/targeting_test.go +++ b/internal/cli/targeting_test.go @@ -74,3 +74,26 @@ func TestTargetingUpdateWarnsWhenAudienceIsCleared(t *testing.T) { t.Fatalf("expected global-scope warning before approval; err=%v stderr=%q", err, stderr) } } + +func TestTargetingShowCheckpointDetailsAndJSON(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"object":"targeting_rule","id":"trle1","rule_type":"checkpoint","state":"scheduled","display_name":"After onboarding","flow_id":"wf1","audience_id":"aud1","checkpoints":[{"checkpoint_id":"chkpt1","position":2,"checkpoint":{"identifier":"app_open"}}],"schedule":{"start_date":null,"end_date":"2026-10-01T00:00:00Z"}}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + args := []string{"targeting", "show", "trle1", "--project-id", "proj", "--api-key", "sk_test", "--no-input"} + out, _, err := runAgentCmd(t, args...) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"wf1", "aud1", "app_open (chkpt1)", "Position 2", "2026-10-01T00:00:00Z"} { + if !strings.Contains(out, want) { + t.Fatalf("missing %q from checkpoint view: %s", want, out) + } + } + out, _, err = runAgentCmd(t, append(args, "--json")...) + if err != nil || !strings.Contains(out, `"checkpoint_id": "chkpt1"`) || !strings.Contains(out, `"end_date": "2026-10-01T00:00:00Z"`) { + t.Fatalf("JSON lost checkpoint details: err=%v out=%s", err, out) + } +} diff --git a/internal/cli/testdata/snapshots/targeting-create-active.golden b/internal/cli/testdata/snapshots/targeting-create-active.golden index 1dafecf7..64cf77c1 100644 --- a/internal/cli/testdata/snapshots/targeting-create-active.golden +++ b/internal/cli/testdata/snapshots/targeting-create-active.golden @@ -14,8 +14,15 @@ $ rc targeting create --name Default paywall --offering ofrng_default --state ac ▍ Plan · 1. Create the rule and apply it to matching customers ✓ Created targeting rule trle_snap -id trle_snap -display_name Default paywall -offering_id ofrng_default -rule_type legacy -state active + Use --json for the complete API response. +▍ Default paywall + trle_snap · legacy · active + +Serves + Offering: ofrng_default + +Audience + Audience: All eligible customers + +Placements + No placement overrides diff --git a/internal/cli/testdata/snapshots/targeting-show.golden b/internal/cli/testdata/snapshots/targeting-show.golden new file mode 100644 index 00000000..7cd1c425 --- /dev/null +++ b/internal/cli/testdata/snapshots/targeting-show.golden @@ -0,0 +1,17 @@ +$ rc targeting show trle_snap --no-input --project-id proj_snap --api-key sk_snap + Use --json for the complete API response. +▍ US annual paywall + trle_snap · legacy · active + +Serves + Offering: ofrng_us + +Audience + Condition 1: platform in ios + +Placements + Fallback: ofrng_default + onboarding: ofrng_us + +Schedule + Start: 2026-09-25T12:00:00Z From cd2b53139030d3622c56b10aac4acec9d08ee55f Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Fri, 25 Sep 2026 14:56:15 -0500 Subject: [PATCH 05/16] feat(targeting): expose rule config fields in schema --- internal/cli/config_schema.go | 51 +++++++++++++++++++++++++++++++++++ internal/cli/schema_test.go | 15 +++++++++++ internal/cli/targeting.go | 4 +-- 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/internal/cli/config_schema.go b/internal/cli/config_schema.go index 7e654cbd..708cd336 100644 --- a/internal/cli/config_schema.go +++ b/internal/cli/config_schema.go @@ -8,10 +8,53 @@ func configFieldsFor(cmd *cobra.Command) map[string]any { return experimentConfigFields(true) case "rc experiments update": return experimentConfigFields(false) + case "rc targeting create": + return targetingConfigFields(true) + case "rc targeting update": + return targetingConfigFields(false) } return nil } +func targetingConfigFields(create bool) map[string]any { + schedule := map[string]any{"type": "object", "description": "UTC timestamps ending in Z", "properties": map[string]any{ + "start_date": configField("string", "Start time, for example 2026-05-25T10:00:00Z"), + "end_date": configField("string", "Optional end time in the same format"), + }} + placements := map[string]any{"type": "object", "description": "Legacy rules only", "properties": map[string]any{ + "fallback_offering_id": configField("string", "Fallback Offering ID"), + "placement_offerings": map[string]any{"type": "array", "items": map[string]any{ + "type": "object", "required": []string{"placement_identifier", "offering_id"}, "properties": map[string]any{ + "placement_identifier": configField("string", "Placement identifier, such as onboarding"), + "offering_id": configField("string", "Offering ID for this placement"), + }, + }}, + }} + fields := map[string]any{ + "position": map[string]any{"type": "integer", "minimum": 1, "description": "One-based priority among rules in the same state; legacy rules only"}, + "state": configEnum("active or inactive for legacy; checkpoint also supports scheduled", "active", "inactive", "scheduled"), + "display_name": configField("string", "Rule name"), + "offering_id": configField("string", "Offering ID served by a legacy rule"), + "audience_id": configField("string", "Audience ID; mutually exclusive with conditions"), + "conditions": targetingConditionsSchema(), + "schedule": schedule, + "placements": placements, + } + if create { + fields["rule_type"] = configEnum("Rule type; defaults to legacy", "legacy", "checkpoint") + fields["id"] = configField("string", "Optional custom ID for a legacy rule") + fields["flow_id"] = configField("string", "Flow ID served by a checkpoint rule") + fields["checkpoints"] = map[string]any{"type": "array", "description": "Exactly one checkpoint for a checkpoint rule", "items": map[string]any{ + "type": "object", "required": []string{"checkpoint_id"}, "properties": map[string]any{ + "checkpoint_id": configField("string", "Checkpoint ID"), + "position": map[string]any{"type": "integer", "minimum": 0, "description": "Priority within the checkpoint"}, + }, + }} + return map[string]any{"type": "object", "properties": fields, "description": "Legacy: display_name and offering_id required. Checkpoint: rule_type, display_name, audience_id, flow_id, checkpoints required."} + } + return map[string]any{"type": "object", "properties": fields, "description": "Partial update of a legacy rule. Checkpoint updates are unavailable."} +} + func configField(kind, description string) map[string]any { return map[string]any{"type": kind, "description": description} } @@ -93,6 +136,14 @@ func targetingConditionsSchema() map[string]any { {"field": "platform", "operator": "in", "value": []string{"ios"}}, {"field": "app_version", "operator": ">=", "value": "1.2.0", "context": "app1a2b3c4"}, }, + "field_rules": map[string]any{ + "app_config": map[string]any{"operators": []string{"in", "not in"}, "value": "array of app IDs"}, + "country": map[string]any{"operators": []string{"in", "not in"}, "value": "array of uppercase two-letter country codes"}, + "platform": map[string]any{"operators": []string{"in", "not in"}, "value": "array of lowercase platforms", "values": []string{"amazon", "android", "ios", "macos", "roku", "tvos", "visionos", "watchos", "web"}}, + "custom_attribute": map[string]any{"operators": []string{"in", "not in"}, "value": "array of strings or integers", "context": "required attribute key"}, + "app_version": map[string]any{"operators": []string{"=", "!=", ">", ">=", "<", "<="}, "value": "semantic version string", "context": "required app ID"}, + "sdk_version": map[string]any{"operators": []string{"=", "!=", ">", ">=", "<", "<="}, "value": "semantic version string", "context": "required SDK flavor, such as ios, android, flutter, or react-native"}, + }, }, } } diff --git a/internal/cli/schema_test.go b/internal/cli/schema_test.go index 5b01f1a3..55e6abcd 100644 --- a/internal/cli/schema_test.go +++ b/internal/cli/schema_test.go @@ -23,6 +23,21 @@ func TestExperimentConfigSchemaExplainsNestedFields(t *testing.T) { } } +func TestTargetingConfigSchemaExplainsConditionsAndRuleTypes(t *testing.T) { + root := NewRootCmd("test") + for _, path := range []string{"targeting create", "targeting update"} { + data, err := json.Marshal(commandSchema(findCommand(t, root, path))["config_fields"]) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"conditions", "app_version", "context", "placement_offerings", "schedule", "position"} { + if !strings.Contains(string(data), want) { + t.Errorf("%s schema missing %q", path, want) + } + } + } +} + func findCommand(t *testing.T, root *cobra.Command, path string) *cobra.Command { t.Helper() cur := root diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index def744e8..592bf39f 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -113,7 +113,7 @@ func newTargetingCreateCmd() *cobra.Command { cmd := &cobra.Command{ Use: "create", Short: "Create a targeting rule", - Long: "Creates an inactive Offering rule by default. Without audience_id or conditions, a legacy rule matches everyone. Use --config for audience_id, conditions, schedule, placements, position, or a checkpoint rule with flow_id and checkpoints. Active or scheduled rules require confirmation.", + Long: "Creates an inactive Offering rule by default. Without audience_id or conditions, a legacy rule matches everyone. Use --config for audience_id, conditions, schedule, placements, position, or a checkpoint rule with flow_id and checkpoints. Run rc schema targeting create for config fields and accepted values. Active or scheduled rules require confirmation.", Example: ` rc targeting create --name "Default paywall" --offering ofrng_default echo '{"rule_type":"legacy","display_name":"US paywall","offering_id":"ofrng_us","conditions":[{"field":"country","operator":"in","value":["US"]}]}' | rc targeting create --config - --no-input echo '{"rule_type":"checkpoint","display_name":"After onboarding","audience_id":"aud_123","flow_id":"wf_123","checkpoints":[{"checkpoint_id":"chkpt_123"}]}' | rc targeting create --config - --no-input`, @@ -283,7 +283,7 @@ func newTargetingUpdateCmd() *cobra.Command { cmd := &cobra.Command{ Use: "update [id]", Short: "Update an Offering targeting rule", - Long: "Partially updates a legacy targeting rule from a JSON object with position, state, display_name, offering_id, audience_id, conditions, schedule, or placements. An active rule or an activation requires confirmation. Checkpoint rule updates are not exposed by this endpoint.", + Long: "Partially updates a legacy targeting rule from a JSON object with position, state, display_name, offering_id, audience_id, conditions, schedule, or placements. Run rc schema targeting update for config fields and accepted values. An active rule or an activation requires confirmation. Checkpoint rule updates are not exposed by this endpoint.", Example: ` echo '{"state":"active","position":1}' | rc targeting update trle_123 --config - --yes --no-input`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { From 56c7ac8e835c87f3fa35a6df6d7385d44dc80e39 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Fri, 25 Sep 2026 14:56:48 -0500 Subject: [PATCH 06/16] test(targeting): exercise approved rule reorder --- internal/cli/targeting_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/internal/cli/targeting_test.go b/internal/cli/targeting_test.go index b8a73f53..e591a6bc 100644 --- a/internal/cli/targeting_test.go +++ b/internal/cli/targeting_test.go @@ -1,6 +1,7 @@ package cli_test import ( + "encoding/json" "io" "net/http" "net/http/httptest" @@ -10,6 +11,37 @@ import ( "testing" ) +func TestTargetingReorderNeedsApprovalAndSendsPosition(t *testing.T) { + var posted map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.Method == http.MethodPost { + if r.URL.Path != "/projects/proj/targeting_rules/trle1" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if err := json.NewDecoder(r.Body).Decode(&posted); err != nil { + t.Error(err) + } + } + _, _ = io.WriteString(w, `{"id":"trle1","rule_type":"legacy","state":"active","display_name":"US paywall","offering_id":"ofrng_us"}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + configPath := filepath.Join(t.TempDir(), "reorder.json") + if err := os.WriteFile(configPath, []byte(`{"position":1}`), 0o600); err != nil { + t.Fatal(err) + } + args := []string{"targeting", "update", "trle1", "--config", configPath, "--project-id", "proj", "--api-key", "sk_test", "--no-input"} + _, _, err := runAgentCmd(t, args...) + if err == nil || !strings.Contains(err.Error(), "--yes") || posted != nil { + t.Fatalf("reorder should require approval: err=%v posted=%v", err, posted) + } + _, _, err = runAgentCmd(t, append(args, "--yes", "--json")...) + if err != nil || posted["position"] != float64(1) { + t.Fatalf("approved reorder failed: err=%v posted=%v", err, posted) + } +} + func TestTargetingActivationRequiresApproval(t *testing.T) { mutations := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From ca76e4a59d97c1acb37ea7476c1596b4b45d0412 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Mon, 28 Sep 2026 11:42:32 -0500 Subject: [PATCH 07/16] fix(targeting): make activation previews readable --- internal/cli/targeting.go | 44 +++++++++++++++++++++------ internal/cli/targeting_format.go | 44 +++++++++++++++++++++++++++ internal/cli/targeting_format_test.go | 28 +++++++++++++++++ internal/cli/targeting_test.go | 24 +++++++++++++++ 4 files changed, 131 insertions(+), 9 deletions(-) create mode 100644 internal/cli/targeting_format_test.go diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index 592bf39f..909eaafe 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -151,7 +151,9 @@ func newTargetingCreateCmd() *cobra.Command { return err } if body.State != "inactive" { - showTargetingCreatePlan(rt, body) + if err := showTargetingCreatePlan(rt, body); err != nil { + return err + } prompt := "Activate targeting rule now?" if body.State == "scheduled" { prompt = "Schedule targeting rule now?" @@ -232,7 +234,7 @@ func gatherTargetingCreateInput(cmd *cobra.Command, rt *Runtime, projectID strin return nil } -func showTargetingCreatePlan(rt *Runtime, body api.TargetingRuleCreate) { +func showTargetingCreatePlan(rt *Runtime, body api.TargetingRuleCreate) error { rt.Out.Title("Targeting rule — " + body.DisplayName) rt.Out.Lead("Apply this rule in priority order when it becomes active.") rt.Out.Field("Type", body.RuleType) @@ -242,7 +244,11 @@ func showTargetingCreatePlan(rt *Runtime, body api.TargetingRuleCreate) { if body.AudienceID != "" { rt.Out.Field("Audience", body.AudienceID) } else if len(body.Conditions) > 0 && string(body.Conditions) != "[]" { - rt.Out.Field("Conditions", string(body.Conditions)) + conditions, err := targetingConfigSummary("conditions", body.Conditions) + if err != nil { + return err + } + rt.Out.Field("Conditions", conditions) } else { rt.Out.Notice("This rule matches everyone. Active rules use the first match.") } @@ -252,17 +258,30 @@ func showTargetingCreatePlan(rt *Runtime, body api.TargetingRuleCreate) { rt.Out.Field("Position", "Append to the end") } if len(body.Placements) > 0 { - rt.Out.Field("Placements", string(body.Placements)) + placements, err := targetingConfigSummary("placements", body.Placements) + if err != nil { + return err + } + rt.Out.Field("Placements", placements) } } else { rt.Out.Field("Flow", body.FlowID) rt.Out.Field("Audience", body.AudienceID) - rt.Out.Field("Checkpoints", string(body.Checkpoints)) + checkpoints, err := targetingConfigSummary("checkpoints", body.Checkpoints) + if err != nil { + return err + } + rt.Out.Field("Checkpoints", checkpoints) } if len(body.Schedule) > 0 { - rt.Out.Field("Schedule", string(body.Schedule)) + schedule, err := targetingConfigSummary("schedule", body.Schedule) + if err != nil { + return err + } + rt.Out.Field("Schedule", schedule) } rt.Out.Plan([]string{"Create the rule and apply it to matching customers"}) + return nil } func validateTargetingCreate(body api.TargetingRuleCreate) error { @@ -336,11 +355,17 @@ func newTargetingUpdateCmd() *cobra.Command { if current.AudienceID != nil { rt.Out.Field("Current audience", *current.AudienceID) } else if len(current.Conditions) > 0 { - rt.Out.Field("Current conditions", compactJSON(current.Conditions)) + conditions, err := targetingConditionsSummary(current.Conditions) + if err != nil { + return err + } + rt.Out.Field("Current conditions", conditions) } else { rt.Out.Field("Current audience", "Everyone") } - rt.Out.Field("Changes", compactJSON(body)) + if err := showTargetingUpdateChanges(rt, body); err != nil { + return err + } resultingAudience, everyone, err := targetingAudienceAfterUpdate(current, body) if err != nil { return err @@ -400,7 +425,8 @@ func targetingAudienceAfterUpdate(current *api.TargetingRule, body api.Targeting return audienceID, false, nil } if len(conditions) > 0 { - return compactJSON(conditions), false, nil + formatted, err := targetingConditionsSummary(conditions) + return formatted, false, err } return "Everyone", true, nil } diff --git a/internal/cli/targeting_format.go b/internal/cli/targeting_format.go index 8c703e11..f2e845fe 100644 --- a/internal/cli/targeting_format.go +++ b/internal/cli/targeting_format.go @@ -3,11 +3,55 @@ package cli import ( "encoding/json" "fmt" + "sort" + "strings" "github.com/revenuecat/cli/internal/api" "github.com/revenuecat/cli/internal/output" ) +func targetingConfigSummary(key string, raw json.RawMessage) (string, error) { + if key == "conditions" { + var conditions []api.ExperimentCondition + if err := json.Unmarshal(raw, &conditions); err != nil { + return "", fmt.Errorf("conditions must be an array: %w", err) + } + if len(conditions) == 0 { + return "Everyone", nil + } + return experimentConditionSummary(conditions), nil + } + var value any + if err := json.Unmarshal(raw, &value); err != nil { + return "", err + } + return experimentChangeValue(value), nil +} + +func targetingConditionsSummary(conditions []any) (string, error) { + raw, err := json.Marshal(conditions) + if err != nil { + return "", err + } + return targetingConfigSummary("conditions", raw) +} + +func showTargetingUpdateChanges(rt *Runtime, changes api.TargetingRuleUpdate) error { + keys := make([]string, 0, len(changes)) + for key := range changes { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + value, err := targetingConfigSummary(key, changes[key]) + if err != nil { + return err + } + rt.Out.Field("Change "+strings.ReplaceAll(key, "_", " "), value) + } + return nil +} + type targetingDisplay struct { Conditions []api.ExperimentCondition `json:"conditions"` Schedule *struct { diff --git a/internal/cli/targeting_format_test.go b/internal/cli/targeting_format_test.go new file mode 100644 index 00000000..1f92c663 --- /dev/null +++ b/internal/cli/targeting_format_test.go @@ -0,0 +1,28 @@ +package cli + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestTargetingConfigSummaryFormatsActivationPreview(t *testing.T) { + conditions := json.RawMessage(`[{"context":null,"field":"country","operator":"in","value":["US"]}]`) + got, err := targetingConfigSummary("conditions", conditions) + if err != nil || got != "country in US" { + t.Fatalf("conditions: got %q, err %v", got, err) + } + var responseConditions []any + if err := json.Unmarshal(conditions, &responseConditions); err != nil { + t.Fatal(err) + } + got, err = targetingConditionsSummary(responseConditions) + if err != nil || got != "country in US" { + t.Fatalf("response conditions: got %q, err %v", got, err) + } + placements := json.RawMessage(`{"fallback_offering_id":"ofrng_default","placement_offerings":[{"placement_identifier":"onboarding","offering_id":"ofrng_us"}]}`) + got, err = targetingConfigSummary("placements", placements) + if err != nil || !strings.Contains(got, "onboarding") || !strings.Contains(got, "ofrng_us") || strings.ContainsAny(got, "{}[]\"") { + t.Fatalf("placements: got %q, err %v", got, err) + } +} diff --git a/internal/cli/targeting_test.go b/internal/cli/targeting_test.go index e591a6bc..6cc94f92 100644 --- a/internal/cli/targeting_test.go +++ b/internal/cli/targeting_test.go @@ -69,6 +69,30 @@ func TestTargetingActivationRequiresApproval(t *testing.T) { } } +func TestTargetingActivationPreviewUsesReadableConditions(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + state := "inactive" + if r.Method == http.MethodPost { + state = "active" + } + _, _ = io.WriteString(w, `{"object":"targeting_rule","id":"trle1","rule_type":"legacy","state":"`+state+`","display_name":"US paywall","offering_id":"ofrng_us","conditions":[{"context":null,"field":"country","operator":"in","value":["US"]}]}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + configPath := filepath.Join(t.TempDir(), "activate.json") + if err := os.WriteFile(configPath, []byte(`{"state":"active"}`), 0o600); err != nil { + t.Fatal(err) + } + _, stderr, err := runAgentCmd(t, "targeting", "update", "trle1", "--config", configPath, "--project-id", "proj", "--api-key", "sk_test", "--yes", "--no-input") + if err != nil { + t.Fatal(err) + } + if strings.Count(stderr, "country in US") < 2 || !strings.Contains(stderr, "Change state") || !strings.Contains(stderr, "active") || strings.Contains(stderr, `{"`) || strings.Contains(stderr, `[{`) { + t.Fatalf("activation preview should be readable: %s", stderr) + } +} + func TestTargetingUpdateActiveRuleRequiresApproval(t *testing.T) { mutations := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From d0c02d68f8f8ce47326c40b5e65a2c9e57c1f21d Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Mon, 28 Sep 2026 11:46:15 -0500 Subject: [PATCH 08/16] fix(targeting): validate audience transitions and document nested fields --- internal/cli/config_schema.go | 4 ++-- internal/cli/schema_test.go | 2 +- internal/cli/targeting.go | 11 +++++++---- internal/cli/targeting_format_test.go | 26 ++++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/internal/cli/config_schema.go b/internal/cli/config_schema.go index 708cd336..f17c0f27 100644 --- a/internal/cli/config_schema.go +++ b/internal/cli/config_schema.go @@ -17,11 +17,11 @@ func configFieldsFor(cmd *cobra.Command) map[string]any { } func targetingConfigFields(create bool) map[string]any { - schedule := map[string]any{"type": "object", "description": "UTC timestamps ending in Z", "properties": map[string]any{ + schedule := map[string]any{"type": "object", "description": "UTC timestamps ending in Z; legacy rules require start_date when schedule is provided", "required_for_legacy": []string{"start_date"}, "properties": map[string]any{ "start_date": configField("string", "Start time, for example 2026-05-25T10:00:00Z"), "end_date": configField("string", "Optional end time in the same format"), }} - placements := map[string]any{"type": "object", "description": "Legacy rules only", "properties": map[string]any{ + placements := map[string]any{"type": "object", "description": "Legacy rules only; both fields are required when placements is provided", "required": []string{"fallback_offering_id", "placement_offerings"}, "properties": map[string]any{ "fallback_offering_id": configField("string", "Fallback Offering ID"), "placement_offerings": map[string]any{"type": "array", "items": map[string]any{ "type": "object", "required": []string{"placement_identifier", "offering_id"}, "properties": map[string]any{ diff --git a/internal/cli/schema_test.go b/internal/cli/schema_test.go index 55e6abcd..b19aa5bc 100644 --- a/internal/cli/schema_test.go +++ b/internal/cli/schema_test.go @@ -30,7 +30,7 @@ func TestTargetingConfigSchemaExplainsConditionsAndRuleTypes(t *testing.T) { if err != nil { t.Fatal(err) } - for _, want := range []string{"conditions", "app_version", "context", "placement_offerings", "schedule", "position"} { + for _, want := range []string{"conditions", "app_version", "context", "placement_offerings", "fallback_offering_id", "required_for_legacy", "start_date", "schedule", "position"} { if !strings.Contains(string(data), want) { t.Errorf("%s schema missing %q", path, want) } diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index 909eaafe..3e0ef5d3 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -347,6 +347,10 @@ func newTargetingUpdateCmd() *cobra.Command { return fmt.Errorf("state must be active or inactive") } } + resultingAudience, everyone, err := targetingAudienceAfterUpdate(current, body) + if err != nil { + return err + } if current.State == "active" || newState == "active" { rt.Out.Title("Targeting rule — " + current.DisplayName) rt.Out.Lead("Change the rule used to choose an Offering for matching customers.") @@ -366,10 +370,6 @@ func newTargetingUpdateCmd() *cobra.Command { if err := showTargetingUpdateChanges(rt, body); err != nil { return err } - resultingAudience, everyone, err := targetingAudienceAfterUpdate(current, body) - if err != nil { - return err - } rt.Out.Field("Resulting audience", resultingAudience) if everyone { rt.Out.Notice("Resulting rule matches everyone. Active rules use the first match.") @@ -421,6 +421,9 @@ func targetingAudienceAfterUpdate(current *api.TargetingRule, body api.Targeting return "", false, fmt.Errorf("conditions must be an array") } } + if audienceID != "" && len(conditions) > 0 { + return "", false, fmt.Errorf("audience_id and conditions cannot both be set; clear the other field in the same update") + } if audienceID != "" { return audienceID, false, nil } diff --git a/internal/cli/targeting_format_test.go b/internal/cli/targeting_format_test.go index 1f92c663..49db223b 100644 --- a/internal/cli/targeting_format_test.go +++ b/internal/cli/targeting_format_test.go @@ -4,6 +4,8 @@ import ( "encoding/json" "strings" "testing" + + "github.com/revenuecat/cli/internal/api" ) func TestTargetingConfigSummaryFormatsActivationPreview(t *testing.T) { @@ -26,3 +28,27 @@ func TestTargetingConfigSummaryFormatsActivationPreview(t *testing.T) { t.Fatalf("placements: got %q, err %v", got, err) } } + +func TestTargetingAudienceAfterUpdateRequiresClearingOtherField(t *testing.T) { + audience := "aud_us" + conditions := []any{map[string]any{"field": "country", "operator": "in", "value": []any{"US"}}} + for _, tc := range []struct { + name string + current *api.TargetingRule + body api.TargetingRuleUpdate + want string + fail bool + }{ + {"conditions with existing audience", &api.TargetingRule{AudienceID: &audience}, api.TargetingRuleUpdate{"conditions": json.RawMessage(`[ {"field":"country","operator":"in","value":["US"]} ]`)}, "", true}, + {"audience with existing conditions", &api.TargetingRule{Conditions: conditions}, api.TargetingRuleUpdate{"audience_id": json.RawMessage(`"aud_us"`)}, "", true}, + {"clear audience before conditions", &api.TargetingRule{AudienceID: &audience}, api.TargetingRuleUpdate{"audience_id": json.RawMessage(`null`), "conditions": json.RawMessage(`[ {"field":"country","operator":"in","value":["US"]} ]`)}, "country in US", false}, + {"clear conditions before audience", &api.TargetingRule{Conditions: conditions}, api.TargetingRuleUpdate{"audience_id": json.RawMessage(`"aud_us"`), "conditions": json.RawMessage(`[]`)}, "aud_us", false}, + } { + t.Run(tc.name, func(t *testing.T) { + got, _, err := targetingAudienceAfterUpdate(tc.current, tc.body) + if (err != nil) != tc.fail || (!tc.fail && got != tc.want) { + t.Fatalf("audience=%q err=%v, want %q fail=%v", got, err, tc.want, tc.fail) + } + }) + } +} From a36968adeaa1ed2be939f938e1368143652e4b99 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Mon, 28 Sep 2026 12:56:14 -0500 Subject: [PATCH 09/16] docs(targeting): explain audience transitions in command schema --- internal/cli/config_schema.go | 2 +- internal/cli/schema_test.go | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/cli/config_schema.go b/internal/cli/config_schema.go index f17c0f27..902da012 100644 --- a/internal/cli/config_schema.go +++ b/internal/cli/config_schema.go @@ -35,7 +35,7 @@ func targetingConfigFields(create bool) map[string]any { "state": configEnum("active or inactive for legacy; checkpoint also supports scheduled", "active", "inactive", "scheduled"), "display_name": configField("string", "Rule name"), "offering_id": configField("string", "Offering ID served by a legacy rule"), - "audience_id": configField("string", "Audience ID; mutually exclusive with conditions"), + "audience_id": map[string]any{"type": "string", "nullable": true, "description": "Audience ID; mutually exclusive with conditions. Set null when switching to conditions; set conditions to [] when switching to an audience."}, "conditions": targetingConditionsSchema(), "schedule": schedule, "placements": placements, diff --git a/internal/cli/schema_test.go b/internal/cli/schema_test.go index b19aa5bc..7313eb3c 100644 --- a/internal/cli/schema_test.go +++ b/internal/cli/schema_test.go @@ -26,7 +26,8 @@ func TestExperimentConfigSchemaExplainsNestedFields(t *testing.T) { func TestTargetingConfigSchemaExplainsConditionsAndRuleTypes(t *testing.T) { root := NewRootCmd("test") for _, path := range []string{"targeting create", "targeting update"} { - data, err := json.Marshal(commandSchema(findCommand(t, root, path))["config_fields"]) + config := commandSchema(findCommand(t, root, path))["config_fields"].(map[string]any) + data, err := json.Marshal(config) if err != nil { t.Fatal(err) } @@ -35,6 +36,10 @@ func TestTargetingConfigSchemaExplainsConditionsAndRuleTypes(t *testing.T) { t.Errorf("%s schema missing %q", path, want) } } + audience := config["properties"].(map[string]any)["audience_id"].(map[string]any) + if audience["nullable"] != true || !strings.Contains(audience["description"].(string), "conditions to []") { + t.Errorf("%s schema must explain how to switch between audiences and conditions", path) + } } } From cd533b7aba7f1b7dba3aef7d3f1beba7edc8ba2c Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Mon, 28 Sep 2026 15:03:52 -0500 Subject: [PATCH 10/16] fix(targeting): require force to delete live rules --- docs/command-surface.md | 2 +- internal/cli/targeting.go | 19 ++++++++++++- internal/cli/targeting_test.go | 50 ++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/docs/command-surface.md b/docs/command-surface.md index 9bba4274..1c298aae 100644 --- a/docs/command-surface.md +++ b/docs/command-surface.md @@ -208,7 +208,7 @@ rc targeting list [--state ] rc targeting show # human detail view shows audience, placements, schedule, and checkpoints rc targeting create (--name --offering [--state inactive|active] | --config ) # initially inactive rc targeting update --config # partial update -rc targeting delete # confirmation/--yes +rc targeting delete [--force] # active or scheduled rules require --force; confirmation/--yes # Rico (AI assistant) rc rico [message] # streaming chat window in a TTY (--plain for a line loop) diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index 3e0ef5d3..ed231e2b 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -435,9 +435,11 @@ func targetingAudienceAfterUpdate(current *api.TargetingRule, body api.Targeting } func newTargetingDeleteCmd() *cobra.Command { - return &cobra.Command{ + var force bool + cmd := &cobra.Command{ Use: "delete [id]", Short: "Delete a targeting rule", + Long: "Permanently deletes a targeting rule. Active or scheduled rules require --force in addition to confirmation or --yes because deleting them may change which Offering customers see.", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { rt := RuntimeFrom(cmd.Context()) @@ -455,6 +457,19 @@ func newTargetingDeleteCmd() *cobra.Command { if err != nil { return err } + current, err := client.TargetingRules.Get(cmd.Context(), projectID, id) + if err != nil { + return err + } + if current.State != "inactive" && !force { + return WithHint( + fmt.Errorf("targeting rule %s is %s and may be serving customers; pass --force to delete it", id, current.State), + "Deactivate it first, or pass --force after confirming this rule should be deleted.", + ) + } + if current.State != "inactive" { + rt.Out.Warn("Deleting this rule may change which Offering customers see.") + } if err := confirmOrAbort(rt, "Delete targeting rule "+id+"?"); err != nil { return err } @@ -465,4 +480,6 @@ func newTargetingDeleteCmd() *cobra.Command { return rt.Out.Render(map[string]any{"id": id, "deleted": true}) }, } + cmd.Flags().BoolVar(&force, "force", false, "delete an active or scheduled targeting rule") + return cmd } diff --git a/internal/cli/targeting_test.go b/internal/cli/targeting_test.go index 6cc94f92..0d4849aa 100644 --- a/internal/cli/targeting_test.go +++ b/internal/cli/targeting_test.go @@ -42,6 +42,56 @@ func TestTargetingReorderNeedsApprovalAndSendsPosition(t *testing.T) { } } +func TestTargetingDeleteRequiresForceForLiveRule(t *testing.T) { + for _, state := range []string{"active", "scheduled"} { + t.Run(state, func(t *testing.T) { + deletes := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/projects/proj/targeting_rules/trle1" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Method == http.MethodDelete { + deletes++ + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"id":"trle1","rule_type":"legacy","state":"`+state+`","display_name":"US paywall","offering_id":"ofrng_us"}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + args := []string{"targeting", "delete", "trle1", "--project-id", "proj", "--api-key", "sk_test", "--no-input"} + _, _, err := runAgentCmd(t, append(args, "--yes")...) + if err == nil || !strings.Contains(err.Error(), "--force") || deletes != 0 { + t.Fatalf("expected force guard: err=%v deletes=%d", err, deletes) + } + _, _, err = runAgentCmd(t, append(args, "--force")...) + if err == nil || !strings.Contains(err.Error(), "--yes") || deletes != 0 { + t.Fatalf("expected confirmation after force: err=%v deletes=%d", err, deletes) + } + _, _, err = runAgentCmd(t, append(args, "--force", "--yes")...) + if err != nil || deletes != 1 { + t.Fatalf("approved force delete failed: err=%v deletes=%d", err, deletes) + } + }) + } +} + +func TestTargetingDeleteInactiveNeedsOnlyConfirmation(t *testing.T) { + deletes := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deletes++ + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"id":"trle1","rule_type":"legacy","state":"inactive","display_name":"US paywall"}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + _, _, err := runAgentCmd(t, "targeting", "delete", "trle1", "--project-id", "proj", "--api-key", "sk_test", "--yes", "--no-input") + if err != nil || deletes != 1 { + t.Fatalf("inactive delete failed: err=%v deletes=%d", err, deletes) + } +} + func TestTargetingActivationRequiresApproval(t *testing.T) { mutations := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From 86c6fdceec233d3d321d33918f7d61a93a0124c3 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Tue, 29 Sep 2026 09:44:24 -0500 Subject: [PATCH 11/16] docs(targeting): explain scheduled offering rules --- docs/command-surface.md | 5 +++++ internal/cli/config_schema.go | 4 ++-- internal/cli/targeting.go | 15 +++++++++++---- internal/cli/targeting_test.go | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/docs/command-surface.md b/docs/command-surface.md index 1c298aae..8a676445 100644 --- a/docs/command-surface.md +++ b/docs/command-surface.md @@ -210,6 +210,11 @@ rc targeting create (--name --offering [--state inactive|ac rc targeting update --config # partial update rc targeting delete [--force] # active or scheduled rules require --force; confirmation/--yes +Legacy Offering rules use `state: "active"` with a future `schedule.start_date` to +start serving later. The API reports their state as `active` before that date, +but targeting skips them until the scheduled window. The `scheduled` state is +only for checkpoint rules. + # Rico (AI assistant) rc rico [message] # streaming chat window in a TTY (--plain for a line loop) rc rico --continue # continue the most recent conversation diff --git a/internal/cli/config_schema.go b/internal/cli/config_schema.go index 902da012..565a898e 100644 --- a/internal/cli/config_schema.go +++ b/internal/cli/config_schema.go @@ -17,7 +17,7 @@ func configFieldsFor(cmd *cobra.Command) map[string]any { } func targetingConfigFields(create bool) map[string]any { - schedule := map[string]any{"type": "object", "description": "UTC timestamps ending in Z; legacy rules require start_date when schedule is provided", "required_for_legacy": []string{"start_date"}, "properties": map[string]any{ + schedule := map[string]any{"type": "object", "description": "UTC timestamps ending in Z. Legacy Offering rules require start_date; set state to active with a future start_date to serve later.", "required_for_legacy": []string{"start_date"}, "properties": map[string]any{ "start_date": configField("string", "Start time, for example 2026-05-25T10:00:00Z"), "end_date": configField("string", "Optional end time in the same format"), }} @@ -32,7 +32,7 @@ func targetingConfigFields(create bool) map[string]any { }} fields := map[string]any{ "position": map[string]any{"type": "integer", "minimum": 1, "description": "One-based priority among rules in the same state; legacy rules only"}, - "state": configEnum("active or inactive for legacy; checkpoint also supports scheduled", "active", "inactive", "scheduled"), + "state": configEnum("Legacy Offering rules use active or inactive; use active with a future schedule.start_date to serve later. The scheduled state is checkpoint-only.", "active", "inactive", "scheduled"), "display_name": configField("string", "Rule name"), "offering_id": configField("string", "Offering ID served by a legacy rule"), "audience_id": map[string]any{"type": "string", "nullable": true, "description": "Audience ID; mutually exclusive with conditions. Set null when switching to conditions; set conditions to [] when switching to an audience."}, diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index ed231e2b..626d1e19 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -59,7 +59,7 @@ func newTargetingListCmd() *cobra.Command { return nil }, } - cmd.Flags().StringVar(&state, "state", "", "filter by active, scheduled, or inactive") + cmd.Flags().StringVar(&state, "state", "", "filter by active, scheduled (checkpoint only), or inactive; future-scheduled Offering rules are active") cmd.Flags().IntVar(&limit, "limit", 20, "maximum rules to return (1–100)") cmd.Flags().StringVar(&cursor, "cursor", "", "item ID to start after (pagination)") return cmd @@ -113,9 +113,10 @@ func newTargetingCreateCmd() *cobra.Command { cmd := &cobra.Command{ Use: "create", Short: "Create a targeting rule", - Long: "Creates an inactive Offering rule by default. Without audience_id or conditions, a legacy rule matches everyone. Use --config for audience_id, conditions, schedule, placements, position, or a checkpoint rule with flow_id and checkpoints. Run rc schema targeting create for config fields and accepted values. Active or scheduled rules require confirmation.", + Long: "Creates an inactive Offering rule by default. Without audience_id or conditions, a legacy rule matches everyone. Use --config for audience_id, conditions, schedule, placements, position, or a checkpoint rule with flow_id and checkpoints. To schedule an Offering rule, set state to active and schedule.start_date to a future UTC time; it will not match customers before then. The scheduled state is only for checkpoint rules. Run rc schema targeting create for config fields and accepted values. Active or scheduled rules require confirmation.", Example: ` rc targeting create --name "Default paywall" --offering ofrng_default echo '{"rule_type":"legacy","display_name":"US paywall","offering_id":"ofrng_us","conditions":[{"field":"country","operator":"in","value":["US"]}]}' | rc targeting create --config - --no-input + echo '{"display_name":"Holiday paywall","offering_id":"ofrng_holiday","state":"active","schedule":{"start_date":"2030-12-01T00:00:00Z","end_date":"2030-12-31T23:59:59Z"}}' | rc targeting create --config - --yes --no-input echo '{"rule_type":"checkpoint","display_name":"After onboarding","audience_id":"aud_123","flow_id":"wf_123","checkpoints":[{"checkpoint_id":"chkpt_123"}]}' | rc targeting create --config - --no-input`, RunE: func(cmd *cobra.Command, _ []string) error { rt := RuntimeFrom(cmd.Context()) @@ -157,6 +158,8 @@ func newTargetingCreateCmd() *cobra.Command { prompt := "Activate targeting rule now?" if body.State == "scheduled" { prompt = "Schedule targeting rule now?" + } else if len(body.Schedule) > 0 { + prompt = "Create targeting rule with schedule now?" } if err := confirmOrAbort(rt, prompt); err != nil { return err @@ -280,7 +283,11 @@ func showTargetingCreatePlan(rt *Runtime, body api.TargetingRuleCreate) error { } rt.Out.Field("Schedule", schedule) } - rt.Out.Plan([]string{"Create the rule and apply it to matching customers"}) + step := "Create the rule and apply it to matching customers" + if len(body.Schedule) > 0 { + step = "Create the rule and match customers during its scheduled window" + } + rt.Out.Plan([]string{step}) return nil } @@ -302,7 +309,7 @@ func newTargetingUpdateCmd() *cobra.Command { cmd := &cobra.Command{ Use: "update [id]", Short: "Update an Offering targeting rule", - Long: "Partially updates a legacy targeting rule from a JSON object with position, state, display_name, offering_id, audience_id, conditions, schedule, or placements. Run rc schema targeting update for config fields and accepted values. An active rule or an activation requires confirmation. Checkpoint rule updates are not exposed by this endpoint.", + Long: "Partially updates a legacy targeting rule from a JSON object with position, state, display_name, offering_id, audience_id, conditions, schedule, or placements. To schedule an Offering rule, set state to active and schedule.start_date to a future UTC time; it will not match customers before then. Run rc schema targeting update for config fields and accepted values. An active rule or an activation requires confirmation. Checkpoint rule updates are not exposed by this endpoint.", Example: ` echo '{"state":"active","position":1}' | rc targeting update trle_123 --config - --yes --no-input`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/internal/cli/targeting_test.go b/internal/cli/targeting_test.go index 0d4849aa..774dd474 100644 --- a/internal/cli/targeting_test.go +++ b/internal/cli/targeting_test.go @@ -119,6 +119,38 @@ func TestTargetingActivationRequiresApproval(t *testing.T) { } } +func TestTargetingCreateScheduledOfferingRule(t *testing.T) { + var posted map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + if err := json.NewDecoder(r.Body).Decode(&posted); err != nil { + t.Error(err) + } + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"object":"targeting_rule","id":"trle1","rule_type":"legacy","state":"active","display_name":"Holiday paywall","offering_id":"ofrng_holiday","schedule":{"start_date":"2030-12-01T00:00:00Z"}}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + configPath := filepath.Join(t.TempDir(), "scheduled.json") + if err := os.WriteFile(configPath, []byte(`{"display_name":"Holiday paywall","offering_id":"ofrng_holiday","state":"active","schedule":{"start_date":"2030-12-01T00:00:00Z"}}`), 0o600); err != nil { + t.Fatal(err) + } + args := []string{"targeting", "create", "--config", configPath, "--project-id", "proj", "--api-key", "sk_test", "--no-input"} + _, stderr, err := runAgentCmd(t, args...) + if err == nil || !strings.Contains(err.Error(), "--yes") || posted != nil || !strings.Contains(stderr, "scheduled window") { + t.Fatalf("scheduled create should show its plan and require approval: err=%v posted=%v stderr=%s", err, posted, stderr) + } + _, _, err = runAgentCmd(t, append(args, "--yes", "--json")...) + if err != nil { + t.Fatal(err) + } + schedule, ok := posted["schedule"].(map[string]any) + if !ok || posted["state"] != "active" || schedule["start_date"] != "2030-12-01T00:00:00Z" { + t.Fatalf("scheduled Offering rule was not sent to the API: %v", posted) + } +} + func TestTargetingActivationPreviewUsesReadableConditions(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") From 18c26be2db1d50db577ba77598bba159842713fd Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Tue, 29 Sep 2026 09:49:25 -0500 Subject: [PATCH 12/16] test(targeting): snapshot scheduled rule preview --- docs/previews/targeting-create-scheduled.svg | 48 +++++++++++++++++++ internal/cli/snapshot_test.go | 14 +++++- internal/cli/targeting.go | 12 ++++- internal/cli/targeting_format.go | 20 ++++++++ internal/cli/targeting_format_test.go | 5 ++ .../cli/testdata/scheduled-targeting.json | 1 + .../targeting-create-scheduled.golden | 33 +++++++++++++ 7 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 docs/previews/targeting-create-scheduled.svg create mode 100644 internal/cli/testdata/scheduled-targeting.json create mode 100644 internal/cli/testdata/snapshots/targeting-create-scheduled.golden diff --git a/docs/previews/targeting-create-scheduled.svg b/docs/previews/targeting-create-scheduled.svg new file mode 100644 index 00000000..2914b5c6 --- /dev/null +++ b/docs/previews/targeting-create-scheduled.svg @@ -0,0 +1,48 @@ + + + + + +$ rc targeting create --config testdata/scheduled-targeting.json --yes --no-input --project-id proj_snap --api-key sk_snap + +▍ Targeting rule — Holiday paywall +  Match customers during the scheduled window, in priority order. + +  Type                        legacy +  State                       active +  Offering                    ofrng_holiday + +▐ During its scheduled window, this rule matches everyone. Active rules use the first match. + +  Position                    Append to the end +  Schedule                    start: 2030-12-01T00:00:00Z · end: 2030-12-31T23:59:59Z + +▍ Plan +· 1. Create the rule and match customers during its scheduled window +✓ Created targeting rule trle_holiday +  Use --json for the complete API response. +▍ Holiday paywall +  trle_holiday · legacy · active + +Serves +  Offering:  ofrng_holiday + +Audience +  Audience:  All eligible customers + +Placements +  No placement overrides + +Schedule +  Start:  2030-12-01T00:00:00Z +  End:    2030-12-31T23:59:59Z + + + diff --git a/internal/cli/snapshot_test.go b/internal/cli/snapshot_test.go index c62c674e..1c33c111 100644 --- a/internal/cli/snapshot_test.go +++ b/internal/cli/snapshot_test.go @@ -13,6 +13,7 @@ package cli_test // internal/output/brand.go and are reviewed there. import ( + "encoding/json" "fmt" "io" "net/http" @@ -32,7 +33,17 @@ func snapshotServer(t *testing.T) *httptest.Server { io.WriteString(w, `{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"US annual paywall","offering_id":"ofrng_us","conditions":[{"field":"platform","operator":"in","value":["ios"]}],"placements":{"fallback_offering_id":"ofrng_default","placement_offerings":[{"placement_identifier":"onboarding","offering_id":"ofrng_us"}]},"schedule":{"start_date":"2026-09-25T12:00:00Z","end_date":null}}`) case strings.HasSuffix(r.URL.Path, "/targeting_rules"): if r.Method == http.MethodPost { - io.WriteString(w, `{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"Default paywall","offering_id":"ofrng_default"}`) + var body struct { + DisplayName string `json:"display_name"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Error(err) + } + if body.DisplayName == "Holiday paywall" { + io.WriteString(w, `{"object":"targeting_rule","id":"trle_holiday","rule_type":"legacy","state":"active","display_name":"Holiday paywall","offering_id":"ofrng_holiday","schedule":{"start_date":"2030-12-01T00:00:00Z","end_date":"2030-12-31T23:59:59Z"}}`) + } else { + io.WriteString(w, `{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"Default paywall","offering_id":"ofrng_default"}`) + } } else { io.WriteString(w, `{"object":"list","items":[{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"US annual paywall","offering_id":"ofrng_us"}],"next_page":null,"url":"/projects/proj_snap/targeting_rules"}`) } @@ -78,6 +89,7 @@ func TestOutputSnapshots(t *testing.T) { {"targeting-list", []string{"targeting", "list", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"targeting-show", []string{"targeting", "show", "trle_snap", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"targeting-create-active", []string{"targeting", "create", "--name", "Default paywall", "--offering", "ofrng_default", "--state", "active", "--yes", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, + {"targeting-create-scheduled", []string{"targeting", "create", "--config", "testdata/scheduled-targeting.json", "--yes", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"apps-list", []string{"apps", "list", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"apps-list-all-projects", []string{"apps", "list", "--all-projects", "--bundle-id", "com.example.moodly", "--no-input", "--api-key", "sk_snap"}}, {"error-not-found", []string{"offerings", "show", "ofrng_missing", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index 626d1e19..11da9ff5 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -239,7 +239,11 @@ func gatherTargetingCreateInput(cmd *cobra.Command, rt *Runtime, projectID strin func showTargetingCreatePlan(rt *Runtime, body api.TargetingRuleCreate) error { rt.Out.Title("Targeting rule — " + body.DisplayName) - rt.Out.Lead("Apply this rule in priority order when it becomes active.") + if len(body.Schedule) > 0 { + rt.Out.Lead("Match customers during the scheduled window, in priority order.") + } else { + rt.Out.Lead("Apply this rule in priority order when it becomes active.") + } rt.Out.Field("Type", body.RuleType) rt.Out.Field("State", body.State) if body.RuleType == "legacy" { @@ -253,7 +257,11 @@ func showTargetingCreatePlan(rt *Runtime, body api.TargetingRuleCreate) error { } rt.Out.Field("Conditions", conditions) } else { - rt.Out.Notice("This rule matches everyone. Active rules use the first match.") + if len(body.Schedule) > 0 { + rt.Out.Notice("During its scheduled window, this rule matches everyone. Active rules use the first match.") + } else { + rt.Out.Notice("This rule matches everyone. Active rules use the first match.") + } } if body.Position != nil { rt.Out.Field("Position", fmt.Sprint(*body.Position)) diff --git a/internal/cli/targeting_format.go b/internal/cli/targeting_format.go index f2e845fe..a3f5876a 100644 --- a/internal/cli/targeting_format.go +++ b/internal/cli/targeting_format.go @@ -21,6 +21,26 @@ func targetingConfigSummary(key string, raw json.RawMessage) (string, error) { } return experimentConditionSummary(conditions), nil } + if key == "schedule" { + if string(raw) == "null" { + return "None", nil + } + var schedule struct { + StartDate *string `json:"start_date"` + EndDate *string `json:"end_date"` + } + if err := json.Unmarshal(raw, &schedule); err != nil { + return "", err + } + parts := make([]string, 0, 2) + if schedule.StartDate != nil { + parts = append(parts, "start: "+*schedule.StartDate) + } + if schedule.EndDate != nil { + parts = append(parts, "end: "+*schedule.EndDate) + } + return strings.Join(parts, " · "), nil + } var value any if err := json.Unmarshal(raw, &value); err != nil { return "", err diff --git a/internal/cli/targeting_format_test.go b/internal/cli/targeting_format_test.go index 49db223b..59a42b8e 100644 --- a/internal/cli/targeting_format_test.go +++ b/internal/cli/targeting_format_test.go @@ -27,6 +27,11 @@ func TestTargetingConfigSummaryFormatsActivationPreview(t *testing.T) { if err != nil || !strings.Contains(got, "onboarding") || !strings.Contains(got, "ofrng_us") || strings.ContainsAny(got, "{}[]\"") { t.Fatalf("placements: got %q, err %v", got, err) } + schedule := json.RawMessage(`{"start_date":"2030-12-01T00:00:00Z","end_date":"2030-12-31T23:59:59Z"}`) + got, err = targetingConfigSummary("schedule", schedule) + if err != nil || got != "start: 2030-12-01T00:00:00Z · end: 2030-12-31T23:59:59Z" { + t.Fatalf("schedule: got %q, err %v", got, err) + } } func TestTargetingAudienceAfterUpdateRequiresClearingOtherField(t *testing.T) { diff --git a/internal/cli/testdata/scheduled-targeting.json b/internal/cli/testdata/scheduled-targeting.json new file mode 100644 index 00000000..12ec6aad --- /dev/null +++ b/internal/cli/testdata/scheduled-targeting.json @@ -0,0 +1 @@ +{"display_name":"Holiday paywall","offering_id":"ofrng_holiday","state":"active","schedule":{"start_date":"2030-12-01T00:00:00Z","end_date":"2030-12-31T23:59:59Z"}} diff --git a/internal/cli/testdata/snapshots/targeting-create-scheduled.golden b/internal/cli/testdata/snapshots/targeting-create-scheduled.golden new file mode 100644 index 00000000..46b5bd9a --- /dev/null +++ b/internal/cli/testdata/snapshots/targeting-create-scheduled.golden @@ -0,0 +1,33 @@ +$ rc targeting create --config testdata/scheduled-targeting.json --yes --no-input --project-id proj_snap --api-key sk_snap + +▍ Targeting rule — Holiday paywall + Match customers during the scheduled window, in priority order. + + Type legacy + State active + Offering ofrng_holiday + +▐ During its scheduled window, this rule matches everyone. Active rules use the first match. + + Position Append to the end + Schedule start: 2030-12-01T00:00:00Z · end: 2030-12-31T23:59:59Z + +▍ Plan +· 1. Create the rule and match customers during its scheduled window +✓ Created targeting rule trle_holiday + Use --json for the complete API response. +▍ Holiday paywall + trle_holiday · legacy · active + +Serves + Offering: ofrng_holiday + +Audience + Audience: All eligible customers + +Placements + No placement overrides + +Schedule + Start: 2030-12-01T00:00:00Z + End: 2030-12-31T23:59:59Z From d02f7175ae0aa7d841106d88a4c9e922054d4f77 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Tue, 6 Oct 2026 16:40:16 -0500 Subject: [PATCH 13/16] fix(targeting): expose ordering contract and verify scheduled updates --- docs/previews/targeting-list.svg | 5 +- internal/cli/config_schema.go | 4 +- internal/cli/targeting.go | 15 ++-- internal/cli/targeting_test.go | 68 +++++++++++++++++++ .../testdata/snapshots/targeting-list.golden | 1 + 5 files changed, 85 insertions(+), 8 deletions(-) diff --git a/docs/previews/targeting-list.svg b/docs/previews/targeting-list.svg index 4deb9abf..cf56a3e0 100644 --- a/docs/previews/targeting-list.svg +++ b/docs/previews/targeting-list.svg @@ -1,6 +1,6 @@ - + + $ rc targeting list --no-input --project-id proj_snap --api-key sk_snap ID         NAME               TYPE    STATE   SERVES   trle_snap  US annual paywall  legacy  active  ofrng_us +· Active Offering rules are listed in evaluation order; the first match wins. diff --git a/internal/cli/config_schema.go b/internal/cli/config_schema.go index 9cb81485..d90d20eb 100644 --- a/internal/cli/config_schema.go +++ b/internal/cli/config_schema.go @@ -31,7 +31,7 @@ func targetingConfigFields(create bool) map[string]any { }}, }} fields := map[string]any{ - "position": map[string]any{"type": "integer", "minimum": 1, "description": "One-based priority among rules in the same state; legacy rules only"}, + "position": map[string]any{"type": "integer", "minimum": 1, "description": "One-based priority among rules in the same state; legacy rules only. List preserves evaluation order, but the API does not return absolute positions."}, "state": configEnum("Legacy Offering rules use active or inactive; use active with a future schedule.start_date to serve later. The scheduled state is checkpoint-only.", "active", "inactive", "scheduled"), "display_name": configField("string", "Rule name"), "offering_id": configField("string", "Offering ID served by a legacy rule"), @@ -52,6 +52,8 @@ func targetingConfigFields(create bool) map[string]any { }} return map[string]any{"type": "object", "properties": fields, "description": "Legacy: display_name and offering_id required. Checkpoint: rule_type, display_name, audience_id, flow_id, checkpoints required."} } + schedule["type"] = []string{"object", "null"} + schedule["description"] = "Omit to keep the current schedule. Set null to remove it, or provide UTC start_date and optional end_date timestamps. Use state active with a future start_date to serve later." return map[string]any{"type": "object", "properties": fields, "description": "Partial update of a legacy rule. Checkpoint updates are unavailable."} } diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index 11da9ff5..99d3b14d 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -29,6 +29,7 @@ func newTargetingListCmd() *cobra.Command { cmd := &cobra.Command{ Use: "list", Short: "List targeting rules", + Long: "Returns one page in API order. Active Offering rules are listed in evaluation order; the first match wins. The API does not return absolute rule positions. Use --state active and --cursor to inspect subsequent pages before reordering.", Example: " rc targeting list --state active\n rc targeting list --json", RunE: func(cmd *cobra.Command, _ []string) error { rt := RuntimeFrom(cmd.Context()) @@ -55,6 +56,9 @@ func newTargetingListCmd() *cobra.Command { if err := rt.Out.RenderTable(output.Table{Columns: []string{"ID", "NAME", "TYPE", "STATE", "SERVES"}, Rows: rows, Raw: page}); err != nil { return err } + if state == "" || state == "active" { + rt.Out.Info("Active Offering rules are listed in evaluation order; the first match wins.") + } hintMoreResults(rt, page) return nil }, @@ -315,11 +319,12 @@ func validateTargetingCreate(body api.TargetingRuleCreate) error { func newTargetingUpdateCmd() *cobra.Command { var config string cmd := &cobra.Command{ - Use: "update [id]", - Short: "Update an Offering targeting rule", - Long: "Partially updates a legacy targeting rule from a JSON object with position, state, display_name, offering_id, audience_id, conditions, schedule, or placements. To schedule an Offering rule, set state to active and schedule.start_date to a future UTC time; it will not match customers before then. Run rc schema targeting update for config fields and accepted values. An active rule or an activation requires confirmation. Checkpoint rule updates are not exposed by this endpoint.", - Example: ` echo '{"state":"active","position":1}' | rc targeting update trle_123 --config - --yes --no-input`, - Args: cobra.MaximumNArgs(1), + Use: "update [id]", + Short: "Update an Offering targeting rule", + Long: "Partially updates a legacy targeting rule from a JSON object with position, state, display_name, offering_id, audience_id, conditions, schedule, or placements. To schedule an Offering rule, set state to active and schedule.start_date to a future UTC time; it will not match customers before then. Run rc schema targeting update for config fields and accepted values. An active rule or an activation requires confirmation. Checkpoint rule updates are not exposed by this endpoint.", + Example: ` echo '{"state":"active","position":1}' | rc targeting update trle_123 --config - --yes --no-input + echo '{"state":"active","schedule":{"start_date":"2030-12-01T00:00:00Z","end_date":"2030-12-31T23:59:59Z"}}' | rc targeting update trle_123 --config - --yes --no-input`, + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if config == "" { return fmt.Errorf("pass --config or --config - for stdin") diff --git a/internal/cli/targeting_test.go b/internal/cli/targeting_test.go index 774dd474..f3fa8b3b 100644 --- a/internal/cli/targeting_test.go +++ b/internal/cli/targeting_test.go @@ -235,3 +235,71 @@ func TestTargetingShowCheckpointDetailsAndJSON(t *testing.T) { t.Fatalf("JSON lost checkpoint details: err=%v out=%s", err, out) } } + +func TestTargetingListPreservesEvaluationOrder(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("state") != "active" { + t.Errorf("state=%s", r.URL.RawQuery) + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"items":[{"id":"trle_z","display_name":"First match","rule_type":"legacy","state":"active","offering_id":"ofrng_z"},{"id":"trle_a","display_name":"Second match","rule_type":"legacy","state":"active","offering_id":"ofrng_a"}],"next_page":"/projects/proj/targeting_rules?state=active&starting_after=trle_a"}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + args := []string{"targeting", "list", "--state", "active", "--project-id", "proj", "--api-key", "sk_test", "--no-input"} + out, stderr, err := runAgentCmd(t, args...) + first, second := strings.Index(out, "trle_z"), strings.Index(out, "trle_a") + if err != nil || first < 0 || second < 0 || first > second || !strings.Contains(stderr, "evaluation order") || !strings.Contains(stderr, "--cursor trle_a") { + t.Fatalf("order or pagination missing: err=%v stdout=%s stderr=%s", err, out, stderr) + } + out, stderr, err = runAgentCmd(t, append(args, "--json")...) + var envelope struct { + Data struct { + Items []struct { + ID string `json:"id"` + } `json:"items"` + NextPage string `json:"next_page"` + } `json:"data"` + } + if err != nil || stderr != "" { + t.Fatalf("JSON list failed: err=%v stderr=%s", err, stderr) + } + if err := json.Unmarshal([]byte(out), &envelope); err != nil { + t.Fatal(err) + } + if len(envelope.Data.Items) != 2 || envelope.Data.Items[0].ID != "trle_z" || envelope.Data.Items[1].ID != "trle_a" || envelope.Data.NextPage == "" { + t.Fatalf("JSON lost order or pagination: %s", out) + } +} + +func TestTargetingUpdateScheduleRequiresApprovalAndPreservesBounds(t *testing.T) { + var posted map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + if err := json.NewDecoder(r.Body).Decode(&posted); err != nil { + t.Error(err) + } + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"id":"trle1","rule_type":"legacy","state":"inactive","display_name":"Holiday paywall","offering_id":"ofrng_holiday"}`) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + configPath := filepath.Join(t.TempDir(), "schedule.json") + if err := os.WriteFile(configPath, []byte(`{"state":"active","schedule":{"start_date":"2030-12-01T00:00:00Z","end_date":"2030-12-31T23:59:59Z"}}`), 0o600); err != nil { + t.Fatal(err) + } + args := []string{"targeting", "update", "trle1", "--config", configPath, "--project-id", "proj", "--api-key", "sk_test", "--no-input"} + _, stderr, err := runAgentCmd(t, args...) + if err == nil || !strings.Contains(err.Error(), "--yes") || posted != nil || !strings.Contains(stderr, "2030-12-01T00:00:00Z") || !strings.Contains(stderr, "2030-12-31T23:59:59Z") { + t.Fatalf("schedule was not reviewed before mutation: err=%v posted=%v stderr=%s", err, posted, stderr) + } + _, _, err = runAgentCmd(t, append(args, "--yes", "--json")...) + if err != nil { + t.Fatal(err) + } + schedule, ok := posted["schedule"].(map[string]any) + if !ok || posted["state"] != "active" || schedule["start_date"] != "2030-12-01T00:00:00Z" || schedule["end_date"] != "2030-12-31T23:59:59Z" { + t.Fatalf("schedule bounds missing from request: %v", posted) + } +} diff --git a/internal/cli/testdata/snapshots/targeting-list.golden b/internal/cli/testdata/snapshots/targeting-list.golden index 31020b38..4a892a48 100644 --- a/internal/cli/testdata/snapshots/targeting-list.golden +++ b/internal/cli/testdata/snapshots/targeting-list.golden @@ -1,3 +1,4 @@ $ rc targeting list --no-input --project-id proj_snap --api-key sk_snap ID NAME TYPE STATE SERVES trle_snap US annual paywall legacy active ofrng_us +· Active Offering rules are listed in evaluation order; the first match wins. From ff46780881366712e596e7e19004e98c4c9a8344 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Tue, 6 Oct 2026 16:48:51 -0500 Subject: [PATCH 14/16] fix(targeting): support checkpoint rule updates --- docs/command-surface.md | 8 +- docs/previews/targeting-update-checkpoint.svg | 45 +++++++ internal/cli/config_schema.go | 18 ++- internal/cli/schema_test.go | 13 ++ internal/cli/snapshot_test.go | 7 ++ internal/cli/targeting.go | 55 ++++++--- internal/cli/targeting_checkpoint_test.go | 114 ++++++++++++++++++ internal/cli/targeting_format.go | 13 ++ .../testdata/checkpoint-targeting-update.json | 6 + .../targeting-update-checkpoint.golden | 30 +++++ 10 files changed, 290 insertions(+), 19 deletions(-) create mode 100644 docs/previews/targeting-update-checkpoint.svg create mode 100644 internal/cli/targeting_checkpoint_test.go create mode 100644 internal/cli/testdata/checkpoint-targeting-update.json create mode 100644 internal/cli/testdata/snapshots/targeting-update-checkpoint.golden diff --git a/docs/command-surface.md b/docs/command-surface.md index 8a676445..a7a15fbb 100644 --- a/docs/command-surface.md +++ b/docs/command-surface.md @@ -207,13 +207,17 @@ rc experiments stop # permanent; confirmatio rc targeting list [--state ] rc targeting show # human detail view shows audience, placements, schedule, and checkpoints rc targeting create (--name --offering [--state inactive|active] | --config ) # initially inactive -rc targeting update --config # partial update +rc targeting update --config # partial update of an Offering or checkpoint rule rc targeting delete [--force] # active or scheduled rules require --force; confirmation/--yes Legacy Offering rules use `state: "active"` with a future `schedule.start_date` to start serving later. The API reports their state as `active` before that date, but targeting skips them until the scheduled window. The `scheduled` state is -only for checkpoint rules. +only for checkpoint rules. Checkpoint updates accept state, display_name, +audience_id, flow_id, checkpoints (exactly one checkpoint_id), and schedule. +Updates to active or scheduled rules, or transitions into those states, require +confirmation/--yes. Checkpoint updates cannot clear audience_id or set Offering +fields or position. # Rico (AI assistant) rc rico [message] # streaming chat window in a TTY (--plain for a line loop) diff --git a/docs/previews/targeting-update-checkpoint.svg b/docs/previews/targeting-update-checkpoint.svg new file mode 100644 index 00000000..a2211dd3 --- /dev/null +++ b/docs/previews/targeting-update-checkpoint.svg @@ -0,0 +1,45 @@ + + + + + +$ rc targeting update chkptrule_snap --config testdata/checkpoint-targeting-update.json --yes --no-input --project-id proj_snap --api-key sk_snap + +▍ Targeting rule — Onboarding +  Change the rule used to choose a Flow at a checkpoint. + +  Current flow                wf_original +  ID                          chkptrule_snap +  Current state               scheduled +  Current audience            aud_original +  Change audience id          aud_new +  Change checkpoints          chkpt_new +  Change flow id              wf_new +  Change state                active +  Resulting audience          aud_new + +▍ Plan +· 1. Update the targeting rule +✓ Updated targeting rule chkptrule_snap +▍ Onboarding +  chkptrule_snap · checkpoint · active + +Serves +  Flow:  wf_new + +Audience +  Audience:  aud_new + +Checkpoints +  chkpt_new:  Position 0 +  Use --json for the complete API response. + + + diff --git a/internal/cli/config_schema.go b/internal/cli/config_schema.go index d90d20eb..6c38ea08 100644 --- a/internal/cli/config_schema.go +++ b/internal/cli/config_schema.go @@ -35,7 +35,7 @@ func targetingConfigFields(create bool) map[string]any { "state": configEnum("Legacy Offering rules use active or inactive; use active with a future schedule.start_date to serve later. The scheduled state is checkpoint-only.", "active", "inactive", "scheduled"), "display_name": configField("string", "Rule name"), "offering_id": configField("string", "Offering ID served by a legacy rule"), - "audience_id": map[string]any{"type": "string", "nullable": true, "description": "Audience ID; mutually exclusive with conditions. Set null when switching to conditions; set conditions to [] when switching to an audience."}, + "audience_id": map[string]any{"type": "string", "nullable": true, "description": "Audience ID; mutually exclusive with conditions. Set null when switching to conditions; set conditions to [] when switching to an audience. Checkpoint rules require a non-null audience ID."}, "conditions": targetingConditionsSchema(), "schedule": schedule, "placements": placements, @@ -53,8 +53,20 @@ func targetingConfigFields(create bool) map[string]any { return map[string]any{"type": "object", "properties": fields, "description": "Legacy: display_name and offering_id required. Checkpoint: rule_type, display_name, audience_id, flow_id, checkpoints required."} } schedule["type"] = []string{"object", "null"} - schedule["description"] = "Omit to keep the current schedule. Set null to remove it, or provide UTC start_date and optional end_date timestamps. Use state active with a future start_date to serve later." - return map[string]any{"type": "object", "properties": fields, "description": "Partial update of a legacy rule. Checkpoint updates are unavailable."} + schedule["description"] = "Omit to keep the current schedule. Set null to remove it, or provide UTC timestamps. Offering rules require start_date and state active to serve later; checkpoint rules allow an omitted start_date unless state is scheduled." + fields["flow_id"] = configField("string", "Flow ID served by a checkpoint rule; checkpoint-only") + fields["checkpoints"] = map[string]any{ + "type": "array", "minItems": 1, "maxItems": 1, + "description": "Move a checkpoint rule to exactly one checkpoint; appends after that checkpoint's existing rules. Position cannot be set here.", + "items": map[string]any{ + "type": "object", "required": []string{"checkpoint_id"}, "additionalProperties": false, + "properties": map[string]any{"checkpoint_id": configField("string", "Checkpoint ID")}, + }, + } + return map[string]any{ + "type": "object", "properties": fields, + "description": "Partial update. Active or scheduled rules require confirmation. Checkpoint rules accept state, display_name, audience_id (non-null), flow_id, checkpoints, and schedule; Offering fields and position are legacy-only.", + } } func configField(kind, description string) map[string]any { diff --git a/internal/cli/schema_test.go b/internal/cli/schema_test.go index 7313eb3c..960f4015 100644 --- a/internal/cli/schema_test.go +++ b/internal/cli/schema_test.go @@ -43,6 +43,19 @@ func TestTargetingConfigSchemaExplainsConditionsAndRuleTypes(t *testing.T) { } } +func TestTargetingUpdateSchemaIncludesCheckpointFieldsAndLimits(t *testing.T) { + root := NewRootCmd("test") + config := commandSchema(findCommand(t, root, "targeting update"))["config_fields"].(map[string]any) + fields := config["properties"].(map[string]any) + if fields["flow_id"] == nil { + t.Fatal("checkpoint Flow field is missing") + } + checkpoints := fields["checkpoints"].(map[string]any) + if checkpoints["minItems"] != 1 || checkpoints["maxItems"] != 1 || !strings.Contains(config["description"].(string), "audience_id (non-null)") { + t.Fatalf("checkpoint update constraints are missing: %v", config) + } +} + func findCommand(t *testing.T, root *cobra.Command, path string) *cobra.Command { t.Helper() cur := root diff --git a/internal/cli/snapshot_test.go b/internal/cli/snapshot_test.go index 24d52efe..d71c721e 100644 --- a/internal/cli/snapshot_test.go +++ b/internal/cli/snapshot_test.go @@ -32,6 +32,12 @@ func snapshotServer(t *testing.T) *httptest.Server { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") switch { + case strings.HasSuffix(r.URL.Path, "/targeting_rules/chkptrule_snap"): + if r.Method == http.MethodPost { + io.WriteString(w, `{"id":"chkptrule_snap","rule_type":"checkpoint","state":"active","display_name":"Onboarding","flow_id":"wf_new","audience_id":"aud_new","checkpoints":[{"checkpoint_id":"chkpt_new","position":0}]}`) + } else { + io.WriteString(w, `{"id":"chkptrule_snap","rule_type":"checkpoint","state":"scheduled","display_name":"Onboarding","flow_id":"wf_original","audience_id":"aud_original","checkpoints":[{"checkpoint_id":"chkpt_original","position":0}],"schedule":{"start_date":"2030-12-01T00:00:00Z"}}`) + } case strings.HasSuffix(r.URL.Path, "/targeting_rules/trle_snap"): io.WriteString(w, `{"object":"targeting_rule","id":"trle_snap","rule_type":"legacy","state":"active","display_name":"US annual paywall","offering_id":"ofrng_us","conditions":[{"field":"platform","operator":"in","value":["ios"]}],"placements":{"fallback_offering_id":"ofrng_default","placement_offerings":[{"placement_identifier":"onboarding","offering_id":"ofrng_us"}]},"schedule":{"start_date":"2026-09-25T12:00:00Z","end_date":null}}`) case strings.HasSuffix(r.URL.Path, "/targeting_rules"): @@ -93,6 +99,7 @@ func TestOutputSnapshots(t *testing.T) { {"experiments-show", []string{"experiments", "show", "exp_snap", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"experiments-results", []string{"experiments", "results", "exp_snap", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"experiments-start", []string{"experiments", "start", "exp_snap", "--yes", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, + {"targeting-update-checkpoint", []string{"targeting", "update", "chkptrule_snap", "--config", "testdata/checkpoint-targeting-update.json", "--yes", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"targeting-list", []string{"targeting", "list", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"targeting-show", []string{"targeting", "show", "trle_snap", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, {"targeting-create-active", []string{"targeting", "create", "--name", "Default paywall", "--offering", "ofrng_default", "--state", "active", "--yes", "--no-input", "--project-id", "proj_snap", "--api-key", "sk_snap"}}, diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index 99d3b14d..5b72e0ba 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -320,10 +320,11 @@ func newTargetingUpdateCmd() *cobra.Command { var config string cmd := &cobra.Command{ Use: "update [id]", - Short: "Update an Offering targeting rule", - Long: "Partially updates a legacy targeting rule from a JSON object with position, state, display_name, offering_id, audience_id, conditions, schedule, or placements. To schedule an Offering rule, set state to active and schedule.start_date to a future UTC time; it will not match customers before then. Run rc schema targeting update for config fields and accepted values. An active rule or an activation requires confirmation. Checkpoint rule updates are not exposed by this endpoint.", + Short: "Update a targeting rule", + Long: "Partially updates a targeting rule. Offering rules accept position, state, display_name, offering_id, audience_id, conditions, schedule, or placements. To schedule an Offering rule, set state to active and schedule.start_date to a future UTC time; it will not match customers before then. Run rc schema targeting update for config fields and accepted values. Active or scheduled rules, and transitions into those states, require confirmation. Checkpoint rules accept state, display_name, audience_id, flow_id, checkpoints, and schedule.", Example: ` echo '{"state":"active","position":1}' | rc targeting update trle_123 --config - --yes --no-input - echo '{"state":"active","schedule":{"start_date":"2030-12-01T00:00:00Z","end_date":"2030-12-31T23:59:59Z"}}' | rc targeting update trle_123 --config - --yes --no-input`, + echo '{"state":"active","schedule":{"start_date":"2030-12-01T00:00:00Z","end_date":"2030-12-31T23:59:59Z"}}' | rc targeting update trle_123 --config - --yes --no-input + echo '{"flow_id":"wf_new","checkpoints":[{"checkpoint_id":"chkpt_new"}]}' | rc targeting update chkptrule_123 --config - --yes --no-input`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if config == "" { @@ -355,27 +356,30 @@ func newTargetingUpdateCmd() *cobra.Command { if err != nil { return err } - if current.RuleType == "checkpoint" { - return fmt.Errorf("checkpoint rule updates are not supported by this endpoint") + if err := validTargetingUpdateForRule(current.RuleType, body); err != nil { + return err } var newState string if value, ok := body["state"]; ok { - if err := json.Unmarshal(value, &newState); err != nil { - return fmt.Errorf("state must be active or inactive") - } - if newState != "active" && newState != "inactive" { - return fmt.Errorf("state must be active or inactive") + if err := json.Unmarshal(value, &newState); err != nil || (newState != "active" && newState != "inactive" && !(current.RuleType == "checkpoint" && newState == "scheduled")) { + return fmt.Errorf("state must be active or inactive (checkpoint rules also support scheduled)") } } resultingAudience, everyone, err := targetingAudienceAfterUpdate(current, body) if err != nil { return err } - if current.State == "active" || newState == "active" { + if current.State != "inactive" || newState == "active" || newState == "scheduled" { rt.Out.Title("Targeting rule — " + current.DisplayName) - rt.Out.Lead("Change the rule used to choose an Offering for matching customers.") + if current.RuleType == "checkpoint" { + rt.Out.Lead("Change the rule used to choose a Flow at a checkpoint.") + rt.Out.Field("Current flow", current.FlowID) + } else { + rt.Out.Lead("Change the rule used to choose an Offering for matching customers.") + rt.Out.Field("Current offering", current.OfferingID) + } + rt.Out.Field("ID", current.ID) rt.Out.Field("Current state", current.State) - rt.Out.Field("Current offering", current.OfferingID) if current.AudienceID != nil { rt.Out.Field("Current audience", *current.AudienceID) } else if len(current.Conditions) > 0 { @@ -415,7 +419,7 @@ func validTargetingUpdate(body api.TargetingRuleUpdate) error { if len(body) == 0 { return fmt.Errorf("config must contain at least one field") } - allowed := map[string]bool{"position": true, "state": true, "display_name": true, "offering_id": true, "audience_id": true, "conditions": true, "schedule": true, "placements": true} + allowed := map[string]bool{"position": true, "state": true, "display_name": true, "offering_id": true, "audience_id": true, "conditions": true, "schedule": true, "placements": true, "flow_id": true, "checkpoints": true} for key := range body { if !allowed[key] { return fmt.Errorf("unknown targeting rule field %q", key) @@ -424,6 +428,29 @@ func validTargetingUpdate(body api.TargetingRuleUpdate) error { return nil } +func validTargetingUpdateForRule(ruleType string, body api.TargetingRuleUpdate) error { + if ruleType != "checkpoint" { + for _, field := range []string{"flow_id", "checkpoints"} { + if _, ok := body[field]; ok { + return fmt.Errorf("%s can only be set on checkpoint rules", field) + } + } + return nil + } + for _, field := range []string{"position", "offering_id", "conditions", "placements"} { + if _, ok := body[field]; ok { + return fmt.Errorf("%s cannot be set on checkpoint rules", field) + } + } + if value, ok := body["audience_id"]; ok { + var audience string + if err := json.Unmarshal(value, &audience); err != nil || audience == "" { + return fmt.Errorf("checkpoint audience_id must be a nonempty string") + } + } + return nil +} + func targetingAudienceAfterUpdate(current *api.TargetingRule, body api.TargetingRuleUpdate) (string, bool, error) { audienceID := "" if current.AudienceID != nil { diff --git a/internal/cli/targeting_checkpoint_test.go b/internal/cli/targeting_checkpoint_test.go new file mode 100644 index 00000000..7d8f8cb1 --- /dev/null +++ b/internal/cli/targeting_checkpoint_test.go @@ -0,0 +1,114 @@ +package cli_test + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestCheckpointTargetingUpdateGuardsLiveAndScheduledChanges(t *testing.T) { + for _, tc := range []struct { + name, currentState, newState string + approval bool + }{ + {"activate", "inactive", "active", true}, + {"schedule", "inactive", "scheduled", true}, + {"edit active", "active", "", true}, + {"edit scheduled", "scheduled", "", true}, + {"edit inactive", "inactive", "", false}, + } { + t.Run(tc.name, func(t *testing.T) { + var posted map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/projects/proj/targeting_rules/chkptrule1" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Method == http.MethodPost { + if err := json.NewDecoder(r.Body).Decode(&posted); err != nil { + t.Error(err) + } + } + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"id":"chkptrule1","rule_type":"checkpoint","state":%q,"display_name":"Onboarding","flow_id":"wf_original","audience_id":"aud_original"}`, tc.currentState) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + config := map[string]any{ + "display_name": "New onboarding", "flow_id": "wf_new", "audience_id": "aud_new", + "checkpoints": []map[string]string{{"checkpoint_id": "chkpt_new"}}, + "schedule": map[string]string{"start_date": "2030-12-01T00:00:00Z", "end_date": "2030-12-31T23:59:59Z"}, + } + if tc.newState != "" { + config["state"] = tc.newState + } + data, err := json.Marshal(config) + if err != nil { + t.Fatal(err) + } + configPath := filepath.Join(t.TempDir(), "checkpoint.json") + if err := os.WriteFile(configPath, data, 0o600); err != nil { + t.Fatal(err) + } + args := []string{"targeting", "update", "chkptrule1", "--config", configPath, "--project-id", "proj", "--api-key", "sk_test", "--no-input"} + if tc.approval { + _, preview, err := runAgentCmd(t, args...) + if err == nil || !strings.Contains(err.Error(), "--yes") || posted != nil { + t.Fatalf("approval did not precede mutation: err=%v posted=%v", err, posted) + } + for _, want := range []string{"Current flow", "wf_original", "wf_new", "chkpt_new", "2030-12-01T00:00:00Z"} { + if !strings.Contains(preview, want) { + t.Fatalf("missing %q in preview: %s", want, preview) + } + } + args = append(args, "--yes") + } + out, stderr, err := runAgentCmd(t, append(args, "--json")...) + if err != nil || stderr != "" || posted == nil || !strings.Contains(out, `"rule_type": "checkpoint"`) { + t.Fatalf("checkpoint update failed: err=%v posted=%v stdout=%s stderr=%s", err, posted, out, stderr) + } + postedJSON, err := json.Marshal(posted) + if err != nil || string(postedJSON) != string(data) { + t.Fatalf("request changed: got=%s want=%s err=%v", postedJSON, data, err) + } + }) + } +} + +func TestTargetingUpdateRejectsFieldsForWrongRuleType(t *testing.T) { + for _, tc := range []struct{ name, ruleType, config, want string }{ + {"checkpoint position", "checkpoint", `{"position":1}`, "cannot be set on checkpoint"}, + {"checkpoint offering", "checkpoint", `{"offering_id":"ofrng1"}`, "cannot be set on checkpoint"}, + {"checkpoint conditions", "checkpoint", `{"conditions":[]}`, "cannot be set on checkpoint"}, + {"checkpoint placements", "checkpoint", `{"placements":null}`, "cannot be set on checkpoint"}, + {"checkpoint null audience", "checkpoint", `{"audience_id":null}`, "nonempty string"}, + {"Offering flow", "legacy", `{"flow_id":"wf1"}`, "only be set on checkpoint"}, + {"Offering checkpoints", "legacy", `{"checkpoints":[{"checkpoint_id":"chkpt1"}]}`, "only be set on checkpoint"}, + {"Offering scheduled state", "legacy", `{"state":"scheduled"}`, "state must be active or inactive"}, + } { + t.Run(tc.name, func(t *testing.T) { + mutations := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + mutations++ + } + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"id":"rule1","rule_type":%q,"state":"inactive"}`, tc.ruleType) + })) + t.Cleanup(srv.Close) + t.Setenv("RC_BASE_URL", srv.URL) + configPath := filepath.Join(t.TempDir(), "update.json") + if err := os.WriteFile(configPath, []byte(tc.config), 0o600); err != nil { + t.Fatal(err) + } + _, _, err := runAgentCmd(t, "targeting", "update", "rule1", "--config", configPath, "--project-id", "proj", "--api-key", "sk_test", "--no-input", "--yes") + if err == nil || !strings.Contains(err.Error(), tc.want) || mutations != 0 { + t.Fatalf("err=%v mutations=%d; want %q before mutation", err, mutations, tc.want) + } + }) + } +} diff --git a/internal/cli/targeting_format.go b/internal/cli/targeting_format.go index a3f5876a..68de2b59 100644 --- a/internal/cli/targeting_format.go +++ b/internal/cli/targeting_format.go @@ -21,6 +21,19 @@ func targetingConfigSummary(key string, raw json.RawMessage) (string, error) { } return experimentConditionSummary(conditions), nil } + if key == "checkpoints" { + var checkpoints []struct { + ID string `json:"checkpoint_id"` + } + if err := json.Unmarshal(raw, &checkpoints); err != nil { + return "", err + } + ids := make([]string, len(checkpoints)) + for i, checkpoint := range checkpoints { + ids[i] = checkpoint.ID + } + return strings.Join(ids, ", "), nil + } if key == "schedule" { if string(raw) == "null" { return "None", nil diff --git a/internal/cli/testdata/checkpoint-targeting-update.json b/internal/cli/testdata/checkpoint-targeting-update.json new file mode 100644 index 00000000..16bc46b6 --- /dev/null +++ b/internal/cli/testdata/checkpoint-targeting-update.json @@ -0,0 +1,6 @@ +{ + "state": "active", + "flow_id": "wf_new", + "audience_id": "aud_new", + "checkpoints": [{"checkpoint_id": "chkpt_new"}] +} diff --git a/internal/cli/testdata/snapshots/targeting-update-checkpoint.golden b/internal/cli/testdata/snapshots/targeting-update-checkpoint.golden new file mode 100644 index 00000000..0ed38d44 --- /dev/null +++ b/internal/cli/testdata/snapshots/targeting-update-checkpoint.golden @@ -0,0 +1,30 @@ +$ rc targeting update chkptrule_snap --config testdata/checkpoint-targeting-update.json --yes --no-input --project-id proj_snap --api-key sk_snap + +▍ Targeting rule — Onboarding + Change the rule used to choose a Flow at a checkpoint. + + Current flow wf_original + ID chkptrule_snap + Current state scheduled + Current audience aud_original + Change audience id aud_new + Change checkpoints chkpt_new + Change flow id wf_new + Change state active + Resulting audience aud_new + +▍ Plan +· 1. Update the targeting rule +✓ Updated targeting rule chkptrule_snap +▍ Onboarding + chkptrule_snap · checkpoint · active + +Serves + Flow: wf_new + +Audience + Audience: aud_new + +Checkpoints + chkpt_new: Position 0 + Use --json for the complete API response. From 0c94120d0761f0d2f474e13365886aa7613d53c8 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Tue, 6 Oct 2026 16:49:49 -0500 Subject: [PATCH 15/16] fix(targeting): preserve preview state and document null values --- internal/cli/config_schema.go | 12 ++++++------ internal/cli/schema_test.go | 16 +++++++++++++++- internal/cli/targeting.go | 1 + internal/cli/targeting_format_test.go | 12 ++++++++++++ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/internal/cli/config_schema.go b/internal/cli/config_schema.go index 6c38ea08..737390bb 100644 --- a/internal/cli/config_schema.go +++ b/internal/cli/config_schema.go @@ -18,15 +18,15 @@ func configFieldsFor(cmd *cobra.Command) map[string]any { func targetingConfigFields(create bool) map[string]any { schedule := map[string]any{"type": "object", "description": "UTC timestamps ending in Z. Legacy Offering rules require start_date; set state to active with a future start_date to serve later.", "required_for_legacy": []string{"start_date"}, "properties": map[string]any{ - "start_date": configField("string", "Start time, for example 2026-05-25T10:00:00Z"), - "end_date": configField("string", "Optional end time in the same format"), + "start_date": map[string]any{"type": []string{"string", "null"}, "description": "Start time in UTC, for example 2026-05-25T10:00:00Z. Null means no start date for a checkpoint rule; Offering schedules require a start date."}, + "end_date": map[string]any{"type": []string{"string", "null"}, "description": "End time in UTC; omit or set null for no end date."}, }} - placements := map[string]any{"type": "object", "description": "Legacy rules only; both fields are required when placements is provided", "required": []string{"fallback_offering_id", "placement_offerings"}, "properties": map[string]any{ - "fallback_offering_id": configField("string", "Fallback Offering ID"), + placements := map[string]any{"type": []string{"object", "null"}, "description": "Legacy rules only. On update, omit to keep overrides or set null to remove them; both fields are required when providing an object.", "required": []string{"fallback_offering_id", "placement_offerings"}, "properties": map[string]any{ + "fallback_offering_id": map[string]any{"type": []string{"string", "null"}, "description": "Fallback Offering ID; null means no fallback."}, "placement_offerings": map[string]any{"type": "array", "items": map[string]any{ "type": "object", "required": []string{"placement_identifier", "offering_id"}, "properties": map[string]any{ "placement_identifier": configField("string", "Placement identifier, such as onboarding"), - "offering_id": configField("string", "Offering ID for this placement"), + "offering_id": map[string]any{"type": []string{"string", "null"}, "description": "Offering ID for this placement; null means no Offering override."}, }, }}, }} @@ -35,7 +35,7 @@ func targetingConfigFields(create bool) map[string]any { "state": configEnum("Legacy Offering rules use active or inactive; use active with a future schedule.start_date to serve later. The scheduled state is checkpoint-only.", "active", "inactive", "scheduled"), "display_name": configField("string", "Rule name"), "offering_id": configField("string", "Offering ID served by a legacy rule"), - "audience_id": map[string]any{"type": "string", "nullable": true, "description": "Audience ID; mutually exclusive with conditions. Set null when switching to conditions; set conditions to [] when switching to an audience. Checkpoint rules require a non-null audience ID."}, + "audience_id": map[string]any{"type": []string{"string", "null"}, "description": "Audience ID; mutually exclusive with conditions. Set null when switching to conditions; set conditions to [] when switching to an audience. Checkpoint rules require a non-null audience ID."}, "conditions": targetingConditionsSchema(), "schedule": schedule, "placements": placements, diff --git a/internal/cli/schema_test.go b/internal/cli/schema_test.go index 960f4015..68dbbc99 100644 --- a/internal/cli/schema_test.go +++ b/internal/cli/schema_test.go @@ -37,7 +37,7 @@ func TestTargetingConfigSchemaExplainsConditionsAndRuleTypes(t *testing.T) { } } audience := config["properties"].(map[string]any)["audience_id"].(map[string]any) - if audience["nullable"] != true || !strings.Contains(audience["description"].(string), "conditions to []") { + if !contains(audience["type"].([]string), "null") || !strings.Contains(audience["description"].(string), "conditions to []") { t.Errorf("%s schema must explain how to switch between audiences and conditions", path) } } @@ -184,3 +184,17 @@ func hasRunnableDescendant(c *cobra.Command) bool { } return false } + +func TestTargetingSchemaDocumentsNullableFields(t *testing.T) { + fields := targetingConfigFields(false)["properties"].(map[string]any) + schedule := fields["schedule"].(map[string]any)["properties"].(map[string]any) + placements := fields["placements"].(map[string]any) + placementFields := placements["properties"].(map[string]any) + itemFields := placementFields["placement_offerings"].(map[string]any)["items"].(map[string]any)["properties"].(map[string]any) + for name, value := range map[string]any{"audience_id": fields["audience_id"], "schedule": fields["schedule"], "start_date": schedule["start_date"], "end_date": schedule["end_date"], "placements": placements, "fallback_offering_id": placementFields["fallback_offering_id"], "placement offering_id": itemFields["offering_id"]} { + types, ok := value.(map[string]any)["type"].([]string) + if !ok || !contains(types, "null") { + t.Errorf("%s does not expose null: %v", name, value) + } + } +} diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index 5b72e0ba..da93cf79 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -464,6 +464,7 @@ func targetingAudienceAfterUpdate(current *api.TargetingRule, body api.Targeting } } if value, ok := body["conditions"]; ok { + conditions = nil if err := json.Unmarshal(value, &conditions); err != nil { return "", false, fmt.Errorf("conditions must be an array") } diff --git a/internal/cli/targeting_format_test.go b/internal/cli/targeting_format_test.go index 59a42b8e..06686861 100644 --- a/internal/cli/targeting_format_test.go +++ b/internal/cli/targeting_format_test.go @@ -57,3 +57,15 @@ func TestTargetingAudienceAfterUpdateRequiresClearingOtherField(t *testing.T) { }) } } + +func TestTargetingAudiencePreviewPreservesCurrentConditions(t *testing.T) { + current := &api.TargetingRule{Conditions: []any{map[string]any{"field": "country", "operator": "in", "value": []any{"US"}}}} + got, _, err := targetingAudienceAfterUpdate(current, api.TargetingRuleUpdate{"conditions": json.RawMessage(`[{"field":"country","operator":"in","value":["CA"]}]`)}) + if err != nil || got != "country in CA" { + t.Fatalf("result=%q err=%v", got, err) + } + original, err := targetingConditionsSummary(current.Conditions) + if err != nil || original != "country in US" { + t.Fatalf("current conditions changed: %q err=%v", original, err) + } +} From 85548b03c38b393a2c0d68e6668ddfd7091c1fa2 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Tue, 6 Oct 2026 17:19:01 -0500 Subject: [PATCH 16/16] fix(targeting): include every page in rule pickers --- internal/cli/targeting.go | 22 +++++++++------ internal/cli/targeting_picker_test.go | 39 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 internal/cli/targeting_picker_test.go diff --git a/internal/cli/targeting.go b/internal/cli/targeting.go index da93cf79..3e3790ef 100644 --- a/internal/cli/targeting.go +++ b/internal/cli/targeting.go @@ -70,15 +70,21 @@ func newTargetingListCmd() *cobra.Command { } func targetingPickerItems(cmd *cobra.Command, client *api.Client, projectID string) ([]PickerItem, error) { - page, err := client.TargetingRules.List(cmd.Context(), projectID, api.ListTargetingRulesOptions{}) - if err != nil { - return nil, err - } - items := make([]PickerItem, len(page.Items)) - for i, rule := range page.Items { - items[i] = PickerItem{ID: rule.ID, Label: fmt.Sprintf("%s (%s)", rule.DisplayName, rule.State)} + var items []PickerItem + cursor := "" + for { + page, err := client.TargetingRules.List(cmd.Context(), projectID, api.ListTargetingRulesOptions{Limit: 100, StartingAfter: cursor}) + if err != nil { + return nil, err + } + for _, rule := range page.Items { + items = append(items, PickerItem{ID: rule.ID, Label: fmt.Sprintf("%s (%s)", rule.DisplayName, rule.State)}) + } + cursor = page.NextCursor() + if cursor == "" { + return items, nil + } } - return items, nil } func newTargetingShowCmd() *cobra.Command { diff --git a/internal/cli/targeting_picker_test.go b/internal/cli/targeting_picker_test.go new file mode 100644 index 00000000..e55131bb --- /dev/null +++ b/internal/cli/targeting_picker_test.go @@ -0,0 +1,39 @@ +package cli + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/revenuecat/cli/internal/api" + "github.com/spf13/cobra" +) + +func TestTargetingPickerIncludesNextPage(t *testing.T) { + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + if r.URL.Query().Get("limit") != "100" { + t.Errorf("limit=%s", r.URL.Query().Get("limit")) + } + w.Header().Set("Content-Type", "application/json") + if r.URL.Query().Get("starting_after") == "" { + fmt.Fprint(w, `{"items":[{"id":"trle1","display_name":"First","state":"inactive"}],"next_page":"/projects/proj/targeting_rules?starting_after=trle1"}`) + } else { + if r.URL.Query().Get("starting_after") != "trle1" { + t.Errorf("cursor=%s", r.URL.RawQuery) + } + fmt.Fprint(w, `{"items":[{"id":"trle2","display_name":"Second","state":"active"}],"next_page":null}`) + } + })) + defer srv.Close() + client := api.NewClient(api.Options{BaseURL: srv.URL, APIKey: "sk_test"}) + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + items, err := targetingPickerItems(cmd, client, "proj") + if err != nil || requests != 2 || len(items) != 2 || items[1].ID != "trle2" { + t.Fatalf("items=%v requests=%d err=%v", items, requests, err) + } +}