Merge "Remove NewSubmitRequirementsUI flag"
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNoteJsonTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNoteJsonTest.java
new file mode 100644
index 0000000..43153ae
--- /dev/null
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNoteJsonTest.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2022 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.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+
+import com.google.gson.Gson;
+import com.google.inject.TypeLiteral;
+import java.util.Optional;
+import org.junit.Test;
+
+public class ChangeNoteJsonTest {
+ private final Gson gson = new ChangeNoteJson().getGson();
+
+ @Test
+ public void shouldSerializeAndDeserializeEmptyOptional() {
+ // given
+ Optional<?> empty = Optional.empty();
+
+ // when
+ String json = gson.toJson(empty);
+
+ // then
+ assertThat(json).isEqualTo("{}");
+
+ // and when
+ Optional<?> result = gson.fromJson(json, Optional.class);
+
+ // and then
+ assertThat(result).isEmpty();
+ }
+
+ @Test
+ public void shouldSerializeAndDeserializeNonEmptyOptional() {
+ // given
+ String value = "foo";
+ Optional<String> nonEmpty = Optional.of(value);
+
+ // when
+ String json = gson.toJson(nonEmpty);
+
+ // then
+ assertThat(json).isEqualTo("{\n \"value\": \"" + value + "\"\n}");
+
+ // and when
+ Optional<String> result = gson.fromJson(json, new TypeLiteral<Optional<String>>() {}.getType());
+
+ // and then
+ assertThat(result).isPresent();
+ assertThat(result.get()).isEqualTo(value);
+ }
+}
diff --git a/polygerrit-ui/README.md b/polygerrit-ui/README.md
index c5c262b..732a82c 100644
--- a/polygerrit-ui/README.md
+++ b/polygerrit-ui/README.md
@@ -240,34 +240,6 @@
the "Before launch" section for IntelliJ. This is a temporary problem until
typescript migration is complete.
-## Running Templates Test
-The templates test validates polymer templates. The test convert polymer
-templates into a plain typescript code and then run TS compiler. The test fails
-if TS compiler reports errors; in this case you will see TS errors in
-the log/output. Gerrit-CI automatically runs templates test.
-
-**Note**: Files defined in `ignore_templates_list` (`polygerrit-ui/app/BUILD`)
-are excluded from code generation and checking. If you don't know how to fix
-a problem, you can add a problematic template in the list.
-
-* To run test locally, use npm command:
-``` sh
-npm run polytest
-```
-
-* Often, the output from the previous command is not clear (cryptic TS errors).
-In this case, run the command
-```sh
-npm run polytest:dev
-```
-This command (re)creates the `polygerrit-ui/app/tmpl_out` directory and put
-generated files into it. For each polygerrit .ts file there is a generated file
-in the `tmpl_out` directory. If an original file doesn't contain a polymer
-template, the generated file is empty.
-
-You can open a problematic file in IDE and fix the problem. Ensure, that IDE
-uses `polygerrit-ui/app/tsconfig.json` as a project (usually, this is default).
-
### Generated file overview
A generated file starts with imports followed by a static content with
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
index 42ff8f6..34080a1 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
@@ -11,7 +11,6 @@
import {resolve} from '../../../models/dependency';
import {
AccountDetailInfo,
- AccountInfo,
ChangeInfo,
NumericChangeId,
ServerInfo,
@@ -31,10 +30,14 @@
import {allSettled} from '../../../utils/async-util';
import {listForSentence} from '../../../utils/string-util';
import {getDisplayName} from '../../../utils/display-name-util';
-import {AccountInputDetail} from '../../shared/gr-account-list/gr-account-list';
+import {
+ AccountInput,
+ AccountInputDetail,
+} from '../../shared/gr-account-list/gr-account-list';
import '@polymer/iron-icon/iron-icon';
import {getReplyByReason} from '../../../utils/attention-set-util';
import {intersection} from '../../../utils/common-util';
+import {accountOrGroupKey} from '../../../utils/account-util';
@customElement('gr-change-list-reviewer-flow')
export class GrChangeListReviewerFlow extends LitElement {
@@ -43,7 +46,7 @@
// contents are given to gr-account-lists to mutate
@state() private updatedAccountsByReviewerState: Map<
ReviewerState,
- AccountInfo[]
+ AccountInput[]
> = new Map([
[ReviewerState.REVIEWER, []],
[ReviewerState.CC, []],
@@ -254,12 +257,11 @@
.filter(account => account?._account_id !== undefined);
return this.updatedAccountsByReviewerState
.get(updatedReviewerState)!
- .filter(
- account =>
- account._account_id !== undefined &&
- accountsInCurrentState.some(
- otherAccount => otherAccount._account_id === account._account_id
- )
+ .filter(account =>
+ accountsInCurrentState.some(
+ otherAccount =>
+ accountOrGroupKey(otherAccount) === accountOrGroupKey(account)
+ )
)
.map(reviewer => getDisplayName(this.serverConfig, reviewer));
}
@@ -302,7 +304,6 @@
reviewerState: ReviewerState,
event: CustomEvent<AccountInputDetail>
) {
- const account = event.detail.account as AccountInfo;
const oppositeReviewerState =
reviewerState === ReviewerState.CC
? ReviewerState.REVIEWER
@@ -311,7 +312,7 @@
oppositeReviewerState
)!;
const oppositeUpdatedAccountIndex = oppositeUpdatedAccounts.findIndex(
- acc => acc._account_id === account._account_id
+ acc => accountOrGroupKey(acc) === accountOrGroupKey(event.detail.account)
);
if (oppositeUpdatedAccountIndex >= 0) {
oppositeUpdatedAccounts.splice(oppositeUpdatedAccountIndex, 1);
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts
index e34a269..4a142e4 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts
@@ -5,7 +5,7 @@
*/
import {fixture, html} from '@open-wc/testing-helpers';
import {SinonStubbedMember} from 'sinon';
-import {AccountInfo, ReviewerState} from '../../../api/rest-api';
+import {AccountInfo, GroupInfo, ReviewerState} from '../../../api/rest-api';
import {
BulkActionsModel,
bulkActionsModelToken,
@@ -17,6 +17,7 @@
import {
createAccountWithIdNameAndEmail,
createChange,
+ createGroupInfo,
} from '../../../test/test-data-generators';
import {
MockPromise,
@@ -43,6 +44,7 @@
createAccountWithIdNameAndEmail(4),
createAccountWithIdNameAndEmail(5),
];
+const groups: GroupInfo[] = [createGroupInfo('groupId')];
const changes: ChangeInfo[] = [
{
...createChange(),
@@ -225,7 +227,7 @@
dialog,
'gr-account-list#cc-list'
);
- reviewerList.accounts.push(accounts[2]);
+ reviewerList.accounts.push(accounts[2], groups[0]);
ccList.accounts.push(accounts[5]);
await flush();
dialog.confirmButton!.click();
@@ -243,6 +245,7 @@
{
reviewers: [
{reviewer: accounts[2]._account_id, state: ReviewerState.REVIEWER},
+ {reviewer: groups[0].id, state: ReviewerState.REVIEWER},
{reviewer: accounts[5]._account_id, state: ReviewerState.CC},
],
ignore_automatic_attention_set_rules: true,
@@ -252,6 +255,10 @@
reason: '<GERRIT_ACCOUNT_1> replied on the change',
user: accounts[2]._account_id,
},
+ {
+ reason: '<GERRIT_ACCOUNT_1> replied on the change',
+ user: groups[0].id,
+ },
],
},
]);
@@ -261,6 +268,7 @@
{
reviewers: [
{reviewer: accounts[2]._account_id, state: ReviewerState.REVIEWER},
+ {reviewer: groups[0].id, state: ReviewerState.REVIEWER},
{reviewer: accounts[5]._account_id, state: ReviewerState.CC},
],
ignore_automatic_attention_set_rules: true,
@@ -270,6 +278,10 @@
reason: '<GERRIT_ACCOUNT_1> replied on the change',
user: accounts[2]._account_id,
},
+ {
+ reason: '<GERRIT_ACCOUNT_1> replied on the change',
+ user: groups[0].id,
+ },
],
},
]);
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index 1f57837..6a9bd92 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -261,7 +261,7 @@
account?: AccountInfo;
@state()
- ccs: (AccountInfoInput | GroupInfoInput)[] = [];
+ ccs: AccountInput[] = [];
@state()
attentionCcsCount = 0;
@@ -1527,8 +1527,8 @@
if (!this.change?.owner || !this.change?.reviewers) return;
this.owner = this.change.owner;
- const reviewers = [];
- const ccs = [];
+ const reviewers: AccountInput[] = [];
+ const ccs: AccountInput[] = [];
if (this.change.reviewers) {
for (const key of Object.keys(this.change.reviewers)) {
diff --git a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
index c80a1cf..a34f880 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
@@ -9,8 +9,9 @@
NumericChangeId,
ChangeStatus,
ReviewerState,
- AccountInfo,
AccountId,
+ AccountInfo,
+ GroupInfo,
} from '../../api/rest-api';
import {Model} from '../model';
import {Finalizable} from '../../services/registry';
@@ -22,6 +23,7 @@
ReviewerInput,
AttentionSetInput,
} from '../../types/common';
+import {accountOrGroupKey} from '../../utils/account-util';
export const bulkActionsModelToken =
define<BulkActionsModel>('bulk-actions-model');
@@ -159,7 +161,7 @@
}
addReviewers(
- changedReviewers: Map<ReviewerState, AccountInfo[]>,
+ changedReviewers: Map<ReviewerState, (AccountInfo | GroupInfo)[]>,
reason: string
): Promise<Response>[] {
const current = this.subject$.getValue();
@@ -260,14 +262,14 @@
private getNewReviewersToChange(
change: ChangeInfo,
state: ReviewerState,
- changedReviewers: Map<ReviewerState, AccountInfo[]>
+ changedReviewers: Map<ReviewerState, (AccountInfo | GroupInfo)[]>
): ReviewerInput[] {
return (
changedReviewers
.get(state)
?.filter(account => !change.reviewers[state]?.includes(account))
.map(account => {
- return {state, reviewer: account._account_id!};
+ return {state, reviewer: accountOrGroupKey(account)};
}) ?? []
);
}
diff --git a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
index aaf7123..84d5c4e 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
@@ -7,6 +7,7 @@
import {
createAccountWithIdNameAndEmail,
createChange,
+ createGroupInfo,
createRevisions,
} from '../../test/test-data-generators';
import {
@@ -18,6 +19,7 @@
AccountInfo,
ReviewerState,
AccountId,
+ GroupInfo,
} from '../../api/rest-api';
import {BulkActionsModel, LoadingState} from './bulk-actions-model';
import {getAppContext} from '../../services/app-context';
@@ -200,6 +202,7 @@
createAccountWithIdNameAndEmail(0),
createAccountWithIdNameAndEmail(1),
];
+ const groups: GroupInfo[] = [createGroupInfo('groupId')];
const changes: ChangeInfo[] = [
{
...createChange(),
@@ -234,20 +237,36 @@
test('adds reviewers/cc only to changes that need it', async () => {
bulkActionsModel.addReviewers(
new Map([
- [ReviewerState.REVIEWER, [accounts[0]]],
+ [ReviewerState.REVIEWER, [accounts[0], groups[0]]],
[ReviewerState.CC, [accounts[1]]],
]),
'<GERRIT_ACCOUNT_12345> replied on the change'
);
- // changes[0] is not updated since it already has the reviewer & CC
- assert.isTrue(saveChangeReviewStub.calledOnce);
+ assert.isTrue(saveChangeReviewStub.calledTwice);
+ // changes[0] only adds the group since it already has the other
+ // reviewer/CCs
assert.sameDeepOrderedMembers(saveChangeReviewStub.firstCall.args, [
+ changes[0]._number,
+ 'current',
+ {
+ reviewers: [{reviewer: groups[0].id, state: ReviewerState.REVIEWER}],
+ ignore_automatic_attention_set_rules: true,
+ add_to_attention_set: [
+ {
+ reason: '<GERRIT_ACCOUNT_12345> replied on the change',
+ user: groups[0].id,
+ },
+ ],
+ },
+ ]);
+ assert.sameDeepOrderedMembers(saveChangeReviewStub.secondCall.args, [
changes[1]._number,
'current',
{
reviewers: [
{reviewer: accounts[0]._account_id, state: ReviewerState.REVIEWER},
+ {reviewer: groups[0].id, state: ReviewerState.REVIEWER},
{reviewer: accounts[1]._account_id, state: ReviewerState.CC},
],
ignore_automatic_attention_set_rules: true,
@@ -256,6 +275,10 @@
reason: '<GERRIT_ACCOUNT_12345> replied on the change',
user: accounts[0]._account_id,
},
+ {
+ reason: '<GERRIT_ACCOUNT_12345> replied on the change',
+ user: groups[0].id,
+ },
],
},
]);
diff --git a/polygerrit-ui/app/services/registry.ts b/polygerrit-ui/app/services/registry.ts
index e7de1ef..74b6997 100644
--- a/polygerrit-ui/app/services/registry.ts
+++ b/polygerrit-ui/app/services/registry.ts
@@ -73,7 +73,7 @@
initializing = true;
initialized.set(name, factory(context));
} catch (e) {
- console.error(`Failed to initialize ${name}`, e);
+ console.error(`Failed to initialize ${String(name)}`, e);
} finally {
initializing = false;
}
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index b7cc77b..b6018ba 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -35,7 +35,7 @@
export const ACCOUNT_TEMPLATE_REGEX = '<GERRIT_ACCOUNT_(\\d+)>';
export function accountKey(account: AccountInfo): AccountId | EmailAddress {
- if (account._account_id) return account._account_id;
+ if (account._account_id !== undefined) return account._account_id;
if (account.email) return account.email;
throw new Error('Account has neither _account_id nor email.');
}
diff --git a/tools/js/template_checker.bzl b/tools/js/template_checker.bzl
index da77234..6c645f4 100644
--- a/tools/js/template_checker.bzl
+++ b/tools/js/template_checker.bzl
@@ -123,9 +123,7 @@
)
# Pack all transformed files. Later files can be materialized in the
- # WORKSPACE/polygerrit-ui/app/tmpl_out dir. The following command do it
- # automatically
- # npm run polytest:dev
+ # WORKSPACE/polygerrit-ui/app/tmpl_out dir.
pkg_tar(
name = name + "_tar",
srcs = generated_dev_files,
