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. >> - >> >> 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 >> >> >>
