From bbb7d7efad58ec34935e87f48c816bc7a9b5b8d1 Mon Sep 17 00:00:00 2001 From: Gao Jun Date: Tue, 3 Feb 2015 16:28:43 +0800 Subject: [PATCH] SAK-29036 Bug of special rank: only check part of user eid --- .../api/app/messageforums/Rank.java | 6 +- .../messageforums/DiscussionForumTool.java | 92 +++++++------------ .../tool/messageforums/ui/ForumRankBean.java | 14 +-- .../app/messageforums/RankManagerImpl.java | 12 +-- .../messageforums/dao/hibernate/Rank.hbm.xml | 17 ++-- .../messageforums/dao/hibernate/RankImpl.java | 14 +-- .../src/sql/mysql/SAK-29036.sql | 38 ++++++++ .../src/sql/oracle/SAK-29036.sql | 43 +++++++++ 8 files changed, 139 insertions(+), 97 deletions(-) create mode 100644 msgcntr/messageforums-hbm/src/sql/mysql/SAK-29036.sql create mode 100644 msgcntr/messageforums-hbm/src/sql/oracle/SAK-29036.sql diff --git a/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/Rank.java b/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/Rank.java index 6fed2ad8cf0d..670feedf6903 100644 --- a/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/Rank.java +++ b/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/Rank.java @@ -20,6 +20,8 @@ **********************************************************************************/ package org.sakaiproject.api.app.messageforums; +import java.util.Set; + public interface Rank extends MutableEntity { public final static String RANK_TYPE_INDIVIDUAL = "1"; public final static String RANK_TYPE_POST_COUNT = "2"; @@ -33,9 +35,9 @@ public interface Rank extends MutableEntity { public void setType(String ranktype); - public String getAssignTo(); + public Set getAssignToIds(); - public void setAssignTo(String assignTo); + public void setAssignToIds(Set assignToIds); public long getMinPosts(); diff --git a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java index 2b989fab8b0e..561de5333f98 100644 --- a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java +++ b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java @@ -28,7 +28,6 @@ import java.util.Date; import java.util.Locale; import java.util.HashMap; -import java.util.TreeMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -61,6 +60,7 @@ import net.sf.json.JSONSerializer; import net.sf.json.JsonConfig; +import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.sakaiproject.api.app.messageforums.Area; @@ -77,12 +77,10 @@ import org.sakaiproject.api.app.messageforums.Message; import org.sakaiproject.api.app.messageforums.MessageForumsMessageManager; import org.sakaiproject.api.app.messageforums.MessageForumsTypeManager; -import org.sakaiproject.api.app.messageforums.MessageMoveHistory; import org.sakaiproject.api.app.messageforums.OpenForum; import org.sakaiproject.api.app.messageforums.PermissionLevel; import org.sakaiproject.api.app.messageforums.PermissionLevelManager; import org.sakaiproject.api.app.messageforums.PermissionsMask; -import org.sakaiproject.api.app.messageforums.PrivateMessage; import org.sakaiproject.api.app.messageforums.Rank; import org.sakaiproject.api.app.messageforums.RankImage; import org.sakaiproject.api.app.messageforums.RankManager; @@ -100,9 +98,7 @@ import org.sakaiproject.authz.cover.AuthzGroupService; import org.sakaiproject.authz.cover.SecurityService; import org.sakaiproject.component.app.messageforums.MembershipItem; -import org.sakaiproject.component.app.messageforums.dao.hibernate.MessageMoveHistoryImpl; import org.sakaiproject.component.app.messageforums.dao.hibernate.util.comparator.ForumBySortIndexAscAndCreatedDateDesc; -import org.sakaiproject.component.app.messageforums.dao.hibernate.RankImpl; import org.sakaiproject.component.cover.ComponentManager; import org.sakaiproject.component.cover.ServerConfigurationService; import org.sakaiproject.content.api.ContentResource; @@ -125,14 +121,12 @@ import org.sakaiproject.event.cover.EventTrackingService; import org.sakaiproject.exception.IdUnusedException; import org.sakaiproject.service.gradebook.shared.Assignment; -import org.sakaiproject.service.gradebook.shared.CommentDefinition; import org.sakaiproject.service.gradebook.shared.GradeDefinition; import org.sakaiproject.service.gradebook.shared.GradebookService; import org.sakaiproject.site.api.Group; import org.sakaiproject.site.api.Site; import org.sakaiproject.site.cover.SiteService; import org.sakaiproject.thread_local.cover.ThreadLocalManager; -import org.sakaiproject.tool.api.Placement; import org.sakaiproject.tool.api.ToolSession; import org.sakaiproject.tool.cover.SessionManager; import org.sakaiproject.tool.cover.ToolManager; @@ -9429,18 +9423,14 @@ public void saveRank(Rank newRank) { if (LOG.isDebugEnabled()) LOG.debug("saveRank: RANK_TYPE_INDIVIDUAL"); newRank.setType(Rank.RANK_TYPE_INDIVIDUAL); - String assigned_to_display = constructAssignedToDisplay(); - newRank.setAssignToDisplay(assigned_to_display); - String assigned_to = constructAssignedTo(); - if (LOG.isDebugEnabled()) LOG.debug("user_eid = " + assigned_to); - newRank.setAssignTo(assigned_to); + Set assignToIds = constructAssignToIds(); + if (LOG.isDebugEnabled()) LOG.debug("user_id = " + assignToIds); + newRank.setAssignToIds(assignToIds); newRank.setMinPosts(0); rankManager.saveRank(newRank); } else if (Rank.RANK_TYPE_POST_COUNT.equalsIgnoreCase(selectedRankType)) { // by # of post if (LOG.isDebugEnabled()) LOG.debug("saveRank: RANK_TYPE_POST_COUNT "); - newRank.setAssignTo(null); - newRank.setAssignToDisplay(null); newRank.setType(Rank.RANK_TYPE_POST_COUNT); rankManager.saveRank(newRank); } else { @@ -9508,8 +9498,9 @@ public String processActionViewRanks() { setErrorMessage(getResourceBundleString(INSUFFICIENT_PRIVILEGES_TO_EDIT_RANKS)); return gotoMain(); } - List ranklist = new ArrayList(); + List ranklist = new ArrayList(); ranklist = rankManager.getRankList(getSiteId()); + constructAssignedToDisplay(ranklist); setRankBeanList(ranklist); return VIEW_RANK; } @@ -9532,11 +9523,10 @@ public String processActionEditRank() { if (Rank.RANK_TYPE_INDIVIDUAL.equalsIgnoreCase(rankBean.getType())) { // get selected individuals for editing - String useridlistString = thisrank.getAssignTo(); - if ((useridlistString == null) || (useridlistString.length() <= 0)) { + Set assignToIds = thisrank.getAssignToIds(); + if (assignToIds == null || assignToIds.isEmpty()) { return VIEW_RANK; // not going anywhere. AssignTo should have at least 1 user. } - StringTokenizer st = new StringTokenizer(useridlistString, ASSIGNEDTO_DELIMITER, false); StringBuffer memberitemidlist = new StringBuffer(); this.courseMemberMap = membershipManager.getFilteredCourseMembers(true, null); List members = membershipManager.convertMemberMapToList(courseMemberMap); @@ -9546,18 +9536,15 @@ public String processActionEditRank() { MembershipItem item = (MembershipItem) i.next(); User itemUser = item.getUser(); if (itemUser != null) { - membersKeyOnUserId.put(itemUser.getEid(), item); + membersKeyOnUserId.put(itemUser.getId(), item); } else { // okay ,not a User membershipItem, could be Group, or Role... } } - Set userIds = new HashSet(); - while (st.hasMoreTokens()) { - String userid = (String) st.nextToken().trim(); - if (membersKeyOnUserId.containsKey(userid)) { - // exist in courseMemberMap - memberitemidlist.append(membersKeyOnUserId.get(userid).getId()); + for (String userId: assignToIds) { + if (membersKeyOnUserId.containsKey(userId)) { + memberitemidlist.append(membersKeyOnUserId.get(userId).getId()); memberitemidlist.append(AGGREGATE_DELIMITER); } } @@ -9699,50 +9686,35 @@ public List getSelectedComposeToList() { return selectedComposeToList; } - public String constructAssignedToDisplay() { - // store the user display name, separated by some delimiter. - // for faster performance. - String assignedtodisplay = ""; - - // store this in a Map to be sorted - Map NamesMap = new HashMap(); - for (int i = 0; i < selectedComposeToList.size(); i++) { - MembershipItem membershipItem = (MembershipItem) courseMemberMap.get(selectedComposeToList.get(i)); - if (membershipItem != null) { - NamesMap.put(membershipItem.getUser().getFirstName() + membershipItem.getUser().getEid(), membershipItem); + private void constructAssignedToDisplay(List ranks) { + courseMemberMap = membershipManager.getFilteredCourseMembers(true, null); + Map memberIdNameMap = new HashMap(); + for (Object o: courseMemberMap.values()) { + MembershipItem item = (MembershipItem) o; + if (item.getUser() != null) { + memberIdNameMap.put(item.getUser().getId(), item.getUser().getDisplayName()); } } - - TreeMap sortNameMap = new TreeMap(NamesMap); - // after sorting - Iterator itr = sortNameMap.keySet().iterator(); - while (itr.hasNext()) { - String firstname = (String) itr.next(); - MembershipItem memberName = (MembershipItem) sortNameMap.get(firstname); - if (memberName != null) { - assignedtodisplay += memberName.getUser().getDisplayName() + ", "; + for (Rank rank: ranks) { + List assignToNames = new ArrayList(); + for (String userId: rank.getAssignToIds()) { + if (memberIdNameMap.get(userId) != null) { + assignToNames.add(memberIdNameMap.get(userId)); + } } + rank.setAssignToDisplay(StringUtils.join(assignToNames, ", ")); } - - if (!"".equals(assignedtodisplay)) { - assignedtodisplay = assignedtodisplay.substring(0, assignedtodisplay.length() - 2); // remove last comma and space - } - return assignedtodisplay; } - public String constructAssignedTo() { - // store eid separated by delimiter. - String assignedto = ""; - for (int i = 0; i < selectedComposeToList.size(); i++) { - MembershipItem item = (MembershipItem) courseMemberMap.get(selectedComposeToList.get(i)); + private Set constructAssignToIds() { + Set assignToIds = new HashSet(); + for (String selectedComponseTo: (List)selectedComposeToList) { + MembershipItem item = (MembershipItem)courseMemberMap.get(selectedComponseTo); if (item != null) { - assignedto += item.getUser().getEid() + "; "; + assignToIds.add(item.getUser().getId()); } } - if (!"".equals(assignedto)) { - assignedto = assignedto.substring(0, assignedto.length() - 2); // remove last comma and space - } - return assignedto; + return assignToIds; } private boolean attachCaneled = false; diff --git a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/ForumRankBean.java b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/ForumRankBean.java index b06307519f9c..7dde58670b0e 100644 --- a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/ForumRankBean.java +++ b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/ForumRankBean.java @@ -20,6 +20,8 @@ **********************************************************************************/ package org.sakaiproject.tool.messageforums.ui; +import java.util.Set; + import org.sakaiproject.api.app.messageforums.Rank; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -32,7 +34,7 @@ public class ForumRankBean { private static final Log LOG = LogFactory.getLog(ForumRankBean.class); private boolean assignErr; - private String assignTo; + private Set assignToIds; private String assignToDisplay; private String contextId; private boolean imageSizeErr; @@ -54,14 +56,14 @@ public ForumRankBean(Rank rank) { this.rank = rank; this.type = rank.getType(); this.title = rank.getTitle(); - this.assignTo = rank.getAssignTo(); + this.assignToIds = rank.getAssignToIds(); this.assignToDisplay = rank.getAssignToDisplay(); this.contextId = rank.getContextId(); this.minPosts = rank.getMinPosts(); } - public String getAssignTo() { - return assignTo; + public Set getAssignToIds() { + return assignToIds; } public String getAssignToDisplay() { @@ -116,8 +118,8 @@ public void setAssignErr(boolean assignErr) { this.assignErr = assignErr; } - public void setAssignTo(String assignTo) { - this.assignTo = assignTo; + public void setAssignToIds(Set assignToIds) { + this.assignToIds = assignToIds; } public void setAssignToDisplay(String assignToDisplay) { diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/RankManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/RankManagerImpl.java index 778fface876f..d49802f4360a 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/RankManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/RankManagerImpl.java @@ -332,21 +332,11 @@ public List findRanksByContextIdUserId(final String contextId, final String user throw new IllegalArgumentException("Null Argument"); } - String userEid = null; - - try { - userEid = userDirectoryService.getUserEid(userid); - } catch (UserNotDefinedException e) { - throw new IllegalArgumentException("Cannot find user with id " + userid); - } - - final String fUserEid = userEid; - HibernateCallback hcb = new HibernateCallback() { public Object doInHibernate(Session session) throws HibernateException, SQLException { Query q = session.getNamedQuery(QUERY_BY_CONTEXT_ID_USERID); q.setParameter("contextId", contextId, Hibernate.STRING); - q.setParameter("userId", fUserEid, Hibernate.STRING); + q.setParameter("userId", userid, Hibernate.STRING); return q.list(); } }; diff --git a/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/Rank.hbm.xml b/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/Rank.hbm.xml index 4dcb5beef3c8..ab127c860153 100644 --- a/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/Rank.hbm.xml +++ b/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/Rank.hbm.xml @@ -33,14 +33,6 @@ - - - - - - - - @@ -53,7 +45,10 @@ - + + + +