Merge "Fix current patchset not updated after Edit Publish"
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
index 52e0b3f..d00fa40 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
@@ -20,6 +20,7 @@
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrAccessSection} from '../gr-access-section/gr-access-section';
import {
+ AutocompleteCommitEvent,
AutocompleteQuery,
AutocompleteSuggestion,
} from '../../shared/gr-autocomplete/gr-autocomplete';
@@ -194,7 +195,7 @@
id="editInheritFromInput"
.text=${this.inheritFromFilter}
.query=${this.query}
- @commit=${(e: ValueChangedEvent) => {
+ @commit=${(e: AutocompleteCommitEvent) => {
this.handleUpdateInheritFrom(e);
}}
@bind-value-changed=${(e: ValueChangedEvent) => {
@@ -388,7 +389,7 @@
}
// private but used in test
- handleUpdateInheritFrom(e: ValueChangedEvent) {
+ handleUpdateInheritFrom(e: AutocompleteCommitEvent) {
this.inheritsFrom = {
...(this.inheritsFrom ?? {}),
id: e.detail.value as UrlEncodedRepoName,
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts
index 11de37e..df63780 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts
@@ -56,7 +56,7 @@
<dialog id="actionModal" tabindex="-1">
<gr-dialog
.disableCancel=${!this.isCancelEnabled()}
- .disabled=${!this.isConfirmEnabled()}
+ .disabled=${this.isDisabled()}
@confirm=${() => this.handleConfirm()}
@cancel=${() => this.handleClose()}
.cancelLabel=${'Close'}
@@ -104,13 +104,13 @@
);
}
- private isConfirmEnabled() {
+ private isDisabled() {
// Action is allowed if none of the changes have any bulk action performed
// on them. In case an error happens then we keep the button disabled.
for (const status of this.progress.values()) {
- if (status !== ProgressStatus.NOT_STARTED) return false;
+ if (status !== ProgressStatus.NOT_STARTED) return true;
}
- return true;
+ return false;
}
private isCancelEnabled() {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
index a9b8028..db82523 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
@@ -161,7 +161,9 @@
<dialog id="actionModal" tabindex="-1">
<gr-dialog
.disableCancel=${!this.isCancelEnabled()}
- .disabled=${!this.isConfirmEnabled()}
+ .disabled=${this.isDisabled(
+ triggerLabels.length + nonTriggerLabels.length
+ )}
?loading=${this.isLoading()}
.loadingLabel=${'Voting in progress...'}
@confirm=${() => this.handleConfirm()}
@@ -289,11 +291,12 @@
return getOverallStatus(this.progressByChange) === ProgressStatus.RUNNING;
}
- private isConfirmEnabled() {
+ private isDisabled(permittedLabelsCount: number) {
// Action is allowed if none of the changes have any bulk action performed
// on them. In case an error happens then we keep the button disabled.
- return (
- getOverallStatus(this.progressByChange) === ProgressStatus.NOT_STARTED
+ return !(
+ getOverallStatus(this.progressByChange) === ProgressStatus.NOT_STARTED &&
+ permittedLabelsCount > 0
);
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts
index 654ed91..8a5bf47 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts
@@ -307,17 +307,18 @@
);
// No common label with change1 so button is disabled
- change2.labels = {
+ const c2 = {...change2}; // create copy so other tests are not affected
+ c2.labels = {
x: {value: null} as LabelInfo,
y: {value: null} as LabelInfo,
z: {value: null} as LabelInfo,
};
- change2.submit_requirements = [
+ c2.submit_requirements = [
createSubmitRequirementResultInfo('label:x=MAX'),
createSubmitRequirementResultInfo('label:y=MAX'),
createSubmitRequirementResultInfo('label:z=MAX'),
];
- changes.push({...change2});
+ changes.push({...c2});
getChangesStub.restore();
getChangesStub.returns(Promise.resolve(changes));
model.sync(changes);
@@ -484,6 +485,45 @@
assert.equal(dispatchEventStub.lastCall.args[0].type, 'reload');
});
+ test('button is disabled if no votes are possible', async () => {
+ const c2 = {...change2}; // create copy so other tests are not affected
+ c2.labels = {
+ x: {value: null} as LabelInfo,
+ y: {value: null} as LabelInfo,
+ z: {value: null} as LabelInfo,
+ };
+ c2.submit_requirements = [
+ createSubmitRequirementResultInfo('label:x=MAX'),
+ createSubmitRequirementResultInfo('label:y=MAX'),
+ createSubmitRequirementResultInfo('label:z=MAX'),
+ ];
+
+ const changes: ChangeInfo[] = [change1, c2];
+ getChangesStub.returns(Promise.resolve(changes));
+
+ stubRestApi('saveChangeReview').callsFake(
+ (_changeNum, _patchNum, _review, errFn) =>
+ Promise.resolve(new Response()).then(res => {
+ errFn && errFn();
+ return res;
+ })
+ );
+
+ model.sync(changes);
+ await waitUntilObserved(
+ model.loadingState$,
+ state => state === LoadingState.LOADED
+ );
+ await selectChange(change1);
+ await selectChange(c2);
+ await element.updateComplete;
+
+ assert.isTrue(
+ queryAndAssert<GrButton>(query(element, 'gr-dialog'), '#confirm')
+ .disabled
+ );
+ });
+
test('closing dialog does not trigger reload if no request made', async () => {
const changes: ChangeInfo[] = [change1, change2];
getChangesStub.returns(Promise.resolve(changes));
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 5792230..f3be5c4 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -329,9 +329,9 @@
private computeIsExpandable() {
const hasSummary = !!this.result?.summary;
const hasMessage = !!this.result?.message;
- const hasLinks = (this.result?.links ?? []).length > 0;
+ const hasMultipleLinks = (this.result?.links ?? []).length > 1;
const hasPointers = (this.result?.codePointers ?? []).length > 0;
- return hasSummary && (hasMessage || hasLinks || hasPointers);
+ return hasSummary && (hasMessage || hasMultipleLinks || hasPointers);
}
override focus() {
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts b/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts
index 113470c..385bde7 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts
@@ -117,6 +117,7 @@
aria-checked="false"
aria-label="Expand result row"
class="show-hide"
+ hidden
role="switch"
tabindex="0"
>
@@ -261,7 +262,6 @@
</h3>
<gr-result-row
class="FAKEErrorFinderFinderFinderFinderFinderFinderFinder"
- isexpandable
>
</gr-result-row>
<gr-result-row
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
index 314e126..6d8dc2b 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
@@ -7,6 +7,7 @@
import '../../shared/gr-icon/gr-icon';
import {ServerInfo} from '../../../types/common';
import {
+ AutocompleteCommitEvent,
AutocompleteQuery,
AutocompleteSuggestion,
GrAutocomplete,
@@ -227,7 +228,7 @@
.threshold=${this.threshold}
tab-complete
.verticalOffset=${30}
- @commit=${(e: Event) => {
+ @commit=${(e: AutocompleteCommitEvent) => {
this.handleInputCommit(e);
}}
@text-changed=${(e: CustomEvent) => {
@@ -285,7 +286,7 @@
return `${baseUrl}/user-search.html`;
}
- private handleInputCommit(e: Event) {
+ private handleInputCommit(e: AutocompleteCommitEvent) {
this.preventDefaultAndNavigateToInputVal(e);
}
@@ -295,7 +296,7 @@
* - e.target is the gr-autocomplete widget (#searchInput)
* - e.target is the input element wrapped within #searchInput
*/
- private preventDefaultAndNavigateToInputVal(e: Event) {
+ private preventDefaultAndNavigateToInputVal(e: AutocompleteCommitEvent) {
e.preventDefault();
if (!this.inputVal) return;
const trimmedInput = this.inputVal.trim();
diff --git a/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts b/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts
index 0509925..08f7c93 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts
@@ -5,6 +5,7 @@
*/
import '../gr-autocomplete/gr-autocomplete';
import {
+ AutocompleteCommitEvent,
AutocompleteQuery,
GrAutocomplete,
} from '../gr-autocomplete/gr-autocomplete';
@@ -110,7 +111,7 @@
return this.input!.text;
}
- private handleInputCommit(e: CustomEvent) {
+ private handleInputCommit(e: AutocompleteCommitEvent) {
this.dispatchEvent(
new CustomEvent('add', {
detail: {value: e.detail.value},
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
index 65d8859..9034259 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
@@ -38,18 +38,17 @@
import {ReviewerState} from '../../../api/rest-api';
const VALID_EMAIL_ALERT = 'Please input a valid email.';
+const VALID_USER_GROUP_ALERT = 'Please input a valid user or group.';
declare global {
interface HTMLElementEventMap {
'accounts-changed': ValueChangedEvent<(AccountInfo | GroupInfo)[]>;
'pending-confirmation-changed': ValueChangedEvent<SuggestedReviewerGroupInfo | null>;
+ 'account-added': CustomEvent<{account: AccountInfo | GroupInfo}>;
}
interface HTMLElementTagNameMap {
'gr-account-list': GrAccountList;
}
- interface HTMLElementEventMap {
- 'account-added': CustomEvent<AccountInputDetail>;
- }
}
export interface AccountInputDetail {
account: AccountInput;
@@ -264,24 +263,28 @@
this.addAccountItem(item);
}
- addAccountItem(item: RawAccountInput) {
+ /**
+ * Check if account or group is valid and add it.
+ *
+ * @return true if account or group is added.
+ */
+ addAccountItem(item: RawAccountInput): boolean {
// Append new account or group to the accounts property. We add our own
// internal properties to the account/group here, so we clone the object
// to avoid cluttering up the shared change object.
- let account;
- let group;
+ let accountOrGroup: AccountInfo | GroupInfo | undefined;
let itemTypeAdded = 'unknown';
if (isAccountObject(item)) {
- account = {...item.account};
- this.accounts.push(account);
+ accountOrGroup = {...item.account};
+ this.accounts.push(accountOrGroup);
itemTypeAdded = 'account';
} else if (isSuggestedReviewerGroupInfo(item)) {
if (item.confirm) {
this.pendingConfirmation = item;
- return;
+ return false;
}
- group = {...item.group};
- this.accounts.push(group);
+ accountOrGroup = {...item.group};
+ this.accounts.push(accountOrGroup);
itemTypeAdded = 'group';
} else if (this.allowAnyInput) {
if (!item.includes('@')) {
@@ -291,13 +294,17 @@
fireAlert(this, VALID_EMAIL_ALERT);
return false;
} else {
- account = {email: item as EmailAddress};
- this.accounts.push(account);
+ accountOrGroup = {email: item as EmailAddress};
+ this.accounts.push(accountOrGroup);
itemTypeAdded = 'email';
}
}
+ if (!accountOrGroup) {
+ fireAlert(this, VALID_USER_GROUP_ALERT);
+ return false;
+ }
fire(this, 'accounts-changed', {value: this.accounts.slice()});
- fire(this, 'account-added', {account: (account ?? group)! as AccountInput});
+ fire(this, 'account-added', {account: accountOrGroup});
this.reporting.reportInteraction(`Add to ${this.id}`, {itemTypeAdded});
this.pendingConfirmation = null;
this.requestUpdate();
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
index cc2723e..814b623 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
@@ -289,6 +289,15 @@
assert.isFalse(element.computeRemovable(newAccount));
});
+ test('addAccountItem with invalid item', () => {
+ const toastHandler = sinon.stub();
+ element.allowAnyInput = false;
+ element.addEventListener(EventType.SHOW_ALERT, toastHandler);
+ const result = element.addAccountItem('test');
+ assert.isFalse(result);
+ assert.isTrue(toastHandler.called);
+ });
+
test('submitEntryText', async () => {
element.allowAnyInput = true;
await element.updateComplete;
diff --git a/polygerrit-ui/app/utils/async-util.ts b/polygerrit-ui/app/utils/async-util.ts
index 905b16c..6ac3211 100644
--- a/polygerrit-ui/app/utils/async-util.ts
+++ b/polygerrit-ui/app/utils/async-util.ts
@@ -75,15 +75,18 @@
let callbackResult;
if (this.callback) callbackResult = this.callback();
if (callbackResult instanceof Promise) {
- callbackResult.finally(() =>
- this.resolvePromise!(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED)
- );
+ callbackResult.finally(() => {
+ this.resolvePromise!(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED);
+ });
} else {
this.resolvePromise!(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED);
}
}
- constructor(private callback: () => void | Promise<void>, waitMs = 0) {
+ constructor(
+ private readonly callback: () => void | Promise<void>,
+ waitMs = 0
+ ) {
this.promise = new Promise(resolve => {
this.resolvePromise = resolve;
this.timerId = window.setTimeout(() => {
