MIFOSX-90: tidy up of implementation around add,delete,update loan charges.

mifosng-provider/src/main/java/org/mifosng/platform/charge/service/ChargeReadPlatformServiceImpl.java

@@ -83,19 +83,19 @@ public class ChargeReadPlatformServiceImpl implements ChargeReadPlatformService
     }
 
     @Override
-    public Collection<ChargeData> retrieveLoanProductCharges(Long loanProductId) {
+    public Collection<ChargeData> retrieveLoanProductCharges(final Long loanProductId) {
 
         this.context.authenticatedUser();
 
         final ChargeMapper rm = new ChargeMapper();
 
-        final String sql = "select " + rm.loanProductChargeSchema() + " where c.is_deleted=0 and plc.product_loan_id=?";
+        final String sql = "select " + rm.loanProductChargeSchema() + " where c.is_deleted=0 and c.is_active=1 and plc.product_loan_id=?";
 
         return this.jdbcTemplate.query(sql, rm, new Object[] {loanProductId});
     }
 
     @Override
-    public Collection<LoanChargeData> retrieveLoanCharges(Long loanId) {
+    public Collection<LoanChargeData> retrieveLoanCharges(final Long loanId) {
         this.context.authenticatedUser();
 
         final LoanChargeMapper rm = new LoanChargeMapper();
@@ -130,8 +130,8 @@ public class ChargeReadPlatformServiceImpl implements ChargeReadPlatformService
 
         public String chargeSchema(){
             return "c.id as id, c.name as name, c.amount as amount, c.currency_code as currencyCode, " +
-                   "charge_applies_to_enum as chargeAppliesTo, charge_time_enum as chargeTime, " +
+                   "c.charge_applies_to_enum as chargeAppliesTo, c.charge_time_enum as chargeTime, " +
-                   "charge_calculation_enum as chargeCalculation, is_active as active, oc.name as currencyName, " +
+                   "c.charge_calculation_enum as chargeCalculation, c.is_active as active, oc.name as currencyName, " +
                    "oc.decimal_places as currencyDecimalPlaces, oc.display_symbol as currencyDisplaySymbol, " +
                    "oc.internationalized_name_code as currencyNameCode from m_charge c " +
                    "join m_organisation_currency oc on c.currency_code = oc.code";

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

@@ -28,6 +28,7 @@ import org.hibernate.annotations.LazyCollectionOption;
 import org.joda.time.LocalDate;
 import org.mifosng.platform.api.LoanScheduleData;
 import org.mifosng.platform.api.commands.LoanApplicationCommand;
+import org.mifosng.platform.api.commands.LoanChargeCommand;
 import org.mifosng.platform.api.data.LoanSchedulePeriodData;
 import org.mifosng.platform.client.domain.Client;
 import org.mifosng.platform.currency.domain.MonetaryCurrency;
@@ -48,7 +49,8 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 	@JoinColumn(name = "client_id")
 	private Client client;
 
-    @ManyToOne(optional = true)
+    @SuppressWarnings("unused")
+	@ManyToOne(optional = true)
     @JoinColumn(name = "group_id")
     private Group group;
 
@@ -251,15 +253,37 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 		}
 		return loanCharges;
 	}
+	
+    private void updateTotalChargesDueAtDisbursement() {
+        this.totalChargesDueAtDisbursement = deriveSumTotalOfChargesDueAtDisbursement();
+    }
+	
+	public void addLoanCharge(final LoanCharge loanCharge) {
+		loanCharge.update(this);
+		this.charges.add(loanCharge);
+		
+		updateTotalChargesDueAtDisbursement();
+	}
+	
+	public void removeLoanCharge(final LoanCharge loanCharge) {
+		boolean removed = this.charges.remove(loanCharge);
+		if (removed) {
+			updateTotalChargesDueAtDisbursement();
+		}
+	}
+	
+	public void updateLoanCharge(final LoanCharge loanCharge, final LoanChargeCommand command) {
+		
+		if (this.charges.contains(loanCharge)) {
+			loanCharge.update(command, getPrincpal().getAmount());
+			updateTotalChargesDueAtDisbursement();
+		}
+	}
 
 	public Client client() {
 		return this.client;
 	}
 
-    public Group getGroup() {
-        return group;
-    }
-
     public LoanProduct loanProduct() {
 		return this.loanProduct;
 	}
@@ -610,10 +634,6 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 		return waiveInterestTransaction;
 	}
 
-    public void updateTotalChargesDueAtDisbursement() {
-        this.totalChargesDueAtDisbursement = deriveSumTotalOfChargesDueAtDisbursement();
-    }
-
 	public void makeRepayment(final LoanTransaction loanTransaction, final LoanLifecycleStateMachine loanLifecycleStateMachine) {
 		handleRepaymentOrWaiverTransaction(loanTransaction, loanLifecycleStateMachine, null);
 	}
@@ -1334,6 +1354,10 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 		return identifier.equalsIgnoreCase(this.externalId)
 				|| identifier.equalsIgnoreCase(this.getId().toString());
 	}
+	
+	public boolean hasIdentifyOf(final Long loanId) {
+		return loanId.equals(this.getId());
+	}
 
 	public LocalDate getInterestChargedFromDate() {
 		LocalDate interestChargedFrom = null;

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

@@ -318,4 +318,12 @@ public class LoanCharge extends AbstractPersistable<Long> {
 	public BigDecimal amount() {
 		return this.amount;
 	}
+
+	public boolean hasNotLoanIdentifiedBy(final Long loanId) {
+		return !hasLoanIdentifiedBy(loanId);
+	}
+	
+	public boolean hasLoanIdentifiedBy(final Long loanId) {
+		return this.loan.hasIdentifyOf(loanId);
+	}
 }

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

@@ -633,13 +633,13 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
         LoanChargeCommandValidator validator = new LoanChargeCommandValidator(command);
         validator.validateForCreate();
 
-        Loan loan = this.loanRepository.findOne(command.getLoanId());
+        final Loan loan = this.loanRepository.findOne(command.getLoanId());
         if (loan == null) {
             throw new LoanNotFoundException(command.getLoanId());
         }
 
-        Charge chargeDefinition = this.chargeRepository.findOne(command.getChargeId());
+        final Charge chargeDefinition = this.chargeRepository.findOne(command.getChargeId());
-        if (chargeDefinition == null) {
+        if (chargeDefinition == null || chargeDefinition.isDeleted()) {
             throw new ChargeNotFoundException(command.getChargeId());
         }
 
@@ -654,12 +654,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
             throw new InvalidCurrencyException("charge", "attach.to.loan", errorMessage);
         }
 
-        this.loanChargeRepository.saveAndFlush(loanCharge);
+        loan.addLoanCharge(loanCharge);
-
-        // FIXME - KW - 
-        loan.getCharges().add(loanCharge);
-        loan.updateTotalChargesDueAtDisbursement();
-
         this.loanRepository.saveAndFlush(loan);
 
         return new EntityIdentifier(loanCharge.getId());
@@ -674,43 +669,55 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
         LoanChargeCommandValidator validator = new LoanChargeCommandValidator(command);
         validator.validateForUpdate();
 
-        Loan loan = this.loanRepository.findOne(command.getLoanId());
+        final Long loanId = command.getLoanId();
+        final Loan loan = this.loanRepository.findOne(loanId);
         if (loan == null) {
-            throw new LoanNotFoundException(command.getLoanId());
+            throw new LoanNotFoundException(loanId);
         }
-
+        
-        for (LoanCharge loanCharge : loan.getCharges()){
+        final Long loanChargeId = command.getId();
-            if (loanCharge.getId().equals(command.getId())){
+		final LoanCharge loanCharge = this.loanChargeRepository.findOne(loanChargeId);
-                loanCharge.update(command, loan.getPrincpal().getAmount());
+        if (loanCharge == null) {
-                loan.updateTotalChargesDueAtDisbursement();
+        	// FIXME - kw - should be loan charge not found, not charge not found.
-                this.loanRepository.saveAndFlush(loan);
+        	throw new ChargeNotFoundException(loanChargeId);
-                return new EntityIdentifier(loanCharge.getId());
-            }
         }
+        
+        if (loanCharge.hasNotLoanIdentifiedBy(loanId)) {
+        	// FIXME - kw - should be loan charge not found, not charge not found.
+        	throw new ChargeNotFoundException(loanChargeId);
+        }
+        
+        loan.updateLoanCharge(loanCharge, command);
+        this.loanRepository.saveAndFlush(loan);
 
-        throw new ChargeNotFoundException(command.getId());
+        return new EntityIdentifier(loanCharge.getId());
     }
 
 	@Transactional
     @Override
-    public EntityIdentifier deleteLoanCharge(Long loanId, Long loanChargeId) {
+    public EntityIdentifier deleteLoanCharge(final Long loanId, final Long loanChargeId) {
 
         this.context.authenticatedUser();
 
-        Loan loan = this.loanRepository.findOne(loanId);
+        final Loan loan = this.loanRepository.findOne(loanId);
         if (loan == null) {
             throw new LoanNotFoundException(loanId);
         }
-
+        
-        for (LoanCharge loanCharge : loan.getCharges()){
+        final LoanCharge loanCharge = this.loanChargeRepository.findOne(loanChargeId);
-            if (loanCharge.getId().equals(loanChargeId)){
+        if (loanCharge == null) {
-                loan.getCharges().remove(loanCharge);
+        	// FIXME - kw - should be loan charge not found, not charge not found.
-                loan.updateTotalChargesDueAtDisbursement();
+        	throw new ChargeNotFoundException(loanChargeId);
-                this.loanRepository.saveAndFlush(loan);
-                return new EntityIdentifier(loanCharge.getId());
-            }
         }
+        
+        if (loanCharge.hasNotLoanIdentifiedBy(loanId)) {
+        	// FIXME - kw - should be loan charge not found, not charge not found.
+        	throw new ChargeNotFoundException(loanChargeId);
+        }
+        
+        loan.removeLoanCharge(loanCharge);
+        this.loanRepository.saveAndFlush(loan);
 
-        throw new ChargeNotFoundException(loanChargeId);
+        return new EntityIdentifier(loanCharge.getId());
     }
 }