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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
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 convert Idfy outputs to internal CRC responses. 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
2 times, most recently
from
c2d1757 to
694b88c
Compare
bytedex
force-pushed
the
feat/integrate-crc-idfy
branch
from
694b88c to
a33ba7e
Compare
3 participants
Type of Change
Description
Additional Changes
Motivation and Context
How did you test it?
Checklist
./dev/format-all-files.shSummary by CodeRabbit