review comments incorporated

This commit is contained in:
Nayan Ambali
2013-03-15 15:32:02 +05:30
parent 772fe35a36
commit 903a958783
5 changed files with 80 additions and 53 deletions
@@ -1 +1,5 @@
ALTER TABLE m_group MODIFY name VARCHAR(100) NOT NULL;
ALTER TABLE m_group MODIFY name VARCHAR(100) NOT NULL;
ALTER TABLE m_group DROP INDEX name;
ALTER TABLE m_group DROP INDEX external_id;
ALTER TABLE m_group ADD UNIQUE INDEX name (name, level_Id);
ALTER TABLE m_group ADD UNIQUE INDEX external_id (external_id, level_Id);
@@ -95,23 +95,23 @@ public final class GroupCommandFromApiJsonDeserializer extends AbstractFromApiJs
}
/*
* OfficeId is mandatory field
* Either OfficeId or ParentId is mandatory to have value
*/
final String officeIdParameterName = "officeId";
final Long officeId = this.fromApiJsonHelper.extractLongNamed(officeIdParameterName, element);
baseDataValidator.reset().parameter(officeIdParameterName).value(officeId).notNull().integerGreaterThanZero();
final String parentIdParameterName = "parentId";
if (this.fromApiJsonHelper.parameterExists(parentIdParameterName, element)) {
final Long parentId = this.fromApiJsonHelper.extractLongNamed(parentIdParameterName, element);
baseDataValidator.reset().parameter(parentIdParameterName).value(parentId).notNull().integerGreaterThanZero();
}else{
final String officeIdParameterName = "officeId";
final Long officeId = this.fromApiJsonHelper.extractLongNamed(officeIdParameterName, element);
baseDataValidator.reset().parameter(officeIdParameterName).value(officeId).notNull().integerGreaterThanZero();
}
final String staffIdParameterName = "staffId";
if (this.fromApiJsonHelper.parameterExists(staffIdParameterName, element)) {
final Long staffId = this.fromApiJsonHelper.extractLongNamed(staffIdParameterName, element);
baseDataValidator.reset().parameter(staffIdParameterName).value(staffId).notNull().integerGreaterThanZero();
baseDataValidator.reset().parameter(staffIdParameterName).value(staffId).ignoreIfNull().integerGreaterThanZero();
}
if (!dataValidationErrors.isEmpty()) { throw new PlatformApiDataValidationException("validation.msg.validation.errors.exist",
@@ -146,13 +146,13 @@ public final class GroupCommandFromApiJsonDeserializer extends AbstractFromApiJs
final String parentIdParameterName = "parentId";
if (this.fromApiJsonHelper.parameterExists(parentIdParameterName, element)) {
final Long parentId = this.fromApiJsonHelper.extractLongNamed(parentIdParameterName, element);
baseDataValidator.reset().parameter(parentIdParameterName).value(parentId).notNull().integerGreaterThanZero();
baseDataValidator.reset().parameter(parentIdParameterName).value(parentId).ignoreIfNull().integerGreaterThanZero();
}
final String staffIdParameterName = "staffId";
if (this.fromApiJsonHelper.parameterExists(staffIdParameterName, element)) {
final Long staffId = this.fromApiJsonHelper.extractLongNamed(staffIdParameterName, element);
baseDataValidator.reset().parameter(staffIdParameterName).value(staffId).notNull().integerGreaterThanZero();
baseDataValidator.reset().parameter(staffIdParameterName).value(staffId).ignoreIfNull().integerGreaterThanZero();
}
if (!dataValidationErrors.isEmpty()) { throw new PlatformApiDataValidationException("validation.msg.validation.errors.exist",
@@ -164,9 +164,6 @@ public final class GroupCommandFromApiJsonDeserializer extends AbstractFromApiJs
final Type typeOfMap = new TypeToken<Map<String, Object>>() {}.getType();
/*
* Office updated is not supported
*/
final Set<String> supportedParametersUnassignStaff = new HashSet<String>(Arrays.asList("staffId"));
this.fromApiJsonHelper.checkForUnsupportedParameters(typeOfMap, json, supportedParametersUnassignStaff);
@@ -177,10 +174,8 @@ public final class GroupCommandFromApiJsonDeserializer extends AbstractFromApiJs
final DataValidatorBuilder baseDataValidator = new DataValidatorBuilder(dataValidationErrors).resource("group");
final String staffIdParameterName = "staffId";
if (this.fromApiJsonHelper.parameterExists(staffIdParameterName, element)) {
final Long staffId = this.fromApiJsonHelper.extractLongNamed(staffIdParameterName, element);
baseDataValidator.reset().parameter(staffIdParameterName).value(staffId).notNull().integerGreaterThanZero();
}
final Long staffId = this.fromApiJsonHelper.extractLongNamed(staffIdParameterName, element);
baseDataValidator.reset().parameter(staffIdParameterName).value(staffId).notNull().integerGreaterThanZero();
if (!dataValidationErrors.isEmpty()) { throw new PlatformApiDataValidationException("validation.msg.validation.errors.exist",
"Validation errors exist.", dataValidationErrors); }
@@ -9,6 +9,7 @@ import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.Collection;
import org.mifosplatform.infrastructure.core.domain.JdbcSupport;
import org.mifosplatform.infrastructure.core.service.TenantAwareRoutingDataSource;
import org.mifosplatform.infrastructure.security.service.PlatformSecurityContext;
import org.mifosplatform.portfolio.group.data.GroupLevelData;
@@ -54,9 +55,9 @@ public class GroupLevelReadPlatformServiceImpl implements GroupLevelReadPlatform
final Long levelId = rs.getLong("id");
final String levelName = rs.getString("levelName");
final Long parentLevelId = rs.getLong("parentLevelId");
final Long parentLevelId = JdbcSupport.getLong(rs, "parentLevelId");
final String parentLevelName = rs.getString("parentName");
final Long childLevelId = rs.getLong("childLevelId");
final Long childLevelId = JdbcSupport.getLong(rs, "childLevelId");
final String childLevelName = rs.getString("childLevelName");
final boolean superParent = rs.getBoolean("superParent");
final boolean recursable = rs.getBoolean("recursable");
@@ -34,6 +34,7 @@ import org.mifosplatform.portfolio.group.domain.Group;
import org.mifosplatform.portfolio.group.domain.GroupRepository;
import org.mifosplatform.portfolio.group.exception.GroupLevelNotFoundException;
import org.mifosplatform.portfolio.group.exception.GroupNotFoundException;
import org.mifosplatform.useradministration.domain.AppUser;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
@@ -64,16 +65,20 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService {
public Collection<GroupData> retrieveAllGroups(final String extraCriteria) {
this.context.authenticatedUser();
AppUser currentUser = context.authenticatedUser();
String hierarchy = currentUser.getOffice().getHierarchy();
String hierarchySearchString = hierarchy + "%";
final GroupDataMapper rm = new GroupDataMapper();
String sql = "select " + rm.groupSchema() + " where g.is_deleted=0";
String sql = "select " + rm.groupSchema() + " where o.hierarchy like ? and g.is_deleted=0 ";
if (StringUtils.isNotBlank(extraCriteria)) {
sql += " and (" + extraCriteria + ")";
}
return this.jdbcTemplate.query(sql, rm, new Object[] {});
return this.jdbcTemplate.query(sql, rm, new Object[] {hierarchySearchString});
}
@Override
@@ -334,8 +339,8 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService {
public String groupSchema() {
return "g.office_id as officeId, g.parent_id as parentId , o.name as officeName,"
+ " g.id as id, g.external_id as externalId, g.name as name , s.display_name as staffName , pg.name as"
+ " parentName , g.staff_id as staffId , g.hierarchy as hierarchy from m_group g join m_office o on o.id = g.office_id left join "
+ "m_staff s on s.id = g.staff_id left join m_group pg on pg.id = g.parent_id";
+ " parentName , g.staff_id as staffId , g.hierarchy as hierarchy from m_group g join m_office o on o.id = g.office_id left join"
+ " m_staff s on s.id = g.staff_id left join m_group pg on pg.id = g.parent_id";
}
@Override
@@ -346,7 +351,7 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService {
final String externalId = rs.getString("externalId");
final Long officeId = rs.getLong("officeId");
final String officeName = rs.getString("officeName");
final Long parentId = rs.getLong("parentId");
final Long parentId = JdbcSupport.getLong(rs, "parentId");
final String parentName = rs.getString("parentName");
final Long staffId = JdbcSupport.getLong(rs, "staffId");
final String staffName = rs.getString("staffName");
@@ -73,6 +73,8 @@ public class GroupWritePlatformServiceJpaRepositoryImpl implements GroupWritePla
@Transactional
@Override
public CommandProcessingResult createGroup(final JsonCommand command) {
GroupLevel groupLevel = null;
try {
this.context.authenticatedUser();
@@ -84,7 +86,7 @@ public class GroupWritePlatformServiceJpaRepositoryImpl implements GroupWritePla
final Long parentId = command.longValueOfParameterNamed("parentId");
Long officeId = null;
GroupLevel groupLevel = null;
Group parentGroup = null;
if (parentId == null) {
@@ -152,7 +154,7 @@ public class GroupWritePlatformServiceJpaRepositoryImpl implements GroupWritePla
.build();
} catch (final DataIntegrityViolationException dve) {
handleGroupDataIntegrityIssues(command, dve);
handleGroupDataIntegrityIssues(command, dve , groupLevel );
return new CommandProcessingResult(Long.valueOf(-1));
}
}
@@ -161,6 +163,8 @@ public class GroupWritePlatformServiceJpaRepositoryImpl implements GroupWritePla
@Override
public CommandProcessingResult updateGroup(final Long grouptId, final JsonCommand command) {
GroupLevel groupLevel = null;
try {
this.context.authenticatedUser();
@@ -197,12 +201,16 @@ public class GroupWritePlatformServiceJpaRepositoryImpl implements GroupWritePla
if (command.isChangeInLongParameterNamed(staffIdParamName, presentStaffId)) {
final Long newValue = command.longValueOfParameterNamed(staffIdParamName);
actualChanges.put(staffIdParamName, newValue);
final Staff newStaff = this.staffRepository.findByOffice(newValue, officeId);
if (newStaff == null) { throw new StaffNotFoundException(newValue); }
Staff newStaff = null;
if(newValue != null){
newStaff = this.staffRepository.findByOffice(newValue, officeId);
if (newStaff == null) { throw new StaffNotFoundException(newValue); }
}
groupForUpdate.setStaff(newStaff);
}
final GroupLevel groupLevel = this.groupLevelRepository.findOne(groupForUpdate.getGroupLevel().getId());
groupLevel = this.groupLevelRepository.findOne(groupForUpdate.getGroupLevel().getId());
/*
* Ignoring parentId param, if group for update is super parent.
@@ -216,27 +224,28 @@ public class GroupWritePlatformServiceJpaRepositoryImpl implements GroupWritePla
final Long newValue = command.longValueOfParameterNamed(parentIdParamName);
actualChanges.put(parentIdParamName, newValue);
final Group newParentGroup = this.groupRepository.findOne(newValue);
Group newParentGroup = null;
if (newValue != null) {
newParentGroup = this.groupRepository.findOne(newValue);
if (newParentGroup == null || newParentGroup.isDeleted()) { throw new StaffNotFoundException(newValue); }
if (newParentGroup == null || newParentGroup.isDeleted()) { throw new StaffNotFoundException(newValue); }
if (!newParentGroup.isOfficeIdentifiedBy(officeId)) {
final String errorMessage = "Group and parent group must have the same office";
throw new InvalidOfficeException("group", "attach.to.parent.group", errorMessage);
}
/*
* If Group is not super parent then validate group
* level's parent level is same as group parent's level
* this check makes sure new group is added at immediate
* next level in hierarchy
*/
if (!newParentGroup.isOfficeIdentifiedBy(officeId)) {
final String errorMessage = "Group and parent group must have the same office";
throw new InvalidOfficeException("group", "attach.to.parent.group", errorMessage);
if (!groupForUpdate.getGroupLevel().isIdentifiedByParentId(newParentGroup.getGroupLevel().getId())) {
final String errorMessage = "Parent group's level is not equal to child level's parent level ";
throw new InvalidGroupLevelException("add", "invalid.level", errorMessage);
}
}
/*
* If Group is not super parent then validate group level's
* parent level is same as group parent's level this check
* makes sure new group is added at immediate next level in
* hierarchy
*/
if (!groupForUpdate.getGroupLevel().isIdentifiedByParentId(newParentGroup.getGroupLevel().getId())) {
final String errorMessage = "Parent group's level is not equal to child level's parent level ";
throw new InvalidGroupLevelException("add", "invalid.level", errorMessage);
}
groupForUpdate.setParent(newParentGroup);
}
}
@@ -262,7 +271,7 @@ public class GroupWritePlatformServiceJpaRepositoryImpl implements GroupWritePla
.build();
} catch (final DataIntegrityViolationException dve) {
handleGroupDataIntegrityIssues(command, dve);
handleGroupDataIntegrityIssues(command, dve , groupLevel);
return new CommandProcessingResult(Long.valueOf(-1));
}
}
@@ -378,14 +387,27 @@ public class GroupWritePlatformServiceJpaRepositoryImpl implements GroupWritePla
* Guaranteed to throw an exception no matter what the data integrity issue
* is.
*/
private void handleGroupDataIntegrityIssues(final JsonCommand command, final DataIntegrityViolationException dve) {
private void handleGroupDataIntegrityIssues(final JsonCommand command, final DataIntegrityViolationException dve, GroupLevel groupLevel) {
final Throwable realCause = dve.getMostSpecificCause();
String errorMessageForUser = null;
String errorMessageForMachine = null;
if (realCause.getMessage().contains("external_id")) {
throw new PlatformDataIntegrityException("error.msg.group.duplicate.externalId", "Group with externalId {0} already exists",
"externalId", command.stringValueOfParameterNamed("externalId"));
} else if (realCause.getMessage().contains("name")) { throw new PlatformDataIntegrityException("error.msg.group.duplicate.name",
"Group with name {0} already exists", "name", command.stringValueOfParameterNamed("name")); }
errorMessageForUser = groupLevel.getLevelName() + " with externalId {0} already exists";
errorMessageForMachine = "error.msg." + groupLevel.getLevelName().toLowerCase() + ".duplicate.externalId";
throw new PlatformDataIntegrityException(errorMessageForMachine, errorMessageForUser, "externalId",
command.stringValueOfParameterNamed("externalId"));
} else if (realCause.getMessage().contains("name")) {
errorMessageForUser = groupLevel.getLevelName() + " with name {0} already exists";
errorMessageForMachine = "error.msg." + groupLevel.getLevelName().toLowerCase() + ".duplicate.name";
throw new PlatformDataIntegrityException(errorMessageForMachine, errorMessageForUser, "name",
command.stringValueOfParameterNamed("name"));
}
logger.error(dve.getMessage(), dve);
throw new PlatformDataIntegrityException("error.msg.group.unknown.data.integrity.issue",