Kapil/Felix, Is it possible to show/write a description from a user/functional perspective?
Kapil Your description is useful but for the non functional requirement (refactoring). Regards El lun, 1 sept 2025 a las 21:53, Kapil Panchal (< kapil.panchal.developm...@gmail.com>) escribió: > The current Collection Sheet efforts started as an attempt to raise a pull > request to the Jira Ticket > https://issues.apache.org/jira/browse/FINERACT-2290, this ticket aims at > solving the technical debt > https://issues.apache.org/jira/browse/FINERACT-2169, In other words, it > replaces the existing (old/archaic) logic processing (CQRS, Serialization > etc) with the new and improved/latest solution which removes much of the > boilerplate and aims at making the code lean such that all the testing and > related infrastructure needed to manage the quality of the code and > deliverables are minimum, it does that by leveraging the framework and the > supported libraries itself. The framework is fully tested and baselined, if > the code that we write falls inline with framework coding/logic paradigm > then most of the testing need not be done as it has already been tested, > this is a lot akin to testing the database with ACID. None of our code > tests the database/framework/webserver/libraries as they are fully tested > and these are the first principles of a good software design, minimal code > maximal output. > > With the above in mind and the instructions in the card itself which > assumes that the command processing logic is to be changed and not the > business logic itself. > > Cutting the story short, I have attached herewith a UML Class diagram to > elicit why the existing CollectionSheet is non functional. The API accepts > 2 parameters as stated below, > > 1) GenerateCollectionSheet, > 2) SaveCollectionSheet. > > Note1: The first parameter 1) Generate Collection Sheet makes a call to > the Service which inturn has a JDBC logic where mysql reserved keywords are > used, this always returns an exception. (This could have been a recent > mysql release and previously this query might have been working? I am using > the mysql version as stated in the .md documentation). > > Note2: The second parameter 2) A CQRS command that is audited, has three > private methods a) updateBulkRepayments, b) updateBulkDisbursals, c) > updateBulkMandatorySavingsDuePayments have errors in extracting the json > payload one of the methods is mentioned in the UML. There are other errors > too that make this Collection Sheet take code paths which do not reflect > the true metric. > > [image: Model_CollectionSheet_v1_Class_Diagram.PNG] > An excerpt from the code: The bold code is where the extraction of JSON > objects if not done correctly will bypass most of the logic and return > flawed response objects. Here it is the case. Please refer to the chain > emails where the correct logic to extract a variable from the json payload > is highlighted. > > public Collection<SavingsAccountTransactionDTO> > assembleBulkMandatorySavingsAccountTransactionDTOs(final JsonCommand > command, > > final PaymentDetail paymentDetail) { > final String json = command.json(); > if (StringUtils.isBlank(json)) { > throw new InvalidJsonException(); > } > final JsonElement element = this.fromApiJsonHelper.parse(json); > final Collection<SavingsAccountTransactionDTO> > savingsAccountTransactions = new ArrayList<>(); > final LocalDate transactionDate = > this.fromApiJsonHelper.extractLocalDateNamed(transactionDateParamName, > element); > final String dateFormat = > this.fromApiJsonHelper.extractDateFormatParameter(element.getAsJsonObject()); > final JsonObject topLevelJsonElement = element.getAsJsonObject(); > final Locale locale = > this.fromApiJsonHelper.extractLocaleParameter(topLevelJsonElement); > final DateTimeFormatter formatter = > DateTimeFormatter.ofPattern(dateFormat).withLocale(locale); > > > > > > > * if (element.isJsonObject()) { if > (topLevelJsonElement.has(bulkSavingsDueTransactionsParamName) > && > topLevelJsonElement.get(bulkSavingsDueTransactionsParamName).isJsonArray()) > { final JsonArray array = > topLevelJsonElement.get(bulkSavingsDueTransactionsParamName).getAsJsonArray(); > for (int i = 0; i < array.size(); i++) {* > > > "If you can raise a JIRA for each of the logical errors (and ideally > provide add a test case into the automated tests to demonstrate each), then > let's first fix these errors. After the API works as expected, we can then > proceed to refactor it." > > > I think this effort can be broken down to 3-4 Jira tickets for it to be > fully functional. Assuming that we will be using the same command > processing logic as we have been using and not the latest one as per > https://issues.apache.org/jira/browse/FINERACT-2169. > > 1) Jira Ticket 1 - Work on getting the Generate Collection Sheet up and > running, including unit tests and integration tests. > 2) Jira Ticket 2 - Work on getting the Save Collection Sheet up and > running, excluding unit tests and/or integration tests (some refactoring). > 3) Jira Ticket 3 - Work on getting the Save Collection Sheet unit tests. > 4) Jira Ticket 4 - Work on getting the Save Collection Sheet integration > tests. > > The release to production of tickets 2 - 4 can be done when all are > approved? Please let me know so that I can start working on those ASAP. > > Thanks, > Kapil > > On Tue, Sep 2, 2025 at 4:48 AM Petri Tuomola <petri.tuom...@gmail.com> > wrote: > >> Thanks Kapil - but this JIRA / PRs seem to be about refactoring the >> Collection Sheet API to the new command processing pattern, not about >> fixing defects in the API. >> >> Before we start any refactoring, we should make sure we're starting from >> a known good state and that there's sufficient unit testing coverage to >> test the functionality, so we know if we've broken something during >> refactoring. >> >> To get to this point, can you share what your comment "The *Collection >> Sheet API* in its current state appears non-functional and contains >> several logical errors" refers to? >> >> If you can raise a JIRA for each of the logical errors (and ideally >> provide add a test case into the automated tests to demonstrate each), then >> let's first fix these errors. After the API works as expected, we can then >> proceed to refactor it. >> >> Regards >> Petri >> >> >> >> ------------------------------ >> *From:* Kapil Panchal <kapil.panchal.developm...@gmail.com> >> *Sent:* Monday, September 1, 2025 7:42 PM >> *To:* dev@fineract.apache.org <dev@fineract.apache.org> >> *Cc:* Edward Cable <edca...@mifos.org> >> *Subject:* Re: Collection Sheet Deprecation was [Re: Questions and >> Observations on FINERACT-2290 (Collection Sheet API Refactor)] >> >> Hi Petri, >> >> Please take a look at the Jira Ticket >> https://issues.apache.org/jira/browse/FINERACT-2290. I had released a >> Pull Request https://github.com/apache/fineract/pull/4943. >> >> Thanks, >> Kapil >> >> On Mon, Sep 1, 2025 at 7:27 AM Petri Tuomola <petri.tuom...@gmail.com> >> wrote: >> >> Hi >> >> I'm happy to help fix issues with Collection Sheet API. >> >> If I look at the emails that started all this, I can only see this >> comment: >> >> "The *Collection Sheet API* in its current state appears non-functional >> and contains several logical errors" >> >> >> I think it's fair to expect a bit more detailed bug report than that. >> >> Is there any more details on this issue: how is it non-functional, and >> what are the logical errors? >> >> >> Who can provide the list of JIRAs that need to be fixed? I can then have >> a look this weekend. >> >> >> Regards >> >> Petri >> >> >> >> ------------------------------ >> *From:* James Dailey <jdai...@apache.org> >> *Sent:* Saturday, August 30, 2025 3:01 AM >> *To:* dev@fineract.apache.org <dev@fineract.apache.org>; Edward Cable < >> edca...@mifos.org> >> *Subject:* Re: Collection Sheet Deprecation was [Re: Questions and >> Observations on FINERACT-2290 (Collection Sheet API Refactor)] >> >> Devs - Are we done with this? >> >> First, I am NOT ruling out this functionality as important but if no one >> is actually demanding it, and no one is willing to maintain it, then it >> doesn't get maintained. That is the nature of open source projects. >> >> Second, it can happen that an outside vendor (e.g. Mifos) that relies >> on this Collection Sheet functionality (or any functionality) inadvertently >> drops that from their front end release. Because the tests at Fineract do >> not cover this fully, no one at the Vendor and no one at the Fineract >> project will see the functionality fail in a build until a customer or >> implementer notices. So, tests are vital for ensuring ongoing >> maintainability. >> >> Third, if this type of thing isn't "naturally" part of the core - then it >> will make a lot more sense to have it be an outside extension - in which >> case the tests have to be written for the API calls, and a Vendor should >> try to get such tests contributed. I could be wrong about this, I would >> be happy to debate where this belongs. >> >> @Edward Cable <edca...@mifos.org> - what is the assessment and status of >> needed functionality and where do you think it should live? >> >> Thanks, >> James >> >> On Mon, Aug 18, 2025 at 8:21 AM James Dailey <jdai...@apache.org> wrote: >> >> Sifiso - >> >> Do we have a way to reach the tens of thousands of institutions you >> believe are there? >> >> My belief (just anecdotal information) is that it’s more like a few >> hundred institutions and that out of that number, less than 10% are on a >> recent release. And, of that smaller number, few, if any, are still using >> Collection Sheets. I could be wrong - I’d like the data! I’d say that >> vendors can show up with their data here to help make this vendor neutral >> space more informed. >> >> If group lending and group-member lending and collection sheets are still >> needed, then perhaps those project volunteers can contribute the needed >> updates to keep the functionality useful. >> >> Thanks >> Thanks >> >> >> On Mon, Aug 18, 2025 at 6:48 AM Sifiso Mtetwa <sif...@skyburgsystems.org> >> wrote: >> >> Hi guys, >> >> >> >> This is an interesting topic. I have been wondering why, in general, we >> seem to be deprecating more and more system functions. The individual >> collection sheet has served us well over the years and still does, If >> anything we could improve on its functionality by maybe adding a bulk >> collection sheet template with little detail compared to a full loan >> repayment bulk import template. >> >> >> >> Fineract is used by tens of thousands of organisations throughout the >> world and most of them are not on this listing and may not have a voice to >> air their concerns. Maybe we can find a way of exposing this thread to >> include more voters. >> >> >> >> Regards, >> >> >> >> *From:* James Dailey [mailto:jdai...@apache.org] >> *Sent:* Saturday, 16 August 2025 02:53 >> *To:* dev@fineract.apache.org >> *Subject:* Re: Collection Sheet Deprecation was [Re: Questions and >> Observations on FINERACT-2290 (Collection Sheet API Refactor)] >> >> >> >> Ed >> >> >> >> I agree this needs to be discussed but it is important to acknowledge >> that THIS Fineract listserv is the only official (and required) discussion >> space. It is not the intention to bury anything in “an email thread”. >> >> >> >> When I started up Mifos we spent a lot of time looking at Collection >> Sheets and designing process flows around them. I fully know that this >> design direction was important back then in 2002-2006. >> >> >> >> However if there is no one here asking for them to be retained besides >> you, then that is a sign that they have perhaps reached an end of their >> utility. Or, that the users are not actually here, which is a different >> problem. >> >> >> >> So,… If there is a group using collection sheets in production AND they >> are not on some permanent forked (old) version, then now is the time to >> speak up. Here. >> >> >> >> Generally, I’m fairly certain we can refactor this with an eye toward >> extracting it from the core. Repeating the logic in two places makes no >> sense either. Collection sheets are kind of assembled from >> constituent loans and savings. The balances and due payments should be >> calculated in the underlying components. >> >> >> >> As the system gets restructured we need to decide to keep this at all, to >> keep it in a new place, or as some external concept/plug in. Why wouldn’t >> we want a separate component? >> >> >> >> Cheers >> >> >> >> On Thu, Aug 14, 2025 at 4:16 PM Ed Cable <edca...@mifos.org> wrote: >> >> I'll leave the other thread for discussion of the API versioning and >> refactoring related to Collection Sheet API but wanted to create a separate >> thread regarding the deprecation of the Collection Sheet. >> >> >> >> In general, for this and removal of any functionality, it's something >> that needs to be discussed openly with the community and with a vote and >> not a decision buried in a mailing list thread. >> >> >> >> For the collection sheet specifically, more thought has to be given to >> its deprecation as the centrality and highly coupled nature of the >> collection sheet is being understated as it isn't merely a report that's to >> be printed or a form filled out via a mobile application. It's a >> significant portion of the user interface and highly coupled to many of the >> microfinance features around groups/centers/meeting scheduling, staff >> assignment, etc. >> >> >> >> I do agree that from a UI perspective, the collection sheet and other >> microfinance-centric functionalities and flows should be viewable based on >> a configurable setting. As it doesn't lend itself to the optimal user >> experience for a large portion of current Fineract user base. >> >> >> >> I also am supportive of a strategy of slimming Fineract to its core >> services and functionality above core Fineract services and APIs can be >> extracted out. >> >> >> >> So I do think we should give thoughtful consideration to what abstracting >> out the collection sheet and corresponding microfinance functionality would >> look like and what that effort would entail to abstract it out without >> adversely impacting the original user base of the software but it's not as >> simple as deprecating these API. >> >> >> >> I welcome others' thoughts and inputs as I know even with microfinance >> itself, the methodology has evolved and group lending and the concept of a >> collection sheet isn't as central as it once was >> >> >> >> Thanks, >> >> >> >> Ed >> >> >> >> >> >> >> >> On Wed, Aug 13, 2025 at 6:36 PM Kapil Panchal < >> kapil.panchal.developm...@gmail.com> wrote: >> >> Hi James, >> >> >> >> I can proceed by marking this feature as @Deprecated and/or performing a >> safe refactor to remove the API. >> >> >> >> For API versioning I agree to Aleksandar Vidakovic's proposal on adapting >> to the SpringBoot v7/Spring Framework v4. If I may, Aleksandar >> Vidakovic takes the lead on this project and I can help to support the >> conversion? >> >> >> >> Thanks, >> >> Kapil >> >> >> >> On Thu, Aug 14, 2025 at 5:07 AM James Dailey <jdai...@apache.org> wrote: >> >> Hi Kapil >> >> >> >> I might suggest looking at this as an opportunity to remove the >> collection sheet entirely from the Fineract namespace. It’s a legacy >> concept I and others designed a long time ago, originally in 2002 based on >> collection sheets we gathered from a dozen countries. It is strongly tied >> to concepts in microfinance field operations, and especially when there was >> no data connectivity. >> >> >> >> It belongs perhaps as a sort of external microservice - data loading via >> a bulk import could still be enabled. >> >> The API versioning is a good idea but needs to be more holistic across >> the platform I think. >> >> >> >> >> >> >> >> On Mon, Aug 11, 2025 at 4:43 AM Ádám Sághy <adamsa...@gmail.com> wrote: >> >> Hi Kapil, >> >> Thank you for raising the concerns below. I’ll need some additional >> details to fully understand your points: >> >> 1. *Collection Sheet API* – You mentioned it appears non-functional >> and contains several logical errors. >> >> o If it’s indeed not working, that’s a separate, high-priority >> discussion. >> >> o Could you clarify which logical errors you were referring to, and >> what specifically makes you think it’s non-functional? >> >> 2. *Service annotations* – You noted that service methods are not >> annotated with @Service and that beans are defined manually. >> >> o Are you referring to the >> CollectionSheetWritePlatformServiceJpaRepositoryImpl bean being defined >> via configuration? >> >> 3. *Repository wrappers annotated with **@Service* – You mentioned >> that this mandates full unit test coverage but that they should ideally be >> annotated with @Component. >> >> o Could you point out the exact classes you had in mind? >> >> As for the other points, I agree we can refactor and remove redundant >> logic—please feel free to suggest specific improvements or start work on >> them immediately! >> >> However, be careful by moving anything into the fineract-core… We are >> aiming to keep it as small as possible as everything is built on top of >> this module! If collection sheet are used for loans and savings - for >> example - than the recommended move is NOT to move this logic into core! >> >> Either: >> >> - we split the logic into fineract-loan and fineract-savings >> >> - Move the logic into a new module >> >> - Leave it in fineract-provider for now >> >> >> >> Shall you have any questions, please let us know! >> >> Regards, >> Adam >> >> >> >> >> >> On 2025. Aug 11., at 12:09, Kapil Panchal < >> kapil.panchal.developm...@gmail.com> wrote: >> >> >> >> Hi Adam, >> >> I’m currently working on *FINERACT-2290* and have a few questions before >> I submit a pull request. >> >> The *Collection Sheet API* in its current state appears non-functional >> and contains several logical errors. It seems there was an earlier attempt >> to convert from a JSON string request parameter to a class-based request >> object, but: >> >> Certain fields are missing. >> >> The serializer is not correctly populating the objects, which causes the >> conditional checks to be bypassed and results in incorrect (false) >> responses. >> >> This change set is *high risk* because it touches most of the loan and >> savings product logic. I’ve had to refactor almost all major methods. >> Extensive integration and end-to-end testing will be required to ensure >> there are no regressions, especially in edge cases. At present, there are >> no unit or integration tests for this functionality, and test creation is >> outside the current ticket scope. I’ve been iterating on this for a while, >> and only today have I reached a stable state after several experimental and >> build-breaking attempts. >> >> *Key Observations:* >> >> Service methods are not annotated with @Service; instead, beans are >> defined manually. >> >> Repository wrappers are annotated with @Service. This mandates full unit >> test coverage for these methods, but they should ideally be annotated with >> @Component. >> >> I agree with prior discussions on separating bean validation — having a >> dedicated @Component validation class allows the request object to handle >> checks independent of database queries. >> >> Validation components can also perform database-related validations; >> these can be injected into service classes for cleaner architecture. >> >> Such validation components should be placed in *Fineract-Core* so they >> are reusable across modules, reducing future refactoring needs. >> >> The current design of having commands in *Fineract-Core* and >> handlers/services/repositories in respective modules is good — it cleanly >> decouples command definition from execution. >> >> There is extensive use of this. in singleton contexts (API, Service, >> Repository). While not harmful, it’s unnecessary boilerplate. >> >> Multiple redundant intermediate DTOs exist where the request DTO itself >> could be reused for data transfer. >> >> I found redundant logic — e.g., a for loop with a break statement that >> effectively executes only once; this can be simplified. >> >> Some JDBC template queries use reserved SQL keywords, causing exceptions. >> Refactoring these queries resolves the issue and returns proper response >> objects. >> >> *Suggestions:* >> >> *Where appropriate, large tickets should be broken into subtasks to >> manage complexity and reviewability.* >> >> It may help to have a dedicated *developer-only Slack channel *for >> technical discussions. This could complement other community spaces if >> there’s a need to keep certain conversations more focused. >> >> What are your thoughts on the above? >> >> Thanks, >> Kapil >> >> >> >> >> >> >> -- >> >> *Ed Cable* >> >> President/CEO, Mifos Initiative >> >> edca...@mifos.org | 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>* >> >> >> >>
