ChangeScreen: Keep 'Auto Merge' selected when navigating to diff and back
If the user preference 'Default Base For Merges' is set to
'First Parent' then the change screen pre-selects the first parent in
the 'Diff Against' drop-down list when no base was specified in the
URL. This is what we want, but if the user selects 'Auto Merge' now
and navigates to a diff and then back to the change screen, by
clicking on the up arrow in the diff screen, then it's expected that
'Auto Merge' is still selected. However in this case the change screen
switches back to the first parent. This is because 'Auto Merge' has no
representation in the URL and we cannot differentiate between no base
selected and 'Auto Merge' selected.
To fix this represent patch set n against 'Auto Merge' in the URL as
'AutoMerge..n'. Do this only if the user preference 'Default Base For
Merges' is set to 'First Parent'. If it is set to 'Auto Merge' there
is no problem because no base is resolved to 'Auto Merge'.
Change-Id: I751f63ac0da7ea46ade4012838672ff052c54d2f
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/DiffObject.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/DiffObject.java
index 77003f9..0a1aadd 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/DiffObject.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/DiffObject.java
@@ -14,6 +14,7 @@
package com.google.gerrit.client;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DefaultBase;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -23,6 +24,7 @@
* merge or an edit patch set.
*/
public class DiffObject {
+ public static final String AUTO_MERGE = "AutoMerge";
/**
* Parses a string that represents a diff object.
@@ -34,6 +36,7 @@
* <li>'0': represents the edit patch set
* <li>empty string or null: represents the parent of a 1-parent patch set,
* also called base
+ * <li>'AutoMerge': represents the auto-merge of a merge patch set
* </ul>
*
* @param changeId the ID of the change to which the diff object belongs
@@ -43,7 +46,11 @@
*/
public static DiffObject parse(Change.Id changeId, String str) {
if (str == null || str.isEmpty()) {
- return new DiffObject(null);
+ return new DiffObject(false);
+ }
+
+ if (AUTO_MERGE.equals(str)) {
+ return new DiffObject(true);
}
try {
@@ -57,14 +64,14 @@
* Create a DiffObject that represents the parent of a 1-parent patch set.
*/
public static DiffObject base() {
- return new DiffObject(null);
+ return new DiffObject(false);
}
/**
* Create a DiffObject that represents the auto-merge for a merge patch set.
*/
public static DiffObject autoMerge() {
- return new DiffObject(null);
+ return new DiffObject(true);
}
/**
@@ -75,9 +82,24 @@
}
private final PatchSet.Id psId;
+ private final boolean autoMerge;
private DiffObject(PatchSet.Id psId) {
this.psId = psId;
+ this.autoMerge = false;
+ }
+
+ private DiffObject(boolean autoMerge) {
+ this.psId = null;
+ this.autoMerge = autoMerge;
+ }
+
+ public boolean isBase() {
+ return psId == null && !autoMerge;
+ }
+
+ public boolean isAutoMerge() {
+ return psId == null && autoMerge;
}
public boolean isBaseOrAutoMerge() {
@@ -132,12 +154,20 @@
* <li>a negative integer for a parent of a merge patch set
* <li>'0' for the edit patch set
* <li>{@code null} for the parent of a 1-parent patch set, also called base
- * <li>{@code null} for the auto-merge of a merge patch set
+ * <li>'AutoMerge' for the auto-merge of a merge patch set
* </ul>
*
* @return string representation of this DiffObject
*/
public String asString() {
+ if (autoMerge) {
+ if (Gerrit.getUserPreferences()
+ .defaultBaseForMerges() != DefaultBase.AUTO_MERGE) {
+ return AUTO_MERGE;
+ }
+ return null;
+ }
+
if (psId != null) {
return psId.getId();
}
@@ -159,6 +189,10 @@
return "Edit Patch Set";
}
- return "Base or Auto-Merge";
+ if (isAutoMerge()) {
+ return "Auto Merge";
+ }
+
+ return "Base";
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
index 0674d9a..e2aba0a 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
@@ -153,7 +153,7 @@
Change.Id c = revision.getParentKey();
StringBuilder p = new StringBuilder();
p.append("/c/").append(c).append("/");
- if (diffBase != null && !diffBase.isBaseOrAutoMerge()) {
+ if (diffBase != null && diffBase.asString() != null) {
p.append(diffBase.asString()).append("..");
}
p.append(revision.getId()).append("/").append(KeyUtil.encode(fileName));
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java
index 1271480..443e18b 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java
@@ -20,6 +20,7 @@
import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.GerritUiExtensionPoint;
+import com.google.gerrit.client.NotFoundScreen;
import com.google.gerrit.client.api.ChangeGlue;
import com.google.gerrit.client.api.ExtensionPanel;
import com.google.gerrit.client.changes.ChangeApi;
@@ -300,7 +301,7 @@
group.addListener(new GerritCallback<Void>() {
@Override
public void onSuccess(Void result) {
- if (base.isBaseOrAutoMerge() && rev.isMerge()) {
+ if (base.isBase() && rev.isMerge()) {
base = DiffObject.parse(info.legacyId(),
Gerrit.getUserPreferences()
.defaultBaseForMerges().getBase());
@@ -976,6 +977,10 @@
private void loadConfigInfo(final ChangeInfo info, DiffObject base) {
final RevisionInfo rev = info.revision(revision);
+ if (base.isAutoMerge() && !initCurrentRevision(info).isMerge()) {
+ Gerrit.display(getToken(), new NotFoundScreen());
+ }
+
RevisionInfo baseRev =
resolveRevisionOrPatchSetId(info, base.asString(), null);
@@ -1491,7 +1496,7 @@
RevisionInfo rev = info.revisions().get(revision);
JsArray<CommitInfo> parents = rev.commit().parents();
if (parents.length() > 1) {
- diffBase.addItem(Util.C.autoMerge(), "");
+ diffBase.addItem(Util.C.autoMerge(), DiffObject.AUTO_MERGE);
for (int i = 0; i < parents.length(); i++) {
int parentNum = i + 1;
diffBase.addItem(Util.M.diffBaseParent(parentNum),
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java
index 56691dc..a95270b 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java
@@ -341,7 +341,7 @@
});
setSavePointerId(
- (!base.isBaseOrAutoMerge() ? base.toString() + ".." : "")
+ (!base.isBase() ? base.asString() + ".." : "")
+ curr.toString());
}
