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. > - > > If it’s indeed not working, that’s a separate, high-priority > discussion. > - > > 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. > - > > 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. > - > > 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 > > >
