488164: reviews editor displays inline comments as threads in patch sets

Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=488164
Change-Id: I2432536f37f56403273e2bd51dcba06b26fb870e
Signed-off-by: Jaxsun McCarthy Huggan <jaxsun.mccarthy@tasktop.com>

7 files changed, 479 insertions, 78 deletions

diff --git a/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/GerritTaskDataHandlerTest.java b/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/GerritTaskDataHandlerTest.java
index a03a3ebda..b3239e2ea 100644
--- a/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/GerritTaskDataHandlerTest.java
+++ b/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/GerritTaskDataHandlerTest.java
@@ -12,9 +12,11 @@
 package org.eclipse.mylyn.internal.gerrit.core;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -26,12 +28,15 @@ import org.eclipse.mylyn.internal.gerrit.core.client.GerritConfiguration;
 import org.eclipse.mylyn.internal.gerrit.core.client.compat.GerritConfigX;
 import org.eclipse.mylyn.internal.gerrit.core.client.data.GerritPerson;
 import org.eclipse.mylyn.internal.gerrit.core.client.data.GerritQueryResult;
+import org.eclipse.mylyn.reviews.core.model.IReview;
+import org.eclipse.mylyn.reviews.internal.core.ReviewFileCommentsMapper;
 import org.eclipse.mylyn.tasks.core.TaskRepository;
 import org.eclipse.mylyn.tasks.core.data.TaskAttribute;
 import org.eclipse.mylyn.tasks.core.data.TaskData;
 import org.junit.Test;
 import org.mockito.Mockito;
 
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.data.AccountInfo;
 import com.google.gerrit.reviewdb.Account;
 import com.google.gerrit.reviewdb.Account.Id;
@@ -43,6 +48,7 @@ import com.google.gerrit.reviewdb.Project;
 /**
  * @author Steffen Pingel
  */
+@SuppressWarnings("restriction")
 public class GerritTaskDataHandlerTest {
 
 	@Test
@@ -60,8 +66,8 @@ public class GerritTaskDataHandlerTest {
 		TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://repository"); //$NON-NLS-1$
 		TaskData data = handler.createTaskData(repository, "1", null); //$NON-NLS-1$
 		TaskData data2 = handler.createTaskData(repository, "2", null); //$NON-NLS-1$
-		assertEquals(GerritTaskSchema.getDefault().UPLOADED.createAttribute(data2.getRoot()), data.getRoot()
-				.getAttribute(GerritTaskSchema.getDefault().UPLOADED.getKey()));
+		assertEquals(GerritTaskSchema.getDefault().UPLOADED.createAttribute(data2.getRoot()),
+				data.getRoot().getAttribute(GerritTaskSchema.getDefault().UPLOADED.getKey()));
 	}
 
 	@Test
@@ -87,8 +93,9 @@ public class GerritTaskDataHandlerTest {
 		GerritTaskDataHandler handler = new GerritTaskDataHandler(new GerritConnector());
 		TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://repository"); //$NON-NLS-1$
 		TaskData data = handler.createTaskData(repository, "1", null); //$NON-NLS-1$
-		handler.updateTaskData(repository, data, createMockGerritChange(), false, null);
+		handler.updateTaskData(repository, data, createMockGerritChange(), createMockReview(), false, null);
 		assertAssignee(data, "joel.user@mylyn.org", "Joel K. User");
+		assertNotNull(data.getRoot().getAttribute(ReviewFileCommentsMapper.FILE_ITEM_COMMENTS));
 	}
 
 	private void assertAssignee(TaskData data, String id, String name) {
@@ -130,9 +137,15 @@ public class GerritTaskDataHandlerTest {
 		return change;
 	}
 
+	private IReview createMockReview() {
+		IReview review = mock(IReview.class);
+		doReturn(ImmutableList.of()).when(review).getSets();
+		return review;
+	}
+
 	private Change mockChange() {
-		return new Change(new Key("1"), new Change.Id(1), new Id(1), new NameKey(new Project.NameKey("parent"),
-				"branch"));
+		return new Change(new Key("1"), new Change.Id(1), new Id(1),
+				new NameKey(new Project.NameKey("parent"), "branch"));
 	}
 
 }
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritTaskDataHandler.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritTaskDataHandler.java
index f0d93332c..61354c7c3 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritTaskDataHandler.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritTaskDataHandler.java
@@ -36,6 +36,7 @@ import org.eclipse.mylyn.reviews.core.model.IReview;
 import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfConsumer;
 import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfObserver;
 import org.eclipse.mylyn.reviews.internal.core.BuildResult;
+import org.eclipse.mylyn.reviews.internal.core.ReviewFileCommentsMapper;
 import org.eclipse.mylyn.reviews.internal.core.TaskBuildStatusMapper;
 import org.eclipse.mylyn.tasks.core.IRepositoryPerson;
 import org.eclipse.mylyn.tasks.core.ITaskMapping;
@@ -96,7 +97,6 @@ public class GerritTaskDataHandler extends AbstractTaskDataHandler {
 	@Override
 	public TaskAttributeMapper getAttributeMapper(TaskRepository repository) {
 		return new TaskAttributeMapper(repository) {
-			@SuppressWarnings("restriction")
 			@Override
 			public boolean equals(TaskAttribute newAttribute, TaskAttribute oldAttribute) {
 
@@ -149,7 +149,7 @@ public class GerritTaskDataHandler extends AbstractTaskDataHandler {
 			Project.NameKey project = gerritChange.getChangeDetail().getChange().getProject();
 			client.refreshConfigOnce(project, monitor);
 			if (!monitor.isCanceled()) {
-				updateTaskData(repository, taskData, gerritChange, !anonymous, id);
+				updateTaskData(repository, taskData, gerritChange, consumer.getModelObject(), !anonymous, id);
 			}
 
 			return taskData;
@@ -163,11 +163,11 @@ public class GerritTaskDataHandler extends AbstractTaskDataHandler {
 
 	private RemoteEmfConsumer<IRepository, IReview, String, GerritChange, String, Date> updateModelData(
 			TaskRepository repository, TaskData taskData, ReviewObserver reviewObserver, IProgressMonitor monitor)
-			throws CoreException {
+					throws CoreException {
 		GerritClient client = connector.getClient(repository);
 		GerritRemoteFactoryProvider factoryProvider = (GerritRemoteFactoryProvider) client.getFactoryProvider();
-		RemoteEmfConsumer<IRepository, IReview, String, GerritChange, String, Date> consumer = factoryProvider.getReviewFactory()
-				.getConsumerForLocalKey(factoryProvider.getRoot(), taskData.getTaskId());
+		RemoteEmfConsumer<IRepository, IReview, String, GerritChange, String, Date> consumer = factoryProvider
+				.getReviewFactory().getConsumerForLocalKey(factoryProvider.getRoot(), taskData.getTaskId());
 
 		consumer.addObserver(reviewObserver);
 		if (!consumer.isRetrieving()) {
@@ -213,7 +213,7 @@ public class GerritTaskDataHandler extends AbstractTaskDataHandler {
 
 	/**
 	 * Get account id for repository
-	 * 
+	 *
 	 * @param client
 	 * @param repository
 	 * @param monitor
@@ -246,8 +246,8 @@ public class GerritTaskDataHandler extends AbstractTaskDataHandler {
 		throw new UnsupportedOperationException();
 	}
 
-	public void updateTaskData(TaskRepository repository, TaskData data, GerritChange gerritReview, boolean canPublish,
-			String accountId) {
+	public void updateTaskData(TaskRepository repository, TaskData data, GerritChange gerritReview, IReview modelReview,
+			boolean canPublish, String accountId) {
 		GerritTaskSchema schema = GerritTaskSchema.getDefault();
 
 		ChangeDetail changeDetail = gerritReview.getChangeDetail();
@@ -302,8 +302,8 @@ public class GerritTaskDataHandler extends AbstractTaskDataHandler {
 		for (Entry<Integer, Collection<BuildResult>> buildResult : resultsByPatchNumber.asMap().entrySet()) {
 			int patchNumber = buildResult.getKey();
 			TaskBuildStatusMapper mapper = new TaskBuildStatusMapper(buildResult.getValue());
-			TaskAttribute attribute = data.getRoot().createAttribute(
-					TaskBuildStatusMapper.ATTR_TYPE_PATCH_SET + patchNumber);
+			TaskAttribute attribute = data.getRoot()
+					.createAttribute(TaskBuildStatusMapper.ATTR_TYPE_PATCH_SET + patchNumber);
 			mapper.applyTo(attribute);
 		}
 
@@ -328,6 +328,8 @@ public class GerritTaskDataHandler extends AbstractTaskDataHandler {
 		}
 		setAttributeValue(data, schema.REVIEW_STATE, reviewState.toString());
 		setAttributeValue(data, schema.VERIFY_STATE, verifyState.toString());
+
+		new ReviewFileCommentsMapper(modelReview).applyTo(data);
 	}
 
 	private Short getStateValue(Short value, Short oldState) {
diff --git a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/internal/core/ReviewFileCommentsMapperTest.java b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/internal/core/ReviewFileCommentsMapperTest.java
new file mode 100644
index 000000000..5e5512148
--- /dev/null
+++ b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/internal/core/ReviewFileCommentsMapperTest.java
@@ -0,0 +1,107 @@
+/*******************************************************************************
+ * Copyright (c) 2016 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.reviews.internal.core;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+
+import org.eclipse.mylyn.reviews.core.model.IComment;
+import org.eclipse.mylyn.reviews.core.model.IFileItem;
+import org.eclipse.mylyn.reviews.core.model.IReview;
+import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
+import org.eclipse.mylyn.tasks.core.TaskRepository;
+import org.eclipse.mylyn.tasks.core.data.TaskAttribute;
+import org.eclipse.mylyn.tasks.core.data.TaskAttributeMapper;
+import org.eclipse.mylyn.tasks.core.data.TaskData;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+
+public class ReviewFileCommentsMapperTest {
+
+	private TaskData taskData;
+
+	private IReview review;
+
+	private ReviewFileCommentsMapper mapper;
+
+	@Before
+	public void setup() {
+		TaskRepository repository = new TaskRepository("kind", "url");
+		TaskAttributeMapper attributeMapper = new TaskAttributeMapper(repository);
+		taskData = new TaskData(attributeMapper, "kind", "url", "id");
+		review = mock(IReview.class);
+		mapper = new ReviewFileCommentsMapper(review);
+	}
+
+	@Test
+	public void emptyReview() {
+		doReturn(ImmutableList.of()).when(review).getSets();
+		mapper.applyTo(taskData);
+		TaskAttribute comments = taskData.getRoot().getAttribute(ReviewFileCommentsMapper.FILE_ITEM_COMMENTS);
+		assertNotNull(comments);
+		assertEquals(ImmutableMap.of(), comments.getAttributes());
+	}
+
+	@Test
+	public void reviewWithoutComments() {
+		IReviewItemSet set = mock(IReviewItemSet.class);
+		IFileItem file = mock(IFileItem.class);
+		doReturn(ImmutableList.of(set)).when(review).getSets();
+		doReturn(ImmutableList.of(file)).when(set).getItems();
+		doReturn(ImmutableList.of()).when(file).getAllComments();
+		mapper.applyTo(taskData);
+		TaskAttribute comments = taskData.getRoot().getAttribute(ReviewFileCommentsMapper.FILE_ITEM_COMMENTS);
+		assertNotNull(comments);
+		assertEquals(ImmutableMap.of(), comments.getAttributes());
+	}
+
+	@Test
+	public void reviewWithComments() {
+		IReviewItemSet set1 = mock(IReviewItemSet.class);
+		IReviewItemSet set2 = mock(IReviewItemSet.class);
+		IFileItem file1 = mock(IFileItem.class);
+		IFileItem file2 = mock(IFileItem.class);
+		IFileItem file3 = mock(IFileItem.class);
+		IComment comment1 = mock(IComment.class);
+		IComment comment2 = mock(IComment.class);
+		IComment comment3 = mock(IComment.class);
+		doReturn(ImmutableList.of(set1, set2)).when(review).getSets();
+		doReturn(ImmutableList.of(file1)).when(set1).getItems();
+		doReturn(ImmutableList.of(file2, file3)).when(set2).getItems();
+		doReturn(ImmutableList.of(comment1)).when(file1).getAllComments();
+		doReturn(ImmutableList.of()).when(file2).getAllComments();
+		doReturn(ImmutableList.of(comment2, comment3)).when(file2).getAllComments();
+		doReturn("id1").when(comment1).getId();
+		doReturn("id2").when(comment2).getId();
+		doReturn("id3").when(comment3).getId();
+		doReturn("comment 1").when(comment1).getDescription();
+		doReturn("comment 2").when(comment2).getDescription();
+		doReturn("comment 3").when(comment3).getDescription();
+		mapper.applyTo(taskData);
+		TaskAttribute comments = taskData.getRoot().getAttribute(ReviewFileCommentsMapper.FILE_ITEM_COMMENTS);
+		assertNotNull(comments);
+		assertComments(ImmutableMap.of("id1", "comment 1", "id2", "comment 2", "id3", "comment 3"), comments);
+	}
+
+	private void assertComments(ImmutableMap<String, String> children, TaskAttribute attribute) {
+		for (String attributeId : children.keySet()) {
+			assertNotNull(attribute.getAttribute(attributeId));
+			assertEquals(children.get(attributeId), attribute.getAttribute(attributeId).getValue());
+		}
+	}
+
+}
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/ReviewFileCommentsMapper.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/ReviewFileCommentsMapper.java
new file mode 100644
index 000000000..2aa7749a1
--- /dev/null
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/ReviewFileCommentsMapper.java
@@ -0,0 +1,50 @@
+/*******************************************************************************
+ * Copyright (c) 2016 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.reviews.internal.core;
+
+import org.eclipse.mylyn.reviews.core.model.IComment;
+import org.eclipse.mylyn.reviews.core.model.IFileItem;
+import org.eclipse.mylyn.reviews.core.model.IReview;
+import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
+import org.eclipse.mylyn.tasks.core.data.TaskAttribute;
+import org.eclipse.mylyn.tasks.core.data.TaskData;
+
+public class ReviewFileCommentsMapper {
+
+	public static final String FILE_ITEM_COMMENTS = "FILE-COMMENTS"; //$NON-NLS-1$
+
+	private final IReview review;
+
+	public ReviewFileCommentsMapper(IReview review) {
+		this.review = review;
+	}
+
+	public void applyTo(TaskData taskData) {
+		TaskAttribute comments = getOrCreateAttribute(taskData.getRoot(), FILE_ITEM_COMMENTS);
+		for (IReviewItemSet set : review.getSets()) {
+			for (IFileItem file : set.getItems()) {
+				for (IComment comment : file.getAllComments()) {
+					TaskAttribute commentAttribute = getOrCreateAttribute(comments, comment.getId());
+					commentAttribute.setValue(comment.getDescription());
+				}
+			}
+		}
+	}
+
+	private TaskAttribute getOrCreateAttribute(TaskAttribute parent, String id) {
+		if (parent.getAttribute(id) == null) {
+			return parent.createAttribute(id);
+		}
+		return parent.getAttribute(id);
+	}
+
+}
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java
index 5c0a22580..065ea40bf 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java
@@ -51,7 +51,7 @@ import org.eclipse.swt.widgets.Display;
 
 /**
  * Base support for reviews labels.
- * 
+ *
  * @author Miles Parker
  * @author Steffen Pingel
  * @author Kevin Sawicki
@@ -114,7 +114,6 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 			}
 			if (element instanceof IReviewItemSet) {
 				return CommonImages.getImage(ReviewsImages.CHANGE_LOG);
-
 			}
 			if (element instanceof IReviewItem) {
 				IReviewItem item = (IReviewItem) element;
@@ -156,6 +155,9 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 			if (element instanceof GlobalCommentsNode) {
 				return CommonImages.getImage(TasksUiImages.TASK);
 			}
+			if (element instanceof ILocation) {
+				return CommonImages.getImage(ReviewsImages.REVIEW_QUOTE);
+			}
 			return null;
 		};
 
@@ -272,10 +274,11 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 					return NLS.bind(Messages.ReviewsLabelProvider_X_Revision_Y, target.getPath(),
 							target.getDescription());
 				} else if (target.getPath() == null && base.getPath() != null) {
-					return NLS.bind(Messages.ReviewsLabelProvider_X_Revision_Y, base.getPath(), target.getDescription());
+					return NLS.bind(Messages.ReviewsLabelProvider_X_Revision_Y, base.getPath(),
+							target.getDescription());
 				} else if (!target.getPath().equals(base.getPath())) {
-					return NLS.bind(Messages.ReviewsLabelProvider_X_renamed_from_Y_Z, new Object[] { target.getPath(),
-							base.getPath(), target.getDescription() });
+					return NLS.bind(Messages.ReviewsLabelProvider_X_renamed_from_Y_Z,
+							new Object[] { target.getPath(), base.getPath(), target.getDescription() });
 				} else {
 					return NLS.bind(Messages.ReviewsLabelProvider_X_Revision_Y, target.getPath(),
 							target.getDescription());
@@ -401,8 +404,8 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 		};
 	};
 
-	public static final TableColumnProvider SINGLE_COLUMN = new TableColumnProvider(
-			Messages.ReviewsLabelProvider_Items, 10, 120, false) {
+	public static final TableColumnProvider SINGLE_COLUMN = new TableColumnProvider(Messages.ReviewsLabelProvider_Items,
+			10, 120, false) {
 
 		@Override
 		public Image getImage(Object element) {
@@ -498,6 +501,8 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 		}
 	};
 
+	private final boolean includeAuthors;
+
 	public static class Flat extends ReviewsLabelProvider {
 		TableColumnProvider[] columns = new TableColumnProvider[] { ARTIFACT_COLUMN, COMMENTS_COLUMN, AUTHORS_COLUMN,
 				DATE_COLUMN };
@@ -511,6 +516,14 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 	public static class Tree extends ReviewsLabelProvider {
 		TableColumnProvider[] columns = new TableColumnProvider[] { ITEMS_COLUMN, AUTHORS_COLUMN, DATE_COLUMN };
 
+		public Tree() {
+			this(false);
+		}
+
+		public Tree(boolean includeAuthors) {
+			super(includeAuthors);
+		}
+
 		@Override
 		public TableColumnProvider[] getColumnProviders() {
 			return columns;
@@ -563,6 +576,14 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 		}
 	}
 
+	public ReviewsLabelProvider() {
+		this(false);
+	}
+
+	public ReviewsLabelProvider(boolean includeAuthors) {
+		this.includeAuthors = includeAuthors;
+	}
+
 	@Override
 	public String getText(Object element) {
 		return ITEMS_COLUMN.getText(element);
@@ -577,7 +598,11 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 		if (element instanceof IComment) {
 			styler = COMMENT_STYLE;
 		}
-		StyledString styledString = new StyledString(getText(element), styler);
+		StyledString styledString = new StyledString();
+		if (includeAuthors) {
+			addAuthorStyle(element, styledString);
+		}
+		styledString.append(getText(element), styler);
 		if (element instanceof GlobalCommentsNode) {
 			element = ((GlobalCommentsNode) element).getReview();
 		}
@@ -594,6 +619,15 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 		return ITEMS_COLUMN.getImage(element);
 	}
 
+	public static void addAuthorStyle(Object element, StyledString styledString) {
+		if (element instanceof IComment) {
+			IComment comment = (IComment) element;
+			String name = comment.getAuthor().getDisplayName();
+			String firstName = StringUtils.substringBefore(name, " "); //$NON-NLS-1$
+			styledString.append(firstName + ": ", AUTHOR_STYLE); //$NON-NLS-1$
+		}
+	}
+
 	public static void addCommentLineNumberStyle(Object element, StyledString styledString) {
 		if (element instanceof IComment) {
 			IComment comment = (IComment) element;
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentProvider.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentProvider.java
new file mode 100644
index 000000000..1b7af0c9c
--- /dev/null
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentProvider.java
@@ -0,0 +1,131 @@
+package org.eclipse.mylyn.reviews.ui.spi.editor;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Comparator;
+import java.util.List;
+
+import org.eclipse.jface.viewers.ITreeContentProvider;
+import org.eclipse.jface.viewers.Viewer;
+import org.eclipse.mylyn.reviews.core.model.IComment;
+import org.eclipse.mylyn.reviews.core.model.IFileItem;
+import org.eclipse.mylyn.reviews.core.model.ILocation;
+import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
+
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Multimaps;
+
+final class ReviewSetContentProvider implements ITreeContentProvider {
+
+	private final Multimap<ILocation, IComment> threads = LinkedHashMultimap.create();
+
+	private final Multimap<String, ILocation> threadLocationsByFile = LinkedHashMultimap.create();
+
+	public Object[] getElements(Object inputElement) {
+		return getReviewItems(inputElement).toArray();
+	}
+
+	@Override
+	public void inputChanged(Viewer viewer, Object oldInput, Object newInput) {
+		// do nothing
+	}
+
+	@Override
+	public void dispose() {
+		threads.clear();
+		threadLocationsByFile.clear();
+	}
+
+	@Override
+	public boolean hasChildren(Object element) {
+		return !getFileComments(element).isEmpty() || element instanceof ILocation;
+	}
+
+	@Override
+	public Object getParent(Object element) {
+		// unsupported
+		return null;
+	}
+
+	@Override
+	public Object[] getChildren(Object parentElement) {
+		if (parentElement instanceof IFileItem) {
+			IFileItem file = (IFileItem) parentElement;
+			Multimap<Long, IComment> commentsByLocation = Multimaps.index(getFileComments(file), commentLineNumber());
+
+			for (ILocation location : threadLocationsByFile.get(file.getId())) {
+				threads.removeAll(location);
+			}
+			threadLocationsByFile.removeAll(file.getId());
+
+			for (Long line : commentsByLocation.keySet()) {
+				ILocation location = null;
+				for (IComment comment : commentsByLocation.get(line)) {
+					if (location == null) {
+						location = comment.getLocations().iterator().next();
+						threadLocationsByFile.put(file.getId(), location);
+					}
+					if (!threads.containsValue(comment)) {
+						threads.put(location, comment);
+					}
+				}
+			}
+			Collection<ILocation> relevantThreads = threadLocationsByFile.get(file.getId());
+			ILocation[] locations = relevantThreads.toArray(new ILocation[relevantThreads.size()]);
+			Arrays.sort(locations, lineNumberComparator());
+			return locations;
+		} else if (parentElement instanceof ILocation) {
+			return threads.get((ILocation) parentElement).toArray();
+		}
+		return new Object[0];
+	}
+
+	private Comparator<ILocation> lineNumberComparator() {
+		return new Comparator<ILocation>() {
+
+			@Override
+			public int compare(ILocation o1, ILocation o2) {
+				return (int) (o1.getIndex() - o2.getIndex());
+			}
+		};
+	}
+
+	private Function<? super IComment, Long> commentLineNumber() {
+		return new Function<IComment, Long>() {
+
+			@Override
+			public Long apply(IComment comment) {
+				return comment.getLocations().iterator().next().getIndex();
+			}
+		};
+	}
+
+	private Predicate<IComment> hasLocation() {
+		return new Predicate<IComment>() {
+
+			@Override
+			public boolean apply(IComment input) {
+				return input.getLocations().iterator().hasNext();
+			}
+		};
+	}
+
+	private List<IFileItem> getReviewItems(Object inputElement) {
+		if (inputElement instanceof IReviewItemSet) {
+			return ((IReviewItemSet) inputElement).getItems();
+		}
+		return ImmutableList.of();
+	}
+
+	private List<IComment> getFileComments(Object inputElement) {
+		if (inputElement instanceof IFileItem) {
+			return FluentIterable.from(((IFileItem) inputElement).getAllComments()).filter(hasLocation()).toList();
+		}
+		return ImmutableList.of();
+	}
+}
\ No newline at end of file
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java
index 807b0613c..5da4f5235 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java
@@ -14,9 +14,10 @@ package org.eclipse.mylyn.reviews.ui.spi.editor;
 
 import java.text.DateFormat;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.eclipse.jface.layout.GridDataFactory;
 import org.eclipse.jface.layout.GridLayoutFactory;
@@ -24,16 +25,18 @@ import org.eclipse.jface.layout.LayoutConstants;
 import org.eclipse.jface.viewers.ColumnViewerToolTipSupport;
 import org.eclipse.jface.viewers.DelegatingStyledCellLabelProvider;
 import org.eclipse.jface.viewers.IOpenListener;
-import org.eclipse.jface.viewers.IStructuredContentProvider;
-import org.eclipse.jface.viewers.IStructuredSelection;
+import org.eclipse.jface.viewers.ITreeSelection;
+import org.eclipse.jface.viewers.ITreeViewerListener;
 import org.eclipse.jface.viewers.OpenEvent;
-import org.eclipse.jface.viewers.TableViewer;
-import org.eclipse.jface.viewers.Viewer;
+import org.eclipse.jface.viewers.TreeExpansionEvent;
+import org.eclipse.jface.viewers.TreePath;
+import org.eclipse.jface.viewers.TreeViewer;
 import org.eclipse.jface.window.ToolTip;
 import org.eclipse.mylyn.commons.ui.compatibility.CommonColors;
 import org.eclipse.mylyn.commons.workbench.forms.ScalingHyperlink;
 import org.eclipse.mylyn.internal.reviews.ui.providers.ReviewsLabelProvider;
 import org.eclipse.mylyn.internal.tasks.ui.editors.EditorUtil;
+import org.eclipse.mylyn.reviews.core.model.IComment;
 import org.eclipse.mylyn.reviews.core.model.ICommit;
 import org.eclipse.mylyn.reviews.core.model.IFileItem;
 import org.eclipse.mylyn.reviews.core.model.IRepository;
@@ -41,6 +44,7 @@ import org.eclipse.mylyn.reviews.core.model.IReview;
 import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
 import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfConsumer;
 import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfObserver;
+import org.eclipse.mylyn.reviews.internal.core.ReviewFileCommentsMapper;
 import org.eclipse.mylyn.reviews.internal.core.TaskBuildStatusMapper;
 import org.eclipse.mylyn.tasks.core.data.TaskAttribute;
 import org.eclipse.mylyn.tasks.ui.editors.AbstractAttributeEditor;
@@ -52,6 +56,7 @@ import org.eclipse.swt.widgets.Control;
 import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Text;
+import org.eclipse.swt.widgets.TreeItem;
 import org.eclipse.ui.forms.FormColors;
 import org.eclipse.ui.forms.IFormColors;
 import org.eclipse.ui.forms.events.ExpansionAdapter;
@@ -66,6 +71,7 @@ import org.eclipse.ui.forms.widgets.Section;
  * @author Miles Parker
  * @author Sam Davis
  */
+@SuppressWarnings("restriction")
 public class ReviewSetContentSection {
 
 	private static final int MAXIMUM_ITEMS_SHOWN = 30;
@@ -76,16 +82,14 @@ public class ReviewSetContentSection {
 
 	private final Section section;
 
-	private TableViewer viewer;
+	private TreeViewer viewer;
 
 	private final RemoteEmfObserver<IReviewItemSet, List<IFileItem>, String, Long> itemListObserver = new RemoteEmfObserver<IReviewItemSet, List<IFileItem>, String, Long>() {
 
 		@Override
 		public void updated(boolean modified) {
 			createItemSetTable();
-			if (modified) {
-				updateItemSetTable();
-			}
+			updateItemSetTable();
 			updateMessage();
 			createButtons();
 		}
@@ -96,7 +100,9 @@ public class ReviewSetContentSection {
 		}
 	};
 
-	private Composite tableContainer;
+	private final RemoteEmfConsumer<IReviewItemSet, List<IFileItem>, String, ?, ?, Long> itemSetConsumer;
+
+	private Composite treeContainer;
 
 	private Composite actionContainer;
 
@@ -104,7 +110,8 @@ public class ReviewSetContentSection {
 
 	private final AbstractTaskEditorPage page;
 
-	public ReviewSetContentSection(ReviewSetSection parentSection, final IReviewItemSet set, AbstractTaskEditorPage page) {
+	public ReviewSetContentSection(ReviewSetSection parentSection, final IReviewItemSet set,
+			AbstractTaskEditorPage page) {
 		this.parentSection = parentSection;
 		this.set = set;
 		this.page = page;
@@ -116,15 +123,14 @@ public class ReviewSetContentSection {
 		section.setTitleBarForeground(parentSection.getToolkit().getColors().getColor(IFormColors.TITLE));
 
 		parentSection.addTextClient(parentSection.getToolkit(), section, "", false); //$NON-NLS-1$
-		final RemoteEmfConsumer<IReviewItemSet, List<IFileItem>, String, ?, ?, Long> itemSetConsumer = getParentSection().getReviewEditorPage()
+		itemSetConsumer = getParentSection().getReviewEditorPage()
 				.getFactoryProvider()
 				.getReviewItemSetContentFactory()
 				.getConsumerForLocalKey(set, set.getId());
 		itemListObserver.setConsumer(itemSetConsumer);
-		final RemoteEmfConsumer<IRepository, IReview, String, ?, ?, Date> reviewConsumer = getParentSection().getReviewEditorPage()
-				.getFactoryProvider()
-				.getReviewFactory()
-				.getConsumerForModel(set.getReview().getRepository(), set.getReview());
+		final RemoteEmfConsumer<IRepository, IReview, String, ?, ?, Date> reviewConsumer = getParentSection()
+				.getReviewEditorPage().getFactoryProvider().getReviewFactory().getConsumerForModel(
+						set.getReview().getRepository(), set.getReview());
 		reviewObserver = new RemoteEmfObserver<IRepository, IReview, String, Date>() {
 			@Override
 			public void updated(boolean modified) {
@@ -149,8 +155,10 @@ public class ReviewSetContentSection {
 				}
 			}
 		});
+
 		createMainSection();
 		createItemSetTable();
+		updateItemSetTable();
 		updateMessage();
 	}
 
@@ -172,9 +180,8 @@ public class ReviewSetContentSection {
 				message += " " + Messages.Reviews_RetrievingContents; //$NON-NLS-1$
 			}
 		} else {
-			message = NLS.bind(Messages.Reviews_UpdateFailure_X, itemListObserver.getConsumer()
-					.getStatus()
-					.getMessage());
+			message = NLS.bind(Messages.Reviews_UpdateFailure_X,
+					itemListObserver.getConsumer().getStatus().getMessage());
 		}
 
 		AbstractReviewSection.appendMessage(getSection(), message);
@@ -209,17 +216,17 @@ public class ReviewSetContentSection {
 			getTaskEditorPage().getAttributeEditorToolkit().adapt(editor);
 		}
 
-		tableContainer = new Composite(composite, SWT.NONE);
-		tableContainer.setBackground(Display.getCurrent().getSystemColor(SWT.COLOR_DARK_GRAY));
-		GridDataFactory.fillDefaults().span(4, 1).grab(true, true).applyTo(tableContainer);
-		GridLayoutFactory.fillDefaults().numColumns(2).applyTo(tableContainer);
+		treeContainer = new Composite(composite, SWT.NONE);
+		treeContainer.setBackground(Display.getCurrent().getSystemColor(SWT.COLOR_DARK_GRAY));
+		GridDataFactory.fillDefaults().span(4, 1).grab(true, true).applyTo(treeContainer);
+		GridLayoutFactory.fillDefaults().numColumns(2).applyTo(treeContainer);
 
 		actionContainer = new Composite(composite, SWT.NONE);
 		GridDataFactory.fillDefaults().span(4, 1).grab(true, true).applyTo(actionContainer);
 		GridLayoutFactory.fillDefaults().numColumns(4).applyTo(actionContainer);
 		createButtons();
 
-		parentSection.getTaskEditorPage().reflow();
+		reflow();
 	}
 
 	private AbstractTaskEditorPage getTaskEditorPage() {
@@ -338,36 +345,19 @@ public class ReviewSetContentSection {
 			} else {
 				style |= SWT.NO_SCROLL;
 			}
-			viewer = new TableViewer(tableContainer, style);
+			viewer = new TreeViewer(treeContainer, style);
 			GridDataFactory.fillDefaults()
 					.span(2, 1)
 					.grab(true, true)
 					.hint(500, heightHint)
 					.applyTo(viewer.getControl());
-			viewer.setContentProvider(new IStructuredContentProvider() {
 
-				public void dispose() {
-					// ignore
-				}
+			viewer.setContentProvider(new ReviewSetContentProvider());
 
-				public Object[] getElements(Object inputElement) {
-					return getReviewItems(inputElement).toArray();
-				}
-
-				private List<IFileItem> getReviewItems(Object inputElement) {
-					if (inputElement instanceof IReviewItemSet) {
-						return ((IReviewItemSet) inputElement).getItems();
-					}
-					return Collections.emptyList();
-				}
-
-				public void inputChanged(final Viewer viewer, Object oldInput, Object newInput) {
-				}
-			});
 			ColumnViewerToolTipSupport.enableFor(viewer, ToolTip.NO_RECREATE);
 
 			final DelegatingStyledCellLabelProvider styledLabelProvider = new DelegatingStyledCellLabelProvider(
-					new ReviewsLabelProvider.Simple()) {
+					new ReviewsLabelProvider.Tree(true)) {
 				@Override
 				public String getToolTipText(Object element) {
 					//For some reason tooltips are not delegated..
@@ -377,24 +367,80 @@ public class ReviewSetContentSection {
 			viewer.setLabelProvider(styledLabelProvider);
 			viewer.addOpenListener(new IOpenListener() {
 				public void open(OpenEvent event) {
-					IStructuredSelection selection = (IStructuredSelection) event.getSelection();
-					IFileItem item = (IFileItem) selection.getFirstElement();
-					if (item != null) {
-						getParentSection().getUiFactoryProvider()
-								.getOpenFileFactory(ReviewSetContentSection.this.getParentSection(), set, item)
-								.execute();
+					// TODO: open to line of selected comment or thread
+					ITreeSelection selection = (ITreeSelection) event.getSelection();
+					TreePath[] paths = selection.getPaths();
+					for (TreePath path : paths) {
+						for (int i = 0; i < path.getSegmentCount(); i++) {
+							Object o = path.getSegment(i);
+							if (o instanceof IFileItem) {
+								IFileItem item = (IFileItem) o;
+								getParentSection().getUiFactoryProvider()
+										.getOpenFileFactory(ReviewSetContentSection.this.getParentSection(), set, item)
+										.execute();
+							}
+						}
 					}
 				}
 			});
-			EditorUtil.addScrollListener(viewer.getTable());
+			EditorUtil.addScrollListener(viewer.getTree());
 			viewer.setInput(set);
-			getParentSection().getTaskEditorPage().reflow();
+			viewer.addTreeListener(new ITreeViewerListener() {
+
+				@Override
+				public void treeExpanded(TreeExpansionEvent event) {
+					reflowAsync();
+				}
+
+				@Override
+				public void treeCollapsed(TreeExpansionEvent event) {
+					reflowAsync();
+				}
+			});
 		}
 	}
 
-	public void updateItemSetTable() {
-		if (set.getItems().size() > 0 && viewer != null) {
+	private void updateItemSetTable() {
+		if (set.getItems().size() > 0 && hasViewer()) {
 			viewer.setInput(set);
+			TaskAttribute fileComments = ReviewSetContentSection.this.page.getModel()
+					.getTaskData()
+					.getRoot()
+					.getAttribute(ReviewFileCommentsMapper.FILE_ITEM_COMMENTS);
+
+			final Set<IComment> toHighlight = new HashSet<IComment>();
+			if (fileComments != null) {
+				List<IFileItem> files = itemSetConsumer.getModelObject();
+				for (IFileItem file : files) {
+					for (IComment comment : file.getAllComments()) {
+						if (fileComments.getAttribute(comment.getId()) == null) {
+							toHighlight.add(comment);
+						}
+					}
+				}
+			}
+
+			Display.getDefault().asyncExec(new Runnable() {
+
+				@Override
+				public void run() {
+					if (hasViewer()) {
+						viewer.refresh();
+						viewer.expandAll();
+						decorateItems(viewer.getTree().getItems(), toHighlight);
+						reflow();
+					}
+				}
+			});
+		}
+	}
+
+	private void decorateItems(TreeItem[] items, Set<IComment> toHighlight) {
+		for (TreeItem item : items) {
+			if (toHighlight.contains(item.getData())) {
+				item.setBackground(getTaskEditorPage().getAttributeEditorToolkit().getColorIncoming());
+			}
+			decorateItems(item.getItems(), toHighlight);
 		}
 	}
 
@@ -406,7 +452,7 @@ public class ReviewSetContentSection {
 			getParentSection().getUiFactoryProvider().createControls(getParentSection(), actionContainer,
 					getParentSection().getToolkit(), set);
 			actionContainer.layout();
-			getParentSection().getTaskEditorPage().reflow();
+			reflow();
 		}
 	}
 
@@ -418,6 +464,24 @@ public class ReviewSetContentSection {
 		return parentSection;
 	}
 
+	private void reflowAsync() {
+		Display.getDefault().asyncExec(new Runnable() {
+
+			@Override
+			public void run() {
+				reflow();
+			}
+		});
+	}
+
+	private void reflow() {
+		getParentSection().getTaskEditorPage().reflow();
+	}
+
+	private boolean hasViewer() {
+		return viewer != null && !viewer.getControl().isDisposed();
+	}
+
 	public void dispose() {
 		itemListObserver.dispose();
 		reviewObserver.dispose();