Merge "Migrate comments-model to new pattern"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 240b0ed..870e194 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -1857,7 +1857,8 @@
* The given change.
* If link:config-gerrit.html#change.submitWholeTopic[`change.submitWholeTopic`]
- is enabled, include all open changes with the same topic.
+ is enabled OR if the `o=TOPIC_CLOSURE` query parameter is passed, include all
+ open changes with the same topic.
* For each change whose submit type is not CHERRY_PICK, include unmerged
ancestors targeting the same branch.
@@ -1884,7 +1885,7 @@
Standard link:#query-options[formatting options] can be specified
with the `o` parameter, as well as the `submitted_together` specific
-option `NON_VISIBLE_CHANGES`.
+options `NON_VISIBLE_CHANGES` and `TOPIC_CLOSURE`.
.Response
----
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 9ca881d..2fb2127 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -26,6 +26,7 @@
import static com.google.gerrit.entities.Patch.COMMIT_MSG;
import static com.google.gerrit.entities.Patch.MERGE_LIST;
import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.NON_VISIBLE_CHANGES;
+import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.TOPIC_CLOSURE;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
@@ -41,6 +42,7 @@
import com.google.common.base.Ticker;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.jimfs.Jimfs;
@@ -80,6 +82,7 @@
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.changes.SubmittedTogetherInfo;
+import com.google.gerrit.extensions.api.changes.SubmittedTogetherOption;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInfo;
import com.google.gerrit.extensions.api.projects.BranchInput;
@@ -1203,9 +1206,37 @@
}
protected void assertSubmittedTogether(String chId, String... expected) throws Exception {
- List<ChangeInfo> actual = gApi.changes().id(chId).submittedTogether();
+ assertSubmittedTogether(chId, ImmutableSet.of(), expected);
+ }
+
+ protected void assertSubmittedTogetherWithTopicClosure(String chId, String... expected)
+ throws Exception {
+ assertSubmittedTogether(chId, ImmutableSet.of(TOPIC_CLOSURE), expected);
+ }
+
+ protected void assertSubmittedTogether(
+ String chId,
+ ImmutableSet<SubmittedTogetherOption> submittedTogetherOptions,
+ String... expected)
+ throws Exception {
+ // This does not include NON_VISIBILE_CHANGES
+ List<ChangeInfo> actual =
+ submittedTogetherOptions.isEmpty()
+ ? gApi.changes().id(chId).submittedTogether()
+ : gApi.changes()
+ .id(chId)
+ .submittedTogether(EnumSet.copyOf(submittedTogetherOptions))
+ .changes;
+
+ EnumSet enumSetIncludingNonVisibleChanges =
+ submittedTogetherOptions.isEmpty()
+ ? EnumSet.of(NON_VISIBLE_CHANGES)
+ : EnumSet.copyOf(submittedTogetherOptions);
+ enumSetIncludingNonVisibleChanges.add(NON_VISIBLE_CHANGES);
+
+ // This includes NON_VISIBLE_CHANGES for comparison.
SubmittedTogetherInfo info =
- gApi.changes().id(chId).submittedTogether(EnumSet.of(NON_VISIBLE_CHANGES));
+ gApi.changes().id(chId).submittedTogether(enumSetIncludingNonVisibleChanges);
assertThat(info.nonVisibleChanges).isEqualTo(0);
assertThat(Iterables.transform(actual, i1 -> i1.changeId))
diff --git a/java/com/google/gerrit/extensions/api/changes/SubmittedTogetherOption.java b/java/com/google/gerrit/extensions/api/changes/SubmittedTogetherOption.java
index e2cab4d..68a4e88 100644
--- a/java/com/google/gerrit/extensions/api/changes/SubmittedTogetherOption.java
+++ b/java/com/google/gerrit/extensions/api/changes/SubmittedTogetherOption.java
@@ -16,5 +16,6 @@
/** Output options available for submitted_together requests. */
public enum SubmittedTogetherOption {
- NON_VISIBLE_CHANGES;
+ NON_VISIBLE_CHANGES,
+ TOPIC_CLOSURE;
}
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 5155a0d..154e45a 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -304,7 +304,10 @@
return null; // submit not visible
}
- ChangeSet cs = mergeSuperSet.get().completeChangeSet(cd.change(), resource.getUser());
+ ChangeSet cs =
+ mergeSuperSet
+ .get()
+ .completeChangeSet(cd.change(), resource.getUser(), /*includingTopicClosure= */ false);
String topic = change.getTopic();
int topicSize = 0;
if (!Strings.isNullOrEmpty(topic)) {
diff --git a/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java b/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
index 214a001..c18e7c2 100644
--- a/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
+++ b/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.restapi.change;
import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.NON_VISIBLE_CHANGES;
+import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.TOPIC_CLOSURE;
import static java.util.Collections.reverseOrder;
import static java.util.stream.Collectors.toList;
@@ -127,7 +128,10 @@
int hidden;
if (c.isNew()) {
- ChangeSet cs = mergeSuperSet.get().completeChangeSet(c, resource.getUser());
+ ChangeSet cs =
+ mergeSuperSet
+ .get()
+ .completeChangeSet(c, resource.getUser(), options.contains(TOPIC_CLOSURE));
cds = ensureRequiredDataIsLoaded(cs.changes().asList());
hidden = cs.nonVisibleChanges().size();
} else if (c.isMerged()) {
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index 64b60bb..b431299 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -480,7 +480,9 @@
logger.atFine().log("Beginning integration of %s", change);
try {
ChangeSet indexBackedChangeSet =
- mergeSuperSet.setMergeOpRepoManager(orm).completeChangeSet(change, caller);
+ mergeSuperSet
+ .setMergeOpRepoManager(orm)
+ .completeChangeSet(change, caller, /* includingTopicClosure= */ false);
if (!indexBackedChangeSet.ids().contains(change.getId())) {
// indexBackedChangeSet contains only open changes, if the change is missing in this set
// it might be that the change was concurrently submitted in the meantime.
diff --git a/java/com/google/gerrit/server/submit/MergeSuperSet.java b/java/com/google/gerrit/server/submit/MergeSuperSet.java
index 67f2907..8581e20 100644
--- a/java/com/google/gerrit/server/submit/MergeSuperSet.java
+++ b/java/com/google/gerrit/server/submit/MergeSuperSet.java
@@ -92,7 +92,19 @@
return this;
}
- public ChangeSet completeChangeSet(Change change, CurrentUser user)
+ /**
+ * Gets the ChangeSet of this {@code change} based on visiblity of the {@code user}. if
+ * change.submitWholeTopic is true, we return the topic closure as well as the dependent changes
+ * of the topic closure. Otherwise, we return just the dependent changes.
+ *
+ * @param change the change for which we get the dependent changes / topic closure.
+ * @param user the current user for visibility purposes.
+ * @param includingTopicClosure when true, return as if change.submitWholeTopic = true, so we
+ * return the topic closure.
+ * @return {@link ChangeSet} object that represents the dependent changes and/or topic closure of
+ * the requested change.
+ */
+ public ChangeSet completeChangeSet(Change change, CurrentUser user, boolean includingTopicClosure)
throws IOException, PermissionBackendException {
try {
if (orm == null) {
@@ -113,7 +125,7 @@
}
ChangeSet changeSet = new ChangeSet(cd, visible);
- if (wholeTopicEnabled(cfg)) {
+ if (wholeTopicEnabled(cfg) || includingTopicClosure) {
return completeChangeSetIncludingTopics(changeSet, user);
}
try (TraceContext traceContext = PluginContext.newTrace(mergeSuperSetComputation)) {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitResolvingMergeCommitIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitResolvingMergeCommitIT.java
index a63d60a..0a9a098 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitResolvingMergeCommitIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitResolvingMergeCommitIT.java
@@ -306,7 +306,10 @@
private void assertChangeSetMergeable(ChangeData change, boolean expected)
throws MissingObjectException, IncorrectObjectTypeException, IOException,
PermissionBackendException {
- ChangeSet cs = mergeSuperSet.get().completeChangeSet(change.change(), user(admin));
+ ChangeSet cs =
+ mergeSuperSet
+ .get()
+ .completeChangeSet(change.change(), user(admin), /* includingTopicClosure= */ false);
assertThat(submit.unmergeableChanges(cs).isEmpty()).isEqualTo(expected);
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java b/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
index a97fb49..7e0bce9 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
@@ -130,6 +130,8 @@
} else {
assertSubmittedTogether(id1);
assertSubmittedTogether(id2);
+ assertSubmittedTogetherWithTopicClosure(id1, id2, id1);
+ assertSubmittedTogetherWithTopicClosure(id2, id2, id1);
}
}
@@ -152,6 +154,8 @@
} else {
assertSubmittedTogether(id1);
assertSubmittedTogether(id2);
+ assertSubmittedTogetherWithTopicClosure(id1, id2, id1);
+ assertSubmittedTogetherWithTopicClosure(id2, id2, id1);
}
}
@@ -180,6 +184,9 @@
assertSubmittedTogether(id1);
assertSubmittedTogether(id2);
assertSubmittedTogether(id3, id3, id2);
+ assertSubmittedTogetherWithTopicClosure(id1, id2, id1);
+ assertSubmittedTogetherWithTopicClosure(id2, id2, id1);
+ assertSubmittedTogetherWithTopicClosure(id3, id3, id2, id1);
}
}
@@ -227,6 +234,13 @@
assertSubmittedTogether(id4, id4, id3, id2);
assertSubmittedTogether(id5);
assertSubmittedTogether(id6, id6, id5);
+
+ assertSubmittedTogetherWithTopicClosure(id1, id6, id5, id3, id2, id1);
+ assertSubmittedTogetherWithTopicClosure(id2, id6, id5, id2);
+ assertSubmittedTogetherWithTopicClosure(id3, id6, id5, id3, id2, id1);
+ assertSubmittedTogetherWithTopicClosure(id4, id6, id5, id4, id3, id2, id1);
+ assertSubmittedTogetherWithTopicClosure(id5);
+ assertSubmittedTogetherWithTopicClosure(id6, id6, id5, id2);
}
}
diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.ts b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.ts
index 172f807..f811ee2 100644
--- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.ts
+++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.ts
@@ -71,7 +71,7 @@
];
interface Rule {
- value: RuleValue;
+ value?: RuleValue;
}
interface RuleValue {
@@ -158,17 +158,17 @@
// Observer _handleValueChange is called after the ready()
// method finishes. Original values must be set later to
// avoid set .modified flag to true
- this._setOriginalRuleValues(this.rule.value);
+ this._setOriginalRuleValues(this.rule?.value);
}
}
- _setupValues(rule: Rule) {
- if (!rule.value) {
+ _setupValues(rule?: Rule) {
+ if (!rule?.value) {
this._setDefaultRuleValues();
}
}
- _computeForce(permission: AccessPermissionId, action: string) {
+ _computeForce(permission: AccessPermissionId, action?: string) {
if (AccessPermissionId.PUSH === permission && action !== Action.DENY) {
return true;
}
@@ -176,7 +176,7 @@
return AccessPermissionId.EDIT_TOPIC_NAME === permission;
}
- _computeForceClass(permission: AccessPermissionId, action: string) {
+ _computeForceClass(permission: AccessPermissionId, action?: string) {
return this._computeForce(permission, action) ? 'force' : '';
}
@@ -213,7 +213,7 @@
return classList.join(' ');
}
- _computeForceOptions(permission: string, action: string) {
+ _computeForceOptions(permission: string, action?: string) {
if (permission === AccessPermissionId.PUSH) {
if (action === Action.ALLOW) {
return ForcePushOptions.ALLOW;
@@ -259,7 +259,7 @@
}
_handleRemoveRule() {
- if (!this.rule) return;
+ if (!this.rule?.value) return;
if (this.rule.value.added) {
fireEvent(this, 'added-rule-removed');
}
@@ -269,13 +269,13 @@
}
_handleUndoRemove() {
- if (!this.rule) return;
+ if (!this.rule?.value) return;
this._deleted = false;
delete this.rule.value.deleted;
}
_handleUndoChange() {
- if (!this.rule) return;
+ if (!this.rule?.value) return;
// gr-permission will take care of removing rules that were added but
// unsaved. We need to keep the added bit for the filter.
if (this.rule.value.added) {
@@ -289,7 +289,7 @@
@observe('rule.value.*')
_handleValueChange() {
- if (!this._originalRuleValues || !this.rule) {
+ if (!this._originalRuleValues || !this.rule?.value) {
return;
}
this.rule.value.modified = true;
@@ -297,7 +297,8 @@
fireEvent(this, 'access-modified');
}
- _setOriginalRuleValues(value: RuleValue) {
+ _setOriginalRuleValues(value?: RuleValue) {
+ if (value === undefined) return;
this._originalRuleValues = {...value};
}
}
diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.js b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.js
deleted file mode 100644
index f3df132..0000000
--- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.js
+++ /dev/null
@@ -1,586 +0,0 @@
-/**
- * @license
- * Copyright (C) 2017 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.
- */
-
-import '../../../test/common-test-setup-karma.js';
-import './gr-rule-editor.js';
-
-const basicFixture = fixtureFromElement('gr-rule-editor');
-
-suite('gr-rule-editor tests', () => {
- let element;
-
- setup(() => {
- element = basicFixture.instantiate();
- });
-
- suite('unit tests', () => {
- test('_computeForce, _computeForceClass, and _computeForceOptions',
- () => {
- const ForcePushOptions = {
- ALLOW: [
- {name: 'Allow pushing (but not force pushing)', value: false},
- {name: 'Allow pushing with or without force', value: true},
- ],
- BLOCK: [
- {name: 'Block pushing with or without force', value: false},
- {name: 'Block force pushing', value: true},
- ],
- };
-
- const FORCE_EDIT_OPTIONS = [
- {
- name: 'No Force Edit',
- value: false,
- },
- {
- name: 'Force Edit',
- value: true,
- },
- ];
- let permission = 'push';
- let action = 'ALLOW';
- assert.isTrue(element._computeForce(permission, action));
- assert.equal(element._computeForceClass(permission, action),
- 'force');
- assert.deepEqual(element._computeForceOptions(permission, action),
- ForcePushOptions.ALLOW);
-
- action = 'BLOCK';
- assert.isTrue(element._computeForce(permission, action));
- assert.equal(element._computeForceClass(permission, action),
- 'force');
- assert.deepEqual(element._computeForceOptions(permission, action),
- ForcePushOptions.BLOCK);
-
- action = 'DENY';
- assert.isFalse(element._computeForce(permission, action));
- assert.equal(element._computeForceClass(permission, action), '');
- assert.equal(
- element._computeForceOptions(permission, action).length, 0);
-
- permission = 'editTopicName';
- assert.isTrue(element._computeForce(permission));
- assert.equal(element._computeForceClass(permission), 'force');
- assert.deepEqual(element._computeForceOptions(permission),
- FORCE_EDIT_OPTIONS);
- permission = 'submit';
- assert.isFalse(element._computeForce(permission));
- assert.equal(element._computeForceClass(permission), '');
- assert.deepEqual(element._computeForceOptions(permission), []);
- });
-
- test('_computeSectionClass', () => {
- let deleted = true;
- let editing = false;
- assert.equal(element._computeSectionClass(editing, deleted), 'deleted');
-
- deleted = false;
- assert.equal(element._computeSectionClass(editing, deleted), '');
-
- editing = true;
- assert.equal(element._computeSectionClass(editing, deleted), 'editing');
-
- deleted = true;
- assert.equal(element._computeSectionClass(editing, deleted),
- 'editing deleted');
- });
-
- test('_getDefaultRuleValues', () => {
- let permission = 'priority';
- let label;
- assert.deepEqual(element._getDefaultRuleValues(permission, label),
- {action: 'BATCH'});
- permission = 'label-Code-Review';
- label = {values: [
- {value: -2, text: 'This shall not be merged'},
- {value: -1, text: 'I would prefer this is not merged as is'},
- {value: -0, text: 'No score'},
- {value: 1, text: 'Looks good to me, but someone else must approve'},
- {value: 2, text: 'Looks good to me, approved'},
- ]};
- assert.deepEqual(element._getDefaultRuleValues(permission, label),
- {action: 'ALLOW', max: 2, min: -2});
- permission = 'push';
- label = undefined;
- assert.deepEqual(element._getDefaultRuleValues(permission, label),
- {action: 'ALLOW', force: false});
- permission = 'submit';
- assert.deepEqual(element._getDefaultRuleValues(permission, label),
- {action: 'ALLOW'});
- });
-
- test('_setDefaultRuleValues', () => {
- element.rule = {id: 123};
- const defaultValue = {action: 'ALLOW'};
- sinon.stub(element, '_getDefaultRuleValues').returns(defaultValue);
- element._setDefaultRuleValues();
- assert.isTrue(element._getDefaultRuleValues.called);
- assert.equal(element.rule.value, defaultValue);
- });
-
- test('_computeOptions', () => {
- const PRIORITY_OPTIONS = [
- 'BATCH',
- 'INTERACTIVE',
- ];
- const DROPDOWN_OPTIONS = [
- 'ALLOW',
- 'DENY',
- 'BLOCK',
- ];
- let permission = 'priority';
- assert.deepEqual(element._computeOptions(permission), PRIORITY_OPTIONS);
- permission = 'submit';
- assert.deepEqual(element._computeOptions(permission), DROPDOWN_OPTIONS);
- });
-
- test('_handleValueChange', () => {
- const modifiedHandler = sinon.stub();
- element.rule = {value: {}};
- element.addEventListener('access-modified', modifiedHandler);
- element._handleValueChange();
- assert.isNotOk(element.rule.value.modified);
- element._originalRuleValues = {};
- element._handleValueChange();
- assert.isTrue(element.rule.value.modified);
- assert.isTrue(modifiedHandler.called);
- });
-
- test('_handleAccessSaved', () => {
- const originalValue = {action: 'DENY'};
- const newValue = {action: 'ALLOW'};
- element._originalRuleValues = originalValue;
- element.rule = {value: newValue};
- element._handleAccessSaved();
- assert.deepEqual(element._originalRuleValues, newValue);
- });
-
- test('_setOriginalRuleValues', () => {
- const value = {
- action: 'ALLOW',
- force: false,
- };
- element._setOriginalRuleValues(value);
- assert.deepEqual(element._originalRuleValues, value);
- });
- });
-
- suite('already existing generic rule', () => {
- setup(async () => {
- element.group = 'Group Name';
- element.permission = 'submit';
- element.rule = {
- id: '123',
- value: {
- action: 'ALLOW',
- force: false,
- },
- };
- element.section = 'refs/*';
-
- // Typically called on ready since elements will have properties defined
- // by the parent element.
- element._setupValues(element.rule);
- await flush();
- element.connectedCallback();
- });
-
- test('_ruleValues and _originalRuleValues are set correctly', () => {
- assert.deepEqual(element._originalRuleValues, element.rule.value);
- });
-
- test('values are set correctly', () => {
- assert.equal(element.$.action.bindValue, element.rule.value.action);
- assert.isNotOk(element.root.querySelector('#labelMin'));
- assert.isNotOk(element.root.querySelector('#labelMax'));
- assert.isFalse(element.$.force.classList.contains('force'));
- });
-
- test('modify and cancel restores original values', () => {
- element.editing = true;
- assert.notEqual(getComputedStyle(element.$.removeBtn).display, 'none');
- assert.isNotOk(element.rule.value.modified);
- element.$.action.bindValue = 'DENY';
- assert.isTrue(element.rule.value.modified);
- element.editing = false;
- assert.equal(getComputedStyle(element.$.removeBtn).display, 'none');
- assert.deepEqual(element._originalRuleValues, element.rule.value);
- assert.equal(element.$.action.bindValue, 'ALLOW');
- assert.isNotOk(element.rule.value.modified);
- });
-
- test('modify value', () => {
- assert.isNotOk(element.rule.value.modified);
- element.$.action.bindValue = 'DENY';
- flush();
- assert.isTrue(element.rule.value.modified);
-
- // The original value should now differ from the rule values.
- assert.notDeepEqual(element._originalRuleValues, element.rule.value);
- });
-
- test('all selects are disabled when not in edit mode', () => {
- const selects = element.root.querySelectorAll('select');
- for (const select of selects) {
- assert.isTrue(select.disabled);
- }
- element.editing = true;
- for (const select of selects) {
- assert.isFalse(select.disabled);
- }
- });
-
- test('remove rule and undo remove', () => {
- element.editing = true;
- element.rule = {id: 123, value: {action: 'ALLOW'}};
- assert.isFalse(
- element.$.deletedContainer.classList.contains('deleted'));
- MockInteractions.tap(element.$.removeBtn);
- assert.isTrue(element.$.deletedContainer.classList.contains('deleted'));
- assert.isTrue(element._deleted);
- assert.isTrue(element.rule.value.deleted);
-
- MockInteractions.tap(element.$.undoRemoveBtn);
- assert.isFalse(element._deleted);
- assert.isNotOk(element.rule.value.deleted);
- });
-
- test('remove rule and cancel', () => {
- element.editing = true;
- assert.notEqual(getComputedStyle(element.$.removeBtn).display, 'none');
- assert.equal(getComputedStyle(element.$.deletedContainer).display,
- 'none');
-
- element.rule = {id: 123, value: {action: 'ALLOW'}};
- MockInteractions.tap(element.$.removeBtn);
- assert.notEqual(getComputedStyle(element.$.removeBtn).display, 'none');
- assert.notEqual(getComputedStyle(element.$.deletedContainer).display,
- 'none');
- assert.isTrue(element._deleted);
- assert.isTrue(element.rule.value.deleted);
-
- element.editing = false;
- assert.isFalse(element._deleted);
- assert.isNotOk(element.rule.value.deleted);
- assert.isNotOk(element.rule.value.modified);
-
- assert.deepEqual(element._originalRuleValues, element.rule.value);
- assert.equal(getComputedStyle(element.$.removeBtn).display, 'none');
- assert.equal(getComputedStyle(element.$.deletedContainer).display,
- 'none');
- });
-
- test('_computeGroupPath', () => {
- const group = '123';
- assert.equal(element._computeGroupPath(group),
- `/admin/groups/123`);
- });
- });
-
- suite('new edit rule', () => {
- setup(async () => {
- element.group = 'Group Name';
- element.permission = 'editTopicName';
- element.rule = {
- id: '123',
- };
- element.section = 'refs/*';
- element._setupValues(element.rule);
- await flush();
- element.rule.value.added = true;
- await flush();
- element.connectedCallback();
- });
-
- test('_ruleValues and _originalRuleValues are set correctly', () => {
- // Since the element does not already have default values, they should
- // be set. The original values should be set to those too.
- assert.isNotOk(element.rule.value.modified);
- const expectedRuleValue = {
- action: 'ALLOW',
- force: false,
- added: true,
- };
- assert.deepEqual(element.rule.value, expectedRuleValue);
- test('values are set correctly', () => {
- assert.equal(element.$.action.bindValue, expectedRuleValue.action);
- assert.equal(element.$.force.bindValue, expectedRuleValue.action);
- });
- });
-
- test('modify value', () => {
- assert.isNotOk(element.rule.value.modified);
- element.$.force.bindValue = true;
- flush();
- assert.isTrue(element.rule.value.modified);
-
- // The original value should now differ from the rule values.
- assert.notDeepEqual(element._originalRuleValues, element.rule.value);
- });
-
- test('remove value', () => {
- element.editing = true;
- const removeStub = sinon.stub();
- element.addEventListener('added-rule-removed', removeStub);
- MockInteractions.tap(element.$.removeBtn);
- flush();
- assert.isTrue(removeStub.called);
- });
- });
-
- suite('already existing rule with labels', () => {
- setup(async () => {
- element.label = {values: [
- {value: -2, text: 'This shall not be merged'},
- {value: -1, text: 'I would prefer this is not merged as is'},
- {value: -0, text: 'No score'},
- {value: 1, text: 'Looks good to me, but someone else must approve'},
- {value: 2, text: 'Looks good to me, approved'},
- ]};
- element.group = 'Group Name';
- element.permission = 'label-Code-Review';
- element.rule = {
- id: '123',
- value: {
- action: 'ALLOW',
- force: false,
- max: 2,
- min: -2,
- },
- };
- element.section = 'refs/*';
- element._setupValues(element.rule);
- await flush();
- element.connectedCallback();
- });
-
- test('_ruleValues and _originalRuleValues are set correctly', () => {
- assert.deepEqual(element._originalRuleValues, element.rule.value);
- });
-
- test('values are set correctly', () => {
- assert.equal(element.$.action.bindValue, element.rule.value.action);
- assert.equal(
- element.root.querySelector('#labelMin').bindValue,
- element.rule.value.min);
- assert.equal(
- element.root.querySelector('#labelMax').bindValue,
- element.rule.value.max);
- assert.isFalse(element.$.force.classList.contains('force'));
- });
-
- test('modify value', () => {
- const removeStub = sinon.stub();
- element.addEventListener('added-rule-removed', removeStub);
- assert.isNotOk(element.rule.value.modified);
- element.root.querySelector('#labelMin').bindValue = 1;
- flush();
- assert.isTrue(element.rule.value.modified);
- assert.isFalse(removeStub.called);
-
- // The original value should now differ from the rule values.
- assert.notDeepEqual(element._originalRuleValues, element.rule.value);
- });
- });
-
- suite('new rule with labels', () => {
- setup(async () => {
- sinon.spy(element, '_setDefaultRuleValues');
- element.label = {values: [
- {value: -2, text: 'This shall not be merged'},
- {value: -1, text: 'I would prefer this is not merged as is'},
- {value: -0, text: 'No score'},
- {value: 1, text: 'Looks good to me, but someone else must approve'},
- {value: 2, text: 'Looks good to me, approved'},
- ]};
- element.group = 'Group Name';
- element.permission = 'label-Code-Review';
- element.rule = {
- id: '123',
- };
- element.section = 'refs/*';
- element._setupValues(element.rule);
- await flush();
- element.rule.value.added = true;
- await flush();
- element.connectedCallback();
- });
-
- test('_ruleValues and _originalRuleValues are set correctly', () => {
- // Since the element does not already have default values, they should
- // be set. The original values should be set to those too.
- assert.isNotOk(element.rule.value.modified);
- assert.isTrue(element._setDefaultRuleValues.called);
-
- const expectedRuleValue = {
- max: element.label.values[element.label.values.length - 1].value,
- min: element.label.values[0].value,
- action: 'ALLOW',
- added: true,
- };
- assert.deepEqual(element.rule.value, expectedRuleValue);
- test('values are set correctly', () => {
- assert.equal(
- element.$.action.bindValue,
- expectedRuleValue.action);
- assert.equal(
- element.root.querySelector('#labelMin').bindValue,
- expectedRuleValue.min);
- assert.equal(
- element.root.querySelector('#labelMax').bindValue,
- expectedRuleValue.max);
- });
- });
-
- test('modify value', () => {
- assert.isNotOk(element.rule.value.modified);
- element.root.querySelector('#labelMin').bindValue = 1;
- flush();
- assert.isTrue(element.rule.value.modified);
-
- // The original value should now differ from the rule values.
- assert.notDeepEqual(element._originalRuleValues, element.rule.value);
- });
- });
-
- suite('already existing push rule', () => {
- setup(async () => {
- element.group = 'Group Name';
- element.permission = 'push';
- element.rule = {
- id: '123',
- value: {
- action: 'ALLOW',
- force: true,
- },
- };
- element.section = 'refs/*';
- element._setupValues(element.rule);
- await flush();
- element.connectedCallback();
- });
-
- test('_ruleValues and _originalRuleValues are set correctly', () => {
- assert.deepEqual(element._originalRuleValues, element.rule.value);
- });
-
- test('values are set correctly', () => {
- assert.isTrue(element.$.force.classList.contains('force'));
- assert.equal(element.$.action.bindValue, element.rule.value.action);
- assert.equal(
- element.root.querySelector('#force').bindValue,
- element.rule.value.force);
- assert.isNotOk(element.root.querySelector('#labelMin'));
- assert.isNotOk(element.root.querySelector('#labelMax'));
- });
-
- test('modify value', () => {
- assert.isNotOk(element.rule.value.modified);
- element.$.action.bindValue = false;
- flush();
- assert.isTrue(element.rule.value.modified);
-
- // The original value should now differ from the rule values.
- assert.notDeepEqual(element._originalRuleValues, element.rule.value);
- });
- });
-
- suite('new push rule', () => {
- setup(async () => {
- element.group = 'Group Name';
- element.permission = 'push';
- element.rule = {
- id: '123',
- };
- element.section = 'refs/*';
- element._setupValues(element.rule);
- await flush();
- element.rule.value.added = true;
- await flush();
- element.connectedCallback();
- });
-
- test('_ruleValues and _originalRuleValues are set correctly', () => {
- // Since the element does not already have default values, they should
- // be set. The original values should be set to those too.
- assert.isNotOk(element.rule.value.modified);
- const expectedRuleValue = {
- action: 'ALLOW',
- force: false,
- added: true,
- };
- assert.deepEqual(element.rule.value, expectedRuleValue);
- test('values are set correctly', () => {
- assert.equal(element.$.action.bindValue, expectedRuleValue.action);
- assert.equal(element.$.force.bindValue, expectedRuleValue.action);
- });
- });
-
- test('modify value', () => {
- assert.isNotOk(element.rule.value.modified);
- element.$.force.bindValue = true;
- flush();
- assert.isTrue(element.rule.value.modified);
-
- // The original value should now differ from the rule values.
- assert.notDeepEqual(element._originalRuleValues, element.rule.value);
- });
- });
-
- suite('already existing edit rule', () => {
- setup(async () => {
- element.group = 'Group Name';
- element.permission = 'editTopicName';
- element.rule = {
- id: '123',
- value: {
- action: 'ALLOW',
- force: true,
- },
- };
- element.section = 'refs/*';
- element._setupValues(element.rule);
- await flush();
- element.connectedCallback();
- });
-
- test('_ruleValues and _originalRuleValues are set correctly', () => {
- assert.deepEqual(element._originalRuleValues, element.rule.value);
- });
-
- test('values are set correctly', () => {
- assert.isTrue(element.$.force.classList.contains('force'));
- assert.equal(element.$.action.bindValue, element.rule.value.action);
- assert.equal(
- element.root.querySelector('#force').bindValue,
- element.rule.value.force);
- assert.isNotOk(element.root.querySelector('#labelMin'));
- assert.isNotOk(element.root.querySelector('#labelMax'));
- });
-
- test('modify value', async () => {
- assert.isNotOk(element.rule.value.modified);
- element.$.action.bindValue = false;
- await flush();
- assert.isTrue(element.rule.value.modified);
-
- // The original value should now differ from the rule values.
- assert.notDeepEqual(element._originalRuleValues, element.rule.value);
- });
- });
-});
-
diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.ts b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.ts
new file mode 100644
index 0000000..1afd123
--- /dev/null
+++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.ts
@@ -0,0 +1,685 @@
+/**
+ * @license
+ * Copyright (C) 2017 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.
+ */
+
+import '../../../test/common-test-setup-karma';
+import './gr-rule-editor';
+import {GrRuleEditor} from './gr-rule-editor';
+import {AccessPermissionId} from '../../../utils/access-util';
+import {query, queryAll, queryAndAssert} from '../../../test/test-utils';
+import {GrButton} from '../../shared/gr-button/gr-button';
+import {GrSelect} from '../../shared/gr-select/gr-select';
+import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
+
+const basicFixture = fixtureFromElement('gr-rule-editor');
+
+suite('gr-rule-editor tests', () => {
+ let element: GrRuleEditor;
+
+ setup(() => {
+ element = basicFixture.instantiate();
+ });
+
+ suite('unit tests', () => {
+ test('_computeForce, _computeForceClass, and _computeForceOptions', () => {
+ const ForcePushOptions = {
+ ALLOW: [
+ {name: 'Allow pushing (but not force pushing)', value: false},
+ {name: 'Allow pushing with or without force', value: true},
+ ],
+ BLOCK: [
+ {name: 'Block pushing with or without force', value: false},
+ {name: 'Block force pushing', value: true},
+ ],
+ };
+
+ const FORCE_EDIT_OPTIONS = [
+ {
+ name: 'No Force Edit',
+ value: false,
+ },
+ {
+ name: 'Force Edit',
+ value: true,
+ },
+ ];
+ let permission = 'push' as AccessPermissionId;
+ let action = 'ALLOW';
+ assert.isTrue(element._computeForce(permission, action));
+ assert.equal(element._computeForceClass(permission, action), 'force');
+ assert.deepEqual(
+ element._computeForceOptions(permission, action),
+ ForcePushOptions.ALLOW
+ );
+
+ action = 'BLOCK';
+ assert.isTrue(element._computeForce(permission, action));
+ assert.equal(element._computeForceClass(permission, action), 'force');
+ assert.deepEqual(
+ element._computeForceOptions(permission, action),
+ ForcePushOptions.BLOCK
+ );
+
+ action = 'DENY';
+ assert.isFalse(element._computeForce(permission, action));
+ assert.equal(element._computeForceClass(permission, action), '');
+ assert.equal(element._computeForceOptions(permission, action).length, 0);
+
+ permission = 'editTopicName' as AccessPermissionId;
+ assert.isTrue(element._computeForce(permission));
+ assert.equal(element._computeForceClass(permission), 'force');
+ assert.deepEqual(
+ element._computeForceOptions(permission),
+ FORCE_EDIT_OPTIONS
+ );
+ permission = 'submit' as AccessPermissionId;
+ assert.isFalse(element._computeForce(permission));
+ assert.equal(element._computeForceClass(permission), '');
+ assert.deepEqual(element._computeForceOptions(permission), []);
+ });
+
+ test('_computeSectionClass', () => {
+ let deleted = true;
+ let editing = false;
+ assert.equal(element._computeSectionClass(editing, deleted), 'deleted');
+
+ deleted = false;
+ assert.equal(element._computeSectionClass(editing, deleted), '');
+
+ editing = true;
+ assert.equal(element._computeSectionClass(editing, deleted), 'editing');
+
+ deleted = true;
+ assert.equal(
+ element._computeSectionClass(editing, deleted),
+ 'editing deleted'
+ );
+ });
+
+ test('_getDefaultRuleValues', () => {
+ let permission = 'priority' as AccessPermissionId;
+ let label;
+ assert.deepEqual(element._getDefaultRuleValues(permission, label), {
+ action: 'BATCH',
+ });
+ permission = 'label-Code-Review' as AccessPermissionId;
+ label = {
+ values: [
+ {value: -2, text: 'This shall not be merged'},
+ {value: -1, text: 'I would prefer this is not merged as is'},
+ {value: -0, text: 'No score'},
+ {value: 1, text: 'Looks good to me, but someone else must approve'},
+ {value: 2, text: 'Looks good to me, approved'},
+ ],
+ };
+ assert.deepEqual(element._getDefaultRuleValues(permission, label), {
+ action: 'ALLOW',
+ max: 2,
+ min: -2,
+ });
+ permission = 'push' as AccessPermissionId;
+ label = undefined;
+ assert.deepEqual(element._getDefaultRuleValues(permission, label), {
+ action: 'ALLOW',
+ force: false,
+ });
+ permission = 'submit' as AccessPermissionId;
+ assert.deepEqual(element._getDefaultRuleValues(permission, label), {
+ action: 'ALLOW',
+ });
+ });
+
+ test('_setDefaultRuleValues', async () => {
+ element.rule = {value: {}};
+ const defaultValue = {action: 'ALLOW'};
+ const getDefaultRuleValuesStub = sinon
+ .stub(element, '_getDefaultRuleValues')
+ .returns(defaultValue);
+ element._setDefaultRuleValues();
+ assert.isTrue(getDefaultRuleValuesStub.called);
+ assert.equal(element.rule!.value, defaultValue);
+ });
+
+ test('_computeOptions', () => {
+ const PRIORITY_OPTIONS = ['BATCH', 'INTERACTIVE'];
+ const DROPDOWN_OPTIONS = ['ALLOW', 'DENY', 'BLOCK'];
+ let permission = 'priority';
+ assert.deepEqual(element._computeOptions(permission), PRIORITY_OPTIONS);
+ permission = 'submit';
+ assert.deepEqual(element._computeOptions(permission), DROPDOWN_OPTIONS);
+ });
+
+ test('_handleValueChange', () => {
+ const modifiedHandler = sinon.stub();
+ element.rule = {value: {}};
+ element.addEventListener('access-modified', modifiedHandler);
+ element._handleValueChange();
+ assert.isNotOk(element.rule!.value!.modified);
+ element._originalRuleValues = {};
+ element._handleValueChange();
+ assert.isTrue(element.rule!.value!.modified);
+ assert.isTrue(modifiedHandler.called);
+ });
+
+ test('_handleAccessSaved', () => {
+ const originalValue = {action: 'DENY'};
+ const newValue = {action: 'ALLOW'};
+ element._originalRuleValues = originalValue;
+ element.rule = {value: newValue};
+ element._handleAccessSaved();
+ assert.deepEqual(element._originalRuleValues, newValue);
+ });
+
+ test('_setOriginalRuleValues', () => {
+ const value = {
+ action: 'ALLOW',
+ force: false,
+ };
+ element._setOriginalRuleValues(value);
+ assert.deepEqual(element._originalRuleValues, value);
+ });
+ });
+
+ suite('already existing generic rule', () => {
+ setup(async () => {
+ element.groupName = 'Group Name';
+ element.permission = 'submit' as AccessPermissionId;
+ element.rule = {
+ value: {
+ action: 'ALLOW',
+ force: false,
+ },
+ };
+ element.section = 'refs/*';
+
+ // Typically called on ready since elements will have properties defined
+ // by the parent element.
+ element._setupValues(element.rule);
+ await flush();
+ element.connectedCallback();
+ });
+
+ test('_ruleValues and _originalRuleValues are set correctly', () => {
+ assert.deepEqual(element._originalRuleValues, element.rule!.value);
+ });
+
+ test('values are set correctly', () => {
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#action').bindValue,
+ element.rule!.value!.action
+ );
+ assert.isNotOk(query<GrSelect>(element, '#labelMin'));
+ assert.isNotOk(query<GrSelect>(element, '#labelMax'));
+ assert.isFalse(
+ queryAndAssert<GrSelect>(element, '#force').classList.contains('force')
+ );
+ });
+
+ test('modify and cancel restores original values', () => {
+ element.editing = true;
+ assert.notEqual(
+ getComputedStyle(queryAndAssert<GrButton>(element, '#removeBtn'))
+ .display,
+ 'none'
+ );
+ assert.isNotOk(element.rule!.value!.modified);
+ const actionBindValue = queryAndAssert<GrSelect>(element, '#action');
+ actionBindValue.bindValue = 'DENY';
+ assert.isTrue(element.rule!.value!.modified);
+ element.editing = false;
+ assert.equal(
+ getComputedStyle(queryAndAssert<GrButton>(element, '#removeBtn'))
+ .display,
+ 'none'
+ );
+ assert.deepEqual(element._originalRuleValues, element.rule!.value);
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#action').bindValue,
+ 'ALLOW'
+ );
+ assert.isNotOk(element.rule!.value!.modified);
+ });
+
+ test('modify value', () => {
+ assert.isNotOk(element.rule!.value!.modified);
+ const actionBindValue = queryAndAssert<GrSelect>(element, '#action');
+ actionBindValue.bindValue = 'DENY';
+ flush();
+ assert.isTrue(element.rule!.value!.modified);
+
+ // The original value should now differ from the rule values.
+ assert.notDeepEqual(element._originalRuleValues, element.rule!.value);
+ });
+
+ test('all selects are disabled when not in edit mode', () => {
+ const selects = queryAll<HTMLSelectElement>(element, 'select');
+ for (const select of selects) {
+ assert.isTrue(select.disabled);
+ }
+ element.editing = true;
+ for (const select of selects) {
+ assert.isFalse(select.disabled);
+ }
+ });
+
+ test('remove rule and undo remove', () => {
+ element.editing = true;
+ element.rule = {value: {action: 'ALLOW'}};
+ assert.isFalse(
+ queryAndAssert<HTMLDivElement>(
+ element,
+ '#deletedContainer'
+ ).classList.contains('deleted')
+ );
+ MockInteractions.tap(queryAndAssert<GrButton>(element, '#removeBtn'));
+ assert.isTrue(
+ queryAndAssert<HTMLDivElement>(
+ element,
+ '#deletedContainer'
+ ).classList.contains('deleted')
+ );
+ assert.isTrue(element._deleted);
+ assert.isTrue(element.rule!.value!.deleted);
+
+ MockInteractions.tap(queryAndAssert<GrButton>(element, '#undoRemoveBtn'));
+ assert.isFalse(element._deleted);
+ assert.isNotOk(element.rule!.value!.deleted);
+ });
+
+ test('remove rule and cancel', () => {
+ element.editing = true;
+ assert.notEqual(
+ getComputedStyle(queryAndAssert<GrButton>(element, '#removeBtn'))
+ .display,
+ 'none'
+ );
+ assert.equal(
+ getComputedStyle(
+ queryAndAssert<HTMLDivElement>(element, '#deletedContainer')
+ ).display,
+ 'none'
+ );
+
+ element.rule = {value: {action: 'ALLOW'}};
+ MockInteractions.tap(queryAndAssert<GrButton>(element, '#removeBtn'));
+ assert.notEqual(
+ getComputedStyle(queryAndAssert<GrButton>(element, '#removeBtn'))
+ .display,
+ 'none'
+ );
+ assert.notEqual(
+ getComputedStyle(
+ queryAndAssert<HTMLDivElement>(element, '#deletedContainer')
+ ).display,
+ 'none'
+ );
+ assert.isTrue(element._deleted);
+ assert.isTrue(element.rule!.value!.deleted);
+
+ element.editing = false;
+ assert.isFalse(element._deleted);
+ assert.isNotOk(element.rule!.value!.deleted);
+ assert.isNotOk(element.rule!.value!.modified);
+
+ assert.deepEqual(element._originalRuleValues, element.rule!.value);
+ assert.equal(
+ getComputedStyle(queryAndAssert<GrButton>(element, '#removeBtn'))
+ .display,
+ 'none'
+ );
+ assert.equal(
+ getComputedStyle(
+ queryAndAssert<HTMLDivElement>(element, '#deletedContainer')
+ ).display,
+ 'none'
+ );
+ });
+
+ test('_computeGroupPath', () => {
+ const group = '123';
+ assert.equal(element._computeGroupPath(group), '/admin/groups/123');
+ });
+ });
+
+ suite('new edit rule', () => {
+ setup(async () => {
+ element.groupName = 'Group Name';
+ element.permission = 'editTopicName' as AccessPermissionId;
+ element.rule = {};
+ element.section = 'refs/*';
+ element._setupValues(element.rule!);
+ await flush();
+ element.rule!.value!.added = true;
+ await flush();
+ element.connectedCallback();
+ });
+
+ test('_ruleValues and _originalRuleValues are set correctly', () => {
+ // Since the element does not already have default values, they should
+ // be set. The original values should be set to those too.
+ assert.isNotOk(element.rule!.value!.modified);
+ const expectedRuleValue = {
+ action: 'ALLOW',
+ force: false,
+ added: true,
+ };
+ assert.deepEqual(element.rule!.value, expectedRuleValue);
+ test('values are set correctly', () => {
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#action').bindValue,
+ expectedRuleValue.action
+ );
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#force').bindValue,
+ expectedRuleValue.action
+ );
+ });
+ });
+
+ test('modify value', () => {
+ assert.isNotOk(element.rule!.value!.modified);
+ const forceBindValue = queryAndAssert<GrSelect>(element, '#force');
+ forceBindValue.bindValue = true;
+ flush();
+ assert.isTrue(element.rule!.value!.modified);
+
+ // The original value should now differ from the rule values.
+ assert.notDeepEqual(element._originalRuleValues, element.rule!.value);
+ });
+
+ test('remove value', () => {
+ element.editing = true;
+ const removeStub = sinon.stub();
+ element.addEventListener('added-rule-removed', removeStub);
+ MockInteractions.tap(queryAndAssert<GrButton>(element, '#removeBtn'));
+ flush();
+ assert.isTrue(removeStub.called);
+ });
+ });
+
+ suite('already existing rule with labels', () => {
+ setup(async () => {
+ element.label = {
+ values: [
+ {value: -2, text: 'This shall not be merged'},
+ {value: -1, text: 'I would prefer this is not merged as is'},
+ {value: -0, text: 'No score'},
+ {value: 1, text: 'Looks good to me, but someone else must approve'},
+ {value: 2, text: 'Looks good to me, approved'},
+ ],
+ };
+ element.groupName = 'Group Name';
+ element.permission = 'label-Code-Review' as AccessPermissionId;
+ element.rule = {
+ value: {
+ action: 'ALLOW',
+ force: false,
+ max: 2,
+ min: -2,
+ },
+ };
+ element.section = 'refs/*';
+ element._setupValues(element.rule);
+ await flush();
+ element.connectedCallback();
+ });
+
+ test('_ruleValues and _originalRuleValues are set correctly', () => {
+ assert.deepEqual(element._originalRuleValues, element.rule!.value);
+ });
+
+ test('values are set correctly', () => {
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#action').bindValue,
+ element.rule!.value!.action
+ );
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#labelMin').bindValue,
+ element.rule!.value!.min
+ );
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#labelMax').bindValue,
+ element.rule!.value!.max
+ );
+ assert.isFalse(
+ queryAndAssert<GrSelect>(element, '#force').classList.contains('force')
+ );
+ });
+
+ test('modify value', () => {
+ const removeStub = sinon.stub();
+ element.addEventListener('added-rule-removed', removeStub);
+ assert.isNotOk(element.rule!.value!.modified);
+ const labelMinBindValue = queryAndAssert<GrSelect>(element, '#labelMin');
+ labelMinBindValue.bindValue = 1;
+ flush();
+ assert.isTrue(element.rule!.value!.modified);
+ assert.isFalse(removeStub.called);
+
+ // The original value should now differ from the rule values.
+ assert.notDeepEqual(element._originalRuleValues, element.rule!.value);
+ });
+ });
+
+ suite('new rule with labels', () => {
+ let setDefaultRuleValuesSpy: sinon.SinonSpy;
+
+ setup(async () => {
+ setDefaultRuleValuesSpy = sinon.spy(element, '_setDefaultRuleValues');
+ element.label = {
+ values: [
+ {value: -2, text: 'This shall not be merged'},
+ {value: -1, text: 'I would prefer this is not merged as is'},
+ {value: -0, text: 'No score'},
+ {value: 1, text: 'Looks good to me, but someone else must approve'},
+ {value: 2, text: 'Looks good to me, approved'},
+ ],
+ };
+ element.groupName = 'Group Name';
+ element.permission = 'label-Code-Review' as AccessPermissionId;
+ element.rule = {};
+ element.section = 'refs/*';
+ element._setupValues(element.rule!);
+ await flush();
+ element.rule!.value!.added = true;
+ await flush();
+ element.connectedCallback();
+ });
+
+ test('_ruleValues and _originalRuleValues are set correctly', () => {
+ // Since the element does not already have default values, they should
+ // be set. The original values should be set to those too.
+ assert.isNotOk(element.rule!.value!.modified);
+ assert.isTrue(setDefaultRuleValuesSpy.called);
+
+ const expectedRuleValue = {
+ max: element.label!.values![element.label!.values.length - 1].value,
+ min: element.label!.values![0].value,
+ action: 'ALLOW',
+ added: true,
+ };
+ assert.deepEqual(element.rule!.value, expectedRuleValue);
+ test('values are set correctly', () => {
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#action').bindValue,
+ expectedRuleValue.action
+ );
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#labelMin').bindValue,
+ expectedRuleValue.min
+ );
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#labelMax').bindValue,
+ expectedRuleValue.max
+ );
+ });
+ });
+
+ test('modify value', () => {
+ assert.isNotOk(element.rule!.value!.modified);
+ const labelMinBindValue = queryAndAssert<GrSelect>(element, '#labelMin');
+ labelMinBindValue.bindValue = 1;
+ flush();
+ assert.isTrue(element.rule!.value!.modified);
+
+ // The original value should now differ from the rule values.
+ assert.notDeepEqual(element._originalRuleValues, element.rule!.value);
+ });
+ });
+
+ suite('already existing push rule', () => {
+ setup(async () => {
+ element.groupName = 'Group Name';
+ element.permission = 'push' as AccessPermissionId;
+ element.rule = {
+ value: {
+ action: 'ALLOW',
+ force: true,
+ },
+ };
+ element.section = 'refs/*';
+ element._setupValues(element.rule!);
+ await flush();
+ element.connectedCallback();
+ });
+
+ test('_ruleValues and _originalRuleValues are set correctly', () => {
+ assert.deepEqual(element._originalRuleValues, element.rule!.value);
+ });
+
+ test('values are set correctly', () => {
+ assert.isTrue(
+ queryAndAssert<GrSelect>(element, '#force').classList.contains('force')
+ );
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#action').bindValue,
+ element.rule!.value!.action
+ );
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#force').bindValue,
+ element.rule!.value!.force
+ );
+ assert.isNotOk(query<GrSelect>(element, '#labelMin'));
+ assert.isNotOk(query<GrSelect>(element, '#labelMax'));
+ });
+
+ test('modify value', () => {
+ assert.isNotOk(element.rule!.value!.modified);
+ const actionBindValue = queryAndAssert<GrSelect>(element, '#action');
+ actionBindValue.bindValue = false;
+ flush();
+ assert.isTrue(element.rule!.value!.modified);
+
+ // The original value should now differ from the rule values.
+ assert.notDeepEqual(element._originalRuleValues, element.rule!.value);
+ });
+ });
+
+ suite('new push rule', () => {
+ setup(async () => {
+ element.groupName = 'Group Name';
+ element.permission = 'push' as AccessPermissionId;
+ element.rule = {};
+ element.section = 'refs/*';
+ element._setupValues(element.rule!);
+ await flush();
+ element.rule!.value!.added = true;
+ await flush();
+ element.connectedCallback();
+ });
+
+ test('_ruleValues and _originalRuleValues are set correctly', () => {
+ // Since the element does not already have default values, they should
+ // be set. The original values should be set to those too.
+ assert.isNotOk(element.rule!.value!.modified);
+ const expectedRuleValue = {
+ action: 'ALLOW',
+ force: false,
+ added: true,
+ };
+ assert.deepEqual(element.rule!.value, expectedRuleValue);
+ test('values are set correctly', () => {
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#action').bindValue,
+ expectedRuleValue.action
+ );
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#force').bindValue,
+ expectedRuleValue.action
+ );
+ });
+ });
+
+ test('modify value', () => {
+ assert.isNotOk(element.rule!.value!.modified);
+ const forceBindValue = queryAndAssert<GrSelect>(element, '#force');
+ forceBindValue.bindValue = true;
+ flush();
+ assert.isTrue(element.rule!.value!.modified);
+
+ // The original value should now differ from the rule values.
+ assert.notDeepEqual(element._originalRuleValues, element.rule!.value);
+ });
+ });
+
+ suite('already existing edit rule', () => {
+ setup(async () => {
+ element.groupName = 'Group Name';
+ element.permission = 'editTopicName' as AccessPermissionId;
+ element.rule = {
+ value: {
+ action: 'ALLOW',
+ force: true,
+ },
+ };
+ element.section = 'refs/*';
+ element._setupValues(element.rule);
+ await flush();
+ element.connectedCallback();
+ });
+
+ test('_ruleValues and _originalRuleValues are set correctly', () => {
+ assert.deepEqual(element._originalRuleValues, element.rule!.value);
+ });
+
+ test('values are set correctly', () => {
+ assert.isTrue(
+ queryAndAssert<GrSelect>(element, '#force').classList.contains('force')
+ );
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#action').bindValue,
+ element.rule!.value!.action
+ );
+ assert.equal(
+ queryAndAssert<GrSelect>(element, '#force').bindValue,
+ element.rule!.value!.force
+ );
+ assert.isNotOk(query<GrSelect>(element, '#labelMin'));
+ assert.isNotOk(query<GrSelect>(element, '#labelMax'));
+ });
+
+ test('modify value', async () => {
+ assert.isNotOk(element.rule!.value!.modified);
+ const actionBindValue = queryAndAssert<GrSelect>(element, '#action');
+ actionBindValue.bindValue = false;
+ await flush();
+ assert.isTrue(element.rule!.value!.modified);
+
+ // The original value should now differ from the rule values.
+ assert.notDeepEqual(element._originalRuleValues, element.rule!.value);
+ });
+ });
+});
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 23363af..af2966c 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -1126,7 +1126,7 @@
private async openDeleteCommentOverlay() {
this.showConfirmDeleteOverlay = true;
await this.updateComplete;
- this.confirmDeleteOverlay?.open();
+ await this.confirmDeleteOverlay?.open();
}
private closeDeleteCommentOverlay() {
