Repository navigation
feat: PM-KISAN application status in the Grievance plugin [#60] - #91
KrutikaPhirangi wants to merge 2 commits into
Conversation
🛡️ Trivy security scan (CRITICAL,HIGH,MEDIUM,LOW)Go dependenciesNo findings at CRITICAL,HIGH,MEDIUM,LOW. Container imageNo findings at CRITICAL,HIGH,MEDIUM,LOW. |
8083ade to
af4f878
Compare
|
📊 Test Coverage: ✅ Passed — 81% of changed lines covered, min 80% |
manjudr
left a comment
There was a problem hiding this comment.
@KrutikaPhirangi - Please walk through the code, Some part I am unable to understand it.
| - id: Grievance | ||
| config: | ||
| bindingKeys: "pmfby|openagrinet:Grievance,pmkisan|openagrinet:Grievance" | ||
| bindingKeys: "pmfby|openagrinet:Grievance,pmkisan|openagrinet:Grievance,pmkisan-chatbot|openagrinet:SchemeApplication" |
There was a problem hiding this comment.
Why is it a chatbot? Can we keep it as PMKisan only?
|
|
||
| // followUp adds a second read's answer under field after a successful call; | ||
| // on failure the field is absent, never empty (ADR-0005). | ||
| type followUp struct { |
There was a problem hiding this comment.
What does 'follow-up' mean? Why do we need this? And why is this in common directory? any other plugins are depending on it?
There was a problem hiding this comment.
Because of the earlier review on #71 asking to move the envelope out of Grievance, and I moved the follow-up along with it.
only the encryption is a fair fit for common, the follow-up isn't. I can move it to Grievance
| default: | ||
| // The participant is the OTP service: verify on its host. | ||
| cbc, onOTPService := e.base.(*cbcCodec) | ||
| if !onOTPService { | ||
| return nil, fmt.Errorf("%s: otpVerifyPath needs otpEnvelope, or envelope %s: the OTP is verified on the OTP service", | ||
| provider, envelopeAESCBCKeyInBand) | ||
| } | ||
| e.verify = &verify{provider: provider, path: s.verifyPath, actions: verifyActions, codec: cbc, | ||
| host: participantHost, hooks: hooks, client: client, budget: model.ActionPlan{TimeoutMs: verifyTimeoutMs}} | ||
| } | ||
|
|
||
| followUpActions := listSet(s.followUpActions) | ||
| switch { | ||
| case s.followUpPath == "" && s.followUpField == "" && len(followUpActions) == 0: | ||
| case s.followUpPath == "" || s.followUpField == "" || len(followUpActions) == 0: | ||
| return nil, fmt.Errorf("%s: followUpPath, followUpField and followUpActions are set together or not at all", provider) | ||
| default: | ||
| for action := range followUpActions { | ||
| if e.otpActions[action] && e.otp == nil || !e.otpActions[action] && e.base == nil { | ||
| return nil, fmt.Errorf("%s: followUpActions names %s, which has no envelope to send a follow-up through", provider, action) | ||
| } | ||
| } | ||
| e.followUp = &followUp{provider: provider, path: s.followUpPath, field: s.followUpField, actions: followUpActions, | ||
| succeeded: hooks.Succeeded, client: client, budget: model.ActionPlan{TimeoutMs: followUpTimeoutMs}} |
There was a problem hiding this comment.
I wasn't able to understand this? Why are making it as default? Probably you may need to walkthrough this @KrutikaPhirangi
|
|
||
| "followUpPath": func(s *envelopeSettings, v string) { s.followUpPath = v }, | ||
| "followUpField": func(s *envelopeSettings, v string) { s.followUpField = v }, | ||
| "followUpActions": func(s *envelopeSettings, v string) { s.followUpActions = v }, | ||
| "followUpTimeoutMs": func(s *envelopeSettings, v string) { s.followUpTimeoutMs = v }, | ||
| } |
There was a problem hiding this comment.
What is "followup" What is the use case here?
There was a problem hiding this comment.
a second call to PM-KISAN, made automatically after the first one succeeds, whose answer is added to the first.
The use case: PM-KISAN application status needs two calls to the chatbot service:
- /ChatbotUserDetails returns the registration: registration number, registration date, instalments paid, eKYC.
- /ChatbotBeneficiaryStatus returns what is blocking payment, for example "Income Tax Payee" or "Land Seeding, KYS".
The second call becomes application.blockers in on_status, so the farmer learns why the money isn't coming. A Beckn status maps to one registry call, so the envelope makes the second call itself and passes both answers to the mapping.
| : $exists($match($d, /^\/Date\([0-9]+([+-][0-9]{4})?\)\/$/)[0]) | ||
| ? $fromMillis($number($split($substring($d, 6), /[^0-9]/)[0]), $istPicture, "+0530") | ||
| : $exists($match($d, /^[0-9]{1,2}\/[0-9]{1,2}\/[0-9]{4}/)[0]) | ||
| ? ( |
There was a problem hiding this comment.
Why we need this complex extraction logic?
There was a problem hiding this comment.
Fair point. The four formats were a guess, because we hadn't seen a real DateOfRegistration I'll check a real staging response and keep only that format, leaving the date out if it doesn't match.
af4f878 to
128895b
Compare
…llow-up read [#60] aesCbcKeyInBand can be a provider's main envelope, and such a provider verifies the challenge on its own host. followUpPath, followUpField and followUpActions make a second read after a successful call and add its answer under followUpField, on its own budget (followUpTimeoutMs); a failed follow-up leaves the field absent.
PM-KISAN application status on the SchemeApplication pack, served by the chatbot service (pmkisan-chatbot): init sends an OTP for an enrolmentId or applicant.mobile; status verifies it, reads /ChatbotUserDetails, then /ChatbotBeneficiaryStatus for what is holding payment up. Only allow-listed fields are returned; the mobile is never echoed. No registration is a 400 BIZ_GENERIC_ERROR.
128895b to
8c5637c
Compare
What
PM-KISAN application status in the
Grievanceplugin, on the SchemeApplication pack. Stacked on #71 (PM-KISAN grievance); review that first. ADRs are in #92.init/ChatbotOTPpmkisan/application-status.init.yamlstatus/ChatbotOTPVerified,/ChatbotUserDetails, then/ChatbotBeneficiaryStatuspmkisan/application-status.status.yamlCommits
aesCbcKeyInBandas a provider's main envelope, verify on the provider's own host, and a follow-up read whose answer is added to the first (absent if it fails). ADR-0005.pmkisan-chatbotprovider block and binding key, mappings, tests. The farmer is named byenrolmentIdorapplicant.mobile; the mobile is never echoed.on_statusreturns only allow-listed fields:enrolmentId,registeredOn,latestInstalmentPaid,ekyc,blockers. No registration is a 400.Testing
go test ./pkg/plugin/implementation/Grievance/... ./pkg/plugin/implementation/internal/...