Merge "InternalChangeQuery: Remove unused methods"
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 9a40b27..185fa07 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -1370,10 +1370,11 @@
 [[capability_createProject]]
 === Create Project
 
-Allow project creation.  This capability allows the granted group to
-either link:cmd-create-project.html[create new git projects via ssh]
-or via the web UI.
+Allow project creation.
 
+This capability allows the granted group to create projects via the web UI, via
+link:rest-api-projects.html#create-project][REST] and via
+link:cmd-create-project.html[SSH].
 
 [[capability_emailReviewers]]
 === Email Reviewers
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 565c491..e12c27c 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -43,6 +43,17 @@
 For more predictable results, use explicit search operators as described
 in the following section.
 
+[IMPORTANT]
+--
+The change search API is backed by a secondary index and might sometimes return
+stale results if the re-indexing operation failed for a change update.
+
+Please also note that changes are not re-indexed if the project configuration
+is updated with newly added or modified
+link:config-submit-requirements.html[submit requirements].
+--
+
+
 [[search-operators]]
 == Search Operators
 
diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java
index 65a81f7..eda6e09 100644
--- a/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/java/com/google/gerrit/server/IdentifiedUser.java
@@ -100,30 +100,30 @@
           enablePeerIPInReflogRecord,
           Providers.of(null),
           state,
-          null);
+          /* realUser= */ null);
     }
 
     public IdentifiedUser create(Account.Id id) {
-      return create(null, id);
+      return create(/* remotePeer= */ null, id);
     }
 
     @VisibleForTesting
     @UsedAt(UsedAt.Project.GOOGLE)
     public IdentifiedUser forTest(Account.Id id, PropertyMap properties) {
-      return runAs(null, id, null, properties);
+      return runAs(/* remotePeer= */ null, id, /* caller= */ null, properties);
     }
 
-    public IdentifiedUser create(SocketAddress remotePeer, Account.Id id) {
-      return runAs(remotePeer, id, null);
+    public IdentifiedUser create(@Nullable SocketAddress remotePeer, Account.Id id) {
+      return runAs(remotePeer, id, /* caller= */ null);
     }
 
     public IdentifiedUser runAs(
-        SocketAddress remotePeer, Account.Id id, @Nullable CurrentUser caller) {
+        @Nullable SocketAddress remotePeer, Account.Id id, @Nullable CurrentUser caller) {
       return runAs(remotePeer, id, caller, PropertyMap.EMPTY);
     }
 
     private IdentifiedUser runAs(
-        SocketAddress remotePeer,
+        @Nullable SocketAddress remotePeer,
         Account.Id id,
         @Nullable CurrentUser caller,
         PropertyMap properties) {
@@ -244,7 +244,7 @@
       AccountCache accountCache,
       GroupBackend groupBackend,
       Boolean enablePeerIPInReflogRecord,
-      @Nullable Provider<SocketAddress> remotePeerProvider,
+      Provider<SocketAddress> remotePeerProvider,
       AccountState state,
       @Nullable CurrentUser realUser) {
     this(
@@ -270,7 +270,7 @@
       AccountCache accountCache,
       GroupBackend groupBackend,
       Boolean enablePeerIPInReflogRecord,
-      @Nullable Provider<SocketAddress> remotePeerProvider,
+      Provider<SocketAddress> remotePeerProvider,
       Account.Id id,
       @Nullable CurrentUser realUser,
       PropertyMap properties) {
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index 389b292..fcfc805 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -211,7 +211,7 @@
         return searchedAsUser.asIdentifiedUser();
       }
       return userFactory.runAs(
-          null, list.get(0).account().id(), requireNonNull(caller).getRealUser());
+          /* remotePeer= */ null, list.get(0).account().id(), requireNonNull(caller).getRealUser());
     }
 
     @VisibleForTesting
diff --git a/java/com/google/gerrit/server/change/RebaseUtil.java b/java/com/google/gerrit/server/change/RebaseUtil.java
index dcbd1ae..8acc925 100644
--- a/java/com/google/gerrit/server/change/RebaseUtil.java
+++ b/java/com/google/gerrit/server/change/RebaseUtil.java
@@ -14,8 +14,6 @@
 
 package com.google.gerrit.server.change;
 
-import static com.google.gerrit.server.project.ProjectCache.illegalState;
-
 import com.google.auto.value.AutoValue;
 import com.google.common.flogger.FluentLogger;
 import com.google.common.primitives.Ints;
@@ -37,7 +35,6 @@
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.inject.Inject;
@@ -70,30 +67,28 @@
     this.rebaseFactory = rebaseFactory;
   }
 
-  public static void verifyRebasePreconditions(
-      ProjectCache projectCache, PatchSetUtil patchSetUtil, RevWalk rw, RevisionResource rsrc)
-      throws ResourceConflictException, IOException, AuthException, PermissionBackendException {
+  /**
+   * Checks whether the given change fulfills all preconditions to be rebased.
+   *
+   * <p>This method does not check whether the calling user is allowed to rebase the change.
+   */
+  public void verifyRebasePreconditions(RevWalk rw, ChangeNotes changeNotes, PatchSet patchSet)
+      throws ResourceConflictException, IOException {
     // Not allowed to rebase if the current patch set is locked.
-    patchSetUtil.checkPatchSetNotLocked(rsrc.getNotes());
+    psUtil.checkPatchSetNotLocked(changeNotes);
 
-    rsrc.permissions().check(ChangePermission.REBASE);
-    projectCache
-        .get(rsrc.getProject())
-        .orElseThrow(illegalState(rsrc.getProject()))
-        .checkStatePermitsWrite();
-
-    if (!rsrc.getChange().isNew()) {
+    Change change = changeNotes.getChange();
+    if (!change.isNew()) {
       throw new ResourceConflictException(
-          String.format(
-              "Change %s is %s", rsrc.getChange().getId(), ChangeUtil.status(rsrc.getChange())));
-    } else if (!hasOneParent(rw, rsrc.getPatchSet())) {
+          String.format("Change %s is %s", change.getId(), ChangeUtil.status(change)));
+    }
+
+    if (!hasOneParent(rw, patchSet)) {
       throw new ResourceConflictException(
           String.format(
               "Error rebasing %s. Cannot rebase %s",
-              rsrc.getChange().getId(),
-              countParents(rw, rsrc.getPatchSet()) > 1
-                  ? "merge commits"
-                  : "commit with no ancestor"));
+              change.getId(),
+              countParents(rw, patchSet) > 1 ? "merge commits" : "commit with no ancestor"));
     }
   }
 
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 0afaa3f..6498d1b 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -76,7 +76,6 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
-import java.util.stream.Stream;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Config;
@@ -301,20 +300,19 @@
   @Override
   public Set<AccountGroup.UUID> guessRelevantGroupUUIDs() {
     try (Timer0.Context ignored = guessRelevantGroupsLatency.start()) {
-      Stream<AccountGroup.UUID> configuredRelevantGroups =
-          Arrays.stream(config.getStringList("groups", /* subsection= */ null, "relevantGroup"))
-              .map(AccountGroup::uuid);
-
-      Stream<AccountGroup.UUID> guessedRelevantGroups =
-          inMemoryProjectCache.asMap().values().stream()
-              .filter(Objects::nonNull)
-              .flatMap(p -> p.getAllGroupUUIDs().stream())
-              // getAllGroupUUIDs shouldn't really return null UUIDs, but harden
-              // against them just in case there is a bug or corner case.
-              .filter(id -> id != null && id.get() != null);
-
       Set<AccountGroup.UUID> relevantGroupUuids =
-          Streams.concat(configuredRelevantGroups, guessedRelevantGroups).collect(toSet());
+          Streams.concat(
+                  Arrays.stream(
+                          config.getStringList("groups", /* subsection= */ null, "relevantGroup"))
+                      .map(AccountGroup::uuid),
+                  all().stream()
+                      .map(n -> inMemoryProjectCache.getIfPresent(n))
+                      .filter(Objects::nonNull)
+                      .flatMap(p -> p.getAllGroupUUIDs().stream())
+                      // getAllGroupUUIDs shouldn't really return null UUIDs, but harden
+                      // against them just in case there is a bug or corner case.
+                      .filter(id -> id != null && id.get() != null))
+              .collect(toSet());
       logger.atFine().log("relevant group UUIDs: %s", relevantGroupUuids);
       return relevantGroupUuids;
     }
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 714516c..ca18ab2 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -551,14 +551,14 @@
 
   @Operator
   public Predicate<ChangeData> mergedBefore(String value) throws QueryParseException {
-    checkFieldAvailable(ChangeField.MERGED_ON_SPEC, OPERATOR_MERGED_BEFORE);
+    checkOperatorAvailable(ChangeField.MERGED_ON_SPEC, OPERATOR_MERGED_BEFORE);
     return new BeforePredicate(
         ChangeField.MERGED_ON_SPEC, ChangeQueryBuilder.OPERATOR_MERGED_BEFORE, value);
   }
 
   @Operator
   public Predicate<ChangeData> mergedAfter(String value) throws QueryParseException {
-    checkFieldAvailable(ChangeField.MERGED_ON_SPEC, OPERATOR_MERGED_AFTER);
+    checkOperatorAvailable(ChangeField.MERGED_ON_SPEC, OPERATOR_MERGED_AFTER);
     return new AfterPredicate(
         ChangeField.MERGED_ON_SPEC, ChangeQueryBuilder.OPERATOR_MERGED_AFTER, value);
   }
@@ -641,7 +641,7 @@
     }
 
     if ("attention".equalsIgnoreCase(value)) {
-      checkFieldAvailable(ChangeField.ATTENTION_SET_USERS, "has:attention");
+      checkOperatorAvailable(ChangeField.ATTENTION_SET_USERS, "has:attention");
       return new IsAttentionPredicate();
     }
 
@@ -684,7 +684,7 @@
     }
 
     if ("uploader".equalsIgnoreCase(value)) {
-      checkFieldAvailable(ChangeField.UPLOADER_SPEC, "is:uploader");
+      checkOperatorAvailable(ChangeField.UPLOADER_SPEC, "is:uploader");
       return ChangePredicates.uploader(self());
     }
 
@@ -707,7 +707,7 @@
     }
 
     if ("merge".equalsIgnoreCase(value)) {
-      checkFieldAvailable(ChangeField.MERGE_SPEC, "is:merge");
+      checkOperatorAvailable(ChangeField.MERGE_SPEC, "is:merge");
       return new BooleanPredicate(ChangeField.MERGE_SPEC);
     }
 
@@ -716,7 +716,7 @@
     }
 
     if ("attention".equalsIgnoreCase(value)) {
-      checkFieldAvailable(ChangeField.ATTENTION_SET_USERS, "is:attention");
+      checkOperatorAvailable(ChangeField.ATTENTION_SET_USERS, "is:attention");
       return new IsAttentionPredicate();
     }
 
@@ -729,7 +729,7 @@
     }
 
     if ("pure-revert".equalsIgnoreCase(value)) {
-      checkFieldAvailable(ChangeField.IS_PURE_REVERT_SPEC, "is:pure-revert");
+      checkOperatorAvailable(ChangeField.IS_PURE_REVERT_SPEC, "is:pure-revert");
       return ChangePredicates.pureRevert("1");
     }
 
@@ -745,12 +745,12 @@
             Predicate.not(new SubmittablePredicate(SubmitRecord.Status.NOT_READY)),
             Predicate.not(new SubmittablePredicate(SubmitRecord.Status.RULE_ERROR)));
       }
-      checkFieldAvailable(ChangeField.IS_SUBMITTABLE_SPEC, "is:submittable");
+      checkOperatorAvailable(ChangeField.IS_SUBMITTABLE_SPEC, "is:submittable");
       return new IsSubmittablePredicate();
     }
 
     if ("started".equalsIgnoreCase(value)) {
-      checkFieldAvailable(ChangeField.STARTED_SPEC, "is:started");
+      checkOperatorAvailable(ChangeField.STARTED_SPEC, "is:started");
       return new BooleanPredicate(ChangeField.STARTED_SPEC);
     }
 
@@ -759,7 +759,7 @@
     }
 
     if ("cherrypick".equalsIgnoreCase(value)) {
-      checkFieldAvailable(ChangeField.CHERRY_PICK_SPEC, "is:cherrypick");
+      checkOperatorAvailable(ChangeField.CHERRY_PICK_SPEC, "is:cherrypick");
       return new BooleanPredicate(ChangeField.CHERRY_PICK_SPEC);
     }
 
@@ -894,7 +894,7 @@
       return ChangePredicates.hashtag(hashtag);
     }
 
-    checkFieldAvailable(ChangeField.FUZZY_HASHTAG, "inhashtag");
+    checkOperatorAvailable(ChangeField.FUZZY_HASHTAG, "inhashtag");
     return ChangePredicates.fuzzyHashtag(hashtag);
   }
 
@@ -904,7 +904,7 @@
       return ChangePredicates.hashtag(hashtag);
     }
 
-    checkFieldAvailable(ChangeField.PREFIX_HASHTAG, "prefixhashtag");
+    checkOperatorAvailable(ChangeField.PREFIX_HASHTAG, "prefixhashtag");
     return ChangePredicates.prefixHashtag(hashtag);
   }
 
@@ -930,7 +930,7 @@
       return ChangePredicates.exactTopic(name);
     }
 
-    checkFieldAvailable(ChangeField.PREFIX_TOPIC, "prefixtopic");
+    checkOperatorAvailable(ChangeField.PREFIX_TOPIC, "prefixtopic");
     return ChangePredicates.prefixTopic(name);
   }
 
@@ -996,7 +996,7 @@
 
   @Operator
   public Predicate<ChangeData> hasfooter(String footerName) throws QueryParseException {
-    checkFieldAvailable(ChangeField.FOOTER_NAME, "hasfooter");
+    checkOperatorAvailable(ChangeField.FOOTER_NAME, "hasfooter");
     return ChangePredicates.hasFooter(footerName);
   }
 
@@ -1147,7 +1147,9 @@
   @Operator
   public Predicate<ChangeData> message(String text) throws QueryParseException {
     if (text.startsWith("^")) {
-      checkFieldAvailable(ChangeField.COMMIT_MESSAGE_EXACT, "messageexact");
+      checkFieldAvailable(
+          ChangeField.COMMIT_MESSAGE_EXACT,
+          "'message' operator with regular expression is not supported on this gerrit host");
       return new RegexMessagePredicate(text);
     }
     return ChangePredicates.message(text);
@@ -1155,13 +1157,14 @@
 
   @Operator
   public Predicate<ChangeData> subject(String value) throws QueryParseException {
-    checkFieldAvailable(ChangeField.SUBJECT_SPEC, ChangeQueryBuilder.FIELD_SUBJECT);
+    checkOperatorAvailable(ChangeField.SUBJECT_SPEC, ChangeQueryBuilder.FIELD_SUBJECT);
     return ChangePredicates.subject(value);
   }
 
   @Operator
   public Predicate<ChangeData> prefixsubject(String value) throws QueryParseException {
-    checkFieldAvailable(ChangeField.PREFIX_SUBJECT_SPEC, ChangeQueryBuilder.FIELD_PREFIX_SUBJECT);
+    checkOperatorAvailable(
+        ChangeField.PREFIX_SUBJECT_SPEC, ChangeQueryBuilder.FIELD_PREFIX_SUBJECT);
     return ChangePredicates.prefixSubject(value);
   }
 
@@ -1249,7 +1252,7 @@
   @Operator
   public Predicate<ChangeData> uploader(String who)
       throws QueryParseException, IOException, ConfigInvalidException {
-    checkFieldAvailable(ChangeField.UPLOADER_SPEC, "uploader");
+    checkOperatorAvailable(ChangeField.UPLOADER_SPEC, "uploader");
     return uploader(parseAccount(who, (AccountState s) -> true));
   }
 
@@ -1264,7 +1267,7 @@
   @Operator
   public Predicate<ChangeData> attention(String who)
       throws QueryParseException, IOException, ConfigInvalidException {
-    checkFieldAvailable(ChangeField.ATTENTION_SET_USERS, "attention");
+    checkOperatorAvailable(ChangeField.ATTENTION_SET_USERS, "attention");
     return attention(parseAccount(who, (AccountState s) -> true));
   }
 
@@ -1309,7 +1312,7 @@
 
   @Operator
   public Predicate<ChangeData> uploaderin(String group) throws QueryParseException, IOException {
-    checkFieldAvailable(ChangeField.UPLOADER_SPEC, "uploaderin");
+    checkOperatorAvailable(ChangeField.UPLOADER_SPEC, "uploaderin");
 
     GroupReference g = GroupBackends.findBestSuggestion(args.groupBackend, group);
     if (g == null) {
@@ -1575,8 +1578,8 @@
 
   @Operator
   public Predicate<ChangeData> cherryPickOf(String value) throws QueryParseException {
-    checkFieldAvailable(ChangeField.CHERRY_PICK_OF_CHANGE, "cherryPickOf");
-    checkFieldAvailable(ChangeField.CHERRY_PICK_OF_PATCHSET, "cherryPickOf");
+    checkOperatorAvailable(ChangeField.CHERRY_PICK_OF_CHANGE, "cherryPickOf");
+    checkOperatorAvailable(ChangeField.CHERRY_PICK_OF_PATCHSET, "cherryPickOf");
     if (Ints.tryParse(value) != null) {
       return ChangePredicates.cherryPickOf(Change.id(Ints.tryParse(value)));
     }
@@ -1648,11 +1651,16 @@
     return Predicate.or(predicates);
   }
 
-  protected void checkFieldAvailable(SchemaField<ChangeData, ?> field, String operator)
+  private void checkOperatorAvailable(SchemaField<ChangeData, ?> field, String operator)
+      throws QueryParseException {
+    checkFieldAvailable(
+        field, String.format("'%s' operator is not supported on this gerrit host", operator));
+  }
+
+  protected void checkFieldAvailable(SchemaField<ChangeData, ?> field, String errorMessage)
       throws QueryParseException {
     if (!args.index.getSchema().hasField(field)) {
-      throw new QueryParseException(
-          String.format("'%s' operator is not supported on this gerrit host", operator));
+      throw new QueryParseException(errorMessage);
     }
   }
 
diff --git a/java/com/google/gerrit/server/restapi/change/Rebase.java b/java/com/google/gerrit/server/restapi/change/Rebase.java
index 1a8f07a..8a8d2ca 100644
--- a/java/com/google/gerrit/server/restapi/change/Rebase.java
+++ b/java/com/google/gerrit/server/restapi/change/Rebase.java
@@ -87,6 +87,13 @@
   @Override
   public Response<ChangeInfo> apply(RevisionResource rsrc, RebaseInput input)
       throws UpdateException, RestApiException, IOException, PermissionBackendException {
+    rsrc.permissions().check(ChangePermission.REBASE);
+
+    projectCache
+        .get(rsrc.getProject())
+        .orElseThrow(illegalState(rsrc.getProject()))
+        .checkStatePermitsWrite();
+
     Change change = rsrc.getChange();
     try (Repository repo = repoManager.openRepository(change.getProject());
         ObjectInserter oi = repo.newObjectInserter();
@@ -94,7 +101,7 @@
         RevWalk rw = CodeReviewCommit.newRevWalk(reader);
         BatchUpdate bu =
             updateFactory.create(change.getProject(), rsrc.getUser(), TimeUtil.now())) {
-      RebaseUtil.verifyRebasePreconditions(projectCache, patchSetUtil, rw, rsrc);
+      rebaseUtil.verifyRebasePreconditions(rw, rsrc.getNotes(), rsrc.getPatchSet());
 
       RebaseChangeOp rebaseOp =
           rebaseUtil.getRebaseOp(
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseChain.java b/java/com/google/gerrit/server/restapi/change/RebaseChain.java
index 4754c69..786bba7 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseChain.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseChain.java
@@ -113,7 +113,11 @@
   @Override
   public Response<RebaseChainInfo> apply(ChangeResource tipRsrc, RebaseInput input)
       throws IOException, PermissionBackendException, RestApiException, UpdateException {
+    tipRsrc.permissions().check(ChangePermission.REBASE);
+
     Project.NameKey project = tipRsrc.getProject();
+    projectCache.get(project).orElseThrow(illegalState(project)).checkStatePermitsWrite();
+
     CurrentUser user = tipRsrc.getUser();
 
     List<Change.Id> upToDateAncestors = new ArrayList<>();
@@ -136,7 +140,8 @@
 
         RevisionResource revRsrc =
             new RevisionResource(changeResourceFactory.create(changeData, user), ps);
-        RebaseUtil.verifyRebasePreconditions(projectCache, patchSetUtil, rw, revRsrc);
+        revRsrc.permissions().check(ChangePermission.REBASE);
+        rebaseUtil.verifyRebasePreconditions(rw, changeData.notes(), ps);
 
         boolean isUpToDate = false;
         RebaseChangeOp rebaseOp = null;
diff --git a/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java b/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
index 491b5cd..f3c741f 100644
--- a/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
+++ b/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
@@ -37,6 +37,7 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
+import java.util.stream.Collectors;
 import javax.inject.Inject;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.Config;
@@ -216,12 +217,19 @@
                   Arrays.asList(
                       cfg.getStringList(ProjectConfig.LABEL, labelName, ProjectConfig.KEY_VALUE)))
               .build();
+      ImmutableList<String> refPatterns =
+          ImmutableList.<String>builder()
+              .addAll(
+                  Arrays.asList(
+                      cfg.getStringList(ProjectConfig.LABEL, labelName, ProjectConfig.KEY_BRANCH)))
+              .build();
       LabelAttributes attributes =
           LabelAttributes.create(
               function == null ? "MaxWithBlock" : function,
               canOverride,
               ignoreSelfApproval,
-              values);
+              values,
+              refPatterns);
       labelTypes.put(labelName, attributes);
     }
     return labelTypes;
@@ -320,6 +328,15 @@
       default:
         break;
     }
+    if (!attributes.refPatterns().isEmpty()) {
+      builder.setApplicabilityExpression(
+          SubmitRequirementExpression.of(
+              String.join(
+                  " OR ",
+                  attributes.refPatterns().stream()
+                      .map(b -> "branch:\\\"" + b + "\\\"")
+                      .collect(Collectors.toList()))));
+    }
     return builder.build();
   }
 
@@ -435,13 +452,16 @@
 
     abstract ImmutableList<String> values();
 
+    abstract ImmutableList<String> refPatterns();
+
     static LabelAttributes create(
         String function,
         boolean canOverride,
         boolean ignoreSelfApproval,
-        ImmutableList<String> values) {
+        ImmutableList<String> values,
+        ImmutableList<String> refPatterns) {
       return new AutoValue_MigrateLabelFunctionsToSubmitRequirement_LabelAttributes(
-          function, canOverride, ignoreSelfApproval, values);
+          function, canOverride, ignoreSelfApproval, values, refPatterns);
     }
   }
 }
diff --git a/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
index 74bfe0f..9d37497 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
@@ -17,6 +17,7 @@
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -260,6 +261,96 @@
   }
 
   @Test
+  public void migrateBlockingLabel_withBranchAttribute() throws Exception {
+    createLabelWithBranch(
+        "Foo",
+        "MaxWithBlock",
+        /* ignoreSelfApproval= */ false,
+        ImmutableList.of("refs/heads/master"));
+
+    assertNonExistentSr(/* srName = */ "Foo");
+
+    TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+    assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+    assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+    assertExistentSr(
+        /* srName */ "Foo",
+        /* applicabilityExpression= */ "branch:\\\"refs/heads/master\\\"",
+        /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+        /* canOverride= */ true);
+    assertLabelFunction("Foo", "NoBlock");
+  }
+
+  @Test
+  public void migrateBlockingLabel_withMultipleBranchAttributes() throws Exception {
+    createLabelWithBranch(
+        "Foo",
+        "MaxWithBlock",
+        /* ignoreSelfApproval= */ false,
+        ImmutableList.of("refs/heads/master", "refs/heads/develop"));
+
+    assertNonExistentSr(/* srName = */ "Foo");
+
+    TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+    assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+    assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+    assertExistentSr(
+        /* srName */ "Foo",
+        /* applicabilityExpression= */ "branch:\\\"refs/heads/master\\\" "
+            + "OR branch:\\\"refs/heads/develop\\\"",
+        /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+        /* canOverride= */ true);
+    assertLabelFunction("Foo", "NoBlock");
+  }
+
+  @Test
+  public void migrateBlockingLabel_withRegexBranchAttribute() throws Exception {
+    createLabelWithBranch(
+        "Foo",
+        "MaxWithBlock",
+        /* ignoreSelfApproval= */ false,
+        ImmutableList.of("^refs/heads/main-.*"));
+
+    assertNonExistentSr(/* srName = */ "Foo");
+
+    TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+    assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+    assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+    assertExistentSr(
+        /* srName */ "Foo",
+        /* applicabilityExpression= */ "branch:\\\"^refs/heads/main-.*\\\"",
+        /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+        /* canOverride= */ true);
+    assertLabelFunction("Foo", "NoBlock");
+  }
+
+  @Test
+  public void migrateBlockingLabel_withRegexAndNonRegexBranchAttributes() throws Exception {
+    createLabelWithBranch(
+        "Foo",
+        "MaxWithBlock",
+        /* ignoreSelfApproval= */ false,
+        ImmutableList.of("refs/heads/master", "^refs/heads/main-.*"));
+
+    assertNonExistentSr(/* srName = */ "Foo");
+
+    TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+    assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+    assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+    assertExistentSr(
+        /* srName */ "Foo",
+        /* applicabilityExpression= */ "branch:\\\"refs/heads/master\\\" "
+            + "OR branch:\\\"^refs/heads/main-.*\\\"",
+        /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+        /* canOverride= */ true);
+    assertLabelFunction("Foo", "NoBlock");
+  }
+
+  @Test
   public void migrationIsIdempotent() throws Exception {
     String oldRefsConfigId;
     try (Repository repo = repoManager.openRepository(project)) {
@@ -381,6 +472,21 @@
     gApi.projects().name(project.get()).label(labelName).create(input);
   }
 
+  private void createLabelWithBranch(
+      String labelName,
+      String function,
+      boolean ignoreSelfApproval,
+      ImmutableList<String> refPatterns)
+      throws Exception {
+    LabelDefinitionInput input = new LabelDefinitionInput();
+    input.name = labelName;
+    input.function = function;
+    input.ignoreSelfApproval = ignoreSelfApproval;
+    input.values = ImmutableMap.of("+1", "Looks Good", " 0", "Don't Know", "-1", "Looks Bad");
+    input.branches = refPatterns;
+    gApi.projects().name(project.get()).label(labelName).create(input);
+  }
+
   @CanIgnoreReturnValue
   private SubmitRequirementApi createSubmitRequirement(
       String name, String submitExpression, boolean canOverride) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 2123ac2..15baa78 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -1262,7 +1262,7 @@
                 + c
                 + "/comment/"
                 + ps1List.get(0).id
-                + " \n"
+                + " :\n"
                 + "PS1, Line 1: initial\n"
                 + "what happened to this?\n"
                 + "\n"
@@ -1274,7 +1274,7 @@
                 + c
                 + "/comment/"
                 + ps1List.get(1).id
-                + " \n"
+                + " :\n"
                 + "PS1, Line 1: boring\n"
                 + "Is it that bad?\n"
                 + "\n"
@@ -1288,7 +1288,7 @@
                 + c
                 + "/comment/"
                 + ps2List.get(0).id
-                + " \n"
+                + " :\n"
                 + "PS2, Line 1: initial content\n"
                 + "comment 1 on base\n"
                 + "\n"
@@ -1300,7 +1300,7 @@
                 + c
                 + "/comment/"
                 + ps2List.get(1).id
-                + " \n"
+                + " :\n"
                 + "PS2, Line 2: \n"
                 + "comment 2 on base\n"
                 + "\n"
@@ -1312,7 +1312,7 @@
                 + c
                 + "/comment/"
                 + ps2List.get(2).id
-                + " \n"
+                + " :\n"
                 + "PS2, Line 1: interesting\n"
                 + "better now\n"
                 + "\n"
@@ -1324,7 +1324,7 @@
                 + c
                 + "/comment/"
                 + ps2List.get(3).id
-                + " \n"
+                + " :\n"
                 + "PS2, Line 2: cntent\n"
                 + "typo: content\n"
                 + "\n"
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 61b5e55..9cd002e 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -825,7 +825,8 @@
         ImmutableList.of(", ", ":\"", ",", "!@#$%^\0&*):\" \n: \r\"#$@,. :");
     for (String strangeTag : strangeTags) {
       Change c = newChange();
-      CurrentUser otherUserAsOwner = userFactory.runAs(null, changeOwner.getAccountId(), otherUser);
+      CurrentUser otherUserAsOwner =
+          userFactory.runAs(/* remotePeer= */ null, changeOwner.getAccountId(), otherUser);
       ChangeUpdate update = newUpdate(c, otherUserAsOwner);
       update.putApproval(LabelId.CODE_REVIEW, (short) 2);
       update.setTag(strangeTag);
diff --git a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
index b53de89..25f2f98 100644
--- a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
@@ -354,7 +354,8 @@
   @Test
   public void realUser() throws Exception {
     Change c = newChange();
-    CurrentUser ownerAsOtherUser = userFactory.runAs(null, otherUserId, changeOwner);
+    CurrentUser ownerAsOtherUser =
+        userFactory.runAs(/* remotePeer= */ null, otherUserId, changeOwner);
     ChangeUpdate update = newUpdate(c, ownerAsOtherUser);
     update.setChangeMessage("Message on behalf of other user");
     update.commit();
diff --git a/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java b/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
index 5e6803e..527e78e 100644
--- a/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
@@ -399,7 +399,9 @@
 
     IdentifiedUser impersonatedChangeOwner =
         this.userFactory.runAs(
-            null, changeOwner.getAccountId(), requireNonNull(otherUser).getRealUser());
+            /* remotePeer= */ null,
+            changeOwner.getAccountId(),
+            requireNonNull(otherUser).getRealUser());
     ChangeUpdate impersonatedChangeMessageUpdate = newUpdate(c, impersonatedChangeOwner);
     impersonatedChangeMessageUpdate.setChangeMessage("Other comment on behalf of");
     impersonatedChangeMessageUpdate.commit();
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index ce9ce2d..e3b3ad4 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -201,7 +201,7 @@
   syntax_highlighting?: boolean;
   tab_size: number;
   font_size: number;
-  // TODO: Missing documentation
+  // Hides the FILE and LOST diff rows. Default is TRUE.
   show_file_comment_button?: boolean;
   line_wrapping?: boolean;
 }
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
index 8d6d89a..3599224 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
@@ -25,7 +25,7 @@
   RepoState,
   SubmitType,
 } from '../../../constants/constants';
-import {hasOwnProperty} from '../../../utils/common-util';
+import {assertIsDefined, hasOwnProperty} from '../../../utils/common-util';
 import {firePageError, fireTitleChange} from '../../../utils/event-util';
 import {getAppContext} from '../../../services/app-context';
 import {WebLinkInfo} from '../../../types/diff';
@@ -36,8 +36,9 @@
 import {sharedStyles} from '../../../styles/shared-styles';
 import {BindValueChangeEvent} from '../../../types/events';
 import {deepClone} from '../../../utils/deep-util';
-import {LitElement, PropertyValues, css, html} from 'lit';
+import {LitElement, PropertyValues, css, html, nothing} from 'lit';
 import {customElement, property, state} from 'lit/decorators.js';
+import {when} from 'lit/directives/when.js';
 import {subscribe} from '../../lit/subscription-controller';
 import {createSearchUrl} from '../../../models/views/search';
 import {userModelToken} from '../../../models/user/user-model';
@@ -150,16 +151,6 @@
           color: var(--deemphasized-text-color);
           content: ' *';
         }
-        .loading,
-        .hide {
-          display: none;
-        }
-        #loading.loading {
-          display: block;
-        }
-        #loading:not(.loading) {
-          display: none;
-        }
         #options .repositorySettings {
           display: none;
         }
@@ -187,49 +178,48 @@
             >
           </div>
         </div>
-        <div id="loading" class=${this.loading ? 'loading' : ''}>
-          Loading...
-        </div>
-        <div id="loadedContent" class=${this.loading ? 'loading' : ''}>
-          ${this.renderDownloadCommands()}
-          <h2
-            id="configurations"
-            class="heading-2 ${configChanged ? 'edited' : ''}"
-          >
-            Configurations
-          </h2>
-          <div id="form">
-            <fieldset>
-              ${this.renderDescription()} ${this.renderRepoOptions()}
-              ${this.renderPluginConfig()}
-              <gr-button
-                ?disabled=${this.readOnly || !configChanged}
-                @click=${this.handleSaveRepoConfig}
-                >Save changes</gr-button
-              >
-            </fieldset>
-            <gr-endpoint-decorator name="repo-config">
-              <gr-endpoint-param
-                name="repoName"
-                .value=${this.repo}
-              ></gr-endpoint-param>
-              <gr-endpoint-param
-                name="readOnly"
-                .value=${this.readOnly}
-              ></gr-endpoint-param>
-            </gr-endpoint-decorator>
-          </div>
-        </div>
+        ${when(
+          this.loading || !this.repoConfig,
+          () => html`<div id="loading">Loading...</div>`,
+          () => html`<div id="loadedContent">
+            ${this.renderDownloadCommands()}
+            <h2
+              id="configurations"
+              class="heading-2 ${configChanged ? 'edited' : ''}"
+            >
+              Configurations
+            </h2>
+            <div id="form">
+              <fieldset>
+                ${this.renderDescription()} ${this.renderRepoOptions()}
+                ${this.renderPluginConfig()}
+                <gr-button
+                  ?disabled=${this.readOnly || !configChanged}
+                  @click=${this.handleSaveRepoConfig}
+                  >Save changes</gr-button
+                >
+              </fieldset>
+              <gr-endpoint-decorator name="repo-config">
+                <gr-endpoint-param
+                  name="repoName"
+                  .value=${this.repo}
+                ></gr-endpoint-param>
+                <gr-endpoint-param
+                  name="readOnly"
+                  .value=${this.readOnly}
+                ></gr-endpoint-param>
+              </gr-endpoint-decorator>
+            </div>
+          </div>`
+        )}
       </div>
     `;
   }
 
   private renderDownloadCommands() {
+    if (!this.schemes.length) return nothing;
     return html`
-      <div
-        id="downloadContent"
-        class=${!this.schemes || !this.schemes.length ? 'hide' : ''}
-      >
+      <div id="downloadContent">
         <h2 id="download" class="heading-2">Download</h2>
         <fieldset>
           <gr-download-commands
@@ -252,6 +242,7 @@
   }
 
   private renderDescription() {
+    assertIsDefined(this.repoConfig, 'repoConfig');
     return html`
       <h3 id="Description" class="heading-3">Description</h3>
       <fieldset>
@@ -263,7 +254,7 @@
           rows="4"
           monospace
           ?disabled=${this.readOnly}
-          .text=${this.repoConfig?.description ?? ''}
+          .text=${this.repoConfig.description ?? ''}
           @text-changed=${this.handleDescriptionTextChanged}
         ></gr-textarea>
       </fieldset>
@@ -725,8 +716,9 @@
 
   private renderPluginConfig() {
     const pluginData = this.computePluginData();
+    if (!pluginData.length) return nothing;
     return html` <div
-      class="pluginConfig ${!pluginData || !pluginData.length ? 'hide' : ''}"
+      class="pluginConfig"
       @plugin-config-changed=${this.handlePluginConfigChanged}
     >
       <h3 class="heading-3">Plugins</h3>
@@ -762,6 +754,12 @@
   // private but used in test
   async loadRepo() {
     if (!this.repo) return Promise.resolve();
+    this.repoConfig = undefined;
+    this.originalConfig = undefined;
+    this.loading = true;
+    this.weblinks = [];
+    this.schemesObj = undefined;
+    this.readOnly = true;
 
     const promises = [];
 
@@ -1121,6 +1119,7 @@
 
   private handleDescriptionTextChanged(e: BindValueChangeEvent) {
     if (!this.repoConfig || this.loading) return;
+    if (this.repoConfig.description === e.detail.value) return;
     this.repoConfig = {
       ...this.repoConfig,
       description: e.detail.value,
@@ -1130,6 +1129,7 @@
 
   private handleStateSelectBindValueChanged(e: BindValueChangeEvent) {
     if (!this.repoConfig || this.loading) return;
+    if (this.repoConfig.state === e.detail.value) return;
     this.repoConfig = {
       ...this.repoConfig,
       state: e.detail.value as RepoState,
@@ -1139,6 +1139,7 @@
 
   private handleSubmitTypeSelectBindValueChanged(e: BindValueChangeEvent) {
     if (!this.repoConfig || this.loading) return;
+    if (this.repoConfig.submit_type === e.detail.value) return;
     this.repoConfig = {
       ...this.repoConfig,
       submit_type: e.detail.value as SubmitType,
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts
index c013c9e..4deb99a 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts
@@ -157,14 +157,17 @@
     element = await fixture(html`<gr-repo></gr-repo>`);
   });
 
-  test('render', () => {
+  test('render', async () => {
+    element.repo = REPO as RepoName;
+    await element.loadRepo();
+    await element.updateComplete;
     // prettier and shadowDom assert do not agree about span.title wrapping
     assert.shadowDom.equal(
       element,
       /* prettier-ignore */ /* HTML */ `
       <div class="gr-form-styles main read-only">
         <div class="info">
-          <h1 class="heading-1" id="Title"></h1>
+          <h1 class="heading-1" id="Title">test-repo</h1>
           <hr />
           <div>
             <a href="">
@@ -178,7 +181,7 @@
                 Browse
               </gr-button>
             </a>
-            <a href="">
+            <a href="/q/project:test-repo">
               <gr-button
                 aria-disabled="false"
                 link=""
@@ -190,15 +193,7 @@
             </a>
           </div>
         </div>
-        <div class="loading" id="loading">Loading...</div>
-        <div class="loading" id="loadedContent">
-          <div class="hide" id="downloadContent">
-            <h2 class="heading-2" id="download">Download</h2>
-            <fieldset>
-              <gr-download-commands id="downloadCommands">
-              </gr-download-commands>
-            </fieldset>
-          </div>
+        <div id="loadedContent">
           <h2 class="heading-2" id="configurations">Configurations</h2>
           <div id="form">
             <fieldset>
@@ -266,7 +261,7 @@
                   </span>
                 </section>
                 <section
-                  class="repositorySettings"
+                  class="repositorySettings showConfig"
                   id="enableSignedPushSettings"
                 >
                   <span class="title"> Enable signed push </span>
@@ -277,7 +272,7 @@
                   </span>
                 </section>
                 <section
-                  class="repositorySettings"
+                  class="repositorySettings showConfig"
                   id="requireSignedPushSettings"
                 >
                   <span class="title"> Require signed push </span>
@@ -379,9 +374,6 @@
                   </span>
                 </section>
               </fieldset>
-              <div class="hide pluginConfig">
-                <h3 class="heading-3">Plugins</h3>
-              </div>
               <gr-button
                 aria-disabled="true"
                 disabled=""
@@ -398,7 +390,51 @@
           </div>
         </div>
       </div>
-    `
+    `,
+      {ignoreTags: ['option']}
+    );
+  });
+
+  test('render loading', async () => {
+    element.repo = REPO as RepoName;
+    element.loading = true;
+    await element.updateComplete;
+    // prettier and shadowDom assert do not agree about span.title wrapping
+    assert.shadowDom.equal(
+      element,
+      /* prettier-ignore */ /* HTML */ `
+      <div class="gr-form-styles main read-only">
+        <div class="info">
+          <h1 class="heading-1" id="Title">test-repo</h1>
+          <hr />
+          <div>
+            <a href="">
+              <gr-button
+                aria-disabled="true"
+                disabled=""
+                link=""
+                role="button"
+                tabindex="-1"
+              >
+                Browse
+              </gr-button>
+            </a>
+            <a href="/q/project:test-repo">
+              <gr-button
+                aria-disabled="false"
+                link=""
+                role="button"
+                tabindex="0"
+              >
+                View Changes
+              </gr-button>
+            </a>
+          </div>
+        </div>
+        <div id="loading">Loading...</div>
+      </div>
+    `,
+      {ignoreTags: ['option']}
     );
   });
 
@@ -451,55 +487,22 @@
     assert.isTrue(requestUpdateStub.called);
   });
 
-  test('loading displays before repo config is loaded', () => {
-    assert.isTrue(
-      queryAndAssert<HTMLDivElement>(element, '#loading').classList.contains(
-        'loading'
-      )
-    );
-    assert.isFalse(
-      getComputedStyle(queryAndAssert<HTMLDivElement>(element, '#loading'))
-        .display === 'none'
-    );
-    assert.isTrue(
-      queryAndAssert<HTMLDivElement>(
-        element,
-        '#loadedContent'
-      ).classList.contains('loading')
-    );
-    assert.isTrue(
-      getComputedStyle(
-        queryAndAssert<HTMLDivElement>(element, '#loadedContent')
-      ).display === 'none'
-    );
-  });
-
-  test('download commands visibility', async () => {
-    element.loading = false;
-    await element.updateComplete;
-    assert.isTrue(
-      queryAndAssert<HTMLDivElement>(
-        element,
-        '#downloadContent'
-      ).classList.contains('hide')
-    );
-    assert.isTrue(
-      getComputedStyle(
-        queryAndAssert<HTMLDivElement>(element, '#downloadContent')
-      ).display === 'none'
-    );
+  test('render download commands', async () => {
+    element.repo = REPO as RepoName;
+    await element.loadRepo();
     element.schemesObj = SCHEMES;
     await element.updateComplete;
-    assert.isFalse(
-      queryAndAssert<HTMLDivElement>(
-        element,
-        '#downloadContent'
-      ).classList.contains('hide')
-    );
-    assert.isFalse(
-      getComputedStyle(
-        queryAndAssert<HTMLDivElement>(element, '#downloadContent')
-      ).display === 'none'
+    const content = queryAndAssert<HTMLDivElement>(element, '#downloadContent');
+    assert.dom.equal(
+      content,
+      /* HTML */ `
+        <div id="downloadContent">
+          <h2 class="heading-2" id="download">Download</h2>
+          <fieldset>
+            <gr-download-commands id="downloadCommands"></gr-download-commands>
+          </fieldset>
+        </div>
+      `
     );
   });
 
@@ -715,9 +718,9 @@
         Promise.resolve(new Response())
       );
 
-      const button = queryAll<GrButton>(element, 'gr-button')[2];
-
       await element.loadRepo();
+
+      const button = queryAll<GrButton>(element, 'gr-button')[2];
       assert.isTrue(button.hasAttribute('disabled'));
       assert.isFalse(
         queryAndAssert<HTMLHeadingElement>(
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index c0ed3b3..d4627e2 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -1256,7 +1256,14 @@
         flatten
         down-arrow
         class="showCopyLinkDialogButton"
-        @click=${() => this.copyLinksDropdown?.toggleDropdown()}
+        @click=${(e: MouseEvent) => {
+          // We don't want to handle clicks on the star or the <a> link.
+          // Calling `stopPropagation()` from the click handler of <a> is not an
+          // option, because then the click does not reach the top-level page.js
+          // click handler and would result is a full page reload.
+          if ((e.target as HTMLElement)?.nodeName !== 'GR-BUTTON') return;
+          this.copyLinksDropdown?.toggleDropdown();
+        }}
         ><gr-change-star
           id="changeStar"
           .change=${this.change}
@@ -1267,10 +1274,7 @@
         <a
           class="changeNumber"
           aria-label=${`Change ${this.change?._number}`}
-          @click=${(e: MouseEvent) => {
-            fireReload(this, true);
-            e.stopPropagation();
-          }}
+          href=${ifDefined(this.computeChangeUrl(true))}
           >${this.change?._number}</a
         >
       </gr-button>
@@ -2075,9 +2079,7 @@
 
   // Private but used in tests.
   viewStateChanged() {
-    // viewState is set by gr-router in handleChangeRoute method and is never
-    // set to undefined
-    assertIsDefined(this.viewState, 'viewState');
+    if (!this.viewState) return;
 
     if (this.isChangeObsolete()) {
       // Tell the app element that we are not going to handle the new change
diff --git a/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts b/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts
index 0a21da4..efc6efe 100644
--- a/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts
+++ b/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts
@@ -8,11 +8,21 @@
 import {LitElement, css, html, PropertyValues, nothing} from 'lit';
 import {customElement, property, state} from 'lit/decorators.js';
 import {RunResult} from '../../models/checks/checks-model';
-import {createFixAction, iconFor} from '../../models/checks/checks-util';
+import {
+  createFixAction,
+  createPleaseFixComment,
+  iconFor,
+} from '../../models/checks/checks-util';
 import {modifierPressed} from '../../utils/dom-util';
 import './gr-checks-results';
 import './gr-hovercard-run';
 import {fontStyles} from '../../styles/gr-font-styles';
+import {Action} from '../../api/checks';
+import {assertIsDefined} from '../../utils/common-util';
+import {resolve} from '../../models/dependency';
+import {commentsModelToken} from '../../models/comments/comments-model';
+import {subscribe} from '../lit/subscription-controller';
+import {changeModelToken} from '../../models/change/change-model';
 
 @customElement('gr-diff-check-result')
 export class GrDiffCheckResult extends LitElement {
@@ -32,6 +42,13 @@
   @state()
   isExpandable = false;
 
+  @state()
+  isOwner = false;
+
+  private readonly getChangeModel = resolve(this, changeModelToken);
+
+  private readonly getCommentsModel = resolve(this, commentsModelToken);
+
   static override get styles() {
     return [
       fontStyles,
@@ -114,6 +131,15 @@
     ];
   }
 
+  constructor() {
+    super();
+    subscribe(
+      this,
+      () => this.getChangeModel().isOwner$,
+      x => (this.isOwner = x)
+    );
+  }
+
   override render() {
     if (!this.result) return;
     const cat = this.result.category.toLowerCase();
@@ -182,14 +208,39 @@
 
   private renderActions() {
     if (!this.isExpanded) return nothing;
-    return html`<div class="actions">${this.renderFixButton()}</div>`;
+    return html`<div class="actions">
+      ${this.renderPleaseFixButton()}${this.renderShowFixButton()}
+    </div>`;
   }
 
-  private renderFixButton() {
+  private renderPleaseFixButton() {
+    if (this.isOwner) return nothing;
+    const action: Action = {
+      name: 'Please Fix',
+      callback: () => {
+        assertIsDefined(this.result, 'result');
+        this.getCommentsModel().saveDraft(createPleaseFixComment(this.result));
+        return undefined;
+      },
+    };
+    return html`
+      <gr-checks-action
+        id="please-fix"
+        context="diff-fix"
+        .action=${action}
+      ></gr-checks-action>
+    `;
+  }
+
+  private renderShowFixButton() {
     const action = createFixAction(this, this.result);
     if (!action) return nothing;
     return html`
-      <gr-checks-action context="diff-fix" .action=${action}></gr-checks-action>
+      <gr-checks-action
+        id="show-fix"
+        context="diff-fix"
+        .action=${action}
+      ></gr-checks-action>
     `;
   }
 
diff --git a/polygerrit-ui/app/elements/checks/gr-diff-check-result_test.ts b/polygerrit-ui/app/elements/checks/gr-diff-check-result_test.ts
index 3892c9a..0377e0e 100644
--- a/polygerrit-ui/app/elements/checks/gr-diff-check-result_test.ts
+++ b/polygerrit-ui/app/elements/checks/gr-diff-check-result_test.ts
@@ -7,6 +7,7 @@
 import {fakeRun1} from '../../models/checks/checks-fakes';
 import {RunResult} from '../../models/checks/checks-model';
 import '../../test/common-test-setup';
+import {queryAndAssert} from '../../utils/common-util';
 import './gr-diff-check-result';
 import {GrDiffCheckResult} from './gr-diff-check-result';
 
@@ -50,4 +51,30 @@
     `
     );
   });
+
+  test('renders expanded', async () => {
+    element.result = {...fakeRun1, ...fakeRun1.results?.[2]} as RunResult;
+    element.isExpanded = true;
+    await element.updateComplete;
+
+    const details = queryAndAssert(element, 'div.details');
+    assert.dom.equal(
+      details,
+      /* HTML */ `
+        <div class="details">
+          <gr-result-expanded hidecodepointers=""></gr-result-expanded>
+          <div class="actions">
+            <gr-checks-action
+              id="please-fix"
+              context="diff-fix"
+            ></gr-checks-action>
+            <gr-checks-action
+              id="show-fix"
+              context="diff-fix"
+            ></gr-checks-action>
+          </div>
+        </div>
+      `
+    );
+  });
 });
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index e9da5b6..c74993f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -732,7 +732,7 @@
       changedProperties.has('focusLineNum') ||
       changedProperties.has('leftSide')
     ) {
-      this.initLineOfInterestAndCursor();
+      this.initCursor();
     }
     if (
       changedProperties.has('change') ||
@@ -774,6 +774,7 @@
         .change=${this.change}
         .patchRange=${this.patchRange}
         .file=${file}
+        .lineOfInterest=${this.getLineOfInterest()}
         .path=${this.path}
         .projectName=${this.change?.project}
         @is-blame-loaded-changed=${this.onIsBlameLoadedChanged}
@@ -1350,13 +1351,6 @@
     return {path: fileList[idx]};
   }
 
-  // Private but used in tests.
-  initLineOfInterestAndCursor() {
-    if (!this.diffHost) return;
-    this.diffHost.lineOfInterest = this.getLineOfInterest();
-    this.initCursor();
-  }
-
   private updateUrlToDiffUrl(lineNum?: number, leftSide?: boolean) {
     if (!this.change) return;
     if (!this.patchNum) return;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index 9bbe4b3..889e9dd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -1315,7 +1315,7 @@
     test('hash is determined from viewState', async () => {
       assertIsDefined(element.diffHost);
       sinon.stub(element.diffHost, 'reload');
-      const initLineStub = sinon.stub(element, 'initLineOfInterestAndCursor');
+      const initLineStub = sinon.stub(element, 'initCursor');
 
       element.focusLineNum = 123;
 
@@ -1819,7 +1819,7 @@
 
     test('File change should trigger setUrl once', async () => {
       element.files = getFilesFromFileList(['file1', 'file2', 'file3']);
-      sinon.stub(element, 'initLineOfInterestAndCursor');
+      sinon.stub(element, 'initCursor');
 
       // Load file1
       viewModel.setState({
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
index 28e83ae..b952a3d 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
@@ -76,6 +76,9 @@
 
     const pairs = this.getLinePairs();
     const responsiveMode = getResponsiveMode(this.diffPrefs, this.renderPrefs);
+    const hideFileCommentButton =
+      this.diffPrefs?.show_file_comment_button === false ||
+      this.renderPrefs?.show_file_comment_button === false;
     const body = html`
       <tbody class=${diffClasses(...extras)}>
         ${this.renderContextControls()} ${this.renderMoveControls()}
@@ -92,6 +95,7 @@
               .tabSize=${this.diffPrefs?.tab_size ?? 2}
               .unifiedDiff=${this.isUnifiedDiff()}
               .responsiveMode=${responsiveMode}
+              .hideFileCommentButton=${hideFileCommentButton}
             >
             </gr-diff-row>
           `;
diff --git a/polygerrit-ui/app/models/checks/checks-util.ts b/polygerrit-ui/app/models/checks/checks-util.ts
index 6a5933c..ba43eb4 100644
--- a/polygerrit-ui/app/models/checks/checks-util.ts
+++ b/polygerrit-ui/app/models/checks/checks-util.ts
@@ -14,12 +14,13 @@
   Replacement,
   RunStatus,
 } from '../../api/checks';
-import {PatchSetNumber} from '../../api/rest-api';
+import {PatchSetNumber, RevisionPatchSetNum} from '../../api/rest-api';
+import {CommentSide} from '../../constants/constants';
 import {FixSuggestionInfo, FixReplacementInfo} from '../../types/common';
 import {OpenFixPreviewEventDetail} from '../../types/events';
 import {isDefined} from '../../types/types';
-import {PROVIDED_FIX_ID} from '../../utils/comment-util';
-import {assert, assertNever} from '../../utils/common-util';
+import {PROVIDED_FIX_ID, UnsavedInfo} from '../../utils/comment-util';
+import {assert, assertIsDefined, assertNever} from '../../utils/common-util';
 import {fire} from '../../utils/event-util';
 import {CheckResult, CheckRun, RunResult} from './checks-model';
 
@@ -86,6 +87,27 @@
   }
 }
 
+function pleaseFixMessage(result: RunResult) {
+  return `Please fix this ${result.category} reported by ${result.checkName}: ${result.summary}
+
+${result.message}`;
+}
+
+export function createPleaseFixComment(result: RunResult): UnsavedInfo {
+  const pointer = result.codePointers?.[0];
+  assertIsDefined(pointer, 'codePointer');
+  return {
+    __unsaved: true,
+    path: pointer.path,
+    patch_set: result.patchset as RevisionPatchSetNum,
+    side: CommentSide.REVISION,
+    line: pointer.range.end_line ?? pointer.range.start_line,
+    range: pointer.range,
+    message: pleaseFixMessage(result),
+    unresolved: true,
+  };
+}
+
 export function createFixAction(
   target: EventTarget,
   result?: RunResult
diff --git a/polygerrit-ui/app/utils/attention-set-util.ts b/polygerrit-ui/app/utils/attention-set-util.ts
index 77834bd..4404e59 100644
--- a/polygerrit-ui/app/utils/attention-set-util.ts
+++ b/polygerrit-ui/app/utils/attention-set-util.ts
@@ -3,7 +3,12 @@
  * Copyright 2020 Google LLC
  * SPDX-License-Identifier: Apache-2.0
  */
-import {AccountInfo, ChangeInfo, ServerInfo} from '../types/common';
+import {
+  AccountInfo,
+  ChangeInfo,
+  DetailedLabelInfo,
+  ServerInfo,
+} from '../types/common';
 import {ParsedChangeInfo} from '../types/types';
 import {
   getAccountTemplate,
@@ -13,6 +18,7 @@
 } from './account-util';
 import {CommentThread, isMentionedThread, isUnresolved} from './comment-util';
 import {hasOwnProperty} from './common-util';
+import {getCodeReviewLabel} from './label-util';
 
 export function canHaveAttention(account?: AccountInfo): boolean {
   return !!account?._account_id && !isServiceUser(account);
@@ -101,9 +107,10 @@
 /**
  *  Sort order:
  * 1. The user themselves
- * 2. Human users in the attention set.
- * 3. Other human users.
- * 4. Service users.
+ * 2. Users in the attention set first.
+ * 3. Human users first.
+ * 4. Users that have voted first in this order of vote values:
+ *    -2, -1, +2, +1, 0 or no vote.
  */
 export function sortReviewers(
   r1: AccountInfo,
@@ -117,7 +124,22 @@
   }
   const a1 = hasAttention(r1, change) ? 1 : 0;
   const a2 = hasAttention(r2, change) ? 1 : 0;
-  const s1 = isServiceUser(r1) ? -2 : 0;
-  const s2 = isServiceUser(r2) ? -2 : 0;
-  return a2 - a1 + s2 - s1;
+  if (a2 - a1 !== 0) return a2 - a1;
+
+  const s1 = isServiceUser(r1) ? -1 : 0;
+  const s2 = isServiceUser(r2) ? -1 : 0;
+  if (s2 - s1 !== 0) return s2 - s1;
+
+  const crLabel = getCodeReviewLabel(change?.labels ?? {}) as DetailedLabelInfo;
+  let v1 =
+    crLabel?.all?.find(vote => vote._account_id === r1._account_id)?.value ?? 0;
+  let v2 =
+    crLabel?.all?.find(vote => vote._account_id === r2._account_id)?.value ?? 0;
+  // We want negative votes getting a higher score than positive votes, so
+  // we choose 10 as a random number that is higher than all positive votes that
+  // are in use, and then add the absolute value of the vote to that.
+  // So -2 becomes 12.
+  if (v1 < 0) v1 = 10 - v1;
+  if (v2 < 0) v2 = 10 - v2;
+  return v2 - v1;
 }
diff --git a/polygerrit-ui/app/utils/attention-set-util_test.ts b/polygerrit-ui/app/utils/attention-set-util_test.ts
index 8092a6e..5bd1924 100644
--- a/polygerrit-ui/app/utils/attention-set-util_test.ts
+++ b/polygerrit-ui/app/utils/attention-set-util_test.ts
@@ -6,9 +6,11 @@
 import '../test/common-test-setup';
 import {
   createAccountDetailWithIdNameAndEmail,
+  createAccountWithId,
   createChange,
   createComment,
   createCommentThread,
+  createParsedChange,
   createServerInfo,
 } from '../test/test-data-generators';
 import {
@@ -22,9 +24,10 @@
   getMentionedReason,
   getReason,
   hasAttention,
+  sortReviewers,
 } from './attention-set-util';
 import {DefaultDisplayNameConfig} from '../api/rest-api';
-import {AccountsVisibility} from '../constants/constants';
+import {AccountsVisibility, AccountTag} from '../constants/constants';
 import {assert} from '@open-wc/testing';
 
 const KERMIT: AccountInfo = {
@@ -101,6 +104,45 @@
     assert.equal(getReason(config, OTHER_ACCOUNT, change), 'Added by kermit');
   });
 
+  test('sortReviewers', () => {
+    const a1 = createAccountWithId(1);
+    a1.tags = [AccountTag.SERVICE_USER];
+    const a2 = createAccountWithId(2);
+    a2.tags = [AccountTag.SERVICE_USER];
+    const a3 = createAccountWithId(3);
+    const a4 = createAccountWithId(4);
+    const a5 = createAccountWithId(5);
+    const a6 = createAccountWithId(6);
+    const a7 = createAccountWithId(7);
+
+    const reviewers = [a1, a2, a3, a4, a5, a6, a7];
+    const change = {
+      ...createParsedChange(),
+      attention_set: {'6': {account: a6}},
+      labels: {
+        'Code-Review': {
+          all: [
+            {...a2, value: 1},
+            {...a4, value: 1},
+            {...a5, value: -1},
+          ],
+        },
+      },
+    };
+    assert.sameOrderedMembers(
+      reviewers.sort((r1, r2) => sortReviewers(r1, r2, change, a7)),
+      [
+        a7, // self
+        a6, // is in the attention set
+        a5, // human user, has voted -1
+        a4, // human user, has voted +1
+        a3, // human user, has not voted
+        a2, // service user, has voted
+        a1, // service user, has not voted
+      ]
+    );
+  });
+
   test('getMentionReason', () => {
     let comment = {
       ...createComment(),
diff --git a/resources/com/google/gerrit/server/mail/Comment.soy b/resources/com/google/gerrit/server/mail/Comment.soy
index 98ab4b2..4b621b5 100644
--- a/resources/com/google/gerrit/server/mail/Comment.soy
+++ b/resources/com/google/gerrit/server/mail/Comment.soy
@@ -77,13 +77,8 @@
       {for $line, $index in $comment.lines}
         {if $index == 0}
           {if $comment.startLine != 0}
-            {$comment.link}
+            {$comment.link}{sp}:{\n}
           {/if}
-
-          // Insert a space before the newline so that Gmail does not mistakenly
-          // link the following line with the file link. See issue 9201.
-          {sp}{\n}
-
           {$comment.linePrefix}
         {else}
           {$comment.linePrefixEmpty}