Hi Adam, Aleksandar,

Thanks for the feedback. I've opened a PR for this:
https://github.com/apache/fineract/pull/6217

Also created the Jira ticket:
https://issues.apache.org/jira/browse/FINERACT-2736

On Mon, Aug 3, 2026 at 5:50 PM Ádám Sághy <[email protected]> wrote:

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

Reply via email to