backend/feat/international: stripe transfer api#1086
Conversation
roman-bodavskiy
force-pushed
the
backend/feat/stripe-transfer-api
branch
4 times, most recently
from
f3d5ceb to
13118cd
Compare
roman-bodavskiy
force-pushed
the
backend/feat/stripe-transfer-api
branch
2 times, most recently
from
c0a3f23 to
368f733
Compare
roman-bodavskiy
force-pushed
the
backend/feat/stripe-transfer-api
branch
from
368f733 to
664a7a8
Compare
roman-bodavskiy
force-pushed
the
backend/feat/stripe-transfer-api
branch
from
664a7a8 to
57726a9
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Stripe transfer types and external-payout lifecycle/types, Flow wiring and Stripe implementations for createTransfer/createExternalPayout, restructures payout orchestration to create a transfer first, and updates payment/refund signatures and amount/currency handling. ChangesStripe Transfer and Payout Implementation
Sequence Diagram(s)sequenceDiagram
participant Client
participant PayoutInterface
participant StripeImpl
participant StripeFlow
participant StripeAPI as Stripe API
Client->>PayoutInterface: createPayoutOrder(config, CreatePayoutOrderReq)
PayoutInterface->>StripeImpl: createTransfer(config, CreateTransferReq)
StripeImpl->>StripeFlow: callStripeAPI "create-transfer"
StripeFlow->>StripeAPI: POST /v1/transfers
StripeAPI-->>StripeFlow: TransferObject
StripeFlow-->>StripeImpl: TransferObject
StripeImpl-->>PayoutInterface: CreateTransferResp(transferId)
PayoutInterface->>StripeImpl: createExternalPayout(config, CreateExternalPayoutReq)
StripeImpl->>StripeFlow: callStripeAPI "create-payout"
StripeFlow->>StripeAPI: POST /v1/payouts
StripeAPI-->>StripeFlow: PayoutObject
StripeFlow-->>StripeImpl: PayoutObject
StripeImpl-->>PayoutInterface: CreateExternalPayoutResp
PayoutInterface->>PayoutInterface: Build CreatePayoutOrderResp(status=SUCCESS, transferId, externalPayoutStatus)
PayoutInterface-->>Client: CreatePayoutOrderResp
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs (1)
776-777: 💤 Low value
amountInUsdvariable name witheurToCents— misleading naming.
buildCreateTransferReqdestructuresamountasamountInUsdthen applieseurToCents, mixing USD/EUR semantics. BothusdToCentsandeurToCentsare currently identical (multiply by 100), but the naming makes the intent unclear. Prefer using the currency-agnostic conversion or a name consistent withreq.currency:-buildCreateTransferReq CreateTransferReq {amount = amountInUsd, ..} = do - let amountInCents = eurToCents amountInUsd +buildCreateTransferReq CreateTransferReq {amount = amountInHighPrec, ..} = do + let amountInCents = round (getHighPrecMoney amountInHighPrec * 100)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs` around lines 776 - 777, The variable name amountInUsd is misleading because buildCreateTransferReq then calls eurToCents; update the code to use a currency-consistent name or a currency-agnostic conversion: in buildCreateTransferReq (pattern matching on CreateTransferReq) rename amountInUsd to amount or amountInMajorUnits, and replace the direct call to eurToCents with a toCents helper that chooses usdToCents/eurToCents based on the request currency (req.currency) or simply a unified toCents function if both conversions are identical; ensure references to eurToCents/usdToCents and CreateTransferReq are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs`:
- Around line 759-784: The createTransfer function currently always passes
senderConnectedAccountId as the Stripe-Account header to Stripe.createTransfer;
change it so the header is only included when the transfer is from a connected
account to the platform (i.e., when destinationAccount ==
TransferPlatformAccount), and omitted when the platform is sending to a
connected account (TransferConnectedAccount). Locate createTransfer, use the
result of destinationAccount pattern-match (or add a new boolean like
isSenderConnected) to decide whether to call Stripe.createTransfer with (Just
req.senderConnectedAccountId) or Nothing, keeping buildCreateTransferReq and
mkCreateTransferResp as-is but ensuring the Stripe.createTransfer invocation
uses the conditional header.
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Types.hs`:
- Around line 936-949: The Transfer-related types currently break JSON parsing
and reduce usability: fix the incorrect modeling of TransferObject in
Transfer.hs so that TransferObject correctly reflects Stripe's Transfer API
(removing or correctly mapping the nonexistent top-level "status" field) and
update mkCreateTransferResp to map from the corrected TransferObject fields (or
remove status mapping if Stripe doesn't provide it); additionally, add standard
deriving clauses to TransferAccount, CreateTransferReq, and CreateTransferResp
(e.g., Show, Eq, Generic, FromJSON, ToJSON, ToSchema) to match other
request/response types and restore debuggability and serialization
compatibility.
In `@lib/mobility-core/src/Kernel/External/Payment/Stripe/Types/Transfer.hs`:
- Around line 31-40: The ToForm instance for TransferReq is dropping the
optional metadata field so metadata is never sent; update the ToForm
implementation for TransferReq to include metadata when present by expanding
Metadata into the expected Stripe form keys (metadata[key]=value) and inserting
those pairs into the Form's HM.fromList list (alongside amount, currency,
destination, description). Ensure you use or adapt the existing
ToHttpApiData/serialization for Metadata (or write a helper that converts
Metadata -> [(Text, Text)] with keys like "metadata[KEY]") so the Form produced
by toForm TransferReq includes those metadata key/value pairs.
- Around line 43-51: TransferObject's fields don't match Stripe: change the
created field from UTCTime to an integer/POSIX type (e.g., Int or POSIXTime) and
remove the non-existent status field, adding reversed :: Bool instead; then
update mkCreateTransferResp to derive CreateTransferResp.status from
Stripe.TransferObject.reversed (e.g., status = if reversed then
TRANSFER_REVERSED else TRANSFER_SUCCEEDED) so createTransfer can parse responses
successfully.
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs`:
- Around line 776-777: The variable name amountInUsd is misleading because
buildCreateTransferReq then calls eurToCents; update the code to use a
currency-consistent name or a currency-agnostic conversion: in
buildCreateTransferReq (pattern matching on CreateTransferReq) rename
amountInUsd to amount or amountInMajorUnits, and replace the direct call to
eurToCents with a toCents helper that chooses usdToCents/eurToCents based on the
request currency (req.currency) or simply a unified toCents function if both
conversions are identical; ensure references to eurToCents/usdToCents and
CreateTransferReq are updated accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8eff8ccc-3843-4deb-8516-b5e996432659
📒 Files selected for processing (8)
lib/mobility-core/mobility-core.caballib/mobility-core/src/Kernel/External/Payment/Interface.hslib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hslib/mobility-core/src/Kernel/External/Payment/Interface/Types.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Config.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Flow.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Types.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Types/Transfer.hs
| createTransfer :: | ||
| forall m r. | ||
| ( Metrics.CoreMetrics m, | ||
| EncFlow m r, | ||
| HasRequestId r, | ||
| MonadReader r m | ||
| ) => | ||
| StripeCfg -> | ||
| CreateTransferReq -> | ||
| m CreateTransferResp | ||
| createTransfer config req = do | ||
| let url = config.url | ||
| apiKey <- decrypt config.apiKey | ||
| transferReq <- buildCreateTransferReq req | ||
| mkCreateTransferResp <$> Stripe.createTransfer url apiKey (Just req.senderConnectedAccountId) transferReq | ||
| where | ||
| buildCreateTransferReq :: CreateTransferReq -> m Stripe.TransferReq | ||
| buildCreateTransferReq CreateTransferReq {amount = amountInUsd, ..} = do | ||
| let amountInCents = eurToCents amountInUsd | ||
| destination <- case destinationAccount of | ||
| TransferConnectedAccount accountId -> pure accountId | ||
| TransferPlatformAccount -> config.platformAccountId & fromMaybeM (InternalError "STRIPE_PLATFORM_ACCOUNT_ID_NOT_FOUND") | ||
| pure Stripe.TransferReq {amount = amountInCents, metadata = Nothing, ..} | ||
|
|
||
| mkCreateTransferResp :: Stripe.TransferObject -> CreateTransferResp | ||
| mkCreateTransferResp Stripe.TransferObject {..} = CreateTransferResp {transferId = id, status} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Stripe Connect Transfer API: when to use Stripe-Account header for transfers between platform and connected account
💡 Result:
When making a Connect Transfer API call (POST /v1/transfers, and the corresponding retrieve/list/update), you normally do NOT use the Stripe-Account header to “target the destination connected account”. Instead: 1) Creating the transfer from the platform side - Use the platform’s API key (your own platform secret key). - Create the transfer with destination set to the connected account ID (acct_...). - Do not add Stripe-Account: acct_... for the request. Evidence: the Transfer create endpoint is described as “To send funds from your Stripe account to a connected account, you create a new transfer object” and uses destination=connected_account_id; example curl uses only -u with your secret key and does not include Stripe-Account header. 2) When Stripe-Account header is needed Stripe-Account header is for “making API calls for connected accounts” (i.e., acting as the connected account for API operations). - Add Stripe-Account: acct_... when you intend the API operation to run “in the context of” that connected account (typically for endpoints where connected-account context matters, such as retrieving that connected account’s charges/balance, etc.). Evidence: Stripe docs explain you “can issue requests on behalf of your connected accounts” by including the Stripe-Account header, per request. 3) Practical decision rule for transfer between platform and connected account - If you are moving money by creating a Transfer and specifying destination=acct_...: use platform key; no Stripe-Account header. - Use Stripe-Account header only if the endpoint you’re calling should be executed “as the connected account” (the docs describe this generally for connected-account API calls), not just because the transfer.destination is a connected account. Note: Stripe-Context header (newer mechanism) supersedes Stripe-Account for broader/expanded context scenarios, but this doesn’t change the fundamental guidance for creating transfers from the platform: the destination parameter is what directs funds. Example (create transfer from platform -> connected): curl https://api.stripe.com/v1/transfers \ -u "<<PLATFORM_SECRET_KEY>>" \ -d amount=400 \ -d currency=usd \ -d destination="acct_..." \ -d transfer_group=ORDER_95 Answer summary: For platform-to-connected transfers via the Transfers API, set destination to the connected account ID and authenticate with the platform key; don’t add the Stripe-Account header solely for that transfer.
Citations:
- 1: https://docs.stripe.com/api/transfers/create
- 2: https://docs.stripe.com/api/transfers/create?api-version=2025-01-27.acacia
- 3: https://docs.stripe.com/connect/authentication
- 4: https://docs.stripe.com/api/connected-accounts?api-version=2024-11-20.acacia
- 5: https://docs.stripe.com/context?locale=en-GB
Stripe-Account header must be conditional based on transfer direction.
createTransfer unconditionally passes senderConnectedAccountId as the Stripe-Account header. Per Stripe's Transfer API documentation, this header should only be used when a connected account initiates the transfer; for platform-initiated transfers to connected accounts, the header must be omitted.
- Platform → connected account (
TransferConnectedAccount): omit the header - Connected account → platform (
TransferPlatformAccount): include the header
<details>
<summary>Suggested fix</summary>
```diff
-mkCreateTransferResp <$> Stripe.createTransfer url apiKey (Just req.senderConnectedAccountId) transferReq
+let stripeAccountHeader = case req.destinationAccount of
+ TransferPlatformAccount -> Just req.senderConnectedAccountId
+ TransferConnectedAccount _ -> Nothing
+in mkCreateTransferResp <$> Stripe.createTransfer url apiKey stripeAccountHeader transferReq🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs` around
lines 759 - 784, The createTransfer function currently always passes
senderConnectedAccountId as the Stripe-Account header to Stripe.createTransfer;
change it so the header is only included when the transfer is from a connected
account to the platform (i.e., when destinationAccount ==
TransferPlatformAccount), and omitted when the platform is sending to a
connected account (TransferConnectedAccount). Locate createTransfer, use the
result of destinationAccount pattern-match (or add a new boolean like
isSenderConnected) to decide whether to call Stripe.createTransfer with (Just
req.senderConnectedAccountId) or Nothing, keeping buildCreateTransferReq and
mkCreateTransferResp as-is but ensuring the Stripe.createTransfer invocation
uses the conditional header.
| data TransferAccount = TransferConnectedAccount AccountId | TransferPlatformAccount | ||
|
|
||
| data CreateTransferReq = CreateTransferReq | ||
| { amount :: HighPrecMoney, | ||
| currency :: Currency, | ||
| senderConnectedAccountId :: AccountId, | ||
| destinationAccount :: TransferAccount, | ||
| description :: Maybe Text | ||
| } | ||
|
|
||
| data CreateTransferResp = CreateTransferResp | ||
| { transferId :: TransferId, | ||
| status :: TransferStatus | ||
| } |
There was a problem hiding this comment.
CreateTransferResp.status :: TransferStatus will be unreachable until TransferObject is fixed.
CreateTransferResp.status is populated by mapping TransferObject.status, but the Stripe Transfer API has no status field — every createTransfer call will fail at JSON parsing before mkCreateTransferResp is ever reached. This is a downstream effect of the TransferObject modelling error in Transfer.hs.
Also, TransferAccount, CreateTransferReq, and CreateTransferResp have no deriving clauses (no Show, Generic, etc.), which limits debuggability and future serialization needs. Other unified request/response types in this file (e.g., CreatePaymentReq, RefundPaymentReq) derive Show, Eq, Generic, FromJSON, ToJSON, ToSchema.
♻️ Proposed derivations (once TransferObject is fixed)
-data TransferAccount = TransferConnectedAccount AccountId | TransferPlatformAccount
+data TransferAccount = TransferConnectedAccount AccountId | TransferPlatformAccount
+ deriving stock (Show, Eq, Generic)
+ deriving anyclass (FromJSON, ToJSON, ToSchema)
-data CreateTransferReq = CreateTransferReq
+data CreateTransferReq = CreateTransferReq
{ amount :: HighPrecMoney,
currency :: Currency,
senderConnectedAccountId :: AccountId,
destinationAccount :: TransferAccount,
description :: Maybe Text
}
+ deriving stock (Show, Eq, Generic)
+ deriving anyclass (FromJSON, ToJSON, ToSchema)
-data CreateTransferResp = CreateTransferResp
+data CreateTransferResp = CreateTransferResp
{ transferId :: TransferId,
status :: TransferStatus
}
+ deriving stock (Show, Eq, Generic)
+ deriving anyclass (FromJSON, ToJSON, ToSchema)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Types.hs` around
lines 936 - 949, The Transfer-related types currently break JSON parsing and
reduce usability: fix the incorrect modeling of TransferObject in Transfer.hs so
that TransferObject correctly reflects Stripe's Transfer API (removing or
correctly mapping the nonexistent top-level "status" field) and update
mkCreateTransferResp to map from the corrected TransferObject fields (or remove
status mapping if Stripe doesn't provide it); additionally, add standard
deriving clauses to TransferAccount, CreateTransferReq, and CreateTransferResp
(e.g., Show, Eq, Generic, FromJSON, ToJSON, ToSchema) to match other
request/response types and restore debuggability and serialization
compatibility.
| instance ToForm TransferReq where | ||
| toForm TransferReq {..} = | ||
| Form $ | ||
| HM.fromList $ | ||
| catMaybes | ||
| [ Just . ("amount",) . pure $ toQueryParam amount, | ||
| Just . ("currency",) . pure $ toQueryParam currency, | ||
| Just . ("destination",) . pure $ toQueryParam destination, | ||
| ("description",) . pure . toQueryParam <$> description | ||
| ] |
There was a problem hiding this comment.
metadata field is silently dropped — never sent to Stripe.
TransferReq declares metadata :: Maybe Metadata but the ToForm instance omits it entirely. Any metadata passed by callers is discarded before the HTTP request is made.
🐛 Proposed fix
instance ToForm TransferReq where
toForm TransferReq {..} =
Form $
HM.fromList $
catMaybes
[ Just . ("amount",) . pure $ toQueryParam amount,
Just . ("currency",) . pure $ toQueryParam currency,
Just . ("destination",) . pure $ toQueryParam destination,
+ ("metadata",) . pure . toQueryParam <$> metadata,
("description",) . pure . toQueryParam <$> description
]Note: Stripe expects metadata as metadata[key]=value key-value pairs in form encoding, so the serialization of Metadata via toQueryParam needs to match that format. Verify that the ToHttpApiData Metadata instance produces the correct Stripe format.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/mobility-core/src/Kernel/External/Payment/Stripe/Types/Transfer.hs`
around lines 31 - 40, The ToForm instance for TransferReq is dropping the
optional metadata field so metadata is never sent; update the ToForm
implementation for TransferReq to include metadata when present by expanding
Metadata into the expected Stripe form keys (metadata[key]=value) and inserting
those pairs into the Form's HM.fromList list (alongside amount, currency,
destination, description). Ensure you use or adapt the existing
ToHttpApiData/serialization for Metadata (or write a helper that converts
Metadata -> [(Text, Text)] with keys like "metadata[KEY]") so the Form produced
by toForm TransferReq includes those metadata key/value pairs.
57726a9 to
bb09f2c
Compare
roman-bodavskiy
force-pushed
the
backend/feat/stripe-transfer-api
branch
3 times, most recently
from
d623620 to
c2de4db
Compare
roman-bodavskiy
force-pushed
the
backend/feat/stripe-transfer-api
branch
from
c2de4db to
7217c1d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/mobility-core/src/Kernel/External/Payment/Stripe/Types/PaymentIntent.hs (1)
50-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSerialize
application_fee_amountas optional intoForm
application_fee_amountis nowMaybe Int(Line 38), but Line 61 still emits it unconditionally. This can send an empty value whenNothing, which may cause Stripe request validation failures. UsemaybeToFormlike other optional fields.Suggested fix
toForm PaymentIntentReq {..} = Form $ HM.fromListWith (++) [ ("amount", [toQueryParam amount]), ("currency", [show currency]), ("confirm", [toQueryParam confirm]), ("customer", [toQueryParam customer]), ("payment_method", [toQueryParam payment_method]), - ("application_fee_amount", [toQueryParam application_fee_amount]), ("capture_method", [toQueryParam capture_method]), ("confirmation_method", [toQueryParam confirmation_method]), ("use_stripe_sdk", [toQueryParam use_stripe_sdk]), ("return_url", [toQueryParam return_url]) ] + <> maybeToForm "application_fee_amount" application_fee_amount <> maybeToForm "transfer_data[destination]" (transfer_data <&> (.destination)) <> maybeToForm "metadata[order_short_id]" metadata.order_short_id <> maybeToForm "description" description <> maybeToForm "receipt_email" receipt_email <> maybeToForm "setup_future_usage" setup_future_usage <> maybeToForm "on_behalf_of" on_behalf_of🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payment/Stripe/Types/PaymentIntent.hs` around lines 50 - 72, The toForm implementation for PaymentIntentReq currently always emits "application_fee_amount" from the HM.fromListWith block even though application_fee_amount is now Maybe Int; update the toForm function so that "application_fee_amount" is serialized with maybeToForm (like metadata.order_short_id, description, etc.) instead of being placed unconditionally in the initial list—locate the toForm function and the "application_fee_amount" entry and replace that unconditional emission with maybeToForm "application_fee_amount" application_fee_amount so the field is omitted when Nothing.lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs (1)
30-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
createExternalPayoutignoresreq.currencywhen converting to cents.
mkCreatePayoutReqalways usesusdToCents amountand only forwardsT.toLower (show currency)to Stripe in thecurrencyfield. Ifreq.currencyis anything other than USD, the on-the-wireamountwill not match Stripe's expectation for that currency's smallest unit (e.g., for GBP this could happen to coincide, for JPY it would be off by 100×). Same concern applies tomkCreateExternalPayoutRespusingcentsToUsd stripeResp.amountunconditionally.If today the only supported currency is USD, an explicit guard (
unless (currency == USD) $ throwError ...) would prevent silent misuse; otherwise dispatch oncurrencylike increateTransfer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs` around lines 30 - 52, createExternalPayout currently always uses usdToCents in mkCreatePayoutReq and centsToUsd in mkCreateExternalPayoutResp, which will mis-handle non-USD currencies; update createExternalPayout to either reject non-USD early (e.g., unless (currency == USD) throwError ...) or, preferably, dispatch conversions based on req.currency like createTransfer does: replace usdToCents/centsToUsd calls in mkCreatePayoutReq and mkCreateExternalPayoutResp with a currency-aware toMinorUnit/fromMinorUnit lookup (or per-currency conversion functions) and ensure Stripe.CreatePayoutReq.amount and the response mapping use those currency-specific converters so the on‑the‑wire smallest unit matches the currency.
🧹 Nitpick comments (3)
lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs (1)
75-92: 💤 Low valueField documentation discrepancy:
transferIdvsidAssignedByServiceProvider.Both fields are now marked "Stripe specific", but the relationship between them isn't obvious. Based on the implementation in
Interface.hs,transferIdcarries the Stripe/v1/transfersID (platform → connected account leg) whileidAssignedByServiceProvidercarries the Stripe payout ID (connected account → bank/card leg). Mixing these silently would be a real correctness hazard in reconciliation/refund paths. A short comment clarifying that distinction would prevent future misuse.📝 Suggested clarifying comments
data CreatePayoutOrderResp = CreatePayoutOrderResp { orderId :: Text, status :: PayoutOrderStatus, -- payout (transfer) status from platform to driver/fleet externalPayoutStatus :: Maybe ExternalPayoutStatus, -- Stripe specific: payout status from driver/fleet connected account to driver/fleet bank account/card orderType :: Maybe Text, - transferId :: Maybe TransferId, -- Stripe specific - idAssignedByServiceProvider :: Maybe Text, -- Stripe specific + transferId :: Maybe TransferId, -- Stripe specific: id of the /v1/transfers leg (platform -> connected account) + idAssignedByServiceProvider :: Maybe Text, -- Stripe specific: id of the /v1/payouts leg (connected account -> bank/card)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs` around lines 75 - 92, The CreatePayoutOrderResp type's Stripe-specific fields transferId and idAssignedByServiceProvider are ambiguous; update the comment on these fields in Types.hs (within CreatePayoutOrderResp) to clearly state that transferId is the Stripe /v1/transfers ID representing the platform→connected-account leg, while idAssignedByServiceProvider is the Stripe payout ID representing the connected-account→bank/card leg, so callers using transferId vs idAssignedByServiceProvider for reconciliation/refund logic will not confuse the two (refer to the mapping implemented in Interface.hs).lib/mobility-core/src/Kernel/External/Payout/Interface.hs (2)
30-30: 💤 Low valueTypo and unresolved retry behaviour in TODO.
-- TODO handle retrie when external payout status is not okay—retrieshould beretry, and given the idempotency concern raised above (noIdempotency-KeyoncreateTransfer), this TODO is also gating real correctness, not just polish. Worth tracking as an issue rather than a code comment.Want me to open an issue for the missing retry/idempotency strategy on
createPayoutOrderso it doesn't get lost?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` at line 30, The TODO comment has a typo ("retrie" → "retry") and signals a missing retry/idempotency strategy for external payouts; update the comment and implement a proper retry with idempotency for createPayoutOrder by ensuring createTransfer calls include an Idempotency-Key (or a generated idempotency token) and that createPayoutOrder retries transient failures safely (exponential backoff, max attempts) while treating retryable vs non-retryable statuses; also open a formal issue tracking the design/implementation of idempotency for createTransfer/createPayoutOrder so this correctness work isn't lost.
111-134: ⚡ Quick win
payoutOrderStatusechoesreq.currentStatusrather than deriving overall status from Stripe response.
status = req.currentStatusmakes the Stripe branch ofpayoutOrderStatusdependent on the caller passing in the rightcurrentStatus, while the function name implies it returns the current status. If the caller passes a stale value, the response will reflect that stale state for thestatusfield, whileexternalPayoutStatusis freshly fetched. This split is easy to misuse downstream (e.g., reconciliation logic comparing both fields).If the intent is "transfer-leg status doesn't change after creation," derive it from the presence of
transferId(or the persisted state) rather than asking the caller to supply it. At minimum, document this contract onPayoutOrderStatusReq.currentStatusso callers know the field is required and authoritative.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` around lines 111 - 134, The Stripe branch currently sets status = req.currentStatus which can be stale; change mkPayoutOrderStatusResp in the StripeConfig handling (the mapping from CreateExternalPayoutResp to CreatePayoutOrderResp) to compute status from the fresh Stripe response and the presence of a transfer (prefer transferId precedence) instead of blindly echoing PayoutOrderStatusReq.currentStatus; specifically, call Stripe.externalPayoutOrderStatus, inspect externalPayoutStatus and req.transferId (or other persisted transfer indicator) and derive the PayoutOrderStatusResp.status value from those, or if you intend to keep caller-supplied currentStatus as authoritative, add validation and clear documentation on PayoutOrderStatusReq.currentStatus explaining it must be the authoritative transfer-leg status.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs`:
- Around line 51-55: The code logs the entire StripeError via show (variable
err) which may contain PII; update the log to avoid printing the full exception
and instead log only the sanitized fields already extracted (errorCode and
errorMessage) and the orderId, and fix missing separators. Replace the existing
logError call that references err with a single formatted message like "Error
while creating external payout: code=<...> message=<...> orderId=<...>" using
errorCode and errorMessage (handle their Maybe values with sensible defaults
such as "unknown"), keep Payment.StripeError/err extraction as-is but do not
include show err anywhere, and ensure spaces/separators are present between
fragments.
- Around line 43-65: The createTransfer call lacks idempotency which can
double-charge on retries; add an idempotencyKey :: Text field to the TransferReq
type, update the CreateTransferAPI to send that field as the "Idempotency-Key"
header, and modify mkTransferReq (and the call site in StripeConfig branch where
mkTransferReq is used) to populate idempotencyKey from req.orderId (or a
request-scoped id) before calling Stripe.createTransfer; also correct the TODO
typo "retrie" to "retry". Ensure all uses of TransferReq and CreateTransferAPI
are updated to accept and forward the new idempotencyKey.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs`:
- Around line 103-109: buildCreateTransferReq and mkCreatePayoutReq are using
hardcoded eurToCents/usdToCents and ignore the CreateTransferReq.currency, which
breaks zero-decimal currencies; replace those calls with a single currency-aware
converter (e.g. implement toMinorUnits :: Currency -> HighPrecMoney -> Int) that
dispatches on Currency (handling 0-, 2-, etc.-decimal currencies) and use it in
buildCreateTransferReq and mkCreatePayoutReq (rename amountInUsd to a neutral
name like amountInMinor or amountInBase before conversion) so amounts sent to
Stripe are correct for each currency.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs`:
- Around line 126-149: Add missing deriving clauses to the new public types so
they match other request/response types: for CreateExternalPayoutResp and
CreateTransferResp add "deriving (Show, Generic, FromJSON, ToJSON, ToSchema)";
for CreateTransferReq and TransferAccount add "deriving (Show, Generic,
FromJSON, ToJSON)". Update the declarations for CreateExternalPayoutResp,
TransferAccount, CreateTransferReq, and CreateTransferResp to include these
deriving clauses so they can be logged, serialized, and included in OpenAPI
specs.
---
Outside diff comments:
In `@lib/mobility-core/src/Kernel/External/Payment/Stripe/Types/PaymentIntent.hs`:
- Around line 50-72: The toForm implementation for PaymentIntentReq currently
always emits "application_fee_amount" from the HM.fromListWith block even though
application_fee_amount is now Maybe Int; update the toForm function so that
"application_fee_amount" is serialized with maybeToForm (like
metadata.order_short_id, description, etc.) instead of being placed
unconditionally in the initial list—locate the toForm function and the
"application_fee_amount" entry and replace that unconditional emission with
maybeToForm "application_fee_amount" application_fee_amount so the field is
omitted when Nothing.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs`:
- Around line 30-52: createExternalPayout currently always uses usdToCents in
mkCreatePayoutReq and centsToUsd in mkCreateExternalPayoutResp, which will
mis-handle non-USD currencies; update createExternalPayout to either reject
non-USD early (e.g., unless (currency == USD) throwError ...) or, preferably,
dispatch conversions based on req.currency like createTransfer does: replace
usdToCents/centsToUsd calls in mkCreatePayoutReq and mkCreateExternalPayoutResp
with a currency-aware toMinorUnit/fromMinorUnit lookup (or per-currency
conversion functions) and ensure Stripe.CreatePayoutReq.amount and the response
mapping use those currency-specific converters so the on‑the‑wire smallest unit
matches the currency.
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs`:
- Line 30: The TODO comment has a typo ("retrie" → "retry") and signals a
missing retry/idempotency strategy for external payouts; update the comment and
implement a proper retry with idempotency for createPayoutOrder by ensuring
createTransfer calls include an Idempotency-Key (or a generated idempotency
token) and that createPayoutOrder retries transient failures safely (exponential
backoff, max attempts) while treating retryable vs non-retryable statuses; also
open a formal issue tracking the design/implementation of idempotency for
createTransfer/createPayoutOrder so this correctness work isn't lost.
- Around line 111-134: The Stripe branch currently sets status =
req.currentStatus which can be stale; change mkPayoutOrderStatusResp in the
StripeConfig handling (the mapping from CreateExternalPayoutResp to
CreatePayoutOrderResp) to compute status from the fresh Stripe response and the
presence of a transfer (prefer transferId precedence) instead of blindly echoing
PayoutOrderStatusReq.currentStatus; specifically, call
Stripe.externalPayoutOrderStatus, inspect externalPayoutStatus and
req.transferId (or other persisted transfer indicator) and derive the
PayoutOrderStatusResp.status value from those, or if you intend to keep
caller-supplied currentStatus as authoritative, add validation and clear
documentation on PayoutOrderStatusReq.currentStatus explaining it must be the
authoritative transfer-leg status.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs`:
- Around line 75-92: The CreatePayoutOrderResp type's Stripe-specific fields
transferId and idAssignedByServiceProvider are ambiguous; update the comment on
these fields in Types.hs (within CreatePayoutOrderResp) to clearly state that
transferId is the Stripe /v1/transfers ID representing the
platform→connected-account leg, while idAssignedByServiceProvider is the Stripe
payout ID representing the connected-account→bank/card leg, so callers using
transferId vs idAssignedByServiceProvider for reconciliation/refund logic will
not confuse the two (refer to the mapping implemented in Interface.hs).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e5cf998-030e-4956-90a6-97d611b4b5df
📒 Files selected for processing (15)
lib/mobility-core/mobility-core.caballib/mobility-core/src/Kernel/External/Payment/Interface.hslib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Config.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Types/PaymentIntent.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Types/Refund.hslib/mobility-core/src/Kernel/External/Payout/Interface.hslib/mobility-core/src/Kernel/External/Payout/Interface/Juspay.hslib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hslib/mobility-core/src/Kernel/External/Payout/Interface/Types.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Config.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Flow.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Types.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Payout.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Transfer.hs
✅ Files skipped from review due to trivial changes (4)
- lib/mobility-core/mobility-core.cabal
- lib/mobility-core/src/Kernel/External/Payment/Stripe/Types/Refund.hs
- lib/mobility-core/src/Kernel/External/Payout/Stripe/Types.hs
- lib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Transfer.hs
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs
- lib/mobility-core/src/Kernel/External/Payment/Interface.hs
| StripeConfig cfg -> do | ||
| connectedAccountId <- req.mConnectedAccountId & fromMaybeM (InvalidRequest "connectedAccountId required for Stripe payout") | ||
| createTransferResp <- Stripe.createTransfer cfg (mkTransferReq connectedAccountId req) | ||
|
|
||
| -- IMPORTANT: always consider status = SUCCESS if transfer was successfull, even if external payout was failed | ||
| result <- withTryCatch "createExternalPayout" $ Stripe.createExternalPayout cfg req | ||
| createExternalPayoutResp <- case result of | ||
| Right resp -> pure resp | ||
| Left e -> do | ||
| let err = fromException @Payment.StripeError e | ||
| errorCode = err <&> toErrorCode | ||
| errorMessage = err >>= toMessage | ||
| logError $ "Error while create external payout : " <> show err <> "error code : " <> show errorCode <> "error message : " <> show errorMessage <> " orderId: " <> req.orderId | ||
| pure | ||
| CreateExternalPayoutResp | ||
| { orderId = req.orderId, | ||
| externalPayoutStatus = Just EXTERNAL_PAYOUT_FAILED, | ||
| orderType = Just req.orderType, | ||
| idAssignedByServiceProvider = Nothing, | ||
| amount = req.amount, | ||
| customerId = Just req.customerId | ||
| } | ||
| pure $ mkCreatePayoutOrderResp createTransferResp createExternalPayoutResp |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Stripe Idempotency-Key header /v1/transfers double charge prevention
💡 Result:
Yes, the Stripe Idempotency-Key header prevents double transfers (or "double charges") on POST /v1/transfers by making the request idempotent. Stripe's API supports idempotency keys on all POST endpoints, including transfers/create, which is a mutating POST request to create a transfer object.[1][2][3][4] This allows safe retries: generate a unique Idempotency-Key (e.g., UUIDv4, up to 255 chars), include it in the header, and if a network error occurs, retry with the same key. Stripe caches the first response (success or failure) for 24 hours; subsequent identical requests return the cached result without duplicating the transfer.[1][2][3][4] Example curl for /v1/transfers with Idempotency-Key (adapted from customer example, as transfers supports the same): curl https://api.stripe.com/v1/transfers \ -u sk_test_...: \ -H "Idempotency-Key: example-uuid-v4-key" \ -d amount=1000 \ -d currency=usd \ -d destination=acct_...[1][3][5] In client libraries, pass idempotencyKey in options (e.g., Node: {idempotencyKey: 'key'}).[1][2] Keys expire after 24 hours; reuse generates a new request if params differ (errors if not matching).[1][2] Use for all mutating POSTs to prevent duplicates from retries.[3][4]
Citations:
- 1: https://docs.stripe.com/api/idempotent_requests?lang=node
- 2: https://docs.stripe.com/api/idempotent_requests
- 3: https://stripe.com/docs/api/idempotent%5Frequests
- 4: https://docs.stripe.com/error-low-level
- 5: https://docs.stripe.com/api/transfers/create
🏁 Script executed:
# Find files related to Stripe transfer handling
find . -name "*.hs" -type f | xargs grep -l "TransferReq\|createTransfer" | head -20Repository: nammayatri/shared-kernel
Length of output: 395
🏁 Script executed:
# Look for the Stripe.hs or Stripe/Flow.hs mentioned in the review
fd -e hs "Stripe" | grep -E "(Stripe.hs|Flow.hs)" | head -20Repository: nammayatri/shared-kernel
Length of output: 199
🏁 Script executed:
# Search for the Interface.hs file mentioned
fd -e hs "Interface.hs" | head -10Repository: nammayatri/shared-kernel
Length of output: 707
🏁 Script executed:
# Examine TransferReq type definition
cat -n lib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Transfer.hsRepository: nammayatri/shared-kernel
Length of output: 2318
🏁 Script executed:
# Look at createTransfer implementation
cat -n lib/mobility-core/src/Kernel/External/Payout/Stripe/Flow.hsRepository: nammayatri/shared-kernel
Length of output: 5128
🏁 Script executed:
# Search for the TODO comment with "retrie" typo
grep -r "retrie" lib/mobility-core/src/Kernel/External/Payout/ --include="*.hs"Repository: nammayatri/shared-kernel
Length of output: 188
🏁 Script executed:
# Search for logging patterns in the payout interface
grep -n "logError\|log" lib/mobility-core/src/Kernel/External/Payout/Interface.hsRepository: nammayatri/shared-kernel
Length of output: 253
🏁 Script executed:
# Read the full Interface.hs file to verify the concerns
wc -l lib/mobility-core/src/Kernel/External/Payout/Interface.hsRepository: nammayatri/shared-kernel
Length of output: 130
🏁 Script executed:
# Read the relevant portions of Interface.hs
sed -n '1,100p' lib/mobility-core/src/Kernel/External/Payout/Interface.hsRepository: nammayatri/shared-kernel
Length of output: 4241
🏁 Script executed:
# Check around line 43-65 and beyond to see the full context
sed -n '40,120p' lib/mobility-core/src/Kernel/External/Payout/Interface.hsRepository: nammayatri/shared-kernel
Length of output: 3397
🏁 Script executed:
# Search for other uses of createTransfer to see if idempotency is handled at a higher level
grep -n "createTransfer" lib/mobility-core/src/Kernel/External/Payout/Interface.hsRepository: nammayatri/shared-kernel
Length of output: 369
🏁 Script executed:
# Check if there's an idempotency key or similar in the request context
grep -r "IdempotencyKey\|idempotency" lib/mobility-core/src/Kernel/External/Payout/ --include="*.hs"Repository: nammayatri/shared-kernel
Length of output: 50
🏁 Script executed:
# Check Stripe.Interface to see if idempotency is handled there
cat -n lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hsRepository: nammayatri/shared-kernel
Length of output: 5164
🏁 Script executed:
# Look at CreateTransferReq to see if it differs from TransferReq
grep -A 20 "data CreateTransferReq" lib/mobility-core/src/Kernel/External/Payout/ -r --include="*.hs"Repository: nammayatri/shared-kernel
Length of output: 1069
🏁 Script executed:
# Check what StripeError contains to assess PII leak risk
grep -A 10 "data StripeError\|newtype StripeError" lib/mobility-core/src/Kernel/External/Payment/ -r --include="*.hs" | head -40Repository: nammayatri/shared-kernel
Length of output: 4417
Critical: missing idempotency safeguard on createTransfer exposes risk of double charges on retry.
The flow is: (1) call Stripe.createTransfer (committing money), (2) call Stripe.createExternalPayout. If the caller retries createPayoutOrder (e.g., because of a transient timeout in step 2, or any client-side failure after step 1 succeeded), step 1 will execute again and create a second Stripe transfer for the same orderId — double-funding the connected account. The IMPORTANT comment documents the intent but provides no actual safeguard.
Stripe's recommended fix is to send an Idempotency-Key header on /v1/transfers (typically derived from req.orderId). The TransferReq type and CreateTransferAPI definition currently lack any idempotency key support. Add an idempotencyKey :: Text field to TransferReq, wire it through the API definition as an Idempotency-Key header, and populate it (e.g., from orderId or a request-scoped identifier) in the Stripe interface layer.
Minor: fix TODO typo at line 30 — "retrie" should be "retry".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` around lines 43 -
65, The createTransfer call lacks idempotency which can double-charge on
retries; add an idempotencyKey :: Text field to the TransferReq type, update the
CreateTransferAPI to send that field as the "Idempotency-Key" header, and modify
mkTransferReq (and the call site in StripeConfig branch where mkTransferReq is
used) to populate idempotencyKey from req.orderId (or a request-scoped id)
before calling Stripe.createTransfer; also correct the TODO typo "retrie" to
"retry". Ensure all uses of TransferReq and CreateTransferAPI are updated to
accept and forward the new idempotencyKey.
| Left e -> do | ||
| let err = fromException @Payment.StripeError e | ||
| errorCode = err <&> toErrorCode | ||
| errorMessage = err >>= toMessage | ||
| logError $ "Error while create external payout : " <> show err <> "error code : " <> show errorCode <> "error message : " <> show errorMessage <> " orderId: " <> req.orderId |
There was a problem hiding this comment.
Avoid logging full Stripe error payloads — risk of leaking PII / sensitive request data.
logError $ "Error while create external payout : " <> show err <> "error code : " <> show errorCode <> "error message : " <> show errorMessage <> " orderId: " <> req.orderId interpolates the entire StripeError exception via show. Stripe error responses can contain echoed request fields (customer email, name, bank/card identifiers, descriptions) and sometimes PII from the request. Logging the structured errorCode + errorMessage is appropriate; logging the full exception is not.
Also, there's no separator between adjacent string fragments (...show err <> "error code : "...) — the resulting log line will read ...error code : ... jammed together.
🛡️ Proposed log-cleanup
- logError $ "Error while create external payout : " <> show err <> "error code : " <> show errorCode <> "error message : " <> show errorMessage <> " orderId: " <> req.orderId
+ logError $
+ "Error while creating external payout"
+ <> " errorCode: " <> show errorCode
+ <> " errorMessage: " <> show errorMessage
+ <> " orderId: " <> req.orderId🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` around lines 51 -
55, The code logs the entire StripeError via show (variable err) which may
contain PII; update the log to avoid printing the full exception and instead log
only the sanitized fields already extracted (errorCode and errorMessage) and the
orderId, and fix missing separators. Replace the existing logError call that
references err with a single formatted message like "Error while creating
external payout: code=<...> message=<...> orderId=<...>" using errorCode and
errorMessage (handle their Maybe values with sensible defaults such as
"unknown"), keep Payment.StripeError/err extraction as-is but do not include
show err anywhere, and ensure spaces/separators are present between fragments.
| buildCreateTransferReq :: CreateTransferReq -> m Stripe.TransferReq | ||
| buildCreateTransferReq CreateTransferReq {amount = amountInUsd, ..} = do | ||
| let amountInCents = eurToCents amountInUsd | ||
| destination <- case destinationAccount of | ||
| TransferConnectedAccount accountId -> pure accountId | ||
| TransferPlatformAccount -> config.platformAccountId & fromMaybeM (InternalError "STRIPE_PLATFORM_ACCOUNT_ID_NOT_FOUND") | ||
| pure Stripe.TransferReq {amount = amountInCents, metadata = Nothing, ..} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Stripe API amount field: which currencies are zero-decimal vs two-decimal vs three-decimal?
💡 Result:
In the Stripe API, the amount field is always an integer representing the value in the currency's smallest unit. Currencies are classified as follows for the amount field [1][2]: Zero-decimal currencies: Amount = nominal value (no multiplication by 100). Examples include JPY (to charge 10 JPY, use amount=10), and the full list from Stripe support is BIF, CLP, DJF, GNF, JPY, KMF, KRW, MGA, PYG, RWF, UGX, VND, VUV, XAF, XOF, XPF [3]. Two-decimal currencies: Amount = nominal value * 100 (e.g., USD: to charge 10.00 USD, use amount=1000). This is the default for all other supported currencies [1][2][4]. Three-decimal currencies: None. Stripe does not support any currencies with three decimal places in the API amount field. All amounts use integer minor units corresponding to 0 or 2 decimals; currencies like IDR (which has higher digit limits) are still treated as two-decimal [5]. Special cases (zero-decimal currencies treated as two-decimal for compatibility): - ISK, UGX: Use two-decimal amounts (e.g., 5 ISK = amount=500), cannot charge fractions [1][2]. - HUF, TWD: Can charge two-decimal, but payouts may round to even 100 units [1]. The official list of zero-decimal currencies is on https://stripe.com/docs/currencies (linked currencies), with details on exceptions [1][2].
Citations:
- 1: https://stripe.com/docs/currencies
- 2: https://docs.stripe.com/currencies
- 3: https://til.computer/stripe/stripe-zero-decimal-currencies
- 4: https://stripe.com/docs/api/payment_intents/create
- 5: https://docs.stripe.com/currencies?locale=id-ID&presentment-currency=CH
🏁 Script executed:
# First, locate and examine the Stripe.hs file
find . -type f -name "Stripe.hs" | head -20Repository: nammayatri/shared-kernel
Length of output: 203
🏁 Script executed:
# Get the file structure and size
wc -l lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hsRepository: nammayatri/shared-kernel
Length of output: 137
🏁 Script executed:
# Examine the specific lines mentioned in the review (103-109) and surrounding context
sed -n '95,120p' lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hsRepository: nammayatri/shared-kernel
Length of output: 1099
🏁 Script executed:
# Look for CreateTransferReq type definition
rg "type CreateTransferReq|data CreateTransferReq" -A 10Repository: nammayatri/shared-kernel
Length of output: 1069
🏁 Script executed:
# Look for currency conversion functions
rg "usdToCents|eurToCents|toMinorUnits|toCents" -B 2 -A 3Repository: nammayatri/shared-kernel
Length of output: 8003
🏁 Script executed:
# Check mkCreatePayoutReq around line 38
sed -n '30,50p' lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hsRepository: nammayatri/shared-kernel
Length of output: 890
🏁 Script executed:
# Search for Currency type definition and any currency-aware converters
rg "data Currency|type Currency" -A 5Repository: nammayatri/shared-kernel
Length of output: 1009
Currency conversion is hardcoded and ignores the currency field, creating a brittle design that will fail silently if zero-decimal currencies are added.
In buildCreateTransferReq (line 104–105), the variable is named amountInUsd but calls eurToCents, which is a naming inconsistency. More critically, both usdToCents and eurToCents are identical (multiply by 100), and the actual currency field in CreateTransferReq is completely ignored during conversion. The same issue appears in mkCreatePayoutReq (line 39), which also hardcodes usdToCents regardless of currency.
Currently, this works because only INR, USD, and EUR are supported—all 2-decimal currencies. However, if zero-decimal currencies (JPY, KRW, BHD, etc.) are ever added to the Currency enum, amounts will be silently off by 100× (Stripe expects amount=10 for 10 JPY, not amount=1000). The conversion must dispatch on currency, not use hardcoded helpers.
Suggested fix: currency-aware conversion
- buildCreateTransferReq CreateTransferReq {amount = amountInUsd, ..} = do
- let amountInCents = eurToCents amountInUsd
+ buildCreateTransferReq CreateTransferReq {..} = do
+ let amountInMinorUnits = toMinorUnits currency amount
destination <- case destinationAccount of
TransferConnectedAccount accountId -> pure accountId
TransferPlatformAccount -> config.platformAccountId & fromMaybeM (InternalError "STRIPE_PLATFORM_ACCOUNT_ID_NOT_FOUND")
- pure Stripe.TransferReq {amount = amountInCents, metadata = Nothing, ..}
+ pure Stripe.TransferReq {amount = amountInMinorUnits, metadata = Nothing, ..}Define toMinorUnits :: Currency -> HighPrecMoney -> Int to dispatch on currency (or unify the three converter functions into one).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs` around
lines 103 - 109, buildCreateTransferReq and mkCreatePayoutReq are using
hardcoded eurToCents/usdToCents and ignore the CreateTransferReq.currency, which
breaks zero-decimal currencies; replace those calls with a single currency-aware
converter (e.g. implement toMinorUnits :: Currency -> HighPrecMoney -> Int) that
dispatches on Currency (handling 0-, 2-, etc.-decimal currencies) and use it in
buildCreateTransferReq and mkCreatePayoutReq (rename amountInUsd to a neutral
name like amountInMinor or amountInBase before conversion) so amounts sent to
Stripe are correct for each currency.
| data CreateExternalPayoutResp = CreateExternalPayoutResp | ||
| { orderId :: Text, | ||
| externalPayoutStatus :: Maybe ExternalPayoutStatus, -- Stripe specific: payout status from driver/fleet connected account to driver/fleet bank account/card | ||
| orderType :: Maybe Text, | ||
| idAssignedByServiceProvider :: Maybe Text, -- Stripe specific | ||
| amount :: HighPrecMoney, | ||
| customerId :: Maybe Text | ||
| } | ||
|
|
||
| type ExternalPayoutOrderStatusResp = CreateExternalPayoutResp | ||
|
|
||
| data TransferAccount = TransferConnectedAccount AccountId | TransferPlatformAccount | ||
|
|
||
| data CreateTransferReq = CreateTransferReq | ||
| { amount :: HighPrecMoney, | ||
| currency :: Currency, | ||
| senderAccountId :: TransferAccount, | ||
| destinationAccount :: TransferAccount, | ||
| description :: Maybe Text | ||
| } | ||
|
|
||
| newtype CreateTransferResp = CreateTransferResp | ||
| { transferId :: TransferId | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add deriving clauses on the new public types for consistency and observability.
CreateExternalPayoutResp, TransferAccount, CreateTransferReq, and CreateTransferResp have no deriving clauses, while every other request/response type in this module (e.g., CreatePayoutOrderReq, CreatePayoutOrderResp, PayoutOrderStatusReq) derives at least Show, Generic, FromJSON, ToJSON, and (for response types) ToSchema. Without Show, these values cannot be logged in error/debug paths (e.g., the logError call site in Interface.hs for createPayoutOrder), and without JSON instances they cannot be persisted, exposed in webhooks, or surfaced in OpenAPI specs.
♻️ Proposed deriving clauses
data CreateExternalPayoutResp = CreateExternalPayoutResp
{ orderId :: Text,
externalPayoutStatus :: Maybe ExternalPayoutStatus, -- Stripe specific: payout status from driver/fleet connected account to driver/fleet bank account/card
orderType :: Maybe Text,
idAssignedByServiceProvider :: Maybe Text, -- Stripe specific
amount :: HighPrecMoney,
customerId :: Maybe Text
}
+ deriving stock (Show, Eq, Generic)
+ deriving anyclass (FromJSON, ToJSON, ToSchema)
data TransferAccount = TransferConnectedAccount AccountId | TransferPlatformAccount
+ deriving stock (Show, Eq, Generic)
+ deriving anyclass (FromJSON, ToJSON, ToSchema)
data CreateTransferReq = CreateTransferReq
{ amount :: HighPrecMoney,
currency :: Currency,
senderAccountId :: TransferAccount,
destinationAccount :: TransferAccount,
description :: Maybe Text
}
+ deriving stock (Show, Eq, Generic)
+ deriving anyclass (FromJSON, ToJSON, ToSchema)
newtype CreateTransferResp = CreateTransferResp
{ transferId :: TransferId
}
+ deriving stock (Show, Eq, Generic)
+ deriving anyclass (FromJSON, ToJSON, ToSchema)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs` around lines
126 - 149, Add missing deriving clauses to the new public types so they match
other request/response types: for CreateExternalPayoutResp and
CreateTransferResp add "deriving (Show, Generic, FromJSON, ToJSON, ToSchema)";
for CreateTransferReq and TransferAccount add "deriving (Show, Generic,
FromJSON, ToJSON)". Update the declarations for CreateExternalPayoutResp,
TransferAccount, CreateTransferReq, and CreateTransferResp to include these
deriving clauses so they can be logged, serialized, and included in OpenAPI
specs.
There was a problem hiding this comment.
generally interface types don't need any instances like ToJSON and FromJSON. Without these insatances interface type won't be used accidentally in any ui request or response (and no backward compatibility issues possible if we change interface types)
There was a problem hiding this comment.
@roman-bodavskiy, that's a sensible design guard — keeping interface types free of ToJSON/FromJSON/ToSchema prevents them from leaking into API surfaces and sidesteps backward-compatibility constraints when the types evolve. I'll drop that part of the suggestion. I've noted this for future reviews.
✏️ Learnings added
Learnt from: roman-bodavskiy
Repo: nammayatri/shared-kernel PR: 1086
File: lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs:126-149
Timestamp: 2026-05-08T09:00:17.731Z
Learning: In `lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs` (Haskell, shared-kernel repo), interface types such as `CreateExternalPayoutResp`, `TransferAccount`, `CreateTransferReq`, and `CreateTransferResp` intentionally do NOT derive `ToJSON`, `FromJSON`, or `ToSchema`. This is by design: keeping these instances absent prevents interface types from being accidentally used in UI request/response types and avoids backward-compatibility issues when interface types need to change.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
roman-bodavskiy
force-pushed
the
backend/feat/stripe-transfer-api
branch
2 times, most recently
from
34f777b to
5115397
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs (2)
327-352: ⚡ Quick winNaming vs conversion mismatch:
amountInUsdis converted viaeurToCents.
createPlatformChargebinds the request amount asamountInUsdbut then callseurToCents(lines 337, 343), while the siblingcreateConnectedAccountCharge(lines 369–370) usesusdToCents. Both helpers are currently identical (* 100), so this works today, but the inconsistency is confusing for readers and fragile if the helpers ever diverge or a non-2-decimal currency is supported.Since
CreatePaymentIntentReqcarries an explicitcurrency :: Currencyfield, prefer a single currency-aware converter (or unifyusdToCents/eurToCentsinto one) and use it consistently acrosscreatePlatformCharge,createConnectedAccountCharge,mkRefundReq,capturePaymentIntent, andupdateAmountInPaymentIntent.♻️ Localized cleanup for the platform-charge path
- createPlatformCharge url apiKey CreatePaymentIntentReq {amount = amountInUsd, ..} = do + createPlatformCharge url apiKey CreatePaymentIntentReq {amount = amountInUsd, ..} = do let paymentIntentReq = mkPlatformPaymentIntentReq paymentIntentResp <- Stripe.createPaymentIntent url apiKey paymentIntentReq ... where mkPlatformPaymentIntentReq :: Stripe.PaymentIntentReq mkPlatformPaymentIntentReq = - let amountInCents = eurToCents amountInUsd + let amountInCents = usdToCents amountInUsd ... (transfer_data, application_fee_amount) = case config.transferPaymentToConnectedAccount of Just False -> (Nothing, Nothing) - _ -> (Just $ Stripe.TransferData {destination = driverAccountId}, Just $ eurToCents applicationFeeAmount) + _ -> (Just $ Stripe.TransferData {destination = driverAccountId}, Just $ usdToCents applicationFeeAmount)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs` around lines 327 - 352, The amount conversion is using eurToCents while the request field is named amountInUsd and other paths use usdToCents; unify to a single currency-aware converter (e.g., amountToMinorUnits :: Currency -> Decimal -> Int) and replace direct calls to eurToCents/usdToCents inside createPlatformCharge (mkPlatformPaymentIntentReq), createConnectedAccountCharge, mkRefundReq, capturePaymentIntent, and updateAmountInPaymentIntent to call amountToMinorUnits using the request’s currency field; ensure transfer/application_fee conversions also use the new converter and update any helper imports/exports accordingly so all paths consistently convert based on CreatePaymentIntentReq.currency.
742-749: ⚡ Quick winSame naming/conversion mismatch in
mkRefundReq.
amountInUsdis converted viaeurToCentshere (line 745), mirroring the inconsistency increatePlatformCharge. Aligning all of these on a single converter (ideally currency-aware, sinceCreateRefundReqis part of an interface that already exposes a currency concept upstream) avoids the cross-helper drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs` around lines 742 - 749, mkRefundReq is using eurToCents on amountInUsd causing a currency/conversion mismatch (same issue as createPlatformCharge); update mkRefundReq (the CreateRefundReq pattern match and where amountInCents is computed) to use the proper currency-aware conversion instead of eurToCents — either call the USD-specific converter (e.g. usdToCents) if CreateRefundReq is guaranteed USD, or wire in the shared converter that reads the request currency (e.g. amountToCents req.currency amountInUsd) so the Stripe.RefundReq.amount is computed with the correct converter.lib/mobility-core/src/Kernel/External/Payment/Interface/Types.hs (1)
924-924: ⚡ Quick winSilent loss of currency info when
readMaybefails.Making
GetRefundResp.currencyaMaybe Currencyaccepts that Stripe's currency string may not map onto theCurrencyenum, but the conversion happens silently inmkGetRefundResp(readMaybe . T.unpack . T.toUpper). If Stripe ever returns a currency code not in theCurrencyenum (e.g., a region not yet onboarded), downstream consumers will seeNothingwith no diagnostic, and any reconciliation/ledger logic will lose the information.Two cheaper alternatives worth considering:
- Keep the underlying refund
currencyasTextend-to-end and only convert at the boundary that needsCurrency.- Log a warning in
mkGetRefundRespwhenreadMaybereturnsNothingso unsupported currencies are observable rather than silently dropped.This is a downstream effect of the
Refund.hschange; flagging here at the interface boundary where the loss becomes user-visible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Types.hs` at line 924, GetRefundResp.currency is defined as Maybe Currency and mkGetRefundResp uses readMaybe . T.unpack . T.toUpper to parse Stripe's currency, which silently drops unsupported currency strings; update the code so currency is preserved and failures are observable: either (A) change the GetRefundResp.currency type to Text (or keep original Text field) and defer Currency parsing to the consumer boundary, or (B) keep Maybe Currency but instrument mkGetRefundResp to log a warning when readMaybe returns Nothing (include the raw currency Text and the refund id/context) so unsupported codes are visible; locate references to GetRefundResp.currency and mkGetRefundResp and implement one of these fixes, adjusting surrounding uses accordingly.lib/mobility-core/src/Kernel/External/Payout/Interface.hs (1)
136-147: 💤 Low valuePublic
createTransferonly supports Stripe — confirm this is the intended public surface.Exposing
createTransferthrough the unifiedPayoutServiceConfiginterface whileJuspayConfigsimply throwsInternalError "Juspay Create Transfer not supported."makes the unsupported case a runtime failure rather than a compile-time guarantee. If transfers are conceptually Stripe-only, consider takingStripeConfigdirectly at the call sites that need it (and not routing throughPayoutServiceConfig), so the type system prevents misuse.If the unified surface is preferred for symmetry with other payout operations, please ensure callers branch on the active service before calling
createTransfer, and the error message is user-actionable (e.g., reference the configured service).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` around lines 136 - 147, The public createTransfer currently accepts a PayoutServiceConfig but only implements Stripe, causing a runtime InternalError for Juspay; either narrow the API to accept StripeConfig directly (change createTransfer's config parameter to StripeConfig and update callers to pass Stripe.createTransfer only), or keep the unified signature but make the failure actionable: change the JuspayConfig branch in createTransfer to return a clear error that references the active service (e.g., "createTransfer not supported for Juspay") and update all callers to switch/branch on PayoutServiceConfig before calling createTransfer to avoid calling it for unsupported services; locate the code in createTransfer and related call sites to apply one of these fixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs`:
- Around line 65-98: The current mkCreatePayoutOrderResp builds
CreatePayoutOrderResp with a hardcoded status = SUCCESS even when
CreateExternalPayoutResp indicates failure (externalPayoutStatus = Just
EXTERNAL_PAYOUT_FAILED); change mkCreatePayoutOrderResp (and any helper that
constructs CreatePayoutOrderResp) to compute status from both CreateTransferResp
and CreateExternalPayoutResp so the top-level PayoutOrderStatus reflects the
worst sub-step (e.g., set status = FAILED or introduce/return a
PARTIAL/PENDING_PAYOUT state when transfer succeeded but external payout
failed), and update any callers or add/docs/comments near
mkCreatePayoutOrderResp and the CreatePayoutOrderResp type to require checking
externalPayoutStatus if you must keep an interim state, ensuring idempotency is
used for retries rather than masking failures by forcing SUCCESS.
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs`:
- Around line 327-352: The amount conversion is using eurToCents while the
request field is named amountInUsd and other paths use usdToCents; unify to a
single currency-aware converter (e.g., amountToMinorUnits :: Currency -> Decimal
-> Int) and replace direct calls to eurToCents/usdToCents inside
createPlatformCharge (mkPlatformPaymentIntentReq), createConnectedAccountCharge,
mkRefundReq, capturePaymentIntent, and updateAmountInPaymentIntent to call
amountToMinorUnits using the request’s currency field; ensure
transfer/application_fee conversions also use the new converter and update any
helper imports/exports accordingly so all paths consistently convert based on
CreatePaymentIntentReq.currency.
- Around line 742-749: mkRefundReq is using eurToCents on amountInUsd causing a
currency/conversion mismatch (same issue as createPlatformCharge); update
mkRefundReq (the CreateRefundReq pattern match and where amountInCents is
computed) to use the proper currency-aware conversion instead of eurToCents —
either call the USD-specific converter (e.g. usdToCents) if CreateRefundReq is
guaranteed USD, or wire in the shared converter that reads the request currency
(e.g. amountToCents req.currency amountInUsd) so the Stripe.RefundReq.amount is
computed with the correct converter.
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Types.hs`:
- Line 924: GetRefundResp.currency is defined as Maybe Currency and
mkGetRefundResp uses readMaybe . T.unpack . T.toUpper to parse Stripe's
currency, which silently drops unsupported currency strings; update the code so
currency is preserved and failures are observable: either (A) change the
GetRefundResp.currency type to Text (or keep original Text field) and defer
Currency parsing to the consumer boundary, or (B) keep Maybe Currency but
instrument mkGetRefundResp to log a warning when readMaybe returns Nothing
(include the raw currency Text and the refund id/context) so unsupported codes
are visible; locate references to GetRefundResp.currency and mkGetRefundResp and
implement one of these fixes, adjusting surrounding uses accordingly.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs`:
- Around line 136-147: The public createTransfer currently accepts a
PayoutServiceConfig but only implements Stripe, causing a runtime InternalError
for Juspay; either narrow the API to accept StripeConfig directly (change
createTransfer's config parameter to StripeConfig and update callers to pass
Stripe.createTransfer only), or keep the unified signature but make the failure
actionable: change the JuspayConfig branch in createTransfer to return a clear
error that references the active service (e.g., "createTransfer not supported
for Juspay") and update all callers to switch/branch on PayoutServiceConfig
before calling createTransfer to avoid calling it for unsupported services;
locate the code in createTransfer and related call sites to apply one of these
fixes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 253ee307-f20a-44ce-b455-09bee6cb1f47
📒 Files selected for processing (16)
lib/mobility-core/mobility-core.caballib/mobility-core/src/Kernel/External/Payment/Interface.hslib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hslib/mobility-core/src/Kernel/External/Payment/Interface/Types.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Config.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Types/PaymentIntent.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Types/Refund.hslib/mobility-core/src/Kernel/External/Payout/Interface.hslib/mobility-core/src/Kernel/External/Payout/Interface/Juspay.hslib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hslib/mobility-core/src/Kernel/External/Payout/Interface/Types.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Config.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Flow.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Types.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Payout.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Transfer.hs
✅ Files skipped from review due to trivial changes (1)
- lib/mobility-core/mobility-core.cabal
🚧 Files skipped from review as they are similar to previous changes (7)
- lib/mobility-core/src/Kernel/External/Payout/Stripe/Types.hs
- lib/mobility-core/src/Kernel/External/Payment/Stripe/Config.hs
- lib/mobility-core/src/Kernel/External/Payment/Stripe/Types/PaymentIntent.hs
- lib/mobility-core/src/Kernel/External/Payout/Stripe/Config.hs
- lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs
- lib/mobility-core/src/Kernel/External/Payment/Interface.hs
- lib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Transfer.hs
roman-bodavskiy
force-pushed
the
backend/feat/stripe-transfer-api
branch
2 times, most recently
from
28f1530 to
ed1a107
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Transfer.hs (1)
29-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
metadatafield is silently dropped — never sent to Stripe.
TransferReqdeclaresmetadata :: Maybe Metadata(line 24) but theToForminstance omits it entirely. Any metadata passed by callers is discarded before the HTTP request is made.Stripe's metadata is a set of key-value pairs that can be attached to an object. Individual keys can be unset by posting an empty value to them. All keys can be unset by posting an empty value to metadata. Stripe expects these as
metadata[key]=valuepairs in form encoding.🐛 Proposed fix
instance ToForm TransferReq where toForm TransferReq {..} = Form $ HM.fromList $ catMaybes [ Just . ("amount",) . pure $ toQueryParam amount, Just . ("currency",) . pure $ toQueryParam currency, Just . ("destination",) . pure $ toQueryParam destination, + ("metadata",) . pure . toQueryParam <$> metadata, ("description",) . pure . toQueryParam <$> description ]Verify that
ToHttpApiData Metadataserialises to themetadata[key]=valueformat Stripe expects; if not, a helper convertingMetadata -> [(Text, [Text])]with keys like"metadata[KEY]"should be used instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Transfer.hs` around lines 29 - 38, The ToForm instance for TransferReq currently omits the metadata field so any metadata is dropped; update the TransferReq ToForm (toForm for TransferReq) to include metadata by serializing Metadata into form pairs named "metadata[KEY]=value" (or use ToHttpApiData Metadata if it is already producing that exact format), e.g. produce additional entries from the TransferReq metadata :: Maybe Metadata into the Form HM list; if ToHttpApiData does not match Stripe's expected "metadata[key]" keys, add a small helper to convert Metadata -> [(Text, [Text])] where each key is prefixed with "metadata[KEY]" and include those pairs in the catMaybes list alongside amount/currency/destination/description.
🧹 Nitpick comments (2)
lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs (1)
339-339: ⚡ Quick winUnify amount-to-cents conversion source to avoid future currency drift.
These changed paths now mix
eurToCentsandusdToCentsfor similarly namedamountInUsdvalues. It works today only because both helpers are currently identical, but this will become a correctness trap once currency-specific conversion is implemented.Suggested refactor
- let amountInCents = eurToCents amountInUsd + let amountInCents = moneyToCents amountInUsd currency ... - let application_fee_amount = Just $ usdToCents applicationFeeAmount - let amountInCents = usdToCents amountInUsd + let application_fee_amount = Just $ moneyToCents applicationFeeAmount currency + let amountInCents = moneyToCents amountInUsd currency ... - amountInCents = eurToCents <$> amountInUsd + amountInCents = (`moneyToCents` currency) <$> amountInUsd-- centralized conversion (example) moneyToCents :: HighPrecMoney -> Currency -> Int moneyToCents (HighPrecMoney money) _ = round (money * 100)Also applies to: 370-372, 744-747
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs` at line 339, The code mixes eurToCents and usdToCents for values named amountInUsd (e.g. the let binding amountInCents = eurToCents amountInUsd), risking future currency drift; fix by centralizing conversion and replacing currency-specific helper calls with a single conversion used everywhere (either call usdToCents for amountInUsd or better add a generic moneyToCents and use it for all conversions). Update all occurrences referenced (the eurToCents call at amountInUsd, the similar spots around lines noted and the blocks that use eurToCents/usdToCents) to call moneyToCents (or consistently use usdToCents) and ensure the helper accepts the money and currency or the correct USD-specific helper for variables named amountInUsd.lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs (1)
126-149: ⚡ Quick winNew types are missing
ShowandGenericderiving — blocks logging and generic utilities.
CreateExternalPayoutResp,TransferAccount,CreateTransferReq, andCreateTransferResphave no deriving clauses at all. Every other concrete type in this module (CreatePayoutOrderReq,CreatePayoutOrderResp,OrderStatusPayoutResp) derives at leastShowandGeneric. WithoutShow, these values can't be emitted in log messages or test failure output; withoutGeneric, generic-based lenses and utilities won't apply. This is distinct from the intentional omission ofToJSON/FromJSON/ToSchemadiscussed previously.♻️ Proposed fix
data CreateExternalPayoutResp = CreateExternalPayoutResp { orderId :: Text, externalPayoutStatus :: Maybe ExternalPayoutStatus, orderType :: Maybe Text, idAssignedByServiceProvider :: Maybe Text, amount :: HighPrecMoney, customerId :: Maybe Text } + deriving stock (Show, Eq, Generic) type ExternalPayoutOrderStatusResp = CreateExternalPayoutResp -data TransferAccount = TransferConnectedAccount AccountId | TransferPlatformAccount +data TransferAccount = TransferConnectedAccount AccountId | TransferPlatformAccount + deriving stock (Show, Eq, Generic) data CreateTransferReq = CreateTransferReq { amount :: HighPrecMoney, currency :: Currency, senderAccountId :: TransferAccount, destinationAccount :: TransferAccount, description :: Maybe Text } + deriving stock (Show, Eq, Generic) newtype CreateTransferResp = CreateTransferResp { transferId :: TransferId } + deriving stock (Show, Eq, Generic)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs` around lines 126 - 149, The listed concrete types (CreateExternalPayoutResp, TransferAccount, CreateTransferReq, CreateTransferResp) are missing deriving clauses which prevents logging and generic utilities; add "deriving (Show, Generic)" to each data/newtype declaration and ensure GHC.Generics.Generic is in scope (add import GHC.Generics if not already present) so these types can be printed and used with generic-based tooling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@lib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Transfer.hs`:
- Around line 29-38: The ToForm instance for TransferReq currently omits the
metadata field so any metadata is dropped; update the TransferReq ToForm (toForm
for TransferReq) to include metadata by serializing Metadata into form pairs
named "metadata[KEY]=value" (or use ToHttpApiData Metadata if it is already
producing that exact format), e.g. produce additional entries from the
TransferReq metadata :: Maybe Metadata into the Form HM list; if ToHttpApiData
does not match Stripe's expected "metadata[key]" keys, add a small helper to
convert Metadata -> [(Text, [Text])] where each key is prefixed with
"metadata[KEY]" and include those pairs in the catMaybes list alongside
amount/currency/destination/description.
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs`:
- Line 339: The code mixes eurToCents and usdToCents for values named
amountInUsd (e.g. the let binding amountInCents = eurToCents amountInUsd),
risking future currency drift; fix by centralizing conversion and replacing
currency-specific helper calls with a single conversion used everywhere (either
call usdToCents for amountInUsd or better add a generic moneyToCents and use it
for all conversions). Update all occurrences referenced (the eurToCents call at
amountInUsd, the similar spots around lines noted and the blocks that use
eurToCents/usdToCents) to call moneyToCents (or consistently use usdToCents) and
ensure the helper accepts the money and currency or the correct USD-specific
helper for variables named amountInUsd.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs`:
- Around line 126-149: The listed concrete types (CreateExternalPayoutResp,
TransferAccount, CreateTransferReq, CreateTransferResp) are missing deriving
clauses which prevents logging and generic utilities; add "deriving (Show,
Generic)" to each data/newtype declaration and ensure GHC.Generics.Generic is in
scope (add import GHC.Generics if not already present) so these types can be
printed and used with generic-based tooling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d483e918-064c-4409-b047-1e2a0926872d
📒 Files selected for processing (16)
lib/mobility-core/mobility-core.caballib/mobility-core/src/Kernel/External/Payment/Interface.hslib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hslib/mobility-core/src/Kernel/External/Payment/Interface/Types.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Config.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Types/PaymentIntent.hslib/mobility-core/src/Kernel/External/Payment/Stripe/Types/Refund.hslib/mobility-core/src/Kernel/External/Payout/Interface.hslib/mobility-core/src/Kernel/External/Payout/Interface/Juspay.hslib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hslib/mobility-core/src/Kernel/External/Payout/Interface/Types.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Config.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Flow.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Types.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Payout.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Transfer.hs
🚧 Files skipped from review as they are similar to previous changes (13)
- lib/mobility-core/src/Kernel/External/Payout/Interface/Juspay.hs
- lib/mobility-core/src/Kernel/External/Payout/Stripe/Config.hs
- lib/mobility-core/src/Kernel/External/Payment/Interface/Types.hs
- lib/mobility-core/mobility-core.cabal
- lib/mobility-core/src/Kernel/External/Payout/Stripe/Flow.hs
- lib/mobility-core/src/Kernel/External/Payment/Interface.hs
- lib/mobility-core/src/Kernel/External/Payout/Stripe/Types.hs
- lib/mobility-core/src/Kernel/External/Payout/Stripe/Types/Payout.hs
- lib/mobility-core/src/Kernel/External/Payment/Stripe/Config.hs
- lib/mobility-core/src/Kernel/External/Payment/Stripe/Types/PaymentIntent.hs
- lib/mobility-core/src/Kernel/External/Payout/Interface.hs
- lib/mobility-core/src/Kernel/External/Payment/Stripe/Types/Refund.hs
- lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
lib/mobility-core/src/Kernel/External/Payout/Interface.hs (3)
50-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLogging full
StripeErrorviashow errrisks leaking PII; also handles non-StripeErrorexceptions poorly.Two issues on the
logErrorat line 53, one of which was flagged previously:
show err(whereerr :: Maybe Payment.StripeError) renders the full Stripe error structure, which can echo back request fields submitted to Stripe (customer email, name, bank/card identifiers, descriptions). Log only the sanitizederrorCode/errorMessageyou've already extracted.- If
eis not aPayment.StripeError,fromException@payment.StripeErrorereturnsNothing, so the log line ends up witherror code : Nothing error message : Nothingand the original exception is silently dropped. At minimum logdisplayException efor the non-Stripe fallback.Also: there are no spaces between fragments (
...show err <> "error code : "...), so the line concatenates into...Nothingerror code : ....🛡️ Proposed log-cleanup
- logError $ "Error while create external payout : " <> show err <> "error code : " <> show errorCode <> "error message : " <> show errorMessage <> " orderId: " <> req.orderId + logError $ + "Error while creating external payout" + <> " orderId: " <> req.orderId + <> " errorCode: " <> fromMaybe "unknown" (show <$> errorCode) + <> " errorMessage: " <> fromMaybe "unknown" errorMessage + <> maybe (" rawException: " <> T.pack (displayException e)) (const "") err🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` around lines 50 - 53, The log currently prints the full Maybe StripeError (err) and can leak PII and also drops non-Stripe exceptions; update the log in the error handler so it does NOT use show err, instead log only the sanitized errorCode and errorMessage (the existing errorCode/errorMessage values) and, if fromException `@Payment.StripeError` e is Nothing, include displayException e as a fallback; also ensure you add proper spacing between fragments so messages don't concatenate. Locate the block using err, errorCode, errorMessage and the exception value e in the function that calls logError and replace the single show err usage with the sanitized fields plus a conditional fallback to displayException e.
98-105:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftStatus mapping conflates terminal failure with in-flight transfer.
EXTERNAL_PAYOUT_FAILEDandEXTERNAL_PAYOUT_CANCELEDboth map toTRANSFERRED. The design intent (per the comment and prior discussion) is to prevent callers from retrying once the platform→connected-account transfer has settled, but the semantic cost is real: every downstream consumer that pattern-matchesPayoutOrderStatus.TRANSFERREDmust now additionally inspectexternalPayoutStatusto know whether the connected-account→bank leg actually succeeded, failed, or was cancelled. A singleTRANSFERREDbucket can't distinguish "still in flight" from "permanently failed".Consider either (a) introducing distinct terminal statuses (e.g.,
TRANSFER_PAID,TRANSFER_FAILED_AT_PAYOUT) and let the cast preserve the distinction, or (b) at minimum, documenting at the type/declaration site ofPayoutOrderStatusthat Stripe consumers MUST consultexternalPayoutStatuswheneverstatus == TRANSFERRED.Also nit: "successfull" → "successful" in the comment at line 98.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` around lines 98 - 105, The current castExternalPayoutStatusToStatus conflates terminal failure/cancel states into TRANSFERRED (see ExternalPayoutStatus -> PayoutOrderStatus mapping in castExternalPayoutStatusToStatus), so either add distinct terminal statuses on PayoutOrderStatus (e.g., TRANSFER_PAID and TRANSFER_FAILED_AT_PAYOUT or TRANSFER_CANCELED) and map EXTERNAL_PAYOUT_PAID -> TRANSFER_PAID, EXTERNAL_PAYOUT_FAILED -> TRANSFER_FAILED_AT_PAYOUT, EXTERNAL_PAYOUT_CANCELED -> TRANSFER_CANCELED (leaving in-flight ones as TRANSFERRED), or if you cannot change the enum, add a clear type-level comment on PayoutOrderStatus that callers MUST consult externalPayoutStatus when status == TRANSFERRED; also fix the comment spelling "successfull" -> "successful".
42-63:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftIdempotency on
Stripe.createTransferstill missing — retry ofcreatePayoutOrdercan double-fund the connected account.This was raised on a prior commit and remains unaddressed: line 44 calls
Stripe.createTransferas the first side-effect, then line 46 callsStripe.createExternalPayout. Any client retry ofcreatePayoutOrder(transient network/timeout on the second call, caller-side crash after step 1) will issue a second Stripe transfer for the samereq.orderId. TheIMPORTANTcomment at line 98 documents the intent ("to avoid multiple charges") but the actual safeguard — Stripe'sIdempotency-Keyheader on/v1/transfers— is not wired throughCreateTransferReqor the Stripe flow.Recommended: add
idempotencyKey :: TexttoCreateTransferReqinlib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs, plumb it throughCreateTransferAPIas theIdempotency-Keyheader, and derive it fromreq.orderIdhere (or a request-scoped UUID).Separately, the synthetic
CreateExternalPayoutRespconstructed on the error branch (lines 55–62) hard-codesidAssignedByServiceProvider = Nothing. If the exception came from a network failure after Stripe successfully created the external payout, the real Stripe-sideidexists but is now unrecoverable from this response — which will break later reconciliation (payoutOrderStatushas noidAssignedByServiceProviderto query with either, since it only hastransferId). Consider persisting whatever is recoverable from the error, or treating partial failures as an explicit retry/poll state rather than synthesizing a "FAILED" response.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` around lines 42 - 63, The Stripe transfer call can be duplicated on retries because CreateTransferReq lacks an Idempotency-Key; update the flow by adding idempotencyKey :: Text to the CreateTransferReq type (in Interface/Types.hs), thread it through the CreateTransferAPI so it is sent as the Idempotency-Key header in Stripe.createTransfer, and derive/pass req.orderId (or a request-scoped UUID) as that idempotencyKey when calling Stripe.createTransfer from createPayoutOrder (the code around Stripe.createTransfer cfg (mkTransferReq connectedAccountId req)). Also adjust the error branch that constructs CreateExternalPayoutResp in createPayoutOrder: don’t always set idAssignedByServiceProvider = Nothing — instead capture and persist any provider id available from the exception/result when possible or change the synthesized response to a retry/poll/unknown state so reconciliation can query Stripe with the correct identifier rather than losing the external id.
🧹 Nitpick comments (1)
lib/mobility-core/src/Kernel/External/Payout/Interface.hs (1)
118-141: 💤 Low valueClarify the
req.transferIdfallback in status mapping.Line 126 silently swaps in
req.currentStatuswheneverreq.transferIdisNothing, even though the Stripe API call at line 119 has just been made andexternalPayoutStatusis in hand. This is hard to follow without context — readers will reasonably ask why we paid for a network round-trip just to discard its outcome. A brief comment explaining the invariant (e.g., "without atransferIdwe can't reliably correlate the Stripe response to this order, so trust the caller's last-known status") would help the next maintainer.If
req.transferIdis structurally required for the Stripe path to be meaningful, consider rejectingNothingup-front at the function boundary (similar to howcreatePayoutOrderrequiresmConnectedAccountId) rather than silently falling back mid-construction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` around lines 118 - 141, The status mapping in mkPayoutOrderStatusResp (pattern-matching CreateExternalPayoutResp) currently uses req.currentStatus whenever req.transferId is Nothing, which is confusing; either add a short comment above the status line explaining the invariant ("without a transferId we cannot reliably correlate the Stripe externalPayoutStatus to this order, so we trust req.currentStatus"), or if the Stripe path truly requires a transferId, validate req.transferId at the function boundary (like createPayoutOrder does for mConnectedAccountId) and return an error/Left early instead of performing the Stripe call and falling back silently; update the code in mkPayoutOrderStatusResp or the caller accordingly (referencing mkPayoutOrderStatusResp, CreateExternalPayoutResp, externalPayoutStatus, and req.transferId).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs`:
- Line 94: The two request types use different aliases for the same Stripe-side
identifier; update PayoutOrderStatusReq.mConnectedAccountId to use
Stripe.AccountId (the same alias used by
CreatePayoutOrderReq.mConnectedAccountId) instead of the local AccountId so both
fields consistently use Stripe.AccountId; locate the PayoutOrderStatusReq type
and replace the AccountId type alias with Stripe.AccountId and adjust imports if
necessary.
---
Duplicate comments:
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs`:
- Around line 50-53: The log currently prints the full Maybe StripeError (err)
and can leak PII and also drops non-Stripe exceptions; update the log in the
error handler so it does NOT use show err, instead log only the sanitized
errorCode and errorMessage (the existing errorCode/errorMessage values) and, if
fromException `@Payment.StripeError` e is Nothing, include displayException e as a
fallback; also ensure you add proper spacing between fragments so messages don't
concatenate. Locate the block using err, errorCode, errorMessage and the
exception value e in the function that calls logError and replace the single
show err usage with the sanitized fields plus a conditional fallback to
displayException e.
- Around line 98-105: The current castExternalPayoutStatusToStatus conflates
terminal failure/cancel states into TRANSFERRED (see ExternalPayoutStatus ->
PayoutOrderStatus mapping in castExternalPayoutStatusToStatus), so either add
distinct terminal statuses on PayoutOrderStatus (e.g., TRANSFER_PAID and
TRANSFER_FAILED_AT_PAYOUT or TRANSFER_CANCELED) and map EXTERNAL_PAYOUT_PAID ->
TRANSFER_PAID, EXTERNAL_PAYOUT_FAILED -> TRANSFER_FAILED_AT_PAYOUT,
EXTERNAL_PAYOUT_CANCELED -> TRANSFER_CANCELED (leaving in-flight ones as
TRANSFERRED), or if you cannot change the enum, add a clear type-level comment
on PayoutOrderStatus that callers MUST consult externalPayoutStatus when status
== TRANSFERRED; also fix the comment spelling "successfull" -> "successful".
- Around line 42-63: The Stripe transfer call can be duplicated on retries
because CreateTransferReq lacks an Idempotency-Key; update the flow by adding
idempotencyKey :: Text to the CreateTransferReq type (in Interface/Types.hs),
thread it through the CreateTransferAPI so it is sent as the Idempotency-Key
header in Stripe.createTransfer, and derive/pass req.orderId (or a
request-scoped UUID) as that idempotencyKey when calling Stripe.createTransfer
from createPayoutOrder (the code around Stripe.createTransfer cfg (mkTransferReq
connectedAccountId req)). Also adjust the error branch that constructs
CreateExternalPayoutResp in createPayoutOrder: don’t always set
idAssignedByServiceProvider = Nothing — instead capture and persist any provider
id available from the exception/result when possible or change the synthesized
response to a retry/poll/unknown state so reconciliation can query Stripe with
the correct identifier rather than losing the external id.
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs`:
- Around line 118-141: The status mapping in mkPayoutOrderStatusResp
(pattern-matching CreateExternalPayoutResp) currently uses req.currentStatus
whenever req.transferId is Nothing, which is confusing; either add a short
comment above the status line explaining the invariant ("without a transferId we
cannot reliably correlate the Stripe externalPayoutStatus to this order, so we
trust req.currentStatus"), or if the Stripe path truly requires a transferId,
validate req.transferId at the function boundary (like createPayoutOrder does
for mConnectedAccountId) and return an error/Left early instead of performing
the Stripe call and falling back silently; update the code in
mkPayoutOrderStatusResp or the caller accordingly (referencing
mkPayoutOrderStatusResp, CreateExternalPayoutResp, externalPayoutStatus, and
req.transferId).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da389f7f-7d5b-44e7-ba66-6b6c1550f320
📒 Files selected for processing (5)
lib/mobility-core/src/Kernel/External/Payout/Interface.hslib/mobility-core/src/Kernel/External/Payout/Interface/Juspay.hslib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hslib/mobility-core/src/Kernel/External/Payout/Interface/Types.hslib/mobility-core/src/Kernel/External/Payout/Juspay/Types/Payout.hs
💤 Files with no reviewable changes (1)
- lib/mobility-core/src/Kernel/External/Payout/Juspay/Types/Payout.hs
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs
| isDynamicWebhookRequired :: Bool, | ||
| mRoutingId :: Maybe Text, -- Juspay specific | ||
| mConnectedAccountId :: Maybe AccountId, -- Stripe specific | ||
| mConnectedAccountId :: Maybe Stripe.AccountId, -- Stripe specific |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether Stripe.AccountId is a Text type synonym or a newtype, and whether
# that affects the local AccountId = Text usage downstream.
fd -e hs 'Stripe' lib/mobility-core/src/Kernel/External/Payment/Stripe/Types | head -20
rg -nP '^\s*(type|newtype|data)\s+AccountId\b' lib/mobility-core/src/Kernel/External/Payment/Stripe -C2
rg -nP 'mConnectedAccountId' --type=hs -C2Repository: nammayatri/shared-kernel
Length of output: 3538
Use consistent type alias for mConnectedAccountId across request types.
CreatePayoutOrderReq.mConnectedAccountId uses Stripe.AccountId (line 94) while PayoutOrderStatusReq.mConnectedAccountId uses the local AccountId (line 138). Both resolve to Text, so there's no type error, but the inconsistency is confusing since both fields refer to the same Stripe-side identifier. For clarity, use Stripe.AccountId consistently across both types.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs` at line 94,
The two request types use different aliases for the same Stripe-side identifier;
update PayoutOrderStatusReq.mConnectedAccountId to use Stripe.AccountId (the
same alias used by CreatePayoutOrderReq.mConnectedAccountId) instead of the
local AccountId so both fields consistently use Stripe.AccountId; locate the
PayoutOrderStatusReq type and replace the AccountId type alias with
Stripe.AccountId and adjust imports if necessary.
e8b87af to
59733c0
Compare
…ngle interface function; removed automatic transfer in create payment intent api
roman-bodavskiy
force-pushed
the
backend/feat/stripe-transfer-api
branch
from
59733c0 to
172b420
Compare
1 participant
Type of Change
Description
Additional Changes
Motivation and Context
How did you test it?
Checklist
./dev/format-all-files.shSummary by CodeRabbit
New Features
Bug Fixes
Chores