MX-236: Implement i18N for the messages sent by SMS or Email#134
MX-236: Implement i18N for the messages sent by SMS or Email#134ansh-varshney wants to merge 4 commits intoopenMF:developfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Thymeleaf Maven dependency, introduces Spring MessageSource imports and minor bean/Javadoc formatting, updates test mocks to include a template engine, and introduces duplicate entries (properties keys and an extra closing tag) in resource/template files. No public Java APIs changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
ansh-varshney
changed the title
![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
🤖 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/org/apache/fineract/selfservice/registration/starter/SelfRegistrationConfiguration.java`:
- Around line 52-73: Add Javadoc comments for the two new public bean factory
methods: registrationMessageSource() and registrationTemplateEngine(). For
registrationMessageSource(), add a brief description that it creates and
configures a ResourceBundleMessageSource for i18n (basename "i18n/messages",
UTF-8, no system locale fallback) and document the return type (MessageSource).
For registrationTemplateEngine(), add a brief description that it creates and
configures a SpringTemplateEngine with a ClassLoaderTemplateResolver for mail
HTML templates (prefix "mail-templates/", suffix ".html", HTML mode, UTF-8), and
document the return type (SpringTemplateEngine). Keep descriptions short and
follow existing Javadoc style used in the codebase.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c6d1a5b4-b884-4c53-b59f-02025a33e102
📒 Files selected for processing (6)
pom.xmlsrc/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImpl.javasrc/main/java/org/apache/fineract/selfservice/registration/starter/SelfRegistrationConfiguration.javasrc/main/resources/i18n/messages.propertiessrc/main/resources/i18n/messages_es.propertiessrc/main/resources/mail-templates/authorization-email.html
|
@AnshVarshney there is an issue, check the CI errors |
|
@AnshVarshney https://github.com/openMF/selfservice-plugin/blob/develop/TESTING.md take a look, this will help you out to run tests locally so that you can confidently raise the prs! |
|
Thanks @DeathGun44, will surely look at it! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImplTest.java (1)
67-89: Add behavioral tests for i18n/template usage, not only constructor wiring.The new mocks are injected, but no test verifies
registrationMessageSource.getMessage(...)orregistrationTemplateEngine.process(...). This leaves the MX-236 behavior unguarded.Suggested test extension
import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ +import org.thymeleaf.context.Context; @@ `@Test` + void createRegistrationRequest_emailMode_usesLocalizedSubjectAndTemplate() { + when(fromApiJsonHelper.extractStringNamed(eq(SelfServiceApiConstants.accountNumberParamName), any())).thenReturn("12345"); + when(fromApiJsonHelper.extractStringNamed(eq(SelfServiceApiConstants.firstNameParamName), any())).thenReturn("John"); + when(fromApiJsonHelper.extractStringNamed(eq(SelfServiceApiConstants.middleNameParamName), any())).thenReturn(null); + when(fromApiJsonHelper.extractStringNamed(eq(SelfServiceApiConstants.lastNameParamName), any())).thenReturn("Doe"); + when(fromApiJsonHelper.extractStringNamed(eq(SelfServiceApiConstants.usernameParamName), any())).thenReturn("jdoe"); + when(fromApiJsonHelper.extractStringNamed(eq(SelfServiceApiConstants.passwordParamName), any())).thenReturn("Password123!"); + when(fromApiJsonHelper.extractStringNamed(eq(SelfServiceApiConstants.authenticationModeParamName), any())).thenReturn("email"); + when(fromApiJsonHelper.extractStringNamed(eq(SelfServiceApiConstants.emailParamName), any())).thenReturn("john@test.com"); + + PasswordValidationPolicy policy = mock(PasswordValidationPolicy.class); + when(policy.getRegex()).thenReturn(".*"); + when(passwordValidationPolicyRepository.findActivePasswordValidationPolicy()).thenReturn(policy); + when(appUserReadPlatformService.isUsernameExist("jdoe")).thenReturn(false); + when(selfServiceRegistrationReadPlatformService.isClientExist(anyString(), anyString(), any(), anyString(), any(), anyBoolean())) + .thenReturn(true); + when(clientRepository.getClientByAccountNumber("12345")).thenReturn(mock(Client.class)); + + when(registrationMessageSource.getMessage(eq("email.subject"), eq(null), any())).thenReturn("Authorization"); + when(registrationTemplateEngine.process(eq("authorization-email"), any(Context.class))).thenReturn("<html/>"); + + service.createRegistrationRequest("{}"); + + verify(registrationMessageSource).getMessage(eq("email.subject"), eq(null), any()); + verify(registrationTemplateEngine).process(eq("authorization-email"), any(Context.class)); + } + + `@Test` void createRegistrationRequest_persistsRegistration() {As per coding guidelines, "Verify both happy path and edge cases."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImplTest.java` around lines 67 - 89, Add behavioral tests in SelfServiceRegistrationWritePlatformServiceImplTest that stub and verify internationalization and template usage: mock registrationMessageSource.getMessage(...) to return a known localized string and assert the service (use the service instance created in setUp) consumes it, and mock registrationTemplateEngine.process(...) to return rendered HTML and verify the service calls it with expected template name/variables. Include both a happy-path test (valid inputs leading to message/template invocation and expected result) and an edge-case test (missing message/template or null return values) asserting graceful handling or exceptions. Use Mockito when(...).thenReturn(...) plus verify(registrationMessageSource).getMessage(...) and verify(registrationTemplateEngine).process(...) to ensure coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImplTest.java`:
- Around line 67-89: Add behavioral tests in
SelfServiceRegistrationWritePlatformServiceImplTest that stub and verify
internationalization and template usage: mock
registrationMessageSource.getMessage(...) to return a known localized string and
assert the service (use the service instance created in setUp) consumes it, and
mock registrationTemplateEngine.process(...) to return rendered HTML and verify
the service calls it with expected template name/variables. Include both a
happy-path test (valid inputs leading to message/template invocation and
expected result) and an edge-case test (missing message/template or null return
values) asserting graceful handling or exceptions. Use Mockito
when(...).thenReturn(...) plus verify(registrationMessageSource).getMessage(...)
and verify(registrationTemplateEngine).process(...) to ensure coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea5d5a0f-2bb2-4b75-8bc0-c52b95c8afe0
📒 Files selected for processing (1)
src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImplTest.java
|
It appears that the local compilation failures are pre-existing on develop and caused by missing fineract-*:1.15.0-SNAPSHOT JARs in the local Maven cache. I have checked all 3 test files covering the changed classes in this pr and it appeared that only CI failure introduced by this pr was constructor mismatch in SelfServiceRegistrationWritePlatformServiceImplTest. I have fixed this now. |
|
@ansh-varshney help me to resolve the conflicts too |
Yes, I am on it. I'll provide a brief explanation about what should de done acc to me for conflict resoltuion. |
|
@IOhacker, |
|
@IOhacker, |
|
@IOhacker, same for the third file also - same pattern only. lets keep both. |
|
Hi Ansh, could you please rebase? |
ansh-varshney
force-pushed
the
MX236-Implement-i18n-for-email-SMS
branch
2 times, most recently
from
4ab4c91 to
82b67fa
Compare
Yes, I have resolved all conflicts now. |
|
@ansh-vashney there is a failing Github action, could you please check? |
|
@ansh-varshney the issue was about using a concrete class and not an interface |
Yes, ill quickly debug, fix and then commit again. |
…ssages Replace hardcoded Spanish strings in sendAuthorizationMail() and sendAuthorizationMessage() with Thymeleaf HTML templates and ResourceBundleMessageSource.
…istrationTemplateEngine beans
…and messageSource mocks
|
Wait, I have taken your changes, fixed it and now I am sending a change
about the location of the templates and the locale.
El lun, 13 abr 2026 a las 23:00, Ansh Varshney ***@***.***>)
escribió:
… *ansh-varshney* left a comment (openMF/selfservice-plugin#134)
<#134 (comment)>
@ansh-varshney <https://github.com/ansh-varshney> the issue was about
using a concrete class and not an interface
Yes, ill quickly debug, fix and then commit again.
—
Reply to this email directly, view it on GitHub
<#134 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALD2ZAQQ7RMC4NN6LRCZABT4VXAXTAVCNFSM6AAAAACXWDQTKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DENBRGM4TQMBSGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
ansh-varshney
force-pushed
the
MX236-Implement-i18n-for-email-SMS
branch
from
82b67fa to
b22d0f1
Compare
Oh sorry, I didn't see the message. already pushed after proper local testing (acc. to testing,md) to ensure ci passes. rebased the new changes also. Can you please check? |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/org/apache/fineract/selfservice/registration/starter/SelfRegistrationConfiguration.java (1)
61-65:⚠️ Potential issue | 🟡 MinorCorrect the misplaced Javadoc for
selfServiceAuthorizationTokenService().The block at Line 61–65 documents a message-source bean, but it is attached to
selfServiceAuthorizationTokenService(Environment env)at Line 68. This leaves method docs inconsistent.Proposed fix
- /** - * Provides localized message bundles for registration notifications. - * - * `@return` configured registration message source - */ + /** + * Provides authorization token service used by self-service registration flows. + * + * `@return` configured authorization token service + */ `@Bean` `@ConditionalOnMissingBean`(SelfServiceAuthorizationTokenService.class) public SelfServiceAuthorizationTokenService selfServiceAuthorizationTokenService(Environment env) { return new SelfServiceAuthorizationTokenService(env); } + /** + * Provides localized message bundles for registration notifications. + * + * `@return` configured registration message source + */ `@Bean` public MessageSource registrationMessageSource() {As per coding guidelines, "Public methods and classes MUST have Javadoc."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/apache/fineract/selfservice/registration/starter/SelfRegistrationConfiguration.java` around lines 61 - 65, The Javadoc block that describes the registration message source is misplaced above the method selfServiceAuthorizationTokenService(Environment env); move that message-source Javadoc to the actual MessageSource bean method (e.g., messageSource() or the method that returns the registration message source) and add a proper Javadoc for the public method selfServiceAuthorizationTokenService(Environment env) describing its purpose and parameters; ensure both public methods (the message source bean and selfServiceAuthorizationTokenService) have appropriate Javadoc headers per coding guidelines with brief descriptions and `@param/`@return where applicable.
🤖 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/resources/mail-templates/authorization-email.html`:
- Around line 19-20: The template contains a duplicate closing </html> tag
causing tag-pair validation failure; remove the extra unmatched </html> so only
a single closing </html> remains at the end of the authorization-email.html
template (delete the redundant </html> on the final line).
---
Duplicate comments:
In
`@src/main/java/org/apache/fineract/selfservice/registration/starter/SelfRegistrationConfiguration.java`:
- Around line 61-65: The Javadoc block that describes the registration message
source is misplaced above the method
selfServiceAuthorizationTokenService(Environment env); move that message-source
Javadoc to the actual MessageSource bean method (e.g., messageSource() or the
method that returns the registration message source) and add a proper Javadoc
for the public method selfServiceAuthorizationTokenService(Environment env)
describing its purpose and parameters; ensure both public methods (the message
source bean and selfServiceAuthorizationTokenService) have appropriate Javadoc
headers per coding guidelines with brief descriptions and `@param/`@return where
applicable.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 98e9da34-4853-458d-bdb1-162dc57c8343
📒 Files selected for processing (7)
pom.xmlsrc/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImpl.javasrc/main/java/org/apache/fineract/selfservice/registration/starter/SelfRegistrationConfiguration.javasrc/main/resources/i18n/messages.propertiessrc/main/resources/i18n/messages_es.propertiessrc/main/resources/mail-templates/authorization-email.htmlsrc/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImplTest.java
✅ Files skipped from review due to trivial changes (2)
- src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImplTest.java
- src/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImpl.java
🚧 Files skipped from review as they are similar to previous changes (3)
- pom.xml
- src/main/resources/i18n/messages_es.properties
- src/main/resources/i18n/messages.properties
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Hi @IOhacker, if you have incorporated the changes, should i close the pr? |
3 participants
Fixes: MX-236
Replace hardcoded Spanish strings in sendAuthorizationMail() and sendAuthorizationMessage() with Thymeleaf HTML templates and ResourceBundleMessageSource.
Summary by CodeRabbit
New Features
Chores
Tests