You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "SamaSVM (via GitHub)" <gi...@apache.org> on 2026/04/10 12:41:13 UTC
[PR] FINERACT-1185: Add anniversary interest posting period types [fineract]
SamaSVM opened a new pull request, #5761:
URL: https://github.com/apache/fineract/pull/5761
## Description
Added four new SavingsPostingInterestPeriodType values that post interest on the same day of the month as the account opening date (the "anniversary" date).
## 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-1185: Add anniversary interest posting period types [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5761:
URL: https://github.com/apache/fineract/pull/5761#discussion_r3187816792
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsHelper.java:
##########
@@ -126,37 +127,37 @@ private LocalDate determineInterestPostingPeriodEndDateFrom(final LocalDate peri
case INVALID:
break;
case DAILY:
- // produce period end date on current day
- periodEndDate = periodStartDate;
+ // interest posting occurs on the next day after current day
+ periodEndDate = periodStartDate.plusDays(1);
Review Comment:
hm... i see.. i might need to understand this part of the logic more
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "SamaSVM (via GitHub)" <gi...@apache.org>.
SamaSVM commented on code in PR #5761:
URL: https://github.com/apache/fineract/pull/5761#discussion_r3281078779
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsHelper.java:
##########
@@ -166,14 +167,28 @@ private LocalDate determineInterestPostingPeriodEndDateFrom(final LocalDate peri
} else {
periodEndDate = periodStartDate.withMonth(financialYearBeginningMonth);
}
- periodEndDate = periodEndDate.with(TemporalAdjusters.lastDayOfMonth());
+ periodEndDate = periodEndDate.with(TemporalAdjusters.lastDayOfMonth()).plusDays(1);
+ break;
+ case ANNIVERSARY_MONTHLY:
+ periodEndDate = adjustToAnniversaryDay(periodStartDate.plusMonths(1), anniversaryDayOfMonth);
+ break;
+ case ANNIVERSARY_QUARTERLY:
+ periodEndDate = adjustToAnniversaryDay(periodStartDate.plusMonths(3), anniversaryDayOfMonth);
+ break;
+ case ANNIVERSARY_BIANNUAL:
+ periodEndDate = adjustToAnniversaryDay(periodStartDate.plusMonths(6), anniversaryDayOfMonth);
+ break;
+ case ANNIVERSARY_ANNUAL:
+ periodEndDate = adjustToAnniversaryDay(periodStartDate.plusMonths(12), anniversaryDayOfMonth);
break;
}
- // interest posting always occurs on next day after the period end date.
Review Comment:
I've added it in each place.
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5761:
URL: https://github.com/apache/fineract/pull/5761#issuecomment-4295143258
@bharathc27 FYI
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #5761:
URL: https://github.com/apache/fineract/pull/5761
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5761:
URL: https://github.com/apache/fineract/pull/5761#issuecomment-4295216006
@SamaSVM Can you please make sure you are updating the Fineract JIRA story and the documentation in `fineract-doc` module to cover these new posting periods?
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "SamaSVM (via GitHub)" <gi...@apache.org>.
SamaSVM commented on code in PR #5761:
URL: https://github.com/apache/fineract/pull/5761#discussion_r3187637578
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsHelper.java:
##########
@@ -126,37 +127,37 @@ private LocalDate determineInterestPostingPeriodEndDateFrom(final LocalDate peri
case INVALID:
break;
case DAILY:
- // produce period end date on current day
- periodEndDate = periodStartDate;
+ // interest posting occurs on the next day after current day
+ periodEndDate = periodStartDate.plusDays(1);
Review Comment:
I’ve left this logic unchanged for all non-ANNIVERSARY period types. I can try removing the plusDays(1) for the old types, but I’m not confident about the impact this might have on the rest of the logic.
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "oleksii-novikov-onix (via GitHub)" <gi...@apache.org>.
oleksii-novikov-onix commented on code in PR #5761:
URL: https://github.com/apache/fineract/pull/5761#discussion_r3074848609
##########
fineract-provider/src/test/java/org/apache/fineract/portfolio/savings/service/SavingsDropdownReadPlatformServiceImplTest.java:
##########
@@ -0,0 +1,56 @@
+/**
+ * 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.savings.service;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.fineract.infrastructure.core.data.EnumOptionData;
+import org.apache.fineract.portfolio.savings.SavingsPostingInterestPeriodType;
+import org.junit.jupiter.api.Test;
+
+class SavingsDropdownReadPlatformServiceImplTest {
+
+ private final SavingsDropdownReadPlatformServiceImpl service = new SavingsDropdownReadPlatformServiceImpl();
+
+ @Test
+ void retrieveInterestPostingPeriodTypeOptions_shouldIncludeAllAnniversaryTypes() {
+ Collection<EnumOptionData> options = service.retrieveInterestPostingPeriodTypeOptions();
Review Comment:
You may see a lot of Collection usage throughout the Fineract codebase, that's legacy code. Whenever possible, these should be rewritten to use more specific types like List or Set
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5761:
URL: https://github.com/apache/fineract/pull/5761#discussion_r3187512245
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsHelper.java:
##########
@@ -168,12 +169,28 @@ private LocalDate determineInterestPostingPeriodEndDateFrom(final LocalDate peri
}
periodEndDate = periodEndDate.with(TemporalAdjusters.lastDayOfMonth());
break;
+ case ANNIVERSARY_MONTHLY:
+ periodEndDate = adjustToAnniversaryDay(periodStartDate.plusMonths(1), anniversaryDayOfMonth).minusDays(1);
Review Comment:
Dont mark it as resolved without any answer, please...
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "SamaSVM (via GitHub)" <gi...@apache.org>.
SamaSVM commented on code in PR #5761:
URL: https://github.com/apache/fineract/pull/5761#discussion_r3187637578
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsHelper.java:
##########
@@ -126,37 +127,37 @@ private LocalDate determineInterestPostingPeriodEndDateFrom(final LocalDate peri
case INVALID:
break;
case DAILY:
- // produce period end date on current day
- periodEndDate = periodStartDate;
+ // interest posting occurs on the next day after current day
+ periodEndDate = periodStartDate.plusDays(1);
Review Comment:
I’ve left this logic unchanged for all non-ANNIVERSARY period types. I can try removing the plusDays(1) for the old types, but I’m not confident about the impact this might have on the rest of the logic.
Earlier we simply had the following at the end:
`periodEndDate = periodEndDate.plusDays(1);`
`return periodEndDate;`
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5761:
URL: https://github.com/apache/fineract/pull/5761#discussion_r3187530112
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsHelper.java:
##########
@@ -126,37 +127,37 @@ private LocalDate determineInterestPostingPeriodEndDateFrom(final LocalDate peri
case INVALID:
break;
case DAILY:
- // produce period end date on current day
- periodEndDate = periodStartDate;
+ // interest posting occurs on the next day after current day
+ periodEndDate = periodStartDate.plusDays(1);
Review Comment:
These changes looks quite breaking ones... you are effectively shifting all period end date by 1 day. Why??
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5761:
URL: https://github.com/apache/fineract/pull/5761#discussion_r3274021624
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsHelper.java:
##########
@@ -166,14 +167,28 @@ private LocalDate determineInterestPostingPeriodEndDateFrom(final LocalDate peri
} else {
periodEndDate = periodStartDate.withMonth(financialYearBeginningMonth);
}
- periodEndDate = periodEndDate.with(TemporalAdjusters.lastDayOfMonth());
+ periodEndDate = periodEndDate.with(TemporalAdjusters.lastDayOfMonth()).plusDays(1);
+ break;
+ case ANNIVERSARY_MONTHLY:
+ periodEndDate = adjustToAnniversaryDay(periodStartDate.plusMonths(1), anniversaryDayOfMonth);
+ break;
+ case ANNIVERSARY_QUARTERLY:
+ periodEndDate = adjustToAnniversaryDay(periodStartDate.plusMonths(3), anniversaryDayOfMonth);
+ break;
+ case ANNIVERSARY_BIANNUAL:
+ periodEndDate = adjustToAnniversaryDay(periodStartDate.plusMonths(6), anniversaryDayOfMonth);
+ break;
+ case ANNIVERSARY_ANNUAL:
+ periodEndDate = adjustToAnniversaryDay(periodStartDate.plusMonths(12), anniversaryDayOfMonth);
break;
}
- // interest posting always occurs on next day after the period end date.
Review Comment:
Please put this comment on top of every places where we are adding an extra day to explain the reason:
`// interest posting always occurs on next day after the period end date.`
--
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-1185: Add anniversary interest posting period types [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5761:
URL: https://github.com/apache/fineract/pull/5761#issuecomment-4344620653
@SamaSVM Please rebase this 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-1185: Add anniversary interest posting period types [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5761:
URL: https://github.com/apache/fineract/pull/5761#discussion_r3122985933
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsHelper.java:
##########
@@ -168,12 +169,28 @@ private LocalDate determineInterestPostingPeriodEndDateFrom(final LocalDate peri
}
periodEndDate = periodEndDate.with(TemporalAdjusters.lastDayOfMonth());
break;
+ case ANNIVERSARY_MONTHLY:
+ periodEndDate = adjustToAnniversaryDay(periodStartDate.plusMonths(1), anniversaryDayOfMonth).minusDays(1);
Review Comment:
minusDays(1) seems a hack to circumvent:
```
// interest posting always occurs on next day after the period end date.
periodEndDate = periodEndDate.plusDays(1);
```
--
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