From 82eb474b0b55a025d3a6278b6389c7e847a4e087 Mon Sep 17 00:00:00 2001 From: "Rym.Ghosn" Date: Wed, 19 Aug 2026 11:30:08 +0300 Subject: [PATCH] FINERACT-2769: Allow editing a tax group component's start date before it takes effect Tax group updates could never carry a start date change through: the JSON-level id-vs-taxComponentId mismatch made the validator require a new taxComponentId even when only an existing mapping's id was submitted, and the write path always discarded any submitted start date. This also fixes a latent bug where adding a new component to an existing group left its tax_group_id foreign key unset. An existing component's start date can now be changed as long as it has not taken effect yet (its current start date is still in the future), and the group/component read side exposes groupEditable/componentEditable so clients can tell which fields are still open for editing. --- .../charge/domain/ChargeRepository.java | 3 ++ .../portfolio/tax/data/TaxGroupData.java | 17 +++++++++-- .../tax/data/TaxGroupMappingsData.java | 4 +++ .../portfolio/tax/service/TaxAssembler.java | 7 +++-- .../service/TaxReadPlatformServiceImpl.java | 30 ++++++++++++++++++- .../tax/starter/TaxConfiguration.java | 6 ++-- .../tax/api/TaxGroupApiResourceSwagger.java | 8 +++++ .../portfolio/tax/domain/TaxGroup.java | 3 +- .../tax/domain/TaxGroupMappings.java | 13 ++++++-- .../portfolio/tax/mapper/TaxGroupMapper.java | 1 + .../tax/mapper/TaxGroupMappingsMapper.java | 2 ++ .../tax/serialization/TaxValidator.java | 27 ++++++++++++++--- 12 files changed, 104 insertions(+), 17 deletions(-) diff --git a/fineract-charge/src/main/java/org/apache/fineract/portfolio/charge/domain/ChargeRepository.java b/fineract-charge/src/main/java/org/apache/fineract/portfolio/charge/domain/ChargeRepository.java index 2c9129ab952..8e62e2231cc 100644 --- a/fineract-charge/src/main/java/org/apache/fineract/portfolio/charge/domain/ChargeRepository.java +++ b/fineract-charge/src/main/java/org/apache/fineract/portfolio/charge/domain/ChargeRepository.java @@ -28,4 +28,7 @@ public interface ChargeRepository extends JpaRepository, JpaSpecif @Query("select lc.id from WorkingCapitalLoanCharge lc where lc.charge.id = :chargeId and lc.active = true") Optional isAnyWorkingCapitalLoansAssociateWithThisCharge(@Param("chargeId") Long chargeId); + + @Query("select case when count(c) > 0 then true else false end from Charge c where c.taxGroup.id = :taxGroupId") + boolean existsByTaxGroupId(@Param("taxGroupId") Long taxGroupId); } diff --git a/fineract-core/src/main/java/org/apache/fineract/portfolio/tax/data/TaxGroupData.java b/fineract-core/src/main/java/org/apache/fineract/portfolio/tax/data/TaxGroupData.java index 5f9804536f6..93a30b31c2e 100644 --- a/fineract-core/src/main/java/org/apache/fineract/portfolio/tax/data/TaxGroupData.java +++ b/fineract-core/src/main/java/org/apache/fineract/portfolio/tax/data/TaxGroupData.java @@ -35,21 +35,32 @@ public final class TaxGroupData implements Serializable { @SuppressWarnings("unused") private final Collection taxComponents; + // Edit control flag: false if tax group is linked to charge products and none of its components has a + // not-yet-effective start date, true otherwise + @SuppressWarnings("unused") + private final Boolean groupEditable; + public static TaxGroupData lookup(final Long id, final String name) { final Collection taxComponents = null; final Collection taxAssociations = null; - return new TaxGroupData(id, name, taxAssociations, taxComponents); + return new TaxGroupData(id, name, taxAssociations, taxComponents, null); } public static TaxGroupData template(final Collection taxComponents) { final Long id = null; final String name = null; final Collection taxAssociations = null; - return new TaxGroupData(id, name, taxAssociations, taxComponents); + return new TaxGroupData(id, name, taxAssociations, taxComponents, null); } public static TaxGroupData template(final TaxGroupData taxGroupData, final Collection taxComponents) { - return new TaxGroupData(taxGroupData.id, taxGroupData.name, taxGroupData.taxAssociations, taxComponents); + return new TaxGroupData(taxGroupData.id, taxGroupData.name, taxGroupData.taxAssociations, taxComponents, + taxGroupData.groupEditable); + } + + public static TaxGroupData template(final TaxGroupData taxGroupData, final Collection taxComponents, + final Boolean groupEditable) { + return new TaxGroupData(taxGroupData.id, taxGroupData.name, taxGroupData.taxAssociations, taxComponents, groupEditable); } } diff --git a/fineract-core/src/main/java/org/apache/fineract/portfolio/tax/data/TaxGroupMappingsData.java b/fineract-core/src/main/java/org/apache/fineract/portfolio/tax/data/TaxGroupMappingsData.java index 643a89eebbf..2f461b00ea5 100644 --- a/fineract-core/src/main/java/org/apache/fineract/portfolio/tax/data/TaxGroupMappingsData.java +++ b/fineract-core/src/main/java/org/apache/fineract/portfolio/tax/data/TaxGroupMappingsData.java @@ -38,6 +38,10 @@ public class TaxGroupMappingsData implements Serializable { private final LocalDate startDate; @SuppressWarnings("unused") private final LocalDate endDate; + // Edit control flag: true if this component's start date is in the future, false otherwise. + // Only meaningful when the owning TaxGroupData's groupEditable is true. + @SuppressWarnings("unused") + private Boolean componentEditable; public boolean occursOnDayFromAndUpToAndIncluding(final LocalDate target) { return DateUtils.isAfter(target, startDate()) && (endDate == null || !DateUtils.isAfter(target, endDate())); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/service/TaxAssembler.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/service/TaxAssembler.java index 8b558e80722..acbfdea77d6 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/service/TaxAssembler.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/service/TaxAssembler.java @@ -131,12 +131,13 @@ public Set assembleTaxGroupMappingsFrom(final JsonCommand comm dateFormat, locale); final LocalDate endDate = this.fromApiJsonHelper.extractLocalDateNamed(TaxApiConstants.endDateParamName, taxComponent, dateFormat, locale); - if (endDate == null && startDate == null) { + final boolean isExistingMapping = isUpdate && mappingId != null; + if (endDate == null && startDate == null && !isExistingMapping) { startDate = DateUtils.getBusinessLocalDate(); } TaxGroupMappings mappings = null; - if (isUpdate && mappingId != null) { - mappings = TaxGroupMappings.createTaxGroupMappings(mappingId, component, endDate); + if (isExistingMapping) { + mappings = TaxGroupMappings.createTaxGroupMappings(mappingId, component, startDate, endDate); } else { mappings = TaxGroupMappings.createTaxGroupMappings(component, startDate); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/service/TaxReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/service/TaxReadPlatformServiceImpl.java index 9c4bc698a06..724a7c473fe 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/service/TaxReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/service/TaxReadPlatformServiceImpl.java @@ -18,12 +18,16 @@ */ package org.apache.fineract.portfolio.tax.service; +import java.time.LocalDate; import java.util.Collection; import java.util.List; import lombok.RequiredArgsConstructor; import org.apache.fineract.accounting.common.AccountingDropdownReadPlatformService; +import org.apache.fineract.infrastructure.core.service.DateUtils; +import org.apache.fineract.portfolio.charge.domain.ChargeRepository; import org.apache.fineract.portfolio.tax.data.TaxComponentData; import org.apache.fineract.portfolio.tax.data.TaxGroupData; +import org.apache.fineract.portfolio.tax.data.TaxGroupMappingsData; import org.apache.fineract.portfolio.tax.domain.TaxComponentRepository; import org.apache.fineract.portfolio.tax.domain.TaxComponentRepositoryWrapper; import org.apache.fineract.portfolio.tax.domain.TaxGroupRepository; @@ -41,6 +45,7 @@ public class TaxReadPlatformServiceImpl implements TaxReadPlatformService { private final TaxGroupRepository taxGroupRepository; private final TaxGroupRepositoryWrapper taxGroupRepositoryWrapper; private final TaxGroupMapper taxGroupMapper; + private final ChargeRepository chargeRepository; @Override public List retrieveAllTaxComponents() { @@ -71,7 +76,30 @@ public TaxGroupData retrieveTaxGroupData(final Long id) { @Override public TaxGroupData retrieveTaxGroupWithTemplate(final Long id) { TaxGroupData taxGroupData = retrieveTaxGroupData(id); - taxGroupData = TaxGroupData.template(taxGroupData, retrieveTaxComponentsForLookUp()); + + final boolean isLinked = chargeRepository.existsByTaxGroupId(id); + final LocalDate today = DateUtils.getBusinessLocalDate(); + + // A linked group is only editable if at least one of its components has not taken effect yet. + Boolean groupEditable = !isLinked; + if (isLinked && taxGroupData.getTaxAssociations() != null) { + for (TaxGroupMappingsData mapping : taxGroupData.getTaxAssociations()) { + if (mapping.getStartDate() != null && mapping.getStartDate().isAfter(today)) { + groupEditable = true; + break; + } + } + } + + if (taxGroupData.getTaxAssociations() != null) { + for (TaxGroupMappingsData mapping : taxGroupData.getTaxAssociations()) { + final boolean componentEditable = Boolean.TRUE.equals(groupEditable) && mapping.getStartDate() != null + && mapping.getStartDate().isAfter(today); + mapping.setComponentEditable(componentEditable); + } + } + + taxGroupData = TaxGroupData.template(taxGroupData, retrieveTaxComponentsForLookUp(), groupEditable); return taxGroupData; } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/starter/TaxConfiguration.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/starter/TaxConfiguration.java index ea566802ec7..ed96de8d71b 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/starter/TaxConfiguration.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/tax/starter/TaxConfiguration.java @@ -21,6 +21,7 @@ import org.apache.fineract.accounting.common.AccountingDropdownReadPlatformService; import org.apache.fineract.accounting.glaccount.domain.GLAccountRepositoryWrapper; import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper; +import org.apache.fineract.portfolio.charge.domain.ChargeRepository; import org.apache.fineract.portfolio.tax.domain.TaxComponentRepository; import org.apache.fineract.portfolio.tax.domain.TaxComponentRepositoryWrapper; import org.apache.fineract.portfolio.tax.domain.TaxGroupRepository; @@ -52,9 +53,10 @@ public TaxAssembler taxAssembler(FromJsonHelper fromApiJsonHelper, GLAccountRepo public TaxReadPlatformService taxReadPlatformService(final TaxComponentRepository taxComponentRepository, final TaxComponentRepositoryWrapper taxComponentRepositoryWrapper, final TaxComponentMapper taxComponentMapper, final TaxGroupRepository taxGroupRepository, final TaxGroupRepositoryWrapper taxGroupRepositoryWrapper, - final TaxGroupMapper taxGroupMapper, AccountingDropdownReadPlatformService accountingDropdownReadPlatformService) { + final TaxGroupMapper taxGroupMapper, AccountingDropdownReadPlatformService accountingDropdownReadPlatformService, + final ChargeRepository chargeRepository) { return new TaxReadPlatformServiceImpl(accountingDropdownReadPlatformService, taxComponentRepository, taxComponentRepositoryWrapper, - taxComponentMapper, taxGroupRepository, taxGroupRepositoryWrapper, taxGroupMapper); + taxComponentMapper, taxGroupRepository, taxGroupRepositoryWrapper, taxGroupMapper, chargeRepository); } @Bean diff --git a/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/api/TaxGroupApiResourceSwagger.java b/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/api/TaxGroupApiResourceSwagger.java index fbd67ae75d6..1388a7f0a35 100644 --- a/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/api/TaxGroupApiResourceSwagger.java +++ b/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/api/TaxGroupApiResourceSwagger.java @@ -54,6 +54,8 @@ private GetTaxesGroupTaxComponent() {} public GetTaxesGroupTaxComponent taxComponent; @Schema(example = "[2016, 4, 11]") public LocalDate startDate; + @Schema(example = "true", description = "Only present on the template response. True if this component's start date is in the future and can still be edited.") + public Boolean componentEditable; } @Schema(example = "7") @@ -61,6 +63,8 @@ private GetTaxesGroupTaxComponent() {} @Schema(example = "tax group 1") public String name; public Set taxAssociations; + @Schema(example = "true", description = "Only present on the template response. False if the group is linked to charge products and none of its components can still be edited.") + public Boolean groupEditable; } @Schema(description = "PostTaxesGroupRequest") @@ -109,6 +113,8 @@ private PutTaxesGroupTaxComponents() {} public Long id; @Schema(example = "7") public Long taxComponentId; + @Schema(example = "22 April 2016", description = "Only accepted when this component's current start date has not taken effect yet, i.e. is still in the future.") + public String startDate; @Schema(example = "22 April 2016") public String endDate; } @@ -135,6 +141,8 @@ static final class PutTaxesGroupModifiedComponents { private PutTaxesGroupModifiedComponents() {} + @Schema(example = "Apr 22, 2016 12:00:00 AM") + public String startDate; @Schema(example = "Apr 22, 2016 12:00:00 AM") public String endDate; @Schema(example = "7") diff --git a/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/domain/TaxGroup.java b/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/domain/TaxGroup.java index b03ceae25ad..12a1a079dec 100644 --- a/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/domain/TaxGroup.java +++ b/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/domain/TaxGroup.java @@ -75,10 +75,11 @@ public Map update(final JsonCommand command, final Set> changes) { + public void update(final LocalDate startDate, final LocalDate endDate, final List> changes) { + if (startDate != null && !DateUtils.isEqual(startDate, this.startDate)) { + this.startDate = startDate; + Map map = new HashMap<>(2); + map.put(TaxApiConstants.startDateParamName, startDate); + map.put(TaxApiConstants.taxComponentIdParamName, this.getTaxComponent().getId()); + changes.add(map); + } if (endDate != null && this.endDate == null) { this.endDate = endDate; Map map = new HashMap<>(2); diff --git a/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/mapper/TaxGroupMapper.java b/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/mapper/TaxGroupMapper.java index 3edadeef55b..21e831af1c0 100644 --- a/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/mapper/TaxGroupMapper.java +++ b/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/mapper/TaxGroupMapper.java @@ -30,6 +30,7 @@ public interface TaxGroupMapper { @Mapping(target = "taxAssociations", source = "taxGroup.taxGroupMappings") @Mapping(target = "taxComponents", ignore = true) + @Mapping(target = "groupEditable", ignore = true) TaxGroupData map(TaxGroup taxGroup); List map(List taxGroups); diff --git a/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/mapper/TaxGroupMappingsMapper.java b/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/mapper/TaxGroupMappingsMapper.java index 4afc643fe3a..4ff78a70c92 100644 --- a/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/mapper/TaxGroupMappingsMapper.java +++ b/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/mapper/TaxGroupMappingsMapper.java @@ -23,10 +23,12 @@ import org.apache.fineract.portfolio.tax.data.TaxGroupMappingsData; import org.apache.fineract.portfolio.tax.domain.TaxGroupMappings; import org.mapstruct.Mapper; +import org.mapstruct.Mapping; @Mapper(config = MapstructMapperConfig.class, uses = { TaxComponentMapper.class }) public interface TaxGroupMappingsMapper { + @Mapping(target = "componentEditable", ignore = true) TaxGroupMappingsData map(TaxGroupMappings taxGroupMapping); List map(List taxGroupMappings); diff --git a/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/serialization/TaxValidator.java b/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/serialization/TaxValidator.java index 05a2ea87933..0be65d7a64e 100644 --- a/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/serialization/TaxValidator.java +++ b/fineract-tax/src/main/java/org/apache/fineract/portfolio/tax/serialization/TaxValidator.java @@ -247,8 +247,7 @@ public void validateForTaxGroupUpdate(final String json) { SUPPORTED_TAX_GROUP_TAX_COMPONENTS_UPDATE_PARAMETERS); final Long taxComponentId = this.fromApiJsonHelper.extractLongNamed(TaxApiConstants.taxComponentIdParamName, taxComponent); - final Long taxMappingId = this.fromApiJsonHelper.extractLongNamed(TaxApiConstants.taxComponentIdParamName, - taxComponent); + final Long taxMappingId = this.fromApiJsonHelper.extractLongNamed(TaxApiConstants.idParamName, taxComponent); if (taxMappingId == null) { baseDataValidator.reset().parameter( TaxApiConstants.taxComponentsParamName + DOT + TaxApiConstants.taxComponentIdParamName + AT_INDEX + i) @@ -262,13 +261,17 @@ public void validateForTaxGroupUpdate(final String json) { .value(taxMappingId).longGreaterThanZero(); } + final LocalDate today = DateUtils.getBusinessLocalDate(); final LocalDate endDate = this.fromApiJsonHelper.extractLocalDateNamed(TaxApiConstants.endDateParamName, taxComponent, dateFormat, locale); baseDataValidator.reset() .parameter(TaxApiConstants.taxComponentsParamName + DOT + TaxApiConstants.endDateParamName + AT_INDEX + i) - .value(endDate).ignoreIfNull().validateDateAfter(DateUtils.getBusinessLocalDate()); + .value(endDate).ignoreIfNull().validateDateAfter(today); final LocalDate startDate = this.fromApiJsonHelper.extractLocalDateNamed(TaxApiConstants.startDateParamName, taxComponent, dateFormat, locale); + baseDataValidator.reset() + .parameter(TaxApiConstants.taxComponentsParamName + DOT + TaxApiConstants.startDateParamName + AT_INDEX + i) + .value(startDate).ignoreIfNull().validateDateAfterOrEqual(today); if (endDate != null && startDate != null) { baseDataValidator.reset().parameter(TaxApiConstants.taxComponentsParamName + AT_INDEX + i) .failWithCode("start.date.end.date.both.should.not.be.present", startDate, endDate); @@ -282,16 +285,32 @@ public void validateForTaxGroupUpdate(final String json) { public void validateTaxGroupEndDateAndTaxComponent(final TaxGroup taxGroup, final Set groupMappings) { final List dataValidationErrors = new ArrayList<>(); final DataValidatorBuilder baseDataValidator = new DataValidatorBuilder(dataValidationErrors).resource(TAX_GROUP); + final LocalDate today = DateUtils.getBusinessLocalDate(); for (TaxGroupMappings mapping : groupMappings) { if (mapping.getId() != null) { TaxGroupMappings existing = taxGroup.findOneBy(mapping); + + // Start date can only be changed while the existing mapping has not taken effect yet; once its + // start date is on or before today, it is locked and only the end date remains editable. + LocalDate effectiveStartDate = existing.startDate(); + if (mapping.startDate() != null && !DateUtils.isEqual(mapping.startDate(), existing.startDate())) { + if (!DateUtils.isAfter(existing.startDate(), today)) { + baseDataValidator.reset().parameter(TaxApiConstants.startDateParamName) + .failWithCode("cannot.be.modified.after.activation"); + } else { + baseDataValidator.reset().parameter(TaxApiConstants.startDateParamName).value(mapping.startDate()) + .validateDateAfterOrEqual(today); + effectiveStartDate = mapping.startDate(); + } + } + if (existing.endDate() != null && mapping.endDate() != null && !DateUtils.isEqual(existing.endDate(), mapping.endDate())) { baseDataValidator.reset().parameter(TaxApiConstants.endDateParamName) .failWithCode("can.not.modify.end.date.once.updated"); } else { baseDataValidator.reset().parameter(TaxApiConstants.endDateParamName).value(mapping.endDate()).ignoreIfNull() - .validateDateAfter(existing.startDate()); + .validateDateAfter(effectiveStartDate); } if (mapping.getTaxComponent() != null && !existing.getTaxComponent().getId().equals(mapping.getTaxComponent().getId())) { baseDataValidator.reset().parameter(TaxApiConstants.taxComponentIdParamName).failWithCode("update.not.supported");