[INJIWEB-1865] Modified the formatValue function#1041
[INJIWEB-1865] Modified the formatValue function#1041Rudhhi-Shah-14 wants to merge 2 commits intoinji:developfrom
Conversation
…support Signed-off-by: Rudhhi Shah <rudhhi.shah@thoughtworks.com>
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR updates CredentialPDFGeneratorService.formatValue to accept alternative map keys for language and 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/test/java/io/mosip/mimoto/service/CredentialPDFGeneratorServiceTest.java (2)
334-362:⚠️ Potential issue | 🟡 MinorSame issue: test uses unsupported
"language"key format.The
testGeneratePdfWithLocaleSpecificMapListFormattingtest also usesMap.of("language", "en", "value", "English Name")which won't work with the updatedformatValuelogic.🔧 Suggested fix
List<Map<String, Object>> localeData = List.of( - Map.of("language", "en", "value", "English Name"), - Map.of("language", "fr", "value", "French Name") + Map.of("@language", "en", "@value", "English Name"), + Map.of("@language", "fr", "@value", "French Name") );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/io/mosip/mimoto/service/CredentialPDFGeneratorServiceTest.java` around lines 334 - 362, The test testGeneratePdfWithLocaleSpecificMapListFormatting constructs locale maps using the old "language" key which no longer matches the updated formatValue logic; update the localeData maps to use the key expected by formatValue (e.g., "locale" or the exact key your formatValue reads) — for example replace Map.of("language", "en", "value", "English Name") with Map.of("locale", "en", "value", "English Name") (and similarly for "fr"), so the VCCredentialProperties credentialSubject in the test matches the shape consumed by formatValue used by credentialPDFGeneratorService.generatePdfForVerifiableCredential.
816-844:⚠️ Potential issue | 🟡 MinorExisting test uses unsupported
"language"key format.The
testFormatValueWithLocaleMapListtest usesMap.of("language", "en", "value", "English Name"), but the production code change now only supports@languageandlangkeys. This test will pass (due toassertNotNull(result)) but won't actually validate localization behavior—the format will silently return an empty string.Either update this test to use the new key format, or if the
"language"key should remain supported for backward compatibility, ensure the production code handles it.🔧 Suggested fix to align test with new key format
List<Map<String, Object>> localeData = List.of( - Map.of("language", "en", "value", "English Name"), - Map.of("language", "fr", "value", "French Name") + Map.of("@language", "en", "@value", "English Name"), + Map.of("@language", "fr", "@value", "French Name") );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/io/mosip/mimoto/service/CredentialPDFGeneratorServiceTest.java` around lines 816 - 844, The test testFormatValueWithLocaleMapList uses locale maps with the old "language" key which the production code no longer recognizes; update the localeData entries in this test to use the new key (e.g., "@language" or "lang") and corresponding values (e.g., Map.of("@language","en","value","English Name")) so that credentialPDFGeneratorService.generatePdfForVerifiableCredential and the formatValue logic will actually select the expected localized string during the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/io/mosip/mimoto/service/CredentialPDFGeneratorService.java`:
- Around line 254-261: The locale-matching lambda in
CredentialPDFGeneratorService currently only checks "@language" and "lang" keys,
causing credentials using "language" to be skipped; update the filter to also
check for "language" (i.e., try m.getOrDefault("@language",
m.getOrDefault("lang", m.get("language")))) so LocaleUtils.matchesLocale
receives the correct locale string, and mirror the same fallback in the
subsequent map that extracts "@value"/"value"/"value" keys if needed; verify
behavior against CredentialPDFGeneratorServiceTest and
CredentialShareServiceImpl usages.
---
Outside diff comments:
In
`@src/test/java/io/mosip/mimoto/service/CredentialPDFGeneratorServiceTest.java`:
- Around line 334-362: The test
testGeneratePdfWithLocaleSpecificMapListFormatting constructs locale maps using
the old "language" key which no longer matches the updated formatValue logic;
update the localeData maps to use the key expected by formatValue (e.g.,
"locale" or the exact key your formatValue reads) — for example replace
Map.of("language", "en", "value", "English Name") with Map.of("locale", "en",
"value", "English Name") (and similarly for "fr"), so the VCCredentialProperties
credentialSubject in the test matches the shape consumed by formatValue used by
credentialPDFGeneratorService.generatePdfForVerifiableCredential.
- Around line 816-844: The test testFormatValueWithLocaleMapList uses locale
maps with the old "language" key which the production code no longer recognizes;
update the localeData entries in this test to use the new key (e.g., "@language"
or "lang") and corresponding values (e.g.,
Map.of("@language","en","value","English Name")) so that
credentialPDFGeneratorService.generatePdfForVerifiableCredential and the
formatValue logic will actually select the expected localized string during the
test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6fa22f27-2042-4a0a-8af7-16877ff0c51f
📒 Files selected for processing (2)
src/main/java/io/mosip/mimoto/service/CredentialPDFGeneratorService.javasrc/test/java/io/mosip/mimoto/service/CredentialPDFGeneratorServiceTest.java
src/main/java/io/mosip/mimoto/service/CredentialPDFGeneratorService.java
Show resolved
Hide resolved
Signed-off-by: Rudhhi Shah <rudhhi.shah@thoughtworks.com>
1 participant
Modified the formatValue function to enable Multilang support
Added the test cases for the same
Summary by CodeRabbit