Fix for #962 - Delete patchset ability

7 files changed, 187 insertions, 9 deletions

diff --git a/src/main/java/com/gitblit/models/TicketModel.java b/src/main/java/com/gitblit/models/TicketModel.java
index fd0b09eb..7495448f 100644
--- a/src/main/java/com/gitblit/models/TicketModel.java
+++ b/src/main/java/com/gitblit/models/TicketModel.java
@@ -107,6 +107,30 @@ public class TicketModel implements Serializable, Comparable<TicketModel> {
 		TicketModel ticket;
 		List<Change> effectiveChanges = new ArrayList<Change>();
 		Map<String, Change> comments = new HashMap<String, Change>();
+		Map<Integer, Integer> latestRevisions = new HashMap<Integer, Integer>();
+		
+		int latestPatchsetNumber = -1;
+		
+		List<Integer> deletedPatchsets = new ArrayList<Integer>();
+		
+		for (Change change : changes) {
+			if (change.patchset != null) {
+				if (change.patchset.isDeleted()) {
+					deletedPatchsets.add(change.patchset.number);
+				} else {
+					Integer latestRev = latestRevisions.get(change.patchset.number);
+					
+					if (latestRev == null || change.patchset.rev > latestRev) {
+						latestRevisions.put(change.patchset.number, change.patchset.rev);
+					}
+					
+					if (change.patchset.number > latestPatchsetNumber) {
+						latestPatchsetNumber = change.patchset.number;
+					}	
+				}
+			}
+		}
+		
 		for (Change change : changes) {
 			if (change.comment != null) {
 				if (comments.containsKey(change.comment.id)) {
@@ -122,6 +146,19 @@ public class TicketModel implements Serializable, Comparable<TicketModel> {
 					effectiveChanges.add(change);
 					comments.put(change.comment.id, change);
 				}
+			} else if (change.patchset != null) {
+				//All revisions of a deleted patchset are not displayed
+				if (!deletedPatchsets.contains(change.patchset.number)) {
+					
+					Integer latestRev = latestRevisions.get(change.patchset.number);
+					
+					if (    (change.patchset.number < latestPatchsetNumber) 
+						 && (change.patchset.rev == latestRev)) {
+						change.patchset.canDelete = true;
+					}
+					
+					effectiveChanges.add(change);
+				}
 			} else {
 				effectiveChanges.add(change);
 			}
@@ -1033,10 +1070,16 @@ public class TicketModel implements Serializable, Comparable<TicketModel> {
 		public int added;
 		public PatchsetType type;
 
+		public transient boolean canDelete = false;
+
 		public boolean isFF() {
 			return PatchsetType.FastForward == type;
 		}
 
+		public boolean isDeleted() {
+			return PatchsetType.Delete == type;
+		}
+
 		@Override
 		public int hashCode() {
 			return toString().hashCode();
@@ -1287,7 +1330,7 @@ public class TicketModel implements Serializable, Comparable<TicketModel> {
 	}
 
 	public static enum PatchsetType {
-		Proposal, FastForward, Rebase, Squash, Rebase_Squash, Amend;
+		Proposal, FastForward, Rebase, Squash, Rebase_Squash, Amend, Delete;
 
 		public boolean isRewrite() {
 			return (this != FastForward) && (this != Proposal);
diff --git a/src/main/java/com/gitblit/tickets/ITicketService.java b/src/main/java/com/gitblit/tickets/ITicketService.java
index 5e3e372a..e8310039 100644
--- a/src/main/java/com/gitblit/tickets/ITicketService.java
+++ b/src/main/java/com/gitblit/tickets/ITicketService.java
@@ -48,6 +48,7 @@ import com.gitblit.models.TicketModel.Attachment;
 import com.gitblit.models.TicketModel.Change;
 import com.gitblit.models.TicketModel.Field;
 import com.gitblit.models.TicketModel.Patchset;
+import com.gitblit.models.TicketModel.PatchsetType;
 import com.gitblit.models.TicketModel.Status;
 import com.gitblit.tickets.TicketIndexer.Lucene;
 import com.gitblit.utils.DeepCopier;
@@ -1213,6 +1214,30 @@ public abstract class ITicketService implements IManager {
 		TicketModel revisedTicket = updateTicket(repository, ticket.number, deletion);
 		return revisedTicket;
 	}
+	
+	/**
+	 * Deletes a patchset from a ticket.
+	 *
+	 * @param ticket
+	 * @param patchset
+	 *            the patchset to delete (should be the highest revision)
+	 * @param userName
+	 * 			the user deleting the commit
+	 * @return the revised ticket if the deletion was successful
+	 * @since 1.8.0
+	 */
+	public final TicketModel deletePatchset(TicketModel ticket, Patchset patchset, String userName) {
+		Change deletion = new Change(userName);
+		deletion.patchset = new Patchset();
+		deletion.patchset.number = patchset.number;
+		deletion.patchset.rev = patchset.rev;
+		deletion.patchset.type = PatchsetType.Delete;
+		
+		RepositoryModel repository = repositoryManager.getRepositoryModel(ticket.repository);
+		TicketModel revisedTicket = updateTicket(repository, ticket.number, deletion);
+		
+		return revisedTicket;
+	} 
 
 	/**
 	 * Commit a ticket change to the repository.
diff --git a/src/main/java/com/gitblit/utils/JGitUtils.java b/src/main/java/com/gitblit/utils/JGitUtils.java
index 90d40020..adcbb4de 100644
--- a/src/main/java/com/gitblit/utils/JGitUtils.java
+++ b/src/main/java/com/gitblit/utils/JGitUtils.java
@@ -1745,13 +1745,9 @@ public class JGitUtils {
 	 * @return true if successful

 	 */

 	public static boolean deleteBranchRef(Repository repository, String branch) {

-		String branchName = branch;

-		if (!branchName.startsWith(Constants.R_HEADS)) {

-			branchName = Constants.R_HEADS + branch;

-		}

 

 		try {

-			RefUpdate refUpdate = repository.updateRef(branchName, false);

+			RefUpdate refUpdate = repository.updateRef(branch, false);

 			refUpdate.setForceUpdate(true);

 			RefUpdate.Result result = refUpdate.delete();

 			switch (result) {

@@ -1762,10 +1758,10 @@ public class JGitUtils {
 				return true;

 			default:

 				LOGGER.error(MessageFormat.format("{0} failed to delete to {1} returned result {2}",

-						repository.getDirectory().getAbsolutePath(), branchName, result));

+						repository.getDirectory().getAbsolutePath(), branch, result));

 			}

 		} catch (Throwable t) {

-			error(t, repository, "{0} failed to delete {1}", branchName);

+			error(t, repository, "{0} failed to delete {1}", branch);

 		}

 		return false;

 	}

diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
index f425615c..cee7eaba 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
@@ -775,4 +775,7 @@ gb.editFile = edit file
 gb.continueEditing = Continue Editing
 gb.commitChanges = Commit Changes
 gb.fileNotMergeable = Unable to commit {0}.  This file can not be automatically merged.
-gb.fileCommitted = Successfully committed {0}.
\ No newline at end of file
+gb.fileCommitted = Successfully committed {0}.
+gb.deletePatchset = Delete Patchset {0}
+gb.deletePatchsetSuccess = Deleted Patchset {0}.
+gb.deletePatchsetFailure = Error deleting Patchset {0}.  
diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.html b/src/main/java/com/gitblit/wicket/pages/TicketPage.html
index 5ae005eb..974dcc03 100644
--- a/src/main/java/com/gitblit/wicket/pages/TicketPage.html
+++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.html
@@ -462,6 +462,7 @@ pt push</pre>
 					<span wicket:id="patchsetType">[revision type]</span>					

 				</td>

 				<td><span class="hidden-phone hidden-tablet aui-lozenge aui-lozenge-subtle" wicket:id="patchsetRevision">[R1]</span>

+					<span class="fa fa-fw" style="padding-left:15px;"><a wicket:id="deleteRevision" class="fa fa-fw fa-trash delete-patchset"></a></span>

 					<span class="hidden-tablet hidden-phone" style="padding-left:15px;"><span wicket:id="patchsetDiffStat"></span></span>

 				</td>			

 				<td style="text-align:right;"><span class="attribution-date" wicket:id="changeDate">[patch date]</span></td>	

diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.java b/src/main/java/com/gitblit/wicket/pages/TicketPage.java
index 8bf5c6d9..b2e63a60 100644
--- a/src/main/java/com/gitblit/wicket/pages/TicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.java
@@ -15,6 +15,7 @@
  */

 package com.gitblit.wicket.pages;

 

+import java.io.IOException;

 import java.text.DateFormat;

 import java.text.MessageFormat;

 import java.text.SimpleDateFormat;

@@ -35,6 +36,7 @@ import org.apache.wicket.AttributeModifier;
 import org.apache.wicket.Component;

 import org.apache.wicket.MarkupContainer;

 import org.apache.wicket.PageParameters;

+import org.apache.wicket.RequestCycle;

 import org.apache.wicket.RestartResponseException;

 import org.apache.wicket.ajax.AjaxRequestTarget;

 import org.apache.wicket.behavior.SimpleAttributeModifier;

@@ -42,12 +44,17 @@ import org.apache.wicket.markup.html.basic.Label;
 import org.apache.wicket.markup.html.image.ContextImage;

 import org.apache.wicket.markup.html.link.BookmarkablePageLink;

 import org.apache.wicket.markup.html.link.ExternalLink;

+import org.apache.wicket.markup.html.link.Link;

+import org.apache.wicket.markup.html.link.StatelessLink;

+import org.apache.wicket.markup.html.pages.RedirectPage;

 import org.apache.wicket.markup.html.panel.Fragment;

 import org.apache.wicket.markup.repeater.Item;

 import org.apache.wicket.markup.repeater.data.DataView;

 import org.apache.wicket.markup.repeater.data.ListDataProvider;

 import org.apache.wicket.model.Model;

+import org.apache.wicket.protocol.http.RequestUtils;

 import org.apache.wicket.protocol.http.WebRequest;

+import org.apache.wicket.request.target.basic.RedirectRequestTarget;

 import org.eclipse.jgit.diff.DiffEntry.ChangeType;

 import org.eclipse.jgit.lib.PersonIdent;

 import org.eclipse.jgit.lib.Ref;

@@ -82,13 +89,16 @@ import com.gitblit.tickets.TicketResponsible;
 import com.gitblit.utils.ArrayUtils;

 import com.gitblit.utils.JGitUtils;

 import com.gitblit.utils.JGitUtils.MergeStatus;

+import com.gitblit.utils.CommitCache;

 import com.gitblit.utils.MarkdownUtils;

+import com.gitblit.utils.RefLogUtils;

 import com.gitblit.utils.StringUtils;

 import com.gitblit.utils.TimeUtils;

 import com.gitblit.wicket.GitBlitWebSession;

 import com.gitblit.wicket.TicketsUI;

 import com.gitblit.wicket.WicketUtils;

 import com.gitblit.wicket.panels.AvatarImage;

+import com.gitblit.wicket.panels.BasePanel.JavascriptEventConfirmation;

 import com.gitblit.wicket.panels.BasePanel.JavascriptTextPrompt;

 import com.gitblit.wicket.panels.CommentPanel;

 import com.gitblit.wicket.panels.DiffStatPanel;

@@ -853,6 +863,9 @@ public class TicketPage extends RepositoryPage {
 				if (event.hasPatchset()) {

 					// patchset

 					Patchset patchset = event.patchset;

+					//In the case of using a cached change list

+					item.setVisible(!patchset.isDeleted());

+					

 					String what;

 					if (event.isStatusChange() && (Status.New == event.getStatus())) {

 						what = getString("gb.proposedThisChange");

@@ -880,6 +893,14 @@ public class TicketPage extends RepositoryPage {
 					}

 					item.add(typeLabel);

 

+					Link<Void> deleteLink = createDeletePatchsetLink(repository, patchset);

+					

+					if (user.canDeleteRef(repository)) {

+						item.add(deleteLink.setVisible(patchset.canDelete));

+					} else {

+						item.add(deleteLink.setVisible(false));

+					}

+

 					// show commit diffstat

 					item.add(new DiffStatPanel("patchsetDiffStat", patchset.insertions, patchset.deletions, patchset.rev > 1));

 				} else if (event.hasComment()) {

@@ -887,6 +908,7 @@ public class TicketPage extends RepositoryPage {
 					item.add(new Label("what", getString("gb.commented")));

 					item.add(new Label("patchsetRevision").setVisible(false));

 					item.add(new Label("patchsetType").setVisible(false));

+					item.add(new Label("deleteRevision").setVisible(false));

 					item.add(new Label("patchsetDiffStat").setVisible(false));

 				} else if (event.hasReview()) {

 					// review

@@ -906,11 +928,13 @@ public class TicketPage extends RepositoryPage {
 							.setEscapeModelStrings(false));

 					item.add(new Label("patchsetRevision").setVisible(false));

 					item.add(new Label("patchsetType").setVisible(false));

+					item.add(new Label("deleteRevision").setVisible(false));

 					item.add(new Label("patchsetDiffStat").setVisible(false));

 				} else {

 					// field change

 					item.add(new Label("patchsetRevision").setVisible(false));

 					item.add(new Label("patchsetType").setVisible(false));

+					item.add(new Label("deleteRevision").setVisible(false));

 					item.add(new Label("patchsetDiffStat").setVisible(false));

 

 					String what = "";

@@ -1600,4 +1624,85 @@ public class TicketPage extends RepositoryPage {
 			return copyFragment;

 		}

 	}

+	

+	private Link<Void> createDeletePatchsetLink(final RepositoryModel repositoryModel, final Patchset patchset)

+	{

+		Link<Void> deleteLink = new Link<Void>("deleteRevision") {

+			private static final long serialVersionUID = 1L;

+

+			@Override

+			public void onClick() {

+				Repository r = app().repositories().getRepository(repositoryModel.name);

+				UserModel user = GitBlitWebSession.get().getUser();

+				

+				if (r == null) {

+					if (app().repositories().isCollectingGarbage(repositoryModel.name)) {

+						error(MessageFormat.format(getString("gb.busyCollectingGarbage"), repositoryModel.name));

+					} else {

+						error(MessageFormat.format("Failed to find repository {0}", repositoryModel.name));

+					}

+					return;

+				}

+				

+				//Construct the ref name based on the patchset

+				String ticketShard = String.format("%02d", ticket.number);

+				ticketShard = ticketShard.substring(ticketShard.length() - 2);

+				final String refName = String.format("%s%s/%d/%d", Constants.R_TICKETS_PATCHSETS, ticketShard, ticket.number, patchset.number);

+

+				Ref ref = null;

+				boolean success = true;

+				

+				try {

+					ref = r.getRef(refName);

+					

+					if (ref != null) {

+						success = JGitUtils.deleteBranchRef(r, ref.getName());

+					} else {

+						success = false;

+					}

+					

+					if (success) {

+						// clear commit cache

+						CommitCache.instance().clear(repositoryModel.name, refName);

+

+						// optionally update reflog

+						if (RefLogUtils.hasRefLogBranch(r)) {

+							RefLogUtils.deleteRef(user, r, ref);

+						}

+

+						TicketModel updatedTicket = app().tickets().deletePatchset(ticket, patchset, user.username);

+												

+						if (updatedTicket == null) {

+							success = false;

+						}

+					}

+				} catch (IOException e) {

+					logger().error("failed to determine ticket from ref", e);

+					success = false;

+				} finally {

+					r.close();

+				}

+

+				if (success) {

+					getSession().info(MessageFormat.format(getString("gb.deletePatchsetSuccess"), patchset.number));

+					logger().info(MessageFormat.format("{0} deleted patchset {1} from ticket {2}", 

+							user.username, patchset.number, ticket.number));

+				} else {

+					getSession().error(MessageFormat.format(getString("gb.deletePatchsetFailure"),patchset.number));

+				}

+				

+				//Force reload of the page to rebuild ticket change cache

+				String relativeUrl = urlFor(TicketsPage.class, getPageParameters()).toString();

+				String absoluteUrl = RequestUtils.toAbsolutePath(relativeUrl);

+				setResponsePage(new RedirectPage(absoluteUrl));

+			}

+		};

+

+		WicketUtils.setHtmlTooltip(deleteLink, MessageFormat.format(getString("gb.deletePatchset"), patchset.number));

+		

+		deleteLink.add(new JavascriptEventConfirmation("onclick", MessageFormat.format(getString("gb.deletePatchset"), patchset.number)));

+		

+		return deleteLink;

+	}

+	

 }

diff --git a/src/main/resources/gitblit.css b/src/main/resources/gitblit.css
index 01991909..1cae6d5f 100644
--- a/src/main/resources/gitblit.css
+++ b/src/main/resources/gitblit.css
@@ -2359,4 +2359,9 @@ div.markdown table.text th, div.markdown table.text td {
 

 .filestore-item {

 	color:#815b3a;

+}

+

+.delete-patchset {

+	color:#D51900;

+	font-size: 1.2em;

 }
\ No newline at end of file