You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/02/21 18:28:02 UTC
[GitHub] [fineract] fynmanoj opened a new pull request #2046: Fineract 1510
fynmanoj opened a new pull request #2046:
URL: https://github.com/apache/fineract/pull/2046
## 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/api-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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] fynmanoj commented on a change in pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
fynmanoj commented on a change in pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#discussion_r812615122
##########
File path: fineract-provider/src/main/resources/db/changelog/tenant/parts/0005_savings_transaction_reversal.xml
##########
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+ <changeSet author="fineract" id="1">
+ <addColumn tableName="m_savings_account_transaction">
+ <column name="original_transaction_id" type="BIGINT"/>
+ <column name="is_reversal" type="TINYINT" defaultValueNumeric="0">
Review comment:
REF: https://github.com/apache/fineract/pull/2049
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a change in pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
galovics commented on a change in pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#discussion_r812617375
##########
File path: fineract-provider/src/main/resources/db/changelog/tenant/parts/0005_savings_transaction_reversal.xml
##########
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+ <changeSet author="fineract" id="1">
+ <addColumn tableName="m_savings_account_transaction">
+ <column name="original_transaction_id" type="BIGINT"/>
+ <column name="is_reversal" type="TINYINT" defaultValueNumeric="0">
Review comment:
Thanks @fynmanoj. It's not gonna be an issue for MySQL 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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vidakovic commented on pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
vidakovic commented on pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#issuecomment-1047144021
@fynmanoj looks like a `./gradlew spotlessApply` will fix 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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] fynmanoj commented on a change in pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
fynmanoj commented on a change in pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#discussion_r812612707
##########
File path: fineract-provider/src/main/resources/db/changelog/tenant/parts/0005_savings_transaction_reversal.xml
##########
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+ <changeSet author="fineract" id="1">
+ <addColumn tableName="m_savings_account_transaction">
+ <column name="original_transaction_id" type="BIGINT"/>
+ <column name="is_reversal" type="TINYINT" defaultValueNumeric="0">
Review comment:
ok, I shall send a new 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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] avikganguly01 commented on pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
avikganguly01 commented on pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#issuecomment-1047635596
@fynmanoj : Some checkstyle errors are there in SavingsAccount files which is failing the basicauth build.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] fynmanoj commented on pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
fynmanoj commented on pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#issuecomment-1047891528
I have changed migration script to liquibase format. Also this migration script is running fine in my local when ran separately
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] avikganguly01 commented on pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
avikganguly01 commented on pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#issuecomment-1047916120
@fynmanoj : testAccountBalanceAfterSavingsTransactionReversalPosting() FAILED
java.lang.AssertionError: 1 expectation failed.
Expected status code <200> but was <404>.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] avikganguly01 merged pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
avikganguly01 merged pull request #2046:
URL: https://github.com/apache/fineract/pull/2046
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] fynmanoj commented on a change in pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
fynmanoj commented on a change in pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#discussion_r812613556
##########
File path: fineract-provider/src/main/resources/db/changelog/tenant/parts/0005_savings_transaction_reversal.xml
##########
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+ <changeSet author="fineract" id="1">
+ <addColumn tableName="m_savings_account_transaction">
+ <column name="original_transaction_id" type="BIGINT"/>
+ <column name="is_reversal" type="TINYINT" defaultValueNumeric="0">
Review comment:
@galovics , hope the change TINYINT -> BOOLEAN will not affect MySQL migration
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] avikganguly01 commented on a change in pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
avikganguly01 commented on a change in pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#discussion_r811780665
##########
File path: integration-tests/src/test/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -2324,4 +2324,56 @@ public void testSavingsAccountWithdrawalChargesOnPaymentTypes() {
HashMap summaryTwo = this.savingsAccountHelper.getSavingsSummary(savingsId);
assertEquals(balanceAfterChargeTwo, summaryTwo.get("accountBalance"), "Verifying Balance after withdrawal charge two ");
}
+
+ /**
+ * Test Transaction reversal feature, here a new reversal transaction is posted when a savings transaction is reversed
+ */
+
+ @Test
+ public void testAccountBalanceAfterSavingsTransactionReversalPosting() {
+ this.savingsAccountHelper = new SavingsAccountHelper(this.requestSpec, this.responseSpec);
+
+ final Integer clientID = ClientHelper.createClient(this.requestSpec, this.responseSpec);
+ ClientHelper.verifyClientCreatedOnServer(this.requestSpec, this.responseSpec, clientID);
+ ClientHelper.verifyClientCreatedOnServer(this.requestSpec, this.responseSpec, clientID);
+ // Assertions.assertNotNull(clientID);
+ final String minBalanceForInterestCalculation = null;
+ final String minRequiredBalance = "0";
+ final String enforceMinRequiredBalance = "false";
+ final boolean allowOverdraft = true;
+ final String MINIMUM_OPENING_BALANCE = "0";
+
+ final Integer savingsProductID = createSavingsProduct(this.requestSpec, this.responseSpec, MINIMUM_OPENING_BALANCE,
+ minBalanceForInterestCalculation, minRequiredBalance, enforceMinRequiredBalance, allowOverdraft);
+ Assertions.assertNotNull(savingsProductID);
+
+ final Integer savingsId = this.savingsAccountHelper.applyForSavingsApplication(clientID, savingsProductID, ACCOUNT_TYPE_INDIVIDUAL);
+ Assertions.assertNotNull(savingsId);
+
+ HashMap savingsStatusHashMap = this.savingsAccountHelper.approveSavings(savingsId);
+ SavingsStatusChecker.verifySavingsIsApproved(savingsStatusHashMap);
+
+ savingsStatusHashMap = this.savingsAccountHelper.activateSavings(savingsId);
+ SavingsStatusChecker.verifySavingsIsActive(savingsStatusHashMap);
+
+ Integer depositTransactionId = (Integer) this.savingsAccountHelper.depositToSavingsAccount(savingsId, "500",
+ SavingsAccountHelper.TRANSACTION_DATE, CommonConstants.RESPONSE_RESOURCE_ID);
+
+ this.savingsAccountHelper.reverseSavingsAccountTransaction(savingsId, depositTransactionId);
+
+ HashMap reversedDepositTransaction = this.savingsAccountHelper.getSavingsTransaction(savingsId, depositTransactionId);
+
+ Assertions.assertTrue((Boolean) reversedDepositTransaction.get("reversed"));
+
+ HashMap reversalDepositTransaction = this.savingsAccountHelper.getSavingsTransaction(savingsId, depositTransactionId+1);
+
+ Assertions.assertTrue((Boolean) reversalDepositTransaction.get("isReversal"));
Review comment:
LGTM
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] avikganguly01 commented on pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
avikganguly01 commented on pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#issuecomment-1047867093
testApplyAnnualFeeForSavingsJobOutcome seems to be failing. Is it passing if you run that test separate from all the other integration tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a change in pull request #2046: Fineract 1510
Posted by GitBox <gi...@apache.org>.
galovics commented on a change in pull request #2046:
URL: https://github.com/apache/fineract/pull/2046#discussion_r812363808
##########
File path: fineract-provider/src/main/resources/db/changelog/tenant/parts/0005_savings_transaction_reversal.xml
##########
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+ <changeSet author="fineract" id="1">
+ <addColumn tableName="m_savings_account_transaction">
+ <column name="original_transaction_id" type="BIGINT"/>
+ <column name="is_reversal" type="TINYINT" defaultValueNumeric="0">
Review comment:
This got merged before I could take a look. Could you please modify this column to be type of BOOLEAN instead of TINYINT? The PostgreSQL support just got merged an hour before this and TINYINT -> BOOLEAN conversion is not automatic for PostgreSQL.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org