-
Notifications
You must be signed in to change notification settings - Fork 0
fix: Adyen installation with V3 #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds version-aware Adyen connector installation: introduces PaymentsVersion and SetVersion, resolves payments version at runtime, reads the script once, and dispatches to V3 or legacy (V0/V1/V2) install flows with appropriate request types and response handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as fctl CLI
participant Ver as versions.GetPaymentsVersion
participant APIv3 as Payments.V3
participant APIv1 as Payments.V1
participant Store as store
User->>CLI: fctl payments connectors install adyen
CLI->>Ver: GetPaymentsVersion(cmd,args,c)
CLI->>CLI: Read script
alt Version == V3
CLI->>CLI: Unmarshal -> shared.V3AdyenConfig
CLI->>APIv3: InstallConnector(V3InstallConnectorRequest)
alt 2xx
APIv3-->>CLI: V3InstallConnectorResponse
CLI->>Store: set Success, Connector, ConnectorID
else error
APIv3-->>CLI: Error
CLI->>User: Report error
end
else V0/V1/V2
CLI->>CLI: Unmarshal -> shared.AdyenConfig
CLI->>APIv1: InstallConnector(InstallConnectorRequest)
alt 2xx
APIv1-->>CLI: ConnectorResponse
CLI->>Store: set Success, Connector, ConnectorID
else error
APIv1-->>CLI: Error
CLI->>User: Report error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNone found. Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cmd/payments/connectors/install/adyen.go (2)
83-90
: Verify connector identifier casing for V3 path; prefer constants for consistencyYou set both Type and Connector to "Adyen". API/SDKs often require a specific casing or enum for these values. To avoid subtle 400s, prefer constants where available and keep the path param consistent with other commands (internal.AdyenConnector is "adyen").
Proposed minimal change for the path parameter (leave Type as-is unless a proper enum exists in shared):
- response, err := store.Client().Payments.V3.InstallConnector(cmd.Context(), operations.V3InstallConnectorRequest{ + response, err := store.Client().Payments.V3.InstallConnector(cmd.Context(), operations.V3InstallConnectorRequest{ V3InstallConnectorRequest: &shared.V3InstallConnectorRequest{ V3AdyenConfig: &config, Type: "Adyen", }, - Connector: "Adyen", + Connector: internal.AdyenConnector, })Please also check if shared has a typed enum for V3 connector Type (e.g., shared.ConnectorTypeAdyen). If so, use it instead of the raw string.
101-103
: Defensive nil-check on V3 response before accessing dataIf V3InstallConnectorResponse is nil, calling GetData() could risk a panic depending on the codegen. A small guard is safer.
- if response.V3InstallConnectorResponse.GetData() != "" { - c.store.ConnectorID = response.V3InstallConnectorResponse.GetData() - } + if response.V3InstallConnectorResponse != nil { + if id := response.V3InstallConnectorResponse.GetData(); id != "" { + c.store.ConnectorID = id + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
cmd/payments/connectors/install/adyen.go
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
cmd/payments/connectors/install/adyen.go (2)
cmd/payments/versions/versions.go (5)
GetPaymentsVersion
(24-52)V3
(17-17)V0
(14-14)V2
(16-16)V1
(15-15)cmd/payments/connectors/internal/connectors.go (1)
AdyenConnector
(4-4)
🪛 GitHub Actions: Default
cmd/payments/connectors/install/adyen.go
[error] 3-8: Unstaged changes detected in repository: modified file cmd/payments/connectors/install/adyen.go. Commit or stash changes to continue.
🔇 Additional comments (6)
cmd/payments/connectors/install/adyen.go (6)
6-6
: Importing versions package — LGTMThis aligns with the new version-aware install flow.
24-31
: Controller version state and setter — LGTMAdding PaymentsVersion and SetVersion cleanly implements the versions.VersionController contract.
67-70
: Runtime selection of payments version — LGTMThis ensures the install path matches the server’s version instead of defaulting to V1.
71-75
: Read script once and reuse — LGTMAvoids duplicate I/O across version branches.
113-118
: Legacy (V0/V1/V2) request payload — LGTMCorrect usage of ConnectorConfig.AdyenConfig and shared.ConnectorAdyen.
1-1
: Configure Git identity and commit changesIt looks like CI is still failing due to unstaged modifications in
cmd/payments/connectors/install/adyen.go
, but your local git commit failed because your user identity isn’t configured. Please:
- Set your git name and email:
git config --global user.name "Your Name" git config --global user.email "[email protected]"
- Stage and commit the file:
git add cmd/payments/connectors/install/adyen.go git commit -m "fix: adyen v3 install flow and version routing"
- Push your branch and rerun CI to confirm the issue is resolved.
case versions.V0: | ||
case versions.V2: | ||
case versions.V1: | ||
var config shared.AdyenConfig | ||
if err := json.Unmarshal([]byte(script), &config); err != nil { | ||
return nil, err | ||
} | ||
|
||
response, err := store.Client().Payments.V1.InstallConnector(cmd.Context(), operations.InstallConnectorRequest{ | ||
ConnectorConfig: shared.ConnectorConfig{ | ||
AdyenConfig: &config, | ||
}, | ||
Connector: shared.ConnectorAdyen, | ||
}) | ||
if err != nil { | ||
return nil, errors.Wrap(err, "installing connector") | ||
} | ||
if response.StatusCode >= 300 { | ||
return nil, fmt.Errorf("unexpected status code: %d", response.StatusCode) | ||
} | ||
|
||
c.store.Success = true | ||
c.store.ConnectorName = internal.AdyenConnector | ||
|
||
if response.ConnectorResponse != nil { | ||
c.store.ConnectorID = response.ConnectorResponse.Data.ConnectorID | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Bug: V0 and V2 cases do nothing (no implicit fallthrough in Go)
In Go, switch cases don’t fall through by default. As written, versions.V0 and versions.V2 will execute no code and silently skip installation. Combine cases and add a default to future-proof.
Apply this diff to group legacy versions and add a default branch:
@@
- case versions.V0:
- case versions.V2:
- case versions.V1:
+ case versions.V0, versions.V2, versions.V1:
var config shared.AdyenConfig
if err := json.Unmarshal([]byte(script), &config); err != nil {
return nil, err
}
response, err := store.Client().Payments.V1.InstallConnector(cmd.Context(), operations.InstallConnectorRequest{
ConnectorConfig: shared.ConnectorConfig{
AdyenConfig: &config,
},
Connector: shared.ConnectorAdyen,
})
if err != nil {
return nil, errors.Wrap(err, "installing connector")
}
if response.StatusCode >= 300 {
return nil, fmt.Errorf("unexpected status code: %d", response.StatusCode)
}
c.store.Success = true
c.store.ConnectorName = internal.AdyenConnector
if response.ConnectorResponse != nil {
c.store.ConnectorID = response.ConnectorResponse.Data.ConnectorID
}
+ default:
+ return nil, fmt.Errorf("unsupported payments version: %d", c.PaymentsVersion)
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
case versions.V0: | |
case versions.V2: | |
case versions.V1: | |
var config shared.AdyenConfig | |
if err := json.Unmarshal([]byte(script), &config); err != nil { | |
return nil, err | |
} | |
response, err := store.Client().Payments.V1.InstallConnector(cmd.Context(), operations.InstallConnectorRequest{ | |
ConnectorConfig: shared.ConnectorConfig{ | |
AdyenConfig: &config, | |
}, | |
Connector: shared.ConnectorAdyen, | |
}) | |
if err != nil { | |
return nil, errors.Wrap(err, "installing connector") | |
} | |
if response.StatusCode >= 300 { | |
return nil, fmt.Errorf("unexpected status code: %d", response.StatusCode) | |
} | |
c.store.Success = true | |
c.store.ConnectorName = internal.AdyenConnector | |
if response.ConnectorResponse != nil { | |
c.store.ConnectorID = response.ConnectorResponse.Data.ConnectorID | |
} | |
} | |
case versions.V0, versions.V2, versions.V1: | |
var config shared.AdyenConfig | |
if err := json.Unmarshal([]byte(script), &config); err != nil { | |
return nil, err | |
} | |
response, err := store.Client().Payments.V1.InstallConnector(cmd.Context(), operations.InstallConnectorRequest{ | |
ConnectorConfig: shared.ConnectorConfig{ | |
AdyenConfig: &config, | |
}, | |
Connector: shared.ConnectorAdyen, | |
}) | |
if err != nil { | |
return nil, errors.Wrap(err, "installing connector") | |
} | |
if response.StatusCode >= 300 { | |
return nil, fmt.Errorf("unexpected status code: %d", response.StatusCode) | |
} | |
c.store.Success = true | |
c.store.ConnectorName = internal.AdyenConnector | |
if response.ConnectorResponse != nil { | |
c.store.ConnectorID = response.ConnectorResponse.Data.ConnectorID | |
} | |
default: | |
return nil, fmt.Errorf("unsupported payments version: %d", c.PaymentsVersion) | |
} |
🤖 Prompt for AI Agents
In cmd/payments/connectors/install/adyen.go around lines 105 to 132, the switch
handles versions.V0 and versions.V2 as empty cases so they do nothing because Go
cases don't fall through; combine the cases so V0, V1 and V2 execute the same
installation logic (e.g., case versions.V0, versions.V1, versions.V2:) and
remove empty branches, and add a default branch that returns a clear error for
unsupported/unknown versions to future-proof the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cmd/payments/connectors/install/adyen.go (1)
106-130
: Legacy versions grouped correctly.Thanks for addressing the earlier fallthrough issue by combining V0/V1/V2 into one branch.
🧹 Nitpick comments (4)
cmd/payments/connectors/install/adyen.go (4)
25-26
: Consider making PaymentsVersion unexported.Unless external packages must mutate it directly, prefer an unexported field (e.g., paymentsVersion) and keep SetVersion as the mutation point.
77-131
: Add a default branch for future-proofing unsupported versions.Your switch covers V0/V1/V2 and V3. A default branch will fail fast if a new enum value is introduced.
Apply this diff to add a default case:
switch c.PaymentsVersion { case versions.V3: ... case versions.V0, versions.V1, versions.V2: ... - } + default: + return nil, fmt.Errorf("unsupported payments version: %d", c.PaymentsVersion) + }
80-83
: Wrap unmarshal error with context (V3).Adds clarity to user-facing errors when the provided config JSON is invalid.
Apply this diff:
- if err := json.Unmarshal([]byte(script), &config); err != nil { - return nil, err - } + if err := json.Unmarshal([]byte(script), &config); err != nil { + return nil, errors.Wrap(err, "invalid V3 Adyen config") + }
108-111
: Wrap unmarshal error with context (legacy V0/V1/V2).Improves diagnostics for configuration parse failures.
Apply this diff:
- if err := json.Unmarshal([]byte(script), &config); err != nil { - return nil, err - } + if err := json.Unmarshal([]byte(script), &config); err != nil { + return nil, errors.Wrap(err, "invalid legacy Adyen config") + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
cmd/payments/connectors/install/adyen.go
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
cmd/payments/connectors/install/adyen.go (2)
cmd/payments/versions/versions.go (5)
GetPaymentsVersion
(24-52)V3
(17-17)V0
(14-14)V1
(15-15)V2
(16-16)cmd/payments/connectors/internal/connectors.go (1)
AdyenConnector
(4-4)
🔇 Additional comments (4)
cmd/payments/connectors/install/adyen.go (4)
7-8
: Importing versions package is appropriate.Needed for runtime version resolution; no issues.
30-33
: SetVersion implementation looks good.Meets the VersionController contract and keeps the versioning concern isolated in the controller.
68-71
: Resolving payments version before dispatch is correct.Good call to derive version once and use it to route the install flow.
72-75
: Reading the script once is the right refactor.Avoids duplicate IO across version branches.
Adds compatibility for Connectivity V3 module and Adyen connector.