fix(plugin): #863 Fix IDA VCI plugin configuration#150
Conversation
Signed-off-by: jaswanthkumartw <jaswanthkumar.p@thoughtworks.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR refactors Spring bean activation conditions and removes deprecated VCI-specific helper classes. Configuration property names are updated from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 0/1 reviews remaining, refill in 20 minutes and 59 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@mosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/helper/TransactionHelper.java`:
- Line 16: The TransactionHelper bean is conditionally activated by
`@ConditionalOnProperty`(value = "mosip.certify.integration.ida-plugin-helper",
havingValue = "IdaPluginImpl") while IdaVCIssuancePluginImpl depends on
TransactionHelper and HelperService and is activated by
mosip.certify.integration.vci-plugin=IdaVCIssuancePluginImpl; align activation
so the helper beans are available when the VCI plugin is enabled by either
changing the TransactionHelper (and related helper bean) `@ConditionalOnProperty`
to check mosip.certify.integration.vci-plugin with havingValue
"IdaVCIssuancePluginImpl" or add the vci-plugin property as an additional
required property on the helper beans (or alternatively add the
ida-plugin-helper condition to IdaVCIssuancePluginImpl) so that
IdaVCIssuancePluginImpl, TransactionHelper, and HelperService are enabled under
the same property set.
In
`@mosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/service/HelperService.java`:
- Around line 44-45: In HelperService.java fix the malformed `@Value` injection
and missing import: add the import for
org.springframework.beans.factory.annotation.Value and change the annotation on
the includePayload field from the invalid
"${{mosip.certify.ida.include-payload:true}" to a valid Spring placeholder like
"${mosip.certify.ida.include-payload:true}" so the includePayload field is
correctly injected at startup.
In
`@mosip-identity-certify-plugin/src/test/java/io/mosip/certify/mosipid/integration/helper/TransactionHelperTest.java`:
- Around line 43-48: The negative test in TransactionHelperTest calling
transactionHelper.getOAuthTransaction("test") lacks an explicit fail path, so if
the call stops throwing the test will incorrectly pass; update the test method
getOAuthTransactionWithInValidDetails_thenFail to call Assert.fail("Expected
exception not thrown") immediately after the
transactionHelper.getOAuthTransaction("test") invocation (before the catch) and
replace the informal assert in the catch with a proper assertion (e.g.,
Assert.assertEquals("cache_missing", e.getMessage())) to ensure failures are
reported correctly.
🪄 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: d7fe2867-2fb1-4901-a1bf-05243157f55c
📒 Files selected for processing (9)
mosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/helper/AuthTransactionHelper.javamosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/helper/TransactionHelper.javamosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/helper/VCIAuthTransactionHelper.javamosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/helper/VCITransactionHelper.javamosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/service/HelperService.javamosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/service/IdaVCIssuancePluginImpl.javamosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/service/VCIHelperService.javamosip-identity-certify-plugin/src/test/java/io/mosip/certify/mosipid/integration/helper/TransactionHelperTest.javamosip-identity-certify-plugin/src/test/java/io/mosip/certify/mosipid/integration/service/IdaVCIssuancePluginImplTest.java
💤 Files with no reviewable changes (3)
- mosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/helper/VCITransactionHelper.java
- mosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/helper/VCIAuthTransactionHelper.java
- mosip-identity-certify-plugin/src/main/java/io/mosip/certify/mosipid/integration/service/VCIHelperService.java
…mport Signed-off-by: jaswanthkumartw <jaswanthkumar.p@thoughtworks.com>
…ration handling Signed-off-by: jaswanthkumartw <jaswanthkumar.p@thoughtworks.com>
Summary by CodeRabbit
Chores
Improvements