MIFOSX-90: disallow ability to remove loan charge if it is waived (or partially waived).

mifosng-provider/src/main/java/org/mifosng/platform/api/LoansApiResource.java

@@ -507,16 +507,16 @@ public class LoansApiResource {
     @Path("{loanId}/charges/{chargeId}")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-    public Response updateLoanCharge(
+    public String updateLoanCharge(
             @PathParam("loanId") final Long loanId,
             @PathParam("chargeId") final Long loanChargeId,
             final String jsonRequestBody){
 
-        LoanChargeCommand command = this.apiDataConversionService.convertJsonToLoanChargeCommand(loanChargeId, loanId, jsonRequestBody);
+        final LoanChargeCommand command = this.apiDataConversionService.convertJsonToLoanChargeCommand(loanChargeId, loanId, jsonRequestBody);
 
-        EntityIdentifier identifier = this.loanWritePlatformService.updateLoanCharge(command);
+        final EntityIdentifier identifier = this.loanWritePlatformService.updateLoanCharge(command);
 
-        return Response.ok().entity(identifier).build();
+        return this.apiJsonSerializerService.serializeEntityIdentifier(identifier);
     }
     
     @POST
@@ -526,16 +526,13 @@ public class LoansApiResource {
     public String waiveLoanCharge(
             @PathParam("loanId") final Long loanId,
             @PathParam("chargeId") final Long loanChargeId,
-            @QueryParam("command") final String commandParam,
+            @QueryParam("command") final String commandParam){
-            final String jsonRequestBody){
 
-//        LoanChargeCommand command = this.apiDataConversionService.convertJsonToLoanChargeCommand(loanChargeId, loanId, jsonRequestBody);
+    	final LoanChargeCommand command = LoanChargeCommand.forWaiver(loanChargeId, loanId);
-        
-    	LoanChargeCommand command = LoanChargeCommand.forWaiver(loanChargeId, loanId);
     	
         String json = "";
 		if (is(commandParam, "waive")) {
-			EntityIdentifier identifier = this.loanWritePlatformService.waiveLoanCharge(command);
+			final EntityIdentifier identifier = this.loanWritePlatformService.waiveLoanCharge(command);
 			json = this.apiJsonSerializerService.serializeEntityIdentifier(identifier);
 		}  else {
 			throw new UnrecognizedQueryParamException("command", commandParam);
@@ -548,13 +545,11 @@ public class LoansApiResource {
     @Path("{loanId}/charges/{chargeId}")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-    public Response deleteLoanCharge(
+    public String deleteLoanCharge(
             @PathParam("loanId") final Long loanId,
             @PathParam("chargeId") final Long loanChargeId){
 
-        EntityIdentifier identifier = this.loanWritePlatformService.deleteLoanCharge(loanId, loanChargeId);
+        final EntityIdentifier identifier = this.loanWritePlatformService.deleteLoanCharge(loanId, loanChargeId);
-
+        return this.apiJsonSerializerService.serializeEntityIdentifier(identifier);
-        return Response.ok().entity(identifier).build();
     }
-
 }

mifosng-provider/src/main/java/org/mifosng/platform/loan/domain/Loan.java

@@ -299,6 +299,13 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 			throw new LoanChargeCannotBeAddedException("loanCharge", "loan.is.closed", defaultUserMessage, getId(), loanCharge.name());
 		}
 	}
+	
+	private void validateLoanChargeIsNotWaived(final LoanCharge loanCharge) {
+		if (loanCharge.isWaivedOrPartiallyWaived(loanCurrency())) {
+			final String defaultUserMessage = "This loan charge cannot be removed as the charge as already been waived.";
+			throw new LoanChargeCannotBeAddedException("loanCharge", "loanCharge.is.waived", defaultUserMessage, getId(), loanCharge.name());
+		}
+	}
 
 	private void validateChargeHasValidSpecifiedDateIfApplicable(final LoanCharge loanCharge, final LocalDate disbursementDate, final LocalDate lastRepaymentPeriodDueDate) {
 		if (loanCharge.isSpecifiedDueDate() && !loanCharge.isDueForCollectionBetween(disbursementDate, lastRepaymentPeriodDueDate)) {
@@ -315,12 +322,26 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 		
 		validateLoanIsNotClosed(loanCharge);
 		
+		// NOTE: to remove this constraint requires that loan transactions 
+		//       that represent the waive of charges also be removed (or reversed)
+		//       if you want ability to remove loan charges that are waived.
+		validateLoanChargeIsNotWaived(loanCharge);
+		
 		boolean removed = this.charges.remove(loanCharge);
 		if (removed) {
 			updateTotalChargesDueAtDisbursement();
 		}
 		
-		// if removing disbursement charge - accomodate this on the transaction that tracked its payment
+		removeOrModifyTransactionAssociatedWithLoanChargeIfDueAtDisbursement(loanCharge);
+		
+		final LoanRepaymentScheduleTransactionProcessor loanRepaymentScheduleTransactionProcessor = this.transactionProcessor.determineProcessor(this.transactionProcessingStrategy);
+		if (!loanCharge.isDueAtDisbursement() && loanCharge.isPaidOrPartiallyPaid(loanCurrency())) {
+			final List<LoanTransaction> allNonContraTransactionsPostDisbursement = retreiveListOfTransactionsPostDisbursement();
+			loanRepaymentScheduleTransactionProcessor.handleTransaction(getDisbursementDate(), allNonContraTransactionsPostDisbursement, getCurrency(), this.repaymentScheduleInstallments, this.charges);
+		}
+	}
+
+	private void removeOrModifyTransactionAssociatedWithLoanChargeIfDueAtDisbursement(final LoanCharge loanCharge) {
 		if (loanCharge.isDueAtDisbursement()) {
 			LoanTransaction transactionToRemove = null;
 			for (LoanTransaction transaction : this.loanTransactions) {
@@ -345,12 +366,6 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 				this.loanTransactions.remove(transactionToRemove);
 			}
 		}
-		
-		final LoanRepaymentScheduleTransactionProcessor loanRepaymentScheduleTransactionProcessor = this.transactionProcessor.determineProcessor(this.transactionProcessingStrategy);
-		if (!loanCharge.isDueAtDisbursement() && loanCharge.isPaidOrPartiallyPaid(loanCurrency())) {
-			final List<LoanTransaction> allNonContraTransactionsPostDisbursement = retreiveListOfTransactionsPostDisbursement();
-			loanRepaymentScheduleTransactionProcessor.handleTransaction(getDisbursementDate(), allNonContraTransactionsPostDisbursement, getCurrency(), this.repaymentScheduleInstallments, this.charges);
-		}
 	}
 	
 	public void updateLoanCharge(final LoanCharge loanCharge, final LoanChargeCommand command) {

mifosng-provider/src/main/java/org/mifosng/platform/loan/domain/LoanCharge.java

@@ -411,6 +411,10 @@ public class LoanCharge extends AbstractPersistable<Long> {
 		final Money amountWaivedOrWrittenOff = getAmountWaived(currency).plus(getAmountWrittenOff(currency));
 		return Money.of(currency, this.amountPaid).plus(amountWaivedOrWrittenOff).isGreaterThanZero();
 	}
+	
+	public boolean isWaivedOrPartiallyWaived(final MonetaryCurrency currency) {
+		return getAmountWaived(currency).isGreaterThanZero();
+	}
 
 	private Money getAmount(final MonetaryCurrency currency) {
 		return Money.of(currency, this.amount);

mifosng-provider/src/main/java/org/mifosng/platform/loan/service/LoanWritePlatformServiceJpaRepositoryImpl.java

@@ -707,7 +707,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
         final LoanCharge loanCharge = retrieveLoanChargeBy(loanId, loanChargeId);
         
         loan.removeLoanCharge(loanCharge);
-        this.loanRepository.saveAndFlush(loan);
+        this.loanRepository.save(loan);
 
         return new EntityIdentifier(loanCharge.getId());
     }