feat: integrate IDfy CRC API for individual court record verification#1316
feat: integrate IDfy CRC API for individual court record verification#1316bytedex wants to merge 1 commit into
Conversation
WalkthroughAdds CRC (Court Record Check) verification end-to-end: request/response types, Idfy request payloads, a Servant client endpoint for ind_court_record, and interface functions to invoke the client and map CRC outputs. ChangesCRC Verification Feature
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.
🧹 Nitpick comments (1)
lib/mobility-core/src/Kernel/External/Verification/Interface/Idfy.hs (1)
213-242: ⚡ Quick winConfirm
entity_typevalue expected by IDfy CRC API
lib/mobility-core/src/Kernel/External/Verification/Interface/Idfy.hshardcodesIdfy.CRCVerificationData.entity_typeto"individual". Publicly indexed IDfy resources don’t expose theind_court_recordendpoint (or an authoritative enum ofentity_typevalues), so it can’t be validated whether"individual"is always correct or whether other values (e.g., organization/company) must be used.What are the valid values for
entity_typein IDfy CRC (ind_court_record) for the individual-vs-organization variants?If multiple values are supported, model
entity_typeas a typed/configured value (e.g., an enum/ADT) instead of a string literal.🤖 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/Verification/Interface/Idfy.hs` around lines 213 - 242, verifyCRCAsync currently hardcodes Idfy.CRCVerificationData.entity_type = "individual", but the IDfy CRC API may accept multiple entity types (individual vs organization) so you must confirm the canonical values with IDfy docs/support and stop using a string literal; introduce a typed ADT/enum (e.g., EntityType = Individual | Organization) and expose it via IdfyCfg or the VerifyCRCReq so the caller can select the correct variant, update buildIdfyRequest and Idfy.CRCVerificationData construction to map the new ADT to the API string, and adjust types/signatures where Idfy.CRCVerificationData.entity_type is set to ensure compile-time safety instead of hardcoded "individual".
🤖 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.
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Verification/Interface/Idfy.hs`:
- Around line 213-242: verifyCRCAsync currently hardcodes
Idfy.CRCVerificationData.entity_type = "individual", but the IDfy CRC API may
accept multiple entity types (individual vs organization) so you must confirm
the canonical values with IDfy docs/support and stop using a string literal;
introduce a typed ADT/enum (e.g., EntityType = Individual | Organization) and
expose it via IdfyCfg or the VerifyCRCReq so the caller can select the correct
variant, update buildIdfyRequest and Idfy.CRCVerificationData construction to
map the new ADT to the API string, and adjust types/signatures where
Idfy.CRCVerificationData.entity_type is set to ensure compile-time safety
instead of hardcoded "individual".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85d2c292-368c-4b73-a72f-c6f9dc6c655a
📒 Files selected for processing (6)
lib/mobility-core/src/Kernel/External/Verification/Idfy/Client.hslib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Request.hslib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Response.hslib/mobility-core/src/Kernel/External/Verification/Interface/Idfy.hslib/mobility-core/src/Kernel/External/Verification/Interface/Types.hslib/mobility-core/src/Kernel/External/Verification/Types.hs
bytedex
left a comment
bytedex
left a comment
There was a problem hiding this comment.
@ClaudeWeb review this pr
khuzema786
left a comment
khuzema786
left a comment
There was a problem hiding this comment.
Review: IDfy CRC API Integration
The integration is structurally sound and consistent with the existing IDfy verification patterns (RC, DL, PAN, Udyam). No security issues found — API keys are properly decrypted via decrypt, PII fields are passed through correctly. A few things to address before merging.
Warning — GetTaskResp is a breaking change for all downstream consumers
File: lib/mobility-core/src/Kernel/External/Verification/Interface/Types.hs
Adding CRCResp to the GetTaskResp sum type means every pattern match on GetTaskResp without a wildcard catch in nammayatri/nammayatri will now be non-exhaustive. Please confirm all call sites are updated alongside this PR or in an atomic follow-up.
If GetTaskResp values are ever serialised and stored (Redis, job queue, etc.), old payloads without the "CRCResp" tag will fail to deserialise after this ships.
Warning — Untyped status and risk_type fields (confidence 80)
File: lib/mobility-core/src/Kernel/External/Verification/Types.hs
status :: Maybe Text and risk_type :: Maybe Text are the primary fields callers will branch on. IDfy returns a finite set of values for both ("completed" / "in_progress" / "failed" and "low" / "medium" / "high"). As free-form Text, a misspelled or unexpected value from the API silently produces wrong downstream behavior.
Suggested fix — model them as ADTs with a fallback:
data CRCStatus = CRCCompleted | CRCInProgress | CRCFailed | CRCStatusUnknown Text
deriving (Show, Generic, ToSchema)
data CRCRiskType = CRCLow | CRCMedium | CRCHigh | CRCRiskUnknown Text
deriving (Show, Generic, ToSchema)Same concern applies to report_mode :: Maybe Text in the request type — if the IDfy API accepts only a fixed set of modes, this should be an ADT.
Warning — No tests added (confidence 85)
convertCRCOutputToCRCVerification is a pure function with no external dependencies — it's trivially testable. A unit test covering the happy path (all fields populated) and the all-Nothing degenerate case would catch any field-mapping mistakes (e.g., riskSummary vs risk_summary mixups). No other conversion functions in this file have tests either, but this PR is a good place to start.
Potential — dob :: Maybe Text format undocumented (confidence 65)
File: lib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Request.hs
IDfy typically expects dob in DD-MM-YYYY format. An incorrect format will fail silently at the type level and only surface as a 4xx at runtime. At minimum add a comment:
dob :: Maybe Text -- Expected format: DD-MM-YYYYA newtype Dob = Dob Text with a smart constructor would be stronger.
Potential — entity_type hardcoded magic string (confidence 65)
File: lib/mobility-core/src/Kernel/External/Verification/Interface/Idfy.hs (~line 228)
entity_type = "individual" is hardcoded. Since ind_court_record is individual-only per the IDfy docs, this is safe today. CodeRabbit already flagged this. If the intent is always individual, consider:
data CRCEntityType = Individual
instance ToJSON CRCEntityType where
toJSON Individual = "individual"Even a one-constructor ADT removes the magic string and makes the intent explicit.
Process — PR description checklist is empty
All checklist items are unchecked. Please tick off formatting/lint check, self-review, and tests (or mark N/A), and add a CHANGELOG.md entry for the new capability.
|
I'll fetch the PR diff, description, and comments in parallel to do a thorough review.The Verdict: Comment (not blocking, but a few things need attention) The integration follows the established IDfy pattern cleanly. No security issues — keys are properly decrypted, PII is handled correctly. Key findings: Warnings
Potential Issues
Process
|
| data CRCEntityDetails = CRCEntityDetails | ||
| { name :: Text, | ||
| father_name :: Maybe Text, | ||
| dob :: Maybe Text, |
There was a problem hiding this comment.
Can be of Day type? Also, these fields seem mandatory
bytedex
force-pushed
the
feat/integrate-crc-idfy
branch
from
9412c23 to
0890c6b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Request.hs (1)
154-154: ⚡ Quick windob date format matches IDfy (
YYYY-MM-DD)
- IDfy expects
dobin ISO 8601YYYY-MM-DD, and the existing formatting ("%F") matches that, so the DD-MM-YYYY mismatch concern doesn’t apply.- Still worth tightening the type/documentation for
dob :: Maybe Textto avoid free-formTextmisuse (e.g., use a typed date/newtype).🤖 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/Verification/Idfy/Types/Request.hs` at line 154, The dob field is currently declared as "dob :: Maybe Text" which allows free-form strings; change it to a strongly typed date (e.g., newtype IdfyDOB = IdfyDOB Day or a newtype wrapping Text with validation) and update the request record to use that type instead of Maybe Text; add parsing/formatting (FromJSON/ToJSON or To/FromText) enforcing the YYYY-MM-DD format and adjust any constructors, serializers, and places that construct or pattern-match on the dob field (search for the dob field name in the request type and its JSON instances to update).
🤖 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.
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Request.hs`:
- Line 154: The dob field is currently declared as "dob :: Maybe Text" which
allows free-form strings; change it to a strongly typed date (e.g., newtype
IdfyDOB = IdfyDOB Day or a newtype wrapping Text with validation) and update the
request record to use that type instead of Maybe Text; add parsing/formatting
(FromJSON/ToJSON or To/FromText) enforcing the YYYY-MM-DD format and adjust any
constructors, serializers, and places that construct or pattern-match on the dob
field (search for the dob field name in the request type and its JSON instances
to update).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 927ec18d-33ba-47bb-84c1-94493452e718
📒 Files selected for processing (6)
lib/mobility-core/src/Kernel/External/Verification/Idfy/Client.hslib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Request.hslib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Response.hslib/mobility-core/src/Kernel/External/Verification/Interface/Idfy.hslib/mobility-core/src/Kernel/External/Verification/Interface/Types.hslib/mobility-core/src/Kernel/External/Verification/Types.hs
🚧 Files skipped from review as they are similar to previous changes (5)
- lib/mobility-core/src/Kernel/External/Verification/Types.hs
- lib/mobility-core/src/Kernel/External/Verification/Interface/Types.hs
- lib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Response.hs
- lib/mobility-core/src/Kernel/External/Verification/Idfy/Client.hs
- lib/mobility-core/src/Kernel/External/Verification/Interface/Idfy.hs
bytedex
force-pushed
the
feat/integrate-crc-idfy
branch
3 times, most recently
from
694b88c to
a33ba7e
Compare
| dob :: Maybe Day, | ||
| address :: Maybe Text, | ||
| panNumber :: Maybe Text, | ||
| monitorCase :: Maybe Bool, |
There was a problem hiding this comment.
this we don't need, and will not have relevant data, let's remove this.
| address :: Maybe Text, | ||
| panNumber :: Maybe Text, | ||
| monitorCase :: Maybe Bool, | ||
| reportMode :: Maybe Text, |
There was a problem hiding this comment.
this we don't need, and will not have relevant data, let's remove this.
|
|
||
| data CRCVerificationOutput = CRCVerificationOutput | ||
| { case_details_link :: Maybe Text, | ||
| monitor_case :: Maybe Bool, |
| data CRCVerificationOutput = CRCVerificationOutput | ||
| { case_details_link :: Maybe Text, | ||
| monitor_case :: Maybe Bool, | ||
| new_cases_count :: Maybe Int, |
| dob :: Maybe Day, | ||
| address :: Maybe Text, | ||
| pan_number :: Maybe Text, | ||
| monitor_case :: Maybe Bool, |
| accountId <- decrypt cfg.accountId | ||
| let reqData = | ||
| Idfy.CRCVerificationData | ||
| { entity_type = "individual", |
There was a problem hiding this comment.
This can be enum , individual and business
| addr.pin | ||
| ] | ||
|
|
||
| convertCRCOutputToCRCVerification :: CRCVerificationOutput -> VT.CRCVerificationResponse |
There was a problem hiding this comment.
instead of this, we can convert camelCase to snakeCase and vice versa via fromJSON and toJSON,
bytedex
force-pushed
the
feat/integrate-crc-idfy
branch
from
a33ba7e to
5ee4505
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Response.hs (1)
70-72: ⚡ Quick winAdd focused tests for the new
ind_court_recorddecode/encode path.Please add unit tests covering (1) happy-path CRC payload decode/encode and (2) missing/
Nothingresult handling so this branch doesn’t regress silently.Also applies to: 87-87, 104-104
🤖 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/Verification/Idfy/Types/Response.hs` around lines 70 - 72, The new "ind_court_record" branch in Response.hs (the parseJSON @(IdfyResponse (SourceOutput VT.CRCVerificationResponse)) path that maps with mapIdfyResponse CRCResult) lacks unit tests; add focused tests that (1) decode a valid ind_court_record JSON into IdfyResponse (SourceOutput VT.CRCVerificationResponse) and round-trip encode-back to assert equality (happy-path), and (2) decode cases where the result field is missing/null to ensure the Nothing handling path of CRCResult is exercised and asserted; place tests next to other Idfy Response specs and reference parseJSON, IdfyResponse, CRCResult and the ind_court_record case so the branch at lines around the parseJSON mapping is covered.lib/mobility-core/src/Kernel/External/Verification/Types.hs (1)
194-201: ⚡ Quick winModel CRC
riskTypeandstatusas ADTs withUnknown Textfallback.Keeping these enum-like fields as
Maybe Textweakens exhaustive handling and lets unknown wire values leak silently through business logic.🤖 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/Verification/Types.hs` around lines 194 - 201, Change CRCVerificationResponse to use ADTs for the enum-like fields: replace the riskType :: Maybe Text and status :: Maybe Text with types like RiskType and VerificationStatus (each defined as concrete constructors for known values plus an Unknown Text fallback, and keep a Maybe wrapper if the whole field can be absent). Implement FromJSON and ToJSON (or parsing functions) for RiskType and VerificationStatus so known strings map to specific constructors and any unrecognized string maps to Unknown with the original text preserved. Update any code that constructs or pattern-matches on CRCVerificationResponse to use the new ADTs (e.g., pattern-match on RiskType/VerificationStatus constructors instead of raw Text) and add conversion helpers if needed to minimize call-site changes; ensure tests/parsers that consume these fields are updated accordingly.
🤖 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/Verification/Interface/Types.hs`:
- Around line 289-290: You added a new constructor CRCResp to the public sum
type GetTaskResp which is a breaking change; revert that constructor addition
from GetTaskResp and instead introduce a new versioned type (e.g. GetTaskRespV2)
that duplicates all current constructors plus CRCResp, or create a separate
CRC-specific wrapper type, and update only the new consumers/parsers to use
GetTaskRespV2 while leaving existing code using GetTaskResp untouched; reference
the GetTaskResp and CRCResp symbols when making the change so all pattern
matches and FromJSON/ToJSON instances remain stable for downstream code.
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Response.hs`:
- Around line 70-72: The new "ind_court_record" branch in Response.hs (the
parseJSON @(IdfyResponse (SourceOutput VT.CRCVerificationResponse)) path that
maps with mapIdfyResponse CRCResult) lacks unit tests; add focused tests that
(1) decode a valid ind_court_record JSON into IdfyResponse (SourceOutput
VT.CRCVerificationResponse) and round-trip encode-back to assert equality
(happy-path), and (2) decode cases where the result field is missing/null to
ensure the Nothing handling path of CRCResult is exercised and asserted; place
tests next to other Idfy Response specs and reference parseJSON, IdfyResponse,
CRCResult and the ind_court_record case so the branch at lines around the
parseJSON mapping is covered.
In `@lib/mobility-core/src/Kernel/External/Verification/Types.hs`:
- Around line 194-201: Change CRCVerificationResponse to use ADTs for the
enum-like fields: replace the riskType :: Maybe Text and status :: Maybe Text
with types like RiskType and VerificationStatus (each defined as concrete
constructors for known values plus an Unknown Text fallback, and keep a Maybe
wrapper if the whole field can be absent). Implement FromJSON and ToJSON (or
parsing functions) for RiskType and VerificationStatus so known strings map to
specific constructors and any unrecognized string maps to Unknown with the
original text preserved. Update any code that constructs or pattern-matches on
CRCVerificationResponse to use the new ADTs (e.g., pattern-match on
RiskType/VerificationStatus constructors instead of raw Text) and add conversion
helpers if needed to minimize call-site changes; ensure tests/parsers that
consume these fields 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: 6d8d6b04-6b20-450a-8f4c-ece923cf675f
📒 Files selected for processing (6)
lib/mobility-core/src/Kernel/External/Verification/Idfy/Client.hslib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Request.hslib/mobility-core/src/Kernel/External/Verification/Idfy/Types/Response.hslib/mobility-core/src/Kernel/External/Verification/Interface/Idfy.hslib/mobility-core/src/Kernel/External/Verification/Interface/Types.hslib/mobility-core/src/Kernel/External/Verification/Types.hs
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/mobility-core/src/Kernel/External/Verification/Idfy/Client.hs
4 participants
Type of Change
Description
Additional Changes
Motivation and Context
How did you test it?
Checklist
./dev/format-all-files.shSummary by CodeRabbit