Skip to content

FINERACT-2765: Tax Component and Tax Group names should be unique #6286

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
rymghosn wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

FINERACT-2765: Tax Component and Tax Group names should be unique #6286

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
68 changes: 67 additions & 1 deletion .../src/main/java/org/apache/fineract/portfolio/tax/service/TaxWritePlatformServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,18 @@
*/
package org.apache.fineract.portfolio.tax.service;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import lombok.RequiredArgsConstructor;
import org.apache.fineract.infrastructure.core.api.JsonCommand;
import org.apache.fineract.infrastructure.core.data.ApiParameterError;
import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
import org.apache.fineract.infrastructure.core.data.CommandProcessingResultBuilder;
import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
import org.apache.fineract.portfolio.tax.api.TaxApiConstants;
import org.apache.fineract.portfolio.tax.domain.TaxComponent;
import org.apache.fineract.portfolio.tax.domain.TaxComponentRepository;
import org.apache.fineract.portfolio.tax.domain.TaxComponentRepositoryWrapper;
Expand All @@ -47,6 +53,21 @@ public class TaxWritePlatformServiceImpl implements TaxWritePlatformService {
public CommandProcessingResult createTaxComponent(final JsonCommand command) {
this.validator.validateForTaxComponentCreate(command.json());
TaxComponent taxComponent = this.taxAssembler.assembleTaxComponentFrom(command);

// Enforce unique tax component name (case-sensitive, relying on DB collation for case handling)
if (taxComponent.getName() != null) {
Optional<TaxComponent> existing = this.taxComponentRepository.findByName(taxComponent.getName());
if (existing.isPresent()) {
final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
String errorCode = "tax.component.name.must.be.unique";
String userMessage = "Tax component with name '" + taxComponent.getName() + "' already exists";
ApiParameterError error = ApiParameterError.parameterError(errorCode, userMessage, TaxApiConstants.nameParamName,
taxComponent.getName());
dataValidationErrors.add(error);
throw new PlatformApiDataValidationException(errorCode, userMessage, dataValidationErrors);
}
}

this.taxComponentRepository.saveAndFlush(taxComponent);
return new CommandProcessingResultBuilder() //
.withCommandId(command.commandId()) //
Expand All @@ -56,9 +77,27 @@ public CommandProcessingResult createTaxComponent(final JsonCommand command) {

@Override
public CommandProcessingResult updateTaxComponent(final Long id, final JsonCommand command) {
this.validator.validateForTaxComponentUpdate(command.json());
this.validator.validateForTaxComponentUpdate(command.json(), id);
final TaxComponent taxComponent = this.taxComponentRepositoryWrapper.findOneWithNotFoundDetection(id);
this.validator.validateStartDate(taxComponent.startDate(), command);

// Enforce unique tax component name when name is being changed
if (command.parameterExists(TaxApiConstants.nameParamName)) {
final String newName = command.stringValueOfParameterNamed(TaxApiConstants.nameParamName);
if (newName != null && !newName.equalsIgnoreCase(taxComponent.getName())) {
Optional<TaxComponent> existing = this.taxComponentRepository.findByName(newName);
if (existing.isPresent() && !existing.get().getId().equals(taxComponent.getId())) {
final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
String errorCode = "tax.component.name.must.be.unique";
String userMessage = "Tax component with name '" + newName + "' already exists";
ApiParameterError error = ApiParameterError.parameterError(errorCode, userMessage, TaxApiConstants.nameParamName,
newName);
dataValidationErrors.add(error);
throw new PlatformApiDataValidationException(errorCode, userMessage, dataValidationErrors);
}
}
}

Map<String, Object> changes = taxComponent.update(command);
this.validator.validateTaxComponentForUpdate(taxComponent);
this.taxComponentRepository.saveAndFlush(taxComponent);
Expand All @@ -72,6 +111,18 @@ public CommandProcessingResult updateTaxComponent(final Long id, final JsonComma
public CommandProcessingResult createTaxGroup(final JsonCommand command) {
this.validator.validateForTaxGroupCreate(command.json());
final TaxGroup taxGroup = this.taxAssembler.assembleTaxGroupFrom(command);

// Enforce unique tax group name (case-insensitive)
if (taxGroup.getName() != null && this.taxGroupRepository.existsByNameIgnoreCase(taxGroup.getName())) {
final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
String errorCode = "tax.group.name.must.be.unique";
String userMessage = "Tax group with name '" + taxGroup.getName() + "' already exists";
ApiParameterError error = ApiParameterError.parameterError(errorCode, userMessage, TaxApiConstants.nameParamName,
taxGroup.getName());
dataValidationErrors.add(error);
throw new PlatformApiDataValidationException(errorCode, userMessage, dataValidationErrors);
}

this.validator.validateTaxGroup(taxGroup);
this.taxGroupRepository.saveAndFlush(taxGroup);
return new CommandProcessingResultBuilder() //
Expand All @@ -84,6 +135,21 @@ public CommandProcessingResult createTaxGroup(final JsonCommand command) {
public CommandProcessingResult updateTaxGroup(final Long id, final JsonCommand command) {
this.validator.validateForTaxGroupUpdate(command.json());
final TaxGroup taxGroup = this.taxGroupRepositoryWrapper.findOneWithNotFoundDetection(id);

// Validate unique tax group name (case-insensitive) BEFORE updating if name is being changed
if (command.parameterExists(TaxApiConstants.nameParamName)) {
final String newName = command.stringValueOfParameterNamed(TaxApiConstants.nameParamName);
if (newName != null && !newName.equalsIgnoreCase(taxGroup.getName())
&& this.taxGroupRepository.existsByNameIgnoreCaseAndIdNot(newName, id)) {
final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
String errorCode = "tax.group.name.must.be.unique";
String userMessage = "Tax group with name '" + newName + "' already exists";
ApiParameterError error = ApiParameterError.parameterError(errorCode, userMessage, TaxApiConstants.nameParamName, newName);
dataValidationErrors.add(error);
throw new PlatformApiDataValidationException(errorCode, userMessage, dataValidationErrors);
}
}

final boolean isUpdate = true;
Set<TaxGroupMappings> groupMappings = this.taxAssembler.assembleTaxGroupMappingsFrom(command, isUpdate);
this.validator.validateTaxGroupEndDateAndTaxComponent(taxGroup, groupMappings);
Expand Down
6 changes: 6 additions & 0 deletions ...ct-provider/src/main/java/org/apache/fineract/portfolio/tax/starter/TaxConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@
@Configuration
public class TaxConfiguration {

@Bean
@ConditionalOnMissingBean(TaxValidator.class)
public TaxValidator taxValidator(FromJsonHelper fromApiJsonHelper, TaxComponentRepository taxComponentRepository) {
return new TaxValidator(fromApiJsonHelper, taxComponentRepository);
}

@Bean
@ConditionalOnMissingBean(TaxAssembler.class)
public TaxAssembler taxAssembler(FromJsonHelper fromApiJsonHelper, GLAccountRepositoryWrapper glAccountRepositoryWrapper,
Expand Down
3 changes: 3 additions & 0 deletions ...ct-tax/src/main/java/org/apache/fineract/portfolio/tax/domain/TaxComponentRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@
*/
package org.apache.fineract.portfolio.tax.domain;

import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;

public interface TaxComponentRepository extends JpaRepository<TaxComponent, Long>, JpaSpecificationExecutor<TaxComponent> {

Optional<TaxComponent> findByName(String name);

}
6 changes: 6 additions & 0 deletions fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/domain/TaxGroupRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,10 @@

public interface TaxGroupRepository extends JpaRepository<TaxGroup, Long>, JpaSpecificationExecutor<TaxGroup> {

// Check if any TaxGroup exists with the given name
boolean existsByNameIgnoreCase(String name);

// Check if any other TaxGroup (excluding the given id) exists with the same name
boolean existsByNameIgnoreCaseAndIdNot(String name, Long id);

}
50 changes: 49 additions & 1 deletion fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/serialization/TaxValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import org.apache.commons.lang3.StringUtils;
import org.apache.fineract.accounting.glaccount.domain.GLAccountType;
Expand All @@ -44,6 +45,7 @@
import org.apache.fineract.infrastructure.core.service.DateUtils;
import org.apache.fineract.portfolio.tax.api.TaxApiConstants;
import org.apache.fineract.portfolio.tax.domain.TaxComponent;
import org.apache.fineract.portfolio.tax.domain.TaxComponentRepository;
import org.apache.fineract.portfolio.tax.domain.TaxGroup;
import org.apache.fineract.portfolio.tax.domain.TaxGroupMappings;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -77,10 +79,12 @@ public class TaxValidator {
public static final String DOT = ".";
public static final String AT_INDEX = ".at.index.";
private final FromJsonHelper fromApiJsonHelper;
private final TaxComponentRepository taxComponentRepository;

@Autowired
public TaxValidator(final FromJsonHelper fromApiJsonHelper) {
public TaxValidator(final FromJsonHelper fromApiJsonHelper, final TaxComponentRepository taxComponentRepository) {
this.fromApiJsonHelper = fromApiJsonHelper;
this.taxComponentRepository = taxComponentRepository;
}

public void validateForTaxComponentCreate(final String json) {
Expand All @@ -101,6 +105,14 @@ public void validateForTaxComponentCreate(final String json) {
final String name = this.fromApiJsonHelper.extractStringNamed(TaxApiConstants.nameParamName, element);
baseDataValidator.reset().parameter(TaxApiConstants.nameParamName).value(name).notBlank();

// Check for duplicate name
if (name != null && this.taxComponentRepository.findByName(name).isPresent()) {
String errorCode = "tax.component.name.must.be.unique";
String userMessage = "Tax component with name '" + name + "' already exists";
ApiParameterError error = ApiParameterError.parameterError(errorCode, userMessage, TaxApiConstants.nameParamName, name);
dataValidationErrors.add(error);
}

final BigDecimal percentage = this.fromApiJsonHelper.extractBigDecimalWithLocaleNamed(TaxApiConstants.percentageParamName, element);
baseDataValidator.reset().parameter(TaxApiConstants.percentageParamName).value(percentage).notBlank().positiveAmount()
.notGreaterThanMax(BigDecimal.valueOf(100));
Expand Down Expand Up @@ -134,6 +146,10 @@ public void validateForTaxComponentCreate(final String json) {
}

public void validateForTaxComponentUpdate(final String json) {
validateForTaxComponentUpdate(json, null);
}

public void validateForTaxComponentUpdate(final String json, final Long taxComponentId) {
if (StringUtils.isBlank(json)) {
throw new InvalidJsonException();
}
Expand All @@ -151,6 +167,19 @@ public void validateForTaxComponentUpdate(final String json) {
if (this.fromApiJsonHelper.parameterExists(TaxApiConstants.nameParamName, element)) {
final String name = this.fromApiJsonHelper.extractStringNamed(TaxApiConstants.nameParamName, element);
baseDataValidator.reset().parameter(TaxApiConstants.nameParamName).value(name).notBlank();

// Check for duplicate name (excluding current entity)
if (name != null && taxComponentId != null) {
this.taxComponentRepository.findByName(name).ifPresent(existingComponent -> {
if (!Objects.equals(existingComponent.getId(), taxComponentId)) {
String errorCode = "tax.component.name.must.be.unique";
String userMessage = "Tax component with name '" + name + "' already exists";
ApiParameterError error = ApiParameterError.parameterError(errorCode, userMessage, TaxApiConstants.nameParamName,
name);
dataValidationErrors.add(error);
}
});
}
}

if (this.fromApiJsonHelper.parameterExists(TaxApiConstants.percentageParamName, element)) {
Expand Down Expand Up @@ -197,6 +226,10 @@ public void validateForTaxGroupCreate(final String json) {
"Please add at least one Tax Component before submitting the Tax Group.", TaxApiConstants.taxComponentsParamName));
}
baseDataValidator.reset().parameter(TaxApiConstants.taxComponentsParamName).value(array.size()).integerGreaterThanZero();

// Ensure each taxComponentId appears at most once
Set<Long> seenComponentIds = new HashSet<>();

for (int i = 1; i <= array.size(); i++) {
final JsonObject taxComponent = array.get(i - 1).getAsJsonObject();
final String arrayObjectJson = this.fromApiJsonHelper.toJson(taxComponent);
Expand All @@ -207,6 +240,11 @@ public void validateForTaxGroupCreate(final String json) {
.parameterAtIndexArray(TaxApiConstants.taxComponentIdParamName, i).value(taxComponentId).notNull()
.longGreaterThanZero();

if (taxComponentId != null && !seenComponentIds.add(taxComponentId)) {
dataValidationErrors.add(ApiParameterError.parameterError("validation.msg.tax.group.duplicate.component",
"Each tax component can be included only once in a tax group.",
TaxApiConstants.taxComponentsParamName + DOT + TaxApiConstants.taxComponentIdParamName + AT_INDEX + i));
}
}
}
throwExceptionIfValidationWarningsExist(dataValidationErrors);
Expand Down Expand Up @@ -240,6 +278,10 @@ public void validateForTaxGroupUpdate(final String json) {
final Locale locale = this.fromApiJsonHelper.extractLocaleParameter(topLevelJsonElement);
if (topLevelJsonElement.get(TaxApiConstants.taxComponentsParamName).isJsonArray()) {
final JsonArray array = topLevelJsonElement.get(TaxApiConstants.taxComponentsParamName).getAsJsonArray();

// Ensure each taxComponentId appears at most once in update payload
Set<Long> seenComponentIds = new HashSet<>();

for (int i = 1; i <= array.size(); i++) {
final JsonObject taxComponent = array.get(i - 1).getAsJsonObject();
final String arrayObjectJson = this.fromApiJsonHelper.toJson(taxComponent);
Expand All @@ -262,6 +304,12 @@ public void validateForTaxGroupUpdate(final String json) {
.value(taxMappingId).longGreaterThanZero();
}

if (taxComponentId != null && !seenComponentIds.add(taxComponentId)) {
dataValidationErrors.add(ApiParameterError.parameterError("validation.msg.tax.group.duplicate.component",
"Each tax component can be included only once in a tax group.",
TaxApiConstants.taxComponentsParamName + DOT + TaxApiConstants.taxComponentIdParamName + AT_INDEX + i));
}

final LocalDate endDate = this.fromApiJsonHelper.extractLocalDateNamed(TaxApiConstants.endDateParamName, taxComponent,
dateFormat, locale);
baseDataValidator.reset()
Expand Down