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>
This commit is contained in:
Edwin Kempin
2016-09-14 17:12:06 +02:00
parent 1a865ce7fd
commit c235ba7e51
4 changed files with 48 additions and 9 deletions

View File

@@ -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 @@ import com.google.gerrit.reviewdb.client.PatchSet;
* 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 @@ public class DiffObject {
* <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 class DiffObject {
*/
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 @@ public class DiffObject {
* 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 @@ public class DiffObject {
}
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 @@ public class DiffObject {
* <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 @@ public class DiffObject {
return "Edit Patch Set";
}
return "Base or Auto-Merge";
if (isAutoMerge()) {
return "Auto Merge";
}
return "Base";
}
}

View File

@@ -153,7 +153,7 @@ public class Dispatcher {
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));

View File

@@ -20,6 +20,7 @@ import com.google.gerrit.client.ErrorDialog;
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 @@ public class ChangeScreen extends Screen {
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 @@ public class ChangeScreen extends Screen {
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 @@ public class ChangeScreen extends Screen {
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),

View File

@@ -341,7 +341,7 @@ public class FileTable extends FlowPanel {
});
setSavePointerId(
(!base.isBaseOrAutoMerge() ? base.toString() + ".." : "")
(!base.isBase() ? base.asString() + ".." : "")
+ curr.toString());
}