You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by angelboxes <gi...@git.apache.org> on 2018/07/30 23:14:51 UTC

[GitHub] fineract pull request #465: Fineract 614: Rates module

GitHub user angelboxes opened a pull request:

    https://github.com/apache/fineract/pull/465

    Fineract 614: Rates module

    Previously defined sub-rates can be added to set the nominal interest rate of a loan product. When a loan account is created you can choose which rates from the associated rates to the loan product should be applied to the new loan account. This is an optional way of defining the nominal rate for loans.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/angelboxes/fineract FINERACT-614

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/fineract/pull/465.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 #465
    
----
commit 471a4c88fbcaec4f9200adc32a38a7d428404e20
Author: Angel Cajas <an...@...>
Date:   2018-07-30T22:52:17Z

    A new rates module was added to define new rates that can be used to set min and max nominal interest rate, when a new loan account is created they can be used to determine which rates may be applicable for the loan account.

----


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r207369492
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/api/LoansApiResource.java ---
    @@ -178,30 +180,30 @@
         private final EntityDatatableChecksReadService entityDatatableChecksReadService;
         private final BulkImportWorkbookService bulkImportWorkbookService;
         private final BulkImportWorkbookPopulatorService bulkImportWorkbookPopulatorService;
    -
    +    private final RateReadService rateReadService;
     
         @Autowired
         public LoansApiResource(final PlatformSecurityContext context, final LoanReadPlatformService loanReadPlatformService,
    -            final LoanProductReadPlatformService loanProductReadPlatformService,
    -            final LoanDropdownReadPlatformService dropdownReadPlatformService, final FundReadPlatformService fundReadPlatformService,
    -            final ChargeReadPlatformService chargeReadPlatformService, final LoanChargeReadPlatformService loanChargeReadPlatformService,
    -            final CollateralReadPlatformService loanCollateralReadPlatformService,
    -            final LoanScheduleCalculationPlatformService calculationPlatformService,
    -            final GuarantorReadPlatformService guarantorReadPlatformService,
    -            final CodeValueReadPlatformService codeValueReadPlatformService, final GroupReadPlatformService groupReadPlatformService,
    -            final DefaultToApiJsonSerializer<LoanAccountData> toApiJsonSerializer,
    -            final DefaultToApiJsonSerializer<LoanApprovalData> loanApprovalDataToApiJsonSerializer,
    -            final DefaultToApiJsonSerializer<LoanScheduleData> loanScheduleToApiJsonSerializer,
    -            final ApiRequestParameterHelper apiRequestParameterHelper, final FromJsonHelper fromJsonHelper,
    -            final PortfolioCommandSourceWritePlatformService commandsSourceWritePlatformService,
    -            final CalendarReadPlatformService calendarReadPlatformService, final NoteReadPlatformServiceImpl noteReadPlatformService,
    -            final PortfolioAccountReadPlatformService portfolioAccountReadPlatformServiceImpl,
    -            final AccountAssociationsReadPlatformService accountAssociationsReadPlatformService,
    -            final LoanScheduleHistoryReadPlatformService loanScheduleHistoryReadPlatformService,
    -            final AccountDetailsReadPlatformService accountDetailsReadPlatformService,
    -            final EntityDatatableChecksReadService entityDatatableChecksReadService,
    -            final BulkImportWorkbookService bulkImportWorkbookService,
    -            final BulkImportWorkbookPopulatorService bulkImportWorkbookPopulatorService) {
    +        final LoanProductReadPlatformService loanProductReadPlatformService,
    +        final LoanDropdownReadPlatformService dropdownReadPlatformService, final FundReadPlatformService fundReadPlatformService,
    +        final ChargeReadPlatformService chargeReadPlatformService, final LoanChargeReadPlatformService loanChargeReadPlatformService,
    +        final CollateralReadPlatformService loanCollateralReadPlatformService,
    +        final LoanScheduleCalculationPlatformService calculationPlatformService,
    +        final GuarantorReadPlatformService guarantorReadPlatformService,
    +        final CodeValueReadPlatformService codeValueReadPlatformService, final GroupReadPlatformService groupReadPlatformService,
    +        final DefaultToApiJsonSerializer<LoanAccountData> toApiJsonSerializer,
    +        final DefaultToApiJsonSerializer<LoanApprovalData> loanApprovalDataToApiJsonSerializer,
    +        final DefaultToApiJsonSerializer<LoanScheduleData> loanScheduleToApiJsonSerializer,
    +        final ApiRequestParameterHelper apiRequestParameterHelper, final FromJsonHelper fromJsonHelper,
    +        final PortfolioCommandSourceWritePlatformService commandsSourceWritePlatformService,
    +        final CalendarReadPlatformService calendarReadPlatformService, final NoteReadPlatformServiceImpl noteReadPlatformService,
    +        final PortfolioAccountReadPlatformService portfolioAccountReadPlatformServiceImpl,
    +        final AccountAssociationsReadPlatformService accountAssociationsReadPlatformService,
    +        final LoanScheduleHistoryReadPlatformService loanScheduleHistoryReadPlatformService,
    +        final AccountDetailsReadPlatformService accountDetailsReadPlatformService,
    +        final EntityDatatableChecksReadService entityDatatableChecksReadService, RateReadService rateReadService,
    +        final BulkImportWorkbookService bulkImportWorkbookService,
    --- End diff --
    
    Ok.


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r207369167
  
    --- Diff: fineract-provider/src/main/resources/sql/migrations/core_db/V343__rates.sql ---
    @@ -0,0 +1,62 @@
    +--
    +-- 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.
    +--
    +
    +CREATE TABLE IF NOT EXISTS `m_rate` (
    +  `id` bigint(20) NOT NULL AUTO_INCREMENT,
    +  `name` varchar(250) NOT NULL,
    +  `percentage` decimal(10,2) NOT NULL,
    +  `active` tinyint(1) DEFAULT '0',
    +  `product_apply` varchar(100) NOT NULL,
    +  `created_date` datetime NULL DEFAULT NULL,
    +  `createdby_id` bigint(20) NOT NULL,
    +  `lastmodifiedby_id` bigint(20) NOT NULL,
    +  `lastmodified_date` datetime NULL DEFAULT NULL,
    +  `approve_user` bigint(20) DEFAULT NULL,
    +  PRIMARY KEY (`id`),
    +  KEY `FK_M_RATE_CREATE_USER` (`createdby_id`),
    +  KEY `FK_M_RATE_APPROVE_USER` (`approve_user`),
    +  CONSTRAINT `FK_M_RATE_APPROVE_USER` FOREIGN KEY (`approve_user`) REFERENCES `m_appuser` (`id`),
    +  CONSTRAINT `FK_M_RATE_CREATE_USER` FOREIGN KEY (`createdby_id`) REFERENCES `m_appuser` (`id`)
    +) ENGINE=InnoDB AUTO_INCREMENT=21 DEFAULT CHARSET=latin1;
    --- End diff --
    
    Some of the scripts included this part and we guessed it was needed. We'll remove all of them in this new script.


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206853888
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/service/LoanProductWritePlatformServiceJpaRepositoryImpl.java ---
    @@ -304,6 +319,32 @@ private boolean anyChangeInCriticalFloatingRateLinkedParams(JsonCommand command,
             return charges;
         }
     
    +    private List<Rate> assembleListOfProductRates(final JsonCommand command) {
    +
    +        final List<Rate> rates = new ArrayList<>();
    +
    +        if (command.parameterExists("rates")) {
    +            final JsonArray ratesArray = command.arrayOfParameterNamed("rates");
    +            if (ratesArray != null) {
    +                for (int i = 0; i < ratesArray.size(); i++) {
    +
    +                    final JsonObject jsonObject = ratesArray.get(i).getAsJsonObject();
    +                    if (jsonObject.has("id")) {
    +                        final Long id = jsonObject.get("id").getAsLong();
    +
    +                        final Rate rate = this.rateRepository.findOne(id);
    --- End diff --
    
    Instead of repeated calls to DB, can you try using single query with in clause and validate 


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206853841
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/service/LoanProductReadPlatformServiceImpl.java ---
    @@ -465,7 +473,7 @@ public LoanProductData mapRow(final ResultSet rs, @SuppressWarnings("unused") fi
                         installmentAmountInMultiplesOf, allowAttributeOverrides, isLinkedToFloatingInterestRates, floatingRateId,
                         floatingRateName, interestRateDifferential, minDifferentialLendingRate, defaultDifferentialLendingRate,
                         maxDifferentialLendingRate, isFloatingInterestRateCalculationAllowed, isVariableIntallmentsAllowed, minimumGap,
    -                    maximumGap, syncExpectedWithDisbursementDate, canUseForTopup, isEqualAmortization);
    +                    maximumGap, syncExpectedWithDisbursementDate, canUseForTopup, isEqualAmortization, null, this.rates);
    --- End diff --
    
    instead of directly passing null, initialize to a variable (variable name similar to the one in called method ) and pass it, for readability. And follow similar approach in all other places


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206854038
  
    --- Diff: fineract-provider/src/main/resources/sql/migrations/core_db/V343__rates.sql ---
    @@ -0,0 +1,62 @@
    +--
    +-- 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.
    +--
    +
    +CREATE TABLE IF NOT EXISTS `m_rate` (
    +  `id` bigint(20) NOT NULL AUTO_INCREMENT,
    +  `name` varchar(250) NOT NULL,
    +  `percentage` decimal(10,2) NOT NULL,
    +  `active` tinyint(1) DEFAULT '0',
    +  `product_apply` varchar(100) NOT NULL,
    +  `created_date` datetime NULL DEFAULT NULL,
    +  `createdby_id` bigint(20) NOT NULL,
    +  `lastmodifiedby_id` bigint(20) NOT NULL,
    +  `lastmodified_date` datetime NULL DEFAULT NULL,
    +  `approve_user` bigint(20) DEFAULT NULL,
    +  PRIMARY KEY (`id`),
    +  KEY `FK_M_RATE_CREATE_USER` (`createdby_id`),
    +  KEY `FK_M_RATE_APPROVE_USER` (`approve_user`),
    +  CONSTRAINT `FK_M_RATE_APPROVE_USER` FOREIGN KEY (`approve_user`) REFERENCES `m_appuser` (`id`),
    +  CONSTRAINT `FK_M_RATE_CREATE_USER` FOREIGN KEY (`createdby_id`) REFERENCES `m_appuser` (`id`)
    +) ENGINE=InnoDB AUTO_INCREMENT=21 DEFAULT CHARSET=latin1;
    --- End diff --
    
    remove ENGINE=InnoDB AUTO_INCREMENT=21 DEFAULT CHARSET=latin1;


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206853418
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/api/LoansApiResource.java ---
    @@ -178,30 +180,30 @@
         private final EntityDatatableChecksReadService entityDatatableChecksReadService;
         private final BulkImportWorkbookService bulkImportWorkbookService;
         private final BulkImportWorkbookPopulatorService bulkImportWorkbookPopulatorService;
    -
    +    private final RateReadService rateReadService;
     
         @Autowired
         public LoansApiResource(final PlatformSecurityContext context, final LoanReadPlatformService loanReadPlatformService,
    -            final LoanProductReadPlatformService loanProductReadPlatformService,
    -            final LoanDropdownReadPlatformService dropdownReadPlatformService, final FundReadPlatformService fundReadPlatformService,
    -            final ChargeReadPlatformService chargeReadPlatformService, final LoanChargeReadPlatformService loanChargeReadPlatformService,
    -            final CollateralReadPlatformService loanCollateralReadPlatformService,
    -            final LoanScheduleCalculationPlatformService calculationPlatformService,
    -            final GuarantorReadPlatformService guarantorReadPlatformService,
    -            final CodeValueReadPlatformService codeValueReadPlatformService, final GroupReadPlatformService groupReadPlatformService,
    -            final DefaultToApiJsonSerializer<LoanAccountData> toApiJsonSerializer,
    -            final DefaultToApiJsonSerializer<LoanApprovalData> loanApprovalDataToApiJsonSerializer,
    -            final DefaultToApiJsonSerializer<LoanScheduleData> loanScheduleToApiJsonSerializer,
    -            final ApiRequestParameterHelper apiRequestParameterHelper, final FromJsonHelper fromJsonHelper,
    -            final PortfolioCommandSourceWritePlatformService commandsSourceWritePlatformService,
    -            final CalendarReadPlatformService calendarReadPlatformService, final NoteReadPlatformServiceImpl noteReadPlatformService,
    -            final PortfolioAccountReadPlatformService portfolioAccountReadPlatformServiceImpl,
    -            final AccountAssociationsReadPlatformService accountAssociationsReadPlatformService,
    -            final LoanScheduleHistoryReadPlatformService loanScheduleHistoryReadPlatformService,
    -            final AccountDetailsReadPlatformService accountDetailsReadPlatformService,
    -            final EntityDatatableChecksReadService entityDatatableChecksReadService,
    -            final BulkImportWorkbookService bulkImportWorkbookService,
    -            final BulkImportWorkbookPopulatorService bulkImportWorkbookPopulatorService) {
    +        final LoanProductReadPlatformService loanProductReadPlatformService,
    +        final LoanDropdownReadPlatformService dropdownReadPlatformService, final FundReadPlatformService fundReadPlatformService,
    +        final ChargeReadPlatformService chargeReadPlatformService, final LoanChargeReadPlatformService loanChargeReadPlatformService,
    +        final CollateralReadPlatformService loanCollateralReadPlatformService,
    +        final LoanScheduleCalculationPlatformService calculationPlatformService,
    +        final GuarantorReadPlatformService guarantorReadPlatformService,
    +        final CodeValueReadPlatformService codeValueReadPlatformService, final GroupReadPlatformService groupReadPlatformService,
    +        final DefaultToApiJsonSerializer<LoanAccountData> toApiJsonSerializer,
    +        final DefaultToApiJsonSerializer<LoanApprovalData> loanApprovalDataToApiJsonSerializer,
    +        final DefaultToApiJsonSerializer<LoanScheduleData> loanScheduleToApiJsonSerializer,
    +        final ApiRequestParameterHelper apiRequestParameterHelper, final FromJsonHelper fromJsonHelper,
    +        final PortfolioCommandSourceWritePlatformService commandsSourceWritePlatformService,
    +        final CalendarReadPlatformService calendarReadPlatformService, final NoteReadPlatformServiceImpl noteReadPlatformService,
    +        final PortfolioAccountReadPlatformService portfolioAccountReadPlatformServiceImpl,
    +        final AccountAssociationsReadPlatformService accountAssociationsReadPlatformService,
    +        final LoanScheduleHistoryReadPlatformService loanScheduleHistoryReadPlatformService,
    +        final AccountDetailsReadPlatformService accountDetailsReadPlatformService,
    +        final EntityDatatableChecksReadService entityDatatableChecksReadService, RateReadService rateReadService,
    +        final BulkImportWorkbookService bulkImportWorkbookService,
    --- End diff --
    
    Kindly make RateReadService rateReadService as the last parameter of the method


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r208853745
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanUtilService.java ---
    @@ -135,10 +135,10 @@ public ScheduleGeneratorDTO buildScheduleGeneratorDTO(final Loan loan, final Loc
             boolean isSkipRepaymentOnFirstMonthEnabled = configurationDomainService.isSkippingMeetingOnFirstDayOfMonthEnabled();
             if(isSkipRepaymentOnFirstMonthEnabled){
                 isSkipRepaymentOnFirstMonth = isLoanRepaymentsSyncWithMeeting(loan.group(), calendar);
    -            if(isSkipRepaymentOnFirstMonth) { numberOfDays = configurationDomainService.retreivePeroidInNumberOfDaysForSkipMeetingDate().intValue(); } 
    +            if(isSkipRepaymentOnFirstMonth) { numberOfDays = configurationDomainService.retreivePeroidInNumberOfDaysForSkipMeetingDate().intValue(); }
    --- End diff --
    
    No changes are made to this file. Kindly remove this file from PR


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206853697
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanUtilService.java ---
    @@ -325,4 +332,35 @@ private LocalDate generateCalculatedRepaymentStartDate(final CalendarHistoryData
             return disbursementDatas;
         }
     
    +    public List<Rate> fetchRateData(final JsonArray element) {
    +        List<Rate> rates = new ArrayList<>();
    +
    +        if (element != null) {
    +            for (JsonElement jsonElement : element) {
    +                final JsonObject rateElement = jsonElement.getAsJsonObject();
    +                if (rateElement.has("id")) {
    +                    final Long rateId = this.fromApiJsonHelper.extractLongNamed("id", rateElement);
    +                    Rate rate = findRateByIdIfProvided(rateId);
    +                    rates.add(rate);
    +                }
    +            }
    +        }else{
    +            System.out.println("");
    +        }
    +
    +        return rates;
    +    }
    +
    +    public Rate findRateByIdIfProvided(final Long rateId) {
    +        Rate rate = null;
    +        if (rateId != null) {
    +            rate = this.rateRepository.findOne(rateId);
    --- End diff --
    
    As per the fineract existing code its advised to access repository methods using wrapper class


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r208664567
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/service/LoanProductReadPlatformServiceImpl.java ---
    @@ -465,7 +473,7 @@ public LoanProductData mapRow(final ResultSet rs, @SuppressWarnings("unused") fi
                         installmentAmountInMultiplesOf, allowAttributeOverrides, isLinkedToFloatingInterestRates, floatingRateId,
                         floatingRateName, interestRateDifferential, minDifferentialLendingRate, defaultDifferentialLendingRate,
                         maxDifferentialLendingRate, isFloatingInterestRateCalculationAllowed, isVariableIntallmentsAllowed, minimumGap,
    -                    maximumGap, syncExpectedWithDisbursementDate, canUseForTopup, isEqualAmortization);
    +                    maximumGap, syncExpectedWithDisbursementDate, canUseForTopup, isEqualAmortization, null, this.rates);
    --- End diff --
    
    Ok


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206768853
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/serialization/JsonParserHelper.java ---
    @@ -454,7 +454,7 @@ public static LocalDateTime convertDateTimeFrom(final String dateTimeAsString, f
             LocalDateTime eventLocalDateTime = null;
             if (StringUtils.isNotBlank(dateTimeAsString)) {
                 try {
    -                eventLocalDateTime = DateTimeFormat.forPattern(dateTimeFormat).withLocale(clientApplicationLocale)
    +            eventLocalDateTime = DateTimeFormat.forPattern(dateTimeFormat).withLocale(clientApplicationLocale)
    --- End diff --
    
    Since no changes are made in this file, kindly remove this file from PR


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r208863144
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/service/LoanProductWritePlatformServiceJpaRepositoryImpl.java ---
    @@ -304,6 +318,29 @@ private boolean anyChangeInCriticalFloatingRateLinkedParams(JsonCommand command,
             return charges;
         }
     
    +    private List<Rate> assembleListOfProductRates(final JsonCommand command) {
    +
    +        final List<Rate> rates = new ArrayList<>();
    +
    +        if (command.parameterExists("rates")) {
    +            final JsonArray ratesArray = command.arrayOfParameterNamed("rates");
    +            if (ratesArray != null) {
    +                List<Long> idList = new ArrayList<>();
    +                for (int i = 0; i < ratesArray.size(); i++) {
    +                    final JsonObject jsonObject = ratesArray.get(i).getAsJsonObject();
    +                    if (jsonObject.has("id")) {
    +                        idList.add(jsonObject.get("id").getAsLong());
    +                    }
    +                }
    +                System.out.println(idList.toString());
    --- End diff --
    
    remove Print statement


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206853778
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProduct.java ---
    @@ -102,6 +103,10 @@
         @JoinTable(name = "m_product_loan_charge", joinColumns = @JoinColumn(name = "product_loan_id"), inverseJoinColumns = @JoinColumn(name = "charge_id"))
         private List<Charge> charges;
     
    +    @ManyToMany(fetch = FetchType.EAGER)
    +    @JoinTable(name = "m_product_loan_rate", joinColumns = @JoinColumn(name = "product_loan_id"), inverseJoinColumns = @JoinColumn(name = "rate_id"))
    --- End diff --
    
    @ManyToMany or OneToMany
    Can the functionality be achieved by using fetch type lazy


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r208390127
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/service/LoanProductWritePlatformServiceJpaRepositoryImpl.java ---
    @@ -304,6 +319,32 @@ private boolean anyChangeInCriticalFloatingRateLinkedParams(JsonCommand command,
             return charges;
         }
     
    +    private List<Rate> assembleListOfProductRates(final JsonCommand command) {
    +
    +        final List<Rate> rates = new ArrayList<>();
    +
    +        if (command.parameterExists("rates")) {
    +            final JsonArray ratesArray = command.arrayOfParameterNamed("rates");
    +            if (ratesArray != null) {
    +                for (int i = 0; i < ratesArray.size(); i++) {
    +
    +                    final JsonObject jsonObject = ratesArray.get(i).getAsJsonObject();
    +                    if (jsonObject.has("id")) {
    +                        final Long id = jsonObject.get("id").getAsLong();
    +
    +                        final Rate rate = this.rateRepository.findOne(id);
    --- End diff --
    
    Ok


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r208863155
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/service/LoanProductWritePlatformServiceJpaRepositoryImpl.java ---
    @@ -304,6 +318,29 @@ private boolean anyChangeInCriticalFloatingRateLinkedParams(JsonCommand command,
             return charges;
         }
     
    +    private List<Rate> assembleListOfProductRates(final JsonCommand command) {
    +
    +        final List<Rate> rates = new ArrayList<>();
    +
    +        if (command.parameterExists("rates")) {
    +            final JsonArray ratesArray = command.arrayOfParameterNamed("rates");
    +            if (ratesArray != null) {
    +                List<Long> idList = new ArrayList<>();
    +                for (int i = 0; i < ratesArray.size(); i++) {
    +                    final JsonObject jsonObject = ratesArray.get(i).getAsJsonObject();
    +                    if (jsonObject.has("id")) {
    +                        idList.add(jsonObject.get("id").getAsLong());
    +                    }
    +                }
    +                System.out.println(idList.toString());
    +                rates.addAll(this.rateRepository.findMultipleWithNotFoundDetection(idList));
    +                System.out.println(rates.toString());
    --- End diff --
    
    remove Print statement


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r208863491
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/rate/serialization/RateDefinitionCommandFromApiJsonDeserializer.java ---
    @@ -0,0 +1,128 @@
    +/**
    + * 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.rate.serialization;
    +
    +import com.google.gson.JsonElement;
    +import com.google.gson.reflect.TypeToken;
    +import java.lang.reflect.Type;
    +import java.math.BigDecimal;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import org.apache.commons.lang.StringUtils;
    +import org.apache.fineract.infrastructure.core.data.ApiParameterError;
    +import org.apache.fineract.infrastructure.core.data.DataValidatorBuilder;
    +import org.apache.fineract.infrastructure.core.exception.InvalidJsonException;
    +import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
    +import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper;
    +import org.springframework.beans.factory.annotation.Autowired;
    +import org.springframework.stereotype.Component;
    +
    +@Component
    +public class RateDefinitionCommandFromApiJsonDeserializer {
    +
    +  /**
    +   * The parameters supported for this command.
    +   */
    +  private final Set<String> supportedParameters = new HashSet<>(
    +      Arrays.asList("id", "name", "percentage", "productApply", "active", "approveUser", "locale"));
    +
    +  private final FromJsonHelper fromApiJsonHelper;
    +
    +  @Autowired
    +  public RateDefinitionCommandFromApiJsonDeserializer(final FromJsonHelper fromApiJsonHelper) {
    +    this.fromApiJsonHelper = fromApiJsonHelper;
    +  }
    +
    +  public void validateForCreate(final String json) {
    +    if (StringUtils.isBlank(json)) {
    +      throw new InvalidJsonException();
    +    }
    +
    +    final Type typeOfMap = new TypeToken<Map<String, Object>>() {
    +    }.getType();
    +
    +    this.fromApiJsonHelper.checkForUnsupportedParameters(typeOfMap, json, this.supportedParameters);
    +
    +    final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
    +
    +    final DataValidatorBuilder baseDataValidator = new DataValidatorBuilder(dataValidationErrors)
    +        .resource("rate");
    +
    +    final JsonElement element = this.fromApiJsonHelper.parse(json);
    +
    +    final String name = this.fromApiJsonHelper.extractStringNamed("name", element);
    --- End diff --
    
    Its advised to keep all parameter names in a constant file and use


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r207370679
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java ---
    @@ -392,20 +395,24 @@
         @OneToOne(cascade = CascadeType.ALL, mappedBy = "loan", optional = true, orphanRemoval = true, fetch=FetchType.EAGER)
         private LoanTopupDetails loanTopupDetails;
     
    +    @ManyToMany(fetch = FetchType.EAGER)
    --- End diff --
    
    About this change I think that ManyToMany is okay as we use a third table to join the loan and rate tables just like in the LoanProduct class. I'll make the change and test if the same result can be achieved.


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r207358094
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanUtilService.java ---
    @@ -325,4 +332,35 @@ private LocalDate generateCalculatedRepaymentStartDate(final CalendarHistoryData
             return disbursementDatas;
         }
     
    +    public List<Rate> fetchRateData(final JsonArray element) {
    +        List<Rate> rates = new ArrayList<>();
    +
    +        if (element != null) {
    +            for (JsonElement jsonElement : element) {
    +                final JsonObject rateElement = jsonElement.getAsJsonObject();
    +                if (rateElement.has("id")) {
    +                    final Long rateId = this.fromApiJsonHelper.extractLongNamed("id", rateElement);
    +                    Rate rate = findRateByIdIfProvided(rateId);
    +                    rates.add(rate);
    +                }
    +            }
    +        }else{
    +            System.out.println("");
    +        }
    +
    +        return rates;
    +    }
    +
    +    public Rate findRateByIdIfProvided(final Long rateId) {
    --- End diff --
    
    Ok. The name is based on the names used by the methods in the class LoanAssembler. We were just following what we assumed it was a name pattern.


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r207365498
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanUtilService.java ---
    @@ -325,4 +332,35 @@ private LocalDate generateCalculatedRepaymentStartDate(final CalendarHistoryData
             return disbursementDatas;
         }
     
    +    public List<Rate> fetchRateData(final JsonArray element) {
    +        List<Rate> rates = new ArrayList<>();
    +
    +        if (element != null) {
    +            for (JsonElement jsonElement : element) {
    +                final JsonObject rateElement = jsonElement.getAsJsonObject();
    +                if (rateElement.has("id")) {
    +                    final Long rateId = this.fromApiJsonHelper.extractLongNamed("id", rateElement);
    +                    Rate rate = findRateByIdIfProvided(rateId);
    +                    rates.add(rate);
    +                }
    +            }
    +        }else{
    +            System.out.println("");
    +        }
    +
    +        return rates;
    +    }
    +
    +    public Rate findRateByIdIfProvided(final Long rateId) {
    +        Rate rate = null;
    +        if (rateId != null) {
    +            rate = this.rateRepository.findOne(rateId);
    --- End diff --
    
    I'll use a wrapper class, I just noticed that it is used for the Loan repository but there are some others that don't make use of it so it was a bit misleading.


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r209071812
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/rate/domain/RateRepositoryWrapper.java ---
    @@ -0,0 +1,72 @@
    +/**
    + * 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.rate.domain;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Objects;
    +import org.apache.fineract.portfolio.rate.exception.RateNotFoundException;
    +import org.springframework.beans.factory.annotation.Autowired;
    +import org.springframework.stereotype.Service;
    +
    +@Service
    +public class RateRepositoryWrapper {
    +
    +  private final RateRepository repository;
    +
    +  @Autowired
    +  public RateRepositoryWrapper(final RateRepository repository) {
    +    this.repository = repository;
    +  }
    +
    +  public Rate findOneWithNotFoundDetection(final Long rateId) {
    +
    +    final Rate rate = this.repository.findOne(rateId);
    +    if (rate == null) {
    +      throw new RateNotFoundException(rateId);
    +    }
    +
    +    return rate;
    +  }
    +
    +  public List<Rate> findMultipleWithNotFoundDetection(final List<Long> rateIds) {
    +    List<Rate> rates = new ArrayList<>();
    +    if (rateIds != null && !rateIds.isEmpty()) {
    +      final List<Rate> foundRates = this.repository.findAll(rateIds);
    +      System.out.println("ids "+rateIds.toString());
    +      System.out.println("found "+foundRates.toString());
    --- End diff --
    
    Ok


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206854066
  
    --- Diff: fineract-provider/src/main/resources/sql/migrations/core_db/V343__rates.sql ---
    @@ -0,0 +1,62 @@
    +--
    +-- 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.
    +--
    +
    +CREATE TABLE IF NOT EXISTS `m_rate` (
    +  `id` bigint(20) NOT NULL AUTO_INCREMENT,
    +  `name` varchar(250) NOT NULL,
    +  `percentage` decimal(10,2) NOT NULL,
    +  `active` tinyint(1) DEFAULT '0',
    +  `product_apply` varchar(100) NOT NULL,
    +  `created_date` datetime NULL DEFAULT NULL,
    +  `createdby_id` bigint(20) NOT NULL,
    +  `lastmodifiedby_id` bigint(20) NOT NULL,
    +  `lastmodified_date` datetime NULL DEFAULT NULL,
    +  `approve_user` bigint(20) DEFAULT NULL,
    +  PRIMARY KEY (`id`),
    +  KEY `FK_M_RATE_CREATE_USER` (`createdby_id`),
    +  KEY `FK_M_RATE_APPROVE_USER` (`approve_user`),
    +  CONSTRAINT `FK_M_RATE_APPROVE_USER` FOREIGN KEY (`approve_user`) REFERENCES `m_appuser` (`id`),
    +  CONSTRAINT `FK_M_RATE_CREATE_USER` FOREIGN KEY (`createdby_id`) REFERENCES `m_appuser` (`id`)
    +) ENGINE=InnoDB AUTO_INCREMENT=21 DEFAULT CHARSET=latin1;
    +
    +
    +CREATE TABLE IF NOT EXISTS `m_loan_rate` (
    +  `loan_id` bigint(20) NOT NULL,
    +  `rate_id` bigint(20) NOT NULL,
    +  PRIMARY KEY (`loan_id`,`rate_id`),
    +  KEY `FK_M_LOAN_RATE_RATE` (`rate_id`),
    +  CONSTRAINT `FK_M_LOAN_RATE_LOAN` FOREIGN KEY (`loan_id`) REFERENCES `m_loan` (`id`),
    +  CONSTRAINT `FK_M_LOAN_RATE_RATE` FOREIGN KEY (`rate_id`) REFERENCES `m_rate` (`id`)
    +) ENGINE=InnoDB DEFAULT CHARSET=latin1;
    +
    +
    +CREATE TABLE IF NOT EXISTS `m_product_loan_rate` (
    +  `product_loan_id` bigint(20) NOT NULL,
    +  `rate_id` bigint(20) NOT NULL,
    +  PRIMARY KEY (`product_loan_id`,`rate_id`),
    +  KEY `FK_M_PRODUCT_LOAN_RATE_RATE` (`rate_id`),
    +  CONSTRAINT `FK_M_PRODUCT_LOAN_RATE_LOAN` FOREIGN KEY (`product_loan_id`) REFERENCES `m_product_loan` (`id`),
    +  CONSTRAINT `FK_M_PRODUCT_LOAN_RATE_RATE` FOREIGN KEY (`rate_id`) REFERENCES `m_rate` (`id`)
    +) ENGINE=InnoDB DEFAULT CHARSET=latin1;
    --- End diff --
    
    remove  ENGINE=InnoDB DEFAULT CHARSET=latin1;
         
     


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r209071876
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/service/LoanProductWritePlatformServiceJpaRepositoryImpl.java ---
    @@ -304,6 +318,29 @@ private boolean anyChangeInCriticalFloatingRateLinkedParams(JsonCommand command,
             return charges;
         }
     
    +    private List<Rate> assembleListOfProductRates(final JsonCommand command) {
    +
    +        final List<Rate> rates = new ArrayList<>();
    +
    +        if (command.parameterExists("rates")) {
    +            final JsonArray ratesArray = command.arrayOfParameterNamed("rates");
    +            if (ratesArray != null) {
    +                List<Long> idList = new ArrayList<>();
    +                for (int i = 0; i < ratesArray.size(); i++) {
    +                    final JsonObject jsonObject = ratesArray.get(i).getAsJsonObject();
    +                    if (jsonObject.has("id")) {
    +                        idList.add(jsonObject.get("id").getAsLong());
    +                    }
    +                }
    +                System.out.println(idList.toString());
    --- End diff --
    
    Ok


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r207369442
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/serialization/JsonParserHelper.java ---
    @@ -454,7 +454,7 @@ public static LocalDateTime convertDateTimeFrom(final String dateTimeAsString, f
             LocalDateTime eventLocalDateTime = null;
             if (StringUtils.isNotBlank(dateTimeAsString)) {
                 try {
    -                eventLocalDateTime = DateTimeFormat.forPattern(dateTimeFormat).withLocale(clientApplicationLocale)
    +            eventLocalDateTime = DateTimeFormat.forPattern(dateTimeFormat).withLocale(clientApplicationLocale)
    --- End diff --
    
    Ok


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r208863399
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/rate/domain/RateRepositoryWrapper.java ---
    @@ -0,0 +1,72 @@
    +/**
    + * 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.rate.domain;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Objects;
    +import org.apache.fineract.portfolio.rate.exception.RateNotFoundException;
    +import org.springframework.beans.factory.annotation.Autowired;
    +import org.springframework.stereotype.Service;
    +
    +@Service
    +public class RateRepositoryWrapper {
    +
    +  private final RateRepository repository;
    +
    +  @Autowired
    +  public RateRepositoryWrapper(final RateRepository repository) {
    +    this.repository = repository;
    +  }
    +
    +  public Rate findOneWithNotFoundDetection(final Long rateId) {
    +
    +    final Rate rate = this.repository.findOne(rateId);
    +    if (rate == null) {
    +      throw new RateNotFoundException(rateId);
    +    }
    +
    +    return rate;
    +  }
    +
    +  public List<Rate> findMultipleWithNotFoundDetection(final List<Long> rateIds) {
    +    List<Rate> rates = new ArrayList<>();
    +    if (rateIds != null && !rateIds.isEmpty()) {
    +      final List<Rate> foundRates = this.repository.findAll(rateIds);
    +      System.out.println("ids "+rateIds.toString());
    +      System.out.println("found "+foundRates.toString());
    --- End diff --
    
    remove Print statement


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206853956
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/rate/handler/DeleteRateCommandHandler.java ---
    @@ -0,0 +1,26 @@
    +/**
    --- End diff --
    
    Is this required?


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206854050
  
    --- Diff: fineract-provider/src/main/resources/sql/migrations/core_db/V343__rates.sql ---
    @@ -0,0 +1,62 @@
    +--
    +-- 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.
    +--
    +
    +CREATE TABLE IF NOT EXISTS `m_rate` (
    +  `id` bigint(20) NOT NULL AUTO_INCREMENT,
    +  `name` varchar(250) NOT NULL,
    +  `percentage` decimal(10,2) NOT NULL,
    +  `active` tinyint(1) DEFAULT '0',
    +  `product_apply` varchar(100) NOT NULL,
    +  `created_date` datetime NULL DEFAULT NULL,
    +  `createdby_id` bigint(20) NOT NULL,
    +  `lastmodifiedby_id` bigint(20) NOT NULL,
    +  `lastmodified_date` datetime NULL DEFAULT NULL,
    +  `approve_user` bigint(20) DEFAULT NULL,
    +  PRIMARY KEY (`id`),
    +  KEY `FK_M_RATE_CREATE_USER` (`createdby_id`),
    +  KEY `FK_M_RATE_APPROVE_USER` (`approve_user`),
    +  CONSTRAINT `FK_M_RATE_APPROVE_USER` FOREIGN KEY (`approve_user`) REFERENCES `m_appuser` (`id`),
    +  CONSTRAINT `FK_M_RATE_CREATE_USER` FOREIGN KEY (`createdby_id`) REFERENCES `m_appuser` (`id`)
    +) ENGINE=InnoDB AUTO_INCREMENT=21 DEFAULT CHARSET=latin1;
    +
    +
    +CREATE TABLE IF NOT EXISTS `m_loan_rate` (
    +  `loan_id` bigint(20) NOT NULL,
    +  `rate_id` bigint(20) NOT NULL,
    +  PRIMARY KEY (`loan_id`,`rate_id`),
    +  KEY `FK_M_LOAN_RATE_RATE` (`rate_id`),
    +  CONSTRAINT `FK_M_LOAN_RATE_LOAN` FOREIGN KEY (`loan_id`) REFERENCES `m_loan` (`id`),
    +  CONSTRAINT `FK_M_LOAN_RATE_RATE` FOREIGN KEY (`rate_id`) REFERENCES `m_rate` (`id`)
    +) ENGINE=InnoDB DEFAULT CHARSET=latin1;
    --- End diff --
    
    remove ENGINE=InnoDB DEFAULT CHARSET=latin1;


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206853632
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanUtilService.java ---
    @@ -325,4 +332,35 @@ private LocalDate generateCalculatedRepaymentStartDate(final CalendarHistoryData
             return disbursementDatas;
         }
     
    +    public List<Rate> fetchRateData(final JsonArray element) {
    +        List<Rate> rates = new ArrayList<>();
    +
    +        if (element != null) {
    +            for (JsonElement jsonElement : element) {
    +                final JsonObject rateElement = jsonElement.getAsJsonObject();
    +                if (rateElement.has("id")) {
    +                    final Long rateId = this.fromApiJsonHelper.extractLongNamed("id", rateElement);
    +                    Rate rate = findRateByIdIfProvided(rateId);
    +                    rates.add(rate);
    +                }
    +            }
    +        }else{
    +            System.out.println("");
    +        }
    +
    +        return rates;
    +    }
    +
    +    public Rate findRateByIdIfProvided(final Long rateId) {
    --- End diff --
    
    kindly provide appropriate name


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r209071792
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/rate/serialization/RateDefinitionCommandFromApiJsonDeserializer.java ---
    @@ -0,0 +1,128 @@
    +/**
    + * 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.rate.serialization;
    +
    +import com.google.gson.JsonElement;
    +import com.google.gson.reflect.TypeToken;
    +import java.lang.reflect.Type;
    +import java.math.BigDecimal;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import org.apache.commons.lang.StringUtils;
    +import org.apache.fineract.infrastructure.core.data.ApiParameterError;
    +import org.apache.fineract.infrastructure.core.data.DataValidatorBuilder;
    +import org.apache.fineract.infrastructure.core.exception.InvalidJsonException;
    +import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
    +import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper;
    +import org.springframework.beans.factory.annotation.Autowired;
    +import org.springframework.stereotype.Component;
    +
    +@Component
    +public class RateDefinitionCommandFromApiJsonDeserializer {
    +
    +  /**
    +   * The parameters supported for this command.
    +   */
    +  private final Set<String> supportedParameters = new HashSet<>(
    +      Arrays.asList("id", "name", "percentage", "productApply", "active", "approveUser", "locale"));
    +
    +  private final FromJsonHelper fromApiJsonHelper;
    +
    +  @Autowired
    +  public RateDefinitionCommandFromApiJsonDeserializer(final FromJsonHelper fromApiJsonHelper) {
    +    this.fromApiJsonHelper = fromApiJsonHelper;
    +  }
    +
    +  public void validateForCreate(final String json) {
    +    if (StringUtils.isBlank(json)) {
    +      throw new InvalidJsonException();
    +    }
    +
    +    final Type typeOfMap = new TypeToken<Map<String, Object>>() {
    +    }.getType();
    +
    +    this.fromApiJsonHelper.checkForUnsupportedParameters(typeOfMap, json, this.supportedParameters);
    +
    +    final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
    +
    +    final DataValidatorBuilder baseDataValidator = new DataValidatorBuilder(dataValidationErrors)
    +        .resource("rate");
    +
    +    final JsonElement element = this.fromApiJsonHelper.parse(json);
    +
    +    final String name = this.fromApiJsonHelper.extractStringNamed("name", element);
    --- End diff --
    
    Ok


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206853561
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanUtilService.java ---
    @@ -325,4 +332,35 @@ private LocalDate generateCalculatedRepaymentStartDate(final CalendarHistoryData
             return disbursementDatas;
         }
     
    +    public List<Rate> fetchRateData(final JsonArray element) {
    +        List<Rate> rates = new ArrayList<>();
    +
    +        if (element != null) {
    +            for (JsonElement jsonElement : element) {
    +                final JsonObject rateElement = jsonElement.getAsJsonObject();
    +                if (rateElement.has("id")) {
    +                    final Long rateId = this.fromApiJsonHelper.extractLongNamed("id", rateElement);
    +                    Rate rate = findRateByIdIfProvided(rateId);
    +                    rates.add(rate);
    +                }
    +            }
    +        }else{
    --- End diff --
    
    remove else block


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by ShruthiRajaram <gi...@git.apache.org>.
Github user ShruthiRajaram commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r206853497
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java ---
    @@ -392,20 +395,24 @@
         @OneToOne(cascade = CascadeType.ALL, mappedBy = "loan", optional = true, orphanRemoval = true, fetch=FetchType.EAGER)
         private LoanTopupDetails loanTopupDetails;
     
    +    @ManyToMany(fetch = FetchType.EAGER)
    --- End diff --
    
    I think it is OneToMany relationship !!
    it possible to change fetchtype to lazy and achieve the expected results?


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r207369345
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/rate/handler/DeleteRateCommandHandler.java ---
    @@ -0,0 +1,26 @@
    +/**
    --- End diff --
    
    It is not, consider it as gone.


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r209071842
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/service/LoanProductWritePlatformServiceJpaRepositoryImpl.java ---
    @@ -304,6 +318,29 @@ private boolean anyChangeInCriticalFloatingRateLinkedParams(JsonCommand command,
             return charges;
         }
     
    +    private List<Rate> assembleListOfProductRates(final JsonCommand command) {
    +
    +        final List<Rate> rates = new ArrayList<>();
    +
    +        if (command.parameterExists("rates")) {
    +            final JsonArray ratesArray = command.arrayOfParameterNamed("rates");
    +            if (ratesArray != null) {
    +                List<Long> idList = new ArrayList<>();
    +                for (int i = 0; i < ratesArray.size(); i++) {
    +                    final JsonObject jsonObject = ratesArray.get(i).getAsJsonObject();
    +                    if (jsonObject.has("id")) {
    +                        idList.add(jsonObject.get("id").getAsLong());
    +                    }
    +                }
    +                System.out.println(idList.toString());
    +                rates.addAll(this.rateRepository.findMultipleWithNotFoundDetection(idList));
    +                System.out.println(rates.toString());
    --- End diff --
    
    Ok


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r207364751
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProduct.java ---
    @@ -102,6 +103,10 @@
         @JoinTable(name = "m_product_loan_charge", joinColumns = @JoinColumn(name = "product_loan_id"), inverseJoinColumns = @JoinColumn(name = "charge_id"))
         private List<Charge> charges;
     
    +    @ManyToMany(fetch = FetchType.EAGER)
    +    @JoinTable(name = "m_product_loan_rate", joinColumns = @JoinColumn(name = "product_loan_id"), inverseJoinColumns = @JoinColumn(name = "rate_id"))
    --- End diff --
    
    It should be ManyToMany as we use a third table to join the rate and loan_product tables in the same way that the charges and loan_products are joined.  About the fetch type Lazy I guess it should work the same but I will do some tests to be sure.


---

[GitHub] fineract pull request #465: Fineract 614: Rates module

Posted by angelboxes <gi...@git.apache.org>.
Github user angelboxes commented on a diff in the pull request:

    https://github.com/apache/fineract/pull/465#discussion_r208664400
  
    --- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanUtilService.java ---
    @@ -325,4 +332,35 @@ private LocalDate generateCalculatedRepaymentStartDate(final CalendarHistoryData
             return disbursementDatas;
         }
     
    +    public List<Rate> fetchRateData(final JsonArray element) {
    +        List<Rate> rates = new ArrayList<>();
    +
    +        if (element != null) {
    +            for (JsonElement jsonElement : element) {
    +                final JsonObject rateElement = jsonElement.getAsJsonObject();
    +                if (rateElement.has("id")) {
    +                    final Long rateId = this.fromApiJsonHelper.extractLongNamed("id", rateElement);
    +                    Rate rate = findRateByIdIfProvided(rateId);
    +                    rates.add(rate);
    +                }
    +            }
    +        }else{
    --- End diff --
    
    removed


---