Update contributing file and check snippets #528
Conversation
- Updated Plugin interface definition to show PSPPlugin and BankingBridgePlugin embedding - Fixed registry.RegisterPlugin call signature to match current implementation - Updated Plugin struct to include models.Plugin embedding and client field - Added missing imports in code examples (fmt, os, path, math/big, etc.) - Fixed New() function to properly initialize Plugin with BasePlugin and client - Updated Install/Uninstall methods to delegate to actual implementation functions - Added missing capabilities to match actual dummypay connector - Corrected import paths and function signatures throughout examples
|
Cursor Agent can help with this pull request. Just |
WalkthroughIntroduces a new PSPPlugin interface and moves webhook methods to the top-level Plugin interface (which now embeds PSPPlugin and BankingBridgePlugin). Adds two capabilities. Updates plugin struct initialization, registration signature, and install/uninstall handlers. Expands client with FetchBalance and adjusts balance paging/handling. Minor documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor PSP as PSP Consumer
participant PL as Plugin (embeds PSPPlugin)
participant CL as Client
participant DS as Data Source (balances.json)
rect rgb(240,245,255)
note over PSP,PL: FetchNextBalances flow (new client FetchBalance)
PSP->>PL: FetchNextBalances(req)
PL->>PL: Extract account context / paging state
PL->>CL: FetchBalance(ctx, accountID)
CL->>DS: Read balances.json
DS-->>CL: Raw balance data
CL-->>PL: PSPBalance (Amount: big.Int)
PL->>PL: Build paging (NextToken/HasMore)
PL-->>PSP: FetchNextBalancesResponse
end
rect rgb(245,255,245)
note over PSP,PL: Webhooks moved to Plugin level
PSP->>PL: CreateWebhooks / VerifyWebhook / TranslateWebhook / TrimWebhook
PL-->>PSP: Corresponding responses
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
CONTRIBUTING.md (4)
574-585: Pagination: HasMore condition should be >= 0next == 0 is a valid next token; use >= 0 to avoid prematurely stopping pagination.
- HasMore: next > 0, + HasMore: next >= 0,
606-633: Stateful pagination: same HasMore fixEnsure consistency with the pagination contract.
- HasMore: next > 0, + HasMore: next >= 0,
710-735: FetchBalance: avoid returning an empty balance structReturning &models.PSPBalance{} on “not found” causes the caller to append an empty balance. Return nil instead.
- return &models.PSPBalance{}, nil + return nil, nilAlso ensure a Balance struct exists:
type Balance struct { AccountID string `json:"account_id"` AmountInMinors int64 `json:"amount_in_minors"` Currency string `json:"currency"` }
824-831: Metadata examples contain typos and type mismatches
- Namespace typo: "com.string.spec/" → "com.stripe.spec/".
- Iterate over providerPayments, not payments.
- Append models.PSPPayment, not models.PSPAccount.
- Use a proper map value for Metadata.
-const ( - stripeMetadataSpecNamespace = "com.string.spec/" +const ( + stripeMetadataSpecNamespace = "com.stripe.spec/" @@ -pspPayments []models.PSPPayment{} -for _, payment := range payments { +pspPayments := []models.PSPPayment{} +for _, payment := range providerPayments { @@ - pspPayments = append(pspPayments, models.PSPAccount{ + pspPayments = append(pspPayments, models.PSPPayment{ @@ - Metadata: { - client.StripePaymentReasonMetadataKey: paymentReason, - client.StripePaymentArrivalTimeMetadataKey: paymentArrivalTime - } + Metadata: models.Metadata{ + client.StripePaymentReasonMetadataKey: paymentReason, + client.StripePaymentArrivalTimeMetadataKey: paymentArrivalTime, + }, })Also applies to: 872-888
🧹 Nitpick comments (2)
CONTRIBUTING.md (2)
258-259: Replace Notion link with repo linkExternal Notion link will be inaccessible to contributors. Link to the interface in this repo instead (as you did earlier).
-In the `plugin.go` file, add the methods required for the [Plugin interface](https://www.notion.so/Build-a-Connector-158066a5ef3280538d23c2fa239fa78a?pvs=21). +In the `plugin.go` file, add the methods required for the [Plugin interface](https://github.com/formancehq/payments/blob/main/internal/models/plugin.go).
221-224: Clarify install endpoint uses plugin name, not directory
In CONTRIBUTING.md before the connector installation example (around thecurl …/install/dummypaysnippet), add a note that the POST path/v3/connectors/install/dummypayuses the plugin slugdummypay(DummyPSPName), not thedummypay2directory name.
📜 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)
CONTRIBUTING.md(8 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CONTRIBUTING.md
50-50: Hard tabs
Column: 1
(MD010, no-hard-tabs)
51-51: Hard tabs
Column: 1
(MD010, no-hard-tabs)
53-53: Hard tabs
Column: 1
(MD010, no-hard-tabs)
54-54: Hard tabs
Column: 1
(MD010, no-hard-tabs)
55-55: Hard tabs
Column: 1
(MD010, no-hard-tabs)
56-56: Hard tabs
Column: 1
(MD010, no-hard-tabs)
58-58: Hard tabs
Column: 1
(MD010, no-hard-tabs)
59-59: Hard tabs
Column: 1
(MD010, no-hard-tabs)
60-60: Hard tabs
Column: 1
(MD010, no-hard-tabs)
61-61: Hard tabs
Column: 1
(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 1
(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 1
(MD010, no-hard-tabs)
211-211: Hard tabs
Column: 1
(MD010, no-hard-tabs)
213-213: Hard tabs
Column: 1
(MD010, no-hard-tabs)
214-214: Hard tabs
Column: 1
(MD010, no-hard-tabs)
215-215: Hard tabs
Column: 1
(MD010, no-hard-tabs)
216-216: Hard tabs
Column: 1
(MD010, no-hard-tabs)
217-217: Hard tabs
Column: 1
(MD010, no-hard-tabs)
221-221: Hard tabs
Column: 1
(MD010, no-hard-tabs)
222-222: Hard tabs
Column: 1
(MD010, no-hard-tabs)
223-223: Hard tabs
Column: 1
(MD010, no-hard-tabs)
227-227: Hard tabs
Column: 1
(MD010, no-hard-tabs)
229-229: Hard tabs
Column: 1
(MD010, no-hard-tabs)
230-230: Hard tabs
Column: 1
(MD010, no-hard-tabs)
231-231: Hard tabs
Column: 1
(MD010, no-hard-tabs)
235-235: Hard tabs
Column: 1
(MD010, no-hard-tabs)
236-236: Hard tabs
Column: 1
(MD010, no-hard-tabs)
237-237: Hard tabs
Column: 1
(MD010, no-hard-tabs)
238-238: Hard tabs
Column: 1
(MD010, no-hard-tabs)
240-240: Hard tabs
Column: 1
(MD010, no-hard-tabs)
241-241: Hard tabs
Column: 1
(MD010, no-hard-tabs)
243-243: Hard tabs
Column: 1
(MD010, no-hard-tabs)
244-244: Hard tabs
Column: 1
(MD010, no-hard-tabs)
245-245: Hard tabs
Column: 1
(MD010, no-hard-tabs)
372-372: Hard tabs
Column: 1
(MD010, no-hard-tabs)
382-382: Hard tabs
Column: 1
(MD010, no-hard-tabs)
469-469: Hard tabs
Column: 1
(MD010, no-hard-tabs)
470-470: Hard tabs
Column: 1
(MD010, no-hard-tabs)
488-488: Hard tabs
Column: 1
(MD010, no-hard-tabs)
489-489: Hard tabs
Column: 1
(MD010, no-hard-tabs)
490-490: Hard tabs
Column: 1
(MD010, no-hard-tabs)
491-491: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (5)
CONTRIBUTING.md (5)
167-191: Config snippet LGTMClear config type with validation and informative errors. No issues.
468-471: account.go: remove unused importmath/big isn’t used in account.go and will fail go build.
-import ( - "math/big" - "time" -) +import "time"Likely an incorrect or invalid review comment.
141-145: CONTRIBUTING.md: replace hard-tab indentation with spaces
Verified thatCAPABILITY_ALLOW_FORMANCE_ACCOUNT_CREATIONandCAPABILITY_ALLOW_FORMANCE_PAYMENT_CREATIONare defined in internal/models. Updated indentation to two spaces to satisfy markdownlint (MD010):- models.CAPABILITY_ALLOW_FORMANCE_ACCOUNT_CREATION, - models.CAPABILITY_ALLOW_FORMANCE_PAYMENT_CREATION, + models.CAPABILITY_ALLOW_FORMANCE_ACCOUNT_CREATION, + models.CAPABILITY_ALLOW_FORMANCE_PAYMENT_CREATION,
64-79: PSPPlugin signatures verified: The method signatures in CONTRIBUTING.md match exactly those in internal/models/plugin_psp.go. No drift detected.
50-62: Add missing webhook methods to docs table and use commit permalink
- Append rows for
TrimWebhookandVerifyWebhookin the “Methods” table in CONTRIBUTING.md.- Replace the branch-specific line-number link to internal/models/plugin.go with a commit-hash permalink to prevent drift.
| models.Plugin | ||
|
|
||
| name string | ||
| logger logging.Logger | ||
| client client.Client | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Embed BasePlugin (concrete) rather than models.Plugin (interface)
Embedding an interface in a struct does not promote methods; embed the concrete BasePlugin to inherit helpers. Also keeps the New() assignment type-safe and idiomatic.
Apply:
-type Plugin struct {
- models.Plugin
-
- name string
- logger logging.Logger
- client client.Client
-}
+type Plugin struct {
+ *plugins.BasePlugin
+ name string
+ logger logging.Logger
+ client client.Client
+}
@@
- return &Plugin{
- Plugin: plugins.NewBasePlugin(),
-
- name: name,
- logger: logger,
- client: client.New(conf.Directory),
- }, nil
+ return &Plugin{
+ BasePlugin: plugins.NewBasePlugin(),
+ name: name,
+ logger: logger,
+ client: client.New(conf.Directory),
+ }, nilAlso applies to: 241-246
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
227-227: Hard tabs
Column: 1
(MD010, no-hard-tabs)
229-229: Hard tabs
Column: 1
(MD010, no-hard-tabs)
230-230: Hard tabs
Column: 1
(MD010, no-hard-tabs)
231-231: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In CONTRIBUTING.md around lines 227-233 (and similarly 241-246), the struct
currently embeds the interface models.Plugin; replace that embedded interface
with the concrete BasePlugin type (e.g., BasePlugin or models.BasePlugin) so
methods are promoted and New() assignments remain type-safe and idiomatic;
update the struct field/type name and any imports/usages accordingly and ensure
constructors/assignments initialize the concrete BasePlugin rather than the
interface.
| func (p *Plugin) Install(ctx context.Context, req models.InstallRequest) (models.InstallResponse, error) { | ||
| return p.install(ctx, req) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Install calls missing helper; provide implementation
p.install is referenced but not defined in the tutorial. Add it or inline the logic so readers can compile/run.
Here’s a minimal helper to include elsewhere in the same file:
func (p *Plugin) install(ctx context.Context, req models.InstallRequest) (models.InstallResponse, error) {
return models.InstallResponse{Workflow: workflow()}, nil
}🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
372-372: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In CONTRIBUTING.md around lines 371 to 374, the method Plugin.Install calls
p.install which is not defined in the tutorial; add the missing helper or inline
its logic so the code compiles and runs. Implement a simple install helper on
the same file that accepts (ctx context.Context, req models.InstallRequest) and
returns (models.InstallResponse, error) with a response that sets Workflow to
workflow() and nil error, or replace the p.install call by equivalent inline
logic that returns models.InstallResponse{Workflow: workflow()}, nil.
| func (p *Plugin) Uninstall(ctx context.Context, req models.UninstallRequest) (models.UninstallResponse, error) { | ||
| return p.uninstall(ctx, req) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Uninstall calls missing helper; provide implementation
Same concern for p.uninstall; add a minimal implementation.
func (p *Plugin) uninstall(ctx context.Context, req models.UninstallRequest) (models.UninstallResponse, error) {
return models.UninstallResponse{}, nil
}🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
382-382: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In CONTRIBUTING.md around lines 381 to 384, the public Uninstall method
delegates to a missing helper p.uninstall; add a minimal implementation for
p.uninstall that matches the signature (accepts context.Context and
models.UninstallRequest and returns models.UninstallResponse, error) and returns
an empty models.UninstallResponse and nil error to satisfy the callsite and
compile; place the helper near the other plugin methods and ensure it uses the
same receiver (*Plugin).
| "encoding/json" | ||
| "fmt" | ||
| "os" | ||
| "path" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
client.go: use filepath + os.ReadFile; fix error text and short-read risk
path.Join is for URL paths; prefer filepath.Join. os.ReadFile simplifies reading and avoids short reads; also correct “failed to create” → “failed to open”.
@@
-import (
- "context"
- "encoding/json"
- "fmt"
- "os"
- "path"
-)
+import (
+ "context"
+ "encoding/json"
+ "fmt"
+ "os"
+ "path/filepath"
+)
@@
-func (c *client) readFile(filename string) (b []byte, err error) {
- filePath := path.Join(c.directory, filename)
- file, err := os.Open(filePath)
- if err != nil {
- return b, fmt.Errorf("failed to create %q: %w", filePath, err)
- }
- defer file.Close()
-
- fileInfo, err := file.Stat()
- if err != nil {
- return b, fmt.Errorf("failed to stat file %q: %w", filePath, err)
- }
-
- buf := make([]byte, fileInfo.Size())
- _, err = file.Read(buf)
- if err != nil {
- return b, fmt.Errorf("failed to read file %q: %w", filePath, err)
- }
- return buf, nil
-}
+func (c *client) readFile(filename string) ([]byte, error) {
+ filePath := filepath.Join(c.directory, filename)
+ b, err := os.ReadFile(filePath)
+ if err != nil {
+ return nil, fmt.Errorf("failed to open %q: %w", filePath, err)
+ }
+ return b, nil
+}Also applies to: 547-566
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
488-488: Hard tabs
Column: 1
(MD010, no-hard-tabs)
489-489: Hard tabs
Column: 1
(MD010, no-hard-tabs)
490-490: Hard tabs
Column: 1
(MD010, no-hard-tabs)
491-491: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In CONTRIBUTING.md around lines 488-492 (and similarly 547-566) change uses of
path.Join to filepath.Join by importing "path/filepath" instead of "path";
replace manual os.Open/read into os.ReadFile to avoid short-read logic and
simplify error handling; update error messages from "failed to create" to
"failed to open" where appropriate and return/log the read error directly;
ensure imports are updated (remove "path", add "path/filepath") and adjust call
sites to use filepath.Join and the []byte result from os.ReadFile.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #528 +/- ##
==========================================
- Coverage 66.71% 66.70% -0.01%
==========================================
Files 761 761
Lines 39754 39754
==========================================
- Hits 26522 26519 -3
- Misses 11846 11848 +2
- Partials 1386 1387 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Update
CONTRIBUTING.mdto reflect the current codebase and provide accurate guidance for new connector development.The existing
CONTRIBUTING.mdcontained outdated plugin interface definitions, incorrect registry registration calls, missing webhook methods, and several inaccurate code snippets and import paths. This PR brings the documentation in line with the currentpaymentsservice architecture andgo-libsusage.