On Tue, Apr 21, 2020 at 4:38 PM Rahul Pawar <[email protected]> wrote:
> > Submitted New PR: https://github.com/apache/fineract/pull/777 and new > JIRA has been raised: https://jira.apache.org/jira/browse/FINERACT-902. > > This was the blocker for GSIM_GLIM in PR : > https://github.com/apache/fineract/pull/738. > So, if anyone has a better solution then let me know or else we can merge > it. > Nope, we can never merge it with that... ;-) I've left some feedback in https://jira.apache.org/jira/browse/FINERACT-902, let's continue this discussion there, to save others who don't care about the details emails on this thread? (Anyone interested is of course welcome to Watch, and Help with by Commenting in FINERACT-902 re. this matter.) > Regards, > Rahul > > > On Tue, Apr 21, 2020 at 4:03 AM Nikhil Pawar <[email protected]> wrote: > >> Oh I recollect this setting now. >> I had added two new tables - glim_accounts and gsim_accounts and they had >> FK with our Gigantic tables Loan and Savings account which caused this >> exception. >> After adding these two settings, the issue was resolved. >> >> Could you please open another PR and we can discuss and commit it before >> merging GSIM and GLIM PR? >> Apologies for missing this point. >> >> By the way, good work on moving swiftly on addressing review comments, >> with this we can wrap up early. >> >> Regards, >> Nikhil >> >> On Mon, Apr 20, 2020 at 6:19 PM Rahul Pawar <[email protected]> >> wrote: >> >>> Hi Michael/Nikhil, >>> >>> After removing the changes from Persistence.xml, I got an error saying >>> "too many tables; MySQL can only use 61 tables in a join hibernate". >>> Looks like this setting is a prerequisite for the PR. >>> >>> >>> Hi Francis, >>> >>> I have removed hyphens from both GSIM/GLIM Accounts. >>> [image: image.png] >>> This is how it looks. (without hyphens 10 digit Account Number) >>> >>> On Mon, Apr 20, 2020 at 9:54 PM Rahul Pawar <[email protected]> >>> wrote: >>> >>>> Thanks Michael, Francis and Nikhil for your feedback. I will address >>>> the concerns raised, based on priority. I will keep you guys posted on the >>>> progress. >>>> >>>> On Mon, Apr 20, 2020 at 7:56 PM Francis Guchie <[email protected]> >>>> wrote: >>>> >>>>> Dear Nikhil, >>>>> >>>>> I would like to make an emphasis on the following >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> Critical Changes: >>>>> >>>>> 1) Remove hyphens from child account numbers: I have explained in >>>>> depth why we should do this in my feedback. >>>>> >>>>> Grouping Clients is paramount especially in East and West Africa and >>>>> that is how i found the name(Abreviation GLIM/GSIM). >>>>> >>>>> Group Lending with Individual Monitoring / Group Savings with >>>>> Individual Monitoring >>>>> >>>>> >>>>> The GLIM - GSIM has helped a lot in getting microfinance companies to >>>>> use MifosX >>>>> >>>>> >>>>> One major challenge i was faced with is this Module is the hyphen- >>>>> >>>>> Clients in a a group that have GSIM accounts with hyphens ; >>>>> >>>>> >>>>> 1. can not be issued with check-books >>>>> 2. can not have BBANs generated "comfortably" as BBANs have a >>>>> standard format that does not allow for hyphens >>>>> 3. the Maximum Account number should 10 digits on with other >>>>> numbers are added using a standard algorithm used for BBANs >>>>> >>>>> With hyphens in place, this limits the intention of MifosX users to >>>>> provide extra financial services to their clients. >>>>> >>>>> >>>>> I need to be pointed to the repo so that i can make detailed comments >>>>> of my extensive reviews and tests on the GLIM / GSIM >>>>> >>>>> My Kind Regards >>>>> >>>>> Francis Guchie Kirago >>>>> *Skype:* francisguchie >>>>> *Whatsapp: *232 79 19 44 07 >>>>> *LINKEDIN:* >>>>> https://www.linkedin.com/in/francis-guchie-kirago-a4379617/ >>>>> twitter: @FrancisGuchie >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> On Sun, Apr 19, 2020 at 9:04 PM Nikhil Pawar <[email protected]> >>>>> wrote: >>>>> >>>>>> Thank you Francis- your inputs would be greatly appreciated. >>>>>> >>>>>> Rahul >>>>>> >>>>>> I went through the code and have already commented my feedback on the >>>>>> files. >>>>>> Here is the list for your reference according to the priority: >>>>>> >>>>>> Critical Changes: >>>>>> 1) Remove hyphens from child account numbers: I have explained in >>>>>> depth why we should do this in my feedback. >>>>>> 2) Revert persistence.xml change as suggested by Michael. >>>>>> 3) Add concurrency fix in GSIM. >>>>>> >>>>>> Non-critical changes: >>>>>> 4) Align GLIM account creation mechanism with GSIM( if possible). >>>>>> 5) Avoid appending sql params in sql string and instead pass them as >>>>>> object params. >>>>>> 6) Omit use of glimID and gsimID from commandProcessing result >>>>>> 7)Remove unwanted whitespaces and new lines in PR. >>>>>> 8) extract functions out of repetitive code in >>>>>> LoanApplicationWritePlatformServiceJpaRepositoryImpl >>>>>> and SavingsAccountWritePlatformServiceJpaRepositoryImpl >>>>>> >>>>>> >>>>>> I would request you to address above stuff based on the priority. >>>>>> This PR has a go ahead from me, if critical changes as addressed. >>>>>> >>>>>> If you need help, please feel to ping me and I would be happy to help >>>>>> you. >>>>>> >>>>>> Regards, >>>>>> Nikhil >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> On Sat, Apr 18, 2020 at 1:42 PM Francis Guchie < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Yes Nikhil, >>>>>>> >>>>>>> I will review >>>>>>> >>>>>>> My Kind Regards >>>>>>> >>>>>>> Francis Guchie Kirago >>>>>>> *Skype:* francisguchie >>>>>>> *Whatsapp: *232 79 19 44 07 >>>>>>> *LINKEDIN:* >>>>>>> https://www.linkedin.com/in/francis-guchie-kirago-a4379617/ >>>>>>> twitter: @FrancisGuchie >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Sat, Apr 18, 2020 at 5:12 PM Nikhil Pawar <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>>> ++ Francis >>>>>>>> If you have time could you please help in doing functional review. >>>>>>>> >>>>>>>> I will also go through the code and provide my feedback. But I >>>>>>>> would strongly recommend that we merge this PR , provided it does not >>>>>>>> break >>>>>>>> any existing functionality. Rebasing this huge chunk of code takes lot >>>>>>>> of >>>>>>>> effort and now that we have a rebased code - this opportunity should >>>>>>>> not be >>>>>>>> missed. >>>>>>>> >>>>>>>> Besides that, this feature also has front end support. The next >>>>>>>> task to be picked up would be to rebase front end code. Once we have >>>>>>>> front >>>>>>>> end support- it would be much easier for functional reviewers to test >>>>>>>> this >>>>>>>> feature. >>>>>>>> >>>>>>>> Regards, >>>>>>>> Nikhil >>>>>>>> >>>>>>>> On Sat, Apr 18, 2020 at 12:35 PM Michael Vorburger < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> Community, >>>>>>>>> >>>>>>>>> Could parties interested in >>>>>>>>> https://issues.apache.org/jira/browse/FINERACT-603 please help >>>>>>>>> with code reviewing https://github.com/apache/fineract/pull/738 ? >>>>>>>>> >>>>>>>>> If nobody cares enough to spend time to help doing reviews in the >>>>>>>>> next say 2 weeks, then I propose we'll just merge it.... >>>>>>>>> >>>>>>>>> Tx! >>>>>>>>> M. >>>>>>>>> _______________________ >>>>>>>>> Michael Vorburger >>>>>>>>> http://www.vorburger.ch >>>>>>>>> >>>>>>>>> >>>>>>>>> On Mon, Mar 23, 2020 at 2:24 AM Michael Vorburger < >>>>>>>>> [email protected]> wrote: >>>>>>>>> >>>>>>>>>> On Fri, Mar 20, 2020 at 5:28 AM RAHUL PAWAR < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >>>>>>>>>>> Hi Ed, >>>>>>>>>>> >>>>>>>>>>> Yes, the build is passing locally in machine. >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> That is very curious, I don't see how that's possible; you should >>>>>>>>>> see the same failures that Travis CI reports... >>>>>>>>>> >>>>>>>>>> ...do make sure that you rebase to the latest master, and run a >>>>>>>>>> full "./gradlew build" and you will reproduce the Checkstyle errors >>>>>>>>>> that >>>>>>>>>> Travis shows if you click on Details. >>>>>>>>>> >>>>>>>>>> I've also just commented on the >>>>>>>>>> https://github.com/apache/fineract/pull/738 with some more >>>>>>>>>> suggestions re. Checkstyle, for anyone interested to read. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> Thanks. >>>>>>>>>>> >>>>>>>>>>> On Fri, Mar 20, 2020 at 6:37 AM Ed Cable <[email protected]> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Wonderful news. We'll try to get it merged promptly. >>>>>>>>>>>> >>>>>>>>>>>> I'm adding @Dev <[email protected]> >>>>>>>>>>>> >>>>>>>>>>>> It's showing the Travis CI build is failing - the build is >>>>>>>>>>>> passing locally for you? >>>>>>>>>>>> >>>>>>>>>>>> On Thu, Mar 19, 2020 at 7:04 AM RAHUL PAWAR < >>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hi Ed, >>>>>>>>>>>>> >>>>>>>>>>>>> The Integration Test for glim_gsim has been completed and >>>>>>>>>>>>> along with it, also fixed some bugs regarding it. >>>>>>>>>>>>> >>>>>>>>>>>>> Pr: https://github.com/apache/fineract/pull/738 >>>>>>>>>>>>> >>>>>>>>>>>>> Thankyou. >>>>>>>>>>>>> >>>>>>>>>>>>> On Thu, Jan 2, 2020 at 8:22 AM Ed Cable <[email protected]> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Thank you for the update. >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Wed, Jan 1, 2020, 07:02 RAHUL PAWAR < >>>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Hi Ed, >>>>>>>>>>>>>>> I am still working on testcases, will be completed soon. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Wed, Nov 13, 2019 at 7:23 AM Nikhil Pawar < >>>>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I am going to take this up with Rahul this holidays, will >>>>>>>>>>>>>>>> keep you posted. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Tue, Nov 12, 2019 at 8:45 PM Ed Cable <[email protected]> >>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Hi Rahul and Nikhil, >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> I would love to get this work shipped to the community. >>>>>>>>>>>>>>>>> Did you get a chance to write the necessary test cases? It's >>>>>>>>>>>>>>>>> likely we'll >>>>>>>>>>>>>>>>> target 1.5 release for this. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> See https://github.com/apache/fineract/pull/527 >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> and https://issues.apache.org/jira/browse/FINERACT-603 >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Ed >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> *Ed Cable* >>>>>>>>>>>> President/CEO, Mifos Initiative >>>>>>>>>>>> [email protected] | Skype: edcable | Mobile: +1.484.477.8649 >>>>>>>>>>>> >>>>>>>>>>>> *Collectively Creating a World of 3 Billion Maries | * >>>>>>>>>>>> http://mifos.org <http://facebook.com/mifos> >>>>>>>>>>>> <http://www.twitter.com/mifos> >>>>>>>>>>>> >>>>>>>>>>>>
