fix(payments): EN-601 Use v3 with get config#104
Conversation
WalkthroughAdds V3 connector config support: the command fetches and stores V3 responses, routes rendering to version-specific renderers, and adds provider-specific DisplayXxxConfigV3 functions to render V3 configs as tables alongside legacy V1/V2 handling. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Client
participant Cmd as getconfig Command
participant API as Payments API
participant Store as Local Store
participant Renderer as Renderer
CLI->>Cmd: invoke getconfig (connector-id, version)
Cmd->>Cmd: validate args
alt version == V3
Cmd->>API: V3.GetConnectorConfig(connector-id)
API-->>Cmd: V3GetConnectorConfigResponse
Cmd->>Cmd: validate response status & derive provider
Cmd->>Store: store V3ConnectorConfig
Cmd->>Renderer: renderV3(provider, v3Config)
Renderer->>Renderer: call DisplayXxxConfigV3(v3Config)
Renderer-->>CLI: render table output
else
Cmd->>API: GetConnectorConfig(connector-id)
API-->>Cmd: ConnectorConfigResponse
Cmd->>Cmd: derive provider
Cmd->>Store: store ConnectorConfig
Cmd->>Renderer: renderV1V2(provider, config)
Renderer->>Renderer: call DisplayXxxConfig(config)
Renderer-->>CLI: render table output
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| connectorID := fctl.GetString(cmd, c.connectorIDFlag) | ||
|
|
||
| switch c.PaymentsVersion { | ||
| case versions.V3: |
There was a problem hiding this comment.
Original getConfig allows to specify only the provider, then to request the user to select the connector if there is multiple. I forced the use of connectorId for this case.
| @@ -18,9 +18,10 @@ import ( | |||
| ) | |||
|
|
|||
| type PaymentsGetConfigStore struct { | |||
There was a problem hiding this comment.
Some connectors don't have the same config in v1 and in v3. I think it's easier on the code and on the maintenance to handle the two different versions.
|
It seems there is some ongoing issue with the CI / pre-commit ( speakeasy last version being downloaded instead of using nix's and updating the lock files ) and that last PRs are being forced. Local build works well. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/payments/connectors/views/banking_circle.go (1)
15-23: Add "Polling Period" to banking_circle v1/v2 display for consistency with other connectors.The v1/v2 renderer lacks the "Polling Period" field that is present in DisplayBankingCircleConfigV3 and in both v1/v2 and v3 renderers of all other connectors (wise, stripe, qonto, moneycorp, modulr, mangopay, currency_cloud, column, atlar, adyen). This appears to be an oversight—either add it or explicitly document why banking_circle differs.
📎 Suggested addition
tableData = append(tableData, []string{pterm.LightCyan("Authorization endpoint:"), config.AuthorizationEndpoint}) +tableData = append(tableData, []string{pterm.LightCyan("Polling Period:"), fctl.StringPointerToString(config.PollingPeriod)}) tableData = append(tableData, []string{pterm.LightCyan("UserCertificate:"), config.UserCertificate})
🤖 Fix all issues with AI agents
In `@cmd/payments/connectors/configs/getconfig.go`:
- Around line 202-233: In PaymentsLoadConfigController.renderV3 change the
default branch so it sets and returns a non-nil error instead of only printing:
assign err (e.g. fmt.Errorf("unknown provider: %s", c.store.Provider)) in the
default case and return that error at the end; make the same change in the
analogous render function referenced by the reviewer (the other render path
around lines 239-263) so both render functions fail the command when the
provider is unknown. Ensure you import or use the appropriate errors/fmt package
to construct the error.
🧹 Nitpick comments (1)
cmd/payments/connectors/views/column.go (1)
19-19: Minor: Inconsistent label formatting.The label
"APIKey:"differs from other connectors which use"API key:"(with a space and lowercase 'k'). Consider aligning for consistency.Suggested fix
- tableData = append(tableData, []string{pterm.LightCyan("APIKey:"), config.APIKey}) + tableData = append(tableData, []string{pterm.LightCyan("API key:"), config.APIKey})
|
@Quentin-David-24 |
2 participants
No description provided.