remove usused warnings, make group data immutable.

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

@@ -121,20 +121,16 @@ public class DataTableApiResource {
 				.retrieveDataTableGenericResultSet(datatable, apptableId,
 						sqlFields, sqlOrder, null);
 
-		boolean genericResultSet = ApiParameterHelper.genericResultSet(uriInfo
-				.getQueryParameters());
-
+		String json = "";
+		final boolean genericResultSet = ApiParameterHelper.genericResultSet(uriInfo.getQueryParameters());
 		if (genericResultSet) {
-			boolean prettyPrints = ApiParameterHelper.prettyPrint(uriInfo
-					.getQueryParameters());
-			return this.apiJsonSerializerService
-					.serializeGenericResultsetDataToJson(prettyPrints, results);
+			boolean prettyPrints = ApiParameterHelper.prettyPrint(uriInfo.getQueryParameters());
+			json = this.apiJsonSerializerService.serializeGenericResultsetDataToJson(prettyPrints, results);
 		} else {
-
-			return this.genericDataService
-					.generateJsonFromGenericResultsetData(results);
+			json = this.genericDataService.generateJsonFromGenericResultsetData(results);
 		}
-
+		
+		return json;
 	}
 
 	@POST

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

@@ -1,6 +1,5 @@
 package org.mifosng.platform.api;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
@@ -20,7 +19,6 @@ import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
 
 import org.mifosng.platform.api.commands.GroupCommand;
-import org.mifosng.platform.api.data.ClientAccountSummaryCollectionData;
 import org.mifosng.platform.api.data.ClientLookup;
 import org.mifosng.platform.api.data.EntityIdentifier;
 import org.mifosng.platform.api.data.GroupAccountSummaryCollectionData;
@@ -94,23 +92,26 @@ public class GroupsApiResource {
         }
 
         boolean prettyPrint = ApiParameterHelper.prettyPrint(uriInfo.getQueryParameters());
-        boolean template = ApiParameterHelper.template(uriInfo.getQueryParameters());
-
+        
         GroupData group = this.groupReadPlatformService.retrieveGroup(groupId);
+        final Collection<ClientLookup> clientMembers = this.groupReadPlatformService.retrieveClientMembers(groupId);
+        Collection<ClientLookup> availableClients = null;
+        Collection<OfficeLookup> allowedOffices = null;
+        
+        group = new GroupData(group, clientMembers, availableClients, allowedOffices);
+        
+        boolean template = ApiParameterHelper.template(uriInfo.getQueryParameters());
+		if (template) {
+			responseParameters.add("allowedClients");
+			responseParameters.add("allowedOffices");
 
-        Collection<ClientLookup> clientMembers = this.groupReadPlatformService.retrieveClientMembers(groupId);
-        group.setClientMembers(clientMembers);
+			availableClients = this.clientReadPlatformService.retrieveAllIndividualClientsForLookup();
+			availableClients.removeAll(group.clientMembers());
 
-        if (template) {
-            Collection<ClientLookup>  availableClients = this.clientReadPlatformService.retrieveAllIndividualClientsForLookup();
-            availableClients.removeAll(group.getClientMembers());
-            group.setAllowedClients(availableClients);
+			allowedOffices = officeReadPlatformService.retrieveAllOfficesForLookup();
 
-            responseParameters.add("allowedClients");
-
-            group.setAllowedOffices(new ArrayList<OfficeLookup>(officeReadPlatformService.retrieveAllOfficesForLookup()));
-            responseParameters.add("allowedOffices");
-        }
+			group = new GroupData(group, group.clientMembers(), availableClients, allowedOffices);
+		}
         
         return this.apiJsonSerializerService.serializeGroupDataToJson(prettyPrint, responseParameters, group);
     }

mifosng-provider/src/main/java/org/mifosng/platform/api/data/ClientLookup.java

@@ -5,11 +5,14 @@ import org.apache.commons.lang.StringUtils;
 public class ClientLookup {
 
     private final Long id;
+    @SuppressWarnings("unused")
     private final String displayName;
-    private final Long officeId;
-    private final String officeName;
+    @SuppressWarnings("unused")
+	private final Long officeId;
+    @SuppressWarnings("unused")
+	private final String officeName;
 
-    public ClientLookup(final Long id, final String firstname, final String lastname, final Long officeId, String officeName) {
+    public ClientLookup(final Long id, final String firstname, final String lastname, final Long officeId, final String officeName) {
         this.id = id;
         
         StringBuilder displayNameBuilder = new StringBuilder(firstname);
@@ -39,18 +42,10 @@ public class ClientLookup {
         this.officeName = null;
     }
 
-    public Long getId() {
-        return id;
-    }
-
-    public String getDisplayName() {
-        return displayName;
-    }
-
     @Override
     public boolean equals(Object obj) {
         ClientLookup clientLookup = (ClientLookup) obj;
-        return this.id.equals(clientLookup.getId());
+        return this.id.equals(clientLookup.id);
     }
 
     @Override

mifosng-provider/src/main/java/org/mifosng/platform/api/data/GroupData.java

@@ -1,46 +1,76 @@
 package org.mifosng.platform.api.data;
 
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.List;
 
+/**
+ * Immutable data object representing groups.
+ */
 public class GroupData {
-    
-    private final Long id;
-    private final String name;
-    private final String externalId;
-    private final Long officeId;
-    private final String officeName;
 
-    private Collection<ClientLookup> clientMembers = new ArrayList<ClientLookup>();
-    private Collection<ClientLookup> allowedClients = new ArrayList<ClientLookup>();
-    private Collection<OfficeLookup> allowedOffices = new ArrayList<OfficeLookup>();
+	private final Long id;
+	private final String name;
+	private final String externalId;
+	private final Long officeId;
+	private final String officeName;
 
-    public GroupData(Long id, Long officeId, String officeName, String name, String externalId) {
-        this.id = id;
-        this.name = name;
-        this.externalId = externalId;
-        this.officeId = officeId;
-        this.officeName = officeName;
-    }
+	private final Collection<ClientLookup> clientMembers;
+	@SuppressWarnings("unused")
+	private final Collection<ClientLookup> allowedClients;
+	@SuppressWarnings("unused")
+	private final Collection<OfficeLookup> allowedOffices;
 
-    public GroupData(Collection<ClientLookup> allowedClients, Collection<OfficeLookup> allowedOffices){
-        this.id = null;
-        this.name = null;
-        this.externalId = null;
-        this.officeId = null;
-        this.officeName = null;
-        this.allowedClients = allowedClients;
-        this.allowedOffices = allowedOffices;
-    }
+	public GroupData(
+			final Long id, 
+			final Long officeId,
+			final String officeName, 
+			final String name, 
+			final String externalId) {
+		this.id = id;
+		this.officeId = officeId;
+		this.officeName = officeName;
+		this.name = name;
+		this.externalId = externalId;
 
-    public Long getId() {
-        return id;
-    }
+		this.clientMembers = null;
+		this.allowedClients = null;
+		this.allowedOffices = null;
+	}
 
-    public String getName() {
-        return name;
-    }
+	public GroupData(final GroupData group, final Collection<ClientLookup> clientMembers, final Collection<ClientLookup> allowedClients,
+			final Collection<OfficeLookup> allowedOffices) {
+		this.id = group.getId();
+		this.officeId = group.getOfficeId();
+		this.officeName = group.getOfficeName();
+		this.name = group.getName();
+		this.externalId = group.getExternalId();
+		
+		this.clientMembers = clientMembers;
+		this.allowedClients = allowedClients;
+		this.allowedOffices = allowedOffices;
+	}
+	
+	public GroupData(final Collection<ClientLookup> allowedClients, final Collection<OfficeLookup> allowedOffices) {
+		this.id = null;
+		this.officeId = null;
+		this.officeName = null;
+		this.name = null;
+		this.externalId = null;
+		this.clientMembers = null;
+		this.allowedClients = allowedClients;
+		this.allowedOffices = allowedOffices;
+	}
+	
+	public Long getId() {
+		return id;
+	}
+
+	public String getName() {
+		return name;
+	}
+
+	public String getExternalId() {
+		return externalId;
+	}
 
 	public Long getOfficeId() {
 		return officeId;
@@ -50,31 +80,7 @@ public class GroupData {
 		return officeName;
 	}
 
-	public String getExternalId() {
-        return externalId;
-    }
-
-    public Collection<ClientLookup> getClientMembers() {
-        return clientMembers;
-    }
-
-    public void setClientMembers(Collection<ClientLookup> clientMembers) {
-        this.clientMembers = clientMembers;
-    }
-
-    public Collection<ClientLookup> getAllowedClients() {
-        return allowedClients;
-    }
-
-    public void setAllowedClients(Collection<ClientLookup> allowedClients) {
-        this.allowedClients = allowedClients;
-    }
-
-    public Collection<OfficeLookup> getAllowedOffices() {
-        return allowedOffices;
-    }
-
-    public void setAllowedOffices(Collection<OfficeLookup> allowedOffices) {
-        this.allowedOffices = allowedOffices;
-    }
-}
+	public Collection<ClientLookup> clientMembers() {
+		return this.clientMembers;
+	}
+}

mifosng-provider/src/main/java/org/mifosng/platform/client/domain/Client.java

@@ -21,7 +21,6 @@ import org.mifosng.platform.user.domain.AppUser;
 @Table(name = "m_client")
 public class Client extends AbstractAuditableCustom<AppUser, Long> {
 
-    @SuppressWarnings("unused")
 	@ManyToOne
     @JoinColumn(name = "office_id", nullable = false)
     private Office       office;
@@ -51,7 +50,7 @@ public class Client extends AbstractAuditableCustom<AppUser, Long> {
 		return new Client(clientOffice, firstname, lastname, joiningDate, externalId);
 	}
 
-    public Client() {
+    protected Client() {
         this.office = null;
         this.joiningDate = null;
         this.firstName = null;
@@ -141,10 +140,6 @@ public class Client extends AbstractAuditableCustom<AppUser, Long> {
 		this.displayName = this.firstName + " " + this.lastName;
 	}
 
-    public Office getOffice() {
-        return office;
-    }
-
     /**
 	 * Delete is a <i>soft delete</i>. Updates flag on client so it wont appear in query/report results.
 	 * 
@@ -158,4 +153,8 @@ public class Client extends AbstractAuditableCustom<AppUser, Long> {
 	public boolean isDeleted() {
 		return deleted;
 	}
+
+	public boolean isOfficeIdentifiedBy(final Long officeId) {
+		return this.office.identifiedBy(officeId);
+	}
 }

mifosng-provider/src/main/java/org/mifosng/platform/group/domain/Group.java

@@ -22,7 +22,8 @@ import org.mifosng.platform.user.domain.AppUser;
 @Table(name = "m_group")
 public class Group extends AbstractAuditableCustom<AppUser, Long> {
 
-    @ManyToOne
+    @SuppressWarnings("unused")
+	@ManyToOne
     @JoinColumn(name = "office_id", nullable = false)
     private Office office;
 

mifosng-provider/src/main/java/org/mifosng/platform/group/service/GroupReadPlatformServiceImpl.java

@@ -56,7 +56,7 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService {
     }
 
     @Override
-    public GroupData retrieveGroup(Long groupId) {
+    public GroupData retrieveGroup(final Long groupId) {
         
         try{
             this.context.authenticatedUser();

mifosng-provider/src/main/java/org/mifosng/platform/group/service/GroupWritePlatformServiceJpaRepositoryImpl.java

@@ -134,12 +134,12 @@ public class GroupWritePlatformServiceJpaRepositoryImpl implements GroupWritePla
 
         if (command.isClientMembersChanged() && !ObjectUtils.isEmpty(clientMembersArray)) {
             for (String clientId : clientMembersArray) {
-                Long id = Long.valueOf(clientId);
+                final Long id = Long.valueOf(clientId);
                 Client client = this.clientRepository.findOne(id);
                 if (client == null || client.isDeleted()) {
                     throw new ClientNotFoundException(id);
                 }
-                if (!client.getOffice().getId().equals(command.getOfficeId())){
+                if (!client.isOfficeIdentifiedBy(command.getOfficeId())){
                     String errorMessage = "Group and Client must have the same office.";
                     throw new InvalidOfficeException("client", "attach.to.group", errorMessage);
                 }

mifosng-provider/src/main/java/org/mifosng/platform/saving/domain/DepositAccount.java

@@ -98,7 +98,6 @@ public class DepositAccount extends AbstractAuditableCustom<AppUser, Long>  {
 	@Column(name = "is_renewal_allowed", nullable = false)
 	private boolean renewalAllowed = false;
 	
-	@SuppressWarnings("unused")
 	@Column(name = "is_preclosure_allowed", nullable = false)
 	private boolean preClosureAllowed = false;
 	

mifosng-provider/src/main/java/org/mifosng/platform/saving/service/DepositAccountReadPlatformServiceImpl.java

@@ -5,8 +5,6 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.Collection;
 
-import javax.security.auth.login.AccountException;
-
 import org.joda.time.LocalDate;
 import org.joda.time.Months;
 import org.mifosng.platform.api.data.ClientData;
@@ -56,26 +54,6 @@ public class DepositAccountReadPlatformServiceImpl implements DepositAccountRead
 		String sql = "select " + mapper.schema() + " where da.is_deleted=0";
 		
 		return this.jdbcTemplate.query(sql,mapper, new Object[]{});
-		
-		// NOTE: when retrieving a list of deposit accounts, its unlikey we want to much detail so at that point in time, 
-		// not likely that returning what transactions have is useful so removing it.
-		
-//		DepositAccountTransactionMapper transactionMapper = new DepositAccountTransactionMapper();
-//		String transactionSchema = "Select " + transactionMapper.schema();
-//		Collection<DepositAccountTransactionData> depositAccountTransactionDatas = this.jdbcTemplate.query(transactionSchema, transactionMapper, new Object[]{});
-		
-//		for (DepositAccountData depositAccountData : depositAccountDatas) {
-//			List<DepositAccountTransactionData> myAccountTransactionDatas = new ArrayList<DepositAccountTransactionData>();
-//			
-//			for (DepositAccountTransactionData depositAccountTransactionData : depositAccountTransactionDatas) {
-//				if (depositAccountData.getId().equals(depositAccountTransactionData.getAccountId())) {
-//					myAccountTransactionDatas.add(depositAccountTransactionData);
-//				}
-//				depositAccountData.setTransactions(myAccountTransactionDatas);
-//			}
-//		}
-		
-//		return depositAccountDatas;
 	}
 
 	@Override

mifosng-provider/src/main/java/org/mifosng/platform/saving/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java

@@ -363,18 +363,27 @@ public class DepositAccountWritePlatformServiceJpaRepositoryImpl implements Depo
 		if (account == null || account.isDeleted()) {
 			throw new DepositAccountNotFoundException(command.getId());
 		}
-		if(account.isRenewalAllowed()&&(new LocalDate().isAfter(account.maturesOnDate())||new LocalDate().isEqual(account.maturesOnDate()))){
-			if(account.getDepositStatus().equals(DepositAccountStatus.ACTIVE.getValue())){
-				final DepositAccount renewedAccount = this.depositAccountAssembler.assembleFrom(account,command);
+		
+		// FIXME - KW - extract whats in this if into a method that naturally describes what you are checking.
+		if (account.isRenewalAllowed()
+				&& (new LocalDate().isAfter(account.maturesOnDate()) || new LocalDate()
+						.isEqual(account.maturesOnDate()))) {
+			
+			// FIXME - KW - rather than getting the status from account and then checking, 
+			// add isActive behaviour on account so all places that use account dont have to repeat this type of checking.
+			if (account.getDepositStatus().equals(
+					DepositAccountStatus.ACTIVE.getValue())) {
+				final DepositAccount renewedAccount = this.depositAccountAssembler
+						.assembleFrom(account, command);
 				this.depositAccountRepository.save(renewedAccount);
 				account.closeDepositAccount(defaultDepositLifecycleStateMachine());
 				this.depositAccountRepository.save(account);
 				return new EntityIdentifier(renewedAccount.getId());
-			}else{
-				throw new DepositAccountReopenException(account.getMaturityDate());
 			}
-		}else{
+
 			throw new DepositAccountReopenException(account.getMaturityDate());
 		}
+
+		throw new DepositAccountReopenException(account.getMaturityDate());
 	}
 }