From: Paul Martin Date: Sun, 3 Apr 2016 19:30:22 +0000 (+0100) Subject: Fix for #962 - Delete patchset ability X-Git-Tag: v1.8.0~12^2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fpull%2F1039%2Fhead;p=gitblit.git 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 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 ticket; List effectiveChanges = new ArrayList(); Map comments = new HashMap(); + Map latestRevisions = new HashMap(); + + int latestPatchsetNumber = -1; + + List deletedPatchsets = new ArrayList(); + + 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 { 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 { 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 { } 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 [revision type] [R1] + [patch date] 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 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 createDeletePatchsetLink(final RepositoryModel repositoryModel, final Patchset patchset) + { + Link deleteLink = new Link("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