DASH-324 code changes based on code review

git-svn-id: https://source.sakaiproject.org/svn/dashboard/trunk@315035 66ffb92e-73f9-0310-93c1-f5514f145a0a

dashboard/impl/src/java/org/sakaiproject/dash/dao/mapper/ContextUserMapper.java

@@ -27,19 +27,22 @@
 import org.sakaiproject.dash.model.Context;
 import org.springframework.jdbc.core.RowMapper;
 
+import org.apache.log4j.Logger;
+
 /**
  * Mapper for the "context_users" string returned
  */
 public class ContextUserMapper implements RowMapper
 {
 
+	private static final Logger log = Logger.getLogger(ContextUserMapper.class);
+
 	public String mapRow(ResultSet rs, int rowNum) throws SQLException {
 		String rv = "";
 		try {
 			rv = rs.getString("context_users");
 		} catch (Exception e) {
-			System.out.println(this + " =============== " + e + " ===============");
-			e.printStackTrace(System.out);
+			log.warn(this + " mapRow: " + e);
 			if(e instanceof SQLException) {
 				throw (SQLException) e;
 			}

dashboard/impl/src/java/org/sakaiproject/dash/jobs/DashSyncUserSitesJob.java

@@ -28,11 +28,13 @@
  * this job is to find out the up-to-date user enrolled sites
  * remove site dashboard items if the user no longer belongs to the site
  * or add site dashboard items if the user is added to the site
+ * Admin user can schedule the job inside Admin Job Scheduler tool, 
+ * and create trigger with desired frequency
  * @author zqian
  *
  */
 public class DashSyncUserSitesJob extends DashQuartzJob {
-	private Log	logger = LogFactory.getLog(DashSyncUserSitesJob.class);
+	private Log logger = LogFactory.getLog(DashSyncUserSitesJob.class);
 
 	//Matches the bean id
 	final static String beanId = "dashSyncUserSitesJob";
@@ -55,7 +57,7 @@ public void execute(JobExecutionContext jobExecutionContext) throws JobExecution
     		try {
     			dashboardCommonLogic.syncDashboardUsersWithSiteUsers();
 			} catch (Exception e) {
-				logger.warn(this + "execute error: " , e);
+				logger.warn(this + " execute error: " , e);
 			}
     	}
     }

dashboard/impl/src/java/org/sakaiproject/dash/logic/DashboardCommonLogicImpl.java

@@ -136,6 +136,11 @@ public class DashboardCommonLogicImpl implements DashboardCommonLogic, Observer
 
 	protected String propLoopTimerEnabledLocally = null;
 
+	// strings to indicate dashboard link type
+	private static final String CALENDAR_LINK_TYPE = "calendar_link_type";
+	private static final String NEWS_LINK_TYPE = "news_link_type";
+
+
 	/************************************************************************
 	 * Spring-injected classes
 	 ************************************************************************/
@@ -1685,7 +1690,7 @@ public void syncDashboardUsersWithSiteUsers()
 		HashMap<String, Set<String>> newsLinksUserMap =  dao.getDashboardNewsContextUserMap();
 
 		// combine the context id from keyset of both maps
-		// so thate we just need to call AuthzGroupService once to find out site members
+		// so that we just need to call AuthzGroupService once to find out site members
 		Set<String> combinedContextIdSet = new HashSet<String>();
 		combinedContextIdSet.addAll(calendarLinksUserMap.keySet());
 		combinedContextIdSet.addAll(newsLinksUserMap.keySet());
@@ -1705,13 +1710,13 @@ public void syncDashboardUsersWithSiteUsers()
 			// remove or add user DashboardCalendarlinks if needed
 			if (calendarLinksUserMap.containsKey(context_id))
 			{
-				addOrRemoveDashboardLinksBasedOnUsersSetComp(context_id, calendarLinksUserMap.get(context_id), siteUserSet, true);
+				addOrRemoveDashboardLinksBasedOnUsersSetComp(context_id, calendarLinksUserMap.get(context_id), siteUserSet, CALENDAR_LINK_TYPE);
 			}
 
 			// remove or add user DashboardNewslinks if needed
 			if (newsLinksUserMap.containsKey(context_id))
 			{
-				addOrRemoveDashboardLinksBasedOnUsersSetComp(context_id, newsLinksUserMap.get(context_id), siteUserSet, false);
+				addOrRemoveDashboardLinksBasedOnUsersSetComp(context_id, newsLinksUserMap.get(context_id), siteUserSet, NEWS_LINK_TYPE);
 			}
 		}
 		logger.info(this + ".syncDashboardUsersWithSiteUsers end " + serverId);
@@ -1739,7 +1744,7 @@ private Collection<String> getSiteUserIdList(String siteId)
 	 * @siteUserSet
 	 * @forCalendarLinks when true, add/remove in DASH_CALENDAR_LINK table; otherwise, add/remove in DASH_NEWS_LINK table
 	 */
-	private void addOrRemoveDashboardLinksBasedOnUsersSetComp(String context_id, Set<String> dashboardUserSet, Set<String> siteUserSet, boolean forCalendarLinks)
+	private void addOrRemoveDashboardLinksBasedOnUsersSetComp(String context_id, Set<String> dashboardUserSet, Set<String> siteUserSet, String linkType)
 	{
 		// construct two base set, one for remove user links, one for add user links
 		Set<String> removeSet = new HashSet<String>();
@@ -1753,29 +1758,33 @@ private void addOrRemoveDashboardLinksBasedOnUsersSetComp(String context_id, Set
 		// need to do the comparison between those two sets: 
 		// 1. add dashboard links if the user is added to site; 
 		addSet.removeAll(dashboardUserSet);
+		if (!addSet.isEmpty())
+			logger.info(this + " addOrRemoveDashboardLinksBasedOnUsersSetComp add dash link user set size=" + addSet.size() + " for context " + context_id);
 		for(String userId: addSet)
 		{	
-			if (forCalendarLinks)
+			if (CALENDAR_LINK_TYPE.equals(linkType))
 			{
 				addCalendarLinks(userId, context_id);
 				logger.debug(this + ".syncDashboardUsersWithSiteUsers ADD calendar links for user= " + userId + " context_id=" + context_id);
 			}
-			else
+			else if (NEWS_LINK_TYPE.equals(linkType))
 			{
 				addNewsLinks(userId, context_id);
 				logger.debug(this + ".syncDashboardUsersWithSiteUsers ADD news links for user= " + userId + " context_id=" + context_id);
 			}
 		}
 		// 2. remove dashboard links if the user is removed from the site
 		removeSet.removeAll(siteUserSet);
+		if (!removeSet.isEmpty())
+			logger.info(this + " addOrRemoveDashboardLinksBasedOnUsersSetComp remove dash link user set size=" + removeSet.size()+ " for context " + context_id);
 		for(String userId: removeSet)
 		{
-			if (forCalendarLinks)
+			if (CALENDAR_LINK_TYPE.equals(linkType))
 			{
 				removeCalendarLinks(userId, context_id);
 				logger.debug(this + ".syncDashboardUsersWithSiteUsers REMOVE calendar links for user= " + userId + " context_id=" + context_id);
 			}
-			else
+			else if (NEWS_LINK_TYPE.equals(linkType))
 			{
 				removeNewsLinks(userId, context_id);
 				logger.debug(this + ".syncDashboardUsersWithSiteUsers REMOVE news links for user= " + userId + " context_id=" + context_id);

dashboard/impl/src/resources/hsqldb.properties

@@ -674,7 +674,7 @@ delete.RepeatingEvent.by.id = delete from dash_repeating_event where id=?
 select.context.user.from.calendar.link = select concat(concat(t3.context_id, ' '), t4.SAKAI_ID) as context_users \
 from \
 (select distinct t1.context_id, t2.person_id \
-from DASH_CONTEXT t1, \
+from dash_context t1, \
 (select  context_id, person_id \
 from dash_calendar_link) t2 \
 where t1.id = t2.context_id) t3, \

dashboard/impl/src/resources/mysql.properties

@@ -682,7 +682,7 @@ delete.RepeatingEvent.by.id = delete from dash_repeating_event where id=?
 select.context.user.from.calendar.link = select concat(concat(t3.context_id, ' '), t4.SAKAI_ID) as context_users \
 from \
 (select distinct t1.context_id, t2.person_id \
-from DASH_CONTEXT t1, \
+from dash_context t1, \
 (select  context_id, person_id \
 from dash_calendar_link) t2 \
 where t1.id = t2.context_id) t3, \

dashboard/impl/src/resources/oracle.properties

@@ -690,7 +690,7 @@ delete.RepeatingEvent.by.id = delete from dash_repeating_event where id=?
 select.context.user.from.calendar.link = select concat(concat(t3.context_id, ' '), t4.SAKAI_ID) as context_users \
 from \
 (select distinct t1.context_id, t2.person_id \
-from DASH_CONTEXT t1, \
+from dash_context t1, \
 (select  context_id, person_id \
 from dash_calendar_link) t2 \
 where t1.id = t2.context_id) t3, \