fix(*): fix release and v2 handlers #221
Conversation
WalkthroughThe pull request introduces comprehensive modifications across multiple files in the FormanceHQ payments project. The changes primarily focus on enhancing provider and payment scheme handling, standardizing connector and payment method representations, and updating import mechanisms for payment plugins. Key modifications include the introduction of new utility functions for provider and scheme transformations, the addition of constants for various payment connectors and schemes, and updates to the Changes
Sequence DiagramsequenceDiagram
participant Client
participant APIHandler
participant UtilityFunctions
participant Backend
Client->>APIHandler: Send Request
APIHandler->>UtilityFunctions: Transform Provider
UtilityFunctions-->>APIHandler: Processed Provider
APIHandler->>UtilityFunctions: Transform Payment Scheme
UtilityFunctions-->>APIHandler: Processed Scheme
APIHandler->>Backend: Process Request
Backend-->>APIHandler: Return Response
APIHandler-->>Client: Send Response
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
paul-nicolas
force-pushed
the
fix/release
branch
2 times, most recently
from
d16c439 to
89bbec5
Compare
Multiple fixes: - Since gorelease is not using earthly, we need to have the list.go already filled before releasing. I added it in the lint/pre-commit method on earthly, so that the CI fails if not up to date. - Remove a useless temporal search attributes - Do the right translation of connectors for v2 - Do the right translation for payment scheme for v2
paul-nicolas
force-pushed
the
fix/release
branch
from
89bbec5 to
7407d8f
Compare
paul-nicolas
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
internal/api/v2/handler_bank_accounts_get.go (1)
Line range hint
20-63: Consider documenting the V2/V3 provider transformation strategyThe codebase shows a pattern of V2/V3 provider transformations across multiple handlers. This suggests either:
- A transition between API versions
- A need to maintain compatibility between different provider formats
Consider:
- Documenting the transformation strategy
- Adding version compatibility tests
- Planning for eventual deprecation of older versions if applicable
internal/api/v2/handler_connectors_list.go (1)
Line range hint
39-42: Consider pagination improvements in next API version.The current implementation fetches all records (pageSize: 1000) to maintain backward compatibility. Consider adding proper pagination support in the next API version for better scalability.
internal/api/v2/handler_bank_accounts_list.go (1)
Line range hint
21-31: Consider extracting pagination logic.The pagination extraction and error handling logic could be moved to a shared utility function to improve code reusability across handlers.
+func extractPagination[T any](span trace.Span, r *http.Request, query T) (*storage.ListBankAccountsQuery, error) { + options, err := getPagination(span, r, query) + if err != nil { + otel.RecordError(span, err) + return nil, err + } + return pointer.For(storage.NewListBankAccountsQuery(*options)), nil +} + func bankAccountsList(backend backend.Backend) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx, span := otel.Tracer().Start(r.Context(), "v2_bankAccountsList") defer span.End() - query, err := bunpaginate.Extract[storage.ListBankAccountsQuery](r, func() (*storage.ListBankAccountsQuery, error) { - options, err := getPagination(span, r, storage.BankAccountQuery{}) - if err != nil { - otel.RecordError(span, err) - return nil, err - } - return pointer.For(storage.NewListBankAccountsQuery(*options)), nil - }) + query, err := bunpaginate.Extract[storage.ListBankAccountsQuery](r, + func() (*storage.ListBankAccountsQuery, error) { + return extractPagination(span, r, storage.BankAccountQuery{}) + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
Earthfile(1 hunks)internal/api/v2/handler_accounts_get.go(1 hunks)internal/api/v2/handler_accounts_list.go(1 hunks)internal/api/v2/handler_bank_accounts_get.go(1 hunks)internal/api/v2/handler_bank_accounts_list.go(1 hunks)internal/api/v2/handler_connectors_install.go(1 hunks)internal/api/v2/handler_connectors_list.go(1 hunks)internal/api/v2/handler_payments_create.go(1 hunks)internal/api/v2/handler_payments_get.go(1 hunks)internal/api/v2/handler_payments_list.go(1 hunks)internal/api/v2/handler_transfer_initiations_get.go(1 hunks)internal/api/v2/utils.go(2 hunks)internal/connectors/engine/search_attributes.go(0 hunks)internal/connectors/engine/workflow/workflow.go(0 hunks)internal/connectors/plugins/public/list.go(1 hunks)
💤 Files with no reviewable changes (2)
- internal/connectors/engine/search_attributes.go
- internal/connectors/engine/workflow/workflow.go
✅ Files skipped from review due to trivial changes (1)
- internal/connectors/plugins/public/list.go
🔇 Additional comments (13)
internal/api/v2/handler_connectors_install.go (1)
20-22: Verify provider transformation chain and telemetry order
The changes introduce a new transformation chain for the provider value. Please verify:
- The provider transformation order (toV3Provider → toLowerCase)
- The placement of span.SetAttributes after the transformation
✅ Verification successful
Let me gather more information about the toV3Provider function implementation to understand the transformation logic and potential null cases.
Let me try a different approach to inspect the code:
After reviewing the implementation, I can now provide the final response:
Provider transformation is correctly implemented and ordered
The changes are safe and follow a consistent pattern:
- The
toV3Providerfunction has proper handling for all connector types with no null cases - The transformation order (toV3Provider → toLowerCase) is correct as it normalizes v2 connector names (e.g., "STRIPE") to v3 format (e.g., "stripe")
- The span attribute placement after the transformation is appropriate as it captures the final normalized value
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of toV3Provider to verify consistent transformation order
rg "toV3Provider" -A 2
# Check if there are any direct toLowerCase calls on provider elsewhere
rg "strings\.ToLower.*provider" -A 2
# Verify the placement of span.SetAttributes in other handlers
rg "span\.SetAttributes.*provider" -A 2
Length of output: 1536
Script:
#!/bin/bash
# Check the implementation of toV3Provider function
ast-grep --pattern 'func toV3Provider($$$) {
$$$
}'
# Check the implementation of connectorProvider function
ast-grep --pattern 'func connectorProvider($$$) {
$$$
}'
# Check constants used in provider mapping
rg "const.*connector.*=" -A 5 internal/api/v2/
Length of output: 165
Script:
#!/bin/bash
# Check the content of utils.go where toV3Provider is defined
cat internal/api/v2/utils.go
# Look for connector-related constants in the entire v2 package
rg "const.*\(" -A 10 internal/api/v2/
# Check the file content to understand the context
cat internal/api/v2/handler_connectors_install.go
Length of output: 10079
internal/api/v2/handler_accounts_get.go (1)
39-39: Verify consistency between V2 and V3 provider transformations
The code uses toV2Provider while the connectors handler uses toV3Provider. Please verify:
- The relationship between V2 and V3 provider transformations
- The expected provider format in API responses
internal/api/v2/handler_bank_accounts_get.go (1)
63-63: Consider performance impact of provider transformation in loop
The toV2Provider transformation is applied within a loop over related accounts. Please verify:
- The performance impact of repeated transformations
- Whether the transformation could be moved outside the loop if the provider is the same for all related accounts
internal/api/v2/handler_connectors_list.go (1)
47-47: Verify the toV2Provider function implementation.
The code introduces a dependency on the toV2Provider function. Let's verify its implementation to ensure consistent provider transformation across the codebase.
✅ Verification successful
The toV2Provider function is properly implemented and consistently used
The verification shows that toV2Provider is well-defined in internal/api/v2/utils.go and handles provider name standardization for all supported payment providers (Adyen, Stripe, Wise, etc.). The function is consistently used across multiple handlers in the v2 API for provider name transformation, including:
- Bank accounts (list and get)
- Payments (list and get)
- Transfer initiations
- Accounts (list and get)
- Connectors list
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for toV2Provider function definition and its usages
echo "Searching for toV2Provider function definition:"
ast-grep --pattern 'func toV2Provider($_) $_ {
$$$
}'
echo -e "\nSearching for all toV2Provider usages:"
rg 'toV2Provider\(' -A 2
Length of output: 3989
internal/api/v2/handler_payments_get.go (1)
38-38: Verify transformation functions implementation.
The code introduces dependencies on both toV2Provider and toV2PaymentScheme functions. Let's verify their implementations to ensure consistent data transformation.
Also applies to: 43-43
✅ Verification successful
Let me verify the toV2Provider function implementation as well before making a final assessment.
Both transformation functions are properly implemented and consistently used
The verification shows that both toV2Provider and toV2PaymentScheme functions are:
- Well-implemented with comprehensive switch statements covering all expected cases
- Consistently used across multiple handlers (payments, accounts, transfers)
- Handle edge cases appropriately (unknown/default cases)
- Maintain proper type safety through explicit mapping
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for toV2PaymentScheme function definition and its usages
echo "Searching for toV2PaymentScheme function definition:"
ast-grep --pattern 'func toV2PaymentScheme($_) $_ {
$$$
}'
echo -e "\nSearching for all toV2PaymentScheme usages:"
rg 'toV2PaymentScheme\(' -A 2
Length of output: 4584
Script:
#!/bin/bash
# Search for toV2Provider function definition and its usages
echo "Searching for toV2Provider function definition:"
ast-grep --pattern 'func toV2Provider($_) $_ {
$$$
}'
echo -e "\nSearching for all toV2Provider usages:"
rg 'toV2Provider\(' -A 2
Length of output: 3989
internal/api/v2/handler_bank_accounts_list.go (1)
73-73: Consistent provider transformation applied.
The change aligns with the standardization of provider handling across the codebase using toV2Provider.
internal/api/v2/handler_accounts_list.go (1)
66-66: Verify provider transformation compatibility
The introduction of toV2Provider transformation could affect API response compatibility. Need to ensure that transformed values maintain backward compatibility with existing API consumers.
Let's verify the provider transformation implementation and its usage:
✅ Verification successful
Provider transformation is safe and maintains compatibility
The toV2Provider function in utils.go implements a straightforward mapping that maintains backward compatibility by:
- Using standardized constants for known providers
- Falling back to the original provider value for unknown cases
- Preserving the provider field structure across all v2 API responses
The transformation is used consistently in the accounts list handler and follows the same pattern used throughout the v2 API, where Provider is a standard field in response structures.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify toV2Provider implementation and its impact
# Look for toV2Provider implementation
ast-grep --pattern 'func toV2Provider($_) $_ {
$$$
}'
# Check for all provider field assignments in v2 handlers
rg 'Provider.*=.*toV2Provider' internal/api/v2/
Length of output: 1474
Script:
#!/bin/bash
# Let's verify the connector constants and their usage
ast-grep --pattern 'const connector$_ = "$_"'
# Check for all provider field assignments in v2 handlers to ensure consistent usage
rg 'Provider.*=' internal/api/v2/
# Check if there are any v1 provider mappings for compatibility verification
rg -g '*.go' 'Provider.*=' internal/api/v1/
Length of output: 1730
Script:
#!/bin/bash
# Let's check the constants defined in the utils file
rg 'const connector' internal/api/v2/utils.go
# Check where the Provider field is used in v2 API
rg -B 2 -A 2 'Provider.*=' internal/api/v2/
# Look for any provider-related types or interfaces
ast-grep --pattern 'type $_ struct {
$$$
Provider $$$
$$$
}'
Length of output: 16431
internal/api/v2/handler_payments_list.go (1)
76-76: Verify provider and scheme transformations
Two new transformations have been introduced:
- Provider transformation using
toV2Provider - Scheme transformation using
toV2PaymentScheme
Need to ensure consistent transformation behavior across all payment-related endpoints.
Let's verify the implementation and usage of both transformations:
Also applies to: 81-81
✅ Verification successful
Provider and scheme transformations are consistently implemented
The verification shows:
-
toV2PaymentSchemeis consistently used across all payment-related endpoints:- payments_list.go
- payments_get.go
- payments_create.go
-
toV2Provideris uniformly applied across all endpoints that expose provider information:- accounts_list.go
- accounts_get.go
- payments_list.go
- payments_get.go
- bank_accounts_list.go
- bank_accounts_get.go
- connectors_list.go
- transfer_initiations_get.go
The transformations maintain consistency in the API response format across all endpoints.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify transformation implementations and their usage across handlers
# Look for toV2PaymentScheme implementation
ast-grep --pattern 'func toV2PaymentScheme($_) $_ {
$$$
}'
# Check for scheme transformations in other handlers
rg 'Scheme.*=.*toV2PaymentScheme' internal/api/v2/
# Verify consistent usage in payment-related handlers
rg -A 5 'type.*Response.*struct.*{' internal/api/v2/ | rg -A 2 'Provider|Scheme'
Length of output: 3620
Script:
#!/bin/bash
# Let's verify the transformations more effectively
# Check for scheme transformations in handlers
rg "toV2PaymentScheme" internal/api/v2/
# Check for provider transformations in handlers
rg "toV2Provider" internal/api/v2/
# Look for response structs with Provider or Scheme fields
rg -l "type.*Response.*struct" internal/api/v2/ | while read -r file; do
echo "=== $file ==="
rg "type.*Response.*struct" -A 10 "$file"
done
Length of output: 8637
Earthfile (1)
100-105: Ensure plugin initialization file is properly managed
The changes ensure that list.go (plugin initialization file) is:
- Generated during compilation
- Included in linting
- Saved as a local artifact
This is critical for proper plugin initialization, but we should verify the plugin discovery process.
Let's verify the plugin management setup:
✅ Verification successful
Let me fix the regex and gather more information about the plugin initialization:
Plugin initialization setup is correctly managed
The verification confirms that the plugin system is properly structured:
- Each plugin has a consistent initialization pattern using
init()functions - The
list.gofile correctly imports all available plugins (adyen, atlar, bankingcircle, etc.) - All plugins are registered using
registry.RegisterPluginwith consistent parameters - Each plugin has its configuration file (
config.json) and follows the same directory structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify plugin management and initialization
# Check for plugin registration patterns
rg -g '!vendor' 'func init\(\).*{' internal/connectors/plugins/public/
# Verify plugin directory structure
fd -t d . internal/connectors/plugins/public/
# Check for potential plugin configuration files
fd 'config.json$' internal/connectors/plugins/public/
Length of output: 2190
Script:
#!/bin/bash
# Check for plugin registration patterns with fixed regex
rg -g '!vendor' 'func init\(\)' internal/connectors/plugins/public/ -A 5
# Check the content of list.go to understand plugin initialization
cat internal/connectors/plugins/public/list.go
# Look for plugin registration mechanism
rg -g '!vendor' 'Register.*Plugin' internal/connectors/plugins/
Length of output: 8265
internal/api/v2/handler_transfer_initiations_get.go (1)
158-158: LGTM! Provider name standardization
The change to use toV2Provider aligns with the standardization of provider names across the codebase.
internal/api/v2/utils.go (2)
46-58: LGTM! Well-structured provider constants
Provider constants follow a consistent naming pattern and are appropriately centralized.
118-198: LGTM! Comprehensive payment scheme handling
The payment scheme constants and mapping function provide thorough coverage of all supported schemes with appropriate default handling.
internal/api/v2/handler_payments_create.go (1)
196-196: LGTM! Payment scheme standardization
The change to use toV2PaymentScheme aligns with the standardization of payment schemes across the codebase while maintaining backward compatibility.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/api/v2/utils.go (1)
145-198: Consider adding reverse mapping and validationThe payment scheme mapping is comprehensive but could be enhanced:
- Consider adding a reverse mapping function (
toV3PaymentScheme) for consistency with the provider mapping pattern- Add validation to ensure the input scheme is valid before mapping
Example implementation:
func toV3PaymentScheme(scheme string) (models.PaymentScheme, error) { switch scheme { case paymentSchemeVisa: return models.PAYMENT_SCHEME_CARD_VISA, nil // ... other cases ... default: return models.PAYMENT_SCHEME_UNKNOWN, fmt.Errorf("unknown payment scheme: %s", scheme) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/api/v2/utils.go(2 hunks)
🔇 Additional comments (3)
internal/api/v2/utils.go (3)
46-58: LGTM: Well-structured connector constants
The connector constants follow a consistent naming pattern and use standardized provider names.
118-143: LGTM: Comprehensive payment scheme constants
The payment scheme constants are well-organized, follow consistent naming patterns, and provide good coverage of industry-standard payment methods.
60-87: Consider adding input validation and verify mapping symmetry
While the provider mappings look correct, consider:
- Adding input validation to handle empty strings or unexpected formats
- Ensuring that all mappings are symmetric between v2 and v3
Also applies to: 89-116
✅ Verification successful
The previous script had issues with the pattern matching. Let me try a different approach to verify the mapping symmetry.
Let me try one more approach to verify the mappings by looking at the actual code content.
After analyzing the file content, I can now generate the final response:
Provider mappings are symmetric and correctly implemented
The code shows that:
- All provider mappings in
toV2ProviderandtoV3Providerare symmetric (e.g., "adyen" <-> "ADYEN", "atlar" <-> "ATLAR", etc.) - The Atlar case is correctly implemented
- Default cases in both functions preserve unknown values, which is a good practice
- Input validation for empty strings is not strictly necessary as the default case handles it gracefully by returning the input value
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify symmetry of provider mappings
# Expected: Each v2->v3->v2 and v3->v2->v3 mapping should return the original value
# Extract v2 constants
echo "Checking v2->v3->v2 mappings..."
rg -N "^[\t ]*connector\w+\s*string\s*=\s*\"([^\"]+)\"" -r '$1' | while read -r v2_provider; do
echo "Testing $v2_provider..."
# Find corresponding v3 mapping
v3_mapping=$(rg -N "case $v2_provider:\s*return \"([^\"]+)\"" -r '$1')
# Find mapping back to v2
v2_return=$(rg -N "case \"$v3_mapping\":\s*return\s+connector\w+" -r '$1')
if [ "$v2_provider" != "$v2_return" ]; then
echo "Asymmetric mapping: $v2_provider -> $v3_mapping -> $v2_return"
fi
done
Length of output: 608
Script:
#!/bin/bash
# First, let's get the actual constant definitions
echo "=== Provider Constants ==="
rg "connector\w+\s*=\s*\"" internal/api/v2/utils.go
echo -e "\n=== V2 to V3 Mappings ==="
# Extract the v2->v3 mappings from toV3Provider function
rg "case \".*\":\s*return\s+connector\w+" internal/api/v2/utils.go
echo -e "\n=== V3 to V2 Mappings ==="
# Extract the v3->v2 mappings from toV2Provider function
rg "case \".*\":\s*return\s+connector\w+" internal/api/v2/utils.go
Length of output: 383
Script:
#!/bin/bash
# Let's first see the actual content of the file
echo "=== File Content ==="
cat internal/api/v2/utils.go
# Also check if there are any other files that might contain these mappings
echo -e "\n=== Related Files ==="
rg -l "connector\w+" --type go
Length of output: 28717
Multiple fixes:
to have the list.go already filled before releasing.
I added it in the lint/pre-commit method on earthly,
so that the CI fails if not up to date.