Check for open changes on branch deletion
Check for open changes when deleting a branch in the Gerrit WebUI. Delete a branch only if there are no open changes for this branch. The motivation for this change is to make the user aware of open changes when deleting a branch. Change-Id: Ifa5068dc5f93a48ced69b171465193ee35d5ab3b Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
		| @@ -25,6 +25,7 @@ import com.google.gwt.user.client.rpc.StatusCodeException; | |||||||
| import com.google.gwt.user.client.ui.Button; | import com.google.gwt.user.client.ui.Button; | ||||||
| import com.google.gwt.user.client.ui.FlowPanel; | import com.google.gwt.user.client.ui.FlowPanel; | ||||||
| import com.google.gwt.user.client.ui.Label; | import com.google.gwt.user.client.ui.Label; | ||||||
|  | import com.google.gwt.user.client.ui.Widget; | ||||||
| import com.google.gwtexpui.safehtml.client.SafeHtml; | import com.google.gwtexpui.safehtml.client.SafeHtml; | ||||||
| import com.google.gwtexpui.user.client.PluginSafePopupPanel; | import com.google.gwtexpui.user.client.PluginSafePopupPanel; | ||||||
| import com.google.gwtjsonrpc.client.RemoteJsonException; | import com.google.gwtjsonrpc.client.RemoteJsonException; | ||||||
| @@ -94,6 +95,11 @@ public class ErrorDialog extends PluginSafePopupPanel { | |||||||
|     body.add(message.toBlockWidget()); |     body.add(message.toBlockWidget()); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public ErrorDialog(final Widget w) { | ||||||
|  |     this(); | ||||||
|  |     body.add(w); | ||||||
|  |   } | ||||||
|  |  | ||||||
|   /** Create a dialog box to nicely format an exception. */ |   /** Create a dialog box to nicely format an exception. */ | ||||||
|   public ErrorDialog(final Throwable what) { |   public ErrorDialog(final Throwable what) { | ||||||
|     this(); |     this(); | ||||||
|   | |||||||
| @@ -104,6 +104,7 @@ public interface GerritCss extends CssResource { | |||||||
|   String errorDialogTitle(); |   String errorDialogTitle(); | ||||||
|   String errorDialogButtons(); |   String errorDialogButtons(); | ||||||
|   String errorDialogErrorType(); |   String errorDialogErrorType(); | ||||||
|  |   String errorDialogText(); | ||||||
|   String fileColumnHeader(); |   String fileColumnHeader(); | ||||||
|   String fileLine(); |   String fileLine(); | ||||||
|   String fileLineCONTEXT(); |   String fileLineCONTEXT(); | ||||||
|   | |||||||
| @@ -91,6 +91,7 @@ public interface AdminConstants extends Constants { | |||||||
|   String initialRevision(); |   String initialRevision(); | ||||||
|   String buttonAddBranch(); |   String buttonAddBranch(); | ||||||
|   String buttonDeleteBranch(); |   String buttonDeleteBranch(); | ||||||
|  |   String branchDeletionOpenChanges(); | ||||||
|  |  | ||||||
|   String groupListPrev(); |   String groupListPrev(); | ||||||
|   String groupListNext(); |   String groupListNext(); | ||||||
|   | |||||||
| @@ -71,6 +71,8 @@ columnBranchRevision = Revision | |||||||
| initialRevision = Initial Revision | initialRevision = Initial Revision | ||||||
| buttonAddBranch = Create Branch | buttonAddBranch = Create Branch | ||||||
| buttonDeleteBranch = Delete | buttonDeleteBranch = Delete | ||||||
|  | branchDeletionOpenChanges = The following branches were not deleted \ | ||||||
|  | because they have open changes: | ||||||
|  |  | ||||||
| groupListPrev = Previous group | groupListPrev = Previous group | ||||||
| groupListNext = Next group | groupListNext = Next group | ||||||
|   | |||||||
| @@ -16,9 +16,11 @@ package com.google.gerrit.client.admin; | |||||||
|  |  | ||||||
| import com.google.gerrit.client.ConfirmationCallback; | import com.google.gerrit.client.ConfirmationCallback; | ||||||
| import com.google.gerrit.client.ConfirmationDialog; | import com.google.gerrit.client.ConfirmationDialog; | ||||||
|  | import com.google.gerrit.client.ErrorDialog; | ||||||
| import com.google.gerrit.client.Gerrit; | import com.google.gerrit.client.Gerrit; | ||||||
| import com.google.gerrit.client.rpc.GerritCallback; | import com.google.gerrit.client.rpc.GerritCallback; | ||||||
| import com.google.gerrit.client.rpc.ScreenLoadCallback; | import com.google.gerrit.client.rpc.ScreenLoadCallback; | ||||||
|  | import com.google.gerrit.client.ui.BranchLink; | ||||||
| import com.google.gerrit.client.ui.FancyFlexTable; | import com.google.gerrit.client.ui.FancyFlexTable; | ||||||
| import com.google.gerrit.client.ui.HintTextBox; | import com.google.gerrit.client.ui.HintTextBox; | ||||||
| import com.google.gerrit.common.data.GitwebLink; | import com.google.gerrit.common.data.GitwebLink; | ||||||
| @@ -26,6 +28,7 @@ import com.google.gerrit.common.data.ListBranchesResult; | |||||||
| import com.google.gerrit.common.errors.InvalidNameException; | import com.google.gerrit.common.errors.InvalidNameException; | ||||||
| import com.google.gerrit.common.errors.InvalidRevisionException; | import com.google.gerrit.common.errors.InvalidRevisionException; | ||||||
| import com.google.gerrit.reviewdb.client.Branch; | import com.google.gerrit.reviewdb.client.Branch; | ||||||
|  | import com.google.gerrit.reviewdb.client.Change; | ||||||
| import com.google.gerrit.reviewdb.client.Project; | import com.google.gerrit.reviewdb.client.Project; | ||||||
| import com.google.gwt.core.client.Scheduler; | import com.google.gwt.core.client.Scheduler; | ||||||
| import com.google.gwt.core.client.Scheduler.ScheduledCommand; | import com.google.gwt.core.client.Scheduler.ScheduledCommand; | ||||||
| @@ -41,6 +44,7 @@ import com.google.gwt.user.client.ui.FlexTable.FlexCellFormatter; | |||||||
| import com.google.gwt.user.client.ui.FlowPanel; | import com.google.gwt.user.client.ui.FlowPanel; | ||||||
| import com.google.gwt.user.client.ui.Grid; | import com.google.gwt.user.client.ui.Grid; | ||||||
| import com.google.gwt.user.client.ui.Label; | import com.google.gwt.user.client.ui.Label; | ||||||
|  | import com.google.gwt.user.client.ui.VerticalPanel; | ||||||
| import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder; | import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder; | ||||||
| import com.google.gwtjsonrpc.client.RemoteJsonException; | import com.google.gwtjsonrpc.client.RemoteJsonException; | ||||||
|  |  | ||||||
| @@ -260,24 +264,52 @@ public class ProjectBranchesScreen extends ProjectScreen { | |||||||
|               b.toSafeHtml(), new ConfirmationCallback() { |               b.toSafeHtml(), new ConfirmationCallback() { | ||||||
|         @Override |         @Override | ||||||
|         public void onOk() { |         public void onOk() { | ||||||
|           Util.PROJECT_SVC.deleteBranch(getProjectKey(), ids, |           deleteBranches(ids); | ||||||
|               new GerritCallback<Set<Branch.NameKey>>() { |  | ||||||
|                 public void onSuccess(final Set<Branch.NameKey> deleted) { |  | ||||||
|                   for (int row = 1; row < table.getRowCount();) { |  | ||||||
|                     final Branch k = getRowItem(row); |  | ||||||
|                     if (k != null && deleted.contains(k.getNameKey())) { |  | ||||||
|                       table.removeRow(row); |  | ||||||
|                     } else { |  | ||||||
|                       row++; |  | ||||||
|                     } |  | ||||||
|                   } |  | ||||||
|                 } |  | ||||||
|               }); |  | ||||||
|         } |         } | ||||||
|       }); |       }); | ||||||
|       confirmationDialog.center(); |       confirmationDialog.center(); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|  |     private void deleteBranches(final Set<Branch.NameKey> branchIds) { | ||||||
|  |       Util.PROJECT_SVC.deleteBranch(getProjectKey(), branchIds, | ||||||
|  |           new GerritCallback<Set<Branch.NameKey>>() { | ||||||
|  |             public void onSuccess(final Set<Branch.NameKey> deleted) { | ||||||
|  |               if (!deleted.isEmpty()) { | ||||||
|  |                 for (int row = 1; row < table.getRowCount();) { | ||||||
|  |                   final Branch k = getRowItem(row); | ||||||
|  |                   if (k != null && deleted.contains(k.getNameKey())) { | ||||||
|  |                     table.removeRow(row); | ||||||
|  |                   } else { | ||||||
|  |                     row++; | ||||||
|  |                   } | ||||||
|  |                 } | ||||||
|  |               } | ||||||
|  |  | ||||||
|  |               branchIds.removeAll(deleted); | ||||||
|  |               if (!branchIds.isEmpty()) { | ||||||
|  |                 final VerticalPanel p = new VerticalPanel(); | ||||||
|  |                 final ErrorDialog errorDialog = new ErrorDialog(p); | ||||||
|  |                 final Label l = new Label(Util.C.branchDeletionOpenChanges()); | ||||||
|  |                 l.setStyleName(Gerrit.RESOURCES.css().errorDialogText()); | ||||||
|  |                 p.add(l); | ||||||
|  |                 for (final Branch.NameKey branch : branchIds) { | ||||||
|  |                   final BranchLink link = | ||||||
|  |                       new BranchLink(branch.getParentKey(), Change.Status.NEW, | ||||||
|  |                           branch.get(), null) { | ||||||
|  |                     @Override | ||||||
|  |                     public void go() { | ||||||
|  |                       errorDialog.hide(); | ||||||
|  |                       super.go(); | ||||||
|  |                     }; | ||||||
|  |                   }; | ||||||
|  |                   p.add(link); | ||||||
|  |                 } | ||||||
|  |                 errorDialog.center(); | ||||||
|  |               } | ||||||
|  |             } | ||||||
|  |           }); | ||||||
|  |     } | ||||||
|  |  | ||||||
|     void display(final List<Branch> result) { |     void display(final List<Branch> result) { | ||||||
|       canDelete = false; |       canDelete = false; | ||||||
|  |  | ||||||
|   | |||||||
| @@ -375,6 +375,18 @@ | |||||||
|   width: 100%; |   width: 100%; | ||||||
|   margin-top: 15px; |   margin-top: 15px; | ||||||
| } | } | ||||||
|  | .errorDialogText { | ||||||
|  |   font-size: 15px; | ||||||
|  |   font-family: verdana; | ||||||
|  | } | ||||||
|  | .errorDialog a, | ||||||
|  | .errorDialog a:visited, | ||||||
|  | .errorDialog a:hover { | ||||||
|  |   color: white; | ||||||
|  |   font-weight: bold; | ||||||
|  |   font-size: 15px; | ||||||
|  |   font-family: verdana; | ||||||
|  | } | ||||||
|  |  | ||||||
|  |  | ||||||
| /** Screen **/ | /** Screen **/ | ||||||
|   | |||||||
| @@ -18,11 +18,13 @@ import com.google.gerrit.common.ChangeHooks; | |||||||
| import com.google.gerrit.httpd.rpc.Handler; | import com.google.gerrit.httpd.rpc.Handler; | ||||||
| import com.google.gerrit.reviewdb.client.Branch; | import com.google.gerrit.reviewdb.client.Branch; | ||||||
| import com.google.gerrit.reviewdb.client.Project; | import com.google.gerrit.reviewdb.client.Project; | ||||||
|  | import com.google.gerrit.reviewdb.server.ReviewDb; | ||||||
| import com.google.gerrit.server.IdentifiedUser; | import com.google.gerrit.server.IdentifiedUser; | ||||||
| import com.google.gerrit.server.git.GitRepositoryManager; | import com.google.gerrit.server.git.GitRepositoryManager; | ||||||
| import com.google.gerrit.server.git.ReplicationQueue; | import com.google.gerrit.server.git.ReplicationQueue; | ||||||
| import com.google.gerrit.server.project.NoSuchProjectException; | import com.google.gerrit.server.project.NoSuchProjectException; | ||||||
| import com.google.gerrit.server.project.ProjectControl; | import com.google.gerrit.server.project.ProjectControl; | ||||||
|  | import com.google.gwtorm.server.OrmException; | ||||||
| import com.google.inject.Inject; | import com.google.inject.Inject; | ||||||
| import com.google.inject.assistedinject.Assisted; | import com.google.inject.assistedinject.Assisted; | ||||||
|  |  | ||||||
| @@ -34,6 +36,7 @@ import org.slf4j.LoggerFactory; | |||||||
|  |  | ||||||
| import java.io.IOException; | import java.io.IOException; | ||||||
| import java.util.HashSet; | import java.util.HashSet; | ||||||
|  | import java.util.Iterator; | ||||||
| import java.util.Set; | import java.util.Set; | ||||||
|  |  | ||||||
| class DeleteBranches extends Handler<Set<Branch.NameKey>> { | class DeleteBranches extends Handler<Set<Branch.NameKey>> { | ||||||
| @@ -50,6 +53,7 @@ class DeleteBranches extends Handler<Set<Branch.NameKey>> { | |||||||
|   private final ReplicationQueue replication; |   private final ReplicationQueue replication; | ||||||
|   private final IdentifiedUser identifiedUser; |   private final IdentifiedUser identifiedUser; | ||||||
|   private final ChangeHooks hooks; |   private final ChangeHooks hooks; | ||||||
|  |   private final ReviewDb db; | ||||||
|  |  | ||||||
|   private final Project.NameKey projectName; |   private final Project.NameKey projectName; | ||||||
|   private final Set<Branch.NameKey> toRemove; |   private final Set<Branch.NameKey> toRemove; | ||||||
| @@ -60,6 +64,7 @@ class DeleteBranches extends Handler<Set<Branch.NameKey>> { | |||||||
|       final ReplicationQueue replication, |       final ReplicationQueue replication, | ||||||
|       final IdentifiedUser identifiedUser, |       final IdentifiedUser identifiedUser, | ||||||
|       final ChangeHooks hooks, |       final ChangeHooks hooks, | ||||||
|  |       final ReviewDb db, | ||||||
|  |  | ||||||
|       @Assisted Project.NameKey name, @Assisted Set<Branch.NameKey> toRemove) { |       @Assisted Project.NameKey name, @Assisted Set<Branch.NameKey> toRemove) { | ||||||
|     this.projectControlFactory = projectControlFactory; |     this.projectControlFactory = projectControlFactory; | ||||||
| @@ -67,6 +72,7 @@ class DeleteBranches extends Handler<Set<Branch.NameKey>> { | |||||||
|     this.replication = replication; |     this.replication = replication; | ||||||
|     this.identifiedUser = identifiedUser; |     this.identifiedUser = identifiedUser; | ||||||
|     this.hooks = hooks; |     this.hooks = hooks; | ||||||
|  |     this.db = db; | ||||||
|  |  | ||||||
|     this.projectName = name; |     this.projectName = name; | ||||||
|     this.toRemove = toRemove; |     this.toRemove = toRemove; | ||||||
| @@ -74,17 +80,23 @@ class DeleteBranches extends Handler<Set<Branch.NameKey>> { | |||||||
|  |  | ||||||
|   @Override |   @Override | ||||||
|   public Set<Branch.NameKey> call() throws NoSuchProjectException, |   public Set<Branch.NameKey> call() throws NoSuchProjectException, | ||||||
|       RepositoryNotFoundException { |       RepositoryNotFoundException, OrmException { | ||||||
|     final ProjectControl projectControl = |     final ProjectControl projectControl = | ||||||
|         projectControlFactory.controlFor(projectName); |         projectControlFactory.controlFor(projectName); | ||||||
|  |  | ||||||
|     for (Branch.NameKey k : toRemove) { |     final Iterator<Branch.NameKey> branchIt = toRemove.iterator(); | ||||||
|  |     while (branchIt.hasNext()) { | ||||||
|  |       final Branch.NameKey k = branchIt.next(); | ||||||
|       if (!projectName.equals(k.getParentKey())) { |       if (!projectName.equals(k.getParentKey())) { | ||||||
|         throw new IllegalArgumentException("All keys must be from same project"); |         throw new IllegalArgumentException("All keys must be from same project"); | ||||||
|       } |       } | ||||||
|       if (!projectControl.controlForRef(k).canDelete()) { |       if (!projectControl.controlForRef(k).canDelete()) { | ||||||
|         throw new IllegalStateException("Cannot delete " + k.getShortName()); |         throw new IllegalStateException("Cannot delete " + k.getShortName()); | ||||||
|       } |       } | ||||||
|  |  | ||||||
|  |       if (db.changes().byBranchOpenAll(k).iterator().hasNext()) { | ||||||
|  |         branchIt.remove(); | ||||||
|  |       } | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     final Set<Branch.NameKey> deleted = new HashSet<Branch.NameKey>(); |     final Set<Branch.NameKey> deleted = new HashSet<Branch.NameKey>(); | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Edwin Kempin
					Edwin Kempin