[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073679864
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -6301,26 +6307,27 @@ public Map
undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
-existingTransactionIds.addAll(findExistingTransactionIds());
-
existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
-final Map actualChanges = new LinkedHashMap<>();
validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST,
getDisbursementDate());
-LocalDate actualDisbursementDate = null;
-LocalDate lastTransactionDate = getDisbursementDate();
-List loanTransactions =
retrieveListOfTransactionsExcludeAccruals();
+
+final Map actualChanges = new LinkedHashMap<>();
+List loanTransactions =
retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
+loanTransactions.sort(Comparator.comparing(LoanTransaction::getId));
+final LoanTransaction lastDisbursalTransaction =
loanTransactions.get(loanTransactions.size() - 1);
Review Comment:
This might not be true. There could be Accrual transaction or any other one
as well.
Please make it sure you understand what the legacy logic was doing and not
do any regression!
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1082702226
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -6301,26 +6307,27 @@ public Map
undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
-existingTransactionIds.addAll(findExistingTransactionIds());
-
existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
-final Map actualChanges = new LinkedHashMap<>();
validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST,
getDisbursementDate());
-LocalDate actualDisbursementDate = null;
-LocalDate lastTransactionDate = getDisbursementDate();
-List loanTransactions =
retrieveListOfTransactionsExcludeAccruals();
+
+final Map actualChanges = new LinkedHashMap<>();
+List loanTransactions =
retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
Review Comment:
This is wrong, we filter the transactions to have only the transactions
which are disbursement, but at line 6323, we are iterating through on the same
list and try to decide whether there was any transaction after the last
disbursement which prevent to undo the disbursement... (any repayment happened,
we cannot undo them)
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2904: FINERACT-1724: Added JMS connection pooling to the external event producer
galovics merged PR #2904: URL: https://github.com/apache/fineract/pull/2904 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2903: [FINERACT-1678] Inline Loan COB for Batch API
galovics merged PR #2903: URL: https://github.com/apache/fineract/pull/2903 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
galovics merged PR #2897: URL: https://github.com/apache/fineract/pull/2897 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics opened a new pull request, #2904: FINERACT-1724: Added JMS connection pooling to the external event producer
galovics opened a new pull request, #2904:
URL: https://github.com/apache/fineract/pull/2904
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] taskain7 opened a new pull request, #2903: [FINERACT-1678] Inline Loan COB for Batch API
taskain7 opened a new pull request, #2903: URL: https://github.com/apache/fineract/pull/2903 ## Description Loan COB API filter executes the Inline COB for soft locked loans for Batch API. -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx opened a new pull request, #2902: FINERACT-1840: Fee income is not realised if the loan got fully repaid
josehernandezfintecheandomx opened a new pull request, #2902:
URL: https://github.com/apache/fineract/pull/2902
## Description
A Loan with a NSF fee is added (specific due date fee) and the loan got
fully repaid on the same day the Fee income is not realised, the Accrual
transaction is not added
[FINERACT-1840](https://issues.apache.org/jira/browse/FINERACT-1840)
https://user-images.githubusercontent.com/44206706/213580591-99a78a6b-a7a3-4c1d-8348-b11c225d0398.png;>
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2901: FINERACT-1707 - Export refactor
galovics commented on code in PR #2901:
URL: https://github.com/apache/fineract/pull/2901#discussion_r1081362685
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableExportTargetParameter.java:
##
@@ -0,0 +1,48 @@
+/**
+ * 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.infrastructure.dataqueries.service;
+
+import javax.ws.rs.core.MultivaluedMap;
+
+public enum DatatableExportTargetParameter {
+
+CSV("exportCSV"), PDF("exportPDF"), S3("exportS3"), JSON("exportJSON"),
PRETTY_JSON("pretty");
+
+private final String value;
+
+DatatableExportTargetParameter(final String value) {
+this.value = value;
+}
+
+public String getValue() {
+return this.value;
+}
+
+public static DatatableExportTargetParameter checkTarget(final
MultivaluedMap queryParams) {
Review Comment:
Would rather call this method "resolve" instead of "checkTarget" since it's
not checking/verifying anything.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2901: FINERACT-1707 - Export refactor
galovics merged PR #2901: URL: https://github.com/apache/fineract/pull/2901 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] taskain7 commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
taskain7 commented on code in PR #2897: URL: https://github.com/apache/fineract/pull/2897#discussion_r1081328719 ## fineract-provider/src/main/resources/db/changelog/tenant/parts/0086_add_cob_business_date_to_loan_account_locks.xml: ## @@ -0,0 +1,32 @@ + + +http://www.liquibase.org/xml/ns/dbchangelog; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance; + xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.3.xsd;> + + + + Review Comment: removed the constraint, we are more backward compatible with the nullable column -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
adamsaghy commented on code in PR #2897:
URL: https://github.com/apache/fineract/pull/2897#discussion_r1081304157
##
fineract-provider/src/main/java/org/apache/fineract/cob/domain/LoanAccountLock.java:
##
@@ -57,10 +60,14 @@ public class LoanAccountLock {
@Column(name = "stacktrace")
private String stacktrace;
+@Column(name = "lock_placed_on_cob_business_date", nullable = false)
+private LocalDate lockPlacedOnCobBusinessDate;
Review Comment:
i see. thanks.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] taskain7 commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
taskain7 commented on code in PR #2897:
URL: https://github.com/apache/fineract/pull/2897#discussion_r1081300032
##
fineract-provider/src/main/java/org/apache/fineract/cob/domain/LoanAccountLock.java:
##
@@ -57,10 +60,14 @@ public class LoanAccountLock {
@Column(name = "stacktrace")
private String stacktrace;
+@Column(name = "lock_placed_on_cob_business_date", nullable = false)
+private LocalDate lockPlacedOnCobBusinessDate;
Review Comment:
we already have a "lock_placed_on", but that's the actual datetime, and we
need the business date as well to be able to connect the lock and the COB
execution
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
adamsaghy commented on code in PR #2897:
URL: https://github.com/apache/fineract/pull/2897#discussion_r1081295733
##
fineract-provider/src/main/java/org/apache/fineract/cob/domain/LoanAccountLock.java:
##
@@ -57,10 +60,14 @@ public class LoanAccountLock {
@Column(name = "stacktrace")
private String stacktrace;
+@Column(name = "lock_placed_on_cob_business_date", nullable = false)
+private LocalDate lockPlacedOnCobBusinessDate;
Review Comment:
Just to give a 3rd opinion: "lock_placed_on" makes more sense to me. We
should not put business logic into db column naming (i know, i know...many
cases its already there but...)
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] taskain7 commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
taskain7 commented on code in PR #2897:
URL: https://github.com/apache/fineract/pull/2897#discussion_r1081292221
##
fineract-provider/src/main/java/org/apache/fineract/cob/loan/StayedLockedLoansTasklet.java:
##
@@ -33,16 +39,25 @@
@RequiredArgsConstructor
public class StayedLockedLoansTasklet implements Tasklet {
-private final LoanAccountLockRepository loanAccountLockRepository;
private final BusinessEventNotifierService businessEventNotifierService;
+private final LoanRepository loanRepository;
@Override
public RepeatStatus execute(StepContribution contribution, ChunkContext
chunkContext) throws Exception {
-List loanAccountLocks =
loanAccountLockRepository.findAll();
-if (!loanAccountLocks.isEmpty()) {
-List loanIds =
loanAccountLocks.stream().map(LoanAccountLock::getLoanId).toList();
-businessEventNotifierService.notifyPostBusinessEvent(new
LoanAccountsStayedLockedBusinessEvent(loanIds));
+LoanAccountsStayedLockedBusinessEvent.Data lockedLoanAccounts =
buildLoanAccountData();
+if (!lockedLoanAccounts.getLoanAccounts().getLoanAccounts().isEmpty())
{
Review Comment:
used unneccessary wrapper class, removed it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] taskain7 commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
taskain7 commented on code in PR #2897:
URL: https://github.com/apache/fineract/pull/2897#discussion_r1081287370
##
fineract-provider/src/main/java/org/apache/fineract/cob/domain/LoanAccountLock.java:
##
@@ -57,10 +60,14 @@ public class LoanAccountLock {
@Column(name = "stacktrace")
private String stacktrace;
+@Column(name = "lock_placed_on_cob_business_date", nullable = false)
+private LocalDate lockPlacedOnCobBusinessDate;
Review Comment:
we could, however, since COB places the lock on the account, the COB busines
date made more sense to me.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
galovics commented on code in PR #2897:
URL: https://github.com/apache/fineract/pull/2897#discussion_r1081272455
##
fineract-provider/src/main/java/org/apache/fineract/cob/domain/LoanAccountLock.java:
##
@@ -57,10 +60,14 @@ public class LoanAccountLock {
@Column(name = "stacktrace")
private String stacktrace;
+@Column(name = "lock_placed_on_cob_business_date", nullable = false)
+private LocalDate lockPlacedOnCobBusinessDate;
Review Comment:
Can't we just call it lockPlacedOnBusinessDate and use it as
DateUtils.getBusinessDate()?
##
fineract-provider/src/main/java/org/apache/fineract/cob/loan/StayedLockedLoansTasklet.java:
##
@@ -33,16 +39,25 @@
@RequiredArgsConstructor
public class StayedLockedLoansTasklet implements Tasklet {
-private final LoanAccountLockRepository loanAccountLockRepository;
private final BusinessEventNotifierService businessEventNotifierService;
+private final LoanRepository loanRepository;
@Override
public RepeatStatus execute(StepContribution contribution, ChunkContext
chunkContext) throws Exception {
-List loanAccountLocks =
loanAccountLockRepository.findAll();
-if (!loanAccountLocks.isEmpty()) {
-List loanIds =
loanAccountLocks.stream().map(LoanAccountLock::getLoanId).toList();
-businessEventNotifierService.notifyPostBusinessEvent(new
LoanAccountsStayedLockedBusinessEvent(loanIds));
+LoanAccountsStayedLockedBusinessEvent.Data lockedLoanAccounts =
buildLoanAccountData();
+if (!lockedLoanAccounts.getLoanAccounts().getLoanAccounts().isEmpty())
{
Review Comment:
why the double getLoanAccounts()?
##
fineract-provider/src/main/resources/db/changelog/tenant/parts/0086_add_cob_business_date_to_loan_account_locks.xml:
##
@@ -0,0 +1,32 @@
+
+
+http://www.liquibase.org/xml/ns/dbchangelog;
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.3.xsd;>
+
+
+
+
Review Comment:
Let's give it a default value in this case.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2886: FINERACT-1724 - Replayed bulk transaction fix
galovics merged PR #2886: URL: https://github.com/apache/fineract/pull/2886 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy merged pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
adamsaghy merged PR #2852: URL: https://github.com/apache/fineract/pull/2852 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2900: FINERACT-1724: Removed accidental closing of JMS connection upon startup
galovics merged PR #2900: URL: https://github.com/apache/fineract/pull/2900 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] b0c1 opened a new pull request, #2901: FINERACT-1707 - Export refactor
b0c1 opened a new pull request, #2901:
URL: https://github.com/apache/fineract/pull/2901
FINERACT-1707 - Export refactor
[x] Refactor Datatable export
[x] Fix CSV exceptions
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] b0c1 closed pull request #2899: FINERACT-1707 - Export refactor
b0c1 closed pull request #2899: FINERACT-1707 - Export refactor URL: https://github.com/apache/fineract/pull/2899 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics opened a new pull request, #2900: FINERACT-1724: Removed accidental closing of JMS connection upon startup
galovics opened a new pull request, #2900:
URL: https://github.com/apache/fineract/pull/2900
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] b0c1 opened a new pull request, #2899: FINERACT-1707 - Export refactor
b0c1 opened a new pull request, #2899:
URL: https://github.com/apache/fineract/pull/2899
FINERACT-1707 - Export refactor
[x] Refactor Datatable export
[x] Fix CSV generation exception
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
galovics merged PR #2891: URL: https://github.com/apache/fineract/pull/2891 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
galovics commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1080999034
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/config/ExternalEventJMSConfiguration.java:
##
@@ -60,4 +61,13 @@ public ActiveMQTopic activeMqTopic() {
public ActiveMQQueue activeMqQueue() {
return new
ActiveMQQueue(fineractProperties.getEvents().getExternal().getProducer().getJms().getEventQueueName());
}
+
+@Bean("externalEventJmsProducerExecutor")
+public ThreadPoolTaskExecutor externalEventJmsProducerExecutor() {
+ThreadPoolTaskExecutor threadPoolTaskExecutor = new
ThreadPoolTaskExecutor();
+threadPoolTaskExecutor.setCorePoolSize(10);
Review Comment:
It's just the core pool size, the threadpool can grow until the max pool
size is reached (100 threads) but that's already a value nobody will utilize in
my opinion. Essentially it'd mean 100 parallel producers.
Based on my initial measurement the sweet spot is around 30-40 ActiveMQ
producers.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
adamsaghy commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1080989821
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/config/ExternalEventJMSConfiguration.java:
##
@@ -60,4 +61,13 @@ public ActiveMQTopic activeMqTopic() {
public ActiveMQQueue activeMqQueue() {
return new
ActiveMQQueue(fineractProperties.getEvents().getExternal().getProducer().getJms().getEventQueueName());
}
+
+@Bean("externalEventJmsProducerExecutor")
+public ThreadPoolTaskExecutor externalEventJmsProducerExecutor() {
+ThreadPoolTaskExecutor threadPoolTaskExecutor = new
ThreadPoolTaskExecutor();
+threadPoolTaskExecutor.setCorePoolSize(10);
Review Comment:
Would it make sense to move these into application.properties?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2898: FINERACT-1844-Loan-Chargeback-Transaction-event-data-fix
galovics merged PR #2898: URL: https://github.com/apache/fineract/pull/2898 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073680744
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -6341,41 +6349,19 @@ public Map
undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
return actualChanges;
}
-/**
- * Reverse only disbursement, accruals, and repayments at disbursal
transactions
- *
- * @param actualDisbursementDate
- * @return
- */
-public List
reverseExistingTransactionsTillLastDisbursal(LocalDate actualDisbursementDate) {
Review Comment:
Accrual and repayment at disbursal can be reversed even if it was after the
last disbursement.
Please be more careful to not do any regression!
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073679864
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -6301,26 +6307,27 @@ public Map
undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
-existingTransactionIds.addAll(findExistingTransactionIds());
-
existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
-final Map actualChanges = new LinkedHashMap<>();
validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST,
getDisbursementDate());
-LocalDate actualDisbursementDate = null;
-LocalDate lastTransactionDate = getDisbursementDate();
-List loanTransactions =
retrieveListOfTransactionsExcludeAccruals();
+
+final Map actualChanges = new LinkedHashMap<>();
+List loanTransactions =
retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
+loanTransactions.sort(Comparator.comparing(LoanTransaction::getId));
+final LoanTransaction lastDisbursalTransaction =
loanTransactions.get(loanTransactions.size() - 1);
Review Comment:
This might not be true. There could be Accrual transaction or any other one
as well.
Please make it sure you understand what the legacy logic was doing and not
do any regression!
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] taskain7 commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
taskain7 commented on code in PR #2897:
URL: https://github.com/apache/fineract/pull/2897#discussion_r1073662589
##
fineract-provider/src/main/java/org/apache/fineract/cob/loan/StayedLockedLoansTasklet.java:
##
@@ -35,14 +40,24 @@ public class StayedLockedLoansTasklet implements Tasklet {
private final LoanAccountLockRepository loanAccountLockRepository;
private final BusinessEventNotifierService businessEventNotifierService;
+private final LoanRepository loanRepository;
@Override
public RepeatStatus execute(StepContribution contribution, ChunkContext
chunkContext) throws Exception {
List loanAccountLocks =
loanAccountLockRepository.findAll();
if (!loanAccountLocks.isEmpty()) {
List loanIds =
loanAccountLocks.stream().map(LoanAccountLock::getLoanId).toList();
-businessEventNotifierService.notifyPostBusinessEvent(new
LoanAccountsStayedLockedBusinessEvent(loanIds));
+businessEventNotifierService.notifyPostBusinessEvent(new
LoanAccountsStayedLockedBusinessEvent(buildLoanAccountData(loanIds)));
}
return RepeatStatus.FINISHED;
}
+
+private LoanAccountsStayedLockedBusinessEvent.Data
buildLoanAccountData(List loanIds) {
+List stayedLockedLoanAccounts =
loanRepository.findAllStayedLockedByLoanIds(loanIds);
Review Comment:
I changed the query, we don't have to deal with the limitation anymore.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2777: [FINERACT-1678] Loan COB Catch Up
galovics merged PR #2777: URL: https://github.com/apache/fineract/pull/2777 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2878: FINERACT-1853: General accounting summary table reports
galovics merged PR #2878: URL: https://github.com/apache/fineract/pull/2878 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
josehernandezfintecheandomx commented on code in PR #2852:
URL: https://github.com/apache/fineract/pull/2852#discussion_r1073648909
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientWritePlatformServiceJpaRepositoryImpl.java:
##
@@ -430,7 +430,8 @@ public CommandProcessingResult updateClient(final Long
clientId, final JsonComma
}
final ExternalId externalId =
externalIdFactory.createFromCommand(command,
ClientApiConstants.externalIdParamName);
-if
(command.isChangeInStringParameterNamed(ClientApiConstants.externalIdParamName,
externalId.getValue())) {
+if
(command.isChangeInStringParameterNamed(ClientApiConstants.externalIdParamName,
Review Comment:
Done
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2895: FINERACT-1707 - S3 upgrade
galovics merged PR #2895: URL: https://github.com/apache/fineract/pull/2895 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] ruchiD opened a new pull request, #2898: FINERACT-1844-Loan-Chargeback-Transaction-event-data-fix
ruchiD opened a new pull request, #2898:
URL: https://github.com/apache/fineract/pull/2898
## Description
Fix for LoanChargebackTransactionBusinessEvent to have correct transaction
payload data.
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
josehernandezfintecheandomx commented on PR #2882: URL: https://github.com/apache/fineract/pull/2882#issuecomment-1387202314 > Kindly review my questions! Comments attended -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
josehernandezfintecheandomx commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073641960
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -4158,20 +4157,12 @@ public LocalDate
getExpectedDisbursedOnLocalDateForTemplate() {
return expectedDisbursementDate;
}
-/*
- * Reason for derving
- */
-
public BigDecimal getDisburseAmountForTemplate() {
-BigDecimal principal =
this.loanRepaymentScheduleDetail.getPrincipal().getAmount();
Review Comment:
Done
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
josehernandezfintecheandomx commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073641584
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -6301,26 +6302,18 @@ public Map
undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
-existingTransactionIds.addAll(findExistingTransactionIds());
-
existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
-final Map actualChanges = new LinkedHashMap<>();
validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST,
getDisbursementDate());
-LocalDate actualDisbursementDate = null;
-LocalDate lastTransactionDate = getDisbursementDate();
-List loanTransactions =
retrieveListOfTransactionsExcludeAccruals();
-Collections.reverse(loanTransactions);
-for (final LoanTransaction previousTransaction : loanTransactions) {
-if
(lastTransactionDate.isBefore(previousTransaction.getTransactionDate())
-&& (previousTransaction.isRepaymentType() ||
previousTransaction.isWaiver() || previousTransaction.isChargePayment())) {
-throw new
UndoLastTrancheDisbursementException(previousTransaction.getId());
-}
-if (previousTransaction.isDisbursement()) {
-lastTransactionDate = previousTransaction.getTransactionDate();
-break;
-}
-}
-actualDisbursementDate = lastTransactionDate;
-updateLoanToLastDisbursalState(actualDisbursementDate);
+
+final Map actualChanges = new LinkedHashMap<>();
Review Comment:
Done
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
galovics commented on code in PR #2897:
URL: https://github.com/apache/fineract/pull/2897#discussion_r1073566287
##
fineract-provider/src/main/java/org/apache/fineract/cob/loan/StayedLockedLoansTasklet.java:
##
@@ -35,14 +40,24 @@ public class StayedLockedLoansTasklet implements Tasklet {
private final LoanAccountLockRepository loanAccountLockRepository;
private final BusinessEventNotifierService businessEventNotifierService;
+private final LoanRepository loanRepository;
@Override
public RepeatStatus execute(StepContribution contribution, ChunkContext
chunkContext) throws Exception {
List loanAccountLocks =
loanAccountLockRepository.findAll();
if (!loanAccountLocks.isEmpty()) {
List loanIds =
loanAccountLocks.stream().map(LoanAccountLock::getLoanId).toList();
-businessEventNotifierService.notifyPostBusinessEvent(new
LoanAccountsStayedLockedBusinessEvent(loanIds));
+businessEventNotifierService.notifyPostBusinessEvent(new
LoanAccountsStayedLockedBusinessEvent(buildLoanAccountData(loanIds)));
}
return RepeatStatus.FINISHED;
}
+
+private LoanAccountsStayedLockedBusinessEvent.Data
buildLoanAccountData(List loanIds) {
+List stayedLockedLoanAccounts =
loanRepository.findAllStayedLockedByLoanIds(loanIds);
Review Comment:
Do we need to at all go to the database to grab this data? Can't we pass
this through some context?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
adamsaghy commented on code in PR #2897:
URL: https://github.com/apache/fineract/pull/2897#discussion_r1073541622
##
fineract-provider/src/main/java/org/apache/fineract/cob/loan/StayedLockedLoansTasklet.java:
##
@@ -35,14 +40,24 @@ public class StayedLockedLoansTasklet implements Tasklet {
private final LoanAccountLockRepository loanAccountLockRepository;
private final BusinessEventNotifierService businessEventNotifierService;
+private final LoanRepository loanRepository;
@Override
public RepeatStatus execute(StepContribution contribution, ChunkContext
chunkContext) throws Exception {
List loanAccountLocks =
loanAccountLockRepository.findAll();
if (!loanAccountLocks.isEmpty()) {
List loanIds =
loanAccountLocks.stream().map(LoanAccountLock::getLoanId).toList();
-businessEventNotifierService.notifyPostBusinessEvent(new
LoanAccountsStayedLockedBusinessEvent(loanIds));
+businessEventNotifierService.notifyPostBusinessEvent(new
LoanAccountsStayedLockedBusinessEvent(buildLoanAccountData(loanIds)));
}
return RepeatStatus.FINISHED;
}
+
+private LoanAccountsStayedLockedBusinessEvent.Data
buildLoanAccountData(List loanIds) {
+List stayedLockedLoanAccounts =
loanRepository.findAllStayedLockedByLoanIds(loanIds);
Review Comment:
Please make sure the loanIds array is not too big / request. Postgres has
some restrictions on the parameter size.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
adamsaghy commented on code in PR #2897:
URL: https://github.com/apache/fineract/pull/2897#discussion_r1073538291
##
fineract-avro-schemas/src/main/avro/loan/v1/LoanAccountStayedLockedDataV1.avsc:
##
@@ -0,0 +1,31 @@
+{
+"name": "LoanAccountStayedLockedDataV1",
+"namespace": "org.apache.fineract.avro.loan.v1",
+"type": "record",
+"fields": [
+{
+"default": null,
Review Comment:
if the loan id cannot be null, please reflect it in the avro schema as well!
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073533927
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -4158,20 +4157,12 @@ public LocalDate
getExpectedDisbursedOnLocalDateForTemplate() {
return expectedDisbursementDate;
}
-/*
- * Reason for derving
- */
-
public BigDecimal getDisburseAmountForTemplate() {
-BigDecimal principal =
this.loanRepaymentScheduleDetail.getPrincipal().getAmount();
Review Comment:
I see. Thanks for letting me know.
Would you please raise a new story and PR with this change?
We should focus to fix one issue / PR, it makes confusing and complex if
more than one issue is addressed in one 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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073533927
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -4158,20 +4157,12 @@ public LocalDate
getExpectedDisbursedOnLocalDateForTemplate() {
return expectedDisbursementDate;
}
-/*
- * Reason for derving
- */
-
public BigDecimal getDisburseAmountForTemplate() {
-BigDecimal principal =
this.loanRepaymentScheduleDetail.getPrincipal().getAmount();
Review Comment:
I see. Thanks for letting me know.
Would you please raise a new story and PR with this change?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2852: FINERACT-1847: Removing Edit function from Goodwill Credit or Payout Refund or Merch…
adamsaghy commented on code in PR #2852:
URL: https://github.com/apache/fineract/pull/2852#discussion_r1073524648
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientWritePlatformServiceJpaRepositoryImpl.java:
##
@@ -430,7 +430,8 @@ public CommandProcessingResult updateClient(final Long
clientId, final JsonComma
}
final ExternalId externalId =
externalIdFactory.createFromCommand(command,
ClientApiConstants.externalIdParamName);
-if
(command.isChangeInStringParameterNamed(ClientApiConstants.externalIdParamName,
externalId.getValue())) {
+if
(command.isChangeInStringParameterNamed(ClientApiConstants.externalIdParamName,
Review Comment:
Please raise a separate PR for fixing the client external id updating bug!
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
josehernandezfintecheandomx commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073519202
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -4158,20 +4157,12 @@ public LocalDate
getExpectedDisbursedOnLocalDateForTemplate() {
return expectedDisbursementDate;
}
-/*
- * Reason for derving
- */
-
public BigDecimal getDisburseAmountForTemplate() {
-BigDecimal principal =
this.loanRepaymentScheduleDetail.getPrincipal().getAmount();
Review Comment:
When you get the disburse template you get a wrong amount after the first
disbursement
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073518496
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -6301,26 +6302,18 @@ public Map
undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
-existingTransactionIds.addAll(findExistingTransactionIds());
-
existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
-final Map actualChanges = new LinkedHashMap<>();
validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST,
getDisbursementDate());
-LocalDate actualDisbursementDate = null;
-LocalDate lastTransactionDate = getDisbursementDate();
-List loanTransactions =
retrieveListOfTransactionsExcludeAccruals();
-Collections.reverse(loanTransactions);
-for (final LoanTransaction previousTransaction : loanTransactions) {
-if
(lastTransactionDate.isBefore(previousTransaction.getTransactionDate())
-&& (previousTransaction.isRepaymentType() ||
previousTransaction.isWaiver() || previousTransaction.isChargePayment())) {
-throw new
UndoLastTrancheDisbursementException(previousTransaction.getId());
-}
-if (previousTransaction.isDisbursement()) {
-lastTransactionDate = previousTransaction.getTransactionDate();
-break;
-}
-}
-actualDisbursementDate = lastTransactionDate;
-updateLoanToLastDisbursalState(actualDisbursementDate);
+
+final Map actualChanges = new LinkedHashMap<>();
Review Comment:
This behaviour looks different to me like it was.
Please correct me if i wrong, but priorly we were not let the customer undo
the last disbursement if there was any repayment, waiver or charge transaction
after it.
I dont see the same restriction with the new logic. Was it purposely?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073511151
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##
@@ -4158,20 +4157,12 @@ public LocalDate
getExpectedDisbursedOnLocalDateForTemplate() {
return expectedDisbursementDate;
}
-/*
- * Reason for derving
- */
-
public BigDecimal getDisburseAmountForTemplate() {
-BigDecimal principal =
this.loanRepaymentScheduleDetail.getPrincipal().getAmount();
Review Comment:
How does this change relates to the "undo last disbursal function" fix?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] taskain7 opened a new pull request, #2897: [FINERACT-1859] Avro schema for LoanAccountsStayedLockedBusinessEvent
taskain7 opened a new pull request, #2897: URL: https://github.com/apache/fineract/pull/2897 ## Description Avro schema for `LoanAccountsStayedLockedBusinessEvent` -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] b0c1 commented on a diff in pull request #2886: FINERACT-1724 - Replayed bulk transaction fix
b0c1 commented on code in PR #2886: URL: https://github.com/apache/fineract/pull/2886#discussion_r1073450401 ## fineract-provider/src/main/java/org/apache/fineract/cob/loan/ApplyChargeToOverdueLoansBusinessStep.java: ## @@ -28,6 +29,7 @@ @Component @RequiredArgsConstructor +@BulkEventSupport Review Comment: The aspect calls the already existing `BusinessEventNotifierService` methods that enable or disable the bulk logging flag. The flag is still checked in the `notifyPostBusinessEvent` method after the global flag check. -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] b0c1 commented on a diff in pull request #2886: FINERACT-1724 - Replayed bulk transaction fix
b0c1 commented on code in PR #2886:
URL: https://github.com/apache/fineract/pull/2886#discussion_r1073447319
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/business/aspect/BulkEventAspect.java:
##
@@ -0,0 +1,61 @@
+/**
+ * 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.infrastructure.event.business.aspect;
+
+import lombok.RequiredArgsConstructor;
+import lombok.SneakyThrows;
+import
org.apache.fineract.infrastructure.event.business.annotation.BulkEventSupport;
+import
org.apache.fineract.infrastructure.event.business.service.BusinessEventNotifierService;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.springframework.stereotype.Component;
+
+@Component
+@RequiredArgsConstructor
+@Aspect
+public class BulkEventAspect {
+
+private final BusinessEventNotifierService businessEventNotifierService;
+
+@Around("execution(*
org.apache.fineract.commands.handler.NewCommandSourceHandler.processCommand(..))
&& @within(bulkEventSupport)")
Review Comment:
Modified using any class (with all methods) or only specific methods
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2896: [FINERACT-1863] Swagger schema fix for configuration APIs
galovics merged PR #2896: URL: https://github.com/apache/fineract/pull/2896 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
galovics commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1073338862
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/producer/jms/JMSMultiExternalEventProducer.java:
##
@@ -0,0 +1,151 @@
+/**
+ * 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.infrastructure.event.external.producer.jms;
+
+import static
org.apache.fineract.infrastructure.core.service.MeasuringUtil.measure;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Future;
+import javax.jms.Connection;
+import javax.jms.Destination;
+import javax.jms.JMSException;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.activemq.ActiveMQConnectionFactory;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.messaging.jms.MessageFactory;
+import org.apache.fineract.infrastructure.core.service.HashingService;
+import
org.apache.fineract.infrastructure.event.external.exception.AcknowledgementTimeoutException;
+import
org.apache.fineract.infrastructure.event.external.producer.ExternalEventProducer;
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.core.task.AsyncTaskExecutor;
+import org.springframework.stereotype.Component;
+
+@Component
+@Slf4j
+@RequiredArgsConstructor
+@ConditionalOnProperty(value =
"fineract.events.external.producer.jms.enabled", havingValue = "true")
+public class JMSMultiExternalEventProducer implements ExternalEventProducer,
InitializingBean {
+
+@Qualifier("eventDestination")
+private final Destination destination;
+
+private final ActiveMQConnectionFactory connectionFactory;
Review Comment:
That's a good point @ruchiD , I'll change it for sure
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] taskain7 commented on a diff in pull request #2777: [FINERACT-1678] Loan COB Catch Up
taskain7 commented on code in PR #2777:
URL: https://github.com/apache/fineract/pull/2777#discussion_r1073290311
##
fineract-provider/src/main/java/org/apache/fineract/cob/service/AsyncLoanCOBExecutorServiceImpl.java:
##
@@ -0,0 +1,107 @@
+/**
+ * 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.cob.service;
+
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.cob.data.ProjectIdAndLastClosedBusinessDate;
+import org.apache.fineract.cob.loan.LoanCOBConstant;
+import org.apache.fineract.infrastructure.businessdate.domain.BusinessDateType;
+import org.apache.fineract.infrastructure.core.domain.FineractContext;
+import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil;
+import org.apache.fineract.infrastructure.jobs.data.JobParameterDTO;
+import org.apache.fineract.infrastructure.jobs.domain.JobParameter;
+import org.apache.fineract.infrastructure.jobs.domain.JobParameterRepository;
+import org.apache.fineract.infrastructure.jobs.domain.ScheduledJobDetail;
+import
org.apache.fineract.infrastructure.jobs.domain.ScheduledJobDetailRepository;
+import org.apache.fineract.infrastructure.jobs.exception.JobNotFoundException;
+import org.apache.fineract.infrastructure.jobs.service.JobStarter;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepository;
+import org.springframework.batch.core.Job;
+import org.springframework.batch.core.JobParametersInvalidException;
+import org.springframework.batch.core.configuration.JobLocator;
+import org.springframework.batch.core.launch.NoSuchJobException;
+import
org.springframework.batch.core.repository.JobExecutionAlreadyRunningException;
+import
org.springframework.batch.core.repository.JobInstanceAlreadyCompleteException;
+import org.springframework.batch.core.repository.JobRestartException;
+import org.springframework.scheduling.annotation.Async;
+import org.springframework.stereotype.Service;
+
+@Service
+@RequiredArgsConstructor
+public class AsyncLoanCOBExecutorServiceImpl implements
AsyncLoanCOBExecutorService {
+
+private final LoanRepository loanRepository;
+private final JobLocator jobLocator;
+private final ScheduledJobDetailRepository scheduledJobDetailRepository;
+private final JobStarter jobStarter;
+private final JobParameterRepository jobParameterRepository;
+
+@Override
+@Async("loanCOBCatchUpThreadPoolTaskExecutor")
+public void executeLoanCOBCatchUpAsync(FineractContext context) {
+try {
+ThreadLocalContextUtil.init(context);
+LocalDate cobBusinessDate =
ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE);
+List
projectIdAndLastClosedBusinessDate = loanRepository
+.findOldestCOBProcessedLoan(cobBusinessDate);
+LocalDate oldestCOBProcessedDate =
!projectIdAndLastClosedBusinessDate.isEmpty()
+?
projectIdAndLastClosedBusinessDate.get(0).getLastClosedBusinessDate()
+: cobBusinessDate;
+if (oldestCOBProcessedDate.isBefore(cobBusinessDate)) {
+
executeLoanCOBDayByDayUntilCOBBusinessDate(oldestCOBProcessedDate,
cobBusinessDate);
+}
+} catch (NoSuchJobException e) {
+throw new JobNotFoundException(LoanCOBConstant.JOB_NAME, e);
+} catch (JobInstanceAlreadyCompleteException | JobRestartException |
JobParametersInvalidException
+| JobExecutionAlreadyRunningException e) {
+throw new RuntimeException(e);
+} finally {
+ThreadLocalContextUtil.reset();
+}
+}
+
+private void executeLoanCOBDayByDayUntilCOBBusinessDate(LocalDate
oldestCOBProcessedDate, LocalDate cobBusinessDate)
+throws NoSuchJobException, JobInstanceAlreadyCompleteException,
JobExecutionAlreadyRunningException,
+JobParametersInvalidException, JobRestartException {
+Job job = jobLocator.getJob(LoanCOBConstant.JOB_NAME);
+ScheduledJobDetail scheduledJobDetail =
[GitHub] [fineract] taskain7 opened a new pull request, #2896: [FINERACT-1863] Swagger schema fix for configuration APIs
taskain7 opened a new pull request, #2896: URL: https://github.com/apache/fineract/pull/2896 ## Description - swagger schema fix for `PutGlobalConfigurationsRequest` - swagger schema fix for `PutExternalEventConfigurationsRequest` -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] ruchiD commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
ruchiD commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1073275255
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/jobs/SendAsynchronousEventsTasklet.java:
##
@@ -69,20 +79,66 @@ private boolean isDownstreamChannelEnabled() {
return
fineractProperties.getEvents().getExternal().getProducer().getJms().isEnabled();
}
-private List getQueuedEventsBatch() {
+private List getQueuedEventsBatch() {
int readBatchSize = getBatchSize();
Pageable batchSize = PageRequest.ofSize(readBatchSize);
-return
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize);
+return measure(() ->
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize),
+(events, timeTaken) -> log.debug("Loaded {} events in {}ms",
events.size(), timeTaken.toMillis()));
+}
+
+private void sendEvents(List queuedEvents) {
+Map> partitions = generatePartitions(queuedEvents);
+List eventIds =
queuedEvents.stream().map(ExternalEventView::getId).toList();
+sendEventsToProducer(partitions);
+markEventsAsSent(eventIds);
}
-private void processEvents(List queuedEvents) throws
IOException {
-for (ExternalEvent event : queuedEvents) {
-MessageV1 message = messageFactory.createMessage(event);
-byte[] byteMessage =
byteBufferConverter.convert(message.toByteBuffer());
-eventProducer.sendEvent(byteMessage);
-event.setStatus(ExternalEventStatus.SENT);
-event.setSentAt(DateUtils.getOffsetDateTimeOfTenant());
-repository.save(event);
+private void sendEventsToProducer(Map> partitions) {
+eventProducer.sendEvents(partitions);
+}
+
+private void markEventsAsSent(List eventIds) {
+OffsetDateTime sentAt = DateUtils.getOffsetDateTimeOfTenant();
+
+// Partitioning dataset to avoid exception: PreparedStatement can have
at most 65,535 parameters
+List> partitions = Lists.partition(eventIds, 5_000);
+partitions.forEach(partitionedEventIds -> {
+measure(() -> {
+repository.markEventsSent(partitionedEventIds, sentAt);
+}, timeTaken -> {
+log.debug("Took {}ms to update {} events",
timeTaken.toMillis(), partitionedEventIds.size());
+});
+});
+}
+
+private Map> generatePartitions(List
queuedEvents) {
+Map> initialPartitions =
queuedEvents.stream().collect(groupingBy(externalEvent -> {
+Long aggregateRootId = externalEvent.getAggregateRootId();
Review Comment:
Yes.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] ruchiD commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
ruchiD commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1073275255
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/jobs/SendAsynchronousEventsTasklet.java:
##
@@ -69,20 +79,66 @@ private boolean isDownstreamChannelEnabled() {
return
fineractProperties.getEvents().getExternal().getProducer().getJms().isEnabled();
}
-private List getQueuedEventsBatch() {
+private List getQueuedEventsBatch() {
int readBatchSize = getBatchSize();
Pageable batchSize = PageRequest.ofSize(readBatchSize);
-return
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize);
+return measure(() ->
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize),
+(events, timeTaken) -> log.debug("Loaded {} events in {}ms",
events.size(), timeTaken.toMillis()));
+}
+
+private void sendEvents(List queuedEvents) {
+Map> partitions = generatePartitions(queuedEvents);
+List eventIds =
queuedEvents.stream().map(ExternalEventView::getId).toList();
+sendEventsToProducer(partitions);
+markEventsAsSent(eventIds);
}
-private void processEvents(List queuedEvents) throws
IOException {
-for (ExternalEvent event : queuedEvents) {
-MessageV1 message = messageFactory.createMessage(event);
-byte[] byteMessage =
byteBufferConverter.convert(message.toByteBuffer());
-eventProducer.sendEvent(byteMessage);
-event.setStatus(ExternalEventStatus.SENT);
-event.setSentAt(DateUtils.getOffsetDateTimeOfTenant());
-repository.save(event);
+private void sendEventsToProducer(Map> partitions) {
+eventProducer.sendEvents(partitions);
+}
+
+private void markEventsAsSent(List eventIds) {
+OffsetDateTime sentAt = DateUtils.getOffsetDateTimeOfTenant();
+
+// Partitioning dataset to avoid exception: PreparedStatement can have
at most 65,535 parameters
+List> partitions = Lists.partition(eventIds, 5_000);
+partitions.forEach(partitionedEventIds -> {
+measure(() -> {
+repository.markEventsSent(partitionedEventIds, sentAt);
+}, timeTaken -> {
+log.debug("Took {}ms to update {} events",
timeTaken.toMillis(), partitionedEventIds.size());
+});
+});
+}
+
+private Map> generatePartitions(List
queuedEvents) {
+Map> initialPartitions =
queuedEvents.stream().collect(groupingBy(externalEvent -> {
+Long aggregateRootId = externalEvent.getAggregateRootId();
Review Comment:
Yes, makes sense.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
galovics commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1073255186
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/jobs/SendAsynchronousEventsTasklet.java:
##
@@ -69,20 +79,66 @@ private boolean isDownstreamChannelEnabled() {
return
fineractProperties.getEvents().getExternal().getProducer().getJms().isEnabled();
}
-private List getQueuedEventsBatch() {
+private List getQueuedEventsBatch() {
int readBatchSize = getBatchSize();
Pageable batchSize = PageRequest.ofSize(readBatchSize);
-return
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize);
+return measure(() ->
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize),
+(events, timeTaken) -> log.debug("Loaded {} events in {}ms",
events.size(), timeTaken.toMillis()));
+}
+
+private void sendEvents(List queuedEvents) {
+Map> partitions = generatePartitions(queuedEvents);
+List eventIds =
queuedEvents.stream().map(ExternalEventView::getId).toList();
+sendEventsToProducer(partitions);
+markEventsAsSent(eventIds);
}
-private void processEvents(List queuedEvents) throws
IOException {
-for (ExternalEvent event : queuedEvents) {
-MessageV1 message = messageFactory.createMessage(event);
-byte[] byteMessage =
byteBufferConverter.convert(message.toByteBuffer());
-eventProducer.sendEvent(byteMessage);
-event.setStatus(ExternalEventStatus.SENT);
-event.setSentAt(DateUtils.getOffsetDateTimeOfTenant());
-repository.save(event);
+private void sendEventsToProducer(Map> partitions) {
+eventProducer.sendEvents(partitions);
+}
+
+private void markEventsAsSent(List eventIds) {
+OffsetDateTime sentAt = DateUtils.getOffsetDateTimeOfTenant();
+
+// Partitioning dataset to avoid exception: PreparedStatement can have
at most 65,535 parameters
+List> partitions = Lists.partition(eventIds, 5_000);
+partitions.forEach(partitionedEventIds -> {
+measure(() -> {
+repository.markEventsSent(partitionedEventIds, sentAt);
+}, timeTaken -> {
+log.debug("Took {}ms to update {} events",
timeTaken.toMillis(), partitionedEventIds.size());
+});
+});
+}
+
+private Map> generatePartitions(List
queuedEvents) {
+Map> initialPartitions =
queuedEvents.stream().collect(groupingBy(externalEvent -> {
+Long aggregateRootId = externalEvent.getAggregateRootId();
Review Comment:
I don't think we need the type. The aggregate root ID is only used for
picking which producer to use when sending the messages to ensure the right
ordering for a single aggregate root. Even if multiple aggregate types have the
same aggregate root ID, they'll be sent by the same producer, but that's all.
Does that explain it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
galovics commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1073253912
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/repository/ExternalEventRepository.java:
##
@@ -19,19 +19,25 @@
package org.apache.fineract.infrastructure.event.external.repository;
import java.time.LocalDate;
+import java.time.OffsetDateTime;
import java.util.List;
import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEvent;
import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEventStatus;
+import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEventView;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
public interface ExternalEventRepository extends JpaRepository {
-List findByStatusOrderById(ExternalEventStatus status,
Pageable batchSize);
+List findByStatusOrderById(ExternalEventStatus status,
Pageable batchSize);
@Modifying(flushAutomatically = true)
@Query("delete from ExternalEvent e where e.status = :status and
e.businessDate <= :dateForPurgeCriteria")
void deleteOlderEventsWithSentStatus(ExternalEventStatus status, LocalDate
dateForPurgeCriteria);
+
+@Modifying
Review Comment:
Nope.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] ruchiD commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
ruchiD commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1073184573
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/jobs/SendAsynchronousEventsTasklet.java:
##
@@ -69,20 +79,66 @@ private boolean isDownstreamChannelEnabled() {
return
fineractProperties.getEvents().getExternal().getProducer().getJms().isEnabled();
}
-private List getQueuedEventsBatch() {
+private List getQueuedEventsBatch() {
int readBatchSize = getBatchSize();
Pageable batchSize = PageRequest.ofSize(readBatchSize);
-return
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize);
+return measure(() ->
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize),
+(events, timeTaken) -> log.debug("Loaded {} events in {}ms",
events.size(), timeTaken.toMillis()));
+}
+
+private void sendEvents(List queuedEvents) {
+Map> partitions = generatePartitions(queuedEvents);
+List eventIds =
queuedEvents.stream().map(ExternalEventView::getId).toList();
+sendEventsToProducer(partitions);
+markEventsAsSent(eventIds);
}
-private void processEvents(List queuedEvents) throws
IOException {
-for (ExternalEvent event : queuedEvents) {
-MessageV1 message = messageFactory.createMessage(event);
-byte[] byteMessage =
byteBufferConverter.convert(message.toByteBuffer());
-eventProducer.sendEvent(byteMessage);
-event.setStatus(ExternalEventStatus.SENT);
-event.setSentAt(DateUtils.getOffsetDateTimeOfTenant());
-repository.save(event);
+private void sendEventsToProducer(Map> partitions) {
+eventProducer.sendEvents(partitions);
+}
+
+private void markEventsAsSent(List eventIds) {
+OffsetDateTime sentAt = DateUtils.getOffsetDateTimeOfTenant();
+
+// Partitioning dataset to avoid exception: PreparedStatement can have
at most 65,535 parameters
+List> partitions = Lists.partition(eventIds, 5_000);
+partitions.forEach(partitionedEventIds -> {
+measure(() -> {
+repository.markEventsSent(partitionedEventIds, sentAt);
+}, timeTaken -> {
+log.debug("Took {}ms to update {} events",
timeTaken.toMillis(), partitionedEventIds.size());
+});
+});
+}
+
+private Map> generatePartitions(List
queuedEvents) {
+Map> initialPartitions =
queuedEvents.stream().collect(groupingBy(externalEvent -> {
+Long aggregateRootId = externalEvent.getAggregateRootId();
Review Comment:
Should we consider type of aggregate also like (client,loan,savings etc)?
Will there be a case that different type of aggregates have same aggregate id?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] ruchiD commented on a diff in pull request #2886: FINERACT-1724 - Replayed bulk transaction fix
ruchiD commented on code in PR #2886: URL: https://github.com/apache/fineract/pull/2886#discussion_r1073156103 ## fineract-provider/src/main/java/org/apache/fineract/cob/loan/ApplyChargeToOverdueLoansBusinessStep.java: ## @@ -28,6 +29,7 @@ @Component @RequiredArgsConstructor +@BulkEventSupport Review Comment: Can we verify that this setting is not conflicting global configuration setting for bulk events for Loan cob. -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] ruchiD commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
ruchiD commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1073148174
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/producer/jms/JMSMultiExternalEventProducer.java:
##
@@ -0,0 +1,151 @@
+/**
+ * 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.infrastructure.event.external.producer.jms;
+
+import static
org.apache.fineract.infrastructure.core.service.MeasuringUtil.measure;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Future;
+import javax.jms.Connection;
+import javax.jms.Destination;
+import javax.jms.JMSException;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.activemq.ActiveMQConnectionFactory;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.messaging.jms.MessageFactory;
+import org.apache.fineract.infrastructure.core.service.HashingService;
+import
org.apache.fineract.infrastructure.event.external.exception.AcknowledgementTimeoutException;
+import
org.apache.fineract.infrastructure.event.external.producer.ExternalEventProducer;
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.core.task.AsyncTaskExecutor;
+import org.springframework.stereotype.Component;
+
+@Component
+@Slf4j
+@RequiredArgsConstructor
+@ConditionalOnProperty(value =
"fineract.events.external.producer.jms.enabled", havingValue = "true")
+public class JMSMultiExternalEventProducer implements ExternalEventProducer,
InitializingBean {
+
+@Qualifier("eventDestination")
+private final Destination destination;
+
+private final ActiveMQConnectionFactory connectionFactory;
Review Comment:
Can we use JMS ConnectionFactory here instead of ActiveMQConnectionFactory?
ActiveMQConnectionFactory is getting created in ExternalEventJMSConfiguration
by default, we can use JMS abstraction here.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx closed pull request #2888: FINERACT-1840: Fee income is not realised if the loan got fully repaid
josehernandezfintecheandomx closed pull request #2888: FINERACT-1840: Fee income is not realised if the loan got fully repaid URL: https://github.com/apache/fineract/pull/2888 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] b0c1 opened a new pull request, #2895: FINERACT-1707 - S3 upgrade
b0c1 opened a new pull request, #2895:
URL: https://github.com/apache/fineract/pull/2895
FINERACT-1707 - S3 upgrade
[x] Upgrade S3 dependencies and libraries
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] b0c1 closed pull request #2894: FINERACT-1707 - S3 upgrade
b0c1 closed pull request #2894: FINERACT-1707 - S3 upgrade URL: https://github.com/apache/fineract/pull/2894 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2777: [FINERACT-1678] Loan COB Catch Up
galovics commented on code in PR #2777:
URL: https://github.com/apache/fineract/pull/2777#discussion_r1072602144
##
fineract-provider/src/main/java/org/apache/fineract/cob/data/ProjectIdAndLastClosedBusinessDate.java:
##
@@ -0,0 +1,28 @@
+/**
+ * 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.cob.data;
+
+import java.time.LocalDate;
+
+public interface ProjectIdAndLastClosedBusinessDate {
Review Comment:
Why don't we call this simply LoanIdAndLastClosedBusinessDate?
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/async/SpringAsyncConfig.java:
##
@@ -0,0 +1,42 @@
+/**
+ * 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.infrastructure.configuration.async;
+
+import java.util.concurrent.Executor;
+import org.springframework.aop.interceptor.AsyncUncaughtExceptionHandler;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.scheduling.annotation.AsyncConfigurer;
+import org.springframework.scheduling.annotation.EnableAsync;
+import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;
+
+@Configuration
+@EnableAsync
+public class SpringAsyncConfig implements AsyncConfigurer {
+
+@Bean(name = "loanCOBCatchUpThreadPoolTaskExecutor")
+public Executor loanCOBCatchUpThreadPoolTaskExecutor() {
+return new ThreadPoolTaskExecutor();
Review Comment:
Let's set the thread prefix on the executor + you can set the max pool size
to 1 since this executor is only intended to run on a single thread.
##
fineract-provider/src/main/java/org/apache/fineract/cob/service/AsyncLoanCOBExecutorServiceImpl.java:
##
@@ -0,0 +1,107 @@
+/**
+ * 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.cob.service;
+
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.cob.data.ProjectIdAndLastClosedBusinessDate;
+import org.apache.fineract.cob.loan.LoanCOBConstant;
+import org.apache.fineract.infrastructure.businessdate.domain.BusinessDateType;
+import org.apache.fineract.infrastructure.core.domain.FineractContext;
+import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil;
+import org.apache.fineract.infrastructure.jobs.data.JobParameterDTO;
+import org.apache.fineract.infrastructure.jobs.domain.JobParameter;
+import org.apache.fineract.infrastructure.jobs.domain.JobParameterRepository;
+import
[GitHub] [fineract] galovics merged pull request #2893: FINERACT-1724: Fix loan status change bug at the event of transaction replay
galovics merged PR #2893: URL: https://github.com/apache/fineract/pull/2893 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2892: FINERACT-1724: Add Charge-off transaction into r_enum_value table
galovics merged PR #2892: URL: https://github.com/apache/fineract/pull/2892 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] b0c1 opened a new pull request, #2894: FINERACT-1707 - S3 upgrade
b0c1 opened a new pull request, #2894:
URL: https://github.com/apache/fineract/pull/2894
FINERACT-1707 - S3 upgrade
[x] Upgrade S3 dependencies and libraries without code modifications
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2886: FINERACT-1724 - Replayed bulk transaction fix
galovics commented on code in PR #2886:
URL: https://github.com/apache/fineract/pull/2886#discussion_r1072434667
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/business/annotation/BulkEventSupport.java:
##
@@ -0,0 +1,36 @@
+/**
+ * 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.infrastructure.event.business.annotation;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Annotation to be used to collect events into BulkEvent
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
Review Comment:
Let's allow this on method level too.
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/business/aspect/BulkEventAspect.java:
##
@@ -0,0 +1,61 @@
+/**
+ * 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.infrastructure.event.business.aspect;
+
+import lombok.RequiredArgsConstructor;
+import lombok.SneakyThrows;
+import
org.apache.fineract.infrastructure.event.business.annotation.BulkEventSupport;
+import
org.apache.fineract.infrastructure.event.business.service.BusinessEventNotifierService;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.springframework.stereotype.Component;
+
+@Component
+@RequiredArgsConstructor
+@Aspect
+public class BulkEventAspect {
+
+private final BusinessEventNotifierService businessEventNotifierService;
+
+@Around("execution(*
org.apache.fineract.commands.handler.NewCommandSourceHandler.processCommand(..))
&& @within(bulkEventSupport)")
Review Comment:
I think we could remove the restriction to use this only on
NewCommandSourceHandler interfaces, what if we just allow it everywhere?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy opened a new pull request, #2893: FINERACT-1724: Fix loan status change bug at the event of transaction replay
adamsaghy opened a new pull request, #2893:
URL: https://github.com/apache/fineract/pull/2893
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
adamsaghy commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072383085
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/repository/ExternalEventRepository.java:
##
@@ -19,19 +19,25 @@
package org.apache.fineract.infrastructure.event.external.repository;
import java.time.LocalDate;
+import java.time.OffsetDateTime;
import java.util.List;
import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEvent;
import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEventStatus;
+import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEventView;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
public interface ExternalEventRepository extends JpaRepository {
-List findByStatusOrderById(ExternalEventStatus status,
Pageable batchSize);
+List findByStatusOrderById(ExternalEventStatus status,
Pageable batchSize);
@Modifying(flushAutomatically = true)
@Query("delete from ExternalEvent e where e.status = :status and
e.businessDate <= :dateForPurgeCriteria")
void deleteOlderEventsWithSentStatus(ExternalEventStatus status, LocalDate
dateForPurgeCriteria);
+
+@Modifying
Review Comment:
Can it happen to fetch the "not yet sent" events again from the DB before
this async event sending logic got executed for all of the events?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
galovics commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072370757
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/repository/ExternalEventRepository.java:
##
@@ -19,19 +19,25 @@
package org.apache.fineract.infrastructure.event.external.repository;
import java.time.LocalDate;
+import java.time.OffsetDateTime;
import java.util.List;
import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEvent;
import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEventStatus;
+import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEventView;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
public interface ExternalEventRepository extends JpaRepository {
-List findByStatusOrderById(ExternalEventStatus status,
Pageable batchSize);
+List findByStatusOrderById(ExternalEventStatus status,
Pageable batchSize);
@Modifying(flushAutomatically = true)
@Query("delete from ExternalEvent e where e.status = :status and
e.businessDate <= :dateForPurgeCriteria")
void deleteOlderEventsWithSentStatus(ExternalEventStatus status, LocalDate
dateForPurgeCriteria);
+
+@Modifying
Review Comment:
I don't see the reason for it, do you?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
galovics commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072367835
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/messaging/jms/ActiveMQMessageFactory.java:
##
@@ -16,21 +16,20 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.fineract.infrastructure.event.external.config;
+package org.apache.fineract.infrastructure.core.messaging.jms;
-import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
-import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Configuration;
-import org.springframework.integration.channel.DirectChannel;
-import org.springframework.integration.config.EnableIntegration;
+import javax.jms.BytesMessage;
+import javax.jms.JMSException;
+import org.apache.activemq.command.ActiveMQBytesMessage;
+import org.springframework.stereotype.Component;
-@Configuration
-@EnableIntegration
-@ConditionalOnProperty(value = "fineract.events.external.enabled", havingValue
= "true")
-public class ExternalEventProducerConfiguration {
+@Component
Review Comment:
Good point and I intentionally left it like this. If RabbitMQ or other
messaging solutions come into play, I wanted to defer the "frameworking" part
of this.
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/producer/jms/JMSMultiExternalEventProducer.java:
##
@@ -0,0 +1,151 @@
+/**
+ * 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.infrastructure.event.external.producer.jms;
+
+import static
org.apache.fineract.infrastructure.core.service.MeasuringUtil.measure;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Future;
+import javax.jms.Connection;
+import javax.jms.Destination;
+import javax.jms.JMSException;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.activemq.ActiveMQConnectionFactory;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.messaging.jms.MessageFactory;
+import org.apache.fineract.infrastructure.core.service.HashingService;
+import
org.apache.fineract.infrastructure.event.external.exception.AcknowledgementTimeoutException;
+import
org.apache.fineract.infrastructure.event.external.producer.ExternalEventProducer;
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.core.task.AsyncTaskExecutor;
+import org.springframework.stereotype.Component;
+
+@Component
+@Slf4j
+@RequiredArgsConstructor
+@ConditionalOnProperty(value =
"fineract.events.external.producer.jms.enabled", havingValue = "true")
+public class JMSMultiExternalEventProducer implements ExternalEventProducer,
InitializingBean {
+
+@Qualifier("eventDestination")
+private final Destination destination;
+
+private final ActiveMQConnectionFactory connectionFactory;
Review Comment:
Same as above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
galovics commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072366927
##
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanAccountsStayedLockedBusinessEvent.java:
##
@@ -39,4 +39,9 @@ public String getType() {
public String getCategory() {
return CATEGORY;
}
+
+@Override
Review Comment:
In case a single aggregate root cannot be identified, for example the
LoanAccountsStayedLockedBusinessEvent. Multiple accounts stayed locked so
there's no singular event hence it doesn't matter which producer picks up the
message to be sent.
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy opened a new pull request, #2892: FINERACT-1724: Add Charge-off transaction into r_enum_value table
adamsaghy opened a new pull request, #2892:
URL: https://github.com/apache/fineract/pull/2892
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
adamsaghy commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072301485
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/repository/ExternalEventRepository.java:
##
@@ -19,19 +19,25 @@
package org.apache.fineract.infrastructure.event.external.repository;
import java.time.LocalDate;
+import java.time.OffsetDateTime;
import java.util.List;
import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEvent;
import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEventStatus;
+import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEventView;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
public interface ExternalEventRepository extends JpaRepository {
-List findByStatusOrderById(ExternalEventStatus status,
Pageable batchSize);
+List findByStatusOrderById(ExternalEventStatus status,
Pageable batchSize);
@Modifying(flushAutomatically = true)
@Query("delete from ExternalEvent e where e.status = :status and
e.businessDate <= :dateForPurgeCriteria")
void deleteOlderEventsWithSentStatus(ExternalEventStatus status, LocalDate
dateForPurgeCriteria);
+
+@Modifying
Review Comment:
Would autoflush make sense here?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
adamsaghy commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072301485
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/repository/ExternalEventRepository.java:
##
@@ -19,19 +19,25 @@
package org.apache.fineract.infrastructure.event.external.repository;
import java.time.LocalDate;
+import java.time.OffsetDateTime;
import java.util.List;
import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEvent;
import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEventStatus;
+import
org.apache.fineract.infrastructure.event.external.repository.domain.ExternalEventView;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
public interface ExternalEventRepository extends JpaRepository {
-List findByStatusOrderById(ExternalEventStatus status,
Pageable batchSize);
+List findByStatusOrderById(ExternalEventStatus status,
Pageable batchSize);
@Modifying(flushAutomatically = true)
@Query("delete from ExternalEvent e where e.status = :status and
e.businessDate <= :dateForPurgeCriteria")
void deleteOlderEventsWithSentStatus(ExternalEventStatus status, LocalDate
dateForPurgeCriteria);
+
+@Modifying
Review Comment:
autoflush?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
adamsaghy commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072297910
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/producer/jms/JMSMultiExternalEventProducer.java:
##
@@ -0,0 +1,151 @@
+/**
+ * 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.infrastructure.event.external.producer.jms;
+
+import static
org.apache.fineract.infrastructure.core.service.MeasuringUtil.measure;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Future;
+import javax.jms.Connection;
+import javax.jms.Destination;
+import javax.jms.JMSException;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.activemq.ActiveMQConnectionFactory;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.messaging.jms.MessageFactory;
+import org.apache.fineract.infrastructure.core.service.HashingService;
+import
org.apache.fineract.infrastructure.event.external.exception.AcknowledgementTimeoutException;
+import
org.apache.fineract.infrastructure.event.external.producer.ExternalEventProducer;
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.core.task.AsyncTaskExecutor;
+import org.springframework.stereotype.Component;
+
+@Component
+@Slf4j
+@RequiredArgsConstructor
+@ConditionalOnProperty(value =
"fineract.events.external.producer.jms.enabled", havingValue = "true")
+public class JMSMultiExternalEventProducer implements ExternalEventProducer,
InitializingBean {
+
+@Qualifier("eventDestination")
+private final Destination destination;
+
+private final ActiveMQConnectionFactory connectionFactory;
Review Comment:
We will support only the ActiveMQ?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
adamsaghy commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072285436
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/jobs/SendAsynchronousEventsTasklet.java:
##
@@ -69,20 +79,66 @@ private boolean isDownstreamChannelEnabled() {
return
fineractProperties.getEvents().getExternal().getProducer().getJms().isEnabled();
}
-private List getQueuedEventsBatch() {
+private List getQueuedEventsBatch() {
int readBatchSize = getBatchSize();
Pageable batchSize = PageRequest.ofSize(readBatchSize);
-return
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize);
+return measure(() ->
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize),
Review Comment:
I guess for production we dont need this :)
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
adamsaghy commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072285436
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/jobs/SendAsynchronousEventsTasklet.java:
##
@@ -69,20 +79,66 @@ private boolean isDownstreamChannelEnabled() {
return
fineractProperties.getEvents().getExternal().getProducer().getJms().isEnabled();
}
-private List getQueuedEventsBatch() {
+private List getQueuedEventsBatch() {
int readBatchSize = getBatchSize();
Pageable batchSize = PageRequest.ofSize(readBatchSize);
-return
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize);
+return measure(() ->
repository.findByStatusOrderById(ExternalEventStatus.TO_BE_SENT, batchSize),
Review Comment:
I guess for production we dont need this :)
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2889: FINERACT-1806: Trigger event when charge-off happens
galovics merged PR #2889: URL: https://github.com/apache/fineract/pull/2889 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
adamsaghy commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072171238
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/messaging/jms/ActiveMQMessageFactory.java:
##
@@ -16,21 +16,20 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.fineract.infrastructure.event.external.config;
+package org.apache.fineract.infrastructure.core.messaging.jms;
-import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
-import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Configuration;
-import org.springframework.integration.channel.DirectChannel;
-import org.springframework.integration.config.EnableIntegration;
+import javax.jms.BytesMessage;
+import javax.jms.JMSException;
+import org.apache.activemq.command.ActiveMQBytesMessage;
+import org.springframework.stereotype.Component;
-@Configuration
-@EnableIntegration
-@ConditionalOnProperty(value = "fineract.events.external.enabled", havingValue
= "true")
-public class ExternalEventProducerConfiguration {
+@Component
Review Comment:
Dont we need condition whether we wanna have ActiveMQMessageFactory?
Or will it be the de facto implementation and others might override it only?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
adamsaghy commented on code in PR #2891:
URL: https://github.com/apache/fineract/pull/2891#discussion_r1072168430
##
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanAccountsStayedLockedBusinessEvent.java:
##
@@ -39,4 +39,9 @@ public String getType() {
public String getCategory() {
return CATEGORY;
}
+
+@Override
Review Comment:
Null?
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2880: FINERACT-1694-External-event-producer-batch-size-configuration
galovics merged PR #2880: URL: https://github.com/apache/fineract/pull/2880 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2883: chore(deps): update all non-major dependencies
galovics merged PR #2883: URL: https://github.com/apache/fineract/pull/2883 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2889: FINERACT-1806: Trigger event when charge-off happens
adamsaghy commented on code in PR #2889:
URL: https://github.com/apache/fineract/pull/2889#discussion_r1072099383
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/mapper/loan/LoanTransactionDataMapper.java:
##
@@ -22,9 +22,11 @@
import
org.apache.fineract.infrastructure.event.external.service.serialization.mapper.support.AvroMapperConfig;
import org.apache.fineract.portfolio.loanaccount.data.LoanTransactionData;
import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
@Mapper(config = AvroMapperConfig.class)
public interface LoanTransactionDataMapper {
+@Mapping(target = "unpaidCharges", ignore = true)
Review Comment:
sure
##
fineract-avro-schemas/src/main/avro/loan/v1/UnpaidChargeDataV1.avsc:
##
@@ -0,0 +1,31 @@
+{
+"name": "UnpaidChargeDataV1",
+"namespace": "org.apache.fineract.avro.loan.v1",
+"type": "record",
+"fields": [
+{
+"default": null,
+"name": "chargeId",
+"type": [
+"null",
+"long"
+]
+},
+{
+"default": null,
+"name": "chargeName",
Review Comment:
yes
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2889: FINERACT-1806: Trigger event when charge-off happens
adamsaghy commented on code in PR #2889:
URL: https://github.com/apache/fineract/pull/2889#discussion_r1072099130
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/data/UnpaidChargeData.java:
##
@@ -0,0 +1,39 @@
+/**
+ * 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.loanaccount.data;
+
+import java.math.BigDecimal;
+import lombok.Getter;
+
+/**
+ * Immutable data object representing the total unpaid charge details
+ */
+@Getter
+public class UnpaidChargeData {
+
+private final Long chargeId;
+private final String chargeName;
+private final BigDecimal outstandingAmount;
+
+public UnpaidChargeData(Long chargeId, String chargeName, BigDecimal
outstandingAmount) {
Review Comment:
sure
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2890: FINERACT-1724: Fix id type of LoanAccountDataV1
galovics merged PR #2890: URL: https://github.com/apache/fineract/pull/2890 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2889: FINERACT-1806: Trigger event when charge-off happens
adamsaghy commented on code in PR #2889:
URL: https://github.com/apache/fineract/pull/2889#discussion_r1072094462
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/mapper/loan/LoanAccountDataMapper.java:
##
@@ -36,5 +36,6 @@ public interface LoanAccountDataMapper {
@Mapping(source = "interestRecalculationEnabled", target =
"isInterestRecalculationEnabled")
LoanAccountDataV1 map(LoanAccountData source);
+@Mapping(target = "unpaidCharges", ignore = true)
Review Comment:
I give a try :)
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2889: FINERACT-1806: Trigger event when charge-off happens
adamsaghy commented on code in PR #2889:
URL: https://github.com/apache/fineract/pull/2889#discussion_r1072091915
##
fineract-avro-schemas/src/main/avro/loan/v1/UnpaidChargeDataV1.avsc:
##
@@ -0,0 +1,31 @@
+{
+"name": "UnpaidChargeDataV1",
+"namespace": "org.apache.fineract.avro.loan.v1",
+"type": "record",
+"fields": [
+{
+"default": null,
+"name": "chargeId",
+"type": [
+"null",
+"long"
+]
+},
+{
+"default": null,
+"name": "chargeName",
+"type": [
+"null",
+"string"
+]
+},
+{
+"default": null,
+"name": "outstandingAmount",
Review Comment:
Greater than 0
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2889: FINERACT-1806: Trigger event when charge-off happens
adamsaghy commented on code in PR #2889:
URL: https://github.com/apache/fineract/pull/2889#discussion_r1072091551
##
fineract-avro-schemas/src/main/avro/loan/v1/UnpaidChargeDataV1.avsc:
##
@@ -0,0 +1,31 @@
+{
+"name": "UnpaidChargeDataV1",
+"namespace": "org.apache.fineract.avro.loan.v1",
+"type": "record",
+"fields": [
+{
+"default": null,
+"name": "chargeId",
+"type": [
+"null",
Review Comment:
Fair point!
--
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics opened a new pull request, #2891: FINERACT-1724: Implemented support for multiple parallel JMS producers for external events
galovics opened a new pull request, #2891:
URL: https://github.com/apache/fineract/pull/2891
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2889: FINERACT-1806: Trigger event when charge-off happens
galovics commented on code in PR #2889:
URL: https://github.com/apache/fineract/pull/2889#discussion_r1072063934
##
fineract-avro-schemas/src/main/avro/loan/v1/UnpaidChargeDataV1.avsc:
##
@@ -0,0 +1,31 @@
+{
+"name": "UnpaidChargeDataV1",
+"namespace": "org.apache.fineract.avro.loan.v1",
+"type": "record",
+"fields": [
+{
+"default": null,
+"name": "chargeId",
+"type": [
+"null",
Review Comment:
This cannot be null, can it?
##
fineract-avro-schemas/src/main/avro/loan/v1/UnpaidChargeDataV1.avsc:
##
@@ -0,0 +1,31 @@
+{
+"name": "UnpaidChargeDataV1",
+"namespace": "org.apache.fineract.avro.loan.v1",
+"type": "record",
+"fields": [
+{
+"default": null,
+"name": "chargeId",
+"type": [
+"null",
+"long"
+]
+},
+{
+"default": null,
+"name": "chargeName",
Review Comment:
Same here, name is mandatory for charges, isn't it?
##
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/data/UnpaidChargeData.java:
##
@@ -0,0 +1,39 @@
+/**
+ * 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.loanaccount.data;
+
+import java.math.BigDecimal;
+import lombok.Getter;
+
+/**
+ * Immutable data object representing the total unpaid charge details
+ */
+@Getter
+public class UnpaidChargeData {
+
+private final Long chargeId;
+private final String chargeName;
+private final BigDecimal outstandingAmount;
+
+public UnpaidChargeData(Long chargeId, String chargeName, BigDecimal
outstandingAmount) {
Review Comment:
Note if what I wrote in the Avro schema is true (i.e. not null values) then
let's add null checks here to ensure we cannot create this object without
having proper data in it.
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/mapper/loan/LoanAccountDataMapper.java:
##
@@ -36,5 +36,6 @@ public interface LoanAccountDataMapper {
@Mapping(source = "interestRecalculationEnabled", target =
"isInterestRecalculationEnabled")
LoanAccountDataV1 map(LoanAccountData source);
+@Mapping(target = "unpaidCharges", ignore = true)
Review Comment:
I don't think we even need this method here.
We could just modify the annotation on the class to this:
```
@Mapper(config = AvroMapperConfig.class, uses = {
LoanTransactionDataMapper.class })
```
##
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/mapper/loan/LoanTransactionDataMapper.java:
##
@@ -22,9 +22,11 @@
import
org.apache.fineract.infrastructure.event.external.service.serialization.mapper.support.AvroMapperConfig;
import org.apache.fineract.portfolio.loanaccount.data.LoanTransactionData;
import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
@Mapper(config = AvroMapperConfig.class)
public interface LoanTransactionDataMapper {
+@Mapping(target = "unpaidCharges", ignore = true)
Review Comment:
Can we put a comment here why this is ignored? Just for future reference.
##
fineract-avro-schemas/src/main/avro/loan/v1/UnpaidChargeDataV1.avsc:
##
@@ -0,0 +1,31 @@
+{
+"name": "UnpaidChargeDataV1",
+"namespace": "org.apache.fineract.avro.loan.v1",
+"type": "record",
+"fields": [
+{
+"default": null,
+"name": "chargeId",
+"type": [
+"null",
+"long"
+]
+},
+{
+"default": null,
+"name": "chargeName",
+"type": [
+"null",
+"string"
+]
+},
+{
+"default": null,
+"name": "outstandingAmount",
Review Comment:
Can this be null or only zero?
--
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:
[GitHub] [fineract] galovics merged pull request #2879: [FINERACT-1852] Swagger schema inconsistency for Business Step Config
galovics merged PR #2879: URL: https://github.com/apache/fineract/pull/2879 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy opened a new pull request, #2890: FINERACT-1724: Fix id type of LoanAccountDataV1
adamsaghy opened a new pull request, #2890:
URL: https://github.com/apache/fineract/pull/2890
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] adamsaghy opened a new pull request, #2889: FINERACT-1806: Trigger event when charge-off happens
adamsaghy opened a new pull request, #2889:
URL: https://github.com/apache/fineract/pull/2889
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2887: [FINERACT-1859] Missing configuration for business event
galovics merged PR #2887: URL: https://github.com/apache/fineract/pull/2887 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] adamsaghy merged pull request #2881: FINERACT-1856: Fix to update datatable entry is failing after column added lately
adamsaghy merged PR #2881: URL: https://github.com/apache/fineract/pull/2881 -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx opened a new pull request, #2888: FINERACT-1840: Fee income is not realised if the loan got fully repaid
josehernandezfintecheandomx opened a new pull request, #2888:
URL: https://github.com/apache/fineract/pull/2888
## Description
Fee income is not realised if the loan got fully repaid due there is only
one debit gl entry with the transaction amount consolidated
https://user-images.githubusercontent.com/44206706/212818228-d6d98932-53c9-4c95-9434-69f3a3dff011.png;>
Ignore if these details are present on the associated [Apache Fineract JIRA
ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per
https://github.com/apache/fineract/#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 for verifying the changes made.
- [ ] Follow coding conventions at
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
- [ ] Submission is not a "code dump". (Large changes can be made "in
repository" via a branch. Ask on the developer mailing list for guidance, if
required.)
FYI our guidelines for code reviews are at
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: commits-unsubscr...@fineract.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
