Fix for #962 - Delete patchset ability
diff --git a/src/main/java/com/gitblit/models/TicketModel.java b/src/main/java/com/gitblit/models/TicketModel.java
index fd0b09e..7495448 100644
--- a/src/main/java/com/gitblit/models/TicketModel.java
+++ b/src/main/java/com/gitblit/models/TicketModel.java
@@ -107,6 +107,30 @@
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 @@
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 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 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 5e3e372..e831003 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.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 @@
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 90d4002..adcbb4d 100644
--- a/src/main/java/com/gitblit/utils/JGitUtils.java
+++ b/src/main/java/com/gitblit/utils/JGitUtils.java
@@ -1745,13 +1745,9 @@
* @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 @@
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 f425615..cee7eab 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
@@ -775,4 +775,7 @@
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 5ae005e..974dcc0 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 @@
<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 8bf5c6d..b2e63a6 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.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.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.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 @@
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 @@
}
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 @@
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 @@
.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 @@
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 0199190..1cae6d5 100644
--- a/src/main/resources/gitblit.css
+++ b/src/main/resources/gitblit.css
@@ -2359,4 +2359,9 @@
.filestore-item {
color:#815b3a;
+}
+
+.delete-patchset {
+ color:#D51900;
+ font-size: 1.2em;
}
\ No newline at end of file
