addres review comments for Client Identifiers

mifosng-provider/build.gradle

@@ -19,7 +19,7 @@ apply plugin: 'project-report'
 project.ext.springVersion = '3.1.1.RELEASE'
 project.ext.springSecurityVersion = '3.1.0.RELEASE'
 project.ext.hibernateValidatorVersion = '4.3.0.Final'
-project.ext.hibernateCoreVersion = '3.6.10.Final'
+project.ext.hibernateCoreVersion = '4.1.3.Final'
 project.ext.jerseyVersion = '1.13'
 project.ext.springDataJpaVersion = '1.0.3.RELEASE'
 

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

@@ -31,11 +31,9 @@ public class Client extends AbstractAuditableCustom<AppUser, Long> {
 	@Column(name = "lastname", length=50)
     private String       lastName;
     
-    @SuppressWarnings("unused")
    	@Column(name = "display_name", length=50)
     private String       displayName;
 
-    @SuppressWarnings("unused")
 	@Column(name = "joining_date")
     @Temporal(TemporalType.DATE)
     private Date         joiningDate;
@@ -157,4 +155,58 @@ public class Client extends AbstractAuditableCustom<AppUser, Long> {
 	public boolean isOfficeIdentifiedBy(final Long officeId) {
 		return this.office.identifiedBy(officeId);
 	}
+
+	public Office getOffice() {
+		return office;
+	}
+
+	public void setOffice(Office office) {
+		this.office = office;
+	}
+
+	public String getFirstName() {
+		return firstName;
+	}
+
+	public void setFirstName(String firstName) {
+		this.firstName = firstName;
+	}
+
+	public String getLastName() {
+		return lastName;
+	}
+
+	public void setLastName(String lastName) {
+		this.lastName = lastName;
+	}
+
+	public String getDisplayName() {
+		return displayName;
+	}
+
+	public void setDisplayName(String displayName) {
+		this.displayName = displayName;
+	}
+
+	public Date getJoiningDate() {
+		return joiningDate;
+	}
+
+	public void setJoiningDate(Date joiningDate) {
+		this.joiningDate = joiningDate;
+	}
+
+	public String getExternalId() {
+		return externalId;
+	}
+
+	public void setExternalId(String externalId) {
+		this.externalId = externalId;
+	}
+
+	public void setDeleted(boolean deleted) {
+		this.deleted = deleted;
+	}
+	
+	
 }

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

@@ -5,6 +5,7 @@ import javax.persistence.Entity;
 import javax.persistence.JoinColumn;
 import javax.persistence.ManyToOne;
 import javax.persistence.Table;
+import javax.persistence.UniqueConstraint;
 
 import org.apache.commons.lang.StringUtils;
 import org.mifosng.platform.api.commands.ClientIdentifierCommand;
@@ -13,7 +14,9 @@ import org.mifosng.platform.organisation.domain.CodeValue;
 import org.mifosng.platform.user.domain.AppUser;
 
 @Entity
-@Table(name = "m_client_identifier")
+@Table(name = "m_client_identifier", uniqueConstraints = {
+		@UniqueConstraint(columnNames = { "document_type_id", "document_key" }, name = "unique_identifier_key"),
+		@UniqueConstraint(columnNames = { "client_id", "document_type_id" }, name = "unique_client_identifier") })
 public class ClientIdentifier extends AbstractAuditableCustom<AppUser, Long> {
 
 	@ManyToOne

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

@@ -2,8 +2,15 @@ package org.mifosng.platform.client.domain;
 
 import org.springframework.data.jpa.repository.JpaRepository;
 import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.repository.query.Param;
 
 public interface ClientIdentifierRepository extends
-		JpaRepository<ClientIdentifier, Long>, JpaSpecificationExecutor<ClientIdentifier> {
+		JpaRepository<ClientIdentifier, Long>,
-	// no added behaviour
+		JpaSpecificationExecutor<ClientIdentifier> {
+
+	@Query("from ClientIdentifier ci where ci.documentType.id= :documentTypeId and ci.documentKey= :documentKey")
+	ClientIdentifier getClientIdentifierByDocumentTypeAndKey(
+			@Param("documentTypeId") Long documentTypeId,
+			@Param("documentKey") String documentKey);
 }

mifosng-provider/src/main/java/org/mifosng/platform/client/service/ClientWritePlatformServiceJpaRepositoryImpl.java

@@ -4,7 +4,6 @@ import org.apache.commons.lang.StringUtils;
 import org.mifosng.platform.api.commands.ClientCommand;
 import org.mifosng.platform.api.commands.ClientIdentifierCommand;
 import org.mifosng.platform.api.commands.NoteCommand;
-import org.mifosng.platform.api.data.ClientLookup;
 import org.mifosng.platform.api.data.EntityIdentifier;
 import org.mifosng.platform.client.domain.Client;
 import org.mifosng.platform.client.domain.ClientIdentifier;
@@ -44,7 +43,6 @@ public class ClientWritePlatformServiceJpaRepositoryImpl implements
 	private final OfficeRepository officeRepository;
 	private final NoteRepository noteRepository;
 	private final CodeValueRepository codeValueRepository;
-	private final ClientReadPlatformService clientReadPlatformService;
 
 	@Autowired
 	public ClientWritePlatformServiceJpaRepositoryImpl(
@@ -53,15 +51,13 @@ public class ClientWritePlatformServiceJpaRepositoryImpl implements
 			final ClientIdentifierRepository clientIdentifierRepository,
 			final OfficeRepository officeRepository,
 			NoteRepository noteRepository,
-			final CodeValueRepository codeValueRepository,
+			final CodeValueRepository codeValueRepository) {
-			final ClientReadPlatformService clientReadPlatformService) {
 		this.context = context;
 		this.clientRepository = clientRepository;
 		this.clientIdentifierRepository = clientIdentifierRepository;
 		this.officeRepository = officeRepository;
 		this.noteRepository = noteRepository;
 		this.codeValueRepository = codeValueRepository;
-		this.clientReadPlatformService = clientReadPlatformService;
 	}
 
 	@Transactional
@@ -216,6 +212,8 @@ public class ClientWritePlatformServiceJpaRepositoryImpl implements
 	@Override
 	public Long addClientIdentifier(
 			ClientIdentifierCommand clientIdentifierCommand) {
+		CodeValue documentType = null;
+		String documentKey = null;
 		try {
 			context.authenticatedUser();
 
@@ -230,32 +228,16 @@ public class ClientWritePlatformServiceJpaRepositoryImpl implements
 						clientIdentifierCommand.getClientId());
 			}
 
-			CodeValue documentType = this.codeValueRepository
+			documentType = this.codeValueRepository
 					.findOne(clientIdentifierCommand.getDocumentTypeId());
 			if (documentType == null) {
 				throw new CodeValueNotFoundException(
 						clientIdentifierCommand.getDocumentTypeId());
 			}
 
-			String documentKey = clientIdentifierCommand.getDocumentKey();
+			documentKey = clientIdentifierCommand.getDocumentKey();
 			String description = clientIdentifierCommand.getDescription();
 
-			/**
-			 * check if a client identifier of the given type already exists for
-			 * the client
-			 **/
-			if (clientReadPlatformService.existsClientIdentifier(
-					clientIdentifierCommand.getClientId(),
-					clientIdentifierCommand.getDocumentTypeId())) {
-				throw new DuplicateClientIdentifierException(
-						documentType.getLabel());
-			}
-
-			/*** Check if any Client has the same identifier ***/
-			checkIdentifierUniqueness(documentType.getId(),
-					documentType.getLabel(), documentKey,
-					clientIdentifierCommand.getClientId());
-
 			ClientIdentifier clientIdentifier = ClientIdentifier.createNew(
 					client, documentType, documentKey, description);
 
@@ -264,6 +246,13 @@ public class ClientWritePlatformServiceJpaRepositoryImpl implements
 			return clientIdentifier.getId();
 		} catch (DataIntegrityViolationException dve) {
 			logger.error(dve.getMessage(), dve);
+			if (dve.getMostSpecificCause().getMessage().contains("unique_client_identifier")) {
+				throw new DuplicateClientIdentifierException(
+						documentType.getLabel());
+			}else if(dve.getMostSpecificCause().getMessage().contains("unique_identifier_key")){
+				handleDuplicateClientIdentifierKeyScenario(documentType.getId(),
+						documentType.getLabel(), documentKey);
+			}
 			throw new PlatformDataIntegrityException(
 					"error.msg.clientIdentifier.unknown.data.integrity.issue",
 					"Unknown data integrity issue with resource.");
@@ -275,7 +264,9 @@ public class ClientWritePlatformServiceJpaRepositoryImpl implements
 	@Override
 	public EntityIdentifier updateClientIdentifier(
 			ClientIdentifierCommand clientIdentifierCommand) {
-
+		CodeValue documentType = null;
+		Long documentTypeId = null;
+		ClientIdentifier clientIdentifierForUpdate = null;
 		try {
 			context.authenticatedUser();
 
@@ -283,8 +274,7 @@ public class ClientWritePlatformServiceJpaRepositoryImpl implements
 					clientIdentifierCommand);
 			validator.validateForUpdate();
 
-			CodeValue documentType = null;
+			documentTypeId = clientIdentifierCommand.getDocumentTypeId();
-			Long documentTypeId = clientIdentifierCommand.getDocumentTypeId();
 			if (clientIdentifierCommand.isDocumentTypeChanged()
 					&& documentTypeId != null) {
 				documentType = this.codeValueRepository.findOne(documentTypeId);
@@ -294,51 +284,13 @@ public class ClientWritePlatformServiceJpaRepositoryImpl implements
 				}
 			}
 
-			ClientIdentifier clientIdentifierForUpdate = this.clientIdentifierRepository
+			clientIdentifierForUpdate = this.clientIdentifierRepository
 					.findOne(clientIdentifierCommand.getId());
 			if (clientIdentifierForUpdate == null) {
 				throw new ClientIdentifierNotFoundException(
 						clientIdentifierCommand.getId());
 			}
 			
-			/**
-			 * check if a client identifier of the given type already exists for
-			 * the client
-			 **/
-			if (clientIdentifierForUpdate.getDocumentType().getId() != clientIdentifierCommand
-					.getDocumentTypeId()) {
-				if (clientReadPlatformService.existsClientIdentifier(
-						clientIdentifierCommand.getClientId(),
-						clientIdentifierCommand.getDocumentTypeId())) {
-					throw new DuplicateClientIdentifierException(
-							documentType.getLabel());
-				}
-			}
-
-			/*** Check if any Client has the same identifier ***/
-		
-			if (clientIdentifierCommand.isDocumentTypeChanged()
-					&& clientIdentifierCommand.isDocumentKeyChanged()) {
-				checkIdentifierUniqueness(
-						clientIdentifierCommand.getDocumentTypeId(),
-						documentType.getLabel(),
-						clientIdentifierCommand.getDocumentKey(),
-						clientIdentifierForUpdate.getClient().getId());
-			} else if (clientIdentifierCommand.isDocumentTypeChanged()
-					&& !clientIdentifierCommand.isDocumentKeyChanged()) {
-				checkIdentifierUniqueness(
-						clientIdentifierCommand.getDocumentTypeId(),
-						documentType.getLabel(),
-						clientIdentifierForUpdate.getDocumentKey(),
-						clientIdentifierForUpdate.getClient().getId());
-			} else if (!clientIdentifierCommand.isDocumentTypeChanged()
-					&& clientIdentifierCommand.isDocumentKeyChanged()) {
-				checkIdentifierUniqueness(clientIdentifierForUpdate 
-						.getDocumentType().getId(), documentType.getLabel(),
-						clientIdentifierCommand.getDocumentKey(),
-						clientIdentifierForUpdate.getClient().getId());
-			}
-			
 			clientIdentifierForUpdate.update(clientIdentifierCommand,
 					documentType);
 
@@ -348,6 +300,30 @@ public class ClientWritePlatformServiceJpaRepositoryImpl implements
 			return new EntityIdentifier(clientIdentifierForUpdate.getId());
 		} catch (DataIntegrityViolationException dve) {
 			logger.error(dve.getMessage(), dve);
+			if (dve.getMostSpecificCause().getMessage().contains("unique_client_identifier")) {
+				throw new DuplicateClientIdentifierException(
+						documentType.getLabel());
+			}else if(dve.getMostSpecificCause().getMessage().contains("unique_identifier_key")){
+				/*** Throw error with details of existing customer with same identifier ***/
+				if (clientIdentifierCommand.isDocumentTypeChanged()
+						&& clientIdentifierCommand.isDocumentKeyChanged()) {
+					handleDuplicateClientIdentifierKeyScenario(
+							clientIdentifierCommand.getDocumentTypeId(),
+							documentType.getLabel(),
+							clientIdentifierCommand.getDocumentKey());
+				} else if (clientIdentifierCommand.isDocumentTypeChanged()
+						&& !clientIdentifierCommand.isDocumentKeyChanged()) {
+					handleDuplicateClientIdentifierKeyScenario(
+							clientIdentifierCommand.getDocumentTypeId(),
+							documentType.getLabel(),
+							clientIdentifierForUpdate.getDocumentKey());
+				} else if (!clientIdentifierCommand.isDocumentTypeChanged()
+						&& clientIdentifierCommand.isDocumentKeyChanged()) {
+					handleDuplicateClientIdentifierKeyScenario(clientIdentifierForUpdate 
+							.getDocumentType().getId(), documentType.getLabel(),
+							clientIdentifierCommand.getDocumentKey());
+				}
+			}
 			throw new PlatformDataIntegrityException(
 					"error.msg.clientIdentifier.unknown.data.integrity.issue",
 					"Unknown data integrity issue with resource.");
@@ -372,13 +348,12 @@ public class ClientWritePlatformServiceJpaRepositoryImpl implements
 	 * @param documentTypeLabel
 	 * @param documentKey
 	 */
-	private void checkIdentifierUniqueness(Long documentTypeId,
+	private void handleDuplicateClientIdentifierKeyScenario(Long documentTypeId,
-			String documentTypeLabel, String documentKey, Long callingClientId) {
+			String documentTypeLabel, String documentKey) {
-		ClientLookup clientLookup = clientReadPlatformService
+		Client client = this.clientIdentifierRepository
-				.getClientByIdentifier(documentTypeId, documentKey);
+				.getClientIdentifierByDocumentTypeAndKey(documentTypeId, documentKey).getClient();
-		if (clientLookup != null && clientLookup.getId()!=callingClientId) {
+		throw new DuplicateClientIdentifierException(
-			throw new DuplicateClientIdentifierException(clientLookup.getDisplayName(),
+				client.getDisplayName(), client.getOffice().getName(),
-					clientLookup.getOfficeName(), documentTypeLabel, documentKey);
+				documentTypeLabel, documentKey);
-		}
 	}
 }

mifosng-provider/src/main/java/org/mifosng/platform/staff/service/StaffReadPlatformServiceImpl.java

@@ -37,7 +37,7 @@ public class StaffReadPlatformServiceImpl implements StaffReadPlatformService {
 		}
 
 		@Override
-		public StaffData mapRow(final ResultSet rs, @SuppressWarnings("unused") final int rowNum)
+		public StaffData mapRow(final ResultSet rs, final int rowNum)
 				throws SQLException {
 
 			Long id = rs.getLong("id");