MIFOSX-90: cater for some validation and edge cases around loan charges and loans.

mifosng-provider/src/main/java/org/mifosng/platform/Specifications.java

@@ -1,24 +0,0 @@
-package org.mifosng.platform;
-
-import javax.persistence.criteria.CriteriaBuilder;
-import javax.persistence.criteria.CriteriaQuery;
-import javax.persistence.criteria.Predicate;
-import javax.persistence.criteria.Root;
-
-import org.mifosng.platform.client.domain.Client;
-import org.mifosng.platform.loan.domain.Loan;
-import org.springframework.data.jpa.domain.Specification;
-
-@SuppressWarnings("unused")
-public final class Specifications {
-	
-	public static Specification<Loan> loansThatMatch(final Client client) {
-		return new Specification<Loan>() {
-
-			@Override
-			public Predicate toPredicate(final Root<Loan> root, final CriteriaQuery<?> query, final CriteriaBuilder cb) {
-				return cb.equal(root.get("client"), client);
-			}
-		};
-	}
-}

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

@@ -2,7 +2,6 @@ package org.mifosng.platform.api;
 
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.Set;
 
 import javax.ws.rs.Consumes;
@@ -289,13 +288,7 @@ public class LoansApiResource {
 		
 		final LoanScheduleData loanSchedule = this.calculationPlatformService.calculateLoanSchedule(command);
 
-		final Set<String> typicalLoanScheduleResponseParameters = new HashSet<String>(
+		final Set<String> responseParameters = ApiParameterHelper.extractFieldsForResponseIfProvided(uriInfo.getQueryParameters());
-				Arrays.asList("periods", "cumulativePrincipalDisbursed"));
-		
-		Set<String> responseParameters = ApiParameterHelper.extractFieldsForResponseIfProvided(uriInfo.getQueryParameters());
-		if (responseParameters.isEmpty()) {
-			responseParameters.addAll(typicalLoanScheduleResponseParameters);
-		}
 		boolean prettyPrint = ApiParameterHelper.prettyPrint(uriInfo.getQueryParameters());
 		
 		return this.apiJsonSerializerService.serializeLoanScheduleDataToJson(prettyPrint, responseParameters, loanSchedule);

mifosng-provider/src/main/java/org/mifosng/platform/api/commands/LoanApplicationCommand.java

@@ -252,7 +252,7 @@ public class LoanApplicationCommand {
 		return this.modifiedParameters.contains("submittedOnDate");
 	}
 
-	public boolean isExpectedDisbursementDateChanged() {
+	public boolean isExpectedDisbursementDatePassed() {
 		return this.modifiedParameters.contains("expectedDisbursementDate");
 	}
 

mifosng-provider/src/main/java/org/mifosng/platform/api/infrastructure/ApiDataConversionServiceImpl.java

@@ -557,16 +557,16 @@ public class ApiDataConversionServiceImpl implements ApiDataConversionService {
                 for (int i=0; i<array.size(); i++) {
                 	
                     final JsonObject loanChargeElement = array.get(i).getAsJsonObject();
-                    final Set<String> chargeModifiedParameters = new HashSet<String>();
+                    final Set<String> parametersPassedInForChargesCommand = new HashSet<String>();
 
-                    final Long id = helper.extractLongNamed("id", loanChargeElement, chargeModifiedParameters);
+                    final Long id = helper.extractLongNamed("id", loanChargeElement, parametersPassedInForChargesCommand);
-                    final Long chargeId = helper.extractLongNamed("chargeId", loanChargeElement, chargeModifiedParameters);
+                    final Long chargeId = helper.extractLongNamed("chargeId", loanChargeElement, parametersPassedInForChargesCommand);
-                    final BigDecimal amount = helper.extractBigDecimalNamed("amount", loanChargeElement, locale, chargeModifiedParameters);
+                    final BigDecimal amount = helper.extractBigDecimalNamed("amount", loanChargeElement, locale, parametersPassedInForChargesCommand);
-                    final Integer chargeTimeType = helper.extractIntegerNamed("chargeTimeType", loanChargeElement, locale, chargeModifiedParameters);
+                    final Integer chargeTimeType = helper.extractIntegerNamed("chargeTimeType", loanChargeElement, locale, parametersPassedInForChargesCommand);
-                    final Integer chargeCalculationType = helper.extractIntegerNamed("chargeCalculationType", loanChargeElement, locale, chargeModifiedParameters);
+                    final Integer chargeCalculationType = helper.extractIntegerNamed("chargeCalculationType", loanChargeElement, locale, parametersPassedInForChargesCommand);
-                    final LocalDate specifiedDueDate = helper.extractLocalDateNamed("specifiedDueDate", loanChargeElement, dateFormat, locale, parametersPassedInCommand);
+                    final LocalDate specifiedDueDate = helper.extractLocalDateNamed("specifiedDueDate", loanChargeElement, dateFormat, locale, parametersPassedInForChargesCommand);
                     
-                    charges[i] = new LoanChargeCommand(chargeModifiedParameters, id, null, chargeId, amount, chargeTimeType, chargeCalculationType, specifiedDueDate);
+                    charges[i] = new LoanChargeCommand(parametersPassedInForChargesCommand, id, null, chargeId, amount, chargeTimeType, chargeCalculationType, specifiedDueDate);
                 }
             }
         }

mifosng-provider/src/main/java/org/mifosng/platform/exceptions/LoanChargeCannotBeAddedException.java

@@ -0,0 +1,12 @@
+package org.mifosng.platform.exceptions;
+
+/**
+ * {@link AbstractPlatformDomainRuleException} thrown when loan charge is not allowed to be added to the loan.
+ */
+public class LoanChargeCannotBeAddedException extends AbstractPlatformDomainRuleException {
+
+    public LoanChargeCannotBeAddedException(final String entity, final String postFix, final String defaultUserMessage, final Object... defaultUserMessageArgs) {
+        super("error.msg."+entity+".cannot.be.added.as." + postFix, defaultUserMessage, defaultUserMessageArgs);
+    }
+
+}

mifosng-provider/src/main/java/org/mifosng/platform/exceptions/LoanChargeNotFoundException.java

@@ -0,0 +1,15 @@
+package org.mifosng.platform.exceptions;
+
+/**
+ * {@link AbstractPlatformDomainRuleException} thrown when loan charge does not exist.
+ */
+public class LoanChargeNotFoundException extends AbstractPlatformResourceNotFoundException {
+
+    public LoanChargeNotFoundException(final Long id) {
+        super("error.msg.loanCharge.id.invalid", "Loan charge with identifier " + id + " does not exist", id);
+    }
+
+	public LoanChargeNotFoundException(final Long id, final Long loanId) {
+		super("error.msg.loanCharge.id.invalid.for.given.loan", "Loan charge with identifier " + id + " does not exist for loan " + loanId, id, loanId);
+	}
+}

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

@@ -35,6 +35,7 @@ import org.mifosng.platform.currency.domain.MonetaryCurrency;
 import org.mifosng.platform.currency.domain.Money;
 import org.mifosng.platform.exceptions.InvalidLoanStateTransitionException;
 import org.mifosng.platform.exceptions.InvalidLoanTransactionTypeException;
+import org.mifosng.platform.exceptions.LoanChargeCannotBeAddedException;
 import org.mifosng.platform.fund.domain.Fund;
 import org.mifosng.platform.group.domain.Group;
 import org.mifosng.platform.infrastructure.AbstractAuditableCustom;
@@ -259,6 +260,23 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
     }
 	
 	public void addLoanCharge(final LoanCharge loanCharge) {
+		
+		if (isDisbursed() && loanCharge.isDueAtDisbursement()) {
+			// Note: added this constraint to restrict adding charges to a loan after it is disbursed
+			// if the loan charge payment type is 'Disbursement'.
+			// To undo this constraint would mean resolving how charges due are disbursement are handled at present.
+			// When a loan is disbursed and has charges due at disbursement, a transaction is created to auto record
+			// payment of the charges (user has no choice in saying they were or werent paid) - so its assumed they were paid.
+			
+			final String defaultUserMessage = "This charge which is due at disbursement cannot be added as the loan is already disbursed.";
+			throw new LoanChargeCannotBeAddedException("loanCharge", "due.at.disbursement.and.loan.is.disbursed", defaultUserMessage, getId(), loanCharge.name());
+		}
+		
+		if (loanCharge.isSpecifiedDueDate() && !loanCharge.isDueForCollectionBetween(getDisbursementDate(), getLastRepaymentPeriodDueDate())) {
+			final String defaultUserMessage = "This charge which is due at disbursement cannot be added as the loan is already disbursed.";
+			throw new LoanChargeCannotBeAddedException("loanCharge", "specified.due.date.outside.range", defaultUserMessage, getDisbursementDate(), getLastRepaymentPeriodDueDate(), loanCharge.name());
+		}
+		
 		loanCharge.update(this);
 		this.charges.add(loanCharge);
 		
@@ -276,6 +294,10 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 		}
 	}
 	
+	private LocalDate getLastRepaymentPeriodDueDate() {
+		return this.repaymentScheduleInstallments.get(this.repaymentScheduleInstallments.size()-1).getDueDate();
+	}
+
 	public void removeLoanCharge(final LoanCharge loanCharge) {
 		boolean removed = this.charges.remove(loanCharge);
 		if (removed) {
@@ -286,6 +308,32 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 		LoanScheduleWrapper wrapper = new LoanScheduleWrapper();
 		wrapper.reprocess(getCurrency(), getDisbursementDate(), this.repaymentScheduleInstallments, this.charges);
 		
+		// if removing disbursement charge - accomodate this on the transaction that tracked its payment
+		if (loanCharge.isDueAtDisbursement()) {
+			LoanTransaction transactionToRemove = null;
+			for (LoanTransaction transaction : this.loanTransactions) {
+				if (transaction.isRepaymentAtDisbursement()) {
+					
+					final MonetaryCurrency currency = loanCurrency();
+					final Money chargeAmount = Money.of(currency, loanCharge.amount());
+					if (transaction.isGreaterThan(chargeAmount)) {
+						final Money principalPortion = Money.zero(currency);
+						final Money interestPortion = Money.zero(currency);
+						final Money penaltychargesPortion = Money.zero(currency);
+						
+						final Money feeChargesPortion = chargeAmount;
+						transaction.updateComponentsAndTotal(principalPortion, interestPortion, feeChargesPortion, penaltychargesPortion);
+					} else {
+						transactionToRemove = transaction;
+					}
+				} 
+			}
+			
+			if (transactionToRemove != null) {
+				this.loanTransactions.remove(transactionToRemove);
+			}
+		}
+		
 		final LoanRepaymentScheduleTransactionProcessor loanRepaymentScheduleTransactionProcessor = this.transactionProcessor.determineProcessor(this.transactionProcessingStrategy);
 		if (!loanCharge.isDueAtDisbursement() && loanCharge.isPaidOrPartiallyPaid(loanCurrency())) {
 			final List<LoanTransaction> allNonContraTransactionsPostDisbursement = retreiveListOfTransactionsPostDisbursement();
@@ -318,6 +366,8 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 			final Staff loanOfficer, 
 			final LoanLifecycleStateMachine loanLifecycleStateMachine) {
 
+		// FIXME - KW - whilst the methods are named isXXXChanged, this really only is asking has the parameter for that field been passed in request, 
+		// an additional check to see if the value passed is different to the current value may bee need for all these like it is for expectedDisbursementDate
 		if (command.isClientChanged()) {
 			this.client = client;
 		}
@@ -346,13 +396,11 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 			this.submittedOnDate = command.getSubmittedOnDate().toDate();
 		}
 		
-		if (command.isExpectedDisbursementDateChanged()) {
+		if (command.isExpectedDisbursementDatePassed()) {
-			if (command.getExpectedDisbursementDate() != null) {
+			if (dateHasChanged(getExpectedDisbursedOnLocalDate(), command.getExpectedDisbursementDate())) {
 				this.expectedDisbursedOnDate = command.getExpectedDisbursementDate().toDate();
 				removeFirstDisbursementTransaction();
 				disburse(command.getExpectedDisbursementDate(), loanLifecycleStateMachine, false);
-			} else {
-				this.expectedDisbursedOnDate = null;
 			}
 		}
 		
@@ -406,6 +454,19 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 		loanRepaymentScheduleTransactionProcessor.handleTransaction(repaymentsOrWaivers, getCurrency(), this.repaymentScheduleInstallments, this.charges);
 	}
 
+	private boolean dateHasChanged(final LocalDate originalLocalDate, final LocalDate providedLocalDate) {
+		
+		boolean dateHasChanged = false;
+		
+		if (originalLocalDate != null) {
+			dateHasChanged = originalLocalDate.equals(providedLocalDate);
+		} else {
+			dateHasChanged = (providedLocalDate != null);
+		}
+		
+		return dateHasChanged;
+	}
+
 	private void removeFirstDisbursementTransaction() {
 		for (LoanTransaction loanTransaction : loanTransactions) {
 			if (loanTransaction.isDisbursement()) {
@@ -564,10 +625,10 @@ public class Loan extends AbstractAuditableCustom<AppUser, Long> {
 
 	public void disburse(final LocalDate disbursedOn, final LoanLifecycleStateMachine loanLifecycleStateMachine, final boolean statusTransition) {
 
-		// ensure loan is in pre-disbursal state
-		updateLoanToPreDisbursalState();
-				
 		if (statusTransition) {
+			// ensure loan is in pre-disbursal state
+			updateLoanToPreDisbursalState();
+			
 			LoanStatus statusEnum = loanLifecycleStateMachine.transition(LoanEvent.LOAN_DISBURSED, LoanStatus.fromInt(this.loanStatus));
 			this.loanStatus = statusEnum.getValue();
 		}

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

@@ -231,6 +231,10 @@ public class LoanCharge extends AbstractPersistable<Long> {
             this.chargeCalculation = ChargeCalculationType.fromInt(command.getChargeCalculationType()).getValue();
         }
         
+        if (command.isSpecifiedDueDateChanged()) {
+        	this.dueForCollectionAsOfDate = command.getSpecifiedDueDate().toDate();
+        }
+        
         if (command.isAmountChanged()) {
             switch (ChargeCalculationType.fromInt(this.chargeCalculation)) {
 			case INVALID:
@@ -264,6 +268,10 @@ public class LoanCharge extends AbstractPersistable<Long> {
 		return ChargeTimeType.fromInt(this.chargeTime).equals(ChargeTimeType.DISBURSEMENT);
 	}
 	
+	public boolean isSpecifiedDueDate() {
+		return ChargeTimeType.fromInt(this.chargeTime).equals(ChargeTimeType.SPECIFIED_DUE_DATE);
+	}
+
 	private boolean isGreaterThanZero(final BigDecimal value) {
 		return value.compareTo(BigDecimal.ZERO) == 1;
 	}
@@ -276,7 +284,7 @@ public class LoanCharge extends AbstractPersistable<Long> {
                 this.amount, this.chargeTime, this.chargeCalculation, specifiedDueDate);
 	}
 
-	private LocalDate getDueForCollectionAsOfLocalDate() {
+	public LocalDate getDueForCollectionAsOfLocalDate() {
 		LocalDate specifiedDueDate = null;
 		if (this.dueForCollectionAsOfDate != null) {
 			specifiedDueDate = new LocalDate(this.dueForCollectionAsOfDate);
@@ -390,4 +398,8 @@ public class LoanCharge extends AbstractPersistable<Long> {
 		
 		return incrementBy.minus(amountPaidOnThisCharge);
 	}
+
+	public String name() {
+		return this.charge.getName();
+	}
 }

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

@@ -224,6 +224,10 @@ public class LoanTransaction extends AbstractAuditableCustom<AppUser, Long> {
 		this.feeChargesPortion = null;
 	}
 
+	public boolean isGreaterThan(final Money monetaryAmount) {
+		return getAmount(monetaryAmount.getCurrency()).isGreaterThan(monetaryAmount);
+	}
+	
 	public boolean isGreaterThanZero(final MonetaryCurrency currency) {
 		return getAmount(currency).isGreaterThanZero();
 	}

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

@@ -9,6 +9,7 @@ import org.mifosng.platform.charge.domain.Charge;
 import org.mifosng.platform.charge.domain.ChargeRepository;
 import org.mifosng.platform.exceptions.ChargeIsNotActiveException;
 import org.mifosng.platform.exceptions.ChargeNotFoundException;
+import org.mifosng.platform.exceptions.LoanChargeNotFoundException;
 import org.mifosng.platform.loan.domain.LoanCharge;
 import org.mifosng.platform.loan.domain.LoanChargeRepository;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -48,7 +49,7 @@ public class LoanChargeAssembler {
 					final Long loanChargeId = loanChargeCommand.getId();
 					final LoanCharge loanCharge = this.loanChargeRepository.findOne(loanChargeId);
 					if (loanCharge == null) {
-						throw new ChargeNotFoundException(loanChargeId);
+						throw new LoanChargeNotFoundException(loanChargeId);
 					}
 					
 					loanCharge.update(loanChargeCommand, loanPrincipal);

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

@@ -31,6 +31,7 @@ import org.mifosng.platform.exceptions.ChargeIsNotActiveException;
 import org.mifosng.platform.exceptions.ChargeNotFoundException;
 import org.mifosng.platform.exceptions.ClientNotFoundException;
 import org.mifosng.platform.exceptions.InvalidCurrencyException;
+import org.mifosng.platform.exceptions.LoanChargeNotFoundException;
 import org.mifosng.platform.exceptions.LoanNotFoundException;
 import org.mifosng.platform.exceptions.LoanNotInSubmittedAndPendingApprovalStateCannotBeDeleted;
 import org.mifosng.platform.exceptions.LoanOfficerAssignmentException;
@@ -145,10 +146,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
 			throw new NoAuthorizationException("Cannot modify backdated loan.");
 		}
 
-		final Loan loan = this.loanRepository.findOne(command.getLoanId());
+		final Loan loan = retrieveLoanBy(command.getLoanId());
-		if (loan == null) {
-			throw new LoanNotFoundException(command.getLoanId());
-		}
 		
 		final LoanProduct loanProduct = this.loanProductRepository.findOne(command.getProductId());
 		if (loanProduct == null) {
@@ -166,6 +164,9 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
 		final Set<LoanCharge> charges = this.loanChargeAssembler.assembleFrom(command.getCharges(), loanProduct.getCharges(), loan.getPrincpal().getAmount());
 
         final LoanScheduleData loanSchedule = this.calculationPlatformService.calculateLoanSchedule(command.toCalculateLoanScheduleCommand());
+        
+        // FIXME - kw - restrict ability to modify loan to modifying the 'loan application' stage - once it has been disbursed, should be allowed to use this feature
+        //       - other facilities are in place such as 'undo disbursal'
 		loan.modifyLoanApplication(command, client, loanProduct, fund, strategy, loanSchedule, charges, loanOfficer, defaultLoanLifecycleStateMachine());
 
 		this.loanRepository.save(loan);
@@ -184,10 +185,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
 		
 		context.authenticatedUser();
 
-		Loan loan = this.loanRepository.findOne(loanId);
+		final Loan loan = retrieveLoanBy(loanId);
-		if (loan == null) {
-			throw new LoanNotFoundException(loanId);
-		}
 		
 		if (loan.isNotSubmittedAndPendingApproval()) {
 			throw new LoanNotInSubmittedAndPendingApprovalStateCannotBeDeleted(loanId);
@@ -210,10 +208,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
 		LoanStateTransitionCommandValidator validator = new LoanStateTransitionCommandValidator(command);
 		validator.validate();
 
-		Loan loan = this.loanRepository.findOne(command.getLoanId());
+		final Loan loan = retrieveLoanBy(command.getLoanId());
-		if (loan == null) {
-			throw new LoanNotFoundException(command.getLoanId());
-		}
 		
 		LocalDate eventDate = command.getEventDate();
 		if (this.isBeforeToday(eventDate) && currentUser.canNotApproveLoanInPast()) {
@@ -238,10 +233,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
 
 		context.authenticatedUser();
 
-		Loan loan = this.loanRepository.findOne(command.getLoanId());
+		final Loan loan = retrieveLoanBy(command.getLoanId());
-		if (loan == null) {
-			throw new LoanNotFoundException(command.getLoanId());
-		}
 		
 		loan.undoApproval(defaultLoanLifecycleStateMachine());
 		this.loanRepository.save(loan);
@@ -264,10 +256,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
 		LoanStateTransitionCommandValidator validator = new LoanStateTransitionCommandValidator(command);
 		validator.validate();
 
-		Loan loan = this.loanRepository.findOne(command.getLoanId());
+		final Loan loan = retrieveLoanBy(command.getLoanId());
-		if (loan == null) {
-			throw new LoanNotFoundException(command.getLoanId());
-		}
 
 		LocalDate eventDate = command.getEventDate();
 		if (this.isBeforeToday(eventDate) && currentUser.canNotRejectLoanInPast()) {
@@ -295,10 +284,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
 		LoanStateTransitionCommandValidator validator = new LoanStateTransitionCommandValidator(command);
 		validator.validate();
 		
-		Loan loan = this.loanRepository.findOne(command.getLoanId());
+		final Loan loan = retrieveLoanBy(command.getLoanId());
-		if (loan == null) {
-			throw new LoanNotFoundException(command.getLoanId());
-		}
 		
 		LocalDate eventDate = command.getEventDate();
 		if (this.isBeforeToday(eventDate) && currentUser.canNotWithdrawByClientLoanInPast()) {
@@ -326,10 +312,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
 		LoanStateTransitionCommandValidator validator = new LoanStateTransitionCommandValidator(command);
 		validator.validate();
 
-		Loan loan = this.loanRepository.findOne(command.getLoanId());
+		final Loan loan = retrieveLoanBy(command.getLoanId());
-		if (loan == null) {
-			throw new LoanNotFoundException(command.getLoanId());
-		}
 
 		String noteText = command.getNote();
 		LocalDate actualDisbursementDate = command.getEventDate();
@@ -673,22 +656,10 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
         validator.validateForUpdate();
 
         final Long loanId = command.getLoanId();
-        final Loan loan = this.loanRepository.findOne(loanId);
+        final Loan loan = retrieveLoanBy(loanId);
-        if (loan == null) {
-            throw new LoanNotFoundException(loanId);
-        }
         
         final Long loanChargeId = command.getId();
-		final LoanCharge loanCharge = this.loanChargeRepository.findOne(loanChargeId);
+		final LoanCharge loanCharge = retrieveLoanChargeBy(loanId, loanChargeId);
-        if (loanCharge == null) {
-        	// FIXME - kw - should be loan charge not found, not charge not found.
-        	throw new ChargeNotFoundException(loanChargeId);
-        }
-        
-        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);
@@ -702,21 +673,9 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
 
         this.context.authenticatedUser();
 
-        final Loan loan = this.loanRepository.findOne(loanId);
+        final Loan loan = retrieveLoanBy(loanId);
-        if (loan == null) {
-            throw new LoanNotFoundException(loanId);
-        }
         
-        final LoanCharge loanCharge = this.loanChargeRepository.findOne(loanChargeId);
+        final LoanCharge loanCharge = retrieveLoanChargeBy(loanId, loanChargeId);
-        if (loanCharge == null) {
-        	// FIXME - kw - should be loan charge not found, not charge not found.
-        	throw new ChargeNotFoundException(loanChargeId);
-        }
-        
-        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);
@@ -724,6 +683,26 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
         return new EntityIdentifier(loanCharge.getId());
     }
 	
+	private Loan retrieveLoanBy(final Long loanId) {
+		final Loan loan = this.loanRepository.findOne(loanId);
+        if (loan == null) {
+            throw new LoanNotFoundException(loanId);
+        }
+		return loan;
+	}
+
+	private LoanCharge retrieveLoanChargeBy(final Long loanId, final Long loanChargeId) {
+		final LoanCharge loanCharge = this.loanChargeRepository.findOne(loanChargeId);
+        if (loanCharge == null) {
+        	throw new LoanChargeNotFoundException(loanChargeId);
+        }
+        
+        if (loanCharge.hasNotLoanIdentifiedBy(loanId)) {
+        	throw new LoanChargeNotFoundException(loanChargeId, loanId);
+        }
+		return loanCharge;
+	}
+
 	@Transactional
     @Override
     public EntityIdentifier bulkLoanReassignment(final BulkLoanReassignmentCommand command) {
@@ -739,10 +718,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
         for (String loanIdString : command.getLoans()){
             final Long loanId = Long.valueOf(loanIdString);
 
-            final Loan loan = this.loanRepository.findOne(loanId);
+            final Loan loan = retrieveLoanBy(loanId);
-            if (loan == null) {
-                throw new LoanNotFoundException(loanId);
-            }
             
             if (!loan.hasLoanOfficer(fromLoanOfficer)){
                 throw new LoanOfficerAssignmentException(loan.getId(), fromLoanOfficer.getId());