[ES-2944] New map entry for resetPasswordChallengeFileds#189
[ES-2944] New map entry for resetPasswordChallengeFileds#189KashiwalHarsh wants to merge 3 commits intomosip:developfrom
Conversation
Signed-off-by: Harsh Kashiwal <harsh.kashiwal@infosys.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdded extraction and wiring of resetPasswordChallengeFields into IdrepoProfileRegistryPluginImpl: new JSON-path config field, a parser method with error handling, and inclusion of the parsed value in the UI specification output; also fixed a stray quote in application.properties endpoint value. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java (2)
254-256: Don't log the fallback as an error.This branch already handles the field being absent by returning
Collections.emptyList(). Logging it aterrorwill create false alerts whenever older UI-spec responses omit the field, sowarnordebugwould fit the behavior better.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java` around lines 254 - 256, In IdrepoProfileRegistryPluginImpl (inside the resetPasswordChallengeFields handling), change the PathNotFoundException catch to stop logging as an error; instead log at warn or debug level (e.g., log.warn or log.debug) and optionally include the exception message/context, then continue returning Collections.emptyList() as before so absence of the field doesn't generate false error alerts.
160-161: Add the matching property to the default config.This new JSONPath is configurable here, but
mosip-identity-plugin/src/main/resources/application.properties:145-164does not listmosip.signup.mosipid.uispec.resetPasswordChallengeFields-jsonpathalongside the other UI-spec JSONPath keys. Please add it there too so the override is discoverable and stays consistent with the rest of the UI-spec settings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java` around lines 160 - 161, Add the new configurable JSONPath key used by IdrepoProfileRegistryPluginImpl -> resetPasswordChallengeFieldsJsonpath to the project's default application.properties so it is discoverable and consistent with other UI-spec keys; add an entry for mosip.signup.mosipid.uispec.resetPasswordChallengeFields-jsonpath with the same default value used in the `@Value` annotation ($[0].jsonSpec[0].spec.resetPasswordChallengeFields) alongside the other UI-spec JSONPath properties and ensure the key formatting matches the surrounding entries.
🤖 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-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`:
- Around line 569-570: The Map.entry list inside the Map.ofEntries call in
IdrepoProfileRegistryPluginImpl has a missing comma between
Map.entry("maxUploadFileSize", maxUploadFileSize) and
Map.entry("resetPasswordChallengeFields", resetPasswordChallengeFields); add the
comma after the maxUploadFileSize entry so the Map.entry(...) elements are
properly separated and the Map.ofEntries(...) block compiles.
---
Nitpick comments:
In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`:
- Around line 254-256: In IdrepoProfileRegistryPluginImpl (inside the
resetPasswordChallengeFields handling), change the PathNotFoundException catch
to stop logging as an error; instead log at warn or debug level (e.g., log.warn
or log.debug) and optionally include the exception message/context, then
continue returning Collections.emptyList() as before so absence of the field
doesn't generate false error alerts.
- Around line 160-161: Add the new configurable JSONPath key used by
IdrepoProfileRegistryPluginImpl -> resetPasswordChallengeFieldsJsonpath to the
project's default application.properties so it is discoverable and consistent
with other UI-spec keys; add an entry for
mosip.signup.mosipid.uispec.resetPasswordChallengeFields-jsonpath with the same
default value used in the `@Value` annotation
($[0].jsonSpec[0].spec.resetPasswordChallengeFields) alongside the other UI-spec
JSONPath properties and ensure the key formatting matches the surrounding
entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 01bfae41-a0ff-4bfb-956d-2885cebabcb1
📒 Files selected for processing (1)
mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java
...in/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Harsh Kashiwal <harsh.kashiwal@infosys.com>
Signed-off-by: Harsh Kashiwal <harsh.kashiwal@infosys.com>
|
Changes are taken from #190 |
Summary by CodeRabbit