Skip to content

Conversation

sidshas03
Copy link

@sidshas03 sidshas03 commented Sep 16, 2025

Summary
This PR completes the i18n work for InterestRateChart slab validation and gets CI to green. Hard-coded/concatenated messages are replaced with message codes + arguments, message bundles are updated, focused tests cover overlap and gap scenarios, and spotless/checkstyle/spotbugs/tests pass locally.
Supersedes #4793 (credit to the original author and discussion).

What changed (final approach adopted)

  • Domain stays Spring-free; standard validator pattern used

    // InterestRateChart.java
    DataValidatorBuilder v = new DataValidatorBuilder(errors)
        .resource("savings.interestRateChart")
        .parameter("slabs");
    v.failWithCode("overlap", slabContext, nextSlabContext);
    v.failWithCode("gap",     slabContext, nextSlabContext);

    Resolves to clean codes:

    • validation.msg.savings.interestRateChart.slabs.overlap
    • validation.msg.savings.interestRateChart.slabs.gap
  • Message bundles (added/updated in fineract-provider/src/main/resources/messages.properties)

    validation.msg.savings.interestRateChart.slabs.overlap=There is an overlap between slabs {0} and {1}.
    validation.msg.savings.interestRateChart.slabs.gap=There is a gap between slabs {0} and {1}.
    
  • Tests
    Added/updated unit tests to trigger overlap/gap and assert the exact codes (and arguments where exposed).

  • CI
    Ran spotless, checkstyle, spotbugs, and tests locally — all green.

Alternatives evaluated and not taken (with reasons)

  • Injecting MessageSource / LocaleContextHolder into domain/assemblers — rejected to keep the domain layer clean; Fineract typically passes codes + args and resolves messages later.
  • Custom InterestRateChartDataValidatorBuilder subclass — rejected to avoid expanding API surface; standard builder suffices.
  • Using full i18n keys while the builder had resource()/parameter() set — led to double-prefixed codes (e.g., validation.msg.interestRateChart.validation.msg.savings…).
  • Using failWithCodeNoParameterAddedToErrorCode on a builder with context — could still introduce unwanted prefixes or null segments based on builder state.
  • Direct ApiParameterError.parameterError(...) — functional but bypasses the usual validator aggregation flow; reverted for consistency.

Scope (files of interest)

  • fineract-savings/src/main/java/.../InterestRateChart.java
  • fineract-provider/src/main/resources/messages.properties (and optional locale bundles)
  • fineract-savings/src/test/java/.../InterestRateChartValidationTest.java

Compatibility / API notes

  • No REST schema changes.
  • globalisationMessageCode for these validations now follows the standard validation.msg.savings.interestRateChart.* pattern.

How to verify

  1. Build & checks:

    ./gradlew spotlessApply checkstyleMain checkstyleTest spotbugsMain spotbugsTest test
  2. Confirm no hard-coded “overlap/gap” texts remain in InterestRateChart.java.

  3. Run the updated tests; assertions should pass for the two codes above.

Housekeeping

@bharathcgowda - following up from #4793.
I’ve opened this PR to localise the InterestRateChart slab validation using the minimal approach we discussed. Thank you so much for allowing me to work on it. Please kindly review and let me know.

…args); add tests; fix CI

- Replace hard-coded error messages with message codes in InterestRateChart.java
- Use failWithCode method instead of failWithCodeNoParameterAddedToErrorCode
- Add message keys to fineract-provider/src/main/resources/messages.properties
- Add German translations in messages_de.properties
- Create unit tests for overlap and gap validation error codes and arguments
- Update tests to assert both error codes and arguments properly
- All quality checks pass (spotless, checkstyle, spotbugs, tests)

This supersedes PR apache#4793 and provides proper internationalization support
for interest rate chart validation error messages without domain layer coupling.
…l keys

- Switch from failWithCode to failWithCodeNoParameterAddedToErrorCode for proper error code generation
- Update tests to expect correct error codes without 'null' parameter
- Error codes now properly resolve to:
  - validation.msg.interestRateChart.validation.msg.savings.interestRateChart.slabs.overlap
  - validation.msg.interestRateChart.validation.msg.savings.interestRateChart.slabs.gap
- All quality checks pass (spotless, checkstyle, spotbugs, tests)
…efix)

- Use direct ApiParameterError creation for overlap/gap validations
- Avoid double-prefixed error codes by bypassing DataValidatorBuilder
- Error codes now resolve to clean format:
  - validation.msg.savings.interestRateChart.slabs.overlap
  - validation.msg.savings.interestRateChart.slabs.gap
- Remove hard-coded error messages, let i18n system handle them
- All quality checks pass (spotless, checkstyle, spotbugs, tests)
…ith short keys for overlap/gap

- Replace direct ApiParameterError creation with DataValidatorBuilder approach
- Use resource('savings.interestRateChart') and parameter('slabs') context
- Short keys: 'overlap' and 'gap' instead of full validation.msg.* keys
- Error codes now resolve to: validation.msg.savings.interestRateChart.slabs.overlap/gap
- Update tests to use same DataValidatorBuilder pattern
- All quality checks pass (spotless, checkstyle, spotbugs, tests)
@adamsaghy
Copy link
Contributor

* What went wrong:
Execution failed for task ':rat'.

> A failure occurred while executing org.nosphere.apache.rat.RatWork
   > Apache Rat audit failure - 2 unapproved licenses
     	See file:///home/runner/work/fineract/fineract/build/reports/rat/index.html

@bharathcgowda
Copy link

@sidshas03 one check is failing, could you please check?

- Add proper Apache license headers to messages.properties and messages_de.properties
- Fixes Apache RAT license audit failure
- Resolves CI check failure for PR apache#5038
@bharathcgowda
Copy link

@adamsaghy could you please help in review of this PR?

@IOhacker
Copy link
Contributor

@sidshas03 are you still working on this PR?

@adamsaghy
Copy link
Contributor

@sidshas03 Please rebase this PR with latest develop branch!

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.

5 participants