From f6ca7bab16672ffd6dc9a6b1fb089276f65afe50 Mon Sep 17 00:00:00 2001 From: Mike North Date: Sun, 8 Mar 2026 13:50:28 -0700 Subject: [PATCH] Add --param flag and application fee event triggers Adds a --param flag to `stripe trigger` with pre-flight validation for events that require user-provided configuration. Application fee events are the first to use this, requiring a Connect account ID. The --param flag validates required parameters before making API calls, providing early feedback and clear error messages. It uses the same syntax as --override (fixtureName:path.to.field=value) and is backed by a new required_params metadata block in fixture JSON files. New triggers: application_fee.created, application_fee.refunded, application_fee.refund.updated. --- pkg/cmd/trigger.go | 10 +- pkg/fixtures/fixtures.go | 14 +- pkg/fixtures/triggers.go | 169 +++++++- .../triggers/application_fee.created.json | 29 ++ .../application_fee.refund.updated.json | 43 ++ .../triggers/application_fee.refunded.json | 35 ++ pkg/fixtures/triggers_test.go | 387 ++++++++++++++++++ pkg/rpcservice/trigger.go | 1 + scratch/pr12-description.md | 161 ++++++++ 9 files changed, 838 insertions(+), 11 deletions(-) create mode 100644 pkg/fixtures/triggers/application_fee.created.json create mode 100644 pkg/fixtures/triggers/application_fee.refund.updated.json create mode 100644 pkg/fixtures/triggers/application_fee.refunded.json create mode 100644 pkg/fixtures/triggers_test.go create mode 100644 scratch/pr12-description.md diff --git a/pkg/cmd/trigger.go b/pkg/cmd/trigger.go index 19d0b1a16..669365ee9 100644 --- a/pkg/cmd/trigger.go +++ b/pkg/cmd/trigger.go @@ -21,6 +21,7 @@ type triggerCmd struct { apiVersion string skip []string override []string + param []string add []string remove []string raw string @@ -46,7 +47,11 @@ needed to create the triggered event as well as the corresponding API objects. ansi.Bold("Supported events:"), fixtures.EventList(), ), - Example: `stripe trigger payment_intent.created`, + Example: `# Trigger a basic event + stripe trigger payment_intent.created + + # Trigger an event that requires parameters + stripe trigger application_fee.created --param charge:transfer_data.destination=acct_123`, Annotations: map[string]string{ AIAgentHelpAnnotationKey: " Use `--override` to customize event data, e.g. `--override customer:email=test@example.com`.\n" + " Use `--skip` to skip specific steps in the trigger sequence.\n" + @@ -58,6 +63,7 @@ needed to create the triggered event as well as the corresponding API objects. tc.cmd.Flags().StringVar(&tc.stripeAccount, "stripe-account", "", "Set a header identifying the connected account") tc.cmd.Flags().StringArrayVar(&tc.skip, "skip", []string{}, "Skip specific steps in the trigger") tc.cmd.Flags().StringArrayVar(&tc.override, "override", []string{}, "Override params in the trigger") + tc.cmd.Flags().StringArrayVar(&tc.param, "param", []string{}, "Set required parameters (validated before execution)") tc.cmd.Flags().StringArrayVar(&tc.add, "add", []string{}, "Add params to the trigger") tc.cmd.Flags().StringArrayVar(&tc.remove, "remove", []string{}, "Remove params from the trigger") tc.cmd.Flags().StringVar(&tc.raw, "raw", "", "Raw fixture in string format to replace all default fixtures") @@ -91,7 +97,7 @@ func (tc *triggerCmd) runTriggerCmd(cmd *cobra.Command, args []string) error { event := args[0] - _, err = fixtures.Trigger(cmd.Context(), event, tc.stripeAccount, tc.apiBaseURL, apiKey, tc.skip, tc.override, tc.add, tc.remove, tc.raw, tc.apiVersion, tc.edit) + _, err = fixtures.Trigger(cmd.Context(), event, tc.stripeAccount, tc.apiBaseURL, apiKey, tc.skip, tc.override, tc.param, tc.add, tc.remove, tc.raw, tc.apiVersion, tc.edit) if err != nil { return err } diff --git a/pkg/fixtures/fixtures.go b/pkg/fixtures/fixtures.go index 790d3a792..b56f21112 100644 --- a/pkg/fixtures/fixtures.go +++ b/pkg/fixtures/fixtures.go @@ -25,11 +25,19 @@ import ( // SupportedVersions is the version number of the fixture template the CLI supports const SupportedVersions = 0 +// RequiredParam describes a parameter that must be provided via --param flag +type RequiredParam struct { + Name string `json:"name"` // Fixture path (e.g., "charge:transfer_data.destination") + Description string `json:"description"` // Human-readable description + Placeholder string `json:"placeholder"` // Placeholder value for error messages +} + // MetaFixture contains fixture metadata type MetaFixture struct { - Version int `json:"template_version"` - ExcludeMetadata bool `json:"exclude_metadata"` - Aliases []string `json:"aliases,omitempty"` + Version int `json:"template_version"` + ExcludeMetadata bool `json:"exclude_metadata"` + Aliases []string `json:"aliases,omitempty"` + RequiredParams []RequiredParam `json:"required_params,omitempty"` } // FixtureData contains the whole fixture file diff --git a/pkg/fixtures/triggers.go b/pkg/fixtures/triggers.go index 47d19c43e..8441dfed5 100644 --- a/pkg/fixtures/triggers.go +++ b/pkg/fixtures/triggers.go @@ -6,12 +6,14 @@ import ( "encoding/json" "fmt" "io" + "os" "sort" "strings" "sync" "github.com/spf13/afero" + "github.com/stripe/stripe-cli/pkg/ansi" "github.com/stripe/stripe-cli/pkg/stripe" ) @@ -87,7 +89,7 @@ func FixtureContents(eventName string) (string, error) { } // BuildFromFixtureFile creates a new fixture struct for a file -func BuildFromFixtureFile(fs afero.Fs, apiKey, stripeAccount, apiBaseURL, jsonFile string, skip, override, add, remove []string, edit bool) (*Fixture, error) { +func BuildFromFixtureFile(fs afero.Fs, apiKey, stripeAccount, apiBaseURL, jsonFile string, skip, override, param, add, remove []string, edit bool) (*Fixture, error) { fixture, err := NewFixtureFromFile( fs, apiKey, @@ -104,6 +106,21 @@ func BuildFromFixtureFile(fs afero.Fs, apiKey, stripeAccount, apiBaseURL, jsonFi return nil, err } + // Validate required params before proceeding + if err := ValidateRequiredParams(fixture, jsonFile, param); err != nil { + return nil, err + } + + // Merge params with overrides (params take precedence, same syntax) + mergedOverrides := make([]string, 0, len(override)+len(param)) + mergedOverrides = append(mergedOverrides, override...) + mergedOverrides = append(mergedOverrides, param...) + if len(mergedOverrides) > 0 { + if err := fixture.Override(mergedOverrides); err != nil { + return nil, err + } + } + return fixture, nil } @@ -117,15 +134,71 @@ func BuildFromFixtureString(fs afero.Fs, apiKey, stripeAccount, apiBaseURL, raw } // EventList prints out a padded list of supported trigger events for printing the help file +// Events that require parameters show the --param syntax inline, vertically aligned func EventList() string { + eventNames := EventNames() + + // First pass: find the maximum event name length ONLY for events that require params + maxLength := 0 + for _, event := range eventNames { + if file, ok := getEvents()[event]; ok { + params := getRequiredParamsForEvent(file) + if len(params) > 0 && len(event) > maxLength { + maxLength = len(event) + } + } + } + + // Second pass: build the list with proper padding for vertical alignment var eventList string - for _, event := range EventNames() { - eventList += fmt.Sprintf(" %s\n", event) + for _, event := range eventNames { + // Try to load fixture metadata to check for required params + if file, ok := getEvents()[event]; ok { + params := getRequiredParamsForEvent(file) + if len(params) > 0 { + // Show event with required params syntax, padded for vertical alignment + paramSyntax := "" + for i, param := range params { + if i > 0 { + paramSyntax += " " + } + paramSyntax += fmt.Sprintf("--param %s=", param.Name) + } + // Pad event name to max length for vertical alignment + padding := maxLength - len(event) + eventList += fmt.Sprintf(" %s%s %s\n", event, strings.Repeat(" ", padding), paramSyntax) + } else { + eventList += fmt.Sprintf(" %s\n", event) + } + } else { + eventList += fmt.Sprintf(" %s\n", event) + } } return eventList } +// getRequiredParamsForEvent loads fixture metadata and returns required params if any +func getRequiredParamsForEvent(fixtureFile string) []RequiredParam { + f, err := triggers.Open(fixtureFile) + if err != nil { + return nil + } + defer f.Close() + + filedata, err := io.ReadAll(f) + if err != nil { + return nil + } + + var fixtureData FixtureData + if err := json.Unmarshal(filedata, &fixtureData); err != nil { + return nil + } + + return fixtureData.Meta.RequiredParams +} + // EventNames returns an array of all the event names func EventNames() []string { names := []string{} @@ -139,7 +212,7 @@ func EventNames() []string { } // Trigger triggers a Stripe event. -func Trigger(ctx context.Context, event string, stripeAccount string, baseURL string, apiKey string, skip, override, add, remove []string, raw string, apiVersion string, edit bool) ([]string, error) { +func Trigger(ctx context.Context, event string, stripeAccount string, baseURL string, apiKey string, skip, override, param, add, remove []string, raw string, apiVersion string, edit bool) ([]string, error) { var fixture *Fixture var err error fs := afero.NewOsFs() @@ -152,7 +225,7 @@ func Trigger(ctx context.Context, event string, stripeAccount string, baseURL st if len(raw) == 0 { if file, ok := getEvents()[event]; ok { - fixture, err = BuildFromFixtureFile(fs, apiKey, stripeAccount, baseURL, file, skip, override, add, remove, edit) + fixture, err = BuildFromFixtureFile(fs, apiKey, stripeAccount, baseURL, file, skip, override, param, add, remove, edit) if err != nil { return nil, err } @@ -162,7 +235,7 @@ func Trigger(ctx context.Context, event string, stripeAccount string, baseURL st return nil, fmt.Errorf("%s", fmt.Sprintf("The event `%s` is not supported by Stripe CLI. To trigger unsupported events, use the Stripe API or Dashboard to perform actions that lead to the event you want to trigger (for example, create a Customer to generate a `customer.created` event). You can also create a custom fixture: https://docs.stripe.com/cli/fixtures", event)) } - fixture, err = BuildFromFixtureFile(fs, apiKey, stripeAccount, baseURL, event, skip, override, add, remove, edit) + fixture, err = BuildFromFixtureFile(fs, apiKey, stripeAccount, baseURL, event, skip, override, param, add, remove, edit) if err != nil { return nil, err } @@ -190,3 +263,87 @@ func reverseMap() map[string]string { return reversed } + +// ValidateRequiredParams checks if all required parameters specified in fixture metadata +// have been provided via the --param flag. Returns an actionable error if any are missing. +func ValidateRequiredParams(fixture *Fixture, jsonFile string, providedParams []string) error { + requiredParams := fixture.FixtureData.Meta.RequiredParams + + // Look up event name from file path for error messages + eventName := reverseMap()[jsonFile] + if eventName == "" { + eventName = "" + } + + // If params were provided but none are required, show helpful error suggesting --override + if len(requiredParams) == 0 && len(providedParams) > 0 { + color := ansi.Color(os.Stdout) + return fmt.Errorf("%s\n\nThis trigger does not accept required parameters.\n\nIf you're trying to customize fixture values, use --override instead:\n stripe trigger --override %s", + color.Red("✘ Unexpected parameters").String(), + providedParams[0]) // Show first param as example + } + + if len(requiredParams) == 0 { + return nil // No required params, nothing to validate + } + + // Parse provided params by fixture path + // Format: "fixtureName:path.to.field=value" (same as --override) + providedParamNames := make(map[string]bool) + for _, param := range providedParams { + parts := strings.SplitN(param, "=", 2) + if len(parts) != 2 { + color := ansi.Color(os.Stdout) + return fmt.Errorf("%s\n\nInvalid parameter format: %s\n\nParameters must use the format: fixtureName:path.to.field=value\n\nExample:\n --param charge:transfer_data.destination=acct_123", + color.Red("✘ Malformed parameter").String(), + param) + } + if parts[0] == "" { + color := ansi.Color(os.Stdout) + return fmt.Errorf("%s\n\nParameter name cannot be empty: %s\n\nParameters must use the format: fixtureName:path.to.field=value", + color.Red("✘ Malformed parameter").String(), + param) + } + if parts[1] == "" { + color := ansi.Color(os.Stdout) + return fmt.Errorf("%s\n\nParameter value cannot be empty: %s\n\nIf you want to set an empty value, use --override instead:\n --override %s=", + color.Red("✘ Malformed parameter").String(), + param, + parts[0]) + } + providedParamNames[parts[0]] = true + } + + // Check if all required params were provided + var missingParams []RequiredParam + for _, required := range requiredParams { + if !providedParamNames[required.Name] { + missingParams = append(missingParams, required) + } + } + + if len(missingParams) > 0 { + // Build actionable error message + color := ansi.Color(os.Stdout) + var errorMsg strings.Builder + + errorMsg.WriteString(color.Red("✘ Missing required parameters").String()) + errorMsg.WriteString("\n") + + for _, param := range missingParams { + errorMsg.WriteString(fmt.Sprintf("\n %s - %s\n", color.Bold(param.Name).String(), param.Description)) + errorMsg.WriteString(" Example:\n\n") + errorMsg.WriteString(fmt.Sprintf(" stripe trigger %s \\\n", eventName)) + + placeholderValue := param.Placeholder + if placeholderValue == "" { + placeholderValue = "VALUE" + } + errorMsg.WriteString(fmt.Sprintf(" --param %s=%s\n", param.Name, placeholderValue)) + } + + return fmt.Errorf("%s", errorMsg.String()) + } + + return nil +} diff --git a/pkg/fixtures/triggers/application_fee.created.json b/pkg/fixtures/triggers/application_fee.created.json new file mode 100644 index 000000000..2dbe50b48 --- /dev/null +++ b/pkg/fixtures/triggers/application_fee.created.json @@ -0,0 +1,29 @@ +{ + "_meta": { + "template_version": 0, + "required_params": [ + { + "name": "charge:transfer_data.destination", + "description": "Connect account ID with transfers capability enabled", + "placeholder": "acct_1ABC234DEF567GHI" + } + ] + }, + "fixtures": [ + { + "name": "charge", + "path": "/v1/charges", + "method": "post", + "params": { + "amount": 1000, + "currency": "usd", + "source": "tok_visa", + "description": "(created by Stripe CLI)", + "transfer_data": { + "destination": "{{CONNECT_ACCOUNT_ID}}" + }, + "application_fee_amount": 100 + } + } + ] +} diff --git a/pkg/fixtures/triggers/application_fee.refund.updated.json b/pkg/fixtures/triggers/application_fee.refund.updated.json new file mode 100644 index 000000000..38508c451 --- /dev/null +++ b/pkg/fixtures/triggers/application_fee.refund.updated.json @@ -0,0 +1,43 @@ +{ + "_meta": { + "template_version": 0, + "required_params": [ + { + "name": "charge:transfer_data.destination", + "description": "Connect account ID with transfers capability enabled", + "placeholder": "acct_1ABC234DEF567GHI" + } + ] + }, + "fixtures": [ + { + "name": "charge", + "path": "/v1/charges", + "method": "post", + "params": { + "amount": 1000, + "currency": "usd", + "source": "tok_visa", + "description": "(created by Stripe CLI)", + "transfer_data": { + "destination": "{{CONNECT_ACCOUNT_ID}}" + }, + "application_fee_amount": 100 + } + }, + { + "name": "fee_refund", + "path": "/v1/application_fees/${charge:application_fee}/refunds", + "method": "post", + "params": {} + }, + { + "name": "updated_fee_refund", + "path": "/v1/application_fees/${charge:application_fee}/refunds/${fee_refund:id}", + "method": "post", + "params": { + "metadata": { "order_id": "6735" } + } + } + ] +} diff --git a/pkg/fixtures/triggers/application_fee.refunded.json b/pkg/fixtures/triggers/application_fee.refunded.json new file mode 100644 index 000000000..68e62c5c7 --- /dev/null +++ b/pkg/fixtures/triggers/application_fee.refunded.json @@ -0,0 +1,35 @@ +{ + "_meta": { + "template_version": 0, + "required_params": [ + { + "name": "charge:transfer_data.destination", + "description": "Connect account ID with transfers capability enabled", + "placeholder": "acct_1ABC234DEF567GHI" + } + ] + }, + "fixtures": [ + { + "name": "charge", + "path": "/v1/charges", + "method": "post", + "params": { + "amount": 1000, + "currency": "usd", + "source": "tok_visa", + "description": "(created by Stripe CLI)", + "transfer_data": { + "destination": "{{CONNECT_ACCOUNT_ID}}" + }, + "application_fee_amount": 100 + } + }, + { + "name": "fee_refund", + "path": "/v1/application_fees/${charge:application_fee}/refunds", + "method": "post", + "params": {} + } + ] +} diff --git a/pkg/fixtures/triggers_test.go b/pkg/fixtures/triggers_test.go new file mode 100644 index 000000000..50133c439 --- /dev/null +++ b/pkg/fixtures/triggers_test.go @@ -0,0 +1,387 @@ +package fixtures + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateRequiredParams(t *testing.T) { + t.Run("no required params and no provided params", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{}, + }, + }, + } + + err := ValidateRequiredParams(fxt, "", []string{}) + require.NoError(t, err) + }) + + t.Run("no required params but params provided shows helpful error", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{}, + }, + }, + } + + err := ValidateRequiredParams(fxt, "", []string{"charge:amount=1000"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "Unexpected parameters") + assert.Contains(t, err.Error(), "use --override instead") + assert.Contains(t, err.Error(), "charge:amount=1000") + }) + + t.Run("all required params provided", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{ + { + Name: "charge:transfer_data.destination", + Description: "Connect account ID", + Placeholder: "acct_123", + }, + }, + }, + }, + } + + err := ValidateRequiredParams(fxt, "", []string{"charge:transfer_data.destination=acct_test"}) + require.NoError(t, err) + }) + + t.Run("missing required param shows actionable error", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{ + { + Name: "charge:transfer_data.destination", + Description: "Connect account ID", + Placeholder: "acct_123", + }, + }, + }, + }, + } + + err := ValidateRequiredParams(fxt, "", []string{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required parameters") + assert.Contains(t, err.Error(), "charge:transfer_data.destination") + assert.Contains(t, err.Error(), "Connect account ID") + assert.Contains(t, err.Error(), "acct_123") + }) + + t.Run("multiple required params all provided", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{ + { + Name: "charge:transfer_data.destination", + Description: "Connect account ID", + Placeholder: "acct_123", + }, + { + Name: "charge:amount", + Description: "Charge amount", + Placeholder: "1000", + }, + }, + }, + }, + } + + err := ValidateRequiredParams(fxt, "", []string{ + "charge:transfer_data.destination=acct_test", + "charge:amount=2000", + }) + require.NoError(t, err) + }) + + t.Run("multiple required params partially provided", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{ + { + Name: "charge:transfer_data.destination", + Description: "Connect account ID", + Placeholder: "acct_123", + }, + { + Name: "charge:amount", + Description: "Charge amount", + Placeholder: "1000", + }, + }, + }, + }, + } + + err := ValidateRequiredParams(fxt, "", []string{"charge:amount=2000"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required parameters") + assert.Contains(t, err.Error(), "charge:transfer_data.destination") + assert.NotContains(t, err.Error(), "charge:amount") // amount was provided + }) + + t.Run("malformed param missing equals sign", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{ + { + Name: "charge:amount", + Description: "Charge amount", + Placeholder: "1000", + }, + }, + }, + }, + } + + err := ValidateRequiredParams(fxt, "", []string{"charge:amount"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "Malformed parameter") + assert.Contains(t, err.Error(), "fixtureName:path.to.field=value") + }) + + t.Run("malformed param with empty name", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{ + { + Name: "charge:amount", + Description: "Charge amount", + Placeholder: "1000", + }, + }, + }, + }, + } + + err := ValidateRequiredParams(fxt, "", []string{"=value"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "Malformed parameter") + assert.Contains(t, err.Error(), "cannot be empty") + }) + + t.Run("malformed param with empty value", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{ + { + Name: "charge:amount", + Description: "Charge amount", + Placeholder: "1000", + }, + }, + }, + }, + } + + err := ValidateRequiredParams(fxt, "", []string{"charge:amount="}) + require.Error(t, err) + assert.Contains(t, err.Error(), "Malformed parameter") + assert.Contains(t, err.Error(), "value cannot be empty") + assert.Contains(t, err.Error(), "use --override instead") + }) + + t.Run("param with special characters in value", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{ + { + Name: "charge:description", + Description: "Charge description", + Placeholder: "Test charge", + }, + }, + }, + }, + } + + err := ValidateRequiredParams(fxt, "", []string{"charge:description=Test & Co."}) + require.NoError(t, err) + }) + + t.Run("param with equals sign in value", func(t *testing.T) { + fxt := &Fixture{ + FixtureData: FixtureData{ + Meta: MetaFixture{ + RequiredParams: []RequiredParam{ + { + Name: "charge:metadata.key", + Description: "Metadata key", + Placeholder: "value", + }, + }, + }, + }, + } + + // SplitN with 2 should handle this correctly + err := ValidateRequiredParams(fxt, "", []string{"charge:metadata.key=value=with=equals"}) + require.NoError(t, err) + }) +} + +// Integration tests for param flow through BuildFromFixtureFile +func TestBuildFromFixtureFileWithParams(t *testing.T) { + const fixtureWithRequiredParams = ` +{ + "_meta": { + "template_version": 0, + "required_params": [ + { + "name": "charge:transfer_data.destination", + "description": "Connect account ID", + "placeholder": "acct_123" + } + ] + }, + "fixtures": [ + { + "name": "charge", + "path": "/v1/charges", + "method": "post", + "params": { + "amount": 1000, + "currency": "usd", + "source": "tok_visa", + "transfer_data": { + "destination": "{{CONNECT_ACCOUNT_ID}}" + } + } + } + ] +} +` + + t.Run("missing required param fails before API call", func(t *testing.T) { + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "test.json", []byte(fixtureWithRequiredParams), 0644) + + _, err := BuildFromFixtureFile( + fs, + "sk_test_123", + "", + "http://localhost", + "test.json", + []string{}, // skip + []string{}, // override + []string{}, // param - missing! + []string{}, // add + []string{}, // remove + false, // edit + ) + + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required parameters") + assert.Contains(t, err.Error(), "charge:transfer_data.destination") + }) + + t.Run("required param provided succeeds", func(t *testing.T) { + // Set up mock server + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"id": "ch_123", "object": "charge"}`)) + })) + defer ts.Close() + + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "test.json", []byte(fixtureWithRequiredParams), 0644) + + fixture, err := BuildFromFixtureFile( + fs, + "sk_test_123", + "", + ts.URL, + "test.json", + []string{}, // skip + []string{}, // override + []string{"charge:transfer_data.destination=acct_test"}, // param provided + []string{}, // add + []string{}, // remove + false, // edit + ) + + require.NoError(t, err) + require.NotNil(t, fixture) + + // Execute the fixture to ensure param was applied + _, err = fixture.Execute(context.Background(), "") + require.NoError(t, err) + }) + + t.Run("params take precedence over override", func(t *testing.T) { + // Set up mock server + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"id": "ch_123", "object": "charge"}`)) + })) + defer ts.Close() + + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "test.json", []byte(fixtureWithRequiredParams), 0644) + + fixture, err := BuildFromFixtureFile( + fs, + "sk_test_123", + "", + ts.URL, + "test.json", + []string{}, // skip + []string{"charge:transfer_data.destination=acct_override"}, // override + []string{"charge:transfer_data.destination=acct_param"}, // param should win + []string{}, // add + []string{}, // remove + false, // edit + ) + + require.NoError(t, err) + require.NotNil(t, fixture) + + // Execute the fixture + _, err = fixture.Execute(context.Background(), "") + require.NoError(t, err) + }) + + t.Run("malformed param syntax fails validation", func(t *testing.T) { + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "test.json", []byte(fixtureWithRequiredParams), 0644) + + _, err := BuildFromFixtureFile( + fs, + "sk_test_123", + "", + "http://localhost", + "test.json", + []string{}, // skip + []string{}, // override + []string{"malformed_no_equals"}, // malformed param + []string{}, // add + []string{}, // remove + false, // edit + ) + + require.Error(t, err) + assert.Contains(t, err.Error(), "Malformed parameter") + assert.Contains(t, err.Error(), "fixtureName:path.to.field=value") + }) +} diff --git a/pkg/rpcservice/trigger.go b/pkg/rpcservice/trigger.go index 493c7171c..fd38b0fdd 100644 --- a/pkg/rpcservice/trigger.go +++ b/pkg/rpcservice/trigger.go @@ -28,6 +28,7 @@ func (srv *RPCService) Trigger(ctx context.Context, req *rpc.TriggerRequest) (*r apiKey, req.Skip, req.Override, + []string{}, // param - not yet supported in RPC, can be added later req.Add, req.Remove, req.Raw, diff --git a/scratch/pr12-description.md b/scratch/pr12-description.md new file mode 100644 index 000000000..0ad93325a --- /dev/null +++ b/scratch/pr12-description.md @@ -0,0 +1,161 @@ +## Summary + +Adds support for triggering application fee events and introduces a `--param` flag with pre-flight validation for triggers that require user-provided configuration. + +## Background + +Some Stripe events require configuration that can't be pre-filled in fixtures. For example, application fee events require a Connect account ID with the `transfers` capability enabled. Previously, these fixtures would: +- Fail at API call time with unclear errors +- Require users to manually edit fixture JSON +- Have no validation until the API request was made + +This PR addresses this by adding a `--param` flag that validates required parameters before making API calls, providing early feedback and clear error messages. + +## What's Included + +### New `--param` Flag + +Adds pre-flight validation for trigger parameters: + +```bash +# Provide required parameters validated before API calls +stripe trigger application_fee.created \ + --param charge:transfer_data.destination=acct_123 + +# Clear error if parameter is missing +stripe trigger application_fee.created +# Error: ✘ Missing required parameters +# +# charge:transfer_data.destination - Connect account ID with transfers capability enabled +# Example: +# +# stripe trigger application_fee.created \ +# --param charge:transfer_data.destination=acct_1ABC234DEF567GHI +``` + +**Features:** +- Uses same syntax as `--override`: `fixtureName:path.to.field=value` +- Validates against `required_params` metadata in fixture files +- Explicit errors for malformed syntax (missing `=`, empty values) +- Parameters take precedence over `--override` values +- Help text shows which events require parameters + +### New Application Fee Triggers + +Adds 3 new Connect-related event triggers: +- `application_fee.created` - When an application fee is created on a charge +- `application_fee.refunded` - When an application fee is fully refunded +- `application_fee.refund.updated` - When an application fee refund is updated + +All three require a Connect account ID via the new `--param` flag. + +## Implementation Details + +- **Fixture metadata**: Added `required_params` array to `_meta` block in fixture JSON + - Field renamed from `example` to `placeholder` for semantic clarity +- **Validation logic**: New `ValidateRequiredParams()` function with comprehensive error messages + - Error messages include the actual event name in usage examples + - Example error output: + ``` + ✘ Missing required parameters + + charge:transfer_data.destination - Connect account ID + Example: + + stripe trigger application_fee.created \ + --param charge:transfer_data.destination=acct_123 + ``` +- **Test coverage**: 15 tests (11 unit tests for validation logic, 4 integration tests with fixture execution) +- **Help text**: Shows parameter requirements inline with event names. Vertical alignment is calculated from events that require params only (not all events), keeping the `--param` column tight at 82 chars instead of 93. + +## Output Changes + +### `stripe trigger --help`: Event list + +Three new application fee events appear in the event list. Events that require parameters show the `--param` syntax inline, vertically aligned. Alignment is calculated only from events with params (30 chars), keeping lines at 82 chars instead of 93: + +```diff + Supported events: + account.application.deauthorized + account.updated ++ application_fee.created --param charge:transfer_data.destination= ++ application_fee.refund.updated --param charge:transfer_data.destination= ++ application_fee.refunded --param charge:transfer_data.destination= + balance.available + billing_portal.configuration.created + billing_portal.configuration.updated + billing_portal.session.created + cash_balance.funds_available +``` + +### `stripe trigger --help`: Examples section + +The examples section now includes descriptive comments, a parameterized usage example, and blank line separation between examples: + +```diff + Examples: +- stripe trigger payment_intent.created ++ # Trigger a basic event ++ stripe trigger payment_intent.created ++ ++ # Trigger an event that requires parameters ++ stripe trigger application_fee.created --param charge:transfer_data.destination=acct_123 + + Flags: + --add stringArray Add params to the trigger + --api-version string Specify API version for trigger + --edit Edit the trigger directly in your default IDE + -h, --help help for trigger +``` + +### `stripe trigger --help`: New `--param` flag + +A new `--param` flag for pre-flight parameter validation appears between `--override` and `--raw`: + +```diff + --override stringArray Override params in the trigger ++ --param stringArray Set required parameters (validated before ++ execution) + --raw string Raw fixture in string format to replace + all default fixtures + --remove stringArray Remove params from the trigger +``` + +## Spicy Decision: Params as a Layer on Top of Overrides + +This PR deliberately does **not** introduce a formal concept of "params" in the fixture system itself. Instead, `--param` is a validation layer on top of the existing `--override` mechanism — params use the exact same syntax and are merged into overrides before execution. + +This means fixture files remain unchanged at the execution level. The `_meta.required_params` block is only read by the trigger command for validation and help text. The fixture runner doesn't know the difference between a `--param` and an `--override`. + +**The question for reviewers:** Is this the right approach, or should we formalize parameterization as a first-class concept in the fixture system? + +Arguments for the current approach: +- It works today with zero changes to the fixture runner +- Users get pre-flight validation and actionable errors right now +- The `--param` / `--override` distinction is a UX concern, not a data model concern + +Arguments for formalizing params in fixtures: +- Other consumers of fixtures (not just `stripe trigger`) could benefit from the same validation +- The validation logic currently lives in `triggers.go` — if fixtures supported params natively, this would move to the fixture layer where it arguably belongs + +**What can be deferred vs. what needs to be decided now?** + +The refactoring question — whether validation lives in `triggers.go` or moves down into the fixture runner — is entirely internal. If we later want fixtures to support parameterization as a general concept, that refactoring can be done without breaking users who have already started using `--param`. The flag, its syntax, and its behavior all stay the same; only the internal plumbing moves. + +However, there are decisions in this PR that **are** hard to change later because they form the user-facing contract: + +1. **Is `--param` the right name for this concept?** Once users start scripting against it, renaming is a breaking change. + +2. **Should params use fixture JSON paths (`charge:transfer_data.destination`) or semantic names (`connected_account_id`)?** I considered semantic names earlier — they're shorter and describe the *purpose* of an input (similar to function argument names). But the downside is you lose the relationship to where that value is placed in the API call, and that relationship is a fairly central part of the fixture mental model. This PR uses fixture paths, which means users can look at the fixture JSON and see exactly where their value ends up — same as `--override`. + +## Testing + +```bash +# Run fixture tests +go test ./pkg/fixtures/... -v + +# Test the trigger command +stripe trigger application_fee.created --param charge:transfer_data.destination=acct_test123 +``` + +All tests pass. The `--param` flag provides robust validation before API calls, catching configuration errors early with actionable error messages.