fix up update user functionality

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

@@ -86,4 +86,28 @@ public class UserCommand implements Serializable {
 	public boolean isUsernameChanged() {
 		return this.modifiedParameters.contains("username");
 	}
+	
+	public boolean isFirstnameChanged() {
+		return this.modifiedParameters.contains("firstname");
+	}
+	
+	public boolean isLastnameChanged() {
+		return this.modifiedParameters.contains("lastname");
+	}
+	
+	public boolean isEmailChanged() {
+		return this.modifiedParameters.contains("email");
+	}
+	
+	public boolean isPasswordChanged() {
+		return this.modifiedParameters.contains("password");
+	}
+	
+	public boolean isOfficeChanged() {
+		return this.modifiedParameters.contains("officeId");
+	}
+	
+	public boolean isRolesChanged() {
+		return this.modifiedParameters.contains("roles");
+	}
 }

mifosng-provider/src/main/java/org/mifosng/platform/api/errorhandling/JsonSyntaxExceptionMapper.java

@@ -0,0 +1,33 @@
+package org.mifosng.platform.api.errorhandling;
+
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.Status;
+import javax.ws.rs.ext.ExceptionMapper;
+import javax.ws.rs.ext.Provider;
+
+import org.mifosng.platform.api.data.ApiParameterError;
+import org.springframework.context.annotation.Scope;
+import org.springframework.stereotype.Component;
+
+import com.google.gson.JsonSyntaxException;
+
+/**
+ * An {@link ExceptionMapper} to map {@link JsonSyntaxException} thrown by platform into a HTTP API friendly format.
+ */
+@Provider
+@Component
+@Scope("singleton")
+public class JsonSyntaxExceptionMapper implements ExceptionMapper<JsonSyntaxException> {
+
+	@Override
+	public Response toResponse(final JsonSyntaxException exception) {
+		
+		String globalisationMessageCode = "error.msg.invalid.request.body";
+		String defaultUserMessage = "The JSON syntax provided in the body of the request is invalid: " + exception.getMessage();
+		
+		ApiParameterError error = ApiParameterError.generalError(globalisationMessageCode, defaultUserMessage);
+		
+		return Response.status(Status.BAD_REQUEST).entity(error).type(MediaType.APPLICATION_JSON).build();
+	}
+}

mifosng-provider/src/main/java/org/mifosng/platform/organisation/domain/Office.java

@@ -49,7 +49,8 @@ public class Office extends AbstractAuditableCustom<AppUser, Long> {
     @Temporal(TemporalType.DATE)
 	private Date openingDate;
 
-    @Column(name = "external_id", length=100)
+    @SuppressWarnings("unused")
+	@Column(name = "external_id", length=100)
 	private String externalId;
 
     public static Office headOffice(final String name, final LocalDate openingDate, final String externalId) {
@@ -67,7 +68,7 @@ public class Office extends AbstractAuditableCustom<AppUser, Long> {
         this.externalId = null;
     }
 
-    public Office(final Office parent, final String name, final LocalDate openingDate, final String externalId) {
+    private Office(final Office parent, final String name, final LocalDate openingDate, final String externalId) {
         this.parent = parent;
         this.openingDate = openingDate.toDateMidnight().toDate();
         if (parent != null) {
@@ -90,14 +91,6 @@ public class Office extends AbstractAuditableCustom<AppUser, Long> {
         this.children.add(office);
     }
     
-    public String getName() {
-    	return this.name;
-    }
-
-    public boolean isHeadOffice() {
-        return this.parent == null;
-    }
-
 	public void update(final OfficeCommand command) {
 		
 		if (command.isNameChanged()) {
@@ -131,15 +124,44 @@ public class Office extends AbstractAuditableCustom<AppUser, Long> {
 		generateHierarchy();
 	}
 
-	public boolean identifiedBy(String identifier) {
-		return identifier.equalsIgnoreCase(this.name) || identifier.equalsIgnoreCase(this.externalId);
-	}
-
 	public boolean identifiedBy(final Long id) {
 		return getId().equals(id);
 	}
 
-	public boolean hasAnOfficeInHierarchyWithId(Long officeId) {
+	public void generateHierarchy() {
+		
+		if (parent != null) {
+			this.hierarchy = this.parent.hierarchyOf(this.getId());
+		} else {
+			this.hierarchy = ".";
+		}
+	}
+
+	private String hierarchyOf(Long id) {
+		return this.hierarchy + id.toString() + ".";
+	}
+	
+    public String getName() {
+    	return this.name;
+    }
+
+	public String getHierarchy() {
+		return hierarchy;
+	}
+
+	public boolean hasParentOf(final Office office) {
+		boolean isParent = false;
+		if (this.parent != null) {
+			isParent = this.parent.equals(office);
+		}
+		return isParent;
+	}
+	
+	public boolean doesNotHaveAnOfficeInHierarchyWithId(Long officeId) {
+		return !this.hasAnOfficeInHierarchyWithId(officeId);
+	}
+	
+	private boolean hasAnOfficeInHierarchyWithId(Long officeId) {
 		
 		boolean match = false;
 		
@@ -160,33 +182,4 @@ public class Office extends AbstractAuditableCustom<AppUser, Long> {
 		
 		return match;
 	}
-
-	public boolean doesNotHaveAnOfficeInHierarchyWithId(Long officeId) {
-		return !this.hasAnOfficeInHierarchyWithId(officeId);
-	}
-
-	public void generateHierarchy() {
-		
-		if (parent != null) {
-			this.hierarchy = this.parent.hierarchyOf(this.getId());
-		} else {
-			this.hierarchy = ".";
-		}
-	}
-
-	private String hierarchyOf(Long id) {
-		return this.hierarchy + id.toString() + ".";
-	}
-
-	public String getHierarchy() {
-		return hierarchy;
-	}
-
-	public boolean hasParentOf(final Office office) {
-		boolean isParent = false;
-		if (this.parent != null) {
-			isParent = this.parent.equals(office);
-		}
-		return isParent;
-	}
 }

mifosng-provider/src/main/java/org/mifosng/platform/user/domain/AppUser.java

@@ -16,6 +16,7 @@ import javax.persistence.ManyToOne;
 import javax.persistence.Table;
 import javax.persistence.UniqueConstraint;
 
+import org.mifosng.platform.api.commands.UserCommand;
 import org.mifosng.platform.infrastructure.AbstractAuditableCustom;
 import org.mifosng.platform.infrastructure.PlatformUser;
 import org.mifosng.platform.organisation.domain.Office;
@@ -138,6 +139,29 @@ public class AppUser extends AbstractAuditableCustom<AppUser, Long> implements P
 	public void updatePassword(final String encodePassword) {
 		this.password = encodePassword;
 	}
+	
+	public void update(final Set<Role> allRoles, final Office office, final UserCommand userCommand) {
+		
+		if (userCommand.isRolesChanged() && !allRoles.isEmpty()) {
+			this.roles.clear();
+			this.roles = allRoles;
+		}
+		if (userCommand.isOfficeChanged()) {
+			this.office = office;
+		}
+		if (userCommand.isUsernameChanged()) {
+			this.username = userCommand.getUsername();
+		}
+		if (userCommand.isFirstnameChanged()) {
+			this.firstname = userCommand.getFirstname();
+		}
+		if (userCommand.isLastnameChanged()) {
+			this.lastname = userCommand.getLastname();
+		}
+		if (userCommand.isEmailChanged()) {
+			this.email = userCommand.getEmail();
+		}
+	}
 
     @Override
     public Collection<GrantedAuthority> getAuthorities() {
@@ -184,6 +208,14 @@ public class AppUser extends AbstractAuditableCustom<AppUser, Long> implements P
     public boolean isEnabled() {
         return this.enabled;
     }
+    
+    public String getFirstname() {
+		return firstname;
+	}
+
+	public String getLastname() {
+		return lastname;
+	}
 
     public String getEmail() {
         return this.email;
@@ -193,97 +225,10 @@ public class AppUser extends AbstractAuditableCustom<AppUser, Long> implements P
         return this.roles;
     }
 
-    public String getRoleNames() {
-        StringBuilder roleNames = new StringBuilder();
-
-        for (Role role : this.roles) {
-            roleNames.append(role.toData().getName()).append(' ');
-        }
-
-        return roleNames.toString();
-    }
-
-    public void setUserIdAs(final Long id) {
-        this.setId(id);
-    }
-
     public Office getOffice() {
         return this.office;
     }
 
-    public boolean isHeadOfficeUser() {
-        boolean headOfficeUser = false;
-        if (this.office != null) {
-            headOfficeUser = this.office.isHeadOffice();
-        }
-        return headOfficeUser;
-    }
-
-    public boolean hasPermissionTo(final String permissionCode) {
-        boolean match = false;
-        for (Role role : this.roles) {
-            if (role.hasPermissionTo(permissionCode)) {
-                match = true;
-                break;
-            }
-        }
-
-        return match;
-    }
-    
-    public boolean hasNotPermissionForAnyOf(final String... permissionCodes) {
-    	boolean hasNotPermission = true;
-    	for (String permissionCode : permissionCodes) {
-			boolean checkPermission = this.hasPermissionTo(permissionCode);
-			if (checkPermission) {
-				hasNotPermission = false;
-				break;
-			}
-		}
-    	return hasNotPermission;
-    }
-
-    public boolean hasNotPermissionTo(final String permissionCode) {
-        return !this.hasPermissionTo(permissionCode);
-    }
-
-	public boolean canAccess(Long officeId) {
-		return this.office.hasAnOfficeInHierarchyWithId(officeId);
-	}
-
-	public String getFirstname() {
-		return firstname;
-	}
-
-	public String getLastname() {
-		return lastname;
-	}
-
-	/**
-	 * When updating details, any parameters with null values are ignored.
-	 */
-	public void update(Set<Role> allRoles, Office office, String username, String firstname, String lastname, String email) {
-		if (!allRoles.isEmpty()) {
-			this.roles.clear();
-			this.roles = allRoles;
-		}
-		if (office != null) {
-			this.office = office;
-		}
-		if (username != null) {
-			this.username = username;
-		}
-		if (firstname != null) {
-			this.firstname = firstname;
-		}
-		if (lastname != null) {
-			this.lastname = lastname;
-		}
-		if (email != null) {
-			this.email = email;
-		}
-	}
-
 	public boolean canNotApproveLoanInPast() {
 		return hasNotPermissionForAnyOf("CAN_APPROVE_LOAN_IN_THE_PAST_ROLE", "PORTFOLIO_MANAGEMENT_SUPER_USER_ROLE");
 	}
@@ -303,4 +248,28 @@ public class AppUser extends AbstractAuditableCustom<AppUser, Long> implements P
 	public boolean canNotMakeRepaymentOnLoanInPast() {
 		return hasNotPermissionForAnyOf("CAN_MAKE_LOAN_REPAYMENT_IN_THE_PAST_ROLE", "PORTFOLIO_MANAGEMENT_SUPER_USER_ROLE");
 	}
+	
+	public boolean hasNotPermissionForAnyOf(final String... permissionCodes) {
+		boolean hasNotPermission = true;
+		for (String permissionCode : permissionCodes) {
+			boolean checkPermission = this.hasPermissionTo(permissionCode);
+			if (checkPermission) {
+				hasNotPermission = false;
+				break;
+			}
+		}
+		return hasNotPermission;
+	}
+
+	private boolean hasPermissionTo(final String permissionCode) {
+		boolean match = false;
+		for (Role role : this.roles) {
+			if (role.hasPermissionTo(permissionCode)) {
+				match = true;
+				break;
+			}
+		}
+
+		return match;
+	}
 }

mifosng-provider/src/main/java/org/mifosng/platform/user/domain/Role.java

@@ -74,7 +74,7 @@ public class Role extends AbstractAuditableCustom<AppUser, Long> {
 		return data;
 	}
 
-	public void update(final RoleCommand command, List<Permission> selectedPermissions) {
+	public void update(final RoleCommand command, final List<Permission> selectedPermissions) {
 		if (command.isNameChanged()) {
 			this.name = command.getName();
 		}

mifosng-provider/src/main/java/org/mifosng/platform/user/service/AppUserWritePlatformServiceJpaRepositoryImpl.java

@@ -67,7 +67,7 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
 			UserCommandValidator validator = new UserCommandValidator(command);
 			validator.validateForCreate();
 			
-			Set<Role> allRoles = assembleSetOfRoles(command.getRoles());
+			final Set<Role> allRoles = assembleSetOfRoles(command);
 
 			Office office = this.officeRepository.findOne(command.getOfficeId());
 			if (office == null) {
@@ -101,7 +101,7 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
 
 	@Transactional
 	@Override
-	public Long updateUser(UserCommand command) {
+	public Long updateUser(final UserCommand command) {
 		
 		try {
 			context.authenticatedUser();
@@ -109,10 +109,10 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
 			UserCommandValidator validator = new UserCommandValidator(command);
 			validator.validateForUpdate();
 			
-			Set<Role> allRoles = assembleSetOfRoles(command.getRoles());
+			final Set<Role> allRoles = assembleSetOfRoles(command);
 
 			Office office = null;
-			if (command.getOfficeId() != null) {
+			if (command.isOfficeChanged()) {
 				office = this.officeRepository.findOne(command.getOfficeId());
 				if (office == null) {
 					throw new OfficeNotFoundException(command.getOfficeId());
@@ -124,7 +124,7 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
 				throw new UserNotFoundException(command.getId());
 			}
 			
-			userToUpdate.update(allRoles, office, command.getUsername(), command.getFirstname(), command.getLastname(), command.getEmail());
+			userToUpdate.update(allRoles, office, command);
 			this.appUserRepository.saveAndFlush(userToUpdate);
 			
 			if (command.getPassword() != null || command.getRepeatPassword() != null) {
@@ -145,9 +145,12 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
 		}
 	}
 	
-	private Set<Role> assembleSetOfRoles(final String[] rolesArray) {
+	private Set<Role> assembleSetOfRoles(final UserCommand command) {
-		Set<Role> allRoles = new HashSet<Role>();
+		
-		if (!ObjectUtils.isEmpty(rolesArray)) {
+		final Set<Role> allRoles = new HashSet<Role>();
+		
+		String[] rolesArray = command.getRoles();
+		if (command.isRolesChanged() && !ObjectUtils.isEmpty(rolesArray)) {
 			for (String roleId : rolesArray) {
 				Long id = Long.valueOf(roleId);
 				Role role = this.roleRepository.findOne(id);
@@ -157,6 +160,7 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
 				allRoles.add(role);
 			}
 		}
+		
 		return allRoles;
 	}