backend/enhancement/added-mandate-status-check-support#1235
Open
dushyant-2002 wants to merge 1 commit into
Open
backend/enhancement/added-mandate-status-check-support#1235dushyant-2002 wants to merge 1 commit into
dushyant-2002 wants to merge 1 commit into
Conversation
WalkthroughThis pull request adds mandate status lookup functionality for the Juspay payment provider. It introduces new request/response types, integrates a Juspay API endpoint, and provides router logic that delegates mandate status queries to Juspay while rejecting the operation for other providers. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Interface as Interface.getMandateStatus
participant Juspay as Juspay.getMandateStatus
participant Flow as Juspay.Flow.mandateStatus
participant JuspayAPI as Juspay API
Client->>Interface: getMandateStatus(config, routingId, req)
Interface->>Interface: Pattern match on config
alt JuspayConfig
Interface->>Juspay: getMandateStatus(config, routingId, req)
Juspay->>Juspay: Decrypt API key
Juspay->>Flow: mandateStatus(baseUrl, apiKey, merchantId, routingId, mandateId)
Flow->>Flow: Create BasicAuthData
Flow->>JuspayAPI: POST /mandate/{mandateId}
JuspayAPI-->>Flow: JuspayMandateStatusResp
Flow-->>Juspay: Response
Juspay->>Juspay: Parse & map to OrderStatusResp
Juspay-->>Interface: OrderStatusResp
else StripeConfig or PaytmEDCConfig
Interface-->>Client: InternalError (not supported)
end
Interface-->>Client: OrderStatusResp
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
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/Flow.hs (1)
486-494: Nit: reusemkBasicAuthDatahelper.This function re-implements the basic auth construction inline (lines 488-492). The module already defines
mkBasicAuthDataat line 277 which several other endpoints in this file use (e.g.,createOrder,verifyVPA). Using the helper here would be consistent with newer wrappers and drop 5 lines. Not blocking — many existingmandate*functions in this file also inline it.♻️ Proposed change
mandateStatus url apiKey merchantId mRoutingId mandateId req = do let eulerClient = Euler.client (Proxy `@MandateStatusAPI`) - let basicAuthData = - BasicAuthData - { basicAuthUsername = DT.encodeUtf8 apiKey, - basicAuthPassword = "" - } - callAPI url (eulerClient (Just merchantId) mRoutingId basicAuthData mandateId req) "mandate-status" (Proxy `@MandateStatusAPI`) + callAPI url (eulerClient (Just merchantId) mRoutingId (mkBasicAuthData apiKey) mandateId req) "mandate-status" (Proxy `@MandateStatusAPI`) >>= fromEitherM (\err -> InternalError $ "Failed to call mandate status API: " <> show err)🤖 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/Flow.hs` around lines 486 - 494, Replace the inline BasicAuthData construction in mandateStatus with the existing mkBasicAuthData helper: in the mandateStatus function, remove the manual BasicAuthData block and call mkBasicAuthData apiKey (or the appropriate signature) to produce basicAuthData, then pass that into the existing callAPI invocation (keeping the Euler.client (Proxy `@MandateStatusAPI`), merchantId, mRoutingId, mandateId and req parameters intact); this reuses the shared helper used by createOrder and verifyVPA and keeps behavior identical.lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Mandate.hs (1)
154-157: Prefernewtypefor single-field record.
MandateStatusReqhas a singlecommandfield; anewtypeis more efficient and consistent withMandateRevokeReqon line 124.♻️ Proposed change
-data MandateStatusReq = MandateStatusReq - { command :: Text - } - deriving (Eq, Show, Generic, ToJSON, FromJSON, ToSchema, ToForm) +newtype MandateStatusReq = MandateStatusReq + { command :: Text + } + deriving (Eq, Show, Generic, ToJSON, FromJSON, ToSchema, ToForm)🤖 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/Mandate.hs` around lines 154 - 157, MandateStatusReq is defined as a single-field record; replace the data declaration with a newtype to match MandateRevokeReq and gain efficiency. Change the declaration of MandateStatusReq (and keep the field name command for external compatibility) from a data record to a newtype and retain the same deriving list (Eq, Show, Generic, ToJSON, FromJSON, ToSchema, ToForm), adding any necessary LANGUAGE pragmas (e.g., GeneralizedNewtypeDeriving) if required by the derived classes.lib/mobility-core/src/Kernel/External/Payment/Interface/Juspay.hs (1)
602-602: Hard-coded"check_status"command — consider a named constant.The command literal
"check_status"is embedded at the interface layer. Other mandate operations in this file use the same pattern (e.g.,"revoke","pause","resume"), so this is consistent, but if Juspay introduces additional commands for the status endpoint, a small constant at the top of the module would ease maintenance.🤖 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` at line 602, Extract the literal "check_status" into a named top-level constant (e.g., mandateStatusCmd or cmdCheckStatus) in this module and replace the inline string in the Juspay.mandateStatus call (and any other mandate command literals like "revoke", "pause", "resume" if present) with that constant; update the Juspay.mandateStatus invocation (the call site referencing juspayResp and Juspay.MandateStatusReq) to use the new constant so future command additions are centralized and easier to maintain.
🤖 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/Juspay.hs`:
- Around line 613-622: The code currently coerces
JuspayMandateStatusResp.order_id to an empty string when building
MandateStatusResp.orderShortId (fromMaybe "" order_id); instead change
MandateStatusResp.orderShortId to a Maybe Text and propagate the original
order_id directly (i.e., assign orderShortId = order_id) so absence is
preserved; update the data type declaration for MandateStatusResp and any places
that construct or consume orderShortId (search for MandateStatusResp,
orderShortId usage) to handle Maybe Text accordingly, and add minimal handling
where a non-empty identifier is required (e.g., explicit error or logging)
rather than silently using "".
In `@lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Mandate.hs`:
- Around line 159-169: The JuspayMandateStatusResp type declares max_amount ::
Double which causes precision loss; change the field to max_amount ::
HighPrecMoney and update its JSON instances if necessary so it deserializes as
HighPrecMoney; then remove the compensating realToFrac conversion in the
downstream mapper (the code that assigns justMandate.max_amount to
mandateMaxAmount in Interface/Juspay.hs and the mapping around line 620) so the
value flows as HighPrecMoney end-to-end.
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Payment/Interface/Juspay.hs`:
- Line 602: Extract the literal "check_status" into a named top-level constant
(e.g., mandateStatusCmd or cmdCheckStatus) in this module and replace the inline
string in the Juspay.mandateStatus call (and any other mandate command literals
like "revoke", "pause", "resume" if present) with that constant; update the
Juspay.mandateStatus invocation (the call site referencing juspayResp and
Juspay.MandateStatusReq) to use the new constant so future command additions are
centralized and easier to maintain.
In `@lib/mobility-core/src/Kernel/External/Payment/Juspay/Flow.hs`:
- Around line 486-494: Replace the inline BasicAuthData construction in
mandateStatus with the existing mkBasicAuthData helper: in the mandateStatus
function, remove the manual BasicAuthData block and call mkBasicAuthData apiKey
(or the appropriate signature) to produce basicAuthData, then pass that into the
existing callAPI invocation (keeping the Euler.client (Proxy `@MandateStatusAPI`),
merchantId, mRoutingId, mandateId and req parameters intact); this reuses the
shared helper used by createOrder and verifyVPA and keeps behavior identical.
In `@lib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Mandate.hs`:
- Around line 154-157: MandateStatusReq is defined as a single-field record;
replace the data declaration with a newtype to match MandateRevokeReq and gain
efficiency. Change the declaration of MandateStatusReq (and keep the field name
command for external compatibility) from a data record to a newtype and retain
the same deriving list (Eq, Show, Generic, ToJSON, FromJSON, ToSchema, ToForm),
adding any necessary LANGUAGE pragmas (e.g., GeneralizedNewtypeDeriving) if
required by the derived classes.
🪄 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: e5cd2022-fcb4-4721-b428-5b9ce905d1a7
📒 Files selected for processing (5)
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/Types.hslib/mobility-core/src/Kernel/External/Payment/Juspay/Flow.hslib/mobility-core/src/Kernel/External/Payment/Juspay/Types/Mandate.hs
Comment on lines
+613
to
+622
| { eventName = Nothing, | ||
| orderShortId = fromMaybe "" order_id, | ||
| status = mandateStatusEnum, | ||
| mandateStartDate = Just startDateUTC, | ||
| mandateEndDate = Just endDateUTC, | ||
| mandateId = mandate_id, | ||
| mandateFrequency = frequencyEnum, | ||
| mandateMaxAmount = realToFrac max_amount, | ||
| upi = mkUpi <$> (payment_info >>= (.upi)) | ||
| } |
There was a problem hiding this comment.
orderShortId silently defaults to empty string when order_id is absent.
order_id is Maybe Text in JuspayMandateStatusResp, and here it's coalesced to "". Downstream code that treats orderShortId as a required, non-empty identifier (e.g., for lookups/logging correlation) will silently misbehave. Consider either:
- Logging at
Info/Warnwhenorder_idis missing, or - Making
orderShortIdonMandateStatusRespaMaybe Text, or - Failing with a clear
InternalErrorif it's truly required.
Given this is a mandate-status response (not necessarily tied to a single order), the middle option is typically the cleanest.
🤖 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 613 - 622, The code currently coerces JuspayMandateStatusResp.order_id to
an empty string when building MandateStatusResp.orderShortId (fromMaybe ""
order_id); instead change MandateStatusResp.orderShortId to a Maybe Text and
propagate the original order_id directly (i.e., assign orderShortId = order_id)
so absence is preserved; update the data type declaration for MandateStatusResp
and any places that construct or consume orderShortId (search for
MandateStatusResp, orderShortId usage) to handle Maybe Text accordingly, and add
minimal handling where a non-empty identifier is required (e.g., explicit error
or logging) rather than silently using "".
Comment on lines
+159
to
+169
| data JuspayMandateStatusResp = JuspayMandateStatusResp | ||
| { mandate_id :: Text, | ||
| status :: Text, | ||
| frequency :: Text, | ||
| start_date :: Text, | ||
| end_date :: Text, | ||
| max_amount :: Double, | ||
| order_id :: Maybe Text, | ||
| payment_info :: Maybe MandatePaymentInfo | ||
| } | ||
| deriving (Eq, Show, Generic, ToJSON, FromJSON, ToSchema) |
There was a problem hiding this comment.
max_amount :: Double loses precision; use HighPrecMoney for consistency.
Other mandate payloads in this codebase model max_amount as HighPrecMoney (see the webhook path at lib/mobility-core/src/Kernel/External/Payment/Interface/Juspay.hs line 669, where justMandate.max_amount is assigned directly to mandateMaxAmount :: HighPrecMoney without conversion). Modeling the status response's max_amount as Double here introduces precision loss and diverges from the rest of the payment module. The downstream mapper at Interface/Juspay.hs line 620 compensates with realToFrac max_amount, which is unnecessary if the field is typed as HighPrecMoney to begin with.
♻️ Proposed change
data JuspayMandateStatusResp = JuspayMandateStatusResp
{ mandate_id :: Text,
status :: Text,
frequency :: Text,
start_date :: Text,
end_date :: Text,
- max_amount :: Double,
+ max_amount :: HighPrecMoney,
order_id :: Maybe Text,
payment_info :: Maybe MandatePaymentInfo
}🤖 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/Mandate.hs` around
lines 159 - 169, The JuspayMandateStatusResp type declares max_amount :: Double
which causes precision loss; change the field to max_amount :: HighPrecMoney and
update its JSON instances if necessary so it deserializes as HighPrecMoney; then
remove the compensating realToFrac conversion in the downstream mapper (the code
that assigns justMandate.max_amount to mandateMaxAmount in Interface/Juspay.hs
and the mapping around line 620) so the value flows as HighPrecMoney end-to-end.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of Change
Description
Additional Changes
Motivation and Context
How did you test it?
Checklist
./dev/format-all-files.shSummary by CodeRabbit
Release Notes