Skip to content
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.

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

FINERACT 2120: Add new command parameters withdrawToLinkedAccount and disburseToLinkedAccount and integrate with PHEE #4046

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1148,6 +1148,15 @@ public CommandWrapperBuilder disburseLoanToSavingsApplication(final Long loanId)
return this;
}

public CommandWrapperBuilder disburseLoanToLinkedAccountApplication(final Long loanId) {
this.actionName = "DISBURSETOLINKEDACCOUNT";
this.entityName = "LOAN";
this.entityId = loanId;
this.loanId = loanId;
this.href = "/loans/" + loanId;
return this;
}

public CommandWrapperBuilder disburseWithoutAutoDownPayment(final Long loanId) {
this.actionName = "DISBURSEWITHOUTAUTODOWNPAYMENT";
this.entityName = "LOAN";
Expand Down Expand Up @@ -1563,6 +1572,15 @@ public CommandWrapperBuilder savingsAccountWithdrawal(final Long accountId) {
return this;
}

public CommandWrapperBuilder savingsAccountWithdrawToLinkedAccount(final Long accountId) {
this.actionName = "WITHDRAWTOLINKEDACCOUNT";
this.entityName = "SAVINGSACCOUNT";
this.savingsId = accountId;
this.entityId = null;
this.href = "/savingsaccounts/" + accountId + "/transactions";
return this;
}

public CommandWrapperBuilder undoSavingsAccountTransaction(final Long accountId, final Long transactionId) {
this.actionName = "UNDOTRANSACTION";
this.entityName = "SAVINGSACCOUNT";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,6 @@ public interface ConfigurationDomainService {

String getNextPaymentDateConfigForLoan();

boolean isPaymentHubIntegrationEnabled();

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.util.Objects;
import java.util.Set;
import lombok.Getter;
import lombok.Setter;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.fineract.infrastructure.core.domain.ExternalId;
Expand All @@ -52,6 +53,7 @@
*/

@Getter
@Setter
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need Setter annotation here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this resolved?

public final class JsonCommand {

private final String jsonCommand;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,15 @@ public static ApiGlobalErrorResponse notFound(final String globalisationMessageC
return create(SC_NOT_FOUND, "error.msg.resource.not.found", msg, msg, errors);
}

public static ApiGlobalErrorResponse notEnabled(final String globalisationMessageCode, final String defaultUserMessage,
final Object... defaultUserMessageArgs) {
String msg = "The requested resource is not enabled.";
final List<ApiParameterError> errors = new ArrayList<>();
errors.add(ApiParameterError.resourceIdentifierNotEnabled(globalisationMessageCode, defaultUserMessage, defaultUserMessageArgs));

return create(SC_FORBIDDEN, "error.msg.resource.not.enabled", msg, msg, errors);
}

public static ApiGlobalErrorResponse badClientRequest(final String globalisationMessageCode, final String defaultUserMessage,
final List<ApiParameterError> errors) {
return create(SC_BAD_REQUEST, globalisationMessageCode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ public static ApiParameterError resourceIdentifierNotFound(final String globalis
return new ApiParameterError(globalisationMessageCode, defaultUserMessage, defaultUserMessageArgs, "id", null);
}

public static ApiParameterError resourceIdentifierNotEnabled(final String globalisationMessageCode, final String defaultUserMessage,
final Object... defaultUserMessageArgs) {
return new ApiParameterError(globalisationMessageCode, defaultUserMessage, defaultUserMessageArgs, "configuration", null);
}

public static ApiParameterError parameterError(final String globalisationMessageCode, final String defaultUserMessage,
final String parameterName, final Object... defaultUserMessageArgs) {
return new ApiParameterError(globalisationMessageCode, defaultUserMessage, defaultUserMessageArgs, parameterName, null);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.fineract.infrastructure.core.exception;

public class ConfigurationNotEnabledException extends AbstractPlatformException {

protected ConfigurationNotEnabledException(String globalisationMessageCode, String defaultUserMessage,
final Object... defaultUserMessageArgs) {
super(globalisationMessageCode, defaultUserMessage, defaultUserMessageArgs);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.fineract.infrastructure.core.exceptionmapper;

import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
import jakarta.ws.rs.ext.Provider;
import lombok.extern.slf4j.Slf4j;
import org.apache.fineract.infrastructure.core.data.ApiGlobalErrorResponse;
import org.apache.fineract.infrastructure.core.exception.ConfigurationNotEnabledException;
import org.apache.fineract.infrastructure.core.exception.ErrorHandler;
import org.springframework.context.annotation.Scope;
import org.springframework.stereotype.Component;

@Provider
@Component
@Scope("singleton")
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed.

@Slf4j
public class ConfigurationNotEnabledExceptionMapper implements ExceptionMapper<ConfigurationNotEnabledException> {

@Override
public Response toResponse(final ConfigurationNotEnabledException exception) {
log.warn("Exception occurred", ErrorHandler.findMostSpecificException(exception));
final ApiGlobalErrorResponse notEnabledErrorResponse = ApiGlobalErrorResponse.notEnabled(exception.getGlobalisationMessageCode(),
exception.getDefaultUserMessage(), exception.getDefaultUserMessageArgs());
return Response.status(Response.Status.FORBIDDEN).entity(notEnabledErrorResponse).type(MediaType.APPLICATION_JSON).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import jakarta.persistence.Table;
import java.util.Map;
import lombok.Getter;
import lombok.Setter;
import org.apache.commons.lang3.StringUtils;
import org.apache.fineract.infrastructure.core.api.JsonCommand;
import org.apache.fineract.infrastructure.core.domain.AbstractPersistableCustom;
Expand All @@ -35,6 +36,7 @@

@Entity
@Getter
@Setter
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did introduced setters here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this resolved?

@Table(name = "m_payment_detail")
public class PaymentDetail extends AbstractPersistableCustom<Long> {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.fineract.portfolio.loanaccount.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.portfolio.loanaccount.service.LoanWritePlatformService;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
@CommandType(entity = "LOAN", action = "DISBURSETOLINKEDACCOUNT")
public class DisburseLoanToLinkedAccountCommandHandler implements NewCommandSourceHandler {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you consider moving payment hub ee related stuff to a separate module?


private final LoanWritePlatformService writePlatformService;

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

return this.writePlatformService.disburseLoanToLinkedAccount(command.entityId(), command, true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ public interface LoanWritePlatformService {

CommandProcessingResult disburseLoan(Long loanId, JsonCommand command, Boolean isAccountTransfer);

CommandProcessingResult disburseLoanToLinkedAccount(Long loanId, JsonCommand command, Boolean isAccountTransfer);

CommandProcessingResult disburseLoan(Long loanId, JsonCommand command, Boolean isAccountTransfer, Boolean isWithoutAutoPayment);

Map<String, Object> bulkLoanDisbursal(JsonCommand command, CollectionSheetBulkDisbursalCommand bulkDisbursalCommand,
Expand Down
13 changes: 11 additions & 2 deletions fineract-provider/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ apply plugin: 'com.google.cloud.tools.jib'
apply plugin: 'org.springframework.boot'
apply plugin: 'se.thinkcode.cucumber-runner'


repositories {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this repository!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This repository is required for this dependency implementation 'org.pheesdk:PaymentHubSDK:1.0.0'

Copy link
Contributor

@adamsaghy adamsaghy Sep 4, 2024

Choose a reason for hiding this comment

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

You cannot put your own repository into a community project. If the payment hub sdk is not available on a public repository that is not controlled by your organization, it has absolutely no place in opersource, community Fineract.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it be possible to load the Payment Hub SDK dependency at runtime if it's hosted in a private JFrog Artifactory?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is absolutely a no-go. If Payment Hub SDK is needed, let's fix it over there to be published to a publicly accessed repository, for example Maven central.

maven {
url = uri('https://jfrog.sandbox.fynarfin.io/artifactory/fyn-libs-snapshot')
}
}

check.dependsOn('cucumber')

compileJava.doLast {
Expand Down Expand Up @@ -164,15 +171,15 @@ configurations.driver.each {File file ->
task createDB {
description= "Creates the MariaDB Database. Needs database name to be passed (like: -PdbName=someDBname)"
doLast {
def sql = Sql.newInstance( 'jdbc:mariadb://localhost:3306/', mysqlUser, mysqlPassword, 'org.mariadb.jdbc.Driver' )
def sql = Sql.newInstance( 'jdbc:mariadb://localhost:3307/', mysqlUser, mysqlPassword, 'org.mariadb.jdbc.Driver' )
Copy link
Contributor

Choose a reason for hiding this comment

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

Please reverse these changes!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do that

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment shouldn't be resolved since the changes weren't yet made.

sql.execute( 'CREATE DATABASE '+"`$dbName` CHARACTER SET utf8mb4" )
}
}

task dropDB {
description= "Drops the specified MariaDB database. The database name has to be passed (like: -PdbName=someDBname)"
doLast {
def sql = Sql.newInstance( 'jdbc:mariadb://localhost:3306/', mysqlUser, mysqlPassword, 'org.mariadb.jdbc.Driver' )
def sql = Sql.newInstance( 'jdbc:mariadb://localhost:3307/', mysqlUser, mysqlPassword, 'org.mariadb.jdbc.Driver' )
Copy link
Contributor

Choose a reason for hiding this comment

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

Please reverse these changes!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do that

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment shouldn't be resolved since the changes weren't yet made.

sql.execute( 'DROP DATABASE '+"`$dbName`")
}
}
Expand Down Expand Up @@ -225,6 +232,7 @@ bootRun {
dependencies {
implementation 'org.mariadb.jdbc:mariadb-java-client'
implementation 'org.postgresql:postgresql'
implementation 'org.pheesdk:PaymentHubSDK:1.0.0'
Copy link
Contributor

@adamsaghy adamsaghy Sep 4, 2024

Choose a reason for hiding this comment

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

I dont think it is right to be here. Would you move to the dependencies.gradle? Also it might be better to have it in a separate module. Most of the ppl would not need payment hub support...
@vidakovic What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

The version shouldn't be here that's for sure. In terms of packaging the PaymentHubSDK with Fineract, there are 2 options:

  • bundle with Fineract as it is
  • extract into a separate module where the payment hub dependency is there - in this case there needs to be build adjustments so that Fineract is published in 2 versions, with and without payment hub

I'm not sure how much extra weight the Payment Hub SDK is, but if it's just DTOs and API classes, I'd say it's acceptable to be bundled with Fineract.

}
}

Expand Down Expand Up @@ -280,6 +288,7 @@ jib {
dependencies {
implementation 'org.mariadb.jdbc:mariadb-java-client'
implementation 'org.postgresql:postgresql'
implementation 'org.pheesdk:PaymentHubSDK:1.0.0'
}

pluginExtensions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public class ConfigurationDomainServiceJpa implements ConfigurationDomainService
private static final String ENABLE_ADDRESS = "Enable-Address";
private static final String ENABLE_COB_BULK_EVENT = "enable-cob-bulk-event";
private static final String EXTERNAL_EVENT_BATCH_SIZE = "external-event-batch-size";
private static final String ENABLE_PAYMENT_HUB_INTEGRATION = "enable_payment_hub_integration";

private static final String REPORT_EXPORT_S3_FOLDER_NAME = "report-export-s3-folder-name";

Expand Down Expand Up @@ -529,4 +530,10 @@ public String getNextPaymentDateConfigForLoan() {
return value;
}

@Override
public boolean isPaymentHubIntegrationEnabled() {
final GlobalConfigurationPropertyData property = getGlobalConfigurationPropertyData(ENABLE_PAYMENT_HUB_INTEGRATION);
return property.isEnabled();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.fineract.infrastructure.configuration.exception;

import org.apache.fineract.infrastructure.core.exception.ConfigurationNotEnabledException;

public class GlobalConfigurationNotEnabledException extends ConfigurationNotEnabledException {

public GlobalConfigurationNotEnabledException(final String configName) {
super("error.msg.configuration.not.enabled", "Configuration `" + configName + "` is not enabled", configName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,6 @@
public interface InteropIdentifierRepository extends JpaRepository<InteropIdentifier, Long>, JpaSpecificationExecutor<InteropIdentifier> {

InteropIdentifier findOneByTypeAndValueAndSubType(InteropIdentifierType idType, String value, String subType);

InteropIdentifier findOneByAccountId(Long accountId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
import org.apache.fineract.interoperation.data.InteropTransactionRequestResponseData;
import org.apache.fineract.interoperation.data.InteropTransactionsData;
import org.apache.fineract.interoperation.data.InteropTransferResponseData;
import org.apache.fineract.interoperation.domain.InteropIdentifier;
import org.apache.fineract.interoperation.domain.InteropIdentifierType;
import org.apache.fineract.portfolio.savings.domain.SavingsAccount;

public interface InteropService {

Expand All @@ -47,6 +49,12 @@ InteropTransactionsData getAccountTransactions(@NotNull String accountId, boolea
InteropIdentifierAccountResponseData getAccountByIdentifier(@NotNull InteropIdentifierType idType, @NotNull String idValue,
String subIdOrType);

@NotNull
InteropIdentifier getIndentifierByAccount(@NotNull SavingsAccount account);

@NotNull
InteropIdentifier getIdentifierByAccountId(@NotNull Long accountId);

@NotNull
InteropIdentifierAccountResponseData registerAccountIdentifier(@NotNull InteropIdentifierType idType, @NotNull String idValue,
String subIdOrType, @NotNull JsonCommand command);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,21 @@ public InteropIdentifierAccountResponseData getAccountByIdentifier(@NotNull Inte
return InteropIdentifierAccountResponseData.build(identifier.getId(), identifier.getAccount().getExternalId().getValue());
}

@NotNull
@Transactional
@Override
public InteropIdentifier getIndentifierByAccount(@NotNull SavingsAccount account) {
InteropIdentifier identifier = findIdentifier(account);
return identifier;
}

@NotNull
@Transactional
@Override
public InteropIdentifier getIdentifierByAccountId(@NotNull Long accountId) {
return identifierRepository.findOneByAccountId(accountId);
}

@NotNull
@Transactional
@Override
Expand Down Expand Up @@ -641,6 +656,10 @@ public InteropIdentifier findIdentifier(@NotNull InteropIdentifierType idType, @
return identifierRepository.findOneByTypeAndValueAndSubType(idType, idValue, subIdOrType);
}

public InteropIdentifier findIdentifier(@NotNull SavingsAccount account) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we have a Transactional annotation on this?

return identifierRepository.findOneByAccountId(account.getId());
}

/*
* Guaranteed to throw an exception no matter what the data integrity issue is.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,8 @@ private String stateTransitions(final Long loanId, final String loanExternalIdSt
commandRequest = builder.disburseLoanApplication(resolvedLoanId).build();
} else if (CommandParameterUtil.is(commandParam, "disburseToSavings")) {
commandRequest = builder.disburseLoanToSavingsApplication(resolvedLoanId).build();
} else if (CommandParameterUtil.is(commandParam, "disburseToLinkedAccount")) {
commandRequest = builder.disburseLoanToLinkedAccountApplication(resolvedLoanId).build();
} else if (CommandParameterUtil.is(commandParam, "disburseWithoutAutoDownPayment")) {
commandRequest = builder.disburseWithoutAutoDownPayment(resolvedLoanId).build();
} else if (CommandParameterUtil.is(commandParam, "undoapproval")) {
Expand Down
Loading