You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "budaidev (via GitHub)" <gi...@apache.org> on 2026/04/15 09:40:24 UTC

[PR] FINERACT-2455: WorkingCapital - % repayment modification options in t… [fineract]

budaidev opened a new pull request, #5771:
URL: https://github.com/apache/fineract/pull/5771

   …he middle of the loan life cycle
   
   ## Description
   
   Describe the changes made and why they were made. (Ignore if these details are present on the associated Apache Fineract JIRA ticket.)
   
   ## Checklist
   
   Please make sure these boxes are checked before submitting your pull request - thanks!
   
   - [ ] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests)
   - [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
   - [ ] Create/update [unit or integration tests](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made.
   - [ ] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions).
   - [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
   - [ ] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute).
   
   Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3170441614


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanWritePlatformServiceImpl.java:
##########
@@ -577,6 +580,67 @@ public CommandProcessingResult creditBalanceRefund(final Long loanId, final Json
                 .withClientId(loan.getClientId()).withLoanId(loanId).with(changes).build();
     }
 
+    @Override
+    @Transactional
+    public CommandProcessingResult updateRate(final Long loanId, final JsonCommand command) {
+        final WorkingCapitalLoan loan = this.loanRepository.findById(loanId)
+                .orElseThrow(() -> new WorkingCapitalLoanNotFoundException(loanId));
+        this.validator.validateUpdateRate(command.json(), loan);
+
+        if (loan.getLoanStatus() != LoanStatus.ACTIVE) {
+            throw new PlatformApiDataValidationException("validation.msg.wc.loan.rate.change.not.allowed",
+                    "Period payment rate change is allowed only for active loans", "loanStatus");
+        }
+
+        final BigDecimal newRate = this.fromApiJsonHelper.extractBigDecimalNamed(WorkingCapitalLoanConstants.periodPaymentRateParamName,
+                command.parsedJson(), new HashSet<>());
+        final BigDecimal previousRate = loan.getLoanProductRelatedDetails().getPeriodPaymentRate();
+
+        if (previousRate != null && previousRate.compareTo(newRate) == 0) {
+            throw new PlatformApiDataValidationException("validation.msg.wc.loan.rate.change.same.rate",
+                    "New period payment rate is the same as the current rate", WorkingCapitalLoanConstants.periodPaymentRateParamName);
+        }

Review Comment:
   These should be in the validator class, no?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3225001431


##########
fineract-working-capital-loan/src/main/resources/db/changelog/tenant/module/workingcapitalloan/parts/0036_wc_loan_period_payment_rate_change.xml:
##########
@@ -0,0 +1,135 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements. See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership. The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License. You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied. See the License for the
+    specific language governing permissions and limitations
+    under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.3.xsd">
+
+    <changeSet author="fineract" id="wcl-0036-1-create-rate-change-table">
+        <preConditions onFail="MARK_RAN">
+            <not>
+                <tableExists tableName="m_wc_loan_period_payment_rate_change"/>
+            </not>
+        </preConditions>
+        <createTable tableName="m_wc_loan_period_payment_rate_change">
+            <column autoIncrement="true" name="id" type="BIGINT">
+                <constraints nullable="false" primaryKey="true"/>
+            </column>
+            <column name="wc_loan_id" type="BIGINT">
+                <constraints nullable="false"/>
+            </column>
+            <column name="effective_date" type="DATE">
+                <constraints nullable="false"/>
+            </column>
+            <column name="previous_rate" type="DECIMAL(19,6)">
+                <constraints nullable="false"/>
+            </column>
+            <column name="new_rate" type="DECIMAL(19,6)">
+                <constraints nullable="false"/>
+            </column>
+            <column name="is_reversed" type="BOOLEAN" defaultValueBoolean="false">
+                <constraints nullable="false"/>
+            </column>
+            <column name="reversed_on_date" type="DATE"/>
+            <column name="created_by" type="BIGINT">
+                <constraints nullable="false"/>
+            </column>
+            <column name="last_modified_by" type="BIGINT">
+                <constraints nullable="false"/>
+            </column>
+            <column name="version" type="INT" defaultValueNumeric="0">
+                <constraints nullable="false"/>
+            </column>
+        </createTable>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-2-audit-columns-my" context="mysql">
+        <addColumn tableName="m_wc_loan_period_payment_rate_change">
+            <column name="created_on_utc" type="DATETIME(6)">
+                <constraints nullable="false"/>
+            </column>
+            <column name="last_modified_on_utc" type="DATETIME(6)">
+                <constraints nullable="false"/>
+            </column>
+        </addColumn>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-2-audit-columns-pg" context="postgresql">
+        <addColumn tableName="m_wc_loan_period_payment_rate_change">
+            <column name="created_on_utc" type="TIMESTAMP WITH TIME ZONE">
+                <constraints nullable="false"/>
+            </column>
+            <column name="last_modified_on_utc" type="TIMESTAMP WITH TIME ZONE">
+                <constraints nullable="false"/>
+            </column>
+        </addColumn>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-3-rate-change-fks">
+        <addForeignKeyConstraint baseColumnNames="wc_loan_id" baseTableName="m_wc_loan_period_payment_rate_change"
+                                 constraintName="fk_wc_rate_change_loan" referencedColumnNames="id"
+                                 referencedTableName="m_wc_loan"/>
+        <addForeignKeyConstraint baseColumnNames="created_by" baseTableName="m_wc_loan_period_payment_rate_change"
+                                 constraintName="fk_wc_rate_change_created_by" referencedColumnNames="id"
+                                 referencedTableName="m_appuser"/>
+        <addForeignKeyConstraint baseColumnNames="last_modified_by" baseTableName="m_wc_loan_period_payment_rate_change"
+                                 constraintName="fk_wc_rate_change_last_modified_by" referencedColumnNames="id"
+                                 referencedTableName="m_appuser"/>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-4-rate-change-index">
+        <createIndex tableName="m_wc_loan_period_payment_rate_change" indexName="idx_wc_rate_change_loan_date">
+            <column name="wc_loan_id"/>
+            <column name="effective_date"/>
+        </createIndex>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-6-update-rate-permission">
+        <preConditions onFail="MARK_RAN">
+            <sqlCheck expectedResult="0">
+                SELECT COUNT(*) FROM m_permission WHERE code = 'UPDATERATE_WORKINGCAPITALLOAN';
+            </sqlCheck>
+        </preConditions>
+        <insert tableName="m_permission">
+            <column name="grouping" value="portfolio"/>
+            <column name="code" value="UPDATERATE_WORKINGCAPITALLOAN"/>
+            <column name="entity_name" value="WORKINGCAPITALLOAN"/>
+            <column name="action_name" value="UPDATERATE"/>
+            <column name="can_maker_checker" valueBoolean="false"/>
+        </insert>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-7-undo-rate-change-permission">

Review Comment:
   it's a future plan, but no implementation behind it. Dropped



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #5771:
URL: https://github.com/apache/fineract/pull/5771


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3153769678


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResource.java:
##########
@@ -346,4 +349,110 @@ private CommandProcessingResult updateDiscount(final Long loanId, final String l
                 .updateDiscountWorkingCapitalLoanApplication(resolvedLoanId).build();
         return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
     }
+
+    @PUT
+    @Path("{loanId}/rate")

Review Comment:
   What is the difference between loadId and loanExternalId?
   
   I am assuming loanExternalId is for external Systems.
   
   (Please Resolve this comment after replying)
   



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResource.java:
##########
@@ -346,4 +349,110 @@ private CommandProcessingResult updateDiscount(final Long loanId, final String l
                 .updateDiscountWorkingCapitalLoanApplication(resolvedLoanId).build();
         return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
     }
+
+    @PUT
+    @Path("{loanId}/rate")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "updateWorkingCapitalLoanRateById", summary = "Update period payment rate for an active Working Capital Loan", description = "Modifies the period payment rate and triggers schedule recalculation for the remaining term.")
+    @RequestBody(required = true, content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PutWorkingCapitalLoansLoanIdRateRequest.class)))
+    @ApiResponses({

Review Comment:
   ApiResponses doesnt provide any value in newer swagger dependecies and aften recommended to be removed by sonarqube



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResource.java:
##########
@@ -346,4 +349,110 @@ private CommandProcessingResult updateDiscount(final Long loanId, final String l
                 .updateDiscountWorkingCapitalLoanApplication(resolvedLoanId).build();
         return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
     }
+
+    @PUT
+    @Path("{loanId}/rate")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "updateWorkingCapitalLoanRateById", summary = "Update period payment rate for an active Working Capital Loan", description = "Modifies the period payment rate and triggers schedule recalculation for the remaining term.")
+    @RequestBody(required = true, content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PutWorkingCapitalLoansLoanIdRateRequest.class)))
+    @ApiResponses({
+            @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = CommandProcessingResult.class))) })
+    public CommandProcessingResult updateRateById(
+            @PathParam("loanId") @Parameter(description = "loanId", required = true) final Long loanId,
+            @Parameter(hidden = true) final String apiRequestBodyAsJson) {
+        return updateRate(loanId, null, apiRequestBodyAsJson);
+    }
+
+    @PUT
+    @Path("external-id/{loanExternalId}/rate")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "updateWorkingCapitalLoanRateByExternalId", summary = "Update period payment rate for an active Working Capital Loan by external id", description = "Modifies the period payment rate and triggers schedule recalculation for the remaining term.")
+    @RequestBody(required = true, content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PutWorkingCapitalLoansLoanIdRateRequest.class)))
+    @ApiResponses({
+            @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = CommandProcessingResult.class))) })
+    public CommandProcessingResult updateRateByExternalId(
+            @PathParam("loanExternalId") @Parameter(description = "loanExternalId", required = true) final String loanExternalId,
+            @Parameter(hidden = true) final String apiRequestBodyAsJson) {
+        return updateRate(null, loanExternalId, apiRequestBodyAsJson);
+    }
+
+    private CommandProcessingResult updateRate(final Long loanId, final String loanExternalIdStr, final String apiRequestBodyAsJson) {
+        final Long resolvedLoanId = loanId != null ? loanId
+                : readPlatformService.getResolvedLoanId(ExternalIdFactory.produce(loanExternalIdStr));
+        if (resolvedLoanId == null) {
+            throw new WorkingCapitalLoanNotFoundException(ExternalIdFactory.produce(loanExternalIdStr));
+        }
+        final CommandWrapper commandRequest = new CommandWrapperBuilder().withJson(apiRequestBodyAsJson)
+                .updateRateWorkingCapitalLoanApplication(resolvedLoanId).build();
+        return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
+    }
+
+    @POST
+    @Path("{loanId}/rate/undo")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "undoWorkingCapitalLoanRateChangeById", summary = "Undo the last period payment rate change", description = "Reverses the most recent non-reversed rate change and restores the previous amortization model.")
+    @RequestBody(content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PostWorkingCapitalLoansLoanIdRateUndoRequest.class)))
+    @ApiResponses({

Review Comment:
   same as above



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResource.java:
##########
@@ -346,4 +349,110 @@ private CommandProcessingResult updateDiscount(final Long loanId, final String l
                 .updateDiscountWorkingCapitalLoanApplication(resolvedLoanId).build();
         return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
     }
+
+    @PUT
+    @Path("{loanId}/rate")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "updateWorkingCapitalLoanRateById", summary = "Update period payment rate for an active Working Capital Loan", description = "Modifies the period payment rate and triggers schedule recalculation for the remaining term.")
+    @RequestBody(required = true, content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PutWorkingCapitalLoansLoanIdRateRequest.class)))
+    @ApiResponses({
+            @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = CommandProcessingResult.class))) })
+    public CommandProcessingResult updateRateById(
+            @PathParam("loanId") @Parameter(description = "loanId", required = true) final Long loanId,
+            @Parameter(hidden = true) final String apiRequestBodyAsJson) {
+        return updateRate(loanId, null, apiRequestBodyAsJson);
+    }
+
+    @PUT
+    @Path("external-id/{loanExternalId}/rate")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "updateWorkingCapitalLoanRateByExternalId", summary = "Update period payment rate for an active Working Capital Loan by external id", description = "Modifies the period payment rate and triggers schedule recalculation for the remaining term.")
+    @RequestBody(required = true, content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PutWorkingCapitalLoansLoanIdRateRequest.class)))
+    @ApiResponses({

Review Comment:
   Same as above



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResource.java:
##########
@@ -346,4 +349,110 @@ private CommandProcessingResult updateDiscount(final Long loanId, final String l
                 .updateDiscountWorkingCapitalLoanApplication(resolvedLoanId).build();
         return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
     }
+
+    @PUT
+    @Path("{loanId}/rate")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "updateWorkingCapitalLoanRateById", summary = "Update period payment rate for an active Working Capital Loan", description = "Modifies the period payment rate and triggers schedule recalculation for the remaining term.")
+    @RequestBody(required = true, content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PutWorkingCapitalLoansLoanIdRateRequest.class)))
+    @ApiResponses({
+            @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = CommandProcessingResult.class))) })
+    public CommandProcessingResult updateRateById(
+            @PathParam("loanId") @Parameter(description = "loanId", required = true) final Long loanId,
+            @Parameter(hidden = true) final String apiRequestBodyAsJson) {
+        return updateRate(loanId, null, apiRequestBodyAsJson);
+    }
+
+    @PUT
+    @Path("external-id/{loanExternalId}/rate")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "updateWorkingCapitalLoanRateByExternalId", summary = "Update period payment rate for an active Working Capital Loan by external id", description = "Modifies the period payment rate and triggers schedule recalculation for the remaining term.")
+    @RequestBody(required = true, content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PutWorkingCapitalLoansLoanIdRateRequest.class)))
+    @ApiResponses({
+            @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = CommandProcessingResult.class))) })
+    public CommandProcessingResult updateRateByExternalId(
+            @PathParam("loanExternalId") @Parameter(description = "loanExternalId", required = true) final String loanExternalId,
+            @Parameter(hidden = true) final String apiRequestBodyAsJson) {
+        return updateRate(null, loanExternalId, apiRequestBodyAsJson);
+    }
+
+    private CommandProcessingResult updateRate(final Long loanId, final String loanExternalIdStr, final String apiRequestBodyAsJson) {
+        final Long resolvedLoanId = loanId != null ? loanId
+                : readPlatformService.getResolvedLoanId(ExternalIdFactory.produce(loanExternalIdStr));
+        if (resolvedLoanId == null) {
+            throw new WorkingCapitalLoanNotFoundException(ExternalIdFactory.produce(loanExternalIdStr));
+        }
+        final CommandWrapper commandRequest = new CommandWrapperBuilder().withJson(apiRequestBodyAsJson)
+                .updateRateWorkingCapitalLoanApplication(resolvedLoanId).build();
+        return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
+    }
+
+    @POST
+    @Path("{loanId}/rate/undo")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "undoWorkingCapitalLoanRateChangeById", summary = "Undo the last period payment rate change", description = "Reverses the most recent non-reversed rate change and restores the previous amortization model.")
+    @RequestBody(content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PostWorkingCapitalLoansLoanIdRateUndoRequest.class)))
+    @ApiResponses({
+            @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = CommandProcessingResult.class))) })
+    public CommandProcessingResult undoRateChangeById(
+            @PathParam("loanId") @Parameter(description = "loanId", required = true) final Long loanId,
+            @Parameter(hidden = true) final String apiRequestBodyAsJson) {
+        return undoRateChange(loanId, null, apiRequestBodyAsJson);
+    }
+
+    @POST
+    @Path("external-id/{loanExternalId}/rate/undo")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "undoWorkingCapitalLoanRateChangeByExternalId", summary = "Undo the last period payment rate change by external id", description = "Reverses the most recent non-reversed rate change and restores the previous amortization model.")
+    @RequestBody(content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PostWorkingCapitalLoansLoanIdRateUndoRequest.class)))
+    @ApiResponses({
+            @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = CommandProcessingResult.class))) })
+    public CommandProcessingResult undoRateChangeByExternalId(
+            @PathParam("loanExternalId") @Parameter(description = "loanExternalId", required = true) final String loanExternalId,
+            @Parameter(hidden = true) final String apiRequestBodyAsJson) {
+        return undoRateChange(null, loanExternalId, apiRequestBodyAsJson);
+    }
+
+    private CommandProcessingResult undoRateChange(final Long loanId, final String loanExternalIdStr, final String apiRequestBodyAsJson) {
+        final Long resolvedLoanId = loanId != null ? loanId
+                : readPlatformService.getResolvedLoanId(ExternalIdFactory.produce(loanExternalIdStr));
+        if (resolvedLoanId == null) {
+            throw new WorkingCapitalLoanNotFoundException(ExternalIdFactory.produce(loanExternalIdStr));
+        }
+        final CommandWrapper commandRequest = new CommandWrapperBuilder().withJson(apiRequestBodyAsJson)
+                .undoRateChangeWorkingCapitalLoanApplication(resolvedLoanId).build();
+        return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
+    }
+
+    @GET
+    @Path("{loanId}/rate-changes")
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "getWorkingCapitalLoanRateChangeHistoryById", summary = "Retrieve rate change history for a Working Capital Loan", description = "Returns all rate change records for the loan, ordered by most recent first.")
+    @ApiResponses({

Review Comment:
   same as above



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3170396500


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResource.java:
##########
@@ -346,4 +349,63 @@ private CommandProcessingResult updateDiscount(final Long loanId, final String l
                 .updateDiscountWorkingCapitalLoanApplication(resolvedLoanId).build();
         return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
     }
+
+    @PUT
+    @Path("{loanId}/rate")

Review Comment:
   "rate" as endpoint is not descriptive, lets rename: `payment-rate`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3225215875


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanAmortizationScheduleWriteServiceImpl.java:
##########
@@ -180,21 +183,56 @@ public RepaymentAmortizationData applyRepayment(final WorkingCapitalLoan loan, f
     private BigDecimal sumRunningNpv(final ProjectedAmortizationScheduleModel model) {
         final MathContext mc = MoneyHelper.getMathContext();
         BigDecimal result = BigDecimal.ZERO;
-        for (ProjectedPayment payment : model.payments()) {
+        for (ProjectedPayment payment : model.projectedPayments()) {
             if (payment.paymentNo() > 0 && payment.npvValue() != null && payment.npvValue().getAmount() != null) {
                 result = result.add(payment.npvValue().getAmount(), mc);
             }
         }
         return result;
     }
 
-    private MonetaryCurrency resolveCurrency(final WorkingCapitalLoan loan) {
+    @Override
+    public void regenerateAmortizationScheduleOnRateChange(final WorkingCapitalLoan loan, final BigDecimal newRate) {
+        Validate.notNull(loan, "loan must not be null");
+        Validate.notNull(newRate, "newRate must not be null");
+
+        final MathContext mc = MoneyHelper.getMathContext();
+        final CurrencyData currency = resolveCurrency(loan);
+        final ProjectedAmortizationScheduleModel model = scheduleRepositoryWrapper.readModel(loan.getId(), mc, currency)
+                .orElseThrow(() -> new IllegalStateException("Projected amortization schedule is not found for loan " + loan.getId()));
+
+        final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+        final LocalDate loanDisbursementDate = resolveLoanDisbursementDate(loan);
+        final int splitDayIndex = (int) ChronoUnit.DAYS.between(loanDisbursementDate, businessDate);
+        final LocalDate modelRateChangeDate = model.expectedDisbursementDate().plusDays(splitDayIndex);
+
+        model.clearLastRateSegment();
+
+        calculator.applyRateChange(model, newRate, modelRateChangeDate);
+
+        scheduleRepositoryWrapper.writeModel(loan, model);
+    }
+
+    private LocalDate resolveLoanDisbursementDate(final WorkingCapitalLoan loan) {
+        if (loan.getDisbursementDetails() != null && !loan.getDisbursementDetails().isEmpty()) {
+            final WorkingCapitalLoanDisbursementDetails detail = loan.getDisbursementDetails().getFirst();
+            if (detail.getActualDisbursementDate() != null) {
+                return detail.getActualDisbursementDate();
+            }
+            if (detail.getExpectedDisbursementDate() != null) {
+                return detail.getExpectedDisbursementDate();

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3225009322


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/serialization/WorkingCapitalLoanDataValidator.java:
##########
@@ -95,6 +96,9 @@ public class WorkingCapitalLoanDataValidator {
                     WorkingCapitalLoanConstants.noteParamName, WorkingCapitalLoanConstants.transactionDateParamName));
     private static final Set<String> CREDIT_BALANCE_REFUND_SUPPORTED_PARAMETERS = new HashSet<>(REPAYMENT_SUPPORTED_PARAMETERS);
 
+    private static final Set<String> UPDATE_RATE_SUPPORTED_PARAMETERS = new HashSet<>(
+            Arrays.asList("locale", WorkingCapitalLoanConstants.periodPaymentRateParamName, WorkingCapitalLoanConstants.noteParamName));

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3225014858


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/serialization/WorkingCapitalLoanDataValidator.java:
##########
@@ -616,6 +620,58 @@ public void validateCreditBalanceRefund(final String json, final WorkingCapitalL
         throwExceptionIfValidationWarningsExist(dataValidationErrors);
     }
 
+    public void validateUpdatePeriodPaymentRate(final String json, final WorkingCapitalLoan loan) {

Review Comment:
   added new unit tests



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5771:
URL: https://github.com/apache/fineract/pull/5771#issuecomment-4344625714

   @budaidev Please rebase.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3170423782


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/calc/ProjectedAmortizationScheduleModel.java:
##########
@@ -247,6 +270,98 @@ public void recalculateNetAmortizationAndDeferredBalanceFrom(final LocalDate rep
         this.payments = List.copyOf(adjusted);
     }
 
+    /**
+     * Applies a rate change at the given date. Adds a {@link RateSegment} covering the remaining term from the change
+     * date forward. The model is mutated in-place; the payment list is rebuilt.
+     *
+     * <p>
+     * Any existing segments at or after the split point are removed first (supports undo/overwrite).
+     *
+     * @param newPeriodPaymentRate
+     *            the new period payment rate
+     * @param rateChangeDate
+     *            the date of the rate change (must be within model's date range)
+     */
+    public void applyRateChange(final BigDecimal newPeriodPaymentRate, final LocalDate rateChangeDate) {
+        Objects.requireNonNull(newPeriodPaymentRate, "newPeriodPaymentRate");
+        Objects.requireNonNull(rateChangeDate, "rateChangeDate");
+
+        final int rawSplitDayIndex = (int) ChronoUnit.DAYS.between(expectedDisbursementDate, rateChangeDate);
+        if (rawSplitDayIndex < 0) {
+            throw new IllegalArgumentException("rateChangeDate must not be before expectedDisbursementDate");
+        }
+
+        // When the rate change is past the base schedule's term, clamp the segment start
+        // to loanTerm. The loan is still active (borrower hasn't paid), so the remaining
+        // balance is netDisbursement - paymentsReceived.
+        final int splitDayIndex = Math.min(rawSplitDayIndex, loanTerm);
+
+        // Remove existing segments at or after split (supports overwrite on second rate change)
+        // Guard against null rateSegments from V1 model deserialization
+        if (rateSegments == null) {
+            throw new IllegalStateException("Model not properly initialized; rateSegments is null");
+        }
+        rateSegments.removeIf(s -> s.startDayIndex() >= splitDayIndex);
+
+        // Collect actual payments received before the split
+        BigDecimal paymentsReceived = BigDecimal.ZERO;
+        for (final ProjectedPayment p : payments) {
+            if (p.paymentNo() <= 0 || p.paymentNo() > splitDayIndex) {
+                continue;
+            }
+            if (p.actualPaymentAmount() != null) {
+                paymentsReceived = paymentsReceived.add(p.actualPaymentAmount().getAmount(), mc);
+            }
+        }
+
+        // Compute balance at split: if past term, use remaining principal; otherwise use base amortization
+        final BigDecimal balanceAtSplit;
+        if (rawSplitDayIndex >= loanTerm) {
+            balanceAtSplit = netDisbursementAmount.getAmount().subtract(paymentsReceived, mc);
+        } else if (splitDayIndex > 0) {
+            final BalancesAndAmortizations ba = computeBaseBalancesUpTo(splitDayIndex);
+            balanceAtSplit = ba.balances().get(splitDayIndex - 1);
+        } else {
+            balanceAtSplit = netDisbursementAmount.getAmount();
+        }
+
+        final BigDecimal origNet = netDisbursementAmount.getAmount();
+        final BigDecimal origDiscount = originationFeeAmount.getAmount();
+        final BigDecimal tpv = totalPaymentValue.getAmount();
+
+        final BigDecimal newNetDisb = balanceAtSplit;
+        final BigDecimal newDiscount = origDiscount.add(origNet, mc).subtract(balanceAtSplit, mc).subtract(paymentsReceived, mc);
+        final BigDecimal newDailyPayment = tpv.multiply(newPeriodPaymentRate, mc).divide(BigDecimal.valueOf(npvDayCount), mc).setScale(2,
+                RoundingMode.HALF_UP);

Review Comment:
   Is it wise to hardcode rounding? I think we should rather rely on the configured rounding rules instead!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3187578868


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/calc/ProjectedAmortizationScheduleModel.java:
##########
@@ -247,6 +270,99 @@ public void recalculateNetAmortizationAndDeferredBalanceFrom(final LocalDate rep
         this.payments = List.copyOf(adjusted);
     }
 
+    /**
+     * Applies a rate change at the given date. Adds a {@link RateSegment} covering the remaining term from the change
+     * date forward. The model is mutated in-place; the payment list is rebuilt.
+     *
+     * <p>
+     * Any existing segments at or after the split point are removed first (supports undo/overwrite).
+     *
+     * @param newPeriodPaymentRate
+     *            the new period payment rate
+     * @param rateChangeDate
+     *            the date of the rate change (must be within model's date range)
+     */
+    public void applyRateChange(final BigDecimal newPeriodPaymentRate, final LocalDate rateChangeDate) {
+        Objects.requireNonNull(newPeriodPaymentRate, "newPeriodPaymentRate");
+        Objects.requireNonNull(rateChangeDate, "rateChangeDate");
+
+        final int rawSplitDayIndex = (int) ChronoUnit.DAYS.between(expectedDisbursementDate, rateChangeDate);
+        if (rawSplitDayIndex < 0) {
+            throw new IllegalArgumentException("rateChangeDate must not be before expectedDisbursementDate");
+        }
+
+        // When the rate change is past the base schedule's term, clamp the segment start
+        // to loanTerm. The loan is still active (borrower hasn't paid), so the remaining
+        // balance is netDisbursement - paymentsReceived.
+        final int splitDayIndex = Math.min(rawSplitDayIndex, loanTerm);
+
+        // Remove existing segments at or after split (supports overwrite on second rate change)
+        // Guard against null rateSegments from V1 model deserialization
+        if (rateSegments == null) {
+            throw new IllegalStateException("Model not properly initialized; rateSegments is null");
+        }
+        rateSegments.removeIf(s -> s.startDayIndex() >= splitDayIndex);
+
+        // Collect actual payments received before the split
+        BigDecimal paymentsReceived = BigDecimal.ZERO;
+        for (final ProjectedPayment p : payments) {
+            if (p.paymentNo() <= 0 || p.paymentNo() > splitDayIndex) {
+                continue;
+            }
+            if (p.actualPaymentAmount() != null) {
+                paymentsReceived = paymentsReceived.add(p.actualPaymentAmount().getAmount(), mc);
+            }
+        }
+
+        // Compute balance at split: if past term, use remaining principal; otherwise use base amortization
+        final BigDecimal balanceAtSplit;
+        if (rawSplitDayIndex >= loanTerm) {
+            balanceAtSplit = netDisbursementAmount.getAmount().subtract(paymentsReceived, mc);
+        } else if (splitDayIndex > 0) {
+            final BalancesAndAmortizations ba = computeBaseBalancesUpTo(splitDayIndex);
+            balanceAtSplit = ba.balances().get(splitDayIndex - 1);
+        } else {
+            balanceAtSplit = netDisbursementAmount.getAmount();
+        }
+
+        final BigDecimal origNet = netDisbursementAmount.getAmount();
+        final BigDecimal origDiscount = originationFeeAmount.getAmount();
+        final BigDecimal tpv = totalPaymentValue.getAmount();
+
+        final BigDecimal newNetDisb = balanceAtSplit;
+        final BigDecimal newDiscount = origDiscount.add(origNet, mc).subtract(balanceAtSplit, mc).subtract(paymentsReceived, mc);
+        final int scale = currency.getDigitsAfterDecimal();
+        final BigDecimal newDailyPayment = tpv.multiply(newPeriodPaymentRate, mc).divide(BigDecimal.valueOf(npvDayCount), mc)
+                .setScale(scale, RoundingMode.HALF_UP);

Review Comment:
   Hardcoded rounding mode still.... 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3225036022


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanPeriodPaymentRateChangeReadServiceImpl.java:
##########
@@ -0,0 +1,47 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.workingcapitalloan.service;
+
+import java.util.List;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.portfolio.workingcapitalloan.data.WorkingCapitalLoanPeriodPaymentRateChangeData;
+import org.apache.fineract.portfolio.workingcapitalloan.domain.WorkingCapitalLoanPeriodPaymentRateChange;
+import org.apache.fineract.portfolio.workingcapitalloan.repository.WorkingCapitalLoanPeriodPaymentRateChangeRepository;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+
+@Service
+@RequiredArgsConstructor
+@Transactional(readOnly = true)
+public class WorkingCapitalLoanPeriodPaymentRateChangeReadServiceImpl implements WorkingCapitalLoanPeriodPaymentRateChangeReadService {
+
+    private final WorkingCapitalLoanPeriodPaymentRateChangeRepository repository;
+
+    @Override
+    public List<WorkingCapitalLoanPeriodPaymentRateChangeData> retrieveRateChangeHistory(final Long loanId) {
+        return repository.findByWorkingCapitalLoanIdOrderByIdDesc(loanId).stream().map(e -> toData(e, loanId)).toList();

Review Comment:
   extra existsById check added



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3221536544


##########
fineract-working-capital-loan/src/main/resources/db/changelog/tenant/module/workingcapitalloan/parts/0036_wc_loan_period_payment_rate_change.xml:
##########
@@ -0,0 +1,135 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements. See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership. The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License. You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied. See the License for the
+    specific language governing permissions and limitations
+    under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.3.xsd">
+
+    <changeSet author="fineract" id="wcl-0036-1-create-rate-change-table">
+        <preConditions onFail="MARK_RAN">
+            <not>
+                <tableExists tableName="m_wc_loan_period_payment_rate_change"/>
+            </not>
+        </preConditions>
+        <createTable tableName="m_wc_loan_period_payment_rate_change">
+            <column autoIncrement="true" name="id" type="BIGINT">
+                <constraints nullable="false" primaryKey="true"/>
+            </column>
+            <column name="wc_loan_id" type="BIGINT">
+                <constraints nullable="false"/>
+            </column>
+            <column name="effective_date" type="DATE">
+                <constraints nullable="false"/>
+            </column>
+            <column name="previous_rate" type="DECIMAL(19,6)">
+                <constraints nullable="false"/>
+            </column>
+            <column name="new_rate" type="DECIMAL(19,6)">
+                <constraints nullable="false"/>
+            </column>
+            <column name="is_reversed" type="BOOLEAN" defaultValueBoolean="false">
+                <constraints nullable="false"/>
+            </column>
+            <column name="reversed_on_date" type="DATE"/>
+            <column name="created_by" type="BIGINT">
+                <constraints nullable="false"/>
+            </column>
+            <column name="last_modified_by" type="BIGINT">
+                <constraints nullable="false"/>
+            </column>
+            <column name="version" type="INT" defaultValueNumeric="0">
+                <constraints nullable="false"/>
+            </column>
+        </createTable>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-2-audit-columns-my" context="mysql">
+        <addColumn tableName="m_wc_loan_period_payment_rate_change">
+            <column name="created_on_utc" type="DATETIME(6)">
+                <constraints nullable="false"/>
+            </column>
+            <column name="last_modified_on_utc" type="DATETIME(6)">
+                <constraints nullable="false"/>
+            </column>
+        </addColumn>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-2-audit-columns-pg" context="postgresql">
+        <addColumn tableName="m_wc_loan_period_payment_rate_change">
+            <column name="created_on_utc" type="TIMESTAMP WITH TIME ZONE">
+                <constraints nullable="false"/>
+            </column>
+            <column name="last_modified_on_utc" type="TIMESTAMP WITH TIME ZONE">
+                <constraints nullable="false"/>
+            </column>
+        </addColumn>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-3-rate-change-fks">
+        <addForeignKeyConstraint baseColumnNames="wc_loan_id" baseTableName="m_wc_loan_period_payment_rate_change"
+                                 constraintName="fk_wc_rate_change_loan" referencedColumnNames="id"
+                                 referencedTableName="m_wc_loan"/>
+        <addForeignKeyConstraint baseColumnNames="created_by" baseTableName="m_wc_loan_period_payment_rate_change"
+                                 constraintName="fk_wc_rate_change_created_by" referencedColumnNames="id"
+                                 referencedTableName="m_appuser"/>
+        <addForeignKeyConstraint baseColumnNames="last_modified_by" baseTableName="m_wc_loan_period_payment_rate_change"
+                                 constraintName="fk_wc_rate_change_last_modified_by" referencedColumnNames="id"
+                                 referencedTableName="m_appuser"/>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-4-rate-change-index">
+        <createIndex tableName="m_wc_loan_period_payment_rate_change" indexName="idx_wc_rate_change_loan_date">
+            <column name="wc_loan_id"/>
+            <column name="effective_date"/>
+        </createIndex>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-6-update-rate-permission">
+        <preConditions onFail="MARK_RAN">
+            <sqlCheck expectedResult="0">
+                SELECT COUNT(*) FROM m_permission WHERE code = 'UPDATERATE_WORKINGCAPITALLOAN';
+            </sqlCheck>
+        </preConditions>
+        <insert tableName="m_permission">
+            <column name="grouping" value="portfolio"/>
+            <column name="code" value="UPDATERATE_WORKINGCAPITALLOAN"/>
+            <column name="entity_name" value="WORKINGCAPITALLOAN"/>
+            <column name="action_name" value="UPDATERATE"/>
+            <column name="can_maker_checker" valueBoolean="false"/>
+        </insert>
+    </changeSet>
+
+    <changeSet author="fineract" id="wcl-0036-7-undo-rate-change-permission">

Review Comment:
   Where's the endpoint / handler / service method for this? I can't find any `UNDORATECHANGE` implementation, just the permission row. Either drop this changeset until you actually implement undo, or include the implementation in this PR. Dead permissions are confusing.



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResource.java:
##########
@@ -310,4 +313,102 @@ private CommandProcessingResult handleStateTransition(final Long loanId, final S
         return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
     }
 
+    @PUT
+    @Path("{loanId}/discount")

Review Comment:
   Is this discount endpoint related to FINERACT-2455? Looks like a separate feature. Let's split it into its own PR.



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/serialization/WorkingCapitalLoanDataValidator.java:
##########
@@ -95,6 +96,9 @@ public class WorkingCapitalLoanDataValidator {
                     WorkingCapitalLoanConstants.noteParamName, WorkingCapitalLoanConstants.transactionDateParamName));
     private static final Set<String> CREDIT_BALANCE_REFUND_SUPPORTED_PARAMETERS = new HashSet<>(REPAYMENT_SUPPORTED_PARAMETERS);
 
+    private static final Set<String> UPDATE_RATE_SUPPORTED_PARAMETERS = new HashSet<>(
+            Arrays.asList("locale", WorkingCapitalLoanConstants.periodPaymentRateParamName, WorkingCapitalLoanConstants.noteParamName));

Review Comment:
   There's a `WorkingCapitalLoanConstants.localeParameterName` already.



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/serialization/WorkingCapitalLoanDataValidator.java:
##########
@@ -616,6 +620,58 @@ public void validateCreditBalanceRefund(final String json, final WorkingCapitalL
         throwExceptionIfValidationWarningsExist(dataValidationErrors);
     }
 
+    public void validateUpdatePeriodPaymentRate(final String json, final WorkingCapitalLoan loan) {

Review Comment:
   Tests please. No unit test covers: blank json, non-active loan, missing `periodPaymentRate`, negative rate, same-as-current rate, below product min, above product max, note too long. Same for `WorkingCapitalLoanWritePlatformServiceImpl.updatePeriodPaymentRate`. The feign tests only happy-path it.



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanWritePlatformServiceImpl.java:
##########
@@ -668,6 +671,57 @@ public CommandProcessingResult creditBalanceRefund(final Long loanId, final Json
                 .withClientId(loan.getClientId()).withLoanId(loanId).with(changes).build();
     }
 
+    @Override
+    @Transactional
+    public CommandProcessingResult updatePeriodPaymentRate(final Long loanId, final JsonCommand command) {
+        final WorkingCapitalLoan loan = this.loanRepository.findById(loanId)
+                .orElseThrow(() -> new WorkingCapitalLoanNotFoundException(loanId));
+        this.validator.validateUpdatePeriodPaymentRate(command.json(), loan);
+
+        final BigDecimal newRate = this.fromApiJsonHelper.extractBigDecimalNamed(WorkingCapitalLoanConstants.periodPaymentRateParamName,
+                command.parsedJson(), new HashSet<>());
+        final BigDecimal previousRate = loan.getLoanProductRelatedDetails().getPeriodPaymentRate();
+
+        final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+
+        final List<WorkingCapitalLoanPeriodPaymentRateChange> activeChanges = this.rateChangeRepository
+                .findByWorkingCapitalLoanIdAndReversedFalse(loanId);
+        for (final WorkingCapitalLoanPeriodPaymentRateChange active : activeChanges) {
+            active.reverse(businessDate);
+        }
+        if (!activeChanges.isEmpty()) {
+            this.rateChangeRepository.saveAll(activeChanges);
+        }
+
+        loan.getLoanProductRelatedDetails().setPeriodPaymentRate(newRate);
+
+        final WorkingCapitalLoanPeriodPaymentRateChange rateChange = WorkingCapitalLoanPeriodPaymentRateChange.create(loan, businessDate,
+                previousRate, newRate);
+        this.rateChangeRepository.save(rateChange);
+
+        try {
+            this.amortizationScheduleWriteService.regenerateAmortizationScheduleOnRateChange(loan, newRate);
+        } catch (IllegalStateException | IllegalArgumentException e) {

Review Comment:
   This is using exceptions as control flow. `IllegalStateException` / `IllegalArgumentException` are thrown by `Validate.notNull` and any number of unrelated places - you'll catch and mask real bugs here. Either validate preconditions upfront in the validator (preferred), or have the calculator throw a domain-specific checked exception you can react to.



##########
fineract-working-capital-loan/src/main/resources/db/changelog/tenant/module/workingcapitalloan/parts/0036_wc_loan_period_payment_rate_change.xml:
##########
@@ -0,0 +1,135 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements. See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership. The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License. You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied. See the License for the
+    specific language governing permissions and limitations
+    under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.3.xsd">
+
+    <changeSet author="fineract" id="wcl-0036-1-create-rate-change-table">

Review Comment:
   The changeset IDs jump: 1, 2-my, 2-pg, 3, 4, 6, 7. What happened to 5? Renumber please.



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/serialization/WorkingCapitalLoanDataValidator.java:
##########
@@ -616,6 +620,58 @@ public void validateCreditBalanceRefund(final String json, final WorkingCapitalL
         throwExceptionIfValidationWarningsExist(dataValidationErrors);
     }
 
+    public void validateUpdatePeriodPaymentRate(final String json, final WorkingCapitalLoan loan) {
+        if (StringUtils.isBlank(json)) {
+            throw new InvalidJsonException();
+        }
+
+        if (loan.getLoanStatus() != LoanStatus.ACTIVE) {
+            throw new PlatformApiDataValidationException("validation.msg.wc.loan.rate.change.not.allowed",

Review Comment:
   Why mix early `throw` with the `DataValidatorBuilder` pattern? Collect all errors via the builder and throw once at the end like the other validate methods in this class. Otherwise the client gets one error at a time which is annoying.



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanAmortizationScheduleReadServiceImpl.java:
##########
@@ -36,21 +37,17 @@
 @Transactional(readOnly = true)
 public class WorkingCapitalLoanAmortizationScheduleReadServiceImpl implements WorkingCapitalLoanAmortizationScheduleReadService {
 
-    // TODO: currency should come from loan product once WCL lifecycle is implemented
-    private static final MonetaryCurrency DEFAULT_CURRENCY = new MonetaryCurrency("USD", 2, null);
-
     private final WorkingCapitalLoanRepository loanRepository;
     private final ProjectedAmortizationScheduleRepositoryWrapper scheduleRepositoryWrapper;
     private final ProjectedAmortizationScheduleMapper mapper;
 
     @Override
     public ProjectedAmortizationScheduleData retrieveAmortizationSchedule(final Long loanId) {
-        if (!loanRepository.existsById(loanId)) {
-            throw new WorkingCapitalLoanNotFoundException(loanId);
-        }
+        final WorkingCapitalLoan loan = loanRepository.findById(loanId).orElseThrow(() -> new WorkingCapitalLoanNotFoundException(loanId));
 
         final MathContext mc = MoneyHelper.getMathContext();
-        final ProjectedAmortizationScheduleModel model = scheduleRepositoryWrapper.readModel(loanId, mc, DEFAULT_CURRENCY)
+        final CurrencyData currency = WorkingCapitalLoanAmortizationScheduleWriteServiceImpl.resolveCurrency(loan);

Review Comment:
   Static call into the write service impl from the read service. Extract `resolveCurrency` into its own helper / domain method on `WorkingCapitalLoan`. The read service shouldn't depend on the write service's impl class.



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanPeriodPaymentRateChangeReadServiceImpl.java:
##########
@@ -0,0 +1,47 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.workingcapitalloan.service;
+
+import java.util.List;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.portfolio.workingcapitalloan.data.WorkingCapitalLoanPeriodPaymentRateChangeData;
+import org.apache.fineract.portfolio.workingcapitalloan.domain.WorkingCapitalLoanPeriodPaymentRateChange;
+import org.apache.fineract.portfolio.workingcapitalloan.repository.WorkingCapitalLoanPeriodPaymentRateChangeRepository;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+
+@Service
+@RequiredArgsConstructor
+@Transactional(readOnly = true)
+public class WorkingCapitalLoanPeriodPaymentRateChangeReadServiceImpl implements WorkingCapitalLoanPeriodPaymentRateChangeReadService {
+
+    private final WorkingCapitalLoanPeriodPaymentRateChangeRepository repository;
+
+    @Override
+    public List<WorkingCapitalLoanPeriodPaymentRateChangeData> retrieveRateChangeHistory(final Long loanId) {
+        return repository.findByWorkingCapitalLoanIdOrderByIdDesc(loanId).stream().map(e -> toData(e, loanId)).toList();

Review Comment:
   No check that the loan actually exists. Bogus `loanId` returns an empty list silently. Let's 404 on missing loan.



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanWritePlatformServiceImpl.java:
##########
@@ -668,6 +671,57 @@ public CommandProcessingResult creditBalanceRefund(final Long loanId, final Json
                 .withClientId(loan.getClientId()).withLoanId(loanId).with(changes).build();
     }
 
+    @Override
+    @Transactional
+    public CommandProcessingResult updatePeriodPaymentRate(final Long loanId, final JsonCommand command) {
+        final WorkingCapitalLoan loan = this.loanRepository.findById(loanId)
+                .orElseThrow(() -> new WorkingCapitalLoanNotFoundException(loanId));
+        this.validator.validateUpdatePeriodPaymentRate(command.json(), loan);
+
+        final BigDecimal newRate = this.fromApiJsonHelper.extractBigDecimalNamed(WorkingCapitalLoanConstants.periodPaymentRateParamName,
+                command.parsedJson(), new HashSet<>());
+        final BigDecimal previousRate = loan.getLoanProductRelatedDetails().getPeriodPaymentRate();
+
+        final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+
+        final List<WorkingCapitalLoanPeriodPaymentRateChange> activeChanges = this.rateChangeRepository
+                .findByWorkingCapitalLoanIdAndReversedFalse(loanId);
+        for (final WorkingCapitalLoanPeriodPaymentRateChange active : activeChanges) {
+            active.reverse(businessDate);
+        }
+        if (!activeChanges.isEmpty()) {
+            this.rateChangeRepository.saveAll(activeChanges);

Review Comment:
   Why call `saveAll`? The entities you fetched are managed within the `@Transactional` method. Same for `loanRepository.saveAndFlush(loan)` further below.



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/domain/WorkingCapitalLoanPeriodPaymentRateChange.java:
##########
@@ -0,0 +1,79 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.workingcapitalloan.domain;
+
+import jakarta.persistence.Column;
+import jakarta.persistence.Entity;
+import jakarta.persistence.FetchType;
+import jakarta.persistence.JoinColumn;
+import jakarta.persistence.ManyToOne;
+import jakarta.persistence.Table;
+import jakarta.persistence.Version;
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.Setter;
+import org.apache.fineract.infrastructure.core.domain.AbstractAuditableWithUTCDateTimeCustom;
+
+@Getter
+@Setter
+@NoArgsConstructor
+@Entity
+@Table(name = "m_wc_loan_period_payment_rate_change")
+public class WorkingCapitalLoanPeriodPaymentRateChange extends AbstractAuditableWithUTCDateTimeCustom<Long> {
+
+    @ManyToOne(fetch = FetchType.LAZY)
+    @JoinColumn(name = "wc_loan_id", nullable = false)
+    private WorkingCapitalLoan workingCapitalLoan;
+
+    @Column(name = "effective_date", nullable = false)
+    private LocalDate effectiveDate;
+
+    @Column(name = "previous_rate", scale = 6, precision = 19, nullable = false)
+    private BigDecimal previousRate;
+
+    @Column(name = "new_rate", scale = 6, precision = 19, nullable = false)
+    private BigDecimal newRate;
+
+    @Column(name = "is_reversed", nullable = false)
+    private boolean reversed;

Review Comment:
   Nothing currently writes `reversedByUser` or other reversal audit fields for reversal - is that intentional, or are we missing reversal audit columns vs the `m_wc_loan_transaction` reversal pattern?



##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanAmortizationScheduleWriteServiceImpl.java:
##########
@@ -180,21 +183,56 @@ public RepaymentAmortizationData applyRepayment(final WorkingCapitalLoan loan, f
     private BigDecimal sumRunningNpv(final ProjectedAmortizationScheduleModel model) {
         final MathContext mc = MoneyHelper.getMathContext();
         BigDecimal result = BigDecimal.ZERO;
-        for (ProjectedPayment payment : model.payments()) {
+        for (ProjectedPayment payment : model.projectedPayments()) {
             if (payment.paymentNo() > 0 && payment.npvValue() != null && payment.npvValue().getAmount() != null) {
                 result = result.add(payment.npvValue().getAmount(), mc);
             }
         }
         return result;
     }
 
-    private MonetaryCurrency resolveCurrency(final WorkingCapitalLoan loan) {
+    @Override
+    public void regenerateAmortizationScheduleOnRateChange(final WorkingCapitalLoan loan, final BigDecimal newRate) {
+        Validate.notNull(loan, "loan must not be null");
+        Validate.notNull(newRate, "newRate must not be null");
+
+        final MathContext mc = MoneyHelper.getMathContext();
+        final CurrencyData currency = resolveCurrency(loan);
+        final ProjectedAmortizationScheduleModel model = scheduleRepositoryWrapper.readModel(loan.getId(), mc, currency)
+                .orElseThrow(() -> new IllegalStateException("Projected amortization schedule is not found for loan " + loan.getId()));
+
+        final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+        final LocalDate loanDisbursementDate = resolveLoanDisbursementDate(loan);
+        final int splitDayIndex = (int) ChronoUnit.DAYS.between(loanDisbursementDate, businessDate);
+        final LocalDate modelRateChangeDate = model.expectedDisbursementDate().plusDays(splitDayIndex);
+
+        model.clearLastRateSegment();
+
+        calculator.applyRateChange(model, newRate, modelRateChangeDate);
+
+        scheduleRepositoryWrapper.writeModel(loan, model);
+    }
+
+    private LocalDate resolveLoanDisbursementDate(final WorkingCapitalLoan loan) {
+        if (loan.getDisbursementDetails() != null && !loan.getDisbursementDetails().isEmpty()) {
+            final WorkingCapitalLoanDisbursementDetails detail = loan.getDisbursementDetails().getFirst();
+            if (detail.getActualDisbursementDate() != null) {
+                return detail.getActualDisbursementDate();
+            }
+            if (detail.getExpectedDisbursementDate() != null) {
+                return detail.getExpectedDisbursementDate();

Review Comment:
   The validator only allows rate changes for `LoanStatus.ACTIVE`. An active loan must have an actual disbursement date - so taking the first disbursement detail and falling back to expected is suspicious. Shouldn't we be using the actual disbursement date strictly here, and throwing a proper domain exception (not `IllegalStateException`) if it's missing?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3221536554


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResource.java:
##########
@@ -310,4 +313,102 @@ private CommandProcessingResult handleStateTransition(final Long loanId, final S
         return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
     }
 
+    @PUT
+    @Path("{loanId}/discount")

Review Comment:
   Is this discount endpoint related to FINERACT-2455? Looks like a separate feature. Let's split it into its own PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3225007532


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/serialization/WorkingCapitalLoanDataValidator.java:
##########
@@ -616,6 +620,58 @@ public void validateCreditBalanceRefund(final String json, final WorkingCapitalL
         throwExceptionIfValidationWarningsExist(dataValidationErrors);
     }
 
+    public void validateUpdatePeriodPaymentRate(final String json, final WorkingCapitalLoan loan) {
+        if (StringUtils.isBlank(json)) {
+            throw new InvalidJsonException();
+        }
+
+        if (loan.getLoanStatus() != LoanStatus.ACTIVE) {
+            throw new PlatformApiDataValidationException("validation.msg.wc.loan.rate.change.not.allowed",

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3170419512


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/calc/ProjectedAmortizationScheduleModel.java:
##########
@@ -79,6 +82,9 @@ public final class ProjectedAmortizationScheduleModel {
     @Getter(AccessLevel.NONE)
     private final List<AppliedPayment> appliedPayments;
 
+    @Getter(AccessLevel.NONE)
+    private final List<RateSegment> rateSegments;

Review Comment:
   I was wondering whether we should rename this to reflect better the involved logic:
   - We are changing the projected payment **amount**
   
   I dont think the model need to know the payment percentage rate, but rather what is the expected payment amount only, no?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3153878286


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResource.java:
##########
@@ -346,4 +349,110 @@ private CommandProcessingResult updateDiscount(final Long loanId, final String l
                 .updateDiscountWorkingCapitalLoanApplication(resolvedLoanId).build();
         return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
     }
+
+    @PUT
+    @Path("{loanId}/rate")

Review Comment:
   yes, 2 types of unique id:
   - id: (always) auto generated, internal
   - externalId: can be manually provided, optionally auto generated, external purposes, not guessable (uuid, etc)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5771:
URL: https://github.com/apache/fineract/pull/5771#issuecomment-4313426925

   @budaidev Please rebase


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5771:
URL: https://github.com/apache/fineract/pull/5771#issuecomment-4296890383

   @budaidev Please rebase


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3224509010


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/domain/WorkingCapitalLoanPeriodPaymentRateChange.java:
##########
@@ -0,0 +1,79 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.workingcapitalloan.domain;
+
+import jakarta.persistence.Column;
+import jakarta.persistence.Entity;
+import jakarta.persistence.FetchType;
+import jakarta.persistence.JoinColumn;
+import jakarta.persistence.ManyToOne;
+import jakarta.persistence.Table;
+import jakarta.persistence.Version;
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.Setter;
+import org.apache.fineract.infrastructure.core.domain.AbstractAuditableWithUTCDateTimeCustom;
+
+@Getter
+@Setter
+@NoArgsConstructor
+@Entity
+@Table(name = "m_wc_loan_period_payment_rate_change")
+public class WorkingCapitalLoanPeriodPaymentRateChange extends AbstractAuditableWithUTCDateTimeCustom<Long> {
+
+    @ManyToOne(fetch = FetchType.LAZY)
+    @JoinColumn(name = "wc_loan_id", nullable = false)
+    private WorkingCapitalLoan workingCapitalLoan;
+
+    @Column(name = "effective_date", nullable = false)
+    private LocalDate effectiveDate;
+
+    @Column(name = "previous_rate", scale = 6, precision = 19, nullable = false)
+    private BigDecimal previousRate;
+
+    @Column(name = "new_rate", scale = 6, precision = 19, nullable = false)
+    private BigDecimal newRate;
+
+    @Column(name = "is_reversed", nullable = false)
+    private boolean reversed;

Review Comment:
   The pattern matches WorkingCapitalLoanTransaction — same is_reversed + reversed_on_date fields, no separate reversed_by. The last_modified_by audit column (from AbstractAuditableWithUTCDateTimeCustom) captures who triggered the reversal, since JPA auditing updates it when reverse() is called. But we can add it if necessary



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3225005065


##########
fineract-working-capital-loan/src/main/resources/db/changelog/tenant/module/workingcapitalloan/parts/0036_wc_loan_period_payment_rate_change.xml:
##########
@@ -0,0 +1,135 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements. See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership. The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License. You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied. See the License for the
+    specific language governing permissions and limitations
+    under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.3.xsd">
+
+    <changeSet author="fineract" id="wcl-0036-1-create-rate-change-table">

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3225016722


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanAmortizationScheduleReadServiceImpl.java:
##########
@@ -36,21 +37,17 @@
 @Transactional(readOnly = true)
 public class WorkingCapitalLoanAmortizationScheduleReadServiceImpl implements WorkingCapitalLoanAmortizationScheduleReadService {
 
-    // TODO: currency should come from loan product once WCL lifecycle is implemented
-    private static final MonetaryCurrency DEFAULT_CURRENCY = new MonetaryCurrency("USD", 2, null);
-
     private final WorkingCapitalLoanRepository loanRepository;
     private final ProjectedAmortizationScheduleRepositoryWrapper scheduleRepositoryWrapper;
     private final ProjectedAmortizationScheduleMapper mapper;
 
     @Override
     public ProjectedAmortizationScheduleData retrieveAmortizationSchedule(final Long loanId) {
-        if (!loanRepository.existsById(loanId)) {
-            throw new WorkingCapitalLoanNotFoundException(loanId);
-        }
+        final WorkingCapitalLoan loan = loanRepository.findById(loanId).orElseThrow(() -> new WorkingCapitalLoanNotFoundException(loanId));
 
         final MathContext mc = MoneyHelper.getMathContext();
-        final ProjectedAmortizationScheduleModel model = scheduleRepositoryWrapper.readModel(loanId, mc, DEFAULT_CURRENCY)
+        final CurrencyData currency = WorkingCapitalLoanAmortizationScheduleWriteServiceImpl.resolveCurrency(loan);

Review Comment:
   refactored



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3225218166


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanWritePlatformServiceImpl.java:
##########
@@ -668,6 +671,57 @@ public CommandProcessingResult creditBalanceRefund(final Long loanId, final Json
                 .withClientId(loan.getClientId()).withLoanId(loanId).with(changes).build();
     }
 
+    @Override
+    @Transactional
+    public CommandProcessingResult updatePeriodPaymentRate(final Long loanId, final JsonCommand command) {
+        final WorkingCapitalLoan loan = this.loanRepository.findById(loanId)
+                .orElseThrow(() -> new WorkingCapitalLoanNotFoundException(loanId));
+        this.validator.validateUpdatePeriodPaymentRate(command.json(), loan);
+
+        final BigDecimal newRate = this.fromApiJsonHelper.extractBigDecimalNamed(WorkingCapitalLoanConstants.periodPaymentRateParamName,
+                command.parsedJson(), new HashSet<>());
+        final BigDecimal previousRate = loan.getLoanProductRelatedDetails().getPeriodPaymentRate();
+
+        final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+
+        final List<WorkingCapitalLoanPeriodPaymentRateChange> activeChanges = this.rateChangeRepository
+                .findByWorkingCapitalLoanIdAndReversedFalse(loanId);
+        for (final WorkingCapitalLoanPeriodPaymentRateChange active : activeChanges) {
+            active.reverse(businessDate);
+        }
+        if (!activeChanges.isEmpty()) {
+            this.rateChangeRepository.saveAll(activeChanges);
+        }
+
+        loan.getLoanProductRelatedDetails().setPeriodPaymentRate(newRate);
+
+        final WorkingCapitalLoanPeriodPaymentRateChange rateChange = WorkingCapitalLoanPeriodPaymentRateChange.create(loan, businessDate,
+                previousRate, newRate);
+        this.rateChangeRepository.save(rateChange);
+
+        try {
+            this.amortizationScheduleWriteService.regenerateAmortizationScheduleOnRateChange(loan, newRate);
+        } catch (IllegalStateException | IllegalArgumentException e) {

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3170431250


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/domain/ProjectedAmortizationLoanModel.java:
##########
@@ -54,4 +54,7 @@ public class ProjectedAmortizationLoanModel extends AbstractPersistableCustom<Lo
 
     @Column(name = "json_model_version", nullable = false)
     private String jsonModelVersion;
+
+    @Column(name = "previous_json_model", columnDefinition = "text")
+    private String previousJsonModel;

Review Comment:
   Do we need to know it?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3170439143


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanWritePlatformServiceImpl.java:
##########
@@ -577,6 +580,67 @@ public CommandProcessingResult creditBalanceRefund(final Long loanId, final Json
                 .withClientId(loan.getClientId()).withLoanId(loanId).with(changes).build();
     }
 
+    @Override
+    @Transactional
+    public CommandProcessingResult updateRate(final Long loanId, final JsonCommand command) {

Review Comment:
   Lets use more descriptive method name... update rate could mean many thing and maybe other similar, but not same operations might be implemented.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3170404030


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResourceSwagger.java:
##########
@@ -609,4 +609,22 @@ private GetWorkingCapitalLoanDelinquencyRangeScheduleTagHistoryResponse() {}
         public BigDecimal delinquentAmount;
     }
 
+    @Schema(description = "Request for updating period payment rate on an active Working Capital Loan")
+    public static final class PutWorkingCapitalLoansLoanIdRateRequest {
+
+        private PutWorkingCapitalLoansLoanIdRateRequest() {}
+
+        @Schema(example = "0.17", description = "New period payment rate")
+        public BigDecimal periodPaymentRate;
+
+        @Schema(example = "Rate change note")
+        public String note;
+
+        @Schema(example = "en_GB")
+        public String locale;
+
+        @Schema(example = "dd MMMM yyyy")
+        public String dateFormat;

Review Comment:
   No dates involved. Having `dateFormat` seems useless, no?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3156380085


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/api/WorkingCapitalLoanApiResource.java:
##########
@@ -346,4 +349,110 @@ private CommandProcessingResult updateDiscount(final Long loanId, final String l
                 .updateDiscountWorkingCapitalLoanApplication(resolvedLoanId).build();
         return this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
     }
+
+    @PUT
+    @Path("{loanId}/rate")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(operationId = "updateWorkingCapitalLoanRateById", summary = "Update period payment rate for an active Working Capital Loan", description = "Modifies the period payment rate and triggers schedule recalculation for the remaining term.")
+    @RequestBody(required = true, content = @Content(schema = @Schema(implementation = WorkingCapitalLoanApiResourceSwagger.PutWorkingCapitalLoansLoanIdRateRequest.class)))
+    @ApiResponses({

Review Comment:
   removed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2455: WorkingCapital - % repayment modification options [fineract]

Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #5771:
URL: https://github.com/apache/fineract/pull/5771#discussion_r3221536576


##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanWritePlatformServiceImpl.java:
##########
@@ -668,6 +671,57 @@ public CommandProcessingResult creditBalanceRefund(final Long loanId, final Json
                 .withClientId(loan.getClientId()).withLoanId(loanId).with(changes).build();
     }
 
+    @Override
+    @Transactional
+    public CommandProcessingResult updatePeriodPaymentRate(final Long loanId, final JsonCommand command) {
+        final WorkingCapitalLoan loan = this.loanRepository.findById(loanId)
+                .orElseThrow(() -> new WorkingCapitalLoanNotFoundException(loanId));
+        this.validator.validateUpdatePeriodPaymentRate(command.json(), loan);
+
+        final BigDecimal newRate = this.fromApiJsonHelper.extractBigDecimalNamed(WorkingCapitalLoanConstants.periodPaymentRateParamName,
+                command.parsedJson(), new HashSet<>());
+        final BigDecimal previousRate = loan.getLoanProductRelatedDetails().getPeriodPaymentRate();
+
+        final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+
+        final List<WorkingCapitalLoanPeriodPaymentRateChange> activeChanges = this.rateChangeRepository
+                .findByWorkingCapitalLoanIdAndReversedFalse(loanId);
+        for (final WorkingCapitalLoanPeriodPaymentRateChange active : activeChanges) {
+            active.reverse(businessDate);
+        }
+        if (!activeChanges.isEmpty()) {
+            this.rateChangeRepository.saveAll(activeChanges);

Review Comment:
   Why call `saveAll`? The entities you fetched are managed within the `@Transactional` method. Same for `loanRepository.saveAndFlush(loan)` further below.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org