You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "galovics (via GitHub)" <gi...@apache.org> on 2023/06/06 13:39:21 UTC
[GitHub] [fineract] galovics opened a new pull request, #3232: FINERACT-1724: Batch API fixes + client validation fixes
galovics opened a new pull request, #3232:
URL: https://github.com/apache/fineract/pull/3232
## 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 commented on a diff in pull request #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219706264
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -443,51 +430,32 @@ private void validateNameParts(final List<ApiParameterError> dataValidationError
}
}
- private void validateActivationDate(final List<ApiParameterError> dataValidationErrors) {
+ private void validateActivationDate() {
if (getSubmittedOnDate() != null && isDateInTheFuture(getSubmittedOnDate())) {
-
- final String defaultUserMessage = "submitted date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.in.the.future",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.in.the.future", "submitted date cannot be in the future.",
+ getSubmittedOnDate());
}
if (getActivationLocalDate() != null && getSubmittedOnDate() != null && getSubmittedOnDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "submitted date cannot be after the activation date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.activation.date",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.activation.date",
+ "submitted date cannot be after the activation date", getSubmittedOnDate());
}
if (getReopenedDate() != null && getActivationLocalDate() != null && getReopenedDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "reopened date cannot be after the submittedon date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.reopened.date",
- defaultUserMessage, ClientApiConstants.reopenedDateParamName, this.reopenedDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.reopened.date",
+ "reopened date cannot be after the submittedon date", this.reopenedDate);
}
if (getActivationLocalDate() != null && isDateInTheFuture(getActivationLocalDate())) {
-
- final String defaultUserMessage = "Activation date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.activationDate.in.the.future",
- defaultUserMessage, ClientApiConstants.activationDateParamName, getActivationLocalDate());
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("activationDate.in.the.future", "Activation date cannot be in the future.",
+ getActivationLocalDate());
}
if (getActivationLocalDate() != null) {
if (this.office.isOpeningDateAfter(getActivationLocalDate())) {
- final String defaultUserMessage = "Client activation date cannot be a date before the office opening date.";
- final ApiParameterError error = ApiParameterError.parameterError(
- "error.msg.clients.activationDate.cannot.be.before.office.activation.date", defaultUserMessage,
- ClientApiConstants.activationDateParamName, getActivationLocalDate());
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("activationDate.cannot.be.before.office.activation.date",
Review Comment:
this might break the contract. Priorly the problematic field name was sent, i reckon.
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219702358
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -443,51 +430,32 @@ private void validateNameParts(final List<ApiParameterError> dataValidationError
}
}
- private void validateActivationDate(final List<ApiParameterError> dataValidationErrors) {
+ private void validateActivationDate() {
if (getSubmittedOnDate() != null && isDateInTheFuture(getSubmittedOnDate())) {
-
- final String defaultUserMessage = "submitted date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.in.the.future",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.in.the.future", "submitted date cannot be in the future.",
Review Comment:
this might break the contract. Priorly the submitted on date and the **problematic field name** was sent, i reckon.
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219705954
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -443,51 +430,32 @@ private void validateNameParts(final List<ApiParameterError> dataValidationError
}
}
- private void validateActivationDate(final List<ApiParameterError> dataValidationErrors) {
+ private void validateActivationDate() {
if (getSubmittedOnDate() != null && isDateInTheFuture(getSubmittedOnDate())) {
-
- final String defaultUserMessage = "submitted date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.in.the.future",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.in.the.future", "submitted date cannot be in the future.",
+ getSubmittedOnDate());
}
if (getActivationLocalDate() != null && getSubmittedOnDate() != null && getSubmittedOnDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "submitted date cannot be after the activation date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.activation.date",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.activation.date",
+ "submitted date cannot be after the activation date", getSubmittedOnDate());
}
if (getReopenedDate() != null && getActivationLocalDate() != null && getReopenedDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "reopened date cannot be after the submittedon date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.reopened.date",
- defaultUserMessage, ClientApiConstants.reopenedDateParamName, this.reopenedDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.reopened.date",
+ "reopened date cannot be after the submittedon date", this.reopenedDate);
}
if (getActivationLocalDate() != null && isDateInTheFuture(getActivationLocalDate())) {
-
- final String defaultUserMessage = "Activation date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.activationDate.in.the.future",
- defaultUserMessage, ClientApiConstants.activationDateParamName, getActivationLocalDate());
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("activationDate.in.the.future", "Activation date cannot be in the future.",
Review Comment:
this might break the contract. Priorly the problematic field name was sent, i reckon.
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219701095
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -443,51 +430,32 @@ private void validateNameParts(final List<ApiParameterError> dataValidationError
}
}
- private void validateActivationDate(final List<ApiParameterError> dataValidationErrors) {
+ private void validateActivationDate() {
if (getSubmittedOnDate() != null && isDateInTheFuture(getSubmittedOnDate())) {
-
- final String defaultUserMessage = "submitted date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.in.the.future",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.in.the.future", "submitted date cannot be in the future.",
Review Comment:
Wrong type of error: The problem is not with the activate date
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219703430
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -443,51 +430,32 @@ private void validateNameParts(final List<ApiParameterError> dataValidationError
}
}
- private void validateActivationDate(final List<ApiParameterError> dataValidationErrors) {
+ private void validateActivationDate() {
if (getSubmittedOnDate() != null && isDateInTheFuture(getSubmittedOnDate())) {
-
- final String defaultUserMessage = "submitted date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.in.the.future",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.in.the.future", "submitted date cannot be in the future.",
+ getSubmittedOnDate());
}
if (getActivationLocalDate() != null && getSubmittedOnDate() != null && getSubmittedOnDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "submitted date cannot be after the activation date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.activation.date",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.activation.date",
Review Comment:
Wrong type of error: The problem is not with the activate date.
this might break the contract. Priorly the submitted on date and the problematic field name was sent, i reckon.
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219693818
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -349,17 +343,10 @@ public void updateAccountNo(final String accountIdentifier) {
this.accountNumberRequiresAutoGeneration = false;
}
- public void activate(final AppUser currentUser, final DateTimeFormatter formatter, final LocalDate activationLocalDate) {
+ public void activate(final AppUser currentUser, final LocalDate activationLocalDate) {
if (isActive()) {
- final String defaultUserMessage = "Cannot activate client. Client is already active.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.already.active", defaultUserMessage,
- ClientApiConstants.activationDateParamName, activationLocalDate.format(formatter));
-
- final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
- dataValidationErrors.add(error);
-
- throw new PlatformApiDataValidationException(dataValidationErrors);
+ throw new ClientAlreadyActiveException();
Review Comment:
this might break the contract. Priorly the activation date was sent, i reckon.
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219702358
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -443,51 +430,32 @@ private void validateNameParts(final List<ApiParameterError> dataValidationError
}
}
- private void validateActivationDate(final List<ApiParameterError> dataValidationErrors) {
+ private void validateActivationDate() {
if (getSubmittedOnDate() != null && isDateInTheFuture(getSubmittedOnDate())) {
-
- final String defaultUserMessage = "submitted date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.in.the.future",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.in.the.future", "submitted date cannot be in the future.",
Review Comment:
this might break the contract. Priorly the **problematic field name** was sent, i reckon.
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -443,51 +430,32 @@ private void validateNameParts(final List<ApiParameterError> dataValidationError
}
}
- private void validateActivationDate(final List<ApiParameterError> dataValidationErrors) {
+ private void validateActivationDate() {
if (getSubmittedOnDate() != null && isDateInTheFuture(getSubmittedOnDate())) {
-
- final String defaultUserMessage = "submitted date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.in.the.future",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.in.the.future", "submitted date cannot be in the future.",
+ getSubmittedOnDate());
}
if (getActivationLocalDate() != null && getSubmittedOnDate() != null && getSubmittedOnDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "submitted date cannot be after the activation date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.activation.date",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.activation.date",
Review Comment:
Wrong type of error: The problem is not with the activate date.
this might break the contract. Priorly the **problematic field name** was sent, i reckon.
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219693818
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -349,17 +343,10 @@ public void updateAccountNo(final String accountIdentifier) {
this.accountNumberRequiresAutoGeneration = false;
}
- public void activate(final AppUser currentUser, final DateTimeFormatter formatter, final LocalDate activationLocalDate) {
+ public void activate(final AppUser currentUser, final LocalDate activationLocalDate) {
if (isActive()) {
- final String defaultUserMessage = "Cannot activate client. Client is already active.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.already.active", defaultUserMessage,
- ClientApiConstants.activationDateParamName, activationLocalDate.format(formatter));
-
- final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
- dataValidationErrors.add(error);
-
- throw new PlatformApiDataValidationException(dataValidationErrors);
+ throw new ClientAlreadyActiveException();
Review Comment:
this might break the contract. Priorly the activation date and the problematic field was sent, i reckon.
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219704267
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -443,51 +430,32 @@ private void validateNameParts(final List<ApiParameterError> dataValidationError
}
}
- private void validateActivationDate(final List<ApiParameterError> dataValidationErrors) {
+ private void validateActivationDate() {
if (getSubmittedOnDate() != null && isDateInTheFuture(getSubmittedOnDate())) {
-
- final String defaultUserMessage = "submitted date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.in.the.future",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.in.the.future", "submitted date cannot be in the future.",
+ getSubmittedOnDate());
}
if (getActivationLocalDate() != null && getSubmittedOnDate() != null && getSubmittedOnDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "submitted date cannot be after the activation date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.activation.date",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.activation.date",
+ "submitted date cannot be after the activation date", getSubmittedOnDate());
}
if (getReopenedDate() != null && getActivationLocalDate() != null && getReopenedDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "reopened date cannot be after the submittedon date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.reopened.date",
- defaultUserMessage, ClientApiConstants.reopenedDateParamName, this.reopenedDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.reopened.date",
Review Comment:
Wrong type of error: The problem is not with the activate date.
this might break the contract. Priorly the problematic field name was sent, i reckon.
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -443,51 +430,32 @@ private void validateNameParts(final List<ApiParameterError> dataValidationError
}
}
- private void validateActivationDate(final List<ApiParameterError> dataValidationErrors) {
+ private void validateActivationDate() {
if (getSubmittedOnDate() != null && isDateInTheFuture(getSubmittedOnDate())) {
-
- final String defaultUserMessage = "submitted date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.in.the.future",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.in.the.future", "submitted date cannot be in the future.",
+ getSubmittedOnDate());
}
if (getActivationLocalDate() != null && getSubmittedOnDate() != null && getSubmittedOnDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "submitted date cannot be after the activation date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.activation.date",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.activation.date",
Review Comment:
Wrong type of error: The problem is not with the activate date.
this might break the contract. Priorly the problematic field name was sent, i reckon.
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219682213
##########
fineract-provider/src/main/java/org/apache/fineract/batch/exception/ErrorHandler.java:
##########
@@ -99,13 +99,13 @@ public static ErrorInfo handler(final RuntimeException exception) {
return handleException(e, new PlatformDomainRuleExceptionMapper(), 9999);
}
if (exception instanceof TransactionException) {
- return new ErrorInfo(HttpStatus.SC_BAD_REQUEST, 4001, "{\"Exception\": %s}".formatted(exception.getMessage()));
+ return new ErrorInfo(HttpStatus.SC_INTERNAL_SERVER_ERROR, 4001, "{\"Exception\": %s}".formatted(exception.getMessage()));
Review Comment:
if you are changing this for 5xx error, you should change the code as well (4001 -> 5000 or 5003?)
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219709274
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java:
##########
@@ -320,6 +320,8 @@ private LoanTransactionBusinessEvent getTransactionRepaymentTypeBusinessEvent(Lo
@Override
public LoanTransaction saveLoanTransactionWithDataIntegrityViolationChecks(LoanTransaction newRepaymentTransaction) {
try {
+ this.loanTransactionRepository.saveAndFlush(newRepaymentTransaction);
Review Comment:
I have no idea what is this for... but i dont like it...
--
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 closed pull request #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics closed pull request #3232: FINERACT-1724: Batch API fixes + client validation fixes
URL: https://github.com/apache/fineract/pull/3232
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219704267
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -443,51 +430,32 @@ private void validateNameParts(final List<ApiParameterError> dataValidationError
}
}
- private void validateActivationDate(final List<ApiParameterError> dataValidationErrors) {
+ private void validateActivationDate() {
if (getSubmittedOnDate() != null && isDateInTheFuture(getSubmittedOnDate())) {
-
- final String defaultUserMessage = "submitted date cannot be in the future.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.in.the.future",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.in.the.future", "submitted date cannot be in the future.",
+ getSubmittedOnDate());
}
if (getActivationLocalDate() != null && getSubmittedOnDate() != null && getSubmittedOnDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "submitted date cannot be after the activation date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.activation.date",
- defaultUserMessage, ClientApiConstants.submittedOnDateParamName, this.submittedOnDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.activation.date",
+ "submitted date cannot be after the activation date", getSubmittedOnDate());
}
if (getReopenedDate() != null && getActivationLocalDate() != null && getReopenedDate().isAfter(getActivationLocalDate())) {
-
- final String defaultUserMessage = "reopened date cannot be after the submittedon date";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.submittedOnDate.after.reopened.date",
- defaultUserMessage, ClientApiConstants.reopenedDateParamName, this.reopenedDate);
-
- dataValidationErrors.add(error);
+ throw new InvalidClientActivationDateException("submittedOnDate.after.reopened.date",
Review Comment:
Wrong type of error: The problem is not with the activate date.
this might break the contract. Priorly the **problematic field name** was sent, i reckon.
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219693818
##########
fineract-core/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -349,17 +343,10 @@ public void updateAccountNo(final String accountIdentifier) {
this.accountNumberRequiresAutoGeneration = false;
}
- public void activate(final AppUser currentUser, final DateTimeFormatter formatter, final LocalDate activationLocalDate) {
+ public void activate(final AppUser currentUser, final LocalDate activationLocalDate) {
if (isActive()) {
- final String defaultUserMessage = "Cannot activate client. Client is already active.";
- final ApiParameterError error = ApiParameterError.parameterError("error.msg.clients.already.active", defaultUserMessage,
- ClientApiConstants.activationDateParamName, activationLocalDate.format(formatter));
-
- final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
- dataValidationErrors.add(error);
-
- throw new PlatformApiDataValidationException(dataValidationErrors);
+ throw new ClientAlreadyActiveException();
Review Comment:
this might break the contract. Priorly the **activation date** and the **problematic field name** was sent, i reckon.
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219710511
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -628,6 +628,7 @@ private Loan saveAndFlushLoanWithDataIntegrityViolationChecks(final Loan loan) {
*/
try {
loanRepaymentScheduleInstallmentRepository.saveAll(loan.getRepaymentScheduleInstallments());
+ loanRepaymentScheduleInstallmentRepository.delete(loan.getRepaymentScheduleInstallments().get(0));
Review Comment:
I have no idea what is this for... but i dont like it
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219709274
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java:
##########
@@ -320,6 +320,8 @@ private LoanTransactionBusinessEvent getTransactionRepaymentTypeBusinessEvent(Lo
@Override
public LoanTransaction saveLoanTransactionWithDataIntegrityViolationChecks(LoanTransaction newRepaymentTransaction) {
try {
+ this.loanTransactionRepository.saveAndFlush(newRepaymentTransaction);
Review Comment:
I have no idea what is this for... but i dont like it
--
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 #3232: FINERACT-1724: Batch API fixes + client validation fixes
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3232:
URL: https://github.com/apache/fineract/pull/3232#discussion_r1219682213
##########
fineract-provider/src/main/java/org/apache/fineract/batch/exception/ErrorHandler.java:
##########
@@ -99,13 +99,13 @@ public static ErrorInfo handler(final RuntimeException exception) {
return handleException(e, new PlatformDomainRuleExceptionMapper(), 9999);
}
if (exception instanceof TransactionException) {
- return new ErrorInfo(HttpStatus.SC_BAD_REQUEST, 4001, "{\"Exception\": %s}".formatted(exception.getMessage()));
+ return new ErrorInfo(HttpStatus.SC_INTERNAL_SERVER_ERROR, 4001, "{\"Exception\": %s}".formatted(exception.getMessage()));
Review Comment:
if you are changing this for 5xx error, you should change the code as well (4001 -> 5001?)
--
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