386705: expand patch set when hyperlink clickedrefs/changes/57/10057/9

Change-Id: I3b61060d9bee2dc8f039fcd071d2540792525d0e
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=386705

7 files changed, 133 insertions, 5 deletions

diff --git a/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/GerritConnectorTest.java b/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/GerritConnectorTest.java
index 9071160..6a680f6 100644
--- a/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/GerritConnectorTest.java
+++ b/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/GerritConnectorTest.java
@@ -86,4 +86,9 @@ public class GerritConnectorTest {
 				connector.getTaskUrl("http://review.source.android.com", "13492")); //$NON-NLS-1$ //$NON-NLS-2$
 	}
 
+	@Test
+	public void testGetTaskUrlTrailingSlash() {
+		assertEquals("http://review.mylyn.org/#/c/4698/", //$NON-NLS-1$
+				connector.getTaskUrl("http://review.mylyn.org/", "4698")); //$NON-NLS-1$ //$NON-NLS-2$
+	}
 }
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritConnector.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritConnector.java
index 16e3241..ca15d3a 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritConnector.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritConnector.java
@@ -23,6 +23,7 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.apache.commons.httpclient.HttpStatus;
+import org.apache.commons.lang.StringUtils;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
@@ -199,6 +200,7 @@ public class GerritConnector extends AbstractRepositoryConnector {
 
 	@Override
 	public String getTaskUrl(String repositoryUrl, String taskId) {
+		repositoryUrl = StringUtils.removeEnd(repositoryUrl, "/"); //$NON-NLS-1$
 		return repositoryUrl + CHANGE_PREFIX_NEW + taskId + "/"; //$NON-NLS-1$
 	}
 
diff --git a/org.eclipse.mylyn.gerrit.ui.tests/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandlerTest.java b/org.eclipse.mylyn.gerrit.ui.tests/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandlerTest.java
index 712d138..1c202df 100644
--- a/org.eclipse.mylyn.gerrit.ui.tests/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandlerTest.java
+++ b/org.eclipse.mylyn.gerrit.ui.tests/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandlerTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012 Tasktop Technologies and others.
+1 * Copyright (c) 2012 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
@@ -96,4 +96,51 @@ public class GerritUrlHandlerTest {
 		assertEquals("4698", handler.getTaskId(repository, "http://review.mylyn.org/#/c/4698/5/foo/bar"));
 	}
 
+	@Test
+	public void testGetPatchSetNumberPatchSet() {
+		TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
+		String url = "http://review.mylyn.org/#/c/4698/5";
+		String taskId = handler.getTaskId(repository, url);
+		assertEquals(5, handler.getPatchSetNumber(repository, url, taskId));
+	}
+
+	@Test
+	public void testGetPatchSetNumberPatchSetTrailingSlash() {
+		TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
+		String url = "http://review.mylyn.org/#/c/4698/5/";
+		String taskId = handler.getTaskId(repository, url);
+		assertEquals(5, handler.getPatchSetNumber(repository, url, taskId));
+	}
+
+	@Test
+	public void testGetPatchSetNumberPatchSetFile() {
+		TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
+		String url = "http://review.mylyn.org/#/c/4698/5/foo/bar";
+		String taskId = handler.getTaskId(repository, url);
+		assertEquals(5, handler.getPatchSetNumber(repository, url, taskId));
+	}
+
+	@Test
+	public void testGetPatchSetNumberNoneSpecified() {
+		TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
+		String url = "http://review.mylyn.org/#/c/4698";
+		String taskId = handler.getTaskId(repository, url);
+		assertEquals(-1, handler.getPatchSetNumber(repository, url, taskId));
+	}
+
+	@Test
+	public void testGetPatchSetNumberNoneSpecifiedTrailingSlash() {
+		TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
+		String url = "http://review.mylyn.org/#/c/4698/";
+		String taskId = handler.getTaskId(repository, url);
+		assertEquals(-1, handler.getPatchSetNumber(repository, url, taskId));
+	}
+
+	@Test
+	public void testGetPatchSetNumberNoneSpecifiedNotAnInteger() {
+		TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
+		String url = "http://review.mylyn.org/#/c/A1";
+		String taskId = handler.getTaskId(repository, url);
+		assertEquals(-1, handler.getPatchSetNumber(repository, url, taskId));
+	}
 }
diff --git a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandler.java b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandler.java
index cedb5ce..58c92dd 100644
--- a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandler.java
+++ b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandler.java
@@ -14,21 +14,28 @@ package org.eclipse.mylyn.internal.gerrit.ui;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.commons.lang.StringUtils;
 import org.eclipse.mylyn.commons.workbench.EditorHandle;
 import org.eclipse.mylyn.commons.workbench.browser.AbstractUrlHandler;
 import org.eclipse.mylyn.internal.gerrit.core.GerritConnector;
+import org.eclipse.mylyn.internal.gerrit.ui.editor.GerritTaskEditorPage;
+import org.eclipse.mylyn.internal.gerrit.ui.editor.PatchSetSection;
 import org.eclipse.mylyn.tasks.core.TaskRepository;
 import org.eclipse.mylyn.tasks.ui.TasksUi;
 import org.eclipse.mylyn.tasks.ui.TasksUiUtil;
+import org.eclipse.mylyn.tasks.ui.editors.TaskEditor;
 import org.eclipse.ui.IWorkbenchPage;
+import org.eclipse.ui.IWorkbenchPart;
+import org.eclipse.ui.forms.editor.IFormPage;
 
 /**
  * @author Steffen Pingel
+ * @author Miles Parker
  */
 public class GerritUrlHandler extends AbstractUrlHandler {
 
 	// http://git.eclipse.org/r/123 or https://git.eclipse.org/r/#/c/123/
-	private static final Pattern URL_PATTERN = Pattern.compile("/?(#/c)?/(\\d+)");
+	private static final Pattern URL_PATTERN = Pattern.compile("/?(#/c)?/(\\d+)"); //$NON-NLS-1$
 
 	public GerritUrlHandler() {
 		// ignore
@@ -39,15 +46,35 @@ public class GerritUrlHandler extends AbstractUrlHandler {
 		for (TaskRepository repository : TasksUi.getRepositoryManager().getRepositories(GerritConnector.CONNECTOR_KIND)) {
 			String taskId = getTaskId(repository, url);
 			if (taskId != null) {
-				return TasksUiUtil.openTaskWithResult(repository, taskId);
+				EditorHandle editorHandle = TasksUiUtil.openTaskWithResult(repository, taskId);
+				int patchSetNumber = getPatchSetNumber(repository, url, taskId);
+				if (patchSetNumber > 0) {
+					revealPatchSet(editorHandle, patchSetNumber);
+				}
+				return editorHandle;
 			}
 		}
 		return null;
 	}
 
+	private void revealPatchSet(EditorHandle editorHandle, Integer patchSetNumber) {
+		IWorkbenchPart part = editorHandle.getPart();
+		if (part instanceof TaskEditor) {
+			TaskEditor taskEditor = (TaskEditor) part;
+			IFormPage activePage = taskEditor.setActivePage(GerritTaskEditorPage.class.getName());
+			if (activePage instanceof GerritTaskEditorPage) {
+				GerritTaskEditorPage gerritPage = (GerritTaskEditorPage) activePage;
+				PatchSetSection section = (PatchSetSection) gerritPage.getPart(PatchSetSection.class.getName());
+				if (section != null) {
+					section.revealPatchSet(patchSetNumber);
+				}
+			}
+		}
+	}
+
 	public String getTaskId(TaskRepository repository, String url) {
 		if (url.startsWith(repository.getRepositoryUrl())) {
-			String path = "/" + url.substring(repository.getRepositoryUrl().length());
+			String path = "/" + url.substring(repository.getRepositoryUrl().length()); //$NON-NLS-1$
 			Matcher matcher = URL_PATTERN.matcher(path);
 			if (matcher.find()) {
 				return matcher.group(2);
@@ -56,4 +83,29 @@ public class GerritUrlHandler extends AbstractUrlHandler {
 		return null;
 	}
 
+	/**
+	 * Returns the patch set number as encoded in a Gerrit URL. For example,
+	 * "http://review.mylyn.org/#/c/4698/5/foo/bar", "4698" -> 5. Returns -1 if the patch set number isn't specified or
+	 * is not an integer.
+	 */
+	int getPatchSetNumber(TaskRepository repository, String url, String taskId) {
+		String taskUrl = TasksUi.getRepositoryConnector(GerritConnector.CONNECTOR_KIND).getTaskUrl(repository.getUrl(),
+				taskId);
+		String urlQualifiers = StringUtils.remove(url, taskUrl);
+		String[] fragments = StringUtils.split(urlQualifiers, "/");
+		if (fragments.length > 0) {
+			String patchSetFragment = fragments[0];
+			try {
+				return Integer.valueOf(patchSetFragment);
+			} catch (NumberFormatException e) {
+				//ignore, the patch fragment simply can't be parsed
+			}
+		}
+		return -1;
+	}
+
+	@Override
+	public int getPriority() {
+		return 200;
+	}
 }
diff --git a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/GerritTaskEditorPage.java b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/GerritTaskEditorPage.java
index a7f741e..e2fa0d5 100644
--- a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/GerritTaskEditorPage.java
+++ b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/GerritTaskEditorPage.java
@@ -40,13 +40,14 @@ import org.eclipse.ui.PlatformUI;
 /**
  * @author Mikael Kober
  * @author Thomas Westling
+ * @author Miles Parker
  */
 public class GerritTaskEditorPage extends AbstractReviewTaskEditorPage {
 
 	IReview review;
 
 	public GerritTaskEditorPage(TaskEditor editor) {
-		super(editor, GerritConnector.CONNECTOR_KIND);
+		super(editor, GerritTaskEditorPage.class.getName(), "Gerrit Page", GerritConnector.CONNECTOR_KIND);
 		setNeedsPrivateSection(true);
 		setNeedsSubmitButton(false);
 	}
diff --git a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/PatchSetSection.java b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/PatchSetSection.java
index 54ce9f3..76963db 100644
--- a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/PatchSetSection.java
+++ b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/PatchSetSection.java
@@ -48,6 +48,7 @@ import org.eclipse.jface.viewers.TableViewer;
 import org.eclipse.jface.viewers.Viewer;
 import org.eclipse.jface.window.Window;
 import org.eclipse.jface.wizard.WizardDialog;
+import org.eclipse.mylyn.commons.workbench.forms.CommonFormUtil;
 import org.eclipse.mylyn.internal.gerrit.core.GerritCorePlugin;
 import org.eclipse.mylyn.internal.gerrit.core.GerritTaskSchema;
 import org.eclipse.mylyn.internal.gerrit.core.GerritUtil;
@@ -113,6 +114,7 @@ import com.google.gerrit.reviewdb.PatchSet;
 /**
  * @author Steffen Pingel
  * @author Sascha Scholz
+ * @author Miles Parker
  */
 public class PatchSetSection extends AbstractGerritSection {
 
@@ -163,6 +165,8 @@ public class PatchSetSection extends AbstractGerritSection {
 
 	private ReviewsLabelProvider labelProvider;
 
+	private List<Section> patchSetSections;
+
 	public PatchSetSection() {
 		setPartName("Patch Sets");
 		this.jobs = new ArrayList<Job>();
@@ -203,6 +207,7 @@ public class PatchSetSection extends AbstractGerritSection {
 
 	@Override
 	public void initialize(AbstractTaskEditorPage taskEditorPage) {
+		patchSetSections = new ArrayList<Section>();
 		super.initialize(taskEditorPage);
 	}
 
@@ -355,6 +360,18 @@ public class PatchSetSection extends AbstractGerritSection {
 				}
 			}
 		});
+		patchSetSections.add(subSection);
+	}
+
+	public void revealPatchSet(int patchSetNumber) {
+		if (patchSetNumber > 0 && patchSetNumber <= patchSetSections.size()) {
+			int index = patchSetNumber - 1;
+			Section patchSetSection = patchSetSections.get(index);
+			if (!patchSetSection.isExpanded()) {
+				CommonFormUtil.setExpanded(patchSetSection, true);
+			}
+			CommonFormUtil.ensureVisible(patchSetSection);
+		}
 	}
 
 	private int getNumComments(PatchSetDetail patchSetDetail) {
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewTaskEditorPage.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewTaskEditorPage.java
index 51dee9b..195d55d 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewTaskEditorPage.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewTaskEditorPage.java
@@ -26,6 +26,10 @@ public abstract class AbstractReviewTaskEditorPage extends AbstractTaskEditorPag
 		super(editor, connectorKind);
 	}
 
+	public AbstractReviewTaskEditorPage(TaskEditor editor, String id, String label, String connectorKind) {
+		super(editor, id, label, connectorKind);
+	}
+
 	/**
 	 * Returns the current review. All instances should provide one open, accessible review model instance at init time,
 	 * and that review should be constant throughout the editor life-cycle.