Merge "Reload repo and group list after creating a repo or group" into stable-2.16
diff --git a/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java b/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java
index 66d6555..0bfe5fd 100644
--- a/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java
+++ b/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java
@@ -13,10 +13,11 @@
 // limitations under the License.
 package com.google.gerrit.server.config;
 
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.Multimap;
 import java.util.Collections;
 import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Objects;
 import java.util.Set;
 import org.apache.commons.lang.StringUtils;
@@ -36,6 +37,8 @@
  * (+ various overloaded versions of these)
  */
 public class ConfigUpdatedEvent {
+  public static final Multimap<UpdateResult, ConfigUpdateEntry> NO_UPDATES =
+      new ImmutableMultimap.Builder<UpdateResult, ConfigUpdateEntry>().build();
   private final Config oldConfig;
   private final Config newConfig;
 
@@ -52,25 +55,29 @@
     return this.newConfig;
   }
 
-  public Update accept(ConfigKey entry) {
+  private String getString(ConfigKey key, Config config) {
+    return config.getString(key.section(), key.subsection(), key.name());
+  }
+
+  public Multimap<UpdateResult, ConfigUpdateEntry> accept(ConfigKey entry) {
     return accept(Collections.singleton(entry));
   }
 
-  public Update accept(Set<ConfigKey> entries) {
+  public Multimap<UpdateResult, ConfigUpdateEntry> accept(Set<ConfigKey> entries) {
     return createUpdate(entries, UpdateResult.APPLIED);
   }
 
-  public Update accept(String section) {
+  public Multimap<UpdateResult, ConfigUpdateEntry> accept(String section) {
     Set<ConfigKey> entries = getEntriesFromSection(oldConfig, section);
     entries.addAll(getEntriesFromSection(newConfig, section));
     return createUpdate(entries, UpdateResult.APPLIED);
   }
 
-  public Update reject(ConfigKey entry) {
+  public Multimap<UpdateResult, ConfigUpdateEntry> reject(ConfigKey entry) {
     return reject(Collections.singleton(entry));
   }
 
-  public Update reject(Set<ConfigKey> entries) {
+  public Multimap<UpdateResult, ConfigUpdateEntry> reject(Set<ConfigKey> entries) {
     return createUpdate(entries, UpdateResult.REJECTED);
   }
 
@@ -87,20 +94,15 @@
     return res;
   }
 
-  private Update createUpdate(Set<ConfigKey> entries, UpdateResult updateResult) {
-    Update update = new Update(updateResult);
+  private Multimap<UpdateResult, ConfigUpdateEntry> createUpdate(
+      Set<ConfigKey> entries, UpdateResult updateResult) {
+    Multimap<UpdateResult, ConfigUpdateEntry> updates = ArrayListMultimap.create();
     entries
         .stream()
         .filter(this::isValueUpdated)
-        .forEach(
-            key -> {
-              update.addConfigUpdate(
-                  new ConfigUpdateEntry(
-                      key,
-                      oldConfig.getString(key.section(), key.subsection(), key.name()),
-                      newConfig.getString(key.section(), key.subsection(), key.name())));
-            });
-    return update;
+        .map(e -> new ConfigUpdateEntry(e, getString(e, oldConfig), getString(e, newConfig)))
+        .forEach(e -> updates.put(updateResult, e));
+    return updates;
   }
 
   public boolean isSectionUpdated(String section) {
@@ -142,31 +144,6 @@
     }
   }
 
-  /**
-   * One Accepted/Rejected Update have one or more config updates (ConfigUpdateEntry) tied to it.
-   */
-  public static class Update {
-    private UpdateResult result;
-    private final Set<ConfigUpdateEntry> configUpdates;
-
-    public Update(UpdateResult result) {
-      this.configUpdates = new LinkedHashSet<>();
-      this.result = result;
-    }
-
-    public UpdateResult getResult() {
-      return result;
-    }
-
-    public List<ConfigUpdateEntry> getConfigUpdates() {
-      return ImmutableList.copyOf(configUpdates);
-    }
-
-    public void addConfigUpdate(ConfigUpdateEntry entry) {
-      this.configUpdates.add(entry);
-    }
-  }
-
   public enum ConfigEntryType {
     ADDED,
     REMOVED,
diff --git a/java/com/google/gerrit/server/config/GerritConfigListener.java b/java/com/google/gerrit/server/config/GerritConfigListener.java
index 337a962..f5b2976 100644
--- a/java/com/google/gerrit/server/config/GerritConfigListener.java
+++ b/java/com/google/gerrit/server/config/GerritConfigListener.java
@@ -14,9 +14,11 @@
 
 package com.google.gerrit.server.config;
 
+import com.google.common.collect.Multimap;
 import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
 import java.util.EventListener;
-import java.util.List;
 
 /**
  * Implementations of the GerritConfigListener interface expects to react GerritServerConfig
@@ -24,5 +26,5 @@
  */
 @ExtensionPoint
 public interface GerritConfigListener extends EventListener {
-  List<ConfigUpdatedEvent.Update> configUpdated(ConfigUpdatedEvent event);
+  Multimap<UpdateResult, ConfigUpdateEntry> configUpdated(ConfigUpdatedEvent event);
 }
diff --git a/java/com/google/gerrit/server/config/GerritConfigListenerHelper.java b/java/com/google/gerrit/server/config/GerritConfigListenerHelper.java
index 1dfa3fc..d21e1c3 100644
--- a/java/com/google/gerrit/server/config/GerritConfigListenerHelper.java
+++ b/java/com/google/gerrit/server/config/GerritConfigListenerHelper.java
@@ -15,13 +15,12 @@
 package com.google.gerrit.server.config;
 
 import com.google.common.collect.ImmutableSet;
-import java.util.Collections;
 
 public class GerritConfigListenerHelper {
   public static GerritConfigListener acceptIfChanged(ConfigKey... keys) {
     return e ->
         e.isEntriesUpdated(ImmutableSet.copyOf(keys))
-            ? Collections.singletonList(e.accept(ImmutableSet.copyOf(keys)))
-            : Collections.emptyList();
+            ? e.accept(ImmutableSet.copyOf(keys))
+            : ConfigUpdatedEvent.NO_UPDATES;
   }
 }
diff --git a/java/com/google/gerrit/server/config/GerritServerConfigReloader.java b/java/com/google/gerrit/server/config/GerritServerConfigReloader.java
index 1890de8..09c10740 100644
--- a/java/com/google/gerrit/server/config/GerritServerConfigReloader.java
+++ b/java/com/google/gerrit/server/config/GerritServerConfigReloader.java
@@ -14,12 +14,14 @@
 
 package com.google.gerrit.server.config;
 
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
-import java.util.ArrayList;
-import java.util.List;
 
 /** Issues a configuration reload from the GerritServerConfigProvider and notify all listeners. */
 @Singleton
@@ -40,18 +42,20 @@
    * Reloads the Gerrit Server Configuration from disk. Synchronized to ensure that one issued
    * reload is fully completed before a new one starts.
    */
-  public List<ConfigUpdatedEvent.Update> reloadConfig() {
+  public Multimap<UpdateResult, ConfigUpdateEntry> reloadConfig() {
     logger.atInfo().log("Starting server configuration reload");
-    List<ConfigUpdatedEvent.Update> updates = fireUpdatedConfigEvent(configProvider.updateConfig());
+    Multimap<UpdateResult, ConfigUpdateEntry> updates =
+        fireUpdatedConfigEvent(configProvider.updateConfig());
     logger.atInfo().log("Server configuration reload completed succesfully");
     return updates;
   }
 
-  public List<ConfigUpdatedEvent.Update> fireUpdatedConfigEvent(ConfigUpdatedEvent event) {
-    ArrayList<ConfigUpdatedEvent.Update> result = new ArrayList<>();
+  public Multimap<UpdateResult, ConfigUpdateEntry> fireUpdatedConfigEvent(
+      ConfigUpdatedEvent event) {
+    Multimap<UpdateResult, ConfigUpdateEntry> updates = ArrayListMultimap.create();
     for (GerritConfigListener configListener : configListeners) {
-      result.addAll(configListener.configUpdated(event));
+      updates.putAll(configListener.configUpdated(event));
     }
-    return result;
+    return updates;
   }
 }
diff --git a/java/com/google/gerrit/server/project/CommentLinkProvider.java b/java/com/google/gerrit/server/project/CommentLinkProvider.java
index 56cf51e..4987d00 100644
--- a/java/com/google/gerrit/server/project/CommentLinkProvider.java
+++ b/java/com/google/gerrit/server/project/CommentLinkProvider.java
@@ -16,15 +16,17 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Multimap;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
 import com.google.gerrit.server.config.ConfigUpdatedEvent;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
 import com.google.gerrit.server.config.GerritConfigListener;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
-import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 import org.eclipse.jgit.lib.Config;
@@ -64,11 +66,11 @@
   }
 
   @Override
-  public List<ConfigUpdatedEvent.Update> configUpdated(ConfigUpdatedEvent event) {
+  public Multimap<UpdateResult, ConfigUpdateEntry> configUpdated(ConfigUpdatedEvent event) {
     if (event.isSectionUpdated(ProjectConfig.COMMENTLINK)) {
       commentLinks = parseConfig(event.getNewConfig());
-      return Collections.singletonList(event.accept(ProjectConfig.COMMENTLINK));
+      return event.accept(ProjectConfig.COMMENTLINK);
     }
-    return Collections.emptyList();
+    return ConfigUpdatedEvent.NO_UPDATES;
   }
 }
diff --git a/java/com/google/gerrit/server/restapi/config/ReloadConfig.java b/java/com/google/gerrit/server/restapi/config/ReloadConfig.java
index de3c3ee..cab07e3 100644
--- a/java/com/google/gerrit/server/restapi/config/ReloadConfig.java
+++ b/java/com/google/gerrit/server/restapi/config/ReloadConfig.java
@@ -16,12 +16,12 @@
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 
+import com.google.common.collect.Multimap;
 import com.google.gerrit.extensions.api.config.ConfigUpdateEntryInfo;
 import com.google.gerrit.extensions.common.Input;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.server.config.ConfigResource;
-import com.google.gerrit.server.config.ConfigUpdatedEvent;
 import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
 import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
 import com.google.gerrit.server.config.GerritServerConfigReloader;
@@ -29,10 +29,11 @@
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.inject.Inject;
-import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 public class ReloadConfig implements RestModifyView<ConfigResource, Input> {
 
@@ -49,25 +50,22 @@
   public Map<String, List<ConfigUpdateEntryInfo>> apply(ConfigResource resource, Input input)
       throws RestApiException, PermissionBackendException {
     permissions.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
-
-    List<ConfigUpdatedEvent.Update> updates = config.reloadConfig();
-
-    Map<String, List<ConfigUpdateEntryInfo>> reply = new HashMap<>();
-    for (UpdateResult result : UpdateResult.values()) {
-      reply.put(result.name().toLowerCase(), new ArrayList<>());
-    }
+    Multimap<UpdateResult, ConfigUpdateEntry> updates = config.reloadConfig();
     if (updates.isEmpty()) {
-      return reply;
+      return Collections.emptyMap();
     }
-    updates
+    return updates
+        .asMap()
+        .entrySet()
         .stream()
-        .forEach(u -> reply.get(u.getResult().name().toLowerCase()).addAll(toEntryInfos(u)));
-    return reply;
+        .collect(
+            Collectors.toMap(
+                e -> e.getKey().name().toLowerCase(), e -> toEntryInfos(e.getValue())));
   }
 
-  private static List<ConfigUpdateEntryInfo> toEntryInfos(ConfigUpdatedEvent.Update update) {
-    return update
-        .getConfigUpdates()
+  private static List<ConfigUpdateEntryInfo> toEntryInfos(
+      Collection<ConfigUpdateEntry> updateEntries) {
+    return updateEntries
         .stream()
         .map(ReloadConfig::toConfigUpdateEntryInfo)
         .collect(toImmutableList());
diff --git a/java/com/google/gerrit/server/restapi/project/SetParent.java b/java/com/google/gerrit/server/restapi/project/SetParent.java
index ca7e7aa..d02d04a 100644
--- a/java/com/google/gerrit/server/restapi/project/SetParent.java
+++ b/java/com/google/gerrit/server/restapi/project/SetParent.java
@@ -19,6 +19,7 @@
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Multimap;
 import com.google.gerrit.extensions.api.projects.ParentInput;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -32,6 +33,8 @@
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.ConfigKey;
 import com.google.gerrit.server.config.ConfigUpdatedEvent;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
 import com.google.gerrit.server.config.GerritConfigListener;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.meta.MetaDataUpdate;
@@ -46,8 +49,6 @@
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
-import java.util.Collections;
-import java.util.List;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Config;
@@ -172,18 +173,18 @@
   }
 
   @Override
-  public List<ConfigUpdatedEvent.Update> configUpdated(ConfigUpdatedEvent event) {
+  public Multimap<UpdateResult, ConfigUpdateEntry> configUpdated(ConfigUpdatedEvent event) {
     ConfigKey receiveSetParent = ConfigKey.create("receive", "allowProjectOwnersToChangeParent");
     if (!event.isValueUpdated(receiveSetParent)) {
-      return Collections.emptyList();
+      return ConfigUpdatedEvent.NO_UPDATES;
     }
     try {
       boolean enabled =
           event.getNewConfig().getBoolean("receive", "allowProjectOwnersToChangeParent", false);
       this.allowProjectOwnersToChangeParent = enabled;
-      return Collections.singletonList(event.accept(receiveSetParent));
     } catch (IllegalArgumentException iae) {
-      return Collections.singletonList(event.reject(receiveSetParent));
+      return event.reject(receiveSetParent);
     }
+    return event.accept(receiveSetParent);
   }
 }
diff --git a/java/com/google/gerrit/sshd/SshLog.java b/java/com/google/gerrit/sshd/SshLog.java
index 0e34889..df3242c 100644
--- a/java/com/google/gerrit/sshd/SshLog.java
+++ b/java/com/google/gerrit/sshd/SshLog.java
@@ -15,6 +15,7 @@
 package com.google.gerrit.sshd;
 
 import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Multimap;
 import com.google.common.collect.MultimapBuilder;
 import com.google.gerrit.extensions.events.LifecycleListener;
 import com.google.gerrit.server.CurrentUser;
@@ -24,6 +25,8 @@
 import com.google.gerrit.server.audit.SshAuditEvent;
 import com.google.gerrit.server.config.ConfigKey;
 import com.google.gerrit.server.config.ConfigUpdatedEvent;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
 import com.google.gerrit.server.config.GerritConfigListener;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.ioutil.HexFormat;
@@ -33,8 +36,6 @@
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
-import java.util.Collections;
-import java.util.List;
 import org.apache.log4j.AsyncAppender;
 import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
@@ -318,25 +319,22 @@
   }
 
   @Override
-  public List<ConfigUpdatedEvent.Update> configUpdated(ConfigUpdatedEvent event) {
+  public Multimap<UpdateResult, ConfigUpdateEntry> configUpdated(ConfigUpdatedEvent event) {
     ConfigKey sshdRequestLog = ConfigKey.create("sshd", "requestLog");
     if (!event.isValueUpdated(sshdRequestLog)) {
-      return Collections.emptyList();
+      return ConfigUpdatedEvent.NO_UPDATES;
     }
     boolean stateUpdated;
     try {
       boolean enabled = event.getNewConfig().getBoolean("sshd", "requestLog", true);
-
       if (enabled) {
         stateUpdated = enableLogging();
       } else {
         stateUpdated = disableLogging();
       }
-      return stateUpdated
-          ? Collections.singletonList(event.accept(sshdRequestLog))
-          : Collections.emptyList();
+      return stateUpdated ? event.accept(sshdRequestLog) : ConfigUpdatedEvent.NO_UPDATES;
     } catch (IllegalArgumentException iae) {
-      return Collections.singletonList(event.reject(sshdRequestLog));
+      return event.reject(sshdRequestLog);
     }
   }
 }
diff --git a/java/com/google/gerrit/sshd/commands/ReloadConfig.java b/java/com/google/gerrit/sshd/commands/ReloadConfig.java
index 1b21230..cbe3c57 100644
--- a/java/com/google/gerrit/sshd/commands/ReloadConfig.java
+++ b/java/com/google/gerrit/sshd/commands/ReloadConfig.java
@@ -16,16 +16,15 @@
 
 import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER_OR_SLAVE;
 
+import com.google.common.collect.Multimap;
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.extensions.annotations.RequiresCapability;
-import com.google.gerrit.server.config.ConfigUpdatedEvent;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
 import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
 import com.google.gerrit.server.config.GerritServerConfigReloader;
 import com.google.gerrit.sshd.CommandMetaData;
 import com.google.gerrit.sshd.SshCommand;
 import com.google.inject.Inject;
-import java.util.List;
-import java.util.stream.Collectors;
 
 /** Issues a reload of gerrit.config. */
 @RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
@@ -39,31 +38,16 @@
 
   @Override
   protected void run() throws Failure {
-    List<ConfigUpdatedEvent.Update> updates = gerritServerConfigReloader.reloadConfig();
+    Multimap<UpdateResult, ConfigUpdateEntry> updates = gerritServerConfigReloader.reloadConfig();
     if (updates.isEmpty()) {
       stdout.println("No config entries updated!");
       return;
     }
 
     // Print out UpdateResult.{ACCEPTED|REJECTED} entries grouped by their type
-    for (UpdateResult updateResult : UpdateResult.values()) {
-      List<ConfigUpdatedEvent.Update> filteredUpdates = filterUpdates(updates, updateResult);
-      if (filteredUpdates.isEmpty()) {
-        continue;
-      }
-      stdout.println(updateResult.toString() + " configuration changes:");
-      filteredUpdates
-          .stream()
-          .flatMap(update -> update.getConfigUpdates().stream())
-          .forEach(cfgEntry -> stdout.println(cfgEntry.toString()));
+    for (UpdateResult result : updates.keySet()) {
+      stdout.println(result.toString() + " configuration changes:");
+      updates.get(result).forEach(cfgEntry -> stdout.println(cfgEntry.toString()));
     }
   }
-
-  public static List<ConfigUpdatedEvent.Update> filterUpdates(
-      List<ConfigUpdatedEvent.Update> updates, UpdateResult result) {
-    return updates
-        .stream()
-        .filter(update -> update.getResult() == result)
-        .collect(Collectors.toList());
-  }
 }
diff --git a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
index d1fdf2f..af982cf 100644
--- a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
+++ b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
@@ -165,6 +165,7 @@
     PREV_FILE: 'PREV_FILE',
     NEXT_FILE_WITH_COMMENTS: 'NEXT_FILE_WITH_COMMENTS',
     PREV_FILE_WITH_COMMENTS: 'PREV_FILE_WITH_COMMENTS',
+    NEXT_UNREVIEWED_FILE: 'NEXT_UNREVIEWED_FILE',
     CURSOR_NEXT_FILE: 'CURSOR_NEXT_FILE',
     CURSOR_PREV_FILE: 'CURSOR_PREV_FILE',
     OPEN_FILE: 'OPEN_FILE',
@@ -255,6 +256,8 @@
       'Mark/unmark file as reviewed');
   _describe(Shortcut.TOGGLE_DIFF_MODE, ShortcutSection.DIFFS,
       'Toggle unified/side-by-side diff');
+  _describe(Shortcut.NEXT_UNREVIEWED_FILE, ShortcutSection.DIFFS,
+      'Mark file as reviewed and go to next unreviewed file');
 
   _describe(Shortcut.NEXT_FILE, ShortcutSection.NAVIGATION, 'Select next file');
   _describe(Shortcut.PREV_FILE, ShortcutSection.NAVIGATION,
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index 6509bb1..bec08a8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -27,6 +27,7 @@
 <link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
 <link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
 <link rel="import" href="../../shared/gr-editable-label/gr-editable-label.html">
+<link rel="import" href="../../shared/gr-icons/gr-icons.html">
 <link rel="import" href="../../shared/gr-limited-text/gr-limited-text.html">
 <link rel="import" href="../../shared/gr-linked-chip/gr-linked-chip.html">
 <link rel="import" href="../../shared/gr-tooltip-content/gr-tooltip-content.html">
@@ -114,6 +115,19 @@
       #parentNotCurrentMessage {
         display: none;
       }
+      .icon {
+        margin: -.25em 0;
+      }
+      .icon.help,
+      .icon.notTrusted {
+        color: #FFA62F;
+      }
+      .icon.invalid {
+        color: var(--vote-text-color-disliked);
+      }
+      .icon.trusted {
+        color: var(--vote-text-color-recommended);
+      }
       .parentList.notCurrent.nonMerge #parentNotCurrentMessage {
         --arrow-color: #ffa62f;
         display: inline-block;
@@ -137,13 +151,40 @@
         <span class="title">Owner</span>
         <span class="value">
           <gr-account-link account="[[change.owner]]"></gr-account-link>
+          <template is="dom-if" if="[[_pushCertificateValidation]]">
+            <gr-tooltip-content
+                has-tooltip
+                title$="[[_pushCertificateValidation.message]]">
+              <iron-icon
+                  class$="icon [[_pushCertificateValidation.class]]"
+                  icon="[[_pushCertificateValidation.icon]]">
+              </iron-icon>
+            </gr-tooltip-content>
+          </template>
         </span>
       </section>
-      <section class$="[[_computeShowUploaderHide(change)]]">
+      <section class$="[[_computeShowRoleClass(change, _CHANGE_ROLE.UPLOADER)]]">
         <span class="title">Uploader</span>
         <span class="value">
           <gr-account-link
-              account="[[_computeShowUploader(change)]]"></gr-account-link>
+              account="[[_getNonOwnerRole(change, _CHANGE_ROLE.UPLOADER)]]"
+              ></gr-account-link>
+        </span>
+      </section>
+      <section class$="[[_computeShowRoleClass(change, _CHANGE_ROLE.AUTHOR)]]">
+        <span class="title">Author</span>
+        <span class="value">
+          <gr-account-link
+              account="[[_getNonOwnerRole(change, _CHANGE_ROLE.AUTHOR)]]"
+              ></gr-account-link>
+        </span>
+      </section>
+      <section class$="[[_computeShowRoleClass(change, _CHANGE_ROLE.COMMITTER)]]">
+        <span class="title">Committer</span>
+        <span class="value">
+          <gr-account-link
+              account="[[_getNonOwnerRole(change, _CHANGE_ROLE.COMMITTER)]]"
+              ></gr-account-link>
         </span>
       </section>
       <section class="assignee">
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
index 8d1546b..d3fc7e0 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -17,6 +17,17 @@
 (function() {
   'use strict';
 
+  const Defs = {};
+
+  /**
+   * @typedef {{
+   *    message: string,
+   *    icon: string,
+   *    class: string,
+   *  }}
+   */
+  Defs.PushCertificateValidation;
+
   const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
 
   const SubmitTypeLabel = {
@@ -30,6 +41,24 @@
 
   const NOT_CURRENT_MESSAGE = 'Not current - rebase possible';
 
+  /**
+   * @enum {string}
+   */
+  const CertificateStatus = {
+    /**
+     * This certificate status is bad.
+     */
+    BAD: 'BAD',
+    /**
+     * This certificate status is OK.
+     */
+    OK: 'OK',
+    /**
+     * This certificate status is TRUSTED.
+     */
+    TRUSTED: 'TRUSTED',
+  };
+
   Polymer({
     is: 'gr-change-metadata',
 
@@ -76,6 +105,13 @@
         type: Boolean,
         computed: '_computeShowReviewersByState(serverConfig)',
       },
+      /**
+       * @type {Defs.PushCertificateValidation}
+       */
+      _pushCertificateValidation: {
+        type: Object,
+        computed: '_computePushCertificateValidation(serverConfig, change)',
+      },
       _showRequirements: {
         type: Boolean,
         computed: '_computeShowRequirements(change)',
@@ -97,6 +133,18 @@
         type: Array,
         computed: '_computeParents(change)',
       },
+
+      /** @type {?} */
+      _CHANGE_ROLE: {
+        type: Object,
+        readOnly: true,
+        value: {
+          OWNER: 'owner',
+          UPLOADER: 'uploader',
+          AUTHOR: 'author',
+          COMMITTER: 'committer',
+        },
+      },
     },
 
     behaviors: [
@@ -248,6 +296,59 @@
       return hasRequirements || hasLabels || !!change.work_in_progress;
     },
 
+    /**
+     * @return {?Defs.PushCertificateValidation} object representing data for
+     *     the push validation.
+     */
+    _computePushCertificateValidation(serverConfig, change) {
+      if (!serverConfig || !serverConfig.receive ||
+          !serverConfig.receive.enable_signed_push) {
+        return null;
+      }
+      const rev = change.revisions[change.current_revision];
+      if (!rev.push_certificate || !rev.push_certificate.key) {
+        return {
+          class: 'help',
+          icon: 'gr-icons:help',
+          message: 'This patch set was created without a push certificate',
+        };
+      }
+
+      const key = rev.push_certificate.key;
+      switch (key.status) {
+        case CertificateStatus.BAD:
+          return {
+            class: 'invalid',
+            icon: 'gr-icons:close',
+            message: this._problems('Push certificate is invalid', key),
+          };
+        case CertificateStatus.OK:
+          return {
+            class: 'notTrusted',
+            icon: 'gr-icons:info',
+            message: this._problems(
+                'Push certificate is valid, but key is not trusted', key),
+          };
+        case CertificateStatus.TRUSTED:
+          return {
+            class: 'trusted',
+            icon: 'gr-icons:check',
+            message: this._problems(
+                'Push certificate is valid and key is trusted', key),
+          };
+        default:
+          throw new Error(`unknown certificate status: ${key.status}`);
+      }
+    },
+
+    _problems(msg, key) {
+      if (!key || !key.problems || key.problems.length === 0) {
+        return msg;
+      }
+
+      return [msg + ':'].concat(key.problems).join('\n');
+    },
+
     _computeProjectURL(project) {
       return Gerrit.Nav.getUrlForProjectChanges(project);
     },
@@ -299,24 +400,45 @@
       return !!change.work_in_progress;
     },
 
-    _computeShowUploaderHide(change) {
-      return this._computeShowUploader(change) ? '' : 'hideDisplay';
+    _computeShowRoleClass(change, role) {
+      return this._getNonOwnerRole(change, role) ? '' : 'hideDisplay';
     },
 
-    _computeShowUploader(change) {
+    /**
+     * Get the user with the specified role on the change. Returns null if the
+     * user with that role is the same as the owner.
+     * @param {!Object} change
+     * @param {string} role One of the values from _CHANGE_ROLE
+     * @return {Object|null} either an accound or null.
+     */
+    _getNonOwnerRole(change, role) {
       if (!change.current_revision ||
           !change.revisions[change.current_revision]) {
         return null;
       }
 
       const rev = change.revisions[change.current_revision];
+      if (!rev) { return null; }
 
-      if (!rev || !rev.uploader ||
-        change.owner._account_id === rev.uploader._account_id) {
-        return null;
+      if (role === this._CHANGE_ROLE.UPLOADER &&
+          rev.uploader &&
+          change.owner._account_id !== rev.uploader._account_id) {
+        return rev.uploader;
       }
 
-      return rev.uploader;
+      if (role === this._CHANGE_ROLE.AUTHOR &&
+          rev.commit && rev.commit.author &&
+          change.owner.email !== rev.commit.author.email) {
+        return rev.commit.author;
+      }
+
+      if (role === this._CHANGE_ROLE.COMMITTER &&
+          rev.commit && rev.commit.committer &&
+          change.owner.email !== rev.commit.committer.email) {
+        return rev.commit.committer;
+      }
+
+      return null;
     },
 
     _computeParents(change) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
index af25d91..c5a569e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
@@ -185,7 +185,138 @@
       assert.equal(element._computeWebLinks(element.commitInfo).length, 1);
     });
 
-    test('_computeShowUploader test for uploader', () => {
+    suite('_getNonOwnerRole', () => {
+      let change;
+
+      setup(() => {
+        change = {
+          owner: {
+            email: 'abc@def',
+            _account_id: 1019328,
+          },
+          revisions: {
+            rev1: {
+              _number: 1,
+              uploader: {
+                email: 'ghi@def',
+                _account_id: 1011123,
+              },
+              commit: {
+                author: {email: 'jkl@def'},
+                committer: {email: 'ghi@def'},
+              },
+            },
+          },
+          current_revision: 'rev1',
+        };
+      });
+
+      suite('role=uploader', () => {
+        test('_getNonOwnerRole for uploader', () => {
+          assert.deepEqual(
+              element._getNonOwnerRole(change, element._CHANGE_ROLE.UPLOADER),
+              {email: 'ghi@def', _account_id: 1011123});
+        });
+
+        test('_getNonOwnerRole that it does not return uploader', () => {
+          // Set the uploader email to be the same as the owner.
+          change.revisions.rev1.uploader._account_id = 1019328;
+          assert.isNull(element._getNonOwnerRole(change,
+              element._CHANGE_ROLE.UPLOADER));
+        });
+
+        test('_getNonOwnerRole null for uploader with no current rev', () => {
+          delete change.current_revision;
+          assert.isNull(element._getNonOwnerRole(change,
+              element._CHANGE_ROLE.UPLOADER));
+        });
+
+        test('_computeShowRoleClass show uploader', () => {
+          assert.equal(element._computeShowRoleClass(
+              change, element._CHANGE_ROLE.UPLOADER), '');
+        });
+
+        test('_computeShowRoleClass hide uploader', () => {
+          // Set the uploader email to be the same as the owner.
+          change.revisions.rev1.uploader._account_id = 1019328;
+          assert.equal(element._computeShowRoleClass(change,
+              element._CHANGE_ROLE.UPLOADER), 'hideDisplay');
+        });
+      });
+
+      suite('role=committer', () => {
+        test('_getNonOwnerRole for committer', () => {
+          assert.deepEqual(
+              element._getNonOwnerRole(change, element._CHANGE_ROLE.COMMITTER),
+              {email: 'ghi@def'});
+        });
+
+        test('_getNonOwnerRole that it does not return committer', () => {
+          // Set the committer email to be the same as the owner.
+          change.revisions.rev1.commit.committer.email = 'abc@def';
+          assert.isNull(element._getNonOwnerRole(change,
+              element._CHANGE_ROLE.COMMITTER));
+        });
+
+        test('_getNonOwnerRole null for committer with no current rev', () => {
+          delete change.current_revision;
+          assert.isNull(element._getNonOwnerRole(change,
+              element._CHANGE_ROLE.COMMITTER));
+        });
+
+        test('_getNonOwnerRole null for committer with no commit', () => {
+          delete change.revisions.rev1.commit;
+          assert.isNull(element._getNonOwnerRole(change,
+              element._CHANGE_ROLE.COMMITTER));
+        });
+
+        test('_getNonOwnerRole null for committer with no committer', () => {
+          delete change.revisions.rev1.commit.committer;
+          assert.isNull(element._getNonOwnerRole(change,
+              element._CHANGE_ROLE.COMMITTER));
+        });
+      });
+
+      suite('role=author', () => {
+        test('_getNonOwnerRole for author', () => {
+          assert.deepEqual(
+              element._getNonOwnerRole(change, element._CHANGE_ROLE.AUTHOR),
+              {email: 'jkl@def'});
+        });
+
+        test('_getNonOwnerRole that it does not return author', () => {
+          // Set the author email to be the same as the owner.
+          change.revisions.rev1.commit.author.email = 'abc@def';
+          assert.isNull(element._getNonOwnerRole(change,
+              element._CHANGE_ROLE.AUTHOR));
+        });
+
+        test('_getNonOwnerRole null for author with no current rev', () => {
+          delete change.current_revision;
+          assert.isNull(element._getNonOwnerRole(change,
+              element._CHANGE_ROLE.AUTHOR));
+        });
+
+        test('_getNonOwnerRole null for author with no commit', () => {
+          delete change.revisions.rev1.commit;
+          assert.isNull(element._getNonOwnerRole(change,
+              element._CHANGE_ROLE.AUTHOR));
+        });
+
+        test('_getNonOwnerRole null for author with no author', () => {
+          delete change.revisions.rev1.commit.author;
+          assert.isNull(element._getNonOwnerRole(change,
+              element._CHANGE_ROLE.AUTHOR));
+        });
+      });
+    });
+
+    test('Push Certificate Validation test BAD', () => {
+      const serverConfig = {
+        receive: {
+          enable_signed_push: true,
+        },
+      };
       const change = {
         change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
         owner: {
@@ -194,8 +325,13 @@
         revisions: {
           rev1: {
             _number: 1,
-            uploader: {
-              _account_id: 1011123,
+            push_certificate: {
+              key: {
+                status: 'BAD',
+                problems: [
+                  'No public keys found for key ID E5E20E52',
+                ],
+              },
             },
           },
         },
@@ -204,54 +340,21 @@
         labels: {},
         mergeable: true,
       };
-      assert.deepEqual(element._computeShowUploader(change),
-          {_account_id: 1011123});
+      const result =
+          element._computePushCertificateValidation(serverConfig, change);
+      assert.equal(result.message,
+          'Push certificate is invalid:\n' +
+          'No public keys found for key ID E5E20E52');
+      assert.equal(result.icon, 'gr-icons:close');
+      assert.equal(result.class, 'invalid');
     });
 
-    test('_computeShowUploader test that it does not return uploader', () => {
-      const change = {
-        change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
-        owner: {
-          _account_id: 1011123,
+    test('Push Certificate Validation test TRUSTED', () => {
+      const serverConfig = {
+        receive: {
+          enable_signed_push: true,
         },
-        revisions: {
-          rev1: {
-            _number: 1,
-            uploader: {
-              _account_id: 1011123,
-            },
-          },
-        },
-        current_revision: 'rev1',
-        status: 'NEW',
-        labels: {},
-        mergeable: true,
       };
-      assert.isNotOk(element._computeShowUploader(change));
-    });
-
-    test('no current_revision makes _computeShowUploader return null', () => {
-      const change = {
-        change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
-        owner: {
-          _account_id: 1011123,
-        },
-        revisions: {
-          rev1: {
-            _number: 1,
-            uploader: {
-              _account_id: 1011123,
-            },
-          },
-        },
-        status: 'NEW',
-        labels: {},
-        mergeable: true,
-      };
-      assert.isNotOk(element._computeShowUploader(change));
-    });
-
-    test('_computeShowUploaderHide test for string which equals true', () => {
       const change = {
         change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
         owner: {
@@ -260,8 +363,10 @@
         revisions: {
           rev1: {
             _number: 1,
-            uploader: {
-              _account_id: 1011123,
+            push_certificate: {
+              key: {
+                status: 'TRUSTED',
+              },
             },
           },
         },
@@ -270,21 +375,28 @@
         labels: {},
         mergeable: true,
       };
-      assert.equal(element._computeShowUploaderHide(change), '');
+      const result =
+          element._computePushCertificateValidation(serverConfig, change);
+      assert.equal(result.message,
+          'Push certificate is valid and key is trusted');
+      assert.equal(result.icon, 'gr-icons:check');
+      assert.equal(result.class, 'trusted');
     });
 
-    test('_computeShowUploaderHide test for hideDisplay', () => {
+    test('Push Certificate Validation is missing test', () => {
+      const serverConfig = {
+        receive: {
+          enable_signed_push: true,
+        },
+      };
       const change = {
         change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
         owner: {
-          _account_id: 1011123,
+          _account_id: 1019328,
         },
         revisions: {
           rev1: {
             _number: 1,
-            uploader: {
-              _account_id: 1011123,
-            },
           },
         },
         current_revision: 'rev1',
@@ -292,8 +404,12 @@
         labels: {},
         mergeable: true,
       };
-      assert.equal(
-          element._computeShowUploaderHide(change), 'hideDisplay');
+      const result =
+          element._computePushCertificateValidation(serverConfig, change);
+      assert.equal(result.message,
+          'This patch set was created without a push certificate');
+      assert.equal(result.icon, 'gr-icons:help');
+      assert.equal(result.class, 'help');
     });
 
     test('_computeParents', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 4296bd4..f6acef6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -575,6 +575,7 @@
       };
       element._change = {
         change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
+        owner: {email: 'abc@def'},
         revisions: {
           rev2: {_number: 2, commit: {parents: []}},
           rev1: {_number: 1, commit: {parents: []}},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 6f361d8..cf8417a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -161,6 +161,10 @@
         type: Object,
         computed: '_getRevisionInfo(_change)',
       },
+      _reviewedFiles: {
+        type: Object,
+        value: () => new Set(),
+      },
     },
 
     behaviors: [
@@ -207,6 +211,7 @@
         [this.Shortcut.TOGGLE_DIFF_MODE]: '_handleToggleDiffMode',
         [this.Shortcut.TOGGLE_FILE_REVIEWED]: '_handleToggleFileReviewed',
         [this.Shortcut.EXPAND_ALL_DIFF_CONTEXT]: '_handleExpandAllDiffContext',
+        [this.Shortcut.NEXT_UNREVIEWED_FILE]: '_handleNextUnreviewedFile',
 
         // Final two are actually handled by gr-diff-comment-thread.
         [this.Shortcut.EXPAND_ALL_COMMENT_THREADS]: null,
@@ -555,10 +560,18 @@
       return {path: fileList[idx]};
     },
 
+    _getReviewedFiles(changeNum, patchNum) {
+      return this.$.restAPI.getReviewedFiles(changeNum, patchNum)
+          .then(files => {
+            this._reviewedFiles = new Set(files);
+            return this._reviewedFiles;
+          });
+    },
+
     _getReviewedStatus(editMode, changeNum, patchNum, path) {
       if (editMode) { return Promise.resolve(false); }
-      return this.$.restAPI.getReviewedFiles(changeNum, patchNum)
-          .then(files => files.includes(path));
+      return this._getReviewedFiles(changeNum, patchNum)
+          .then(files => files.has(path));
     },
 
     _paramsChanged(value) {
@@ -1012,5 +1025,15 @@
       if (this.shouldSuppressKeyboardShortcut(e)) { return; }
       this.$.diffHost.expandAllContext();
     },
+
+    _handleNextUnreviewedFile(e) {
+      this._setReviewed(true);
+      // Ensure that the currently viewed file always appears in unreviewedFiles
+      // so we resolve the right "next" file.
+      const unreviewedFiles = this._fileList
+          .filter(file =>
+          (file === this._path || !this._reviewedFiles.has(file)));
+      this._navToFile(this._path, unreviewedFiles, 1);
+    },
   });
 })();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index 431578b..958acdb 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -67,6 +67,7 @@
     kb.bindShortcut(kb.Shortcut.EXPAND_ALL_DIFF_CONTEXT, 'shift+x');
     kb.bindShortcut(kb.Shortcut.EXPAND_ALL_COMMENT_THREADS, 'e');
     kb.bindShortcut(kb.Shortcut.COLLAPSE_ALL_COMMENT_THREADS, 'shift+e');
+    kb.bindShortcut(kb.Shortcut.NEXT_UNREVIEWED_FILE, 'shift+m');
 
     let element;
     let sandbox;
@@ -1106,5 +1107,22 @@
       assert.isTrue(setStub.calledOnce);
       assert.isTrue(setStub.calledWith(101, 'test-project'));
     });
+
+    test('shift+m navigates to next unreviewed file', () => {
+      element._fileList = ['file1', 'file2', 'file3'];
+      element._reviewedFiles = new Set(['file1', 'file2']);
+      element._path = 'file1';
+      const reviewedStub = sandbox.stub(element, '_setReviewed');
+      const navStub = sandbox.stub(element, '_navToFile');
+      MockInteractions.pressAndReleaseKeyOn(element, 77, 'shift', 'm');
+      flushAsynchronousOperations();
+
+      assert.isTrue(reviewedStub.lastCall.args[0]);
+      assert.deepEqual(navStub.lastCall.args, [
+        'file1',
+        ['file1', 'file3'],
+        1,
+      ]);
+    });
   });
 </script>
diff --git a/polygerrit-ui/app/elements/gr-app.js b/polygerrit-ui/app/elements/gr-app.js
index 0cf517d..321dc58 100644
--- a/polygerrit-ui/app/elements/gr-app.js
+++ b/polygerrit-ui/app/elements/gr-app.js
@@ -275,6 +275,8 @@
       this.bindShortcut(
           this.Shortcut.TOGGLE_FILE_REVIEWED, 'r');
       this.bindShortcut(
+          this.Shortcut.NEXT_UNREVIEWED_FILE, 'shift+m');
+      this.bindShortcut(
           this.Shortcut.TOGGLE_ALL_INLINE_DIFFS, 'shift+i:keyup');
       this.bindShortcut(
           this.Shortcut.TOGGLE_INLINE_DIFF, 'i:keyup');
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
index 4f80475..4ea8cc7 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
@@ -48,6 +48,10 @@
       <g id="publishEdit"><path d="M5 4v2h14V4H5zm0 10h4v6h6v-6h4l-7-7-7 7z"/></g>
       <!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/editor-icons.html -->
       <g id="delete"><path d="M6 19c0 1.1.9 2 2 2h8c1.1 0 2-.9 2-2V7H6v12zM19 4h-3.5l-1-1h-5l-1 1H5v2h14V4z"/></g>
+      <!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.js -->
+      <g id="help"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 17h-2v-2h2v2zm2.07-7.75l-.9.92C13.45 12.9 13 13.5 13 15h-2v-.5c0-1.1.45-2.1 1.17-2.83l1.24-1.26c.37-.36.59-.86.59-1.41 0-1.1-.9-2-2-2s-2 .9-2 2H8c0-2.21 1.79-4 4-4s4 1.79 4 4c0 .88-.36 1.68-.93 2.25z"></path></g>
+      <!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.js -->
+      <g id="info"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-6h2v6zm0-8h-2V7h2v2z"></path></g>
       <!-- This SVG is a copy from material.io https://material.io/icons/#ic_hourglass_full-->
       <g id="hourglass"><path d="M6 2v6h.01L6 8.01 10 12l-4 4 .01.01H6V22h12v-5.99h-.01L18 16l-4-4 4-3.99-.01-.01H18V2H6z"/><path d="M0 0h24v24H0V0z" fill="none"/></g>
       <!-- This is a custom PolyGerrit SVG -->
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index a736de2..d9b0cbf 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -1321,21 +1321,27 @@
      * @param {function()=} opt_cancelCondition
      */
     getChangeDetail(changeNum, opt_errFn, opt_cancelCondition) {
-      const options = this.listChangesOptionsToHex(
-          this.ListChangesOption.ALL_COMMITS,
-          this.ListChangesOption.ALL_REVISIONS,
-          this.ListChangesOption.CHANGE_ACTIONS,
-          this.ListChangesOption.CURRENT_ACTIONS,
-          this.ListChangesOption.DETAILED_LABELS,
-          this.ListChangesOption.DOWNLOAD_COMMANDS,
-          this.ListChangesOption.MESSAGES,
-          this.ListChangesOption.SUBMITTABLE,
-          this.ListChangesOption.WEB_LINKS,
-          this.ListChangesOption.SKIP_MERGEABLE
-      );
-      return this._getChangeDetail(
-          changeNum, options, opt_errFn, opt_cancelCondition)
-          .then(GrReviewerUpdatesParser.parse);
+      const options = [
+        this.ListChangesOption.ALL_COMMITS,
+        this.ListChangesOption.ALL_REVISIONS,
+        this.ListChangesOption.CHANGE_ACTIONS,
+        this.ListChangesOption.CURRENT_ACTIONS,
+        this.ListChangesOption.DETAILED_LABELS,
+        this.ListChangesOption.DOWNLOAD_COMMANDS,
+        this.ListChangesOption.MESSAGES,
+        this.ListChangesOption.SUBMITTABLE,
+        this.ListChangesOption.WEB_LINKS,
+        this.ListChangesOption.SKIP_MERGEABLE,
+      ];
+      return this.getConfig(false).then(config => {
+        if (config.receive && config.receive.enable_signed_push) {
+          options.push(this.ListChangesOption.PUSH_CERTIFICATES);
+        }
+        const optionsHex = this.listChangesOptionsToHex(...options);
+        return this._getChangeDetail(
+            changeNum, optionsHex, opt_errFn, opt_cancelCondition)
+            .then(GrReviewerUpdatesParser.parse);
+      });
     },
 
     /**
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index b2bf42b..667f24c 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -734,15 +734,6 @@
       assert.deepEqual(element._send.lastCall.args[0].body, {token: 'foo'});
     });
 
-    test('GrReviewerUpdatesParser.parse is used', () => {
-      sandbox.stub(GrReviewerUpdatesParser, 'parse').returns(
-          Promise.resolve('foo'));
-      return element.getChangeDetail(42).then(result => {
-        assert.isTrue(GrReviewerUpdatesParser.parse.calledOnce);
-        assert.equal(result, 'foo');
-      });
-    });
-
     test('setAccountStatus', () => {
       sandbox.stub(element, '_send').returns(Promise.resolve('OOO'));
       element._cache.set('/accounts/self/detail', {});
@@ -1114,7 +1105,49 @@
       });
     });
 
-    suite('_getChangeDetail', () => {
+    suite('getChangeDetail', () => {
+      suite('change detail options', () => {
+        let toHexStub;
+
+        setup(() => {
+          toHexStub = sandbox.stub(element, 'listChangesOptionsToHex',
+              options => 'deadbeef');
+          sandbox.stub(element, '_getChangeDetail',
+              async (changeNum, options) => ({changeNum, options}));
+        });
+
+        test('signed pushes disabled', async () => {
+          const {PUSH_CERTIFICATES} = element.ListChangesOption;
+          sandbox.stub(element, 'getConfig', async () => ({}));
+          const {changeNum, options} = await element.getChangeDetail(123);
+          assert.strictEqual(123, changeNum);
+          assert.strictEqual('deadbeef', options);
+          assert.isTrue(toHexStub.calledOnce);
+          assert.isFalse(toHexStub.lastCall.args.includes(PUSH_CERTIFICATES));
+        });
+
+        test('signed pushes enabled', async () => {
+          const {PUSH_CERTIFICATES} = element.ListChangesOption;
+          sandbox.stub(element, 'getConfig', async () => {
+            return {receive: {enable_signed_push: true}};
+          });
+          const {changeNum, options} = await element.getChangeDetail(123);
+          assert.strictEqual(123, changeNum);
+          assert.strictEqual('deadbeef', options);
+          assert.isTrue(toHexStub.calledOnce);
+          assert.isTrue(toHexStub.lastCall.args.includes(PUSH_CERTIFICATES));
+        });
+      });
+
+      test('GrReviewerUpdatesParser.parse is used', () => {
+        sandbox.stub(GrReviewerUpdatesParser, 'parse').returns(
+            Promise.resolve('foo'));
+        return element.getChangeDetail(42).then(result => {
+          assert.isTrue(GrReviewerUpdatesParser.parse.calledOnce);
+          assert.equal(result, 'foo');
+        });
+      });
+
       test('_getChangeDetail passes params to ETags decorator', () => {
         const changeNum = 4321;
         element._projectLookup[changeNum] = 'test';