You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/11/24 00:28:47 UTC
[GitHub] [fineract] adamsaghy opened a new pull request, #2758: FINERACT-1760: Enhanced external id support for loan transactions
adamsaghy opened a new pull request, #2758:
URL: https://github.com/apache/fineract/pull/2758
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy merged pull request #2758: FINERACT-1760: Enhanced external id support for loan transactions
Posted by GitBox <gi...@apache.org>.
adamsaghy merged PR #2758:
URL: https://github.com/apache/fineract/pull/2758
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2758: FINERACT-1760: Enhanced external id support for loan transactions
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2758:
URL: https://github.com/apache/fineract/pull/2758#discussion_r1031775599
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java:
##########
@@ -193,9 +177,9 @@ toLoanAccount, new CommandProcessingResultBuilder(), transactionDate, transactio
fromLoanAccountId = command.longValueOfParameterNamed(fromAccountIdParamName);
final Loan fromLoanAccount = this.loanAccountAssembler.assembleFrom(fromLoanAccountId);
-
+ ExternalId externalId = externalIdFactory.fromString(null);
Review Comment:
its either empty or generate a new uuid, based on configuration
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2758: FINERACT-1760: Enhanced external id support for loan transactions
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2758:
URL: https://github.com/apache/fineract/pull/2758#discussion_r1031582421
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformService.java:
##########
@@ -155,4 +156,7 @@ LoanScheduleData retrieveRepaymentSchedule(Long loanId, RepaymentScheduleRelated
List<LoanTransactionRelationData> retrieveLoanTransactionRelationsByLoanTransactionId(Long loanTransactionId);
+ Long retrieveLoanTransactionIdByExternalId(ExternalId externalId);
+
+ Long retrieveLoanIdByExternalId(String externalId);
Review Comment:
It is not loan transaction. A following story will handle and enhance external id support for Loan
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2758: FINERACT-1760: Enhanced external id support for loan transactions
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2758:
URL: https://github.com/apache/fineract/pull/2758#discussion_r1031662120
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/ExternalId.java:
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.domain;
+
+import java.io.Serial;
+import java.io.Serializable;
+import java.util.UUID;
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import org.apache.fineract.infrastructure.core.exception.PlatformInternalServerException;
+
+/**
+ * ExternalId Value object
+ */
+@Getter
+@EqualsAndHashCode
+@Builder
+@AllArgsConstructor
Review Comment:
ok :)
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/ExternalId.java:
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.domain;
+
+import java.io.Serial;
+import java.io.Serializable;
+import java.util.UUID;
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import org.apache.fineract.infrastructure.core.exception.PlatformInternalServerException;
+
+/**
+ * ExternalId Value object
+ */
+@Getter
+@EqualsAndHashCode
+@Builder
+@AllArgsConstructor
+public class ExternalId implements Serializable {
+
+ @Serial
+ private static final long serialVersionUID = 1;
+ private static final ExternalId empty = new ExternalId(null);
+ private final String value;
+
+ /**
+ * @return Create a new ExternalId object where value is a newly generated UUID
+ */
+ public static ExternalId generate() {
+ return new ExternalId(UUID.randomUUID().toString());
+ }
+
+ /**
+ * @return Create and return an empty ExternalId object
+ */
+ public static ExternalId empty() {
+ return empty;
+ }
+
+ /**
+ * @return whether value was set for the ExternalId object (return false if value is null)
+ */
+ public boolean isSet() {
+ return value != null;
+ }
+
+ /**
+ * Throws exception if value was not set (value is null) for this object
+ *
+ * @throws PlatformInternalServerException
+ * if value was not set (value is null) for this object
+ */
+ public void throwErrorIfItsNotSet() {
Review Comment:
Roger
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2758: FINERACT-1760: Enhanced external id support for loan transactions
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2758:
URL: https://github.com/apache/fineract/pull/2758#discussion_r1031431071
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/ExternalId.java:
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.domain;
+
+import java.io.Serial;
+import java.io.Serializable;
+import java.util.UUID;
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import org.apache.fineract.infrastructure.core.exception.PlatformInternalServerException;
+
+/**
+ * ExternalId Value object
+ */
+@Getter
+@EqualsAndHashCode
+@Builder
+@AllArgsConstructor
Review Comment:
Instead of the generated constructor, we could define the invariant for this class as following:
- it can have a null value associated, that means an empty ExternalId
- if the value is not null, it should not be blank.
Also, I'd create 2 constructors. One for internal use to create the empty ExternalId reference, and one for checking whether the incoming value is not blank
```
public ExternalId(String value) {
if (StringUtils.isBlank(value)) {
throw new IllegalArgumentException(...)
}
this.value = value;
}
private ExternalId() {
this.value = null;
}
```
Something like this. Thoughts?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/ExternalId.java:
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.domain;
+
+import java.io.Serial;
+import java.io.Serializable;
+import java.util.UUID;
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import org.apache.fineract.infrastructure.core.exception.PlatformInternalServerException;
+
+/**
+ * ExternalId Value object
+ */
+@Getter
+@EqualsAndHashCode
+@Builder
+@AllArgsConstructor
+public class ExternalId implements Serializable {
+
+ @Serial
+ private static final long serialVersionUID = 1;
+ private static final ExternalId empty = new ExternalId(null);
+ private final String value;
+
+ /**
+ * @return Create a new ExternalId object where value is a newly generated UUID
+ */
+ public static ExternalId generate() {
+ return new ExternalId(UUID.randomUUID().toString());
+ }
+
+ /**
+ * @return Create and return an empty ExternalId object
+ */
+ public static ExternalId empty() {
+ return empty;
+ }
+
+ /**
+ * @return whether value was set for the ExternalId object (return false if value is null)
+ */
+ public boolean isSet() {
+ return value != null;
+ }
+
+ /**
+ * Throws exception if value was not set (value is null) for this object
+ *
+ * @throws PlatformInternalServerException
+ * if value was not set (value is null) for this object
+ */
+ public void throwErrorIfItsNotSet() {
+ if (!isSet()) {
+ throw new PlatformInternalServerException("error.external.id.is.not.set", "Internal state violation: External id is not set");
+ }
+ }
+
+ @Override
+ public String toString() {
Review Comment:
@ToString lombok annotation?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/ExternalId.java:
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.domain;
+
+import java.io.Serial;
+import java.io.Serializable;
+import java.util.UUID;
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import org.apache.fineract.infrastructure.core.exception.PlatformInternalServerException;
+
+/**
+ * ExternalId Value object
+ */
+@Getter
+@EqualsAndHashCode
+@Builder
Review Comment:
I'd say let's not allow a Builder for a value object. Right now it's not an issue but in the future if a value object includes more than 1 field, it could end up in an inconsistent state where all the invariants are not fulfilled. (For example think about a client's name. It'll consist of a firstname and a lastname. And that'll be an invariant for the ClientName value object to have a firstname and a lastname. With the builder, we can't enforce to have those 2 values set, only with a constructor)
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/ExternalId.java:
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.domain;
+
+import java.io.Serial;
+import java.io.Serializable;
+import java.util.UUID;
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import org.apache.fineract.infrastructure.core.exception.PlatformInternalServerException;
+
+/**
+ * ExternalId Value object
+ */
+@Getter
+@EqualsAndHashCode
+@Builder
+@AllArgsConstructor
+public class ExternalId implements Serializable {
+
+ @Serial
+ private static final long serialVersionUID = 1;
+ private static final ExternalId empty = new ExternalId(null);
+ private final String value;
+
+ /**
+ * @return Create a new ExternalId object where value is a newly generated UUID
+ */
+ public static ExternalId generate() {
+ return new ExternalId(UUID.randomUUID().toString());
+ }
+
+ /**
+ * @return Create and return an empty ExternalId object
+ */
+ public static ExternalId empty() {
+ return empty;
+ }
+
+ /**
+ * @return whether value was set for the ExternalId object (return false if value is null)
+ */
+ public boolean isSet() {
+ return value != null;
+ }
+
+ /**
+ * Throws exception if value was not set (value is null) for this object
+ *
+ * @throws PlatformInternalServerException
+ * if value was not set (value is null) for this object
+ */
+ public void throwErrorIfItsNotSet() {
Review Comment:
Not sure I'd put this here but I can live with it. Naming-wise probably we can just rename it to `throwExceptionIfEmpty` and the `isSet` could be also renamed to `isEmpty` to be consistent with the naming for an empty ExternalId (`empty` method)
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/mapper/loan/LoanTransactionDataMapper.java:
##########
@@ -22,9 +22,11 @@
import org.apache.fineract.infrastructure.event.external.service.serialization.mapper.support.AvroMapperConfig;
import org.apache.fineract.portfolio.loanaccount.data.LoanTransactionData;
import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
@Mapper(config = AvroMapperConfig.class)
public interface LoanTransactionDataMapper {
+ @Mapping(target = "externalId", source = "externalId.value")
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java:
##########
@@ -421,15 +436,21 @@ public Long transferFunds(final AccountTransferDTO accountTransferDTO) {
this.savingsAccountAssembler.setHelpers(toSavingsAccount);
}
LoanTransaction loanTransaction = null;
+
+ ExternalId externalId = accountTransferDTO.getTxnExternalId();
+ if ((externalId == null || !externalId.isSet()) && configurationDomainService.isExternalIdAutoGenerationEnabled()) {
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java:
##########
@@ -474,13 +495,22 @@ public AccountTransferDetails repayLoanWithTopup(AccountTransferDTO accountTrans
this.loanAccountAssembler.setHelpers(toLoanAccount);
}
+ ExternalId externalIdForDisbursement = accountTransferDTO.getTxnExternalId();
+ ExternalId externalIdForRepayment = ExternalId.empty();
+ if ((externalIdForDisbursement == null || !externalIdForDisbursement.isSet())
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformService.java:
##########
@@ -155,4 +156,7 @@ LoanScheduleData retrieveRepaymentSchedule(Long loanId, RepaymentScheduleRelated
List<LoanTransactionRelationData> retrieveLoanTransactionRelationsByLoanTransactionId(Long loanTransactionId);
+ Long retrieveLoanTransactionIdByExternalId(ExternalId externalId);
+
+ Long retrieveLoanIdByExternalId(String externalId);
Review Comment:
Any reason this externalId is a string?
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java:
##########
@@ -329,12 +339,17 @@ public Long transferFunds(final AccountTransferDTO accountTransferDTO) {
accountTransferDTO.getFmt(), accountTransferDTO.getTransactionDate(), accountTransferDTO.getTransactionAmount(),
accountTransferDTO.getPaymentDetail(), transactionBooleanValues, backdatedTxnsAllowedTill);
- LoanTransaction loanTransaction = null;
+ LoanTransaction loanTransaction;
+
+ ExternalId externalId = accountTransferDTO.getTxnExternalId();
+ if ((externalId == null || !externalId.isSet()) && configurationDomainService.isExternalIdAutoGenerationEnabled()) {
Review Comment:
Since the ExternalId is a value object and it can represent an empty ExternalId. Can we skip the nullcheck completely and make sure the getTxnExternalId actually returns an instance?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/mapper/loan/LoanAccountDataMapper.java:
##########
@@ -33,4 +35,7 @@ public interface LoanAccountDataMapper {
@Mapping(source = "topup", target = "isTopup")
@Mapping(source = "interestRecalculationEnabled", target = "isInterestRecalculationEnabled")
LoanAccountDataV1 map(LoanAccountData source);
+
+ @Mapping(target = "externalId", source = "externalId.value")
Review Comment:
You can do it generally as well. Create a Mapper just for the ExternalId -> String mapping and add it to the `AvroMapperConfig`, just like I did with the `AvroDateTimeMapper`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2758: FINERACT-1760: Enhanced external id support for loan transactions
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2758:
URL: https://github.com/apache/fineract/pull/2758#discussion_r1031662312
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/ExternalId.java:
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.domain;
+
+import java.io.Serial;
+import java.io.Serializable;
+import java.util.UUID;
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import org.apache.fineract.infrastructure.core.exception.PlatformInternalServerException;
+
+/**
+ * ExternalId Value object
+ */
+@Getter
+@EqualsAndHashCode
+@Builder
Review Comment:
Roger
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2758: FINERACT-1760: Enhanced external id support for loan transactions
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2758:
URL: https://github.com/apache/fineract/pull/2758#discussion_r1031771133
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java:
##########
@@ -541,8 +539,10 @@ public CommandProcessingResult refundByTransfer(JsonCommand command) {
throw new InvalidPaidInAdvanceAmountException(overpaid.toPlainString());
}
+ ExternalId externalId = externalIdFactory.fromString(null);
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java:
##########
@@ -193,9 +177,9 @@ toLoanAccount, new CommandProcessingResultBuilder(), transactionDate, transactio
fromLoanAccountId = command.longValueOfParameterNamed(fromAccountIdParamName);
final Loan fromLoanAccount = this.loanAccountAssembler.assembleFrom(fromLoanAccountId);
-
+ ExternalId externalId = externalIdFactory.fromString(null);
Review Comment:
Why not just use ExternalId.empty() here?
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/collectionsheet/serialization/CollectionSheetBulkRepaymentCommandFromApiJsonDeserializer.java:
##########
@@ -90,14 +91,17 @@ public CollectionSheetBulkRepaymentCommand commandFromApiJson(final String json,
final JsonObject loanTransactionElement = array.get(i).getAsJsonObject();
final Long loanId = this.fromApiJsonHelper.extractLongNamed("loanId", loanTransactionElement);
+ final String externalIdStr = this.fromApiJsonHelper.extractStringNamed("externalId", loanTransactionElement);
+ final ExternalId externalId = new ExternalId(externalIdStr);
Review Comment:
Isn't this gonna be an issue in case the externalId param was not provided?
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/guarantor/service/GuarantorDomainServiceImpl.java:
##########
@@ -245,11 +249,11 @@ public void transaferFundsFromGuarantor(final Loan loan) {
remainingAmount = remainingAmount.multiply(loan.getPrincipal().getAmount()).divide(loan.getGuaranteeAmount(),
MoneyHelper.getRoundingMode());
}
+ ExternalId externalId = externalIdFactory.fromString(null);
Review Comment:
ExternalId.empty?
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java:
##########
@@ -730,8 +725,10 @@ public Map<String, Object> foreCloseLoan(final Loan loan, final LocalDate foreCl
Money penaltyPortion = foreCloseDetail.getPenaltyChargesCharged(currency).minus(accruedReceivables[2]);
Money total = interestPortion.plus(feePortion).plus(penaltyPortion);
if (total.isGreaterThanZero()) {
+ ExternalId accrualExternalId = externalIdFactory.fromString(null);
Review Comment:
ExternalId.empty?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org