Skip to content

FINERACT-1932: Fineract modularization #5562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nidhiii128 wants to merge 1 commit into from
Closed

FINERACT-1932: Fineract modularization #5562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fineract-core/src/main/java/org/apache/fineract/commands/service/CommandWrapperBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Locale;
import java.util.Set;
import org.apache.fineract.commands.domain.CommandWrapper;
import org.apache.fineract.infrastructure.accountnumberformat.service.AccountNumberFormatConstants;
Expand Down Expand Up @@ -3113,7 +3114,7 @@ public CommandWrapperBuilder updateAccount(String accountType, final Long accoun

public CommandWrapperBuilder createProductCommand(String productType, String command, final Long productId) {
this.entityName = productType.toUpperCase() + "PRODUCT";
this.actionName = "CREATE" + "_" + command.toUpperCase();
this.actionName = "CREATE" + "_" + command.toUpperCase(Locale.ROOT);
this.entityId = productId;
this.href = "/products/" + productType + "/" + productId + "?command=" + command;
return this;
Expand Down
61 changes: 61 additions & 0 deletions fineract-core/src/main/java/org/apache/fineract/organisation/staff/domain/Staff.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,67 @@ public static Staff fromJson(final Office staffOffice, final JsonCommand command
return new Staff(staffOffice, firstname, lastname, externalId, mobileNo, isLoanOfficer, isActive, joiningDate);
}

public static Staff fromRequest(final Office staffOffice, final StaffRequest request) {
Copy link
Copy Markdown
Contributor

@vidakovic vidakovic Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more a comment to us as a community and less for you @nidhiii128 ... we have functions like this all over the place, but I have some doubts about the usefulness of them in general. I don't see where this is actually used and/or where it would be useful. Do we really optimize here anything? Isn't it just easier to return the whole updated object if we really have to (I wouldn't... if the client needs that information really from the backend then it should just call the "read" endpoint). The client initiates the change... if a http 200 is received then it "knows" that the changes are ok, if 4xx or 5xx then the updates didn't succeed... all needed information is already available on the client side.

But let's assume this is really necessary and I'm missing something essential here... then I would really urge to use a library that does diffing way better than any of our handwritten (boilerplate) code: https://javers.org

Copy link
Copy Markdown
Contributor Author

@nidhiii128 nidhiii128 Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @vidakovic , I completely agree with your point. The manual boilerplate we have for these updates is definitely not ideal and makes the code harder to maintain. Using a library like Javers would be a much cleaner and more professional way to handle object diffing across Fineract. I've followed the current fromRequest pattern here just to stay consistent with the existing modules for now, but I'm definitely in favor of the community moving toward a better solution like the one you suggested.

Copy link
Copy Markdown
Contributor

@vidakovic vidakovic Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good @nidhiii128 your approach makes sense to get this PR approved; introducing new libs/dependencies "on the fly" without at least a Jira ticket usually doesn't get a PR approved. Will start the build so that we can see where we are with this.

final String firstname = request.getFirstname();
final String lastname = request.getLastname();
final String externalId = request.getExternalId();
final String mobileNo = request.getMobileNo();
final boolean isLoanOfficer = request.getIsLoanOfficer() != null ? request.getIsLoanOfficer() : false;
final Boolean isActive = request.getIsActive();

LocalDate joiningDate = null;
return new Staff(staffOffice, firstname, lastname, externalId, mobileNo, isLoanOfficer, isActive, joiningDate);
}

public Map<String, Object> update(final StaffRequest request) {
final Map<String, Object> actualChanges = new LinkedHashMap<>(7);

if (request.getOfficeId() != null && !request.getOfficeId().equals(this.office.getId())) {
actualChanges.put("officeId", request.getOfficeId());
}

boolean firstnameChanged = false;
if (request.getFirstname() != null && !request.getFirstname().equals(this.firstname)) {
actualChanges.put("firstname", request.getFirstname());
this.firstname = request.getFirstname();
firstnameChanged = true;
}

boolean lastnameChanged = false;
if (request.getLastname() != null && !request.getLastname().equals(this.lastname)) {
actualChanges.put("lastname", request.getLastname());
this.lastname = request.getLastname();
lastnameChanged = true;
}

// Fixed: Combined the flags you created above
Copy link
Copy Markdown
Contributor

@airajena airajena Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel such comments should be avoided.

if (firstnameChanged || lastnameChanged) {
deriveDisplayName(this.firstname);
}

if (request.getExternalId() != null && !request.getExternalId().equals(this.externalId)) {
actualChanges.put("externalId", request.getExternalId());
this.externalId = request.getExternalId();
}

if (request.getMobileNo() != null && !request.getMobileNo().equals(this.mobileNo)) {
actualChanges.put("mobileNo", request.getMobileNo());
this.mobileNo = StringUtils.defaultIfEmpty(request.getMobileNo(), null);
}

if (request.getIsLoanOfficer() != null && request.getIsLoanOfficer() != this.loanOfficer) {
actualChanges.put("isLoanOfficer", request.getIsLoanOfficer());
this.loanOfficer = request.getIsLoanOfficer();
}

if (request.getIsActive() != null && request.getIsActive() != this.active) {
actualChanges.put("isActive", request.getIsActive());
this.active = request.getIsActive();
}

return actualChanges;
}

protected Staff() {
//
}
Expand Down
23 changes: 8 additions & 15 deletions ...organisation/staff/data/StaffRequest.java → ...ganisation/staff/domain/StaffRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,43 +16,36 @@
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.fineract.organisation.staff.data;
package org.apache.fineract.organisation.staff.domain;

import io.swagger.v3.oas.annotations.media.Schema;
import com.fasterxml.jackson.annotation.JsonFormat;
import java.io.Serial;
import java.io.Serializable;
import java.time.LocalDate;
import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.NoArgsConstructor;
import lombok.experimental.FieldNameConstants;

@Data
@NoArgsConstructor
@AllArgsConstructor
@FieldNameConstants
public class StaffRequest implements Serializable {

@Serial
private static final long serialVersionUID = 1L;

@Schema(example = "1")
private Long officeId;
@Schema(example = "John")
private String firstname;
@Schema(example = "Doe")
private String lastname;
@Schema(example = "true")
private Boolean isLoanOfficer;
@Schema(example = "17H")
private String externalId;
@Schema(example = "+353851239876")
private String mobileNo;
@Schema(example = "true")
private Boolean isLoanOfficer;
private Boolean isActive;
@Schema(example = "01 January 2009")
private String joiningDate;
@Schema(example = "en")
@JsonFormat(pattern = "dd MMMM yyyy", locale = "en")
private LocalDate joiningDate;
private String locale;
@Schema(example = "dd MMMM yyyy")
private String dateFormat;
@Schema(example = "true")
private Boolean forceStatus;
}
3 changes: 2 additions & 1 deletion ...ract-core/src/main/java/org/apache/fineract/portfolio/client/domain/ClientIdentifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import jakarta.persistence.Table;
import jakarta.persistence.UniqueConstraint;
import java.util.LinkedHashMap;
import java.util.Locale;
import java.util.Map;
import org.apache.commons.lang3.StringUtils;
import org.apache.fineract.infrastructure.codes.domain.CodeValue;
Expand Down Expand Up @@ -74,7 +75,7 @@ private ClientIdentifier(final Client client, final CodeValue documentType, fina
this.documentType = documentType;
this.documentKey = StringUtils.defaultIfEmpty(documentKey, null);
this.description = StringUtils.defaultIfEmpty(description, null);
ClientIdentifierStatus statusEnum = ClientIdentifierStatus.valueOf(statusName.toUpperCase());
ClientIdentifierStatus statusEnum = ClientIdentifierStatus.valueOf(statusName.toUpperCase(Locale.ROOT));
this.active = null;
if (statusEnum.isActive()) {
this.active = statusEnum.getValue();
Expand Down
2 changes: 1 addition & 1 deletion ...t-provider/src/main/java/org/apache/fineract/organisation/staff/api/StaffApiResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
import org.apache.fineract.organisation.office.data.OfficeData;
import org.apache.fineract.organisation.office.service.OfficeReadPlatformService;
import org.apache.fineract.organisation.staff.data.StaffData;
import org.apache.fineract.organisation.staff.data.StaffRequest;
import org.apache.fineract.organisation.staff.domain.StaffRequest;
import org.apache.fineract.organisation.staff.service.StaffReadPlatformService;
import org.glassfish.jersey.media.multipart.FormDataContentDisposition;
import org.glassfish.jersey.media.multipart.FormDataParam;
Expand Down
15 changes: 4 additions & 11 deletions ...c/main/java/org/apache/fineract/organisation/staff/handler/CreateStaffCommandHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,23 @@
*/
package org.apache.fineract.organisation.staff.handler;

import lombok.RequiredArgsConstructor;
import org.apache.fineract.commands.annotation.CommandType;
import org.apache.fineract.commands.handler.NewCommandSourceHandler;
import org.apache.fineract.infrastructure.core.api.JsonCommand;
import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
import org.apache.fineract.organisation.staff.service.StaffWritePlatformService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.stereotype.Component;

@Service
@Component
@CommandType(entity = "STAFF", action = "CREATE")
@RequiredArgsConstructor
public class CreateStaffCommandHandler implements NewCommandSourceHandler {

private final StaffWritePlatformService writePlatformService;

@Autowired
public CreateStaffCommandHandler(final StaffWritePlatformService writePlatformService) {
this.writePlatformService = writePlatformService;
}

@Transactional
@Override
public CommandProcessingResult processCommand(final JsonCommand command) {

return this.writePlatformService.createStaff(command);
}
}
24 changes: 24 additions & 0 deletions ...apache/fineract/organisation/staff/serialization/StaffCommandFromApiJsonDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.fineract.infrastructure.core.exception.InvalidJsonException;
import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper;
import org.apache.fineract.organisation.staff.domain.StaffRequest;
import org.apache.fineract.organisation.staff.service.StaffReadPlatformService;
import org.apache.fineract.portfolio.client.api.ClientApiConstants;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -224,4 +225,27 @@ private void throwExceptionIfValidationWarningsExist(final List<ApiParameterErro
dataValidationErrors);
}
}

public StaffRequest commandFromApiJson(final String json) {
if (StringUtils.isBlank(json)) {
throw new InvalidJsonException();
}

final JsonElement element = this.fromApiJsonHelper.parse(json);

final Long officeId = this.fromApiJsonHelper.extractLongNamed(OFFICE_ID, element);
final String firstname = this.fromApiJsonHelper.extractStringNamed(FIRSTNAME, element);
final String lastname = this.fromApiJsonHelper.extractStringNamed(LASTNAME, element);
final String externalId = this.fromApiJsonHelper.extractStringNamed(EXTERNAL_ID, element);
final String mobileNo = this.fromApiJsonHelper.extractStringNamed(MOBILE_NO, element);
final Boolean isLoanOfficer = this.fromApiJsonHelper.extractBooleanNamed(IS_LOAN_OFFICER, element);
final Boolean isActive = this.fromApiJsonHelper.extractBooleanNamed(IS_ACTIVE, element);
final LocalDate joiningDate = this.fromApiJsonHelper.extractLocalDateNamed(JOINING_DATE, element);
final String locale = this.fromApiJsonHelper.extractStringNamed(LOCALE, element);
final String dateFormat = this.fromApiJsonHelper.extractStringNamed(DATE_FORMAT, element);
final Boolean forceStatus = this.fromApiJsonHelper.extractBooleanNamed(FORCE_STATUS, element);

return new StaffRequest(officeId, firstname, lastname, externalId, mobileNo, isLoanOfficer, isActive, joiningDate, locale,
dateFormat, forceStatus);
}
}
83 changes: 34 additions & 49 deletions ...pache/fineract/organisation/staff/service/StaffWritePlatformServiceJpaRepositoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@
*/
package org.apache.fineract.organisation.staff.service;

import jakarta.persistence.PersistenceException;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.fineract.infrastructure.core.api.JsonCommand;
import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
import org.apache.fineract.infrastructure.core.data.CommandProcessingResultBuilder;
Expand All @@ -32,101 +30,88 @@
import org.apache.fineract.organisation.office.domain.Office;
import org.apache.fineract.organisation.office.domain.OfficeRepositoryWrapper;
import org.apache.fineract.organisation.staff.domain.Staff;
import org.apache.fineract.organisation.staff.domain.StaffRepository;
import org.apache.fineract.organisation.staff.exception.StaffNotFoundException;
import org.apache.fineract.organisation.staff.domain.StaffRepositoryWrapper;
import org.apache.fineract.organisation.staff.domain.StaffRequest;
import org.apache.fineract.organisation.staff.serialization.StaffCommandFromApiJsonDeserializer;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.orm.jpa.JpaSystemException;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Slf4j
@Service
@RequiredArgsConstructor
public class StaffWritePlatformServiceJpaRepositoryImpl implements StaffWritePlatformService {

private final StaffCommandFromApiJsonDeserializer fromApiJsonDeserializer;
private final StaffRepository staffRepository;
private final StaffRepositoryWrapper staffRepositoryWrapper;
private final OfficeRepositoryWrapper officeRepositoryWrapper;
private final StaffCommandFromApiJsonDeserializer fromApiJsonDeserializer;

@Transactional
@Override
public CommandProcessingResult createStaff(final JsonCommand command) {

final StaffRequest request = this.fromApiJsonDeserializer.commandFromApiJson(command.json());
try {
this.fromApiJsonDeserializer.validateForCreate(command.json());

final Long officeId = command.longValueOfParameterNamed("officeId");

final Office staffOffice = this.officeRepositoryWrapper.findOneWithNotFoundDetection(officeId);
final Staff staff = Staff.fromJson(staffOffice, command);
final Office staffOffice = this.officeRepositoryWrapper.findOneWithNotFoundDetection(request.getOfficeId());
final Staff staff = Staff.fromRequest(staffOffice, request);
this.staffRepositoryWrapper.save(staff);

this.staffRepository.saveAndFlush(staff);
return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withEntityId(staff.getId())
.withOfficeId(staff.officeId()).build();

return new CommandProcessingResultBuilder() //
.withCommandId(command.commandId()) //
.withEntityId(staff.getId()).withOfficeId(officeId) //
.build();
} catch (final JpaSystemException | DataIntegrityViolationException dve) {
handleStaffDataIntegrityIssues(command, dve.getMostSpecificCause(), dve);
return CommandProcessingResult.empty();
} catch (final PersistenceException dve) {
Throwable throwable = ExceptionUtils.getRootCause(dve.getCause());
handleStaffDataIntegrityIssues(command, throwable, dve);
handleStaffDataIntegrityIssues(request, dve.getMostSpecificCause(), dve);
return CommandProcessingResult.empty();
}
}

@Transactional
@Override
public CommandProcessingResult updateStaff(final Long staffId, final JsonCommand command) {

final StaffRequest request = this.fromApiJsonDeserializer.commandFromApiJson(command.json()); // We move the
// 'try' here so
// it covers all
// your logic
// below
try {
this.fromApiJsonDeserializer.validateForUpdate(command.json(), staffId);

final Staff staffForUpdate = this.staffRepository.findById(staffId).orElseThrow(() -> new StaffNotFoundException(staffId));
final Map<String, Object> changesOnly = staffForUpdate.update(command);
final Staff staffForUpdate = this.staffRepositoryWrapper.findOneWithNotFoundDetection(staffId);
final Map<String, Object> changes = staffForUpdate.update(request);

if (changesOnly.containsKey("officeId")) {
final Long officeId = (Long) changesOnly.get("officeId");
final Office newOffice = this.officeRepositoryWrapper.findOneWithNotFoundDetection(officeId);
if (changes.containsKey("officeId")) {
final Office newOffice = this.officeRepositoryWrapper.findOneWithNotFoundDetection(request.getOfficeId());
staffForUpdate.changeOffice(newOffice);
}

if (!changesOnly.isEmpty()) {
this.staffRepository.saveAndFlush(staffForUpdate);
if (!changes.isEmpty()) {
this.staffRepositoryWrapper.save(staffForUpdate);
}

return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withEntityId(staffId)
.withOfficeId(staffForUpdate.officeId()).with(changesOnly).build();
.withOfficeId(staffForUpdate.officeId()).with(changes).build();

} catch (final JpaSystemException | DataIntegrityViolationException dve) {
handleStaffDataIntegrityIssues(command, dve.getMostSpecificCause(), dve);
return CommandProcessingResult.empty();
} catch (final PersistenceException dve) {
Throwable throwable = ExceptionUtils.getRootCause(dve.getCause());
handleStaffDataIntegrityIssues(command, throwable, dve);
handleStaffDataIntegrityIssues(request, dve.getMostSpecificCause(), dve);
return CommandProcessingResult.empty();
}
}

/*
* Guaranteed to throw an exception no matter what the data integrity issue is.
*/
private void handleStaffDataIntegrityIssues(final JsonCommand command, final Throwable realCause, final Exception dve) {
private void handleStaffDataIntegrityIssues(final StaffRequest request, final Throwable realCause, final Exception dve) {
if (realCause.getMessage().contains("external_id")) {
final String externalId = command.stringValueOfParameterNamed("externalId");
final String externalId = request.getExternalId();
throw new PlatformDataIntegrityException("error.msg.staff.duplicate.externalId",
"Staff with externalId `" + externalId + "` already exists", "externalId", externalId);
} else if (realCause.getMessage().contains("display_name")) {
final String lastname = command.stringValueOfParameterNamed("lastname");
// FIX: Use 'request' getters, not 'command'
Copy link
Copy Markdown
Contributor

@airajena airajena Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comments belongs to PR description, we should keep code clean.

final String lastname = request.getLastname();
String displayName = lastname;
if (!StringUtils.isBlank(displayName)) {
final String firstname = command.stringValueOfParameterNamed("firstname");
displayName = lastname + ", " + firstname;
if (StringUtils.isNotBlank(request.getFirstname())) {
displayName = lastname + ", " + request.getFirstname();
}
throw new PlatformDataIntegrityException("error.msg.staff.duplicate.displayName",
"A staff with the given display name '" + displayName + "' already exists", "displayName", displayName);
}

log.error("Error occured.", dve);
log.error("Error occurred.", dve);
throw ErrorHandler.getMappable(dve, "error.msg.staff.unknown.data.integrity.issue",
"Unknown data integrity issue with resource: " + realCause.getMessage());
}
Expand Down
Loading
Loading