SAK-29036 Bug of special rank: only check part of user eid

vishnumitraha · Feb 3, 2015 · bbb7d7e · bbb7d7e
1 parent eff900e
commit bbb7d7e
Show file tree

Hide file tree

Showing 8 changed files with 139 additions and 97 deletions.
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
@@ -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<String> getAssignToIds();
 
-    public void setAssignTo(String assignTo);
+    public void setAssignToIds(Set<String> assignToIds);
 
     public long getMinPosts();
 

diff --git a/...r/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java b/...r/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<String> 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<RankImpl> ranklist = new ArrayList();
+		List<Rank> 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<String> 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<String, MembershipItem> 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<Rank> ranks) {
+		courseMemberMap = membershipManager.getFilteredCourseMembers(true, null);
+		Map<String, String> memberIdNameMap = new HashMap<String, String>();
+		for (Object o: courseMemberMap.values()) {
+			MembershipItem item = (MembershipItem) o;
+			if (item.getUser() != null) {
+				memberIdNameMap.put(item.getUser().getId(), item.getUser().getDisplayName());
 			}
 		}
-
-		TreeMap<String, MembershipItem> sortNameMap = new TreeMap<String, MembershipItem>(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<String> assignToNames = new ArrayList<String>();
+			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<String> constructAssignToIds() {
+		Set<String> assignToIds = new HashSet<String>();
+		for (String selectedComponseTo: (List<String>)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
@@ -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<String> 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<String> 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<String> assignToIds) {
+        this.assignToIds = assignToIds;
     }
 
     public void setAssignToDisplay(String assignToDisplay) {

diff --git a/...component-impl/src/java/org/sakaiproject/component/app/messageforums/RankManagerImpl.java b/...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/...rums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/Rank.hbm.xml b/...rums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/Rank.hbm.xml
@@ -33,14 +33,6 @@
     <property name="type">
       <column name="RANKTYPE" length="19" not-null="true" />
     </property>
-
-		<property name="assignTo">
-			<column name="ASSIGNTO" length="255"  />
-		</property> 
-
-		<property name="assignToDisplay">
-			<column name="ASSIGNTODISPLAY" length="255"  />
-		</property>
 
 		<property name="minPosts">
 			<column name="MIN_POST" length="19" />
@@ -53,7 +45,10 @@
        <many-to-one name="rankImage" cascade="all" lazy="false"
                      class="org.sakaiproject.component.app.messageforums.dao.hibernate.RankImageImpl"/>
 
-
+		<set name="assignToIds" table="MFR_RANK_INDIVIDUAL_T" lazy="false" cascade="all">
+			<key column="RANK_ID" />
+			<element column="USER_ID" type="string" length="99" not-null="true" />
+		</set>
   </class>
 
  <!-- 
@@ -71,7 +66,7 @@
 	</query>
 
    	<query name="findRanksByContextIdUserID">
-		<![CDATA[from org.sakaiproject.component.app.messageforums.dao.hibernate.RankImpl as us where us.contextId = :contextId and instr(assignto, :userId) >0]]>
+		<![CDATA[from org.sakaiproject.component.app.messageforums.dao.hibernate.RankImpl as us where us.contextId = :contextId and :userId in elements (us.assignToIds)]]>
 	</query>
 
  	<query name="findRanksByContextId">
@@ -87,4 +82,4 @@
 		<![CDATA[from org.sakaiproject.component.app.messageforums.dao.hibernate.RankImpl as us where us.id = :rankId]]>
 	</query>
 
-</hibernate-mapping>
+</hibernate-mapping>
diff --git a/...ums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/RankImpl.java b/...ums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/RankImpl.java
@@ -39,19 +39,19 @@ public class RankImpl extends MutableEntityImpl implements Rank
 	private static final Log LOG = LogFactory.getLog(RankImpl.class);
 	private String title;
 	private String type;
-	private String assignTo;
+	private Set<String> assignToIds;
 	private String assignToDisplay;
 	private String contextId;
 	private long minPosts;
 	private RankImage rankImage;
 
 
-	public RankImpl(String title, String type, String assignTo, String assignToDisplay,
+	public RankImpl(String title, String type, Set<String> assignToIds, String assignToDisplay,
 			String contextId, long minPosts) {
 		super();
 		this.title = title;
 		this.type = type;
-		this.assignTo = assignTo;
+		this.assignToIds = assignToIds;
 		this.assignToDisplay = assignToDisplay;
 		this.contextId = contextId;
 		this.minPosts = minPosts;
@@ -79,12 +79,12 @@ public void setTitle(String rannktitle) {
 
 	}
 
-	public String getAssignTo() {
-		return assignTo;
+	public Set<String> getAssignToIds() {
+		return assignToIds;
 	}
 
-	public void setAssignTo(String assignTo) {
-		this.assignTo = assignTo;
+	public void setAssignToIds(Set<String> assignToIds) {
+		this.assignToIds = assignToIds;
 	}
 
 	public String getContextId() {