You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@fineract.apache.org by sachinkulkarni12 <gi...@git.apache.org> on 2016/02/26 09:34:16 UTC
[GitHub] incubator-fineract pull request: adding organisation start in glob...
GitHub user sachinkulkarni12 opened a pull request:
https://github.com/apache/incubator-fineract/pull/12
adding organisation start in global configuartion
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sachinkulkarni12/incubator-fineract CC-58
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-fineract/pull/12.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #12
----
commit 757da409f34dc63731f5c1321927f348ae31ba78
Author: sachinkulkarni12 <sa...@confluxtechnologies.com>
Date: 2016-02-26T08:30:46Z
adding organisation start in global configuartion
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: adding organisation start in glob...
Posted by sachinkulkarni12 <gi...@git.apache.org>.
Github user sachinkulkarni12 closed the pull request at:
https://github.com/apache/incubator-fineract/pull/12
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: adding organisation start in glob...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/12#discussion_r54368307
--- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java ---
@@ -1566,15 +1571,20 @@ public LoanTermVariationsData mapRow(final ResultSet rs, @SuppressWarnings("unus
.append(" where ((ls.fee_charges_amount <> if(ls.accrual_fee_charges_derived is null,0, ls.accrual_fee_charges_derived))")
.append(" or ( ls.penalty_charges_amount <> if(ls.accrual_penalty_charges_derived is null,0,ls.accrual_penalty_charges_derived))")
.append(" or ( ls.interest_amount <> if(ls.accrual_interest_derived is null,0,ls.accrual_interest_derived)))")
- .append(" and loan.loan_status_id=? and mpl.accounting_type=? and loan.is_npa=0 and ls.duedate <= CURDATE() order by loan.id,ls.duedate");
- return this.jdbcTemplate.query(sqlBuilder.toString(), mapper, new Object[] { LoanStatus.ACTIVE.getValue(),
- AccountingRuleType.ACCRUAL_PERIODIC.getValue() });
+ .append(" and ls.duedate > :organisationstartdate and loan.loan_status_id=:active and mpl.accounting_type=:type and loan.is_npa=0 and ls.duedate <= CURDATE() order by loan.id,ls.duedate");
+ Map<String, Object> paramMap = new HashMap<>(3);
+ paramMap.put("active", LoanStatus.ACTIVE.getValue());
+ paramMap.put("type", AccountingRuleType.ACCRUAL_PERIODIC.getValue());
+ paramMap.put("organisationstartdate", formatter.print(new LocalDate(organisationStartDate)));
--- End diff --
what happens when organisationStartDate is disabled?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: adding organisation start in glob...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/12#discussion_r54368261
--- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java ---
@@ -454,7 +457,7 @@ public void recalculateAccruals(Loan loan) {
Set<LoanCharge> loanCharges = loan.charges();
for (LoanRepaymentScheduleInstallment installment : installments) {
- if (!accruedTill.isBefore(installment.getDueDate())
+ if (isOrganisationDateEnabled && new LocalDate(organisationStartDate).isBefore(installment.getDueDate()) && !accruedTill.isBefore(installment.getDueDate())
--- End diff --
If organisationStartDate is not enabled.... this value might be null.... is this scenario tested?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: adding organisation start in glob...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/12#discussion_r54368208
--- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java ---
@@ -437,6 +438,8 @@ public void reverseTransfer(final LoanTransaction loanTransaction) {
@Override
public void recalculateAccruals(Loan loan) {
LocalDate accruedTill = loan.getAccruedTill();
+ boolean isOrganisationDateEnabled = this.configurationDomainService.isOrganisationstartDateEnable();
+ Date organisationStartDate = this.configurationDomainService.retriveOrganisationStartDate();
--- End diff --
Do this only when Start Date is enabled.... one DB query can be avoided....
Also it is better to keep API spellings clean
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
RE: [GitHub] incubator-fineract pull request: adding organisation
start in glob...
Posted by Markus Geiß <ma...@live.de>.
Nope, looks good to me.
Best,
Markus
.::YAGNI likes a DRY KISS::.
> Subject: Re: [GitHub] incubator-fineract pull request: adding organisation start in glob...
> From: jim@jaguNET.com
> Date: Fri, 26 Feb 2016 13:21:03 -0500
> To: dev@fineract.incubator.apache.org
>
> Anyone have issues w/ this?
>
> > On Feb 26, 2016, at 3:34 AM, sachinkulkarni12 <gi...@git.apache.org> wrote:
> >
> > GitHub user sachinkulkarni12 opened a pull request:
> >
> > https://github.com/apache/incubator-fineract/pull/12
> >
> > adding organisation start in global configuartion
> >
> >
> >
> > You can merge this pull request into a Git repository by running:
> >
> > $ git pull https://github.com/sachinkulkarni12/incubator-fineract CC-58
> >
> > Alternatively you can review and apply these changes as the patch at:
> >
> > https://github.com/apache/incubator-fineract/pull/12.patch
> >
> > To close this pull request, make a commit to your master/trunk branch
> > with (at least) the following in the commit message:
> >
> > This closes #12
> >
> > ----
> > commit 757da409f34dc63731f5c1321927f348ae31ba78
> > Author: sachinkulkarni12 <sa...@confluxtechnologies.com>
> > Date: 2016-02-26T08:30:46Z
> >
> > adding organisation start in global configuartion
> >
> > ----
> >
> >
> > ---
> > If your project is set up for it, you can reply to this email and have your
> > reply appear on GitHub as well. If your project does not have this feature
> > enabled and wishes so, or if the feature is enabled but not working, please
> > contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> > with INFRA.
> > ---
>
Re: [GitHub] incubator-fineract pull request: adding organisation start in glob...
Posted by Jim Jagielski <ji...@jaguNET.com>.
Anyone have issues w/ this?
> On Feb 26, 2016, at 3:34 AM, sachinkulkarni12 <gi...@git.apache.org> wrote:
>
> GitHub user sachinkulkarni12 opened a pull request:
>
> https://github.com/apache/incubator-fineract/pull/12
>
> adding organisation start in global configuartion
>
>
>
> You can merge this pull request into a Git repository by running:
>
> $ git pull https://github.com/sachinkulkarni12/incubator-fineract CC-58
>
> Alternatively you can review and apply these changes as the patch at:
>
> https://github.com/apache/incubator-fineract/pull/12.patch
>
> To close this pull request, make a commit to your master/trunk branch
> with (at least) the following in the commit message:
>
> This closes #12
>
> ----
> commit 757da409f34dc63731f5c1321927f348ae31ba78
> Author: sachinkulkarni12 <sa...@confluxtechnologies.com>
> Date: 2016-02-26T08:30:46Z
>
> adding organisation start in global configuartion
>
> ----
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
[GitHub] incubator-fineract pull request: adding organisation start in glob...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/12#discussion_r54368112
--- Diff: fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/data/GlobalConfigurationDataValidator.java ---
@@ -71,6 +73,11 @@ public void validateForUpdate(final JsonCommand command) {
final Long valueStr = this.fromApiJsonHelper.extractLongNamed(VALUE, element);
baseDataValidator.reset().parameter(ENABLED).value(valueStr).zeroOrPositiveAmount();
}
+
+ if (this.fromApiJsonHelper.parameterExists(DATE_VALUE, element)) {
+ final LocalDate dateValue = this.fromApiJsonHelper.extractLocalDateNamed(DATE_VALUE, element);
+ baseDataValidator.reset().parameter(DATE_VALUE).value(dateValue).ignoreIfNull();
--- End diff --
Nothing is being validated? looks like missing validation
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: adding organisation start in glob...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/12#discussion_r54368300
--- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java ---
@@ -1559,6 +1563,7 @@ public LoanTermVariationsData mapRow(final ResultSet rs, @SuppressWarnings("unus
public Collection<LoanScheduleAccrualData> retriveScheduleAccrualData() {
LoanScheduleAccrualMapper mapper = new LoanScheduleAccrualMapper();
+ final Date organisationStartDate = this.configurationDomainService.retriveOrganisationStartDate();
--- End diff --
what happens when organisationStartDate is disabled?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: adding organisation start in glob...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/12#discussion_r54368217
--- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java ---
@@ -437,6 +438,8 @@ public void reverseTransfer(final LoanTransaction loanTransaction) {
@Override
public void recalculateAccruals(Loan loan) {
LocalDate accruedTill = loan.getAccruedTill();
+ boolean isOrganisationDateEnabled = this.configurationDomainService.isOrganisationstartDateEnable();
--- End diff --
Make the API as Enabled.... it is better for readability
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---