added ProgramWalletBalanceResp type#1154
Conversation
Hari-Krishna-Dupaguntla
force-pushed
the
feat_added_insert_query
branch
from
d6ce0e2 to
aa72c4d
Compare
Hari-Krishna-Dupaguntla
force-pushed
the
feat_added_insert_query
branch
from
aa72c4d to
64a684c
Compare
Hari-Krishna-Dupaguntla
force-pushed
the
feat_added_insert_query
branch
from
64a684c to
b5f9eb5
Compare
WalkthroughAdds paymentRules to create payment/order requests, enriches order status responses with loyaltyInfo and txnList, expands Juspay and PaytmEDC types for loyalty/transaction details, and introduces a new Juspay wallet loyaltyInfo flow with DTOs, config, and API client integration. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WalletIF as Wallet.Interface
participant WalletJ as Wallet.Juspay
participant JuspayFlow as Juspay.Flow
participant External as Juspay API
Client->>WalletIF: loyaltyInfo(LoyaltyCfg, LoyaltyInfoRequest)
WalletIF->>WalletJ: loyaltyInfo(LoyaltyCfg, request)
WalletJ->>WalletJ: decrypt apiKey, build Authorization header
WalletJ->>JuspayFlow: loyaltyInfo(baseUrl, authHeader, merchantId, tenantId, request)
JuspayFlow->>External: POST /loyalty/programs (headers + body)
External-->>JuspayFlow: LoyaltyInfoResponse
JuspayFlow-->>WalletJ: LoyaltyInfoResponse
WalletJ-->>WalletIF: LoyaltyInfoResponse
WalletIF-->>Client: LoyaltyInfoResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (3)
lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/CreateOrder.hs (1)
235-235: Identity mapping infieldLabelModifieris unnecessary but harmless.The mapping
"payment_rules" -> "payment_rules"is redundant since theother -> otherfallback handles unmapped fields. However, keeping it explicit documents the field name used in the API.🤖 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/Juspay/Types/CreateOrder.hs` at line 235, Remove the redundant explicit mapping "payment_rules" -> "payment_rules" from the fieldLabelModifier map in CreateOrder.hs; locate the Aeson options (the fieldLabelModifier function or the record-to-JSON mapping in the CreateOrder module) and delete that identity entry so the fallback clause (other -> other) handles it, leaving any other explicit mappings intact.lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Common.hs (1)
329-330: The emptyReadinstance is unusual but acceptable.The
Readinstance returning[](no valid parses) is a pragmatic workaround when aReadinstance is required by a constraint but the type is too complex to parse from string representation. Consider adding a brief comment explaining this design choice.🤖 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/Juspay/Types/Common.hs` around lines 329 - 330, Add a short comment above the Read instance for TxnDetail explaining why readsPrec always returns []: clarify that the instance is a deliberate pragmatic stub to satisfy typeclass constraints (instance Read TxnDetail with readsPrec _ _ = []) because TxnDetail is too complex or not intended to be parsed from strings, and note that parsing is unsupported and any attempt to read should fail; reference the instance declaration (instance Read TxnDetail) and the readsPrec implementation to locate where to add the comment.lib/mobility-core/src/Kernel/External/Payment/Interface/Juspay.hs (1)
734-735: Clarify the uncertainty comment.The
--Not Surecomment indicates uncertainty about whethertxnListshould beNothingfor the transaction-only webhook case. Consider investigating whetherjustTransactioncontains atxn_listfield that should be mapped, or document whyNothingis the correct value here.🤖 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/Juspay.hs` around lines 734 - 735, The comment warns that txnList is set to Nothing with uncertainty; inspect the justTransaction value (and any field named txn_list) and either map it into txnList or explicitly document why it must be Nothing for transaction-only webhooks. Specifically, check the structure parsed into justTransaction (look for a txn_list or similar field) and if present map it into the txnList field (e.g., txnList = Just <mappedTxnList>), otherwise replace the ambiguous comment with a clear comment explaining why txnList is intentionally Nothing for the transaction-only webhook case.
🤖 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.hs`:
- Around line 605-606: The PaytmEDC payment request construction includes an
unused field paymentRules (seen where CreateOrderReq is assembled with basket =
req.basket, paymentRules = req.paymentRules) but PaytmEDC only serializes a
PaytmEDCSaleRequestBody and never uses paymentRules; remove the paymentRules
assignment from the CreateOrderReq construction and any type/record fields
specific to PaytmEDC that reference paymentRules, update the
CreateOrderReq/PaytmEDC request type signatures and any callers to reflect the
removed field, and ensure PaytmEDCSaleRequestBody mapping continues to include
only the actual used fields (e.g., basket and other existing fields).
In `@lib/mobility-core/src/Kernel/External/Payment/PaytmEDC/Types.hs`:
- Around line 180-185: PaytmEDCResponseBody currently derives default
ToJSON/FromJSON (camelCase) while nested types LoyaltyInfo and TxnList use
jsonSnakeOptions (snake_case), causing deserialization mismatches; add explicit
instance declarations for FromJSON and ToJSON for PaytmEDCResponseBody using
jsonSnakeOptions (same options used by LoyaltyInfo and TxnList) so fields like
loyalty_info and txn_details parse correctly, or alternatively update
LoyaltyInfo/TxnList to match the parent — locate PaytmEDCResponseBody in
Types.hs and implement the jsonSnakeOptions-based instances to align names
across types.
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Juspay.hs`:
- Around line 734-735: The comment warns that txnList is set to Nothing with
uncertainty; inspect the justTransaction value (and any field named txn_list)
and either map it into txnList or explicitly document why it must be Nothing for
transaction-only webhooks. Specifically, check the structure parsed into
justTransaction (look for a txn_list or similar field) and if present map it
into the txnList field (e.g., txnList = Just <mappedTxnList>), otherwise replace
the ambiguous comment with a clear comment explaining why txnList is
intentionally Nothing for the transaction-only webhook case.
In `@lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Common.hs`:
- Around line 329-330: Add a short comment above the Read instance for TxnDetail
explaining why readsPrec always returns []: clarify that the instance is a
deliberate pragmatic stub to satisfy typeclass constraints (instance Read
TxnDetail with readsPrec _ _ = []) because TxnDetail is too complex or not
intended to be parsed from strings, and note that parsing is unsupported and any
attempt to read should fail; reference the instance declaration (instance Read
TxnDetail) and the readsPrec implementation to locate where to add the comment.
In `@lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/CreateOrder.hs`:
- Line 235: Remove the redundant explicit mapping "payment_rules" ->
"payment_rules" from the fieldLabelModifier map in CreateOrder.hs; locate the
Aeson options (the fieldLabelModifier function or the record-to-JSON mapping in
the CreateOrder module) and delete that identity entry so the fallback clause
(other -> other) handles it, leaving any other explicit mappings intact.
🪄 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: 6466675e-deef-4669-84ff-7c9b9d32136e
📒 Files selected for processing (7)
lib/mobility-core/src/Kernel/External/Payment/Interface.hslib/mobility-core/src/Kernel/External/Payment/Interface/Juspay.hslib/mobility-core/src/Kernel/External/Payment/Interface/PaytmEDC.hslib/mobility-core/src/Kernel/External/Payment/Interface/Types.hslib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Common.hslib/mobility-core/src/Kernel/External/Payment/Juspay/Types/CreateOrder.hslib/mobility-core/src/Kernel/External/Payment/PaytmEDC/Types.hs
| basket = req.basket, | ||
| paymentRules = req.paymentRules |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if PaytmEDC flow actually uses the paymentRules field
rg -n "paymentRules" lib/mobility-core/src/Kernel/External/Payment/PaytmEDC/
# Check the PaytmEDC sale request body structure
ast-grep --pattern 'data PaytmEDCSaleRequestBody = PaytmEDCSaleRequestBody { $$$ }'Repository: nammayatri/shared-kernel
Length of output: 1066
🏁 Script executed:
# Check the actual code at lines 605-606 in Interface.hs
cat -n lib/mobility-core/src/Kernel/External/Payment/Interface.hs | sed -n '595,620p'Repository: nammayatri/shared-kernel
Length of output: 1462
🏁 Script executed:
# Check what CreateOrderReq is being used for PaytmEDC
rg -n "CreateOrderReq" lib/mobility-core/src/Kernel/External/Payment/PaytmEDC/ -A 5
# Check if paymentRules is filtered or excluded for any provider
rg -n "paymentRules" lib/mobility-core/src/Kernel/External/Payment/ -B 3 -A 3Repository: nammayatri/shared-kernel
Length of output: 3967
🏁 Script executed:
# Get the PaytmEDC Types file to understand all available request types
cat -n lib/mobility-core/src/Kernel/External/Payment/PaytmEDC/Types.hsRepository: nammayatri/shared-kernel
Length of output: 15107
🏁 Script executed:
# Find PaytmEDCCreateOrderReq definition
rg -n "data PaytmEDCCreateOrderReq" lib/mobility-core/src/Kernel/External/Payment/ -A 20
# Check PaytmEDC implementation - how it converts the request
cat -n lib/mobility-core/src/Kernel/External/Payment/PaytmEDC/Impl.hs | head -100Repository: nammayatri/shared-kernel
Length of output: 162
🏁 Script executed:
# Check if there's any handling of paymentRules in PaytmEDC flow
rg -n "paymentRules" lib/mobility-core/src/Kernel/External/Payment/Interface.hs -B 10 -A 3 | head -60Repository: nammayatri/shared-kernel
Length of output: 1733
🏁 Script executed:
# Find PaytmEDCCreateOrderReq definition
rg -n "PaytmEDCCreateOrderReq" lib/mobility-core/src/Kernel/External/Payment/ -A 15
# Find PaytmEDC.createOrder implementation
rg -n "createOrder" lib/mobility-core/src/Kernel/External/Payment/PaytmEDC/ -A 10Repository: nammayatri/shared-kernel
Length of output: 50
🏁 Script executed:
# List all PaytmEDC files
find lib/mobility-core/src/Kernel/External/Payment/PaytmEDC/ -type f -name "*.hs"Repository: nammayatri/shared-kernel
Length of output: 326
🏁 Script executed:
# Find PaytmEDCCreateOrderReq - it might be in Interface/Types.hs
rg -n "PaytmEDCCreateOrderReq" lib/mobility-core/src/Kernel/External/Payment/Interface/Types.hs -A 20
# Check PaytmEDC Flow implementation
cat -n lib/mobility-core/src/Kernel/External/Payment/PaytmEDC/Flow.hsRepository: nammayatri/shared-kernel
Length of output: 4392
🏁 Script executed:
# Search for PaytmEDCCreateOrderReq in Interface types
rg -n "PaytmEDCCreateOrderReq" lib/mobility-core/src/Kernel/External/Payment/Interface/Types.hs -B 2 -A 20
# Also search where createOrder is called from PaytmEDC module
rg -n "PaytmEDC.createOrder" lib/mobility-core/src/Kernel/External/Payment/ -B 3 -A 10Repository: nammayatri/shared-kernel
Length of output: 2768
🏁 Script executed:
# Check the whole Interface.hs createPaymentOrder function around line 605
cat -n lib/mobility-core/src/Kernel/External/Payment/Interface.hs | sed -n '560,625p'Repository: nammayatri/shared-kernel
Length of output: 3282
🏁 Script executed:
# Find PaytmEDC module with createOrder function
find lib/mobility-core/src/Kernel/External/Payment -name "*.hs" -type f | xargs grep -l "createOrder.*PaytmEDC" | head -5
# Check PaytmEDC module exports
rg -n "module.*PaytmEDC" lib/mobility-core/src/Kernel/External/Payment/ -A 30Repository: nammayatri/shared-kernel
Length of output: 14348
🏁 Script executed:
# Search for PaytmEDC.createOrder definition
rg -n "createOrder.*::" lib/mobility-core/src/Kernel/External/Payment/PaytmEDC/ -A 15Repository: nammayatri/shared-kernel
Length of output: 50
🏁 Script executed:
# Get the full PaytmEDC createOrder implementation
cat -n lib/mobility-core/src/Kernel/External/Payment/Interface/PaytmEDC.hsRepository: nammayatri/shared-kernel
Length of output: 15638
Remove unused paymentRules field from PaytmEDC payment request.
The paymentRules field is passed to PaytmEDC's CreateOrderReq but is never used in the implementation. The PaytmEDC flow only extracts and sends a subset of fields to the actual API request (PaytmEDCSaleRequestBody), excluding paymentRules entirely. Unlike Juspay which actively converts this field, PaytmEDC silently ignores it. Remove it to avoid confusion and keep the request object focused on fields that are actually used.
🤖 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.hs` around lines 605
- 606, The PaytmEDC payment request construction includes an unused field
paymentRules (seen where CreateOrderReq is assembled with basket = req.basket,
paymentRules = req.paymentRules) but PaytmEDC only serializes a
PaytmEDCSaleRequestBody and never uses paymentRules; remove the paymentRules
assignment from the CreateOrderReq construction and any type/record fields
specific to PaytmEDC that reference paymentRules, update the
CreateOrderReq/PaytmEDC request type signatures and any callers to reflect the
removed field, and ensure PaytmEDCSaleRequestBody mapping continues to include
only the actual used fields (e.g., basket and other existing fields).
| paymentMode :: Maybe Text, | ||
| loyaltyInfo :: Maybe LoyaltyInfo, | ||
| txnDetails :: Maybe [TxnList] | ||
| } | ||
| deriving stock (Show, Eq, Generic) | ||
| deriving anyclass (ToJSON, FromJSON) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how PaytmEDCResponseBody derives JSON instances
rg -A5 "deriving anyclass.*FromJSON" lib/mobility-core/src/Kernel/External/Payment/PaytmEDC/Types.hs
# Check how LoyaltyInfo and TxnList derive JSON instances
rg -B5 -A10 "instance FromJSON LoyaltyInfo" lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Common.hs
rg -B5 -A10 "instance FromJSON TxnList" lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Common.hsRepository: nammayatri/shared-kernel
Length of output: 2858
Fix JSON serialization mismatch between parent and nested types.
PaytmEDCResponseBody uses default ToJSON/FromJSON derivation (camelCase), but the nested LoyaltyInfo and TxnList types have explicit instances using jsonSnakeOptions (snake_case). When PaytmEDCResponseBody deserializes JSON with fields loyalty_info and txn_details, parsing will fail because the parent expects camelCase field names.
Add explicit FromJSON/ToJSON instances to PaytmEDCResponseBody using jsonSnakeOptions to match the nested types, or ensure the API response format aligns with the expected field naming convention.
🤖 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/PaytmEDC/Types.hs` around lines
180 - 185, PaytmEDCResponseBody currently derives default ToJSON/FromJSON
(camelCase) while nested types LoyaltyInfo and TxnList use jsonSnakeOptions
(snake_case), causing deserialization mismatches; add explicit instance
declarations for FromJSON and ToJSON for PaytmEDCResponseBody using
jsonSnakeOptions (same options used by LoyaltyInfo and TxnList) so fields like
loyalty_info and txn_details parse correctly, or alternatively update
LoyaltyInfo/TxnList to match the parent — locate PaytmEDCResponseBody in
Types.hs and implement the jsonSnakeOptions-based instances to align names
across types.
b5f9eb5 to
68ae95b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/mobility-core/src/Kernel/External/Wallet/Interface.hs (1)
20-27: 🛠️ Refactor suggestion | 🟠 MajorAdd the repo-preferred Prelude import explicitly.
This module is relying on transitive imports instead of bringing in
Kernel.Prelude/EulerHS.Preludedirectly, which is out of line with the repository standard forNoImplicitPreludemodules.As per coding guidelines,
All modules must import Kernel.Prelude or EulerHS.Prelude instead of the standard Prelude due to NoImplicitPrelude language pragma.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mobility-core/src/Kernel/External/Wallet/Interface.hs` around lines 20 - 27, This module is using transitive Prelude symbols instead of importing the repo-preferred prelude; update Kernel/External/Wallet/Interface.hs to explicitly import the project prelude (e.g. add import Kernel.Prelude or import EulerHS.Prelude at the top of the module) rather than relying on transitive imports, and ensure functions/types used in this file (referenced in this module like Payment, Juspay, WalletConfig, CoreMetrics, GenericError/InvalidRequest) continue to resolve—remove any accidental dependency on standard Prelude symbols and keep existing qualified imports as-is.
🧹 Nitpick comments (6)
lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Common.hs (2)
471-478: Remove commented-out code after migration is complete.The old
TxnDetaildefinition is commented out. Consider removing it once you've verified all downstream usages work correctly with the new expandedTxnDetailtype.🤖 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/Juspay/Types/Common.hs` around lines 471 - 478, Remove the commented-out legacy TxnDetail block from Common.hs now that the migration to the new expanded TxnDetail is complete; locate the commented section containing the old TxnDetail definition (the block starting with "data TxnDetail = TxnDetail" and ending with the deriving lines) and delete it so the file contains only the current TxnDetail type and its instances, ensuring no commented legacy definitions remain.
329-330: StubReadinstance always fails - verify this is intentional.The
Read TxnDetailinstance returns[]for any input, meaningreadwill always fail. This appears to be a workaround becauseTxnListderivesReadand containsMaybe TxnDetail. While acceptable forMaybefields (they'll default toNothingon parse failure in some contexts), consider documenting this behavior or adding a comment explaining why a fullReadinstance isn't provided.📝 Suggested documentation
instance Read TxnDetail where + -- Stub instance: TxnDetail has too many fields for meaningful string parsing. + -- Required because TxnList derives Read. readsPrec _ _ = []🤖 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/Juspay/Types/Common.hs` around lines 329 - 330, The Read instance for TxnDetail currently always fails (readsPrec _ _ = []) which makes read on TxnDetail unusable; either document this intentional stub or implement a proper parser—locate the instance declaration "instance Read TxnDetail" in Common.hs and either add a comment explaining that TxnDetail is intentionally non-parsable (and why, e.g. relying on TxnList's Maybe semantics) or replace the stub with a correct readsPrec implementation that parses the textual representation of TxnDetail consistent with its Show/derived form so that read and readsPrec behave correctly.lib/mobility-core/src/Kernel/External/Payment/Interface/Types.hs (1)
355-362: Remove commented-outTxnDetaildefinition.Similar to
Juspay/Types/Common.hs, this commented-out localTxnDetaildefinition should be removed once migration is verified. The type is now correctly imported fromJuspay.Types.🤖 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 355 - 362, Remove the dead/commented TxnDetail declaration block: delete the commented-out TxnDetail definition (the block starting with "data TxnDetail = TxnDetail" and its deriving clauses) in Types.hs since TxnDetail is now imported from Juspay.Types; ensure no other local references remain and rely on the existing import of Juspay.Types for the TxnDetail type.lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs (1)
136-143: Model the three headers as required in the API type.The implementation treats all three headers as mandatory, but plain
Headerkeeps them optional in the generated client and forces the extraJustplumbing. Servant supportsHeader' '[Required, Strict]for this exact case, which would make the type-level contract match the endpoint contract. (hackage.haskell.org)🔧 Suggested tightening
type LoyaltyInfoAPI = "loyalty" :> "programs" - :> Header "Authorization" Text - :> Header "X-MerchantId" Text - :> Header "x-tenant-id" Text + :> Header' '[Required, Strict] "Authorization" Text + :> Header' '[Required, Strict] "X-MerchantId" Text + :> Header' '[Required, Strict] "x-tenant-id" Text :> ReqBody '[JSON] LoyaltyInfoRequest :> Post '[JSON] LoyaltyInfoResponse @@ loyaltyInfo url authHeader merchantId tenantId req = do let proxy = Proxy `@LoyaltyInfoAPI` - eulerClient = Euler.client proxy (Just authHeader) (Just merchantId) (Just tenantId) req + eulerClient = Euler.client proxy authHeader merchantId tenantId reqAlso applies to: 155-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs` around lines 136 - 143, The API type LoyaltyInfoAPI currently uses plain Header which makes "Authorization", "X-MerchantId" and "x-tenant-id" optional; change each Header to Header' '[Required, Strict] to mark them required and strict so the generated client and type-level contract match the implementation; update the same pattern for the other occurrence mentioned (the similar API type at the second location) ensuring the three header symbols are replaced with Header' '[Required, Strict] for Authorization, X-MerchantId and x-tenant-id.lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs (1)
145-146: Omit absent request fields from the loyalty payload.
jsonSnakeOptionsonly changesfieldLabelModifier, so it still inherits Aeson's defaultomitNothingFields = False. That meansorder = Nothingwill be sent as"order": nullinstead of being omitted, which is an avoidable interoperability risk for an outbound API request. (hackage.haskell.org)💡 Minimal fix
+requestJsonOptions :: Options +requestJsonOptions = jsonSnakeOptions {omitNothingFields = True} + instance ToJSON LoyaltyInfoRequest where - toJSON = genericToJSON jsonSnakeOptions + toJSON = genericToJSON requestJsonOptions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs` around lines 145 - 146, The ToJSON instance for LoyaltyInfoRequest uses genericToJSON jsonSnakeOptions which doesn't set omitNothingFields, so Nothing fields are encoded as null; update the ToJSON instance to use an Aeson Options value derived from jsonSnakeOptions with omitNothingFields = True (either by creating a new options value like jsonSnakeOmitNothing = jsonSnakeOptions { omitNothingFields = True } or by passing a modified record inline) and call genericToJSON with that options value so absent request fields are omitted from the payload; ensure the change targets the LoyaltyInfoRequest ToJSON instance and reuses jsonSnakeOptions as the base.lib/mobility-core/src/Kernel/External/Wallet/Interface.hs (1)
93-101: This top-level interface now leaks the Juspay-specific config type.Every other wallet entry point in this module dispatches on
PaymentServiceConfig, butloyaltyInfonow forces callers to know aboutKernel.External.Wallet.Juspay.Config.LoyaltyCfg. I'd either route this through a common config sum as well, or keep it in the Juspay-specific interface until there is a provider-neutral abstraction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mobility-core/src/Kernel/External/Wallet/Interface.hs` around lines 93 - 101, The top-level loyaltyInfo function leaks the Juspay-specific WalletConfig.LoyaltyCfg; change its API to accept the provider-agnostic PaymentServiceConfig (the same sum used by other wallet entry points) and dispatch to Juspay.loyaltyInfo only when the PaymentServiceConfig matches the Juspay variant, or alternatively move this loyaltyInfo implementation into the Juspay-specific module and keep the top-level signature provider-neutral; update the loyaltyInfo type signature and body to pattern-match on PaymentServiceConfig and call Juspay.loyaltyInfo with the extracted LoyaltyCfg when appropriate.
🤖 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/Wallet/Interface/Types.hs`:
- Around line 30-31: The OpenAPI ToSchema instances are not using the same Aeson
field options, causing schema names to differ from serialized JSON; update each
ToSchema derivation to use openapi3's fromAesonOptions by converting your
existing Aeson Options (e.g., jsonSnakeOptions, walletProgramOptions,
burnOptionOptions, and any other custom Options defined in this file) into
SchemaOptions and pass those to the Generic-based ToSchema derivation (use
fromAesonOptions jsonSnakeOptions etc.) so generated schemas reflect the Aeson
fieldLabelModifier mappings (apply this change for the types referenced around
jsonSnakeOptions and at the other listed sections).
In `@lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs`:
- Around line 153-157: The current error handling in loyaltyInfo embeds the full
Servant error via show err which can leak headers/auth; change the lambda passed
to fromEitherM to pattern-match on Servant's ClientError (e.g. FailureResponse,
ConnectionError, DecodeFailure, InvalidContentTypeHeader,
WrongRequestBodyLength) and construct an InternalError that only includes safe
fields (for FailureResponse use responseStatusCode and a short/truncated
responseBody or a fixed message like "response body omitted"; for
ConnectionError include a sanitized exception message without request/headers).
Update the mapping so callAPI ... >>= fromEitherM (\err -> ...) builds this
redacted message (referencing ClientError, FailureResponse, responseStatusCode,
responseBody, loyaltyInfo, callAPI, fromEitherM, InternalError) instead of using
show err.
---
Outside diff comments:
In `@lib/mobility-core/src/Kernel/External/Wallet/Interface.hs`:
- Around line 20-27: This module is using transitive Prelude symbols instead of
importing the repo-preferred prelude; update Kernel/External/Wallet/Interface.hs
to explicitly import the project prelude (e.g. add import Kernel.Prelude or
import EulerHS.Prelude at the top of the module) rather than relying on
transitive imports, and ensure functions/types used in this file (referenced in
this module like Payment, Juspay, WalletConfig, CoreMetrics,
GenericError/InvalidRequest) continue to resolve—remove any accidental
dependency on standard Prelude symbols and keep existing qualified imports
as-is.
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Types.hs`:
- Around line 355-362: Remove the dead/commented TxnDetail declaration block:
delete the commented-out TxnDetail definition (the block starting with "data
TxnDetail = TxnDetail" and its deriving clauses) in Types.hs since TxnDetail is
now imported from Juspay.Types; ensure no other local references remain and rely
on the existing import of Juspay.Types for the TxnDetail type.
In `@lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Common.hs`:
- Around line 471-478: Remove the commented-out legacy TxnDetail block from
Common.hs now that the migration to the new expanded TxnDetail is complete;
locate the commented section containing the old TxnDetail definition (the block
starting with "data TxnDetail = TxnDetail" and ending with the deriving lines)
and delete it so the file contains only the current TxnDetail type and its
instances, ensuring no commented legacy definitions remain.
- Around line 329-330: The Read instance for TxnDetail currently always fails
(readsPrec _ _ = []) which makes read on TxnDetail unusable; either document
this intentional stub or implement a proper parser—locate the instance
declaration "instance Read TxnDetail" in Common.hs and either add a comment
explaining that TxnDetail is intentionally non-parsable (and why, e.g. relying
on TxnList's Maybe semantics) or replace the stub with a correct readsPrec
implementation that parses the textual representation of TxnDetail consistent
with its Show/derived form so that read and readsPrec behave correctly.
In `@lib/mobility-core/src/Kernel/External/Wallet/Interface.hs`:
- Around line 93-101: The top-level loyaltyInfo function leaks the
Juspay-specific WalletConfig.LoyaltyCfg; change its API to accept the
provider-agnostic PaymentServiceConfig (the same sum used by other wallet entry
points) and dispatch to Juspay.loyaltyInfo only when the PaymentServiceConfig
matches the Juspay variant, or alternatively move this loyaltyInfo
implementation into the Juspay-specific module and keep the top-level signature
provider-neutral; update the loyaltyInfo type signature and body to
pattern-match on PaymentServiceConfig and call Juspay.loyaltyInfo with the
extracted LoyaltyCfg when appropriate.
In `@lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs`:
- Around line 145-146: The ToJSON instance for LoyaltyInfoRequest uses
genericToJSON jsonSnakeOptions which doesn't set omitNothingFields, so Nothing
fields are encoded as null; update the ToJSON instance to use an Aeson Options
value derived from jsonSnakeOptions with omitNothingFields = True (either by
creating a new options value like jsonSnakeOmitNothing = jsonSnakeOptions {
omitNothingFields = True } or by passing a modified record inline) and call
genericToJSON with that options value so absent request fields are omitted from
the payload; ensure the change targets the LoyaltyInfoRequest ToJSON instance
and reuses jsonSnakeOptions as the base.
In `@lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs`:
- Around line 136-143: The API type LoyaltyInfoAPI currently uses plain Header
which makes "Authorization", "X-MerchantId" and "x-tenant-id" optional; change
each Header to Header' '[Required, Strict] to mark them required and strict so
the generated client and type-level contract match the implementation; update
the same pattern for the other occurrence mentioned (the similar API type at the
second location) ensuring the three header symbols are replaced with Header'
'[Required, Strict] for Authorization, X-MerchantId and x-tenant-id.
🪄 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: 085e20d6-84be-4a75-ba39-6fdb090e9c9c
📒 Files selected for processing (12)
lib/mobility-core/src/Kernel/External/Payment/Interface.hslib/mobility-core/src/Kernel/External/Payment/Interface/Juspay.hslib/mobility-core/src/Kernel/External/Payment/Interface/PaytmEDC.hslib/mobility-core/src/Kernel/External/Payment/Interface/Types.hslib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Common.hslib/mobility-core/src/Kernel/External/Payment/Juspay/Types/CreateOrder.hslib/mobility-core/src/Kernel/External/Payment/PaytmEDC/Types.hslib/mobility-core/src/Kernel/External/Wallet/Interface.hslib/mobility-core/src/Kernel/External/Wallet/Interface/Juspay.hslib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hslib/mobility-core/src/Kernel/External/Wallet/Juspay/Config.hslib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs
✅ Files skipped from review due to trivial changes (3)
- lib/mobility-core/src/Kernel/External/Payment/Interface/PaytmEDC.hs
- lib/mobility-core/src/Kernel/External/Payment/Interface.hs
- lib/mobility-core/src/Kernel/External/Wallet/Juspay/Config.hs
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/mobility-core/src/Kernel/External/Payment/Interface/Juspay.hs
| jsonSnakeOptions :: Options | ||
| jsonSnakeOptions = defaultOptions {fieldLabelModifier = camelToSnakeCase} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine its contents
wc -l "lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs"Repository: nammayatri/shared-kernel
Length of output: 135
🏁 Script executed:
# Read the file to understand its structure, focusing on the import statements first
head -40 "lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs"Repository: nammayatri/shared-kernel
Length of output: 1517
🏁 Script executed:
# Check the jsonSnakeOptions definition around lines 30-31
sed -n '25,35p' "lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs"Repository: nammayatri/shared-kernel
Length of output: 415
🏁 Script executed:
# Look for ToSchema instances in the file
rg "ToSchema|SchemaOptions|fromAesonOptions" "lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs" -nRepository: nammayatri/shared-kernel
Length of output: 1894
🏁 Script executed:
# Check the custom Aeson options usage and field mappings
sed -n '135,160p' "lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs"Repository: nammayatri/shared-kernel
Length of output: 815
🏁 Script executed:
# Check for any SchemaOptions or fromAesonOptions usage
rg "SchemaOptions|schemaOptions|fromAesonOptions" "lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs" -nRepository: nammayatri/shared-kernel
Length of output: 50
🏁 Script executed:
# Look at more custom FromJSON/ToJSON instances around the mentioned lines
sed -n '140,160p' "lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs"Repository: nammayatri/shared-kernel
Length of output: 664
🏁 Script executed:
# Check around lines 187-227
sed -n '187,227p' "lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs"Repository: nammayatri/shared-kernel
Length of output: 1200
🏁 Script executed:
# Check around lines 320-345
sed -n '320,345p' "lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs"Repository: nammayatri/shared-kernel
Length of output: 745
🏁 Script executed:
# Check if openapi3 is imported or used in any way
rg "openapi3|OpenAPI|SchemaOptions" "lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs" -nRepository: nammayatri/shared-kernel
Length of output: 50
Keep ToSchema aligned with custom Aeson field mappings.
Types throughout this file use custom Aeson options (jsonSnakeOptions, walletProgramOptions, burnOptionOptions, etc.) to remap JSON field names (e.g., customerId → id, id_ → id, type_ → type, camelCase → snake_case), but the generic ToSchema derivations do not apply these same options. The OpenAPI schema will describe Haskell field names instead of the actual JSON keys, creating a mismatch. Use openapi3's fromAesonOptions to wire your Aeson options into SchemaOptions for ToSchema derivation.
Applies to: lines 30–31, 140–158, 187–227, 320–345, 398–417, 445–479
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/mobility-core/src/Kernel/External/Wallet/Interface/Types.hs` around lines
30 - 31, The OpenAPI ToSchema instances are not using the same Aeson field
options, causing schema names to differ from serialized JSON; update each
ToSchema derivation to use openapi3's fromAesonOptions by converting your
existing Aeson Options (e.g., jsonSnakeOptions, walletProgramOptions,
burnOptionOptions, and any other custom Options defined in this file) into
SchemaOptions and pass those to the Generic-based ToSchema derivation (use
fromAesonOptions jsonSnakeOptions etc.) so generated schemas reflect the Aeson
fieldLabelModifier mappings (apply this change for the types referenced around
jsonSnakeOptions and at the other listed sections).
| loyaltyInfo url authHeader merchantId tenantId req = do | ||
| let proxy = Proxy @LoyaltyInfoAPI | ||
| eulerClient = Euler.client proxy (Just authHeader) (Just merchantId) (Just tenantId) req | ||
| callAPI url eulerClient "loyalty-info" proxy | ||
| >>= fromEitherM (\err -> InternalError $ "Failed to call loyalty info API: " <> show err) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant section
if [ -f "lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs" ]; then
echo "=== File exists, reading lines 150-160 ==="
sed -n '150,160p' "lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs" | cat -n
else
echo "File not found, listing directory:"
ls -la lib/mobility-core/src/Kernel/External/Wallet/Juspay/ 2>/dev/null || echo "Directory not found"
fiRepository: nammayatri/shared-kernel
Length of output: 552
🏁 Script executed:
# Search for similar patterns in the codebase with fromEitherM and show err
rg "fromEitherM.*show err" --type hs -B 2 -A 2Repository: nammayatri/shared-kernel
Length of output: 15297
🏁 Script executed:
# Check the imports in the Flow.hs file to understand what's being used
head -30 "lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs" | cat -nRepository: nammayatri/shared-kernel
Length of output: 1449
🏁 Script executed:
# Look for how ClientError is being imported and used
rg "ClientError" "lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs" -B 2 -A 2Repository: nammayatri/shared-kernel
Length of output: 50
🏁 Script executed:
# Find callAPI definition
rg "callAPI ::|callAPI\s*::" --type hs -A 5Repository: nammayatri/shared-kernel
Length of output: 993
🏁 Script executed:
# Find LoyaltyInfoAPI type definition
sed -n '136,160p' "lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs" | cat -nRepository: nammayatri/shared-kernel
Length of output: 967
🏁 Script executed:
# Search for where authHeader is created or used in loyaltyInfo context
rg "loyaltyInfo|authHeader" "lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs" -B 3 -A 3Repository: nammayatri/shared-kernel
Length of output: 652
🏁 Script executed:
# Check the types used for auth in Juspay APIs
rg "mkBasicAuthData|BasicAuthData" "lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs" -B 2 -A 2Repository: nammayatri/shared-kernel
Length of output: 50
🏁 Script executed:
# Check where loyaltyInfo is called to understand how authHeader is prepared
rg "loyaltyInfo\s*\(" --type hs -B 2 -A 2Repository: nammayatri/shared-kernel
Length of output: 50
🏁 Script executed:
# Look for any sanitization or redaction of auth headers
rg "show|Show.*ClientError|redact|sanitize" "lib/mobility-core/src/Kernel/Utils/Servant/Client.hs" -B 2 -A 2 | head -40Repository: nammayatri/shared-kernel
Length of output: 1755
🏁 Script executed:
# Check Servant version in cabal or other dependency files
fd -e cabal -e yaml -o lock | xargs grep -l "servant-client" 2>/dev/null | head -5Repository: nammayatri/shared-kernel
Length of output: 195
🏁 Script executed:
# Look at the callAPI' implementation to understand what error type is returned
sed -n '1,100p' "lib/mobility-core/src/Kernel/Utils/Servant/Client.hs" | cat -nRepository: nammayatri/shared-kernel
Length of output: 4316
🏁 Script executed:
# Search for how loyaltyInfo is called - where does authHeader come from?
rg "loyaltyInfo.*=" --type hs -B 5 -A 2Repository: nammayatri/shared-kernel
Length of output: 5655
🏁 Script executed:
# Check if there's any credential/key handling in the Wallet module
rg "decrypt|BasicAuth|Authorization" "lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs" -B 2 -A 2Repository: nammayatri/shared-kernel
Length of output: 199
🏁 Script executed:
# Look at Servant ClientError Show instance behavior - check what it includes
cat "lib/mobility-core/src/Kernel/Utils/Servant/Client.hs" | sed -n '1,60p'Repository: nammayatri/shared-kernel
Length of output: 2428
🏁 Script executed:
# Search for the actual call site of loyaltyInfo to understand authHeader's origin
rg "loyaltyInfo" --type hs --type hsc -B 3 -A 3 | head -50Repository: nammayatri/shared-kernel
Length of output: 95
🏁 Script executed:
# Check if there's any redaction/sanitization of error logging for ClientError
rg "logError|logWarning" "lib/mobility-core/src/Kernel/Utils/Servant/Client.hs" -B 3 -A 3 | head -30Repository: nammayatri/shared-kernel
Length of output: 1204
🏁 Script executed:
# Look at the full loyaltyInfo setup in Juspay.hs to confirm authHeader is the decrypted credential
cat "lib/mobility-core/src/Kernel/External/Wallet/Interface/Juspay.hs" | sed -n '1,60p'Repository: nammayatri/shared-kernel
Length of output: 2135
🏁 Script executed:
# Check if there are any header redaction utilities in the codebase
rg "sanitiz|redact|scrub.*[Hh]eader" --type hsRepository: nammayatri/shared-kernel
Length of output: 1412
🏁 Script executed:
# Find the exact lines where authHeader is constructed in Juspay.hs
rg "authHeader.*Basic|Basic.*apiKey" "lib/mobility-core/src/Kernel/External/Wallet/Interface/Juspay.hs" -B 2 -A 2Repository: nammayatri/shared-kernel
Length of output: 304
🏁 Script executed:
# Find the cabal file or dependency declaration to see Servant version
find . -name "*.cabal" -o -name "cabal.project" | head -5Repository: nammayatri/shared-kernel
Length of output: 216
🏁 Script executed:
# Check what version of servant-client is being used
rg "servant-client" "cabal.project" -A 1 -B 1 2>/dev/null | head -20Repository: nammayatri/shared-kernel
Length of output: 50
🏁 Script executed:
# Look for any existing mitigation or header filtering in the error handling
rg "ClientError" "lib/mobility-core/src/Kernel/Utils/Servant/Client.hs" -B 5 -A 5 | head -60Repository: nammayatri/shared-kernel
Length of output: 801
🏁 Script executed:
# Search for imports of ClientError to understand where it comes from
rg "import.*ClientError|from.*ClientError" "lib/mobility-core/src/Kernel" --type hsRepository: nammayatri/shared-kernel
Length of output: 2067
🏁 Script executed:
# Let's examine the exact Servant ClientError definition to confirm it includes request headers
grep -r "data ClientError" --type hs -A 10Repository: nammayatri/shared-kernel
Length of output: 185
🏁 Script executed:
# Check if there's any existing error filtering/redaction for secrets in fromEitherM
rg "fromEitherM" "lib/mobility-core/src/Kernel/Utils/Common.hs" -B 3 -A 5Repository: nammayatri/shared-kernel
Length of output: 50
🏁 Script executed:
# Verify the Servant version in the dependencies
cat "lib/mobility-core/mobility-core.cabal" | grep -i "servant" | head -10Repository: nammayatri/shared-kernel
Length of output: 414
🏁 Script executed:
# Get the full dependency list from cabal file
cat "lib/mobility-core/mobility-core.cabal" | grep -A 200 "build-depends:" | grep -i "servant"Repository: nammayatri/shared-kernel
Length of output: 443
🏁 Script executed:
# Check what ClientError contains by looking at imports and uses
rg "FailureResponse|ConnectionError|DecodeFailure" "lib/mobility-core/src/Kernel/Utils/Servant/Client.hs" -B 2 -A 2Repository: nammayatri/shared-kernel
Length of output: 1004
🏁 Script executed:
# Look at how ClientError is pattern matched to understand its structure
rg "ClientError\s*\(" --type hs -A 2 | head -30Repository: nammayatri/shared-kernel
Length of output: 1561
🏁 Script executed:
# Check if the issue mentions redacting errors elsewhere - look at error handling in callAPI
sed -n '140,180p' "lib/mobility-core/src/Kernel/Utils/Servant/Client.hs"Repository: nammayatri/shared-kernel
Length of output: 1565
Redact the Servant error before embedding it in InternalError.
When callAPI returns a ClientError from a failed request, its Show instance includes the full HTTP request including headers. Since authHeader carries the decrypted API key as a Base64-encoded Basic authentication credential, calling show err will serialize this secret into the error message and expose it via logError logging and telemetry systems.
Instead of show err, extract only the status code and safe error details. Consider using Servant's FailureResponse pattern to access the status selectively without serializing headers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/mobility-core/src/Kernel/External/Wallet/Juspay/Flow.hs` around lines 153
- 157, The current error handling in loyaltyInfo embeds the full Servant error
via show err which can leak headers/auth; change the lambda passed to
fromEitherM to pattern-match on Servant's ClientError (e.g. FailureResponse,
ConnectionError, DecodeFailure, InvalidContentTypeHeader,
WrongRequestBodyLength) and construct an InternalError that only includes safe
fields (for FailureResponse use responseStatusCode and a short/truncated
responseBody or a fixed message like "response body omitted"; for
ConnectionError include a sanitized exception message without request/headers).
Update the mapping so callAPI ... >>= fromEitherM (\err -> ...) builds this
redacted message (referencing ClientError, FailureResponse, responseStatusCode,
responseBody, loyaltyInfo, callAPI, fromEitherM, InternalError) instead of using
show err.
1 participant
Type of Change
Description
Additional Changes
Motivation and Context
How did you test it?
Checklist
./dev/format-all-files.shSummary by CodeRabbit