Skip to content

[DRAFT PR] Code Review for Release 1.0.0#172

Closed
swatigoel wants to merge 9 commits into
masterfrom
release-1.x.x
Closed

[DRAFT PR] Code Review for Release 1.0.0#172
swatigoel wants to merge 9 commits into
masterfrom
release-1.x.x

Conversation

@swatigoel

@swatigoel swatigoel commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Chores

    • Version bumped to 1.0.0-SNAPSHOT across multiple modules.
    • Updated CI/CD workflow configurations.
  • Documentation

    • Clarified README to describe the repository's multi-plugin architecture.
  • Refactor

    • Simplified credential issuance plugins by streamlining helper dependencies and removing deprecated components.
    • Enhanced logging for credential context validation.

Piyush7034 and others added 9 commits January 29, 2026 13:06
* [INJICERT-1277] Remove cache dependency from mdoc vc issuance

Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>

* [INJICERT-1277] Remove unused injected dependencies

Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>

* [INJICERT-1277] Fix nitpick comments

Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>

---------

Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>
Signed-off-by: Rajapandi M <138785181+rajapandi1234@users.noreply.github.com>
…t in validateContextUrl (#143)

* [INJICERT] Add info logs to debug contextList and vcRequestContextList in validateContextUrl

Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>

* [INJICERT] Update plugin version to 0.7.0-SNAPSHOT in all pom files

Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>

---------

Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: Mahesh-Binayak <76687012+Mahesh-Binayak@users.noreply.github.com>
Signed-off-by: jaswanthkumartw <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: Mahesh-Binayak <76687012+Mahesh-Binayak@users.noreply.github.com>
Signed-off-by: jaswanthkumartw <jaswanthkumar.p@thoughtworks.com>
* feat(nonce): #863 Fix IDA VCI plugin configuration

Signed-off-by: jaswanthkumartw <jaswanthkumar.p@thoughtworks.com>

* fix(plugin): #863 correct @value placeholder syntax and add missing import

Signed-off-by: jaswanthkumartw <jaswanthkumar.p@thoughtworks.com>

* fix(plugin): #863 change includePayload to boolean for proper configuration handling

Signed-off-by: jaswanthkumartw <jaswanthkumar.p@thoughtworks.com>

---------

Signed-off-by: jaswanthkumartw <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: Sakshi Jhanwar <sakshi.jhanwar@ad.infosys.com>
Co-authored-by: Sakshi Jhanwar <sakshi.jhanwar@ad.infosys.com>
@swatigoel swatigoel marked this pull request as draft June 17, 2026 07:58
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

The PR removes deprecated VCI* helper classes (VCITransactionHelper, VCIAuthTransactionHelper, VCIHelperService) and simplifies MDocMockVCIssuancePlugin to resolve document number directly from the sub claim instead of via crypto/cache paths. ConditionalOnProperty conditions are updated across helpers, and HelperService gains a configurable includePayload flag. All module versions are bumped to 1.0.0-SNAPSHOT and CI Sonar jobs are re-pinned.

Changes

Plugin refactoring: remove deprecated VCI* helpers and simplify ID resolution

Layer / File(s) Summary
MDocMockVCIssuancePlugin: remove crypto/cache path, use sub claim directly
mock-certify-plugin/src/main/java/.../MDocMockVCIssuancePlugin.java, mock-certify-plugin/src/test/.../MDocMockVCIssuancePluginTest.java
Drops caching, OIDC transaction lookup, AES decryption, HSM/key-alias retrieval, and related imports. documentNumber is now resolved from identityDetails.get("sub") with a null-check throwing VCIExchangeException. Tests remove all cache/crypto test methods and mock only MdocGenerator.
Remove deprecated VCIAuthTransactionHelper, VCITransactionHelper, VCIHelperService
mosip-identity-certify-plugin/src/main/java/.../helper/VCIAuthTransactionHelper.java, .../helper/VCITransactionHelper.java, .../service/VCIHelperService.java, .../helper/AuthTransactionHelper.java, .../helper/TransactionHelper.java, .../service/HelperService.java
VCIAuthTransactionHelper, VCITransactionHelper, and VCIHelperService are fully deleted. AuthTransactionHelper and TransactionHelper switch @ConditionalOnProperty to mosip.certify.integration.ida-plugin-helper=IdaPluginImpl. HelperService adopts the same condition and adds a @Value-injected includePayload boolean replacing the hardcoded true.
IdaVCIssuancePluginImpl rewiring and test updates
mosip-identity-certify-plugin/src/main/java/.../service/IdaVCIssuancePluginImpl.java, mosip-identity-certify-plugin/src/test/java/.../helper/TransactionHelperTest.java, .../service/IdaVCIssuancePluginImplTest.java
IdaVCIssuancePluginImpl injects TransactionHelper and HelperService replacing VCITransactionHelper and VCIHelperService. All Mockito stubs and field injections in IdaVCIssuancePluginImplTest and the renamed TransactionHelperTest are updated accordingly.

Version bumps, CI workflow updates, and minor housekeeping

Layer / File(s) Summary
Version bumps to 1.0.0-SNAPSHOT across all modules
mock-certify-plugin/pom.xml, mosip-identity-certify-plugin/pom.xml, postgres-dataprovider-plugin/pom.xml, sunbird-rc-certify-integration-impl/pom.xml
Project versions bumped from 0.6.0-SNAPSHOT and certify-core/certify-integration-api dependency versions bumped from 0.14.0-SNAPSHOT to 1.0.0-SNAPSHOT across all four modules.
CI workflow pin, Slack webhook changes, SunbirdRC logging, README
.github/workflows/push-trigger.yml, sunbird-rc-certify-integration-impl/src/main/java/.../SunbirdRCVCIssuancePlugin.java, README.md
All sonar_analysis jobs updated to the reusable workflow pinned at @dsd9685; sunbird-rc and postgres-dataprovider jobs switch to SLACK_WEBHOOK_INJI_TEAM. Three log statements added in SunbirdRCVCIssuancePlugin.validateContextUrl. README introductory sentence rephrased.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • inji/digital-credential-plugins#171: Both PRs bump the same Maven module versions and io.inji.certify dependency versions to 1.0.0-SNAPSHOT across mock-certify-plugin, mosip-identity-certify-plugin, postgres-dataprovider-plugin, and sunbird-rc-certify-integration-impl.

Poem

🐇 Hop hop, the old VCI helpers are gone,
No more HSM caches to carry along!
Just grab the sub claim, simple and bright,
One IdaPluginImpl sets everything right.
The rabbit refactors, the codebase grows lean,
1.0.0-SNAPSHOT — the cleanest we've seen! 🌿

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title '[DRAFT PR] Code Review for Release 1.0.0' is vague and generic, using non-descriptive language that does not convey the specific technical changes made across multiple modules, version bumps, and architectural refactoring. Revise the title to clearly describe the main changes, such as 'Bump plugin versions to 1.0.0 and refactor Ida integration helpers' or similar, removing '[DRAFT PR]' and 'Code Review for' boilerplate.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch release-1.x.x
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch release-1.x.x

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@swatigoel

Copy link
Copy Markdown
Contributor Author

changes looks fine hence closing the PR

@swatigoel swatigoel closed this Jun 17, 2026
@Prafulrakhade Prafulrakhade deleted the release-1.x.x branch June 17, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants