Hi Adam, Thanks for being thorough!
I suggest you remove these “dead code” with a pull request. Regards, Adam > On Aug 3, 2026, at 2:17 PM, Ashhar Ahmad Khan <[email protected]> wrote: > > Hi Adam, thanks. I already checked this more carefully before assuming > anything. > > The office-to-product access restriction logic (FineractEntityAccessUtil, > used by Charges, Loan Products, and Savings Products) is very much alive, and > it reads from m_entity_to_entity_mapping via > FineractEntityAccessReadServiceImpl. That's the working part, and I won't > touch any of that, createEntityToEntityMapping, updateEntityToEntityMapping, > and deleteEntityToEntityMapping stay exactly as they are. > > Separately, the FineractEntityAccess domain object, its repository, and the > m_entity_to_entity_access table are only ever referenced from inside > FineractEntityAccessWriteServiceImpl, specifically the two dead methods, > createEntityAccess and addNewEntityAccess. I checked the schema for foreign > keys and indexes, sample data, all three tiers of tests, the client SDK, > OpenAPI spec, and JPA relationships from other entities, nothing references > that table or those two methods anywhere else. It's a genuinely separate, > unused table from the live mapping-based access control, so it looks safe to > remove alongside the two stubs. > > > On Mon, Aug 3, 2026 at 5:25 PM Ádám Sághy <[email protected] > <mailto:[email protected]>> wrote: >> Hi there, >> >> I’m not entirely sure what entity access functionality, but it looks some of >> the entity-to-entity functionalities appear to be implemented. >> >> I’m not sure if anyone is currently using it, but I wouldn’t touch the >> working parts. >> >> However, I’m all for removing the stub and todo parts. >> >> Regards, >> Adam >> >>> On Aug 3, 2026, at 1:45 PM, Ashhar Ahmad Khan <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> Hi Aleksandar, >>> >>> I checked FINERACT-2169 before replying. As far as I can tell, entityaccess >>> isn't part of that effort. It isn't one of the 43 subtasks, I couldn't find >>> any commit overlap, and the module itself hasn't been touched by that >>> migration. >>> >>> At the moment, I'm leaning towards removing the unused methods, along with >>> FineractEntityAccess, its repository, and the underlying table, since >>> nothing currently appears to depend on any of them. I'd still like to hear >>> from anyone who was around for the 2016 change. >>> >>> >>> On Sun, Aug 2, 2026 at 9:47 PM Aleksandar Vidakovic >>> <[email protected] <mailto:[email protected]>> wrote: >>>> ... anything that touches JsonCommand is eventually legacy... you should >>>> be watching this instead >>>> https://issues.apache.org/jira/browse/FINERACT-2169 ... and independently: >>>> if it was not touched in such a long time...: >>>> >>>> (a) is it still relevant >>>> (b) ... or not >>>> >>>> On Sun, Aug 2, 2026 at 4:48 PM Ashhar Ahmad Khan <[email protected] >>>> <mailto:[email protected]>> wrote: >>>>> Hi everyone, >>>>> >>>>> While going through the infrastructure/entityaccess module I found that >>>>> `FineractEntityAccessWriteServiceImpl` has a method, >>>>> `createEntityAccess(JsonCommand command)`, that has never had a body: >>>>> >>>>> ```java >>>>> @Override >>>>> public CommandProcessingResult createEntityAccess(JsonCommand command) { >>>>> // TODO Auto-generated method stub >>>>> return null; >>>>> } >>>>> ``` >>>>> >>>>> This traces back to the very first commit that created the file, >>>>> `9d11434ee7` ("Branch Specific Products and Charges - initial commit", >>>>> Binny G Sreevas, Dec 6 2014), where it was already a stub. It has never >>>>> been implemented since, on any branch. There is no API route, >>>>> `CommandWrapperBuilder` method, permission record, or command handler for >>>>> it anywhere in the codebase, and there never has been. >>>>> `FineractEntityApiResource` only exposes >>>>> `createMap`/`updateMap`/`delete`, which route to >>>>> `createEntityToEntityMapping`/`updateEntityToEntityMapping`/`deleteEntityToEntityMapping`, >>>>> not to `createEntityAccess`. As far as I can tell this method has been >>>>> unreachable from any entry point since it was written. >>>>> >>>>> The same file has a second method, `addNewEntityAccess(...)`, which was >>>>> implemented from the start and did have a caller: >>>>> `FineractEntityAccessUtil.checkConfigurationAndAddProductResrictionsForUserOffice(...)`. >>>>> That changed in commit `66f8d7ce43` ("Changing dependency of >>>>> m_entity_to_entity_access to m_entity_mapping and adding validation for >>>>> new loan application", June 9 2016), which rewrote that method to call >>>>> `fineractEntityToEntityMappingRepository.save(...)` directly instead, >>>>> bypassing the write service entirely. From that point on >>>>> `addNewEntityAccess` has had zero callers. >>>>> >>>>> So the current state is one class, still a live `@Service` bean, still >>>>> injected into three command handlers >>>>> (`CreateEntityToEntityMappingCommandHandler`, >>>>> `UpdateEntityToEntityMappingCommandHandler`, >>>>> `DeleteEntityToEntityMappingCommandHandler`) for the methods that are >>>>> actually used, sitting alongside two methods that have had no caller for >>>>> roughly ten and twelve years respectively. >>>>> >>>>> On dependency surface, I checked: >>>>> >>>>> - No reference to `createEntityAccess` or `addNewEntityAccess` in >>>>> `fineract-client` or `fineract-client-feign` >>>>> - No OpenAPI/swagger entry for `createEntityAccess` >>>>> - No test file references either method >>>>> - `m_entity_to_entity_access`, the table `addNewEntityAccess` used to >>>>> write to, has 0 rows in `load_sample_data.sql`, and the project's own >>>>> generated schema docs list it at 0 rows as well >>>>> - No frontend file in the repo references either method >>>>> >>>>> I don't have a strong sense of what the right next step is here. A few >>>>> options as I see it: >>>>> >>>>> (a) implement `createEntityAccess` and restore a caller for >>>>> `addNewEntityAccess`, if there was ever an intended use case for >>>>> entity-level (as opposed to entity-to-entity-mapping) access control that >>>>> just never got finished, >>>>> (b) remove both methods along with `FineractEntityAccess`, its >>>>> repository, and the underlying table, since nothing appears to depend on >>>>> any of it, >>>>> (c) leave it as is if anyone knows of a reason it's still there, e.g. >>>>> downstream forks or a planned use I haven't found, or >>>>> (d) something else. >>>>> >>>>> I don't have a preference yet between these and would like to hear from >>>>> the community, particularly if anyone was around for the 2016 change and >>>>> remembers why `addNewEntityAccess`'s only call site was rewritten instead >>>>> of removed along with it. >>>>> >>>>> Thanks, >>>>> Ashhar >>
