-
Notifications
You must be signed in to change notification settings - Fork 7
feat(psu): add payment service user entity #424
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces comprehensive support for "Payment Service Users" (PSUs) across the API, backend, storage, SDK, and documentation. It adds new database tables and storage logic for PSUs, including encrypted personal and contact details, and their association with bank accounts. The backend and service layers are extended with PSU CRUD and association methods, and new API endpoints (with handlers and validation) are provided for creating, listing, retrieving, associating bank accounts, and forwarding bank accounts to connectors. The engine and workflow logic is updated to handle bank account forwarding with enriched metadata from PSUs. The SDK and documentation are updated to expose these new features, and extensive tests are implemented at both unit and end-to-end levels. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Service
participant Storage
participant Engine
participant Connector
Client->>API: POST /v3/payment-service-users (create PSU)
API->>Service: PaymentServiceUsersCreate
Service->>Storage: PaymentServiceUsersCreate
Storage-->>Service: PSU created
Service-->>API: Success
API-->>Client: 201 Created
Client->>API: POST /v3/payment-service-users/{psuID}/bank-accounts/{bankAccountID}
API->>Service: PaymentServiceUsersAddBankAccount
Service->>Storage: PaymentServiceUsersAddBankAccount
Storage-->>Service: Success
Service-->>API: Success
API-->>Client: 204 No Content
Client->>API: POST /v3/payment-service-users/{psuID}/bank-accounts/{bankAccountID}/forward
API->>Service: PaymentServiceUsersForwardBankAccountToConnector
Service->>Storage: BankAccountsGet (with sensitive data)
Storage-->>Service: BankAccount
Service->>Storage: PaymentServiceUsersGet
Storage-->>Service: PSU
Service->>Engine: ForwardBankAccount (BankAccount enriched with PSU info)
Engine->>Connector: Forward to PSP
Engine-->>Service: TaskID
Service-->>API: TaskID
API-->>Client: 202 Accepted (taskID)
Assessment against linked issues
Poem
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 18
🔭 Outside diff range comments (1)
pkg/client/models/components/v3createpaymentserviceuserrequest.go (1)
1-47
: 💡 Verification agent🧩 Analysis chain
Verify spelling of 'PhoneNuber' in related V3ContactDetailsRequest struct.
Based on the relevant code snippets, there appears to be a typo in the related V3ContactDetailsRequest struct:
PhoneNuber
should bePhoneNumber
.
🏁 Script executed:
#!/bin/bash # Check the spelling in the V3ContactDetailsRequest struct cat pkg/client/models/components/v3contactdetailsrequest.goLength of output: 506
Fix typo ‘PhoneNuber’ to ‘PhoneNumber’ in V3ContactDetailsRequest
The
V3ContactDetailsRequest
struct and its getter currently use “PhoneNuber” (andjson:"phoneNuber"
). This appears to be a typo. Please update both the field name, JSON tag, and getter method:• File:
pkg/client/models/components/v3contactdetailsrequest.go
- Rename field and tag
- PhoneNuber *string
json:"phoneNuber,omitempty"
- PhoneNumber *string
json:"phoneNumber,omitempty"
- Rename getter
- func (o *V3ContactDetailsRequest) GetPhoneNuber() *string {
- func (o *V3ContactDetailsRequest) GetPhoneNumber() *string {
• Ensure any callers or downstream code are updated to use
GetPhoneNumber()
and the corrected JSON key.
🧹 Nitpick comments (22)
pkg/client/docs/models/components/v3contactdetails.md (1)
8-9
: Fix Markdown formatting of the Type column
The entries**string*
have mismatched asterisks. To match the styling in other docs, use italicized code or plain code, e.g.*string*
or`string`
.pkg/client/docs/models/components/v3getpaymentserviceuserresponse.md (1)
1-8
: Documentation looks good but add a description for Data field.The documentation structure is clear, but consider adding a more descriptive explanation for the
Data
field instead of "N/A", such as "Contains the payment service user details".-| `Data` | [components.V3PaymentServiceUser](../../models/components/v3paymentserviceuser.md) | :heavy_check_mark: | N/A | +| `Data` | [components.V3PaymentServiceUser](../../models/components/v3paymentserviceuser.md) | :heavy_check_mark: | Contains the payment service user details |pkg/client/docs/models/components/v3forwardpaymentserviceuserbankaccountresponse.md (1)
1-8
: Consider adding a description for the Data fieldWhile the documentation structure is clear, the description for the
Data
field is set to "N/A". Consider adding a brief description explaining what this field represents to improve the documentation's completeness.- | `Data` | [components.V3ForwardPaymentServiceUserBankAccountResponseData](../../models/components/v3forwardpaymentserviceuserbankaccountresponsedata.md) | :heavy_check_mark: | N/A | + | `Data` | [components.V3ForwardPaymentServiceUserBankAccountResponseData](../../models/components/v3forwardpaymentserviceuserbankaccountresponsedata.md) | :heavy_check_mark: | Contains the task information for the asynchronous bank account forwarding operation |internal/api/services/payment_service_users_create.go (1)
9-11
: Consider adding pre-storage validationThe implementation correctly handles error wrapping for better diagnostics, but consider whether any validation of the PSU model should happen at this service layer before delegating to storage.
func (s *Service) PaymentServiceUsersCreate(ctx context.Context, psu models.PaymentServiceUser) error { + // Validate required fields + if psu.Name == "" { + return errors.New("payment service user name is required") + } + return newStorageError(s.storage.PaymentServiceUsersCreate(ctx, psu), "cannot create payment service user") }internal/api/services/payment_service_users_get.go (1)
10-17
: PaymentServiceUsersGet implementation looks good.The service method correctly retrieves a payment service user from storage and handles errors appropriately with contextual error messages. Good use of the
newStorageError
helper to wrap storage errors.Consider adding logging statements to aid in observability, especially for the error path. This would align with typical service method patterns in the codebase.
func (s *Service) PaymentServiceUsersGet(ctx context.Context, id uuid.UUID) (*models.PaymentServiceUser, error) { psu, err := s.storage.PaymentServiceUsersGet(ctx, id) if err != nil { + log.Ctx(ctx).Error().Err(err).Str("paymentServiceUserID", id.String()).Msg("Failed to get payment service user") return nil, newStorageError(err, "cannot get payment service user") } + log.Ctx(ctx).Debug().Str("paymentServiceUserID", id.String()).Msg("Successfully retrieved payment service user") return psu, nil }internal/api/services/bank_accounts_forward_to_connector.go (1)
11-19
: Fix typo in comment and improve error message.The implementation correctly retrieves the bank account before forwarding it to the engine, which is a good practice. However, there's a grammatical error in the comment.
if ba == nil { - // Should not happened, but just in case + // Should not happen, but just in case return models.Task{}, newStorageError(nil, "bank account not found") }internal/api/services/payment_service_users_list.go (1)
11-18
: Service implementation looks goodThe implementation correctly wraps the storage call with appropriate error handling. The service properly returns the paginated results from storage when successful.
Consider adding logging at the service level, especially for errors, to aid in debugging and monitoring service operations.
internal/api/services/payment_service_users_add_bank_account.go (1)
9-11
: Consider adding input validation for UUIDsWhile the implementation is correct and concise, consider adding validation to ensure that neither
psuID
norbankAccountID
are zero values before calling the storage layer. This would prevent unnecessary database calls for obviously invalid inputs.func (s *Service) PaymentServiceUsersAddBankAccount(ctx context.Context, psuID uuid.UUID, bankAccountID uuid.UUID) error { + if psuID == uuid.Nil { + return fmt.Errorf("payment service user ID cannot be empty") + } + if bankAccountID == uuid.Nil { + return fmt.Errorf("bank account ID cannot be empty") + } return newStorageError(s.storage.PaymentServiceUsersAddBankAccount(ctx, psuID, bankAccountID), "failed to add bank account to payment service user") }You'll need to add
"fmt"
to your imports if this change is implemented.pkg/client/docs/models/components/v3paymentserviceuser.md (1)
1-14
: Add descriptive text for each field in the documentationThe documentation provides a clear structure for the
V3PaymentServiceUser
model, but all field descriptions are marked as "N/A". Consider adding meaningful descriptions for each field to help developers understand their purpose and usage.For example, you could enhance the documentation like this:
| Field | Type | Required | Description | | --------------- | -------------- | -------------- | ------------------------------------------------------- | -| `ID` | *string* | :heavy_check_mark: | N/A | +| `ID` | *string* | :heavy_check_mark: | Unique identifier for the payment service user | -| `Name` | *string* | :heavy_check_mark: | N/A | +| `Name` | *string* | :heavy_check_mark: | Full name of the payment service user |pkg/client/docs/models/operations/v3listpaymentserviceusersrequest.md (1)
10-10
: ImproveRequestBody
field documentationThe
RequestBody
field lacks a clear description of its purpose and what key-value pairs are accepted for filtering payment service users. This makes it difficult for API consumers to understand how to use this parameter effectively.-| `RequestBody` | map[string]*any* | :heavy_minus_sign: | N/A | | +| `RequestBody` | map[string]*any* | :heavy_minus_sign: | Optional filter criteria for payment service users. Supports filtering on fields like name, metadata, etc. | |internal/api/v3/handler_payment_service_users_list_test.go (1)
15-52
: Good test coverage but consider adding more test casesThe tests cover basic success and error scenarios for the payment service users list handler. The setup with GoMock and the assertions using the response recorder are properly implemented.
Consider adding tests for:
- Different query parameter combinations
- Pagination scenarios (next cursor, limit variations)
- Empty results vs. populated results
internal/api/v3/handler_payment_service_users_get.go (1)
13-35
: Handler looks good but avoid duplicate function callsThe handler correctly implements the retrieval of a payment service user with proper error handling and tracing.
The
paymentServiceUserID(r)
function is called twice (lines 18 and 19). Extract the ID once to improve readability and avoid potential duplicate work:- span.SetAttributes(attribute.String("paymentServiceUserID", paymentServiceUserID(r))) - id, err := uuid.Parse(paymentServiceUserID(r)) + psuID := paymentServiceUserID(r) + span.SetAttributes(attribute.String("paymentServiceUserID", psuID)) + id, err := uuid.Parse(psuID)internal/api/services/payment_service_users_create_test.go (1)
15-58
: Good test structure but consider testing with varied input dataThe test is well-structured with table-driven tests covering success and error scenarios.
Consider enhancing the tests by:
- Testing with different PSU configurations (with different optional fields populated)
- Verifying that the exact PSU object is passed to the store by using a gomock.Matcher or capturing the argument
// Example of capturing and verifying the input var capturedPSU models.PaymentServiceUser store.EXPECT(). PaymentServiceUsersCreate(gomock.Any(), gomock.Any()). DoAndReturn(func(_ context.Context, psu models.PaymentServiceUser) error { capturedPSU = psu return test.err }) // After service call require.Equal(t, expectedPSU, capturedPSU)internal/api/v3/handler_payment_service_users_forward_bank_account_to_connector_test.go (1)
71-83
: Consider validating task data in success responseWhile the test correctly checks for a 202 Accepted status code and confirms the response contains a "data" field, it could be more robust by also validating the content of the returned Task object.
Consider enhancing the assertion to verify the task data structure in the response:
handlerFn(w, prepareQueryRequestWithBody(http.MethodPost, body, "paymentServiceUserID", psuID.String(), "bankAccountID", bankAccountID.String())) - assertExpectedResponse(w.Result(), http.StatusAccepted, "data") + // Verify response status and the presence of task data + resp := w.Result() + assertExpectedResponse(resp, http.StatusAccepted, "data") + // Additional verification of task structure could be added here if neededinternal/api/v3/handler_payment_service_users_get_test.go (1)
53-61
: Consider using a populated PSU object in success case testCurrently, the test mocks the backend to return an empty PaymentServiceUser object. To make the test more robust, consider using a populated object to ensure that all fields are properly serialized in the response.
- m.EXPECT().PaymentServiceUsersGet(gomock.Any(), psuID).Return( - &models.PaymentServiceUser{}, nil, - ) + m.EXPECT().PaymentServiceUsersGet(gomock.Any(), psuID).Return( + &models.PaymentServiceUser{ + ID: psuID, + Name: "Test User", + Email: "test@example.com", + // Add other relevant fields + }, nil, + )internal/api/services/payment_service_users_forward_bank_account_test.go (1)
16-123
: Comprehensive test coverage for bank account forwarding.The test cases cover success scenarios and various error conditions, including validation errors, not found errors, engine errors, and storage errors for both bank account and PSU retrieval.
Consider using more realistic test data for the mocked return values. Currently, empty structs are returned for both bank account and PSU:
-store.EXPECT().BankAccountsGet(gomock.Any(), test.bankAccountID, true).Return(&models.BankAccount{}, test.bankAccountStorageErr) +store.EXPECT().BankAccountsGet(gomock.Any(), test.bankAccountID, true).Return(&models.BankAccount{ + ID: test.bankAccountID, + // Add more realistic fields here +}, test.bankAccountStorageErr) -store.EXPECT().PaymentServiceUsersGet(gomock.Any(), test.psuID).Return(&models.PaymentServiceUser{}, test.psuStorageErr) +store.EXPECT().PaymentServiceUsersGet(gomock.Any(), test.psuID).Return(&models.PaymentServiceUser{ + ID: test.psuID, + Name: "Test PSU", + // Add more realistic fields here +}, test.psuStorageErr)This would make the tests more realistic and potentially catch issues related to the data structure.
internal/api/services/payment_service_users_forward_bank_account.go (1)
17-20
: Unnecessary nil check after storage retrieval.The storage layer should already return an appropriate error if the bank account isn't found, making this additional check redundant.
Consider removing this nil check or adding a log message if this unexpected condition occurs:
- if ba == nil { - // Should not happened, but just in case - return models.Task{}, newStorageError(storage.ErrNotFound, "bank account not found") - } + // If we reach here, ba should never be nil, as storage.ErrNotFound would be returned + // by the storage layer. No need for an additional nil check.Also, note the typo in the comment: "happened" should be "happen".
internal/api/v3/handler_payment_service_users_list.go (1)
19-25
: Consider adding validation for query parametersThe pagination extraction logic looks good, but consider adding specific validation for any PSU-specific query parameters that might be relevant beyond the standard pagination options.
internal/models/psu.go (1)
80-81
: Error handling could be improved for UUID parsing.The UUID parsing in the UnmarshalJSON method ignores potential errors, which could lead to silent failures if invalid UUIDs are provided in the input JSON.
Consider adding error checking for UUID parsing to ensure data integrity:
-psu.ID, _ = uuid.Parse(aux.ID) +var err error +psu.ID, err = uuid.Parse(aux.ID) +if err != nil { + return fmt.Errorf("invalid ID format: %w", err) +} // Later in the BankAccountIDs loop: -psu.BankAccountIDs[i], _ = uuid.Parse(id) +psu.BankAccountIDs[i], err = uuid.Parse(id) +if err != nil { + return fmt.Errorf("invalid bank account ID format at index %d: %w", i, err) +}Don't forget to add
fmt
to the imports.Also applies to: 90-91
internal/api/v3/handler_payment_service_users_create.go (1)
121-123
: Potentially unbounded span attributesLooping over
req.Metadata
blindly turns every key/value pair into an OpenTelemetry attribute. Large or sensitive values can exceed attribute limits or blow up the trace payload. Consider capping the number/size of attributes or hashing values.internal/storage/psu.go (1)
143-165
: SQL-injection-safe, but error wording &%w
usage
- The error returned for unknown keys reads “unknown key … when building query: %w” but
%w
is not supplied – the wrapped error is lost."'%s' column can only be used with $match: %w"
similarly omits the wrapped error.Use
%w
only when you actually wrap another error value.internal/storage/psu_test.go (1)
206-221
: Typos in sub-test name hinder readability & grepping
"lsit psu by metadata"
is repeated twice and misspells “list”. Consider renaming both sub-tests:-t.Run("lsit psu by metadata", func(t *testing.T) { +t.Run("list psu by metadata", func(t *testing.T) {A tiny change, but it helps when scanning
go test -v
output or grepping for failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
openapi.yaml
is excluded by!**/*.yaml
openapi/v3/v3-api.yaml
is excluded by!**/*.yaml
openapi/v3/v3-parameters.yaml
is excluded by!**/*.yaml
openapi/v3/v3-schemas.yaml
is excluded by!**/*.yaml
pkg/client/.speakeasy/gen.lock
is excluded by!**/*.lock
,!**/*.lock
📒 Files selected for processing (94)
docs/api/README.md
(2 hunks)internal/api/backend/backend.go
(1 hunks)internal/api/backend/backend_generated.go
(1 hunks)internal/api/services/bank_accounts_forward_to_connector.go
(1 hunks)internal/api/services/bank_accounts_forward_to_connector_test.go
(1 hunks)internal/api/services/payment_service_users_add_bank_account.go
(1 hunks)internal/api/services/payment_service_users_add_bank_account_test.go
(1 hunks)internal/api/services/payment_service_users_create.go
(1 hunks)internal/api/services/payment_service_users_create_test.go
(1 hunks)internal/api/services/payment_service_users_forward_bank_account.go
(1 hunks)internal/api/services/payment_service_users_forward_bank_account_test.go
(1 hunks)internal/api/services/payment_service_users_get.go
(1 hunks)internal/api/services/payment_service_users_get_test.go
(1 hunks)internal/api/services/payment_service_users_list.go
(1 hunks)internal/api/services/payment_service_users_list_test.go
(1 hunks)internal/api/v3/errors.go
(2 hunks)internal/api/v3/handler_payment_service_users_add_bank_account.go
(1 hunks)internal/api/v3/handler_payment_service_users_add_bank_account_test.go
(1 hunks)internal/api/v3/handler_payment_service_users_create.go
(1 hunks)internal/api/v3/handler_payment_service_users_create_test.go
(1 hunks)internal/api/v3/handler_payment_service_users_forward_bank_account_to_connector.go
(1 hunks)internal/api/v3/handler_payment_service_users_forward_bank_account_to_connector_test.go
(1 hunks)internal/api/v3/handler_payment_service_users_get.go
(1 hunks)internal/api/v3/handler_payment_service_users_get_test.go
(1 hunks)internal/api/v3/handler_payment_service_users_list.go
(1 hunks)internal/api/v3/handler_payment_service_users_list_test.go
(1 hunks)internal/api/v3/router.go
(2 hunks)internal/connectors/engine/activities/activity.go
(1 hunks)internal/connectors/engine/activities/storage_payment_service_users_get.go
(1 hunks)internal/connectors/engine/engine.go
(4 hunks)internal/connectors/engine/engine_generated.go
(1 hunks)internal/connectors/engine/engine_test.go
(3 hunks)internal/connectors/engine/workflow/create_bank_account.go
(4 hunks)internal/connectors/engine/workflow/create_bank_account_test.go
(6 hunks)internal/connectors/engine/workflow/main_test.go
(2 hunks)internal/connectors/plugins/public/bankingcircle/payouts.go
(3 hunks)internal/connectors/plugins/public/bankingcircle/payouts_test.go
(1 hunks)internal/models/bank_accounts.go
(2 hunks)internal/models/psu.go
(1 hunks)internal/storage/connector_tasks_tree.go
(3 hunks)internal/storage/connectors.go
(4 hunks)internal/storage/migrations/14-create-psu-tables.sql
(1 hunks)internal/storage/migrations/migrations.go
(2 hunks)internal/storage/payment_initiation_reversals.go
(2 hunks)internal/storage/payment_initiations.go
(1 hunks)internal/storage/payments.go
(3 hunks)internal/storage/psu.go
(1 hunks)internal/storage/psu_test.go
(1 hunks)internal/storage/schedules.go
(1 hunks)internal/storage/storage.go
(1 hunks)internal/storage/storage_generated.go
(1 hunks)internal/storage/workflow_instances.go
(1 hunks)pkg/client/README.md
(1 hunks)pkg/client/docs/models/components/v3address.md
(1 hunks)pkg/client/docs/models/components/v3addressrequest.md
(1 hunks)pkg/client/docs/models/components/v3contactdetails.md
(1 hunks)pkg/client/docs/models/components/v3contactdetailsrequest.md
(1 hunks)pkg/client/docs/models/components/v3createpaymentserviceuserrequest.md
(1 hunks)pkg/client/docs/models/components/v3createpaymentserviceuserresponse.md
(1 hunks)pkg/client/docs/models/components/v3forwardpaymentserviceuserbankaccountrequest.md
(1 hunks)pkg/client/docs/models/components/v3forwardpaymentserviceuserbankaccountresponse.md
(1 hunks)pkg/client/docs/models/components/v3forwardpaymentserviceuserbankaccountresponsedata.md
(1 hunks)pkg/client/docs/models/components/v3getpaymentserviceuserresponse.md
(1 hunks)pkg/client/docs/models/components/v3paymentserviceuser.md
(1 hunks)pkg/client/docs/models/components/v3paymentserviceuserscursorresponse.md
(1 hunks)pkg/client/docs/models/components/v3paymentserviceuserscursorresponsecursor.md
(1 hunks)pkg/client/docs/models/operations/v3addbankaccounttopaymentserviceuserrequest.md
(1 hunks)pkg/client/docs/models/operations/v3addbankaccounttopaymentserviceuserresponse.md
(1 hunks)pkg/client/docs/models/operations/v3createpaymentserviceuserresponse.md
(1 hunks)pkg/client/docs/models/operations/v3forwardpaymentserviceuserbankaccountrequest.md
(1 hunks)pkg/client/docs/models/operations/v3forwardpaymentserviceuserbankaccountresponse.md
(1 hunks)pkg/client/docs/models/operations/v3getpaymentserviceuserrequest.md
(1 hunks)pkg/client/docs/models/operations/v3getpaymentserviceuserresponse.md
(1 hunks)pkg/client/docs/models/operations/v3listpaymentserviceusersrequest.md
(1 hunks)pkg/client/docs/models/operations/v3listpaymentserviceusersresponse.md
(1 hunks)pkg/client/docs/sdks/v3/README.md
(2 hunks)pkg/client/models/components/v3address.go
(1 hunks)pkg/client/models/components/v3addressrequest.go
(1 hunks)pkg/client/models/components/v3contactdetails.go
(1 hunks)pkg/client/models/components/v3contactdetailsrequest.go
(1 hunks)pkg/client/models/components/v3createpaymentserviceuserrequest.go
(1 hunks)pkg/client/models/components/v3createpaymentserviceuserresponse.go
(1 hunks)pkg/client/models/components/v3forwardpaymentserviceuserbankaccountrequest.go
(1 hunks)pkg/client/models/components/v3forwardpaymentserviceuserbankaccountresponse.go
(1 hunks)pkg/client/models/components/v3getpaymentserviceuserresponse.go
(1 hunks)pkg/client/models/components/v3paymentserviceuser.go
(1 hunks)pkg/client/models/components/v3paymentserviceuserscursorresponse.go
(1 hunks)pkg/client/models/operations/v3addbankaccounttopaymentserviceuser.go
(1 hunks)pkg/client/models/operations/v3createpaymentserviceuser.go
(1 hunks)pkg/client/models/operations/v3forwardpaymentserviceuserbankaccount.go
(1 hunks)pkg/client/models/operations/v3getpaymentserviceuser.go
(1 hunks)pkg/client/models/operations/v3listpaymentserviceusers.go
(1 hunks)pkg/client/v3.go
(1 hunks)test/e2e/api_payment_service_users_test.go
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (37)
internal/storage/schedules.go (1)
internal/connectors/engine/plugins/plugin.go (1)
ErrValidation
(23-23)
internal/api/v3/errors.go (2)
internal/storage/error.go (2)
ErrForeignKeyViolation
(21-21)ErrValidation
(16-16)internal/utils/errors/errors.go (1)
Cause
(29-49)
internal/connectors/engine/activities/activity.go (2)
internal/connectors/plugins/public/wise/client/recipient_accounts.go (1)
Name
(19-21)internal/connectors/engine/activities/storage_payment_service_users_get.go (1)
StoragePaymentServiceUsersGet
(21-25)
internal/storage/payment_initiations.go (2)
internal/api/v3/errors.go (1)
ErrValidation
(14-14)internal/storage/error.go (1)
ErrValidation
(16-16)
internal/api/services/payment_service_users_get.go (2)
internal/api/services/services.go (1)
Service
(8-12)internal/models/psu.go (1)
PaymentServiceUser
(24-34)
internal/api/v3/handler_payment_service_users_get.go (3)
internal/api/backend/backend.go (1)
Backend
(16-96)internal/otel/otel.go (2)
Tracer
(18-24)RecordError
(26-29)internal/api/v3/errors.go (1)
ErrInvalidID
(15-15)
internal/api/services/payment_service_users_list.go (3)
internal/api/services/services.go (1)
Service
(8-12)internal/storage/psu.go (1)
ListPSUsQuery
(133-133)internal/models/psu.go (1)
PaymentServiceUser
(24-34)
internal/api/v3/handler_payment_service_users_forward_bank_account_to_connector_test.go (5)
internal/api/backend/backend_generated.go (2)
MockBackend
(27-31)NewMockBackend
(39-43)internal/api/v3/handler_payment_service_users_forward_bank_account_to_connector.go (1)
PaymentServiceUserForwardBankAccountToConnectorRequest
(16-18)internal/api/validation/validation.go (1)
NewValidator
(29-49)internal/api/v3/errors.go (2)
ErrInvalidID
(15-15)ErrValidation
(14-14)internal/models/tasks.go (1)
Task
(19-33)
internal/connectors/plugins/public/bankingcircle/payouts_test.go (1)
internal/models/bank_accounts.go (4)
AccountIBANMetadataKey
(28-28)AccountAccountNumberMetadataKey
(29-29)AccountSwiftBicCodeMetadataKey
(32-32)AccountBankAccountCountryMetadataKey
(31-31)
internal/storage/storage.go (2)
internal/models/psu.go (1)
PaymentServiceUser
(24-34)internal/storage/psu.go (1)
ListPSUsQuery
(133-133)
internal/storage/connectors.go (2)
internal/models/connectors.go (1)
Connector
(8-22)internal/connectors/engine/plugins/plugin.go (1)
ErrValidation
(23-23)
internal/api/v3/handler_payment_service_users_create_test.go (5)
internal/api/backend/backend_generated.go (2)
MockBackend
(27-31)NewMockBackend
(39-43)internal/api/validation/validation.go (1)
NewValidator
(29-49)internal/api/v3/errors.go (2)
ErrMissingOrInvalidBody
(16-16)ErrValidation
(14-14)internal/api/v3/handler_payment_service_users_create.go (3)
PaymentServiceUsersCreateRequest
(33-40)AddressRequest
(24-31)ContactDetailsRequest
(19-22)internal/models/psu.go (1)
ContactDetails
(19-22)
internal/api/v3/handler_payment_service_users_add_bank_account_test.go (2)
internal/api/backend/backend_generated.go (2)
MockBackend
(27-31)NewMockBackend
(39-43)internal/api/v3/errors.go (1)
ErrInvalidID
(15-15)
internal/api/services/payment_service_users_create_test.go (3)
internal/storage/storage_generated.go (1)
NewMockStorage
(36-40)internal/connectors/engine/engine_generated.go (1)
NewMockEngine
(35-39)internal/models/psu.go (1)
PaymentServiceUser
(24-34)
internal/api/services/payment_service_users_create.go (2)
internal/api/services/services.go (1)
Service
(8-12)internal/models/psu.go (1)
PaymentServiceUser
(24-34)
internal/storage/payments.go (1)
internal/api/v3/errors.go (1)
ErrValidation
(14-14)
pkg/client/models/components/v3createpaymentserviceuserresponse.go (2)
internal/connectors/plugins/public/moneycorp/client/transactions.go (1)
Data
(36-39)pkg/client/models/operations/v3createpaymentserviceuser.go (1)
V3CreatePaymentServiceUserResponse
(9-15)
pkg/client/models/components/v3forwardpaymentserviceuserbankaccountrequest.go (1)
pkg/client/models/operations/v3forwardpaymentserviceuserbankaccount.go (1)
V3ForwardPaymentServiceUserBankAccountRequest
(9-15)
pkg/client/models/operations/v3createpaymentserviceuser.go (1)
pkg/client/models/components/v3createpaymentserviceuserresponse.go (1)
V3CreatePaymentServiceUserResponse
(5-8)
internal/api/v3/handler_payment_service_users_add_bank_account.go (3)
internal/api/backend/backend.go (1)
Backend
(16-96)internal/otel/otel.go (2)
Tracer
(18-24)RecordError
(26-29)internal/api/v3/errors.go (1)
ErrInvalidID
(15-15)
internal/api/backend/backend.go (3)
internal/models/psu.go (1)
PaymentServiceUser
(24-34)internal/storage/psu.go (1)
ListPSUsQuery
(133-133)internal/models/tasks.go (1)
Task
(19-33)
pkg/client/models/components/v3paymentserviceuser.go (2)
pkg/client/models/components/v3contactdetails.go (1)
V3ContactDetails
(5-8)pkg/client/models/components/v3address.go (1)
V3Address
(5-12)
internal/connectors/plugins/public/bankingcircle/payouts.go (1)
internal/models/bank_accounts.go (4)
AccountAccountNumberMetadataKey
(29-29)AccountIBANMetadataKey
(28-28)AccountSwiftBicCodeMetadataKey
(32-32)AccountBankAccountCountryMetadataKey
(31-31)
internal/api/services/payment_service_users_add_bank_account.go (1)
internal/api/services/services.go (1)
Service
(8-12)
test/e2e/api_payment_service_users_test.go (10)
test/e2e/suite_test.go (1)
UseTemplatedDatabase
(128-130)pkg/client/models/components/v3createpaymentserviceuserrequest.go (1)
V3CreatePaymentServiceUserRequest
(5-11)pkg/testserver/helpers.go (2)
NewTestServer
(21-28)Subscribe
(30-39)pkg/client/models/components/v3contactdetailsrequest.go (1)
V3ContactDetailsRequest
(5-8)pkg/client/models/components/v3addressrequest.go (1)
V3AddressRequest
(5-12)pkg/client/models/components/v3forwardpaymentserviceuserbankaccountrequest.go (1)
V3ForwardPaymentServiceUserBankAccountRequest
(5-7)internal/models/task_id.go (2)
TaskIDFromString
(37-49)TaskID
(12-15)pkg/testserver/matchers.go (2)
Event
(142-147)WithPayload
(84-89)pkg/events/event.go (1)
EventTypeSavedBankAccount
(14-14)internal/events/bank_account.go (2)
BankAccountMessagePayload
(11-24)BankAccountRelatedAccountsPayload
(26-31)
internal/connectors/engine/activities/storage_payment_service_users_get.go (2)
internal/connectors/engine/activities/activity.go (1)
Activities
(17-26)internal/models/psu.go (1)
PaymentServiceUser
(24-34)
internal/connectors/engine/engine.go (4)
internal/connectors/plugins/public/mangopay/client/bank_accounts.go (1)
BankAccount
(130-134)internal/models/tasks.go (1)
Task
(19-33)internal/connectors/engine/ids.go (1)
IDPrefixBankAccountCreate
(10-10)internal/models/task_id.go (1)
TaskID
(12-15)
pkg/client/models/components/v3createpaymentserviceuserrequest.go (2)
pkg/client/models/components/v3contactdetailsrequest.go (1)
V3ContactDetailsRequest
(5-8)pkg/client/models/components/v3addressrequest.go (1)
V3AddressRequest
(5-12)
internal/api/services/bank_accounts_forward_to_connector_test.go (3)
internal/api/services/errors.go (2)
ErrValidation
(12-12)ErrNotFound
(13-13)pkg/testserver/server.go (1)
T
(32-37)internal/models/tasks.go (1)
Task
(19-33)
pkg/client/models/operations/v3forwardpaymentserviceuserbankaccount.go (2)
pkg/client/models/components/v3forwardpaymentserviceuserbankaccountrequest.go (1)
V3ForwardPaymentServiceUserBankAccountRequest
(5-7)pkg/client/models/components/v3forwardpaymentserviceuserbankaccountresponse.go (1)
V3ForwardPaymentServiceUserBankAccountResponse
(18-20)
pkg/client/models/operations/v3listpaymentserviceusers.go (1)
pkg/client/models/components/v3paymentserviceuserscursorresponse.go (1)
V3PaymentServiceUsersCursorResponse
(48-50)
internal/api/v3/handler_payment_service_users_create.go (5)
internal/models/psu.go (2)
ContactDetails
(19-22)PaymentServiceUser
(24-34)internal/api/backend/backend.go (1)
Backend
(16-96)internal/api/validation/validation.go (1)
Validator
(24-27)internal/otel/otel.go (2)
Tracer
(18-24)RecordError
(26-29)internal/api/v3/errors.go (2)
ErrMissingOrInvalidBody
(16-16)ErrValidation
(14-14)
pkg/client/models/operations/v3getpaymentserviceuser.go (1)
pkg/client/models/components/v3getpaymentserviceuserresponse.go (1)
V3GetPaymentServiceUserResponse
(5-7)
internal/models/bank_accounts.go (1)
internal/models/psu.go (3)
PaymentServiceUser
(24-34)Address
(10-17)ContactDetails
(19-22)
internal/api/services/bank_accounts_forward_to_connector.go (1)
internal/models/tasks.go (1)
Task
(19-33)
internal/storage/psu.go (1)
internal/models/psu.go (2)
PaymentServiceUser
(24-34)ContactDetails
(19-22)
pkg/client/v3.go (11)
pkg/client/models/components/v3createpaymentserviceuserrequest.go (1)
V3CreatePaymentServiceUserRequest
(5-11)pkg/client/models/components/v3createpaymentserviceuserresponse.go (1)
V3CreatePaymentServiceUserResponse
(5-8)pkg/client/models/operations/v3createpaymentserviceuser.go (1)
V3CreatePaymentServiceUserResponse
(9-15)pkg/client/models/operations/v3listpaymentserviceusers.go (2)
V3ListPaymentServiceUsersResponse
(39-45)V3ListPaymentServiceUsersRequest
(9-16)pkg/client/models/components/v3paymentserviceuserscursorresponse.go (1)
V3PaymentServiceUsersCursorResponse
(48-50)pkg/client/models/components/v3getpaymentserviceuserresponse.go (1)
V3GetPaymentServiceUserResponse
(5-7)pkg/client/models/operations/v3getpaymentserviceuser.go (2)
V3GetPaymentServiceUserResponse
(21-27)V3GetPaymentServiceUserRequest
(9-12)pkg/client/models/operations/v3addbankaccounttopaymentserviceuser.go (2)
V3AddBankAccountToPaymentServiceUserResponse
(30-34)V3AddBankAccountToPaymentServiceUserRequest
(9-14)pkg/client/models/components/v3forwardpaymentserviceuserbankaccountrequest.go (1)
V3ForwardPaymentServiceUserBankAccountRequest
(5-7)pkg/client/models/operations/v3forwardpaymentserviceuserbankaccount.go (2)
V3ForwardPaymentServiceUserBankAccountRequest
(9-15)V3ForwardPaymentServiceUserBankAccountResponse
(38-44)pkg/client/models/components/v3forwardpaymentserviceuserbankaccountresponse.go (1)
V3ForwardPaymentServiceUserBankAccountResponse
(18-20)
🪛 markdownlint-cli2 (0.17.2)
pkg/client/docs/sdks/v3/README.md
1821-1821: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1822-1822: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1823-1823: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1824-1824: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1873-1873: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1874-1874: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1875-1875: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1876-1876: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1927-1927: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1928-1928: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1929-1929: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1930-1930: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1979-1979: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1980-1980: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1981-1981: Hard tabs
Column: 1
(MD010, no-hard-tabs)
1982-1982: Hard tabs
Column: 1
(MD010, no-hard-tabs)
2032-2032: Hard tabs
Column: 1
(MD010, no-hard-tabs)
2033-2033: Hard tabs
Column: 1
(MD010, no-hard-tabs)
2034-2034: Hard tabs
Column: 1
(MD010, no-hard-tabs)
2035-2035: Hard tabs
Column: 1
(MD010, no-hard-tabs)
docs/api/README.md
2235-2235: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
2286-2286: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
2291-2291: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
2365-2365: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
2432-2432: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
2488-2488: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
4933-4933: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
4962-4962: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
4988-4988: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
5009-5009: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
5060-5060: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
5106-5106: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
5133-5133: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
5159-5159: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
5207-5207: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
5227-5227: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
5249-5249: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
🪛 GitHub Actions: Default
internal/storage/psu_test.go
[error] 377-377: Staticcheck SA5011: possible nil pointer dereference at line 377
[error] 371-371: Staticcheck SA5011 related information: this check suggests that the pointer can be nil at line 371
[error] 378-378: Staticcheck SA5011: possible nil pointer dereference at line 378
[error] 379-379: Staticcheck SA5011: possible nil pointer dereference at line 379
[error] 389-389: Staticcheck SA5011 related information: this check suggests that the pointer can be nil at line 389
🔇 Additional comments (123)
internal/storage/workflow_instances.go (2)
89-89
: Clean modernization of error handlingThe migration from
errors.Wrap
tofmt.Errorf
with%w
verb follows Go 1.13+ best practices for error wrapping, making your error handling consistent with the standard library.
93-93
: Good standardization of error handlingConsistently updating error wrapping to use
fmt.Errorf
with%w
makes the codebase more maintainable and aligned with modern Go error handling conventions.internal/storage/connectors.go (1)
54-54
: Systematic improvement to error handlingConverting all instances of
errors.Wrap
tofmt.Errorf
with%w
error wrapping verb is a good modernization that:
- Reduces external dependencies
- Aligns with Go standard library error handling (since Go 1.13)
- Maintains the same error wrapping functionality
- Makes error handling consistent across the codebase
Also applies to: 90-90, 98-98, 136-136, 222-222
internal/storage/payment_initiation_reversals.go (1)
147-147
: Consistent error handling modernizationThe error handling updates match the pattern applied throughout the storage package, moving from the external
github.com/pkg/errors
package to the standard library's error wrapping capabilities.Also applies to: 156-156
pkg/client/docs/models/components/v3createpaymentserviceuserresponse.md (1)
1-8
: Clear and concise API documentationThis documentation for the
V3CreatePaymentServiceUserResponse
model clearly defines the response structure for creating a Payment Service User (PSU), which is a core part of the new PSU feature being introduced. The format follows the established documentation pattern and clearly indicates the required field and its purpose.internal/storage/schedules.go (1)
88-88
: Use standard library for error wrapping
The change fromerrors.Wrap
tofmt.Errorf(...%w)
correctly standardizes error wrapping, preserves the originalErrValidation
, and removes an external dependency. This aligns with other storage methods.pkg/client/docs/models/operations/v3getpaymentserviceuserrequest.md (1)
1-8
: Documentation looks accurate
TheV3GetPaymentServiceUserRequest
model is clearly documented with its requiredPaymentServiceUserID
field. The Markdown table formatting is consistent with other operation request docs.pkg/client/docs/models/components/v3paymentserviceuserscursorresponse.md (1)
1-8
: Documentation is complete and consistent
TheV3PaymentServiceUsersCursorResponse
schema is correctly documented with the requiredCursor
field and proper link to its component. Table formatting and markdown syntax are valid.pkg/client/docs/models/components/v3forwardpaymentserviceuserbankaccountrequest.md (1)
1-8
: Documentation accurately reflects the new request model
TheV3ForwardPaymentServiceUserBankAccountRequest
model'sConnectorID
field is properly listed as required. The table structure and Markdown syntax are correct and consistent.internal/connectors/plugins/public/bankingcircle/payouts_test.go (1)
56-59
: LGTM: Metadata keys updated correctlyThe test setup now uses the new
Account*MetadataKey
constants which aligns with the model changes in the codebase. This ensures the tests remain in sync with the updated bank account metadata key structure.internal/storage/connector_tasks_tree.go (1)
6-6
: Improved error handling with standard libraryThe change from
github.com/pkg/errors
to Go's standard libraryfmt.Errorf
with%w
verb for error wrapping is a good modernization. This reduces external dependencies while maintaining the same error wrapping functionality.Also applies to: 23-23, 52-52
pkg/client/docs/models/components/v3address.md (1)
1-13
: LGTM: Well-structured documentationThe documentation for the V3Address model is clear and follows the standard format with all required fields properly documented.
internal/storage/payment_initiations.go (2)
449-451
: Error formatting looks good.The updated error formatting correctly uses Go's native error wrapping with
fmt.Errorf
and the%w
verb, which is an improvement over the previous approach witherrors.Wrap
.
458-459
: Error formatting looks good.The updated error formatting for unknown keys correctly uses Go's native error wrapping with
fmt.Errorf
and the%w
verb, which is consistent with the other error handling changes in this file.internal/connectors/engine/activities/activity.go (1)
190-193
:✅ Verification successful
Good addition of PaymentServiceUsers support.
The addition of
StoragePaymentServiceUsersGet
activity aligns with the new Payment Service User (PSU) entity feature. This allows workflows to retrieve PSU data from storage, which is necessary for operations like bank account forwarding that might need PSU information.
🏁 Script executed:
#!/bin/bash # Check if the implementation of StoragePaymentServiceUsersGet activity matches the interface # First, check if the activity function is properly implemented rg -A 3 "func \(a Activities\) StoragePaymentServiceUsersGet" internal/connectors/engine/activities/ # Second, check if the storage interface has a corresponding method rg "PaymentServiceUsersGet" internal/storage/storage.goLength of output: 788
✅ Approved: StoragePaymentServiceUsersGet activity verified
Excellent addition of PaymentServiceUsers support. Confirmed that:
Activities.StoragePaymentServiceUsersGet
is implemented in
internal/connectors/engine/activities/storage_payment_service_users_get.go
- the storage interface defines
PaymentServiceUsersGet(ctx context.Context, id uuid.UUID) (*models.PaymentServiceUser, error)
in
internal/storage/storage.go
pkg/client/docs/models/operations/v3addbankaccounttopaymentserviceuserrequest.md (1)
1-9
: Good documentation for the API request model.The documentation clearly outlines the request structure for adding a bank account to a payment service user, with well-defined required fields and descriptions.
pkg/client/models/components/v3getpaymentserviceuserresponse.go (1)
1-14
: Proper struct definition with useful nil-safety checksThe
V3GetPaymentServiceUserResponse
struct is well-defined with the proper JSON tag for theData
field. TheGetData()
method includes appropriate nil-receiver handling to prevent potential nil pointer panics, which is a good practice for API client models.pkg/client/docs/models/components/v3forwardpaymentserviceuserbankaccountresponsedata.md (1)
1-8
: Task ID documentation is clear and provides useful contextThe documentation for the
V3ForwardPaymentServiceUserBankAccountResponseData
structure is well-formatted with clear explanation of the TaskID field's purpose and its relation to the asynchronous nature of the operation.pkg/client/models/components/v3contactdetails.go (2)
5-8
: Contact details struct is appropriately definedThe
V3ContactDetails
struct is well-designed with optional fields using pointer types and appropriateomitempty
JSON tags, following best practices for API client models where fields may be optional.
10-22
: Getter methods implement proper nil-safetyThe getter methods for
PhoneNumber
include appropriate nil-receiver checks, preventing potential nil pointer panics. This is a good defensive programming practice, especially for client-side code.pkg/client/README.md (1)
220-224
: LGTM - Well-structured SDK documentation for new PSU featuresThe additions clearly document the new Payment Service User (PSU) functionality in the SDK, maintaining consistent style with existing entries.
pkg/client/docs/models/operations/v3addbankaccounttopaymentserviceuserresponse.md (1)
1-9
: Documentation follows standard response model patternThe new response model documentation maintains a consistent structure with other response models and properly documents both success and error cases.
internal/storage/migrations/migrations.go (1)
28-29
:✅ Verification successful
Migration follows proper transaction and error handling patterns
The PSU table migration follows the established pattern in the codebase with proper transaction usage and error handling. Consider verifying that appropriate indexes are created for fields frequently used in queries to ensure optimal performance.
Also applies to: 196-206
🏁 Script executed:
#!/bin/bash # Examine the PSU table creation SQL to verify indexing and constraints # Check the embedded SQL file content cat internal/storage/migrations/14-create-psu-tables.sqlLength of output: 1243
Migration index verification completed
payment_service_users
table:
- Primary key on
id
(automatically indexed)- Composite index
psu_created_at_sort_id
on(created_at, sort_id)
psu_bank_accounts
table:
- Primary key
(psu_id, bank_account_id)
(automatically indexed)- Foreign key constraints leverage these indexes
If your application frequently queries
psu_bank_accounts
bybank_account_id
alone, consider adding a dedicated index on that column. Otherwise, the existing indexes meet the current requirements.pkg/client/docs/models/operations/v3forwardpaymentserviceuserbankaccountresponse.md (1)
1-10
: Documentation structure is well-formed and complete.The documentation for
V3ForwardPaymentServiceUserBankAccountResponse
model follows the standard markdown format with clear field definitions, types, and descriptions. The table structure properly indicates which fields are required and which are optional.internal/connectors/plugins/public/bankingcircle/payouts.go (3)
36-38
: Correctly updated metadata key references.The metadata key references have been appropriately updated from
BankAccount*
toAccount*
prefix to align with the new model changes.
83-86
: Consistent use of updated metadata keys.The account metadata key reference is correctly updated to use
models.AccountAccountNumberMetadataKey
andmodels.AccountIBANMetadataKey
instead of the previousBankAccount
prefixed keys.
105-107
: Financial institution and country metadata keys properly updated.These lines correctly update the metadata references to use the new
Account
prefix for SWIFT/BIC code and bank account country.internal/api/services/bank_accounts_forward_to_connector.go (1)
21-26
: Updated engine call to use the full bank account object.The engine call now correctly passes the full bank account object rather than just the ID, which is a good improvement that provides more context to the engine.
pkg/client/docs/models/operations/v3getpaymentserviceuserresponse.md (1)
1-10
: Documentation structure looks good.The documentation properly details the response model fields for the V3GetPaymentServiceUserResponse operation, including required fields, types, and descriptions with appropriate links to related component documentation.
pkg/client/models/components/v3forwardpaymentserviceuserbankaccountrequest.go (2)
5-7
: Request model correctly implemented.The V3ForwardPaymentServiceUserBankAccountRequest struct is properly defined with a single field for the connector ID.
9-14
: Getter method follows best practices.The GetConnectorID method properly handles nil receiver cases, returning an empty string when the struct pointer is nil. This is a good defensive programming practice.
pkg/client/docs/models/components/v3paymentserviceuserscursorresponsecursor.md (1)
1-12
: Pagination cursor documentation is complete.The documentation for the V3PaymentServiceUsersCursorResponseCursor model includes all necessary fields for a pagination implementation, with proper types, required flags, and examples. This provides clear guidance for API consumers.
internal/connectors/engine/workflow/main_test.go (2)
36-36
: Test suite struct properly extended.Added a new field to accommodate payment service user test data.
124-146
: Test data initialization is comprehensive.The payment service user test data is well-structured with appropriate test values for all fields including contact details, address information, bank account associations, and metadata. The implementation correctly references the existing bank account test data to establish the relationship between entities.
pkg/client/docs/models/operations/v3createpaymentserviceuserresponse.md (1)
1-10
: Documentation looks well-structured and complete.The markdown documentation for the
V3CreatePaymentServiceUserResponse
model is clear and follows the standard format consistently used across the project. It properly documents the response fields, their types, requirements, and descriptions while providing helpful links to related model documentation.pkg/client/models/components/v3createpaymentserviceuserresponse.go (1)
1-15
: Generated code structure looks good.The implementation correctly defines the
V3CreatePaymentServiceUserResponse
struct with a singleData
field to hold the created payment service user ID. TheGetData()
method provides a safe way to access the field value with proper nil checking, which is a good defensive programming practice.pkg/client/docs/models/components/v3addressrequest.md (1)
1-13
: Address model documentation is complete.The markdown documentation for the
V3AddressRequest
model is comprehensive and follows the standard format. It appropriately documents all address fields (StreetNumber, StreetName, City, Region, PostalCode, Country) and correctly indicates that all are optional with the:heavy_minus_sign:
notation.internal/api/v3/router.go (2)
126-138
: PSU routes follow RESTful pattern and are properly structured.The new payment service user routes are well-organized and follow the established RESTful pattern used throughout the router. The routes provide endpoints for creating, listing, and retrieving PSUs, as well as managing their bank accounts.
169-171
: Utility function for extracting payment service user ID is consistent.The
paymentServiceUserID
helper function follows the same pattern as other ID extraction functions in the router, maintaining code consistency.internal/api/services/payment_service_users_list_test.go (1)
14-58
: Well-structured and comprehensive test coverageThe test is well-designed with a table-driven approach, parallel execution, and proper mocking of dependencies. It correctly verifies the behavior of the
PaymentServiceUsersList
method for success and error scenarios, including appropriate error wrapping validation.internal/storage/payments.go (3)
113-113
: Good modernization of error handlingThis change appropriately updates error wrapping to use the standard library's
fmt.Errorf
with%w
verb instead of the externalgithub.heygears.com/pkg/errors
package, following modern Go error handling practices.
294-294
: Good modernization of error handlingThis change correctly updates the error wrapping for validation errors to use the standard library's
fmt.Errorf
with%w
verb, maintaining the same error semantics while reducing external dependencies.
303-303
: Good modernization of error handlingThis change appropriately updates error wrapping for unknown key validation errors to use the standard library's
fmt.Errorf
with%w
verb, consistent with modern Go error handling practices.internal/storage/storage.go (1)
97-102
: LGTM! The new PSU methods follow the storage interface pattern.The addition of Payment Service Users methods to the Storage interface follows the established pattern in the codebase and provides a comprehensive set of operations for managing PSUs and their bank account associations.
pkg/client/models/components/v3address.go (1)
1-55
: Well-structured address model with proper nil safetyThe V3Address struct and its getters follow good Go practices:
- All fields are properly defined as optional pointers with appropriate JSON tags
- Each getter safely handles nil receiver cases, preventing nil pointer panics
- The code structure is consistent throughout the implementation
internal/api/services/payment_service_users_add_bank_account_test.go (1)
1-59
: Well-structured test implementation for PaymentServiceUsersAddBankAccountThis unit test is well-designed, using table-driven tests to verify multiple scenarios: success case, storage not found error, and generic storage error. The test correctly verifies that the service layer properly passes the parameters to storage and wraps storage errors with descriptive messages.
internal/api/v3/handler_payment_service_users_forward_bank_account_to_connector_test.go (1)
18-69
: Comprehensive validation error test casesThe test suite covers all essential validation scenarios for the payment service users forward bank account to connector endpoint, including invalid UUIDs and connector ID validation errors. The use of table-driven tests with
DescribeTable
is a good approach for testing multiple validation scenarios.internal/api/v3/handler_payment_service_users_get_test.go (1)
36-41
: Good error handling for invalid ID parameterThe test correctly verifies that the handler returns a bad request status with the appropriate error code when an invalid UUID is provided.
internal/connectors/engine/workflow/create_bank_account_test.go (6)
49-56
: LGTM: Updated CreateBankAccount struct to use full BankAccount objectThe changes to use the full BankAccount object instead of just the BankAccountID align with the API design change supporting the Payment Service User feature. This modification enables enriching bank account data with PSU information when forwarding to connectors.
72-79
: Consistent API usage across test casesThe changes consistently update all test cases to use the new CreateBankAccount struct format with the full BankAccount object. This ensures test coverage remains comprehensive after the API change.
98-105
: Consistent structure updates across all error test casesThe changes maintain consistency in how the CreateBankAccount struct is initialized across all test cases, which is good practice.
125-132
: Maintaining test coverage for StorageBankAccountsAddRelatedAccount error scenarioThe test correctly continues to validate the error handling when adding related accounts fails, using the updated input structure.
151-158
: Properly updated runSendEvents error test caseThe test for handling errors from the runSendEvents workflow is correctly updated to use the new input structure.
177-184
: Updated StorageTasksStore error test case with new structureThe final test case for task storage errors is properly updated with the new input structure.
internal/connectors/engine/engine_test.go (2)
302-308
: BankAccount parameter change properly implemented in testsThe test setup has been correctly updated to use a complete
models.BankAccount
struct instead of just a UUID, which aligns with the interface changes in the engine'sForwardBankAccount
method.
314-317
: Test cases correctly updated to use the new parameter signatureAll test cases for
ForwardBankAccount
have been properly updated to pass the full bank account object instead of just an ID. The test logic remains unchanged while accommodating the new parameter types, which maintains test coverage for all error cases and success paths.Also applies to: 322-325, 333-336, 349-352, 364-370
internal/api/v3/handler_payment_service_users_add_bank_account.go (1)
13-43
: Well-structured handler with proper validation and tracingThe implementation for adding a bank account to a payment service user follows good practices:
- Uses OpenTelemetry for tracing and setting attributes
- Properly validates UUIDs with appropriate error responses
- Handles service errors through a common error handler
- Returns appropriate HTTP status code (204 No Content) on success
The handler correctly integrates with the backend interface that was extended to support PSU operations.
internal/api/v3/handler_payment_service_users_add_bank_account_test.go (1)
14-63
: Comprehensive test coverage for the PSU add bank account handlerThe test suite effectively covers all critical paths:
- Invalid PSU ID validation
- Invalid bank account ID validation
- Backend error handling
- Successful operation
The tests use appropriate mocking and assertion techniques to verify both the error responses and success case.
internal/connectors/engine/workflow/create_bank_account.go (4)
11-15
: Updated workflow struct to use full BankAccount objectsThe
CreateBankAccount
struct now takes a completemodels.BankAccount
rather than just an ID, which aligns with the broader architectural changes across the codebase. This approach eliminates the need to fetch the bank account separately in the workflow.
47-54
: Simplified workflow implementation with direct object accessThe workflow now uses the bank account object directly from the input parameters instead of fetching it from storage. This change:
- Simplifies the code
- Potentially improves performance by eliminating a database call
- Aligns with the PSU feature implementation across the system
86-93
: Correctly updated reference to use BankAccount.IDReferences to the bank account ID have been properly updated to use the ID field from the bank account object, maintaining the same functionality while adapting to the new parameter structure.
97-114
: Properly passing bank account to child workflowThe bank account is correctly passed to the
SendEvents
child workflow, ensuring that event publishing works with the new parameter structure.pkg/client/models/components/v3addressrequest.go (1)
1-55
: Well-structured address request model with proper nil handling.The V3AddressRequest struct design is appropriate with optional pointer fields and proper JSON tags. The getter methods correctly handle nil receiver cases to prevent nil pointer dereferences, which is a good defensive programming practice.
internal/connectors/engine/activities/storage_payment_service_users_get.go (1)
11-25
: Good implementation of Temporal activity and workflow function.The StoragePaymentServiceUsersGet activity and workflow function follow the established patterns in the codebase. The error handling correctly wraps storage errors, and the workflow function properly executes the activity and returns the result.
internal/api/services/payment_service_users_forward_bank_account.go (1)
27-27
:❌ Incorrect review comment
Verify error handling for FillBankAccountMetadataWithPaymentServiceUserInfo.
The function call to enrich bank account metadata with PSU info doesn't check for potential errors.
🏁 Script executed:
#!/bin/bash # Check if FillBankAccountMetadataWithPaymentServiceUserInfo returns an error that should be handled # Look for the function definition ast-grep --pattern 'func FillBankAccountMetadataWithPaymentServiceUserInfo(bankAccount *BankAccount, psu *PaymentServiceUser) error' # Or ast-grep --pattern 'func FillBankAccountMetadataWithPaymentServiceUserInfo(bankAccount *BankAccount, psu *PaymentServiceUser)'Length of output: 263
To confirm whether
FillBankAccountMetadataWithPaymentServiceUserInfo
actually returns an error (and thus needs handling), let’s locate its definition and signature:This will show where and how the function is declared (including whether it returns an error). Once we have that, we can determine if the call in
internal/api/services/payment_service_users_forward_bank_account.go:27
needs error handling.
🏁 Script executed:
#!/bin/bash # Search for the function signature and its return values rg --color=never -n 'FillBankAccountMetadataWithPaymentServiceUserInfo' .Length of output: 351
No error return from FillBankAccountMetadataWithPaymentServiceUserInfo—no handling needed
The function’s signature in
internal/models/bank_accounts.go:119
is:func FillBankAccountMetadataWithPaymentServiceUserInfo(ba *BankAccount, psu *PaymentServiceUser) { // … }
- It does not return an
error
, so there is nothing to check in
internal/api/services/payment_service_users_forward_bank_account.go:27
.- The existing call is correct and does not require error handling.
You can safely ignore the original suggestion.
Likely an incorrect or invalid review comment.
internal/api/v3/handler_payment_service_users_list.go (2)
14-41
: Good implementation of the PSU list handlerThe handler follows established patterns in the codebase: proper OpenTelemetry tracing, consistent error handling, and effective backend interaction. The pagination extraction and handling is well implemented.
32-37
: Error handling is properly implementedGood use of the
handleServiceErrors
helper to consistently handle backend errors and proper recording of errors with OpenTelemetry.internal/connectors/engine/engine_generated.go (2)
161-173
: Correct update to ForwardBankAccount method signatureThe parameter change from
bankAccountID uuid.UUID
toba models.BankAccount
aligns with the enriched summary, allowing the method to access full bank account details without needing to fetch them separately. This should improve performance and provide better context for the operation.
161-167
:✅ Verification successful
Verify all call sites are updated
Since this is a significant interface change, ensure all call sites that invoke
ForwardBankAccount
have been updated to pass the full bank account model instead of just the ID.
🏁 Script executed:
#!/bin/bash # Description: Find all call sites of ForwardBankAccount echo "Searching for ForwardBankAccount call sites..." rg "ForwardBankAccount" --type go -A 2 -B 2 | grep -v "engine_generated.go"Length of output: 33404
All ForwardBankAccount call sites updated
I’ve verified every invocation ofengine.ForwardBankAccount
now passes a fullmodels.BankAccount
(not just its ID) in:
- Service implementations (
internal/api/services/bank_accounts_forward_to_connector.go
&payment_service_users_forward_bank_account.go
)- Corresponding service tests (both bank-accounts and PSU suites)
- Engine tests (
internal/connectors/engine/engine_test.go
)- Generated mocks (
internal/connectors/engine/engine_generated.go
)No further changes are needed.
internal/api/v3/handler_payment_service_users_create_test.go (5)
37-42
: Good error handling test for missing bodyProper test for the case when the request body is missing, ensuring the handler returns the expected error.
44-53
: Comprehensive validation testingThe table-driven approach is excellent for testing various validation scenarios. Good coverage of different validation error cases.
55-64
: Good error handling test for backend errorsProper test for backend errors, ensuring they're correctly propagated to the client.
66-74
: Verify minimal valid requestGood test for the success case with minimal fields.
76-99
: Comprehensive test with all optional fieldsExcellent test case covering all optional fields, ensuring the handler correctly processes a complete request.
internal/api/backend/backend.go (1)
69-75
: Well-structured interface for PSU operationsThe new Payment Service Users section in the Backend interface provides a comprehensive set of operations for managing PSUs:
- Creation, retrieval, and listing operations
- Associating bank accounts with PSUs
- Forwarding bank accounts to connectors
The method signatures are consistent with the rest of the interface and provide clear, specific functionality.
pkg/client/models/operations/v3createpaymentserviceuser.go (1)
1-36
: LGTM! The response type structure looks well-defined.The
V3CreatePaymentServiceUserResponse
struct is correctly implemented with appropriate fields for HTTP metadata, success response, and error response. The getter methods properly handle nil receivers, returning zero values or nil pointers as appropriate.pkg/client/models/operations/v3addbankaccounttopaymentserviceuser.go (1)
1-49
: LGTM! The request and response model structures are well-defined.The implementation includes:
- A request struct with proper path parameter tags for
PaymentServiceUserID
andBankAccountID
- A response struct with HTTP metadata and error response field
- Comprehensive getter methods that safely handle nil receivers
This structure follows the established pattern for API operations in the codebase.
pkg/client/models/components/v3paymentserviceuser.go (1)
1-79
: LGTM! The PSU model is well-structured with appropriate fields.The
V3PaymentServiceUser
struct includes:
- Essential fields like ID, name, and creation timestamp
- Optional contact details and address as nested structs
- A slice of bank account IDs to represent the one-to-many relationship
- A metadata map for extensibility
- Custom JSON marshaling methods
- Comprehensive getter methods that safely handle nil receivers
This model provides a solid foundation for the payment service user functionality.
pkg/client/models/components/v3forwardpaymentserviceuserbankaccountresponse.go (1)
1-28
: LGTM! The response model appropriately handles the asynchronous nature of the operation.The implementation correctly:
- Documents that forwarding is asynchronous with a resulting task ID
- Provides a clear structure with a data wrapper following API conventions
- Includes proper getter methods that handle nil receivers
This approach allows clients to track the asynchronous forwarding process using the task API.
internal/api/services/bank_accounts_forward_to_connector_test.go (2)
31-71
: LGTM! Test cases now properly handle both storage and engine error scenarios.The test matrix is comprehensive with clear separation between:
- Storage-related errors (not found, generic errors)
- Engine-related errors (validation, not found, generic errors)
- Success cases
This provides better test coverage for the error handling logic in the service method.
76-93
: LGTM! Test implementation correctly enforces the sequence of operations.The test now properly:
- First mocks the storage layer call to retrieve the bank account
- Only mocks the engine call when storage succeeds
- Uses appropriate assertions based on the expected error type
This reflects the actual service implementation flow where storage retrieval happens before engine forwarding.
pkg/client/models/components/v3createpaymentserviceuserrequest.go (2)
5-11
: Well-structured data model for payment service user creation.The struct has a clear definition with appropriate JSON tags and field types. Required fields are properly marked without omitempty tags.
13-18
: Good defensive programming with nil-safe getter methods.These getter methods properly handle nil receivers, which prevents potential nil pointer panics in client code.
Also applies to: 20-25, 27-32, 34-39, 41-46
internal/connectors/engine/engine.go (2)
49-49
: Improved API design by passing full bank account model.Changing the ForwardBankAccount method to accept a complete bank account model (rather than just an ID) is a good design choice that eliminates the need for a separate database lookup, simplifying the workflow and reducing database access.
492-554
: Implementation correctly adapted to use the complete bank account model.The implementation properly uses the passed bank account object, with appropriate changes to task ID generation and workflow parameters.
internal/api/v3/handler_payment_service_users_forward_bank_account_to_connector.go (2)
16-22
: Well-structured request and response models.The request and response structures are clearly defined with appropriate JSON tags and validation tags. The
required
validation on connectorID ensures the request won't proceed without this mandatory field.
24-74
: Well-implemented handler with proper validation, tracing, and error handling.The handler follows best practices:
- Establishes OpenTelemetry tracing
- Validates input parameters
- Records spans with appropriate attributes
- Handles errors at each processing step
- Returns appropriate HTTP status codes
The code structure is clean and follows a logical flow from validation to processing to response.
pkg/client/models/operations/v3getpaymentserviceuser.go (2)
9-19
: Clean request model with proper path parameter annotation.The request struct correctly defines the payment service user ID as a path parameter with appropriate metadata tags, and includes a nil-safe getter method.
21-48
: Well-structured response model with proper error handling.The response model:
- Incorporates both success and error cases
- Includes HTTP metadata
- Provides nil-safe getter methods for all fields
This allows client code to safely access response data without nil pointer risks.
internal/storage/migrations/14-create-psu-tables.sql (1)
1-47
: Well-structured database schema for payment service user entities.The migration script creates two well-designed tables:
payment_service_users
- storing PSU information with encrypted personal datapsu_bank_accounts
- establishing the many-to-many relationship with bank accountsThe design shows good security practices with binary storage for encrypted fields and proper indexing for optimized queries.
internal/api/backend/backend_generated.go (1)
492-563
: Clean implementation of backend mock methods for payment service users.The added mock methods follow the consistent pattern used throughout the file:
PaymentServiceUsersAddBankAccount
PaymentServiceUsersCreate
PaymentServiceUsersForwardBankAccountToConnector
PaymentServiceUsersGet
PaymentServiceUsersList
Each implementation correctly extracts and returns the mock controller's return values.
test/e2e/api_payment_service_users_test.go (4)
1-20
: Well-structured imports for the payment service users test suite.The imports are organized appropriately, separating standard library imports, internal packages, and external dependencies.
90-108
: Test for creating and retrieving a payment service user is comprehensive.The test properly validates:
- Creating a PSU with the API
- Retrieving the PSU by ID
- Verifying the returned data matches expectations
This ensures the create and get functionality works end-to-end.
111-157
: Thorough test coverage for PSU-bank account association.The tests comprehensively cover:
- Adding a bank account to a PSU
- Idempotency (adding the same bank account twice)
- Error handling for non-existent bank accounts
- Error handling for non-existent PSUs
This ensures the association functionality is robust.
160-252
: Comprehensive testing of PSU bank account forwarding to connector.The tests thoroughly validate:
- Error handling for invalid connector IDs
- Error handling for invalid bank account IDs
- Successful forwarding with detailed verification of:
- Task creation
- Bank account data forwarding
- Event publication with correct metadata
- Sensitive data masking in events
The test setup and teardown properly manages connector installation and uninstallation.
pkg/client/models/components/v3paymentserviceuserscursorresponse.go (3)
1-4
: Appropriate package declaration for generated SDK code.This file is correctly identified as generated by Speakeasy and placed in the components package.
5-46
: Well-designed cursor response structure with safe getters.The
V3PaymentServiceUsersCursorResponseCursor
struct is well-designed with:
- Pagination controls (pageSize, hasMore, previous/next tokens)
- Data array containing the actual payment service users
- Safe getter methods that handle nil receivers appropriately
This follows SDK best practices for null safety and usability.
48-57
: Appropriately wrapped cursor response with safe getter.The
V3PaymentServiceUsersCursorResponse
wraps the cursor in a consistent manner with other API responses, and provides a null-safe getter method.internal/models/bank_accounts.go (5)
5-9
: Import additions for new functionality.The additions of the
fmt
package and the pointer utility package are appropriate for supporting the new PSU-related functionality.
16-32
: Well-organized metadata key structure.The metadata keys have been thoughtfully reorganized into clear sections - bank account owner information and account-specific information. The naming convention is consistent and descriptive, making the code more maintainable.
100-116
: Updated metadata key references.The function has been correctly updated to use the newly renamed account-specific metadata keys, maintaining consistency with the metadata key reorganization.
119-141
: Well-implemented PSU metadata enrichment.This new function effectively maps PSU data to bank account metadata, with good handling of optional fields. The address line formatting logic is sensible, combining street number and name when both are available.
143-147
: Good helper function for null safety.The
fillMetadata
helper function is a clean abstraction that ensures metadata is only set when values are non-nil, preventing potential nil dereference issues.pkg/client/models/operations/v3forwardpaymentserviceuserbankaccount.go (3)
1-8
: Generated SDK code with appropriate imports.This file appears to be automatically generated by Speakeasy and includes the necessary import for component models.
9-36
: Well-structured request model with null-safe accessors.The request model clearly defines the required path parameters and request body, with proper documentation. The null-safe getter methods prevent nil pointer dereferences, improving robustness.
38-65
: Well-structured response model with appropriate error handling.The response model appropriately handles both successful and error responses, with null-safe getters. The separation of HTTP metadata from the response body is a good practice.
pkg/client/models/operations/v3listpaymentserviceusers.go (3)
1-8
: Generated SDK code with appropriate imports.This file appears to be automatically generated by Speakeasy and includes the necessary import for component models.
9-37
: Well-designed request model with pagination support.The request model properly implements cursor-based pagination with optional page size and cursor parameters. The null-safe getter methods prevent potential nil pointer issues.
Note that the generic
map[string]any
request body provides flexibility but might make client usage less type-safe. This is likely a design choice of the code generator.
39-66
: Well-structured response model with proper error handling.The response model appropriately handles both successful and error responses with null-safe getters. The cursor-based pagination response structure aligns with best practices for paginated APIs.
pkg/client/docs/sdks/v3/README.md (2)
44-48
: SDK documentation is appropriately updated with new PSU operations.The SDK documentation has been correctly updated to include the new Payment Service User (PSU) operations, which aligns with the feature implementation described in the PR.
1811-2075
: Comprehensive documentation for new PSU endpoints follows established patterns.The detailed documentation for each new PSU operation is well-structured and follows the same pattern as existing operations, including example usage, parameter tables, response types, and error information. This consistency helps maintain a cohesive developer experience.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1821-1821: Hard tabs
Column: 1(MD010, no-hard-tabs)
1822-1822: Hard tabs
Column: 1(MD010, no-hard-tabs)
1823-1823: Hard tabs
Column: 1(MD010, no-hard-tabs)
1824-1824: Hard tabs
Column: 1(MD010, no-hard-tabs)
1873-1873: Hard tabs
Column: 1(MD010, no-hard-tabs)
1874-1874: Hard tabs
Column: 1(MD010, no-hard-tabs)
1875-1875: Hard tabs
Column: 1(MD010, no-hard-tabs)
1876-1876: Hard tabs
Column: 1(MD010, no-hard-tabs)
1927-1927: Hard tabs
Column: 1(MD010, no-hard-tabs)
1928-1928: Hard tabs
Column: 1(MD010, no-hard-tabs)
1929-1929: Hard tabs
Column: 1(MD010, no-hard-tabs)
1930-1930: Hard tabs
Column: 1(MD010, no-hard-tabs)
1979-1979: Hard tabs
Column: 1(MD010, no-hard-tabs)
1980-1980: Hard tabs
Column: 1(MD010, no-hard-tabs)
1981-1981: Hard tabs
Column: 1(MD010, no-hard-tabs)
1982-1982: Hard tabs
Column: 1(MD010, no-hard-tabs)
2032-2032: Hard tabs
Column: 1(MD010, no-hard-tabs)
2033-2033: Hard tabs
Column: 1(MD010, no-hard-tabs)
2034-2034: Hard tabs
Column: 1(MD010, no-hard-tabs)
2035-2035: Hard tabs
Column: 1(MD010, no-hard-tabs)
internal/models/psu.go (4)
10-22
: Well-structured optional field models with proper JSON tags.The
Address
andContactDetails
structs are designed appropriately with pointer fields to make them truly optional in the JSON representation. This allows for partial updates and optional fields in API requests/responses.
24-34
: Clean main PaymentServiceUser model structure with appropriate field types.The PaymentServiceUser model appropriately uses UUID for IDs, includes required and optional fields, and has proper JSON tags for serialization. The slice of bank account UUIDs and metadata map provide good flexibility for user-bank account relationships.
36-63
: Proper JSON marshaling with UUID string conversion and empty slice handling.The custom MarshalJSON method correctly converts UUIDs to strings and handles empty slices by returning nil instead of an empty array, which is good for consistent API responses.
87-94
: Good handling of nil slices for BankAccountIDs.The code properly handles nil slices for BankAccountIDs, ensuring that empty arrays are represented as nil rather than an empty slice. This is consistent with the marshaling approach.
internal/storage/psu.go (1)
191-195
: Verify ordering columnsort_id
query.Order("created_at DESC", "sort_id DESC")
refers tosort_id
, which does not exist in the table definition and will cause a SQL error at runtime unless added by a trigger/view. Please remove or replace with a real column.internal/storage/storage_generated.go (1)
832-888
: Generated mocks look goodThe newly-added PSU mock methods follow gomock conventions and compile-time contracts. No issues spotted.
pkg/client/v3.go (6)
7371-7592
: Well-implemented client method for creating payment service usersThis method follows the established SDK pattern for API calls and properly handles request formation, error cases, and response parsing. It correctly implements the CreatePaymentServiceUser endpoint with appropriate content types, status code handling, and error responses.
7594-7825
: Good implementation of pagination support for listing payment service usersThe implementation correctly handles pagination parameters (pageSize and cursor) and properly deserializes the cursor-based response format. Error handling is comprehensive with appropriate status code management.
7827-8046
: Well-structured implementation for retrieving a payment service user by IDThe method properly handles path parameters, sets appropriate headers, and follows the SDK's established patterns for response parsing and error handling. The implementation is consistent with other "Get" operations in the codebase.
8047-8246
: Clean implementation of bank account association functionalityThis method properly handles the association between a payment service user and a bank account. The implementation uses appropriate HTTP status codes (204 for success with no content) and follows established error handling patterns.
8248-8475
: Complete implementation of bank account forwarding functionalityThe method correctly handles the complex request structure with nested path parameters and request body. The response handling is thorough with proper deserialization of the accepted (202) response. All error cases are appropriately managed.
7371-8475
: Excellent extension of the V3 client with Payment Service User capabilitiesThe added methods form a comprehensive and consistent API for managing payment service users and their bank accounts. The implementation follows established SDK patterns throughout, with proper error handling, timeout management, retry logic, and response parsing. The methods integrate well with the existing codebase and maintain the same level of quality and consistency.
Key strengths:
- Consistent implementation pattern across all methods
- Proper handling of pagination for list operations
- Comprehensive error scenarios for all HTTP status code ranges
- Clean integration with the SDK's security, retry, and hook infrastructure
- Appropriate serialization/deserialization of request and response bodies
These additions enhance the SDK with a complete set of operations for the new PSU entity without disrupting the existing code structure.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #424 +/- ##
==========================================
+ Coverage 60.93% 61.12% +0.18%
==========================================
Files 566 579 +13
Lines 28843 29418 +575
==========================================
+ Hits 17576 17982 +406
- Misses 10317 10470 +153
- Partials 950 966 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 0
🧹 Nitpick comments (1)
internal/api/validation/validation_test.go (1)
289-295
: Email validation test is implemented correctly.The test validates a standard email format, which is sufficient for basic testing.
Consider adding a few more test cases for different email formats (with subdomains, with plus addressing, etc.) to ensure the validation regex handles a wider variety of valid formats.
It("email supports expected values", func(ctx SpecContext) { _, err := validate.Validate(CustomStruct{ Email: "dev@formance.com", EmailNullable: pointer.For("dev@formance.com"), }) Expect(err).To(BeNil()) + + _, err = validate.Validate(CustomStruct{ + Email: "dev+test@formance.com", + EmailNullable: pointer.For("dev+test@formance.com"), + }) + Expect(err).To(BeNil()) + + _, err = validate.Validate(CustomStruct{ + Email: "dev@sub.formance.com", + EmailNullable: pointer.For("dev@sub.formance.com"), + }) + Expect(err).To(BeNil()) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
internal/api/v3/handler_payment_service_users_create.go
(1 hunks)internal/api/v3/handler_payment_service_users_create_test.go
(1 hunks)internal/api/validation/checkers.go
(2 hunks)internal/api/validation/validation.go
(1 hunks)internal/api/validation/validation_test.go
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/api/v3/handler_payment_service_users_create_test.go
- internal/api/v3/handler_payment_service_users_create.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/api/validation/validation.go (1)
internal/api/validation/checkers.go (2)
IsPhoneNumber
(149-156)IsEmail
(158-165)
🔇 Additional comments (8)
internal/api/validation/validation.go (1)
45-46
: Custom validation registered correctly.The new validators for phoneNumber and email are properly registered and follow the established pattern for custom validators in the codebase.
internal/api/validation/checkers.go (3)
12-15
: Regular expressions for validation look good.The regular expressions for phone number and email validation are defined at the package level, which is appropriate for these compile-once, use-many patterns.
149-156
: Phone number validation is implemented correctly.The implementation follows the established pattern in the codebase, properly handling type conversion and validation against the regular expression.
158-165
: Email validation is implemented correctly.The implementation follows the established pattern in the codebase, properly handling type conversion and validation against the regular expression.
internal/api/validation/validation_test.go (4)
43-46
: Test struct updated correctly with new fields.The
CustomStruct
type has been properly extended with the new fields for phone number and email validation testing.
178-194
: Comprehensive test cases for invalid phone numbers.The test cases cover a variety of scenarios including invalid values in both required and optional fields, for both string and pointer types, as well as unsupported types.
195-210
: Comprehensive test cases for invalid emails.The test cases cover a variety of scenarios including invalid values in both required and optional fields, for both string and pointer types, as well as unsupported types.
264-288
: Phone number validation tests are thorough.The tests validate a good variety of phone number formats, including international numbers with country codes, local numbers, and numbers with different separator styles.
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
🔭 Outside diff range comments (1)
internal/storage/bank_accounts.go (1)
276-284
:⚠️ Potential issueMissing PSU ID initialization in the model conversion.
While the PsuID field is initialized to nil in fromBankAccountModels, the toBankAccountModels function (lines 307-335) doesn't handle the PSU relationship when converting back to the model representation. This means the PSU association information will be lost when retrieving bank accounts.
Add PSU ID to the model conversion:
func toBankAccountModels(from bankAccount) models.BankAccount { ba := models.BankAccount{ ID: from.ID, CreatedAt: from.CreatedAt.Time, Name: from.Name, Country: from.Country, Metadata: from.Metadata, + PsuID: from.PsuID, }
Make sure to also update the BankAccount model in internal/models to include the PsuID field.
♻️ Duplicate comments (1)
internal/storage/psu.go (1)
108-111
:⚠️ Potential issueDouble commit issue in transaction handling.
This was previously flagged in a review: on commit failure you're calling
tx.Commit()
twice - once to geterr
and a second time inside thee()
function. The second call will returnsql.ErrTxDone
, which masks the real error.Fix the double commit issue:
-if err := tx.Commit(); err != nil { - errTx = err - return e("commit transaction", err) -} +if err := tx.Commit(); err != nil { + errTx = err + return e("commit transaction: %w", err) +}
🧹 Nitpick comments (2)
internal/storage/psu.go (2)
43-56
: Inconsistent transaction rollback pattern.The transaction rollback pattern here differs from the pattern used in bank_accounts.go. In bank_accounts.go, you use the
rollbackOnTxError
function, but here you manually checkerrTx != nil
and calltx.Rollback()
. For consistency, consider using the same pattern across all files.Use the common rollback pattern:
var errTx error defer func() { - if errTx != nil { - if err := tx.Rollback(); err != nil { - s.logger.Errorf("failed to rollback transaction: %v", err) - } - } + rollbackOnTxError(ctx, &tx, errTx) }()
220-240
: Repetitive error handling in AddBankAccount method.The error handling logic for checking RowsAffected is duplicated from the PSU creation method. Consider extracting this common pattern into a helper function for reuse.
Create a helper function:
func (s *store) updateBankAccountWithPSU(ctx context.Context, psuID, bankAccountID uuid.UUID) error { res, err := s.db.NewUpdate(). Model((*bankAccount)(nil)). Set("psu_id = ?", psuID). Where("id = ?", bankAccountID). Exec(ctx) if err != nil { return e("update bank account to add psu id", err) } rowsAffected, err := res.RowsAffected() if err != nil { return e("update bank account to add psu id", err) } if rowsAffected == 0 { return e("bank account", ErrNotFound) } return nil }Then reuse this function in both PaymentServiceUsersCreate and PaymentServiceUsersAddBankAccount.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
internal/storage/bank_accounts.go
(6 hunks)internal/storage/migrations/14-create-psu-tables.sql
(1 hunks)internal/storage/psu.go
(1 hunks)internal/storage/psu_test.go
(1 hunks)test/e2e/api_payment_service_users_test.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- test/e2e/api_payment_service_users_test.go
- internal/storage/psu_test.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/storage/psu.go (1)
internal/models/psu.go (2)
PaymentServiceUser
(24-34)ContactDetails
(19-22)
🔇 Additional comments (12)
internal/storage/migrations/14-create-psu-tables.sql (2)
1-26
: Well-structured PSU table definition with encryption for sensitive data.The payment_service_users table appropriately uses the bytea type for encrypted fields (name, address components, contact details). The structure includes mandatory fields (id, created_at), encrypted fields for PII, and optional fields with defaults.
28-29
: Good index choice for pagination.The index on (created_at, sort_id) will optimize the common query pattern of retrieving PSUs ordered by creation time, especially when paginating results.
internal/storage/bank_accounts.go (5)
37-41
: Good integration of PSU relationship in bank account model.The bank account model has been properly updated with nullable PsuID field and a bidirectional relationship with the PSU entity. The
scanonly
tag ensures these relationships are only populated when explicitly loaded.
48-52
: Improved rollback pattern with local error variable.Using a local
errTx
variable for tracking transaction errors is a good practice. The variable is properly used in the deferred rollback function.
57-59
: Column list updated to include psu_id.The Column list is properly updated to include the new psu_id field when upserting bank accounts.
144-145
: Column list updated to include psu_id in BankAccountsGet.The Column list is correctly updated to retrieve the psu_id field when getting bank accounts.
175-176
: Filter support added for psu_id.The query builder properly supports filtering by psu_id using the $match operator.
internal/storage/psu.go (5)
15-38
: Well-structured PSU storage model.The paymentServiceUser struct is correctly defined with encrypted fields for sensitive data, appropriate field tags for the Bun ORM, and a relationship to bank accounts.
57-80
: Effective use of raw SQL for encrypted field insertion.The raw SQL query efficiently handles the encryption of multiple fields in a single operation. The parameter placeholders are clear and the ON CONFLICT clause handles idempotent inserts appropriately.
82-106
: Appropriate handling of bank account relationship updates.The code correctly updates bank account records to associate them with the PSU, and properly handles the case where the bank account doesn't exist.
299-312
: Efficient null checking for optional address fields.The fillAddress function appropriately checks if all address fields are nil before returning nil, avoiding the creation of empty address objects.
314-323
: Efficient null checking for optional contact fields.The fillContactDetails function appropriately checks if all contact fields are nil before returning nil, avoiding the creation of empty contact objects.
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 (3)
internal/storage/psu_test.go (3)
365-379
: Prevent possible nil-pointer dereference incompareCounterPartiesAddressed
If
expected
oractual
isnil
, thedefault
arm of theswitch
marks the test failed but continues to dereference both pointers, which is exactly what static-check SA5011 is warning about.
Early-return right after the failure (or move the field-by-field assertions inside theexpected != nil && actual != nil
branch).default: require.Fail(t, "Address is different") + return }
381-392
: Replicate the nil-check fix forcompareCounterPartiesContactDetails
Same defect – after
require.Fail
the function continues and may dereferencenil
. Addreturn
after the failure.default: require.Fail(t, "ContactDetails is different") + return }
394-404
: Avoid nil-dereference incompareInterface
compareInterface
has the identical control-flow problem. Either:
return
immediately afterrequire.Failf
, or- Move the
require.Equal
inside the(expected != nil && actual != nil)
branch.default: require.Failf(t, "%s field is different", name) + return }
🧹 Nitpick comments (3)
internal/storage/psu_test.go (3)
100-101
: Fix naming inconsistency.The comment refers to "counter party" but the variable and struct are named "PSU" (Payment Service User). This appears to be a copy-paste error.
- // Should not update the counter party + // Should not update the PSU
104-115
: Improve error test case naming.The current test name "unknown bank account id id" has a duplicated "id" word and doesn't clearly indicate the expected behavior.
- t.Run("unknown bank account id id", func(t *testing.T) { + t.Run("should error when using unknown bank account id", func(t *testing.T) {
343-363
: Consider using testify's require.ElementsMatch for arrays.For bank account IDs comparison, the current comparison assumes the order is preserved. If order doesn't matter, consider using
require.ElementsMatch
instead, which is more flexible.- require.Equal(t, len(expected.BankAccountIDs), len(actual.BankAccountIDs)) - for i := range expected.BankAccountIDs { - require.Equal(t, expected.BankAccountIDs[i], actual.BankAccountIDs[i]) - } + require.ElementsMatch(t, expected.BankAccountIDs, actual.BankAccountIDs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
internal/storage/bank_accounts.go
(6 hunks)internal/storage/migrations/14-create-psu-tables.sql
(1 hunks)internal/storage/psu.go
(1 hunks)internal/storage/psu_test.go
(1 hunks)test/e2e/api_payment_service_users_test.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- test/e2e/api_payment_service_users_test.go
- internal/storage/bank_accounts.go
- internal/storage/migrations/14-create-psu-tables.sql
- internal/storage/psu.go
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
primary key (psu_id, bank_account_id) | ||
); | ||
alter table bank_accounts | ||
add column if not exists psu_id uuid default null; |
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.
I think you'll get the same effect without aggressively locking the entire table if you change the alter table to this:
add column if not exists psu_id uuid;
Not defining a default means that value will be null by default. But I think the moment you mention the DEFAULT
keyword in an alter table postgres will choose an access exclusive lock.
Description
This PR adds a new payments entity called PSU: Payment Service User
Payment Service User
A Payment Service User is either a person or a business that uses a payment service to view, send or receive money.
In our model, a psu has a name, and potentially some contact details like a phoneNumber or an email, and also an address.
A PSU can also be linked to a variety of other entities: bank accounts, cards etc... For now in our models, it will be linked to bank accounts.
API
This PR adds multiple api endpoints for a psu:
Tests
This PR also adds unit tests for every package as well as integration tests using the SDK generated with the last version of the openapi which contains the psu endpoints and schemas
Fixes PMNT-53