Skip to content

Conversation

@nikpawar89
Copy link
Member

GLIM and GSIM functionalities

@nikpawar89 nikpawar89 force-pushed the GLIM_GSIM_enhancement branch from 0c46b9d to a4b570c Compare February 19, 2018 05:14
Copy link
Contributor

@avikganguly01 avikganguly01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me other than some files being unreviewable. I would like to see how the APIs are being used. Can you point me to your community-app PR?

UNIQUE INDEX `gsim_account_no_UNIQUE` (`account_number`),
INDEX `FK_gsim_group_id` (`group_id`),
CONSTRAINT `FK_gsim_group_id` FOREIGN KEY (`group_id`) REFERENCES `m_group` (`id`)
) COLLATE='utf8_general_ci' ENGINE=InnoDB AUTO_INCREMENT=1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove collate, engine and auto increment specifications from the migration scripts.

import com.google.gson.JsonArray;
import com.google.gson.JsonElement;

@Service
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreviewable code. Please make sure file diff for existing files is readable by changing only parts which you intend to change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been due to squash I performed to merge several commits, I will get it corrected

if(isGsim && (deposit.getId()!=null))
{
GroupSavingsIndividualMonitoring gsim=gsimRepository.findOne(account.getGsim().getId());
System.out.println("parent deposit "+gsim.getParentDeposit());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to slf4j logger debug level if you wish to log something for debugging purposes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noted

import org.apache.fineract.accounting.common.AccountingConstants.SAVINGS_PRODUCT_ACCOUNTING_PARAMS;

public class DepositsApiConstants {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreviewable file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will get it corrected

@nikpawar89
Copy link
Member Author

For community app, I will have to prepare PR, by getting out gsim and gsim code.. will try get it out soon. Meanwhile we can work on getting this merged

@ippezshelby
Copy link
Contributor

@edcable, @avikganguly01 and @nikpawar89 , How soon can this be merged into Fineract including community-app side. It seems we have a need for this, please advice

@francisguchie
Copy link
Contributor

@nikpawar89 , @avikganguly01 ,
How soon will this be merged into Fineract and the comm-app. There are some good updates inside of 18.03.01 that would be nice to have with this included onto them

@rrpawar96 rrpawar96 mentioned this pull request Nov 17, 2018
@vorburger
Copy link
Member

@nikpawar89 thank you very much for your interest in and contribution to Fineract/Mifos! I'm an (old) volunteer who got reminded of Mifos/Fineract this week-end at the FOSDEM conference, and I'm taking a moment to help the project clean up some of it's old issues and pull requests etc.

We were wondering whether you are still alive, and would be willing to git rebase develop this PR and take any previous review feedback into account? If we don't hear from you within 2 weeks, we will take the liberty to close this PR without merging it, to avoid confusion for future new contributors looking at this repository - hope you understand and that's fair.

@francisguchie @Ippezrobert this needs someone to rebase and resolve conflicts, if you could do that, or find someone who could, then I'm sure other committers (probably not myself) would gladly merge this.

/Cc FYI @apache/fineract-committers

@vorburger
Copy link
Member

Friendly service reminder: I will close this PR in 3 days to help this project have a clean list of active PRs.

@francisguchie
Copy link
Contributor

@vorburger , Me and @nikpawar have been discussing about this today. He is still alive and will be responding soon on this.

@nikpawar89
Copy link
Member Author

this PR is being handled in PR:
#504

@nikpawar89 nikpawar89 closed this Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants