Merge "Refactor SetAccess and CreateAccessChange."
diff --git a/java/com/google/gerrit/server/config/VersionedDefaultPreferences.java b/java/com/google/gerrit/server/config/VersionedDefaultPreferences.java
index bea6dd3..45a9ddf 100644
--- a/java/com/google/gerrit/server/config/VersionedDefaultPreferences.java
+++ b/java/com/google/gerrit/server/config/VersionedDefaultPreferences.java
@@ -16,13 +16,11 @@
import static com.google.common.base.Preconditions.checkState;
-import com.google.common.base.Strings;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.server.git.meta.VersionedMetaData;
+import com.google.gerrit.server.git.meta.VersionedConfigFile;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
@@ -30,11 +28,9 @@
* Low-level storage API to load Gerrit's default config from {@code All-Users}. Should not be used
* directly.
*/
-public class VersionedDefaultPreferences extends VersionedMetaData {
+public class VersionedDefaultPreferences extends VersionedConfigFile {
private static final String PREFERENCES_CONFIG = "preferences.config";
- private Config cfg;
-
public static Config get(Repository allUsersRepo, AllUsersName allUsersName)
throws StorageException, ConfigInvalidException {
VersionedDefaultPreferences versionedDefaultPreferences = new VersionedDefaultPreferences();
@@ -46,27 +42,13 @@
return versionedDefaultPreferences.getConfig();
}
- @Override
- protected String getRefName() {
- return RefNames.REFS_USERS_DEFAULT;
+ public VersionedDefaultPreferences() {
+ super(RefNames.REFS_USERS_DEFAULT, PREFERENCES_CONFIG, "Update default preferences\n");
}
+ @Override
public Config getConfig() {
checkState(cfg != null, "Default preferences not loaded yet.");
return cfg;
}
-
- @Override
- protected void onLoad() throws IOException, ConfigInvalidException {
- cfg = readConfig(PREFERENCES_CONFIG);
- }
-
- @Override
- protected boolean onSave(CommitBuilder commit) throws IOException {
- if (Strings.isNullOrEmpty(commit.getMessage())) {
- commit.setMessage("Update default preferences\n");
- }
- saveConfig(PREFERENCES_CONFIG, cfg);
- return true;
- }
}
diff --git a/java/com/google/gerrit/server/git/meta/VersionedConfigFile.java b/java/com/google/gerrit/server/git/meta/VersionedConfigFile.java
new file mode 100644
index 0000000..de8dabd
--- /dev/null
+++ b/java/com/google/gerrit/server/git/meta/VersionedConfigFile.java
@@ -0,0 +1,79 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git.meta;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.entities.RefNames;
+import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Config;
+
+/**
+ * Versioned configuration file living in git
+ *
+ * <p>This class is a low-level API that allows callers to read the config directly from a
+ * repository and make updates to it.
+ */
+public class VersionedConfigFile extends VersionedMetaData {
+ protected final String ref;
+ protected final String fileName;
+ protected final String defaultOnSaveMessage;
+ protected Config cfg;
+
+ public VersionedConfigFile(String fileName) {
+ this(RefNames.REFS_CONFIG, fileName);
+ }
+
+ public VersionedConfigFile(String ref, String fileName) {
+ this(ref, fileName, "Updated configuration\n");
+ }
+
+ public VersionedConfigFile(String ref, String fileName, String defaultOnSaveMessage) {
+ this.ref = ref;
+ this.fileName = fileName;
+ this.defaultOnSaveMessage = defaultOnSaveMessage;
+ }
+
+ public Config getConfig() {
+ if (cfg == null) {
+ cfg = new Config();
+ }
+ return cfg;
+ }
+
+ protected String getFileName() {
+ return fileName;
+ }
+
+ @Override
+ protected String getRefName() {
+ return ref;
+ }
+
+ @Override
+ protected void onLoad() throws IOException, ConfigInvalidException {
+ cfg = readConfig(fileName);
+ }
+
+ @Override
+ protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException {
+ if (Strings.isNullOrEmpty(commit.getMessage())) {
+ commit.setMessage(defaultOnSaveMessage);
+ }
+ saveConfig(fileName, cfg);
+ return true;
+ }
+}
diff --git a/java/com/google/gerrit/server/project/LabelConfigValidator.java b/java/com/google/gerrit/server/project/LabelConfigValidator.java
index 33e0a87..c4bc9fb 100644
--- a/java/com/google/gerrit/server/project/LabelConfigValidator.java
+++ b/java/com/google/gerrit/server/project/LabelConfigValidator.java
@@ -26,6 +26,7 @@
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.events.CommitReceivedEvent;
+import com.google.gerrit.server.git.meta.VersionedConfigFile;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.CommitValidationMessage;
@@ -352,7 +353,7 @@
private Config loadNewConfig(CommitReceivedEvent receiveEvent)
throws IOException, ConfigInvalidException {
- ProjectLevelConfig.Bare bareConfig = new ProjectLevelConfig.Bare(ProjectConfig.PROJECT_CONFIG);
+ VersionedConfigFile bareConfig = new VersionedConfigFile(ProjectConfig.PROJECT_CONFIG);
bareConfig.load(receiveEvent.project.getNameKey(), receiveEvent.revWalk, receiveEvent.commit);
return bareConfig.getConfig();
}
@@ -364,8 +365,7 @@
}
try {
- ProjectLevelConfig.Bare bareConfig =
- new ProjectLevelConfig.Bare(ProjectConfig.PROJECT_CONFIG);
+ VersionedConfigFile bareConfig = new VersionedConfigFile(ProjectConfig.PROJECT_CONFIG);
bareConfig.load(
receiveEvent.project.getNameKey(),
receiveEvent.revWalk,
diff --git a/java/com/google/gerrit/server/project/ProjectLevelConfig.java b/java/com/google/gerrit/server/project/ProjectLevelConfig.java
index 8256198..7ab7629 100644
--- a/java/com/google/gerrit/server/project/ProjectLevelConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectLevelConfig.java
@@ -18,60 +18,15 @@
import com.google.common.collect.Streams;
import com.google.gerrit.entities.ImmutableConfig;
-import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.server.git.meta.VersionedMetaData;
-import java.io.IOException;
import java.util.Arrays;
import java.util.Set;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import org.eclipse.jgit.annotations.Nullable;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
/** Configuration file in the projects refs/meta/config branch. */
public class ProjectLevelConfig {
- /**
- * This class is a low-level API that allows callers to read the config directly from a repository
- * and make updates to it.
- */
- public static class Bare extends VersionedMetaData {
- private final String fileName;
- @Nullable private Config cfg;
-
- public Bare(String fileName) {
- this.fileName = fileName;
- this.cfg = null;
- }
-
- public Config getConfig() {
- if (cfg == null) {
- cfg = new Config();
- }
- return cfg;
- }
-
- @Override
- protected String getRefName() {
- return RefNames.REFS_CONFIG;
- }
-
- @Override
- protected void onLoad() throws IOException, ConfigInvalidException {
- cfg = readConfig(fileName);
- }
-
- @Override
- protected boolean onSave(CommitBuilder commit) throws IOException {
- if (commit.getMessage() == null || "".equals(commit.getMessage())) {
- commit.setMessage("Updated configuration\n");
- }
- saveConfig(fileName, cfg);
- return true;
- }
- }
-
private final String fileName;
private final ProjectState project;
private final ImmutableConfig immutableConfig;
diff --git a/java/com/google/gerrit/server/schema/MigrateLabelConfigToCopyCondition.java b/java/com/google/gerrit/server/schema/MigrateLabelConfigToCopyCondition.java
index 46a6857..927d3fd1 100644
--- a/java/com/google/gerrit/server/schema/MigrateLabelConfigToCopyCondition.java
+++ b/java/com/google/gerrit/server/schema/MigrateLabelConfigToCopyCondition.java
@@ -29,8 +29,8 @@
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.git.meta.VersionedConfigFile;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.ProjectLevelConfig;
import com.google.inject.Inject;
import java.io.IOException;
import java.util.ArrayList;
@@ -127,8 +127,7 @@
* parsed
*/
public void execute(Project.NameKey projectName) throws IOException, ConfigInvalidException {
- ProjectLevelConfig.Bare projectConfig =
- new ProjectLevelConfig.Bare(ProjectConfig.PROJECT_CONFIG);
+ VersionedConfigFile projectConfig = new VersionedConfigFile(ProjectConfig.PROJECT_CONFIG);
try (Repository repo = repoManager.openRepository(projectName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, projectName, repo)) {
boolean isAlreadyMigrated = hasMigrationAlreadyRun(repo);
diff --git a/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java b/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
index 2ca79342..d8da13d 100644
--- a/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
+++ b/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
@@ -28,8 +28,8 @@
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.git.meta.VersionedConfigFile;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.ProjectLevelConfig;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
@@ -130,8 +130,7 @@
ui.message(String.format("Skipping project %s because it has prolog rules", project));
return Status.HAS_PROLOG;
}
- ProjectLevelConfig.Bare projectConfig =
- new ProjectLevelConfig.Bare(ProjectConfig.PROJECT_CONFIG);
+ VersionedConfigFile projectConfig = new VersionedConfigFile(ProjectConfig.PROJECT_CONFIG);
boolean migrationPerformed = false;
try (Repository repo = repoManager.openRepository(project);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, project, repo)) {
@@ -275,7 +274,7 @@
cfg.setString(ProjectConfig.LABEL, labelName, ProjectConfig.KEY_FUNCTION, function);
}
- private void commit(ProjectLevelConfig.Bare projectConfig, MetaDataUpdate md) throws IOException {
+ private void commit(VersionedConfigFile projectConfig, MetaDataUpdate md) throws IOException {
md.getCommitBuilder().setAuthor(serverUser);
md.getCommitBuilder().setCommitter(serverUser);
md.setMessage(COMMIT_MSG);
diff --git a/java/com/google/gerrit/server/schema/VersionedAccountPreferences.java b/java/com/google/gerrit/server/schema/VersionedAccountPreferences.java
index 468c26b..fe28d6f 100644
--- a/java/com/google/gerrit/server/schema/VersionedAccountPreferences.java
+++ b/java/com/google/gerrit/server/schema/VersionedAccountPreferences.java
@@ -14,17 +14,12 @@
package com.google.gerrit.server.schema;
-import com.google.common.base.Strings;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.server.git.meta.VersionedMetaData;
-import java.io.IOException;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.CommitBuilder;
-import org.eclipse.jgit.lib.Config;
+import com.google.gerrit.server.git.meta.VersionedConfigFile;
/** Preferences for user accounts during schema migrations. */
-class VersionedAccountPreferences extends VersionedMetaData {
+class VersionedAccountPreferences extends VersionedConfigFile {
static final String PREFERENCES = "preferences.config";
static VersionedAccountPreferences forUser(Account.Id id) {
@@ -35,33 +30,7 @@
return new VersionedAccountPreferences(RefNames.REFS_USERS_DEFAULT);
}
- private final String ref;
- private Config cfg;
-
protected VersionedAccountPreferences(String ref) {
- this.ref = ref;
- }
-
- @Override
- protected String getRefName() {
- return ref;
- }
-
- Config getConfig() {
- return cfg;
- }
-
- @Override
- protected void onLoad() throws IOException, ConfigInvalidException {
- cfg = readConfig(PREFERENCES);
- }
-
- @Override
- protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException {
- if (Strings.isNullOrEmpty(commit.getMessage())) {
- commit.setMessage("Updated preferences\n");
- }
- saveConfig(PREFERENCES, cfg);
- return true;
+ super(ref, PREFERENCES, "Updated preferences\n");
}
}
diff --git a/java/com/google/gerrit/truth/OptionalSubject.java b/java/com/google/gerrit/truth/OptionalSubject.java
index 545932b..2023765 100644
--- a/java/com/google/gerrit/truth/OptionalSubject.java
+++ b/java/com/google/gerrit/truth/OptionalSubject.java
@@ -34,17 +34,7 @@
return assertAbout(optionals()).thatCustom(optional, valueSubjectFactory);
}
- /*
- * This could be made to return OptionalSubject<Subject, T>. But that wouldn't help users, since T
- * is not used in the API. And it could result in a longer type ("OptionalSubject<Subject,
- * SomeVeryLongType<Foo, Bar>>") in some cases.
- *
- * The parameter type "Optional<T>" remains important, however: It causes javac to prefer this
- * assertThat method over Truth.assertThat(Optional) when we static import assertThat from both
- * this class and Truth.
- */
- @SuppressWarnings("AlmostJavadoc") // "<T>" looks like an HTML tag?
- public static <T> OptionalSubject<Subject, ?> assertThat(Optional<T> optional) {
+ public static OptionalSubject<Subject, ?> assertThat(Optional<?> optional) {
return assertAbout(optionals()).that(optional);
}
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index 5bdf91f..6204115 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -49,6 +49,7 @@
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.truth.NullAwareCorrespondence;
+import com.google.gerrit.truth.OptionalSubject;
import com.google.inject.Inject;
import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
@@ -1629,7 +1630,7 @@
TestHumanComment comment = changeOperations.change(changeId).comment(childCommentUuid).get();
- assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
+ OptionalSubject.assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
}
@Test
@@ -1640,7 +1641,7 @@
TestHumanComment comment = changeOperations.change(changeId).comment(childCommentUuid).get();
- assertThat(comment.tag()).value().isEqualTo("tag");
+ OptionalSubject.assertThat(comment.tag()).value().isEqualTo("tag");
}
@Test
@@ -1700,7 +1701,7 @@
TestHumanComment comment =
changeOperations.change(changeId).draftComment(childCommentUuid).get();
- assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
+ OptionalSubject.assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
}
@Test
@@ -1730,7 +1731,7 @@
TestRobotComment comment =
changeOperations.change(changeId).robotComment(childCommentUuid).get();
- assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
+ OptionalSubject.assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
}
private ChangeInfo getChangeFromServer(Change.Id changeId) throws RestApiException {
diff --git a/javatests/com/google/gerrit/server/notedb/IntBlobTest.java b/javatests/com/google/gerrit/server/notedb/IntBlobTest.java
index 333c229..7ed2391 100644
--- a/javatests/com/google/gerrit/server/notedb/IntBlobTest.java
+++ b/javatests/com/google/gerrit/server/notedb/IntBlobTest.java
@@ -22,6 +22,7 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.git.LockFailureException;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.truth.OptionalSubject;
import java.io.IOException;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
@@ -52,7 +53,7 @@
@Test
public void parseNoRef() throws Exception {
- assertThat(IntBlob.parse(repo, "refs/nothing")).isEmpty();
+ OptionalSubject.assertThat(IntBlob.parse(repo, "refs/nothing")).isEmpty();
}
@Test
@@ -66,14 +67,18 @@
public void parseValid() throws Exception {
String refName = "refs/foo";
ObjectId id = tr.update(refName, tr.blob("123"));
- assertThat(IntBlob.parse(repo, refName)).value().isEqualTo(IntBlob.create(id, 123));
+ OptionalSubject.assertThat(IntBlob.parse(repo, refName))
+ .value()
+ .isEqualTo(IntBlob.create(id, 123));
}
@Test
public void parseWithWhitespace() throws Exception {
String refName = "refs/foo";
ObjectId id = tr.update(refName, tr.blob(" 123 "));
- assertThat(IntBlob.parse(repo, refName)).value().isEqualTo(IntBlob.create(id, 123));
+ OptionalSubject.assertThat(IntBlob.parse(repo, refName))
+ .value()
+ .isEqualTo(IntBlob.create(id, 123));
}
@Test
@@ -92,7 +97,7 @@
IntBlob.tryStore(repo, rw, projectName, refName, null, 123, GitReferenceUpdated.DISABLED);
assertThat(ru.getResult()).isEqualTo(RefUpdate.Result.NEW);
assertThat(ru.getName()).isEqualTo(refName);
- assertThat(IntBlob.parse(repo, refName))
+ OptionalSubject.assertThat(IntBlob.parse(repo, refName))
.value()
.isEqualTo(IntBlob.create(ru.getNewObjectId(), 123));
}
@@ -105,7 +110,7 @@
repo, rw, projectName, refName, ObjectId.zeroId(), 123, GitReferenceUpdated.DISABLED);
assertThat(ru.getResult()).isEqualTo(RefUpdate.Result.NEW);
assertThat(ru.getName()).isEqualTo(refName);
- assertThat(IntBlob.parse(repo, refName))
+ OptionalSubject.assertThat(IntBlob.parse(repo, refName))
.value()
.isEqualTo(IntBlob.create(ru.getNewObjectId(), 123));
}
@@ -118,7 +123,7 @@
IntBlob.tryStore(repo, rw, projectName, refName, id, 456, GitReferenceUpdated.DISABLED);
assertThat(ru.getResult()).isEqualTo(RefUpdate.Result.FORCED);
assertThat(ru.getName()).isEqualTo(refName);
- assertThat(IntBlob.parse(repo, refName))
+ OptionalSubject.assertThat(IntBlob.parse(repo, refName))
.value()
.isEqualTo(IntBlob.create(ru.getNewObjectId(), 456));
}
@@ -137,14 +142,14 @@
GitReferenceUpdated.DISABLED);
assertThat(ru.getResult()).isEqualTo(RefUpdate.Result.LOCK_FAILURE);
assertThat(ru.getName()).isEqualTo(refName);
- assertThat(IntBlob.parse(repo, refName)).isEmpty();
+ OptionalSubject.assertThat(IntBlob.parse(repo, refName)).isEmpty();
}
@Test
public void storeNoOldId() throws Exception {
String refName = "refs/foo";
IntBlob.store(repo, rw, projectName, refName, null, 123, GitReferenceUpdated.DISABLED);
- assertThat(IntBlob.parse(repo, refName))
+ OptionalSubject.assertThat(IntBlob.parse(repo, refName))
.value()
.isEqualTo(IntBlob.create(getRef(refName), 123));
}
@@ -154,7 +159,7 @@
String refName = "refs/foo";
IntBlob.store(
repo, rw, projectName, refName, ObjectId.zeroId(), 123, GitReferenceUpdated.DISABLED);
- assertThat(IntBlob.parse(repo, refName))
+ OptionalSubject.assertThat(IntBlob.parse(repo, refName))
.value()
.isEqualTo(IntBlob.create(getRef(refName), 123));
}
@@ -164,7 +169,7 @@
String refName = "refs/foo";
ObjectId id = tr.update(refName, tr.blob("123"));
IntBlob.store(repo, rw, projectName, refName, id, 456, GitReferenceUpdated.DISABLED);
- assertThat(IntBlob.parse(repo, refName))
+ OptionalSubject.assertThat(IntBlob.parse(repo, refName))
.value()
.isEqualTo(IntBlob.create(getRef(refName), 456));
}
@@ -185,7 +190,7 @@
123,
GitReferenceUpdated.DISABLED));
assertThat(thrown.getFailedRefs()).containsExactly("refs/foo");
- assertThat(IntBlob.parse(repo, refName)).isEmpty();
+ OptionalSubject.assertThat(IntBlob.parse(repo, refName)).isEmpty();
}
private ObjectId getRef(String refName) throws IOException {
diff --git a/plugins/webhooks b/plugins/webhooks
index d864a2c..2e5ec3b 160000
--- a/plugins/webhooks
+++ b/plugins/webhooks
@@ -1 +1 @@
-Subproject commit d864a2c71e261b39b89ea9425280fad425a336f3
+Subproject commit 2e5ec3b3bcf5e7ba50edba9eca3c15c8057ad6c2
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index 8fc3c1b..1640b7a 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -314,8 +314,16 @@
for (const modifier of [Modifier.CTRL_KEY, Modifier.META_KEY]) {
this.shortcuts.addLocal(
{key: Key.ENTER, modifiers: [modifier]},
- () => {
+ e => {
this.save();
+ // We don't stop propagation for patchset comment
+ // (this.permanentEditingMode = true), but we stop it for normal
+ // comments. This prevents accidentally sending a reply when
+ // editing/saving them in the reply dialog.
+ if (!this.permanentEditingMode) {
+ e.preventDefault();
+ e.stopPropagation();
+ }
},
{preventDefault: false}
);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 82f520a..098b79a 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -39,7 +39,7 @@
import {ReplyToCommentEvent} from '../../../types/events';
import {GrConfirmDeleteCommentDialog} from '../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog';
import {assertIsDefined} from '../../../utils/common-util';
-import {Modifier} from '../../../utils/dom-util';
+import {Key, Modifier} from '../../../utils/dom-util';
import {SinonStubbedMember} from 'sinon';
import {fixture, html, assert} from '@open-wc/testing';
import {GrButton} from '../gr-button/gr-button';
@@ -596,6 +596,46 @@
assert.isTrue(spy.called);
});
+ suite('ctrl+ENTER ', () => {
+ test('saves comment', async () => {
+ const spy = sinon.stub(element, 'save');
+ element.messageText = 'is that the horse from horsing around??';
+ element.editing = true;
+ await element.updateComplete;
+ pressKey(
+ element.textarea!.textarea!.textarea,
+ Key.ENTER,
+ Modifier.CTRL_KEY
+ );
+ assert.isTrue(spy.called);
+ });
+ test('propagates on patchset comment', async () => {
+ const event = new KeyboardEvent('keydown', {
+ key: Key.ENTER,
+ ctrlKey: true,
+ });
+ const stopPropagationStub = sinon.stub(event, 'stopPropagation');
+ element.permanentEditingMode = true;
+ element.messageText = 'is that the horse from horsing around??';
+ element.editing = true;
+ await element.updateComplete;
+ element.dispatchEvent(event);
+ assert.isFalse(stopPropagationStub.called);
+ });
+ test('does not propagate on normal comment', async () => {
+ const event = new KeyboardEvent('keydown', {
+ key: Key.ENTER,
+ ctrlKey: true,
+ });
+ const stopPropagationStub = sinon.stub(event, 'stopPropagation');
+ element.messageText = 'is that the horse from horsing around??';
+ element.editing = true;
+ await element.updateComplete;
+ element.dispatchEvent(event);
+ assert.isTrue(stopPropagationStub.called);
+ });
+ });
+
test('save', async () => {
const savePromise = mockPromise<DraftInfo>();
const stub = sinon.stub(commentsModel, 'saveDraft').returns(savePromise);
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts
index 19a6457..8b333e6 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts
@@ -582,7 +582,8 @@
gr-diff-row tr.diff-row,
gr-diff-row td.content,
gr-diff-section tbody.contextControl,
- gr-diff-row td.blame {
+ gr-diff-row td.blame,
+ #diffHeader {
-webkit-user-select: none;
-moz-user-select: none;
-ms-user-select: none;
