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