Refactor metadata extraction, fix retryable errors & createdAt format

Author: gitstart_bot

internal/connectors/engine/activities/errors.go

@@ -41,6 +41,8 @@ func (a Activities) temporalPluginErrorCheck(ctx context.Context, err error, isP
 		return temporal.NewNonRetryableApplicationError(err.Error(), ErrTypeInvalidArgument, err)
 	case errors.Is(err, models.ErrMissingConnectorMetadata):
 		return temporal.NewNonRetryableApplicationError(err.Error(), ErrTypeInvalidArgument, err)
+	case errors.As(err, &models.NonRetryableError):
+		return temporal.NewNonRetryableApplicationError(err.Error(), ErrTypeInvalidArgument, err)
 
 	// Potentially retry
 	case errors.Is(err, plugins.ErrUpstreamRatelimit):

internal/connectors/engine/activities/plugin_create_bank_account_test.go

@@ -91,7 +91,7 @@ var _ = Describe("Plugin Create Bank Account", func() {
 				Expect(temporalErr.Type()).To(Equal(activities.ErrTypeInvalidArgument))
 			},
 			Entry("wrapped invalid client request", fmt.Errorf("invalid: %w", pluginsError.ErrInvalidClientRequest)),
-			Entry("connector metadata error custom type", models.NewConnectorMetadataError("some-field")),
+			Entry("connector metadata error custom type", models.NewConnectorValidationError("some-field", models.ErrMissingConnectorMetadata)),
 		)
 	})
 })

internal/connectors/plugins/public/gocardless/README.md

@@ -56,19 +56,27 @@ The connector supports the following currencies:
 Bank accounts can be created for both creditors and customers. The type is determined by metadata in the request:
 
 ```json
+ { "name": "Gitstart Union Bank",
+  "accountNumber": "21616901",
+  "swiftBicCode": "021000021",
+  "country": "US", // Country code in ISO 3166-1 alpha-2 code.
+  "metadata": {
+    "com.gocardless.spec/currency": "USD",
+    "com.gocardless.spec/customer": "CU001DRQMY17P6",
+    "com.gocardless.spec/account_type": "savings"
+  }
+}
 {
-  "name": "John Doe",
-  "accountNumber": "09878762343",
-  "iban": "2354321234",
-  "swiftBicCode": "34XZZT34",
+  "name": "Gitstart Union Bank",
+  "accountNumber": "21616901",
+  "swiftBicCode": "021000021",
   "country": "US", // Country code in ISO 3166-1 alpha-2 code.
   "metadata": {
-    "branch_code": "required_branch_code",
-    "currency": "USD",
-    "creditor": "CR123", // For creditor bank account
+    "com.gocardless.spec/currency": "USD",
+    "com.gocardless.spec/creditor": "CR123", // For creditor bank account
     // OR
-    "customer": "CU123", // For customer bank account
-    "account_type": "savings" // Required for US accounts
+    "com.gocardless.spec/customer": "CU123", // For customer bank account
+    "com.gocardless.spec/account_type": "savings" // Required for US accounts
   }
 }
 ```

internal/connectors/plugins/public/gocardless/bank_account_creation.go

@@ -2,84 +2,11 @@ package gocardless
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/formancehq/payments/internal/connectors/plugins/public/gocardless/client"
 	"github.com/formancehq/payments/internal/models"
 )
 
-func validateExternalBankAccount(newExternalBankAccount models.BankAccount) error {
-	_, err := extractNamespacedMetadata(newExternalBankAccount.Metadata, client.GocardlessBranchCodeMetadataKey)
-
-	if err != nil {
-		return fmt.Errorf("required metadata field com.gocardless.spec/branch_code is missing")
-	}
-
-	reqCurrency, err := extractNamespacedMetadata(newExternalBankAccount.Metadata, client.GocardlessCurrencyMetadataKey)
-	if err != nil {
-		return fmt.Errorf("required metadata field com.gocardless.spec/currency is missing")
-	}
-
-	_, ok := SupportedCurrenciesWithDecimal[reqCurrency]
-
-	if !ok {
-
-		return fmt.Errorf("com.gocardless.spec/currency %s not supported", reqCurrency)
-	}
-
-	creditor, _ := extractNamespacedMetadata(newExternalBankAccount.Metadata, client.GocardlessCreditorMetadataKey)
-	customer, _ := extractNamespacedMetadata(newExternalBankAccount.Metadata, client.GocardlessCustomerMetadataKey)
-
-	if len(creditor) > 1 && creditor[:2] != "CR" {
-		return fmt.Errorf("com.gocardless.spec/creditor ID must start with 'CR'")
-	}
-
-	if len(customer) > 1 && customer[:2] != "CU" {
-		return fmt.Errorf("com.gocardless.spec/customer ID must start with 'CU'")
-	}
-
-	if customer == "" && creditor == "" {
-		return fmt.Errorf("you must provide com.gocardless.spec/customer or com.gocardless.spec/creditor metadata field")
-	}
-
-	if customer != "" && creditor != "" {
-		return fmt.Errorf("you must provide either com.gocardless.spec/customer or com.gocardless.spec/creditor metadata field but not both")
-	}
-
-	if newExternalBankAccount.Country != nil && *newExternalBankAccount.Country == "US" {
-
-		accountType, err := extractNamespacedMetadata(newExternalBankAccount.Metadata, client.GocardlessAccountTypeMetadataKey)
-
-		if err != nil {
-			return fmt.Errorf("required metadata field com.gocardless.spec/account_type is missing")
-		}
-
-		if accountType != "checking" && accountType != "savings" {
-			return fmt.Errorf("metadata field com.gocardless.spec/account_type must be checking or savings")
-
-		}
-
-	}
-
-	if newExternalBankAccount.AccountNumber == nil {
-		return fmt.Errorf("account number is required")
-	}
-
-	if newExternalBankAccount.SwiftBicCode == nil {
-		return fmt.Errorf("swift bic code is required")
-	}
-
-	if newExternalBankAccount.Country == nil {
-		return fmt.Errorf("country is required")
-	}
-
-	if newExternalBankAccount.IBAN == nil {
-		return fmt.Errorf("IBAN is required")
-	}
-
-	return nil
-}
-
 func (p *Plugin) createBankAccount(ctx context.Context, ba models.BankAccount) (models.CreateBankAccountResponse, error) {
 	err := validateExternalBankAccount(ba)
 	if err != nil {
@@ -91,47 +18,19 @@ func (p *Plugin) createBankAccount(ctx context.Context, ba models.BankAccount) (
 	var externalBankAccount client.GocardlessGenericAccount
 
 	if creditor != "" {
-		creditorBankAccount, err := p.client.CreateCreditorBankAccount(ctx, creditor, ba)
+		externalBankAccount, err = p.client.CreateCreditorBankAccount(ctx, creditor, ba)
 
 		if err != nil {
 			return models.CreateBankAccountResponse{}, err
 		}
-
-		parsedTime, err := ParseGocardlessTimestamp(creditorBankAccount.CreatedAt)
-		if err != nil {
-			return models.CreateBankAccountResponse{}, fmt.Errorf("failed to parse creation time: %w", err)
-		}
-
-		externalBankAccount = client.GocardlessGenericAccount{
-			ID:                creditorBankAccount.Id,
-			CreatedAt:         parsedTime.Unix(),
-			AccountHolderName: creditorBankAccount.AccountHolderName,
-			Metadata:          creditorBankAccount.Metadata,
-			Currency:          creditorBankAccount.Currency,
-			AccountType:       creditorBankAccount.AccountType,
-		}
 	}
 
 	if customer != "" {
-		customerBankAccount, err := p.client.CreateCustomerBankAccount(ctx, customer, ba)
+		externalBankAccount, err = p.client.CreateCustomerBankAccount(ctx, customer, ba)
 
 		if err != nil {
 			return models.CreateBankAccountResponse{}, err
 		}
-
-		parsedTime, err := ParseGocardlessTimestamp(customerBankAccount.CreatedAt)
-		if err != nil {
-			return models.CreateBankAccountResponse{}, fmt.Errorf("failed to parse creation time: %w", err)
-		}
-
-		externalBankAccount = client.GocardlessGenericAccount{
-			ID:                customerBankAccount.Id,
-			CreatedAt:         parsedTime.Unix(),
-			AccountHolderName: customerBankAccount.AccountHolderName,
-			Metadata:          customerBankAccount.Metadata,
-			Currency:          customerBankAccount.Currency,
-			AccountType:       customerBankAccount.AccountType,
-		}
 	}
 
 	bankAccount, err := externalAccountFromGocardlessData(externalBankAccount)