Skip to content

FINERACT-2048: Make amount/recurrence fields optional for DUES standing instruction type #5362

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

Open
Monica-CodingWorld wants to merge 1 commit into
base: develop
Choose a base branch
from
+505 −24
Open

FINERACT-2048: Make amount/recurrence fields optional for DUES standing instruction type #5362

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
111 changes: 87 additions & 24 deletions ...ain/java/org/apache/fineract/portfolio/account/data/StandingInstructionDataValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,10 @@ public void validateForCreate(final JsonCommand command) {

final BigDecimal transferAmount = this.fromApiJsonHelper
.extractBigDecimalWithLocaleNamed(StandingInstructionApiConstants.amountParamName, element);
baseDataValidator.reset().parameter(StandingInstructionApiConstants.amountParamName).value(transferAmount).positiveAmount();

if (transferAmount != null) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.amountParamName).value(transferAmount).positiveAmount();
}

final Integer transferType = this.fromApiJsonHelper.extractIntegerNamed(transferTypeParamName, element, Locale.getDefault());
baseDataValidator.reset().parameter(transferTypeParamName).value(transferType).notNull().inMinMaxRange(1, 3);
Expand All @@ -132,40 +135,56 @@ public void validateForCreate(final JsonCommand command) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.instructionTypeParamName).value(standingInstructionType)
.notNull().inMinMaxRange(1, 2);

if (isAmountRequired(standingInstructionType)) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.amountParamName).value(transferAmount).notNull();
}

final Integer recurrenceType = this.fromApiJsonHelper.extractIntegerNamed(StandingInstructionApiConstants.recurrenceTypeParamName,
element, Locale.getDefault());
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceTypeParamName).value(recurrenceType).notNull()
.inMinMaxRange(1, 2);

if (isRecurrenceRequired(standingInstructionType)) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceTypeParamName).value(recurrenceType).notNull()
.inMinMaxRange(1, 2);
}

boolean isPeriodic = false;
if (recurrenceType != null) {
isPeriodic = AccountTransferRecurrenceType.fromInt(recurrenceType).isPeriodicRecurrence();
}

final Integer recurrenceFrequency = this.fromApiJsonHelper
.extractIntegerNamed(StandingInstructionApiConstants.recurrenceFrequencyParamName, element, Locale.getDefault());
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceFrequencyParamName).value(recurrenceFrequency)
.inMinMaxRange(0, 3);

if (isRecurrenceRequired(standingInstructionType)) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceFrequencyParamName).value(recurrenceFrequency)
.inMinMaxRange(1, 3);
}

if (recurrenceFrequency != null) {
PeriodFrequencyType frequencyType = PeriodFrequencyType.fromInt(recurrenceFrequency);
if (frequencyType.isMonthly() || frequencyType.isYearly()) {
if (frequencyType != null && (frequencyType.isMonthly() || frequencyType.isYearly())) {
final MonthDay monthDay = this.fromApiJsonHelper
.extractMonthDayNamed(StandingInstructionApiConstants.recurrenceOnMonthDayParamName, element);
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceOnMonthDayParamName).value(monthDay)
.notNull();

if (isRecurrenceRequired(standingInstructionType)) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceOnMonthDayParamName).value(monthDay)
.notNull();
}
}
}

final Integer recurrenceInterval = this.fromApiJsonHelper
.extractIntegerNamed(StandingInstructionApiConstants.recurrenceIntervalParamName, element, Locale.getDefault());
if (isPeriodic) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceIntervalParamName).value(recurrenceInterval)
.notNull();
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceFrequencyParamName).value(recurrenceFrequency)
.notNull();
if (isRecurrenceRequired(standingInstructionType)) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceIntervalParamName).value(recurrenceInterval)
Copy link
Contributor

@Aman-Mittal Aman-Mittal Jan 22, 2026

Choose a reason for hiding this comment

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

This seems a little bit complicated. and if conditions can be merged, also of there is any business rule that is implemented make sure to document that helper function

Business rules should be explicit and self-documenting

while the Github Action is not run here is a pro tip you can run the check on your forked repo and add testcase for more coverage

Copy link
Contributor Author

@Monica-CodingWorld Monica-CodingWorld Jan 26, 2026

Choose a reason for hiding this comment

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

@Aman-Mittal Thank you so much for your guidance and suggestions. I will improve the PR and update shortly.

.notNull();
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceFrequencyParamName).value(recurrenceFrequency)
.notNull();
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceIntervalParamName).value(recurrenceInterval)
.integerGreaterThanZero();
}
}
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceIntervalParamName).value(recurrenceInterval)
.integerGreaterThanZero();

final String name = this.fromApiJsonHelper.extractStringNamed(StandingInstructionApiConstants.nameParamName, element);
baseDataValidator.reset().parameter(StandingInstructionApiConstants.nameParamName).value(name).notNull();
Expand All @@ -178,7 +197,7 @@ public void validateForCreate(final JsonCommand command) {
.inMinMaxRange(1, 1);

}
if (standingInstructionType != null && StandingInstructionType.fromInt(standingInstructionType).isFixedAmoutTransfer()) {
if (isAmountRequired(standingInstructionType)) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.amountParamName).value(transferAmount).notNull();
}

Expand Down Expand Up @@ -230,10 +249,22 @@ public void validateForUpdate(final JsonCommand command) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.validTillParamName).value(validTill).notNull();
}

Integer standingInstructionType = null;
if (this.fromApiJsonHelper.parameterExists(StandingInstructionApiConstants.instructionTypeParamName, element)) {
standingInstructionType = this.fromApiJsonHelper.extractIntegerNamed(StandingInstructionApiConstants.instructionTypeParamName,
element, Locale.getDefault());
baseDataValidator.reset().parameter(StandingInstructionApiConstants.instructionTypeParamName).value(standingInstructionType)
.notNull().inMinMaxRange(1, 2);
}

if (this.fromApiJsonHelper.parameterExists(StandingInstructionApiConstants.amountParamName, element)) {
final BigDecimal transferAmount = this.fromApiJsonHelper
.extractBigDecimalWithLocaleNamed(StandingInstructionApiConstants.amountParamName, element);
baseDataValidator.reset().parameter(StandingInstructionApiConstants.amountParamName).value(transferAmount).positiveAmount();

if (isAmountRequired(standingInstructionType)) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.amountParamName).value(transferAmount).notNull();
}
}

if (this.fromApiJsonHelper.parameterExists(StandingInstructionApiConstants.statusParamName, element)) {
Expand All @@ -250,13 +281,6 @@ public void validateForUpdate(final JsonCommand command) {
.inMinMaxRange(1, 4);
}

if (this.fromApiJsonHelper.parameterExists(StandingInstructionApiConstants.instructionTypeParamName, element)) {
final Integer standingInstructionType = this.fromApiJsonHelper
.extractIntegerNamed(StandingInstructionApiConstants.instructionTypeParamName, element, Locale.getDefault());
baseDataValidator.reset().parameter(StandingInstructionApiConstants.instructionTypeParamName).value(standingInstructionType)
.notNull().inMinMaxRange(1, 2);
}

if (this.fromApiJsonHelper.parameterExists(StandingInstructionApiConstants.recurrenceTypeParamName, element)) {
final Integer recurrenceType = this.fromApiJsonHelper
.extractIntegerNamed(StandingInstructionApiConstants.recurrenceTypeParamName, element, Locale.getDefault());
Expand All @@ -274,8 +298,11 @@ public void validateForUpdate(final JsonCommand command) {
if (this.fromApiJsonHelper.parameterExists(StandingInstructionApiConstants.recurrenceIntervalParamName, element)) {
final Integer recurrenceInterval = this.fromApiJsonHelper
.extractIntegerNamed(StandingInstructionApiConstants.recurrenceIntervalParamName, element, Locale.getDefault());
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceIntervalParamName).value(recurrenceInterval)
.integerGreaterThanZero();

if (isRecurrenceRequired(standingInstructionType)) {
baseDataValidator.reset().parameter(StandingInstructionApiConstants.recurrenceIntervalParamName).value(recurrenceInterval)
.integerGreaterThanZero();
}
}

if (this.fromApiJsonHelper.parameterExists(StandingInstructionApiConstants.nameParamName, element)) {
Expand All @@ -291,4 +318,40 @@ private void throwExceptionIfValidationWarningsExist(final List<ApiParameterErro
throw new PlatformApiDataValidationException(dataValidationErrors);
}
}

// ============================================
// PRIVATE HELPER METHODS
// ============================================

/**
* Determines if the standing instruction type requires amount validation. Amount is required only for
* FIXED_AMOUNT_TRANSFER type. For DUES type, amount is calculated based on outstanding dues.
*
* @param standingInstructionType
* the instruction type code
* @return true if amount validation is required, false otherwise
*/
private boolean isAmountRequired(Integer standingInstructionType) {
if (standingInstructionType == null) {
return false;
}
StandingInstructionType type = StandingInstructionType.fromInt(standingInstructionType);
return type != null && type.isFixedAmoutTransfer();
}

/**
* Determines if recurrence fields should be validated. Recurrence is required for all types except DUES. DUES
* instructions are processed based on loan due dates, not recurrence pattern.
*
* @param standingInstructionType
* the instruction type code
* @return true if recurrence validation is required, false otherwise
*/
private boolean isRecurrenceRequired(Integer standingInstructionType) {
if (standingInstructionType == null) {
return true;
}
StandingInstructionType type = StandingInstructionType.fromInt(standingInstructionType);
return type != null && !type.isDuesAmoutTransfer();
}
}
Loading
Loading