Merge changes I70ca4eba,I4bd23d2f
* changes:
Add a metric to count rebase requests
Add a metric to measure the success of rebasing on behalf of the uploader
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 75d8847..2810d1e 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6709,6 +6709,7 @@
|`patch` |required|
The patch to be applied. Must be compatible with `git diff` output.
For example, link:#get-patch[Get Patch] output.
+The patch must be provided as UTF-8 text, either directly or base64-encoded.
|=================================
[[applypatchpatchset-input]]
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index a149f29..36bc3c4 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -67,6 +67,7 @@
import com.google.gerrit.server.util.ReplicaUtil;
import com.google.gerrit.server.util.SocketUtil;
import com.google.gerrit.server.util.SystemLog;
+import com.google.gerrit.testing.FakeAccountPatchReviewStore.FakeAccountPatchReviewStoreModule;
import com.google.gerrit.testing.FakeEmailSender.FakeEmailSenderModule;
import com.google.gerrit.testing.InMemoryRepositoryManager;
import com.google.gerrit.testing.SshMode;
@@ -414,6 +415,7 @@
}
},
site);
+ daemon.setAccountPatchReviewStoreModuleForTesting(new FakeAccountPatchReviewStoreModule());
daemon.setEmailModuleForTesting(new FakeEmailSenderModule());
daemon.setAuditEventModuleForTesting(
MoreObjects.firstNonNull(testAuditModule, new FakeGroupAuditServiceModule()));
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 845cc9a..744f91b 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -215,6 +215,7 @@
private Path runFile;
private boolean inMemoryTest;
private AbstractModule indexModule;
+ private Module accountPatchReviewStoreModule;
private Module emailModule;
private List<Module> testSysModules = new ArrayList<>();
private List<Module> testSshModules = new ArrayList<>();
@@ -333,6 +334,11 @@
}
@VisibleForTesting
+ public void setAccountPatchReviewStoreModuleForTesting(Module module) {
+ accountPatchReviewStoreModule = module;
+ }
+
+ @VisibleForTesting
public void setEmailModuleForTesting(Module module) {
emailModule = module;
}
@@ -442,7 +448,11 @@
modules.add(new WorkQueueModule());
modules.add(new StreamEventsApiListenerModule());
modules.add(new EventBrokerModule());
- modules.add(new JdbcAccountPatchReviewStoreModule(config));
+ if (accountPatchReviewStoreModule != null) {
+ modules.add(accountPatchReviewStoreModule);
+ } else {
+ modules.add(new JdbcAccountPatchReviewStoreModule(config));
+ }
modules.add(new SysExecutorModule());
modules.add(new DiffExecutorModule());
modules.add(new MimeUtil2Module());
diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD
index 62da2f2..dd0ec78d 100644
--- a/java/com/google/gerrit/server/restapi/BUILD
+++ b/java/com/google/gerrit/server/restapi/BUILD
@@ -34,6 +34,7 @@
"//lib/auto:auto-factory",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
+ "//lib/commons:codec",
"//lib/commons:compress",
"//lib/commons:lang3",
"//lib/errorprone:annotations",
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java b/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
index d4f549a..4021f77 100644
--- a/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
@@ -23,6 +23,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
+import org.apache.commons.codec.binary.Base64;
import org.eclipse.jgit.api.errors.PatchApplyException;
import org.eclipse.jgit.api.errors.PatchFormatException;
import org.eclipse.jgit.lib.ObjectId;
@@ -51,8 +52,12 @@
throws IOException, RestApiException {
checkNotNull(mergeTip);
RevTree tip = mergeTip.getTree();
- InputStream patchStream =
- new ByteArrayInputStream(input.patch.getBytes(StandardCharsets.UTF_8));
+ InputStream patchStream;
+ if (Base64.isBase64(input.patch)) {
+ patchStream = new ByteArrayInputStream(org.eclipse.jgit.util.Base64.decode(input.patch));
+ } else {
+ patchStream = new ByteArrayInputStream(input.patch.getBytes(StandardCharsets.UTF_8));
+ }
try {
PatchApplier applier = new PatchApplier(repo, tip, oi);
PatchApplier.Result applyResult = applier.applyPatch(patchStream);
diff --git a/java/com/google/gerrit/testing/BUILD b/java/com/google/gerrit/testing/BUILD
index fb9e64e..81a6443 100644
--- a/java/com/google/gerrit/testing/BUILD
+++ b/java/com/google/gerrit/testing/BUILD
@@ -51,6 +51,7 @@
"//lib:junit",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
+ "//lib/errorprone:annotations",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-servlet",
diff --git a/java/com/google/gerrit/testing/FakeAccountPatchReviewStore.java b/java/com/google/gerrit/testing/FakeAccountPatchReviewStore.java
new file mode 100644
index 0000000..1533aeb
--- /dev/null
+++ b/java/com/google/gerrit/testing/FakeAccountPatchReviewStore.java
@@ -0,0 +1,141 @@
+// Copyright (C) 2023 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.testing;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.server.change.AccountPatchReviewStore;
+import com.google.inject.Singleton;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+
+/**
+ * An implementation of the {@link AccountPatchReviewStore} that's only used in tests. This
+ * implementation stores reviewed files in memory.
+ */
+@Singleton
+public class FakeAccountPatchReviewStore implements AccountPatchReviewStore, LifecycleListener {
+
+ private final Set<Entity> store = new HashSet<>();
+
+ @Override
+ public void start() {}
+
+ @Override
+ public void stop() {}
+
+ public static class FakeAccountPatchReviewStoreModule extends LifecycleModule {
+ @Override
+ protected void configure() {
+ DynamicItem.bind(binder(), AccountPatchReviewStore.class)
+ .to(FakeAccountPatchReviewStore.class);
+ listener().to(FakeAccountPatchReviewStore.class);
+ }
+ }
+
+ @AutoValue
+ abstract static class Entity {
+ abstract PatchSet.Id psId();
+
+ abstract Account.Id accountId();
+
+ abstract String path();
+
+ static Entity create(PatchSet.Id psId, Account.Id accountId, String path) {
+ return new AutoValue_FakeAccountPatchReviewStore_Entity(psId, accountId, path);
+ }
+ }
+
+ @Override
+ @CanIgnoreReturnValue
+ public boolean markReviewed(PatchSet.Id psId, Account.Id accountId, String path) {
+ synchronized (store) {
+ Entity entity = Entity.create(psId, accountId, path);
+ return store.add(entity);
+ }
+ }
+
+ @Override
+ public void markReviewed(PatchSet.Id psId, Account.Id accountId, Collection<String> paths) {
+ paths.forEach(path -> markReviewed(psId, accountId, path));
+ }
+
+ @Override
+ public void clearReviewed(PatchSet.Id psId, Account.Id accountId, String path) {
+ synchronized (store) {
+ store.remove(Entity.create(psId, accountId, path));
+ }
+ }
+
+ @Override
+ public void clearReviewed(PatchSet.Id psId) {
+ synchronized (store) {
+ List<Entity> toRemove = new ArrayList<>();
+ for (Entity entity : store) {
+ if (entity.psId().equals(psId)) {
+ toRemove.add(entity);
+ }
+ }
+ store.removeAll(toRemove);
+ }
+ }
+
+ @Override
+ public void clearReviewed(Change.Id changeId) {
+ synchronized (store) {
+ List<Entity> toRemove = new ArrayList<>();
+ for (Entity entity : store) {
+ if (entity.psId().changeId().equals(changeId)) {
+ toRemove.add(entity);
+ }
+ }
+ store.removeAll(toRemove);
+ }
+ }
+
+ @Override
+ public Optional<PatchSetWithReviewedFiles> findReviewed(PatchSet.Id psId, Account.Id accountId) {
+ synchronized (store) {
+ int matchedPsNumber = -1;
+ Optional<PatchSetWithReviewedFiles> result = Optional.empty();
+ for (Entity entity : store) {
+ if (entity.accountId() != accountId || !entity.psId().changeId().equals(psId.changeId())) {
+ continue;
+ }
+ int entityPsNumber = Integer.parseInt(entity.psId().getId());
+ if (entityPsNumber <= psId.get() && entityPsNumber > matchedPsNumber) {
+ matchedPsNumber = entityPsNumber;
+ result =
+ Optional.of(
+ PatchSetWithReviewedFiles.create(
+ PatchSet.id(psId.changeId(), matchedPsNumber),
+ ImmutableSet.of(entity.path())));
+ }
+ }
+ return result;
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
index 898e1ff..0b55563 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
@@ -213,6 +213,29 @@
}
@Test
+ public void applyGerritBasedPatchUsingRestWithEncodedPatch_success() throws Exception {
+ String head = getHead(repo(), HEAD).name();
+ createBranchWithRevision(BranchNameKey.create(project, "branch"), head);
+ PushOneCommit.Result baseCommit = createChange("Add file", ADDED_FILE_NAME, ADDED_FILE_CONTENT);
+ baseCommit.assertOkStatus();
+ createBranchWithRevision(BranchNameKey.create(project, DESTINATION_BRANCH), head);
+ RestResponse patchResp =
+ userRestSession.get("/changes/" + baseCommit.getChangeId() + "/revisions/current/patch");
+ patchResp.assertOK();
+ String originalEncodedPatch = patchResp.getEntityContent();
+ String originalDecodedPatch = new String(Base64.decode(patchResp.getEntityContent()), UTF_8);
+ ApplyPatchPatchSetInput in = buildInput(originalEncodedPatch);
+ PushOneCommit.Result destChange = createChange();
+
+ RestResponse resp =
+ adminRestSession.post("/changes/" + destChange.getChangeId() + "/patch:apply", in);
+
+ resp.assertOK();
+ BinaryResult resultPatch = gApi.changes().id(destChange.getChangeId()).current().patch();
+ assertThat(removeHeader(resultPatch)).isEqualTo(removeHeader(originalDecodedPatch));
+ }
+
+ @Test
public void applyPatchWithConflict_fails() throws Exception {
initBaseWithFile(MODIFIED_FILE_NAME, "Unexpected base content");
ApplyPatchPatchSetInput in = buildInput(MODIFIED_FILE_DIFF);
@@ -404,6 +427,6 @@
}
private String removeHeader(String s) {
- return s.substring(s.indexOf("\ndiff --git"), s.length() - 1);
+ return s.substring(s.lastIndexOf("\ndiff --git"), s.length() - 1);
}
}
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/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 721d650..2798bb9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -1193,7 +1193,6 @@
<gr-download-dialog
id="downloadDialog"
.change=${this.change}
- .patchNum=${this.patchRange?.patchNum}
.config=${this.serverConfig?.download}
@close=${this.handleDownloadDialogClose}
></gr-download-dialog>
@@ -1244,7 +1243,6 @@
<div class="changeStatuses">
${this.changeStatuses.map(
status => html` <gr-change-status
- .change=${this.change}
.revertedChange=${this.revertedChange}
.status=${status}
.resolveWeblinks=${resolveWeblinks}
@@ -1535,8 +1533,6 @@
.editMode=${this.getEditMode()}
.loggedIn=${this.loggedIn}
.shownFileCount=${this.shownFileCount}
- .patchNum=${this.patchRange?.patchNum}
- .basePatchNum=${this.patchRange?.basePatchNum}
.filesExpanded=${this.fileList?.filesExpanded}
@open-diff-prefs=${this.handleOpenDiffPrefs}
@open-download-dialog=${this.handleOpenDownloadDialog}
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
index c2739f3..ad2ba8f 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
@@ -22,7 +22,6 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {ValueChangedEvent} from '../../../types/events';
import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
-import {KnownExperimentId} from '../../../services/flags/flags';
export interface RebaseChange {
name: string;
@@ -99,8 +98,6 @@
private readonly restApiService = getAppContext().restApiService;
- private readonly flagsService = getAppContext().flagsService;
-
constructor() {
super();
this.query = input => this.getChangeSuggestions(input);
@@ -234,8 +231,7 @@
>
</div>
${when(
- this.flagsService.isEnabled(KnownExperimentId.REBASE_CHAIN) &&
- this.hasParent,
+ this.hasParent,
() =>
html`<div>
<input
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
index d291ebb..7efea5f 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
@@ -17,6 +17,9 @@
import {assertIsDefined} from '../../../utils/common-util';
import {BindValueChangeEvent} from '../../../types/events';
import {ShortcutController} from '../../lit/shortcut-controller';
+import {subscribe} from '../../lit/subscription-controller';
+import {resolve} from '../../../models/dependency';
+import {changeModelToken} from '../../../models/change/change-model';
@customElement('gr-download-dialog')
export class GrDownloadDialog extends LitElement {
@@ -38,15 +41,21 @@
@property({type: Object})
config?: DownloadInfo;
- @property({type: String})
- patchNum: PatchSetNum | undefined;
+ @state() patchNum?: PatchSetNum;
@state() private selectedScheme?: string;
private readonly shortcuts = new ShortcutController(this);
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
constructor() {
super();
+ subscribe(
+ this,
+ () => this.getChangeModel().patchNum$,
+ x => (this.patchNum = x)
+ );
for (const key of ['1', '2', '3', '4', '5']) {
this.shortcuts.addLocal({key}, e => this.handleNumberKey(e));
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index c1e866c..f73dab9 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -40,6 +40,7 @@
import {configModelToken} from '../../../models/config/config-model';
import {createChangeUrl} from '../../../models/views/change';
import {userModelToken} from '../../../models/user/user-model';
+import {changeModelToken} from '../../../models/change/change-model';
@customElement('gr-file-list-header')
export class GrFileListHeader extends LitElement {
@@ -84,14 +85,12 @@
shownFileCount = 0;
@property({type: String})
- patchNum?: PatchSetNum;
-
- @property({type: String})
- basePatchNum?: BasePatchSetNum;
-
- @property({type: String})
filesExpanded?: FilesExpandedState;
+ @state() patchNum?: PatchSetNum;
+
+ @state() basePatchNum?: BasePatchSetNum;
+
@state()
diffPrefs?: DiffPreferencesInfo;
@@ -119,6 +118,8 @@
private readonly getNavigation = resolve(this, navigationToken);
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
constructor() {
super();
subscribe(
@@ -136,6 +137,16 @@
this.serverConfig = config;
}
);
+ subscribe(
+ this,
+ () => this.getChangeModel().patchNum$,
+ x => (this.patchNum = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().basePatchNum$,
+ x => (this.basePatchNum = x)
+ );
}
static override styles = [
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
index 873e6ce..3a83fa1 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
@@ -5,7 +5,7 @@
*/
import '../gr-label-score-row/gr-label-score-row';
import '../../../styles/shared-styles';
-import {LitElement, css, html} from 'lit';
+import {LitElement, css, html, nothing} from 'lit';
import {customElement, property} from 'lit/decorators.js';
import {
ChangeInfo,
@@ -107,8 +107,7 @@
label => !this.permittedLabels || this.permittedLabels[label.name]
).length === 0
) {
- return html`<h3 class="heading-4">Trigger Votes</h3>
- <div class="permissionMessage">You don't have permission to vote</div>`;
+ return nothing;
}
return html`<h3 class="heading-4">Trigger Votes</h3>
${this.renderLabels(labels)}`;
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/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
index 1608c22..9d29cea 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -36,7 +36,6 @@
import {GrSyntaxLayerWorker} from '../../../embed/diff/gr-syntax-layer/gr-syntax-layer-worker';
import {highlightServiceToken} from '../../../services/highlight/highlight-service';
import {anyLineTooLong} from '../../../embed/diff/gr-diff/gr-diff-utils';
-import {changeModelToken} from '../../../models/change/change-model';
import {fireReload} from '../../../utils/event-util';
interface FilePreview {
@@ -92,9 +91,6 @@
diffPrefs?: DiffPreferencesInfo;
@state()
- isOwner = false;
-
- @state()
onCloseFixPreviewCallbacks: ((fixapplied: boolean) => void)[] = [];
private readonly restApiService = getAppContext().restApiService;
@@ -103,8 +99,6 @@
private readonly getNavigation = resolve(this, navigationToken);
- private readonly getChangeModel = resolve(this, changeModelToken);
-
private readonly syntaxLayer = new GrSyntaxLayerWorker(
resolve(this, highlightServiceToken),
() => getAppContext().reportingService
@@ -114,11 +108,6 @@
super();
subscribe(
this,
- () => this.getChangeModel().isOwner$,
- x => (this.isOwner = x)
- );
- subscribe(
- this,
() => this.getUserModel().preferences$,
preferences => {
const layers: DiffLayer[] = [this.syntaxLayer];
@@ -341,7 +330,6 @@
private computeTooltip() {
if (!this.change || !this.patchNum) return '';
- if (!this.isOwner) return 'Fix can only be applied by author';
const latestPatchNum =
this.change.revisions[this.change.current_revision]._number;
return latestPatchNum !== this.patchNum
@@ -351,7 +339,6 @@
private computeDisableApplyFixButton() {
if (!this.change || !this.patchNum) return true;
- if (!this.isOwner) return true;
const latestPatchNum =
this.change.revisions[this.change.current_revision]._number;
return this.patchNum !== latestPatchNum || this.isApplyFixLoading;
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
index 9284fb2..a686b20 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
@@ -71,7 +71,6 @@
element.changeNum = change._number;
element.patchNum = change.revisions[change.current_revision]._number;
element.change = change;
- element.isOwner = true;
element.diffPrefs = {
...createDefaultDiffPrefs(),
font_size: 12,
@@ -161,22 +160,8 @@
assert.equal(button.getAttribute('title'), '');
});
- test('apply fix button is disabled for non-author', async () => {
- element.isOwner = false;
- await element.updateComplete;
- await open(TWO_FIXES);
- assert.equal(element.currentFix!.fix_id, 'fix_1');
- assert.equal(element.currentPreviews.length, 2);
- const button = getConfirmButton();
- assert.isTrue(button.hasAttribute('disabled'));
- assert.equal(
- button.getAttribute('title'),
- 'Fix can only be applied by author'
- );
- });
-
test('apply fix button is disabled on older patchset', async () => {
- element.change = {
+ element.change = element.change = {
...createParsedChange(),
revisions: createRevisions(2),
current_revision: getCurrentRevision(0),
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/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts
index 91e601c..661ffa0 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts
@@ -35,6 +35,16 @@
selected: HTMLElement | null;
}
+export enum AutocompleteQueryStatusType {
+ LOADING = 'loading',
+ ERROR = 'error',
+}
+
+export interface AutocompleteQueryStatus {
+ type: AutocompleteQueryStatusType;
+ message: string;
+}
+
@customElement('gr-autocomplete-dropdown')
export class GrAutocompleteDropdown extends LitElement {
/**
@@ -58,8 +68,8 @@
/** If specified a single non-interactable line is shown instead of
* suggestions.
*/
- @property({type: String})
- errorMessage?: String;
+ @property({type: Object})
+ queryStatus?: AutocompleteQueryStatus;
@property({type: Number})
verticalOffset = 0;
@@ -117,10 +127,12 @@
li.selected {
background-color: var(--hover-background-color);
}
- li.query-error {
+ li.query-status {
background-color: var(--disabled-background);
- color: var(--error-foreground);
cursor: default;
+ }
+ li.query-status.error {
+ color: var(--error-foreground);
white-space: pre-wrap;
}
@media only screen and (max-height: 35em) {
@@ -140,7 +152,7 @@
}
private isSuggestionListInteractible() {
- return !this.isHidden && !this.errorMessage;
+ return !this.isHidden && !this.queryStatus;
}
constructor() {
@@ -172,7 +184,8 @@
override updated(changedProperties: PropertyValues) {
if (
changedProperties.has('suggestions') ||
- changedProperties.has('isHidden')
+ changedProperties.has('isHidden') ||
+ changedProperties.has('queryStatus')
) {
if (!this.isHidden) {
this.computeCursorStopsAndRefit();
@@ -180,15 +193,19 @@
}
}
- private renderError() {
+ private renderStatus() {
return html`
<li
tabindex="-1"
- aria-label="autocomplete query error"
- class="query-error"
+ aria-label="autocomplete query status"
+ class="query-status ${this.queryStatus?.type}"
>
- <span>${this.errorMessage}</span>
- <span class="label">ERROR</span>
+ <span>${this.queryStatus?.message}</span>
+ <span class="label"
+ >${this.queryStatus?.type === AutocompleteQueryStatusType.ERROR
+ ? 'ERROR'
+ : ''}</span
+ >
</li>
`;
}
@@ -198,8 +215,8 @@
<div class="dropdown-content" id="suggestions" role="listbox">
<ul>
${when(
- this.errorMessage,
- () => this.renderError(),
+ this.queryStatus,
+ () => this.renderStatus(),
() => html`
${repeat(
this.suggestions,
@@ -236,7 +253,7 @@
}
getCurrentText() {
- if (!this.errorMessage) {
+ if (!this.queryStatus) {
return this.getCursorTarget()?.dataset['value'] || '';
}
return '';
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.ts
index 54d054b..10ba5d0 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.ts
@@ -5,7 +5,10 @@
*/
import '../../../test/common-test-setup';
import './gr-autocomplete-dropdown';
-import {GrAutocompleteDropdown} from './gr-autocomplete-dropdown';
+import {
+ AutocompleteQueryStatusType,
+ GrAutocompleteDropdown,
+} from './gr-autocomplete-dropdown';
import {
pressKey,
queryAll,
@@ -177,7 +180,7 @@
});
});
- suite('error tests', () => {
+ suite('status tests', () => {
let element: GrAutocompleteDropdown;
setup(async () => {
@@ -185,7 +188,10 @@
html`<gr-autocomplete-dropdown></gr-autocomplete-dropdown>`
);
element.open();
- element.errorMessage = 'Failed query error';
+ element.queryStatus = {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Failed query error',
+ };
await waitEventLoop();
});
@@ -200,8 +206,8 @@
<div class="dropdown-content" id="suggestions" role="listbox">
<ul>
<li
- aria-label="autocomplete query error"
- class="query-error"
+ aria-label="autocomplete query status"
+ class="query-status error"
tabindex="-1"
>
<span>Failed query error</span>
@@ -213,6 +219,31 @@
);
});
+ test('renders loading', async () => {
+ element.queryStatus = {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ };
+ await waitEventLoop();
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <div class="dropdown-content" id="suggestions" role="listbox">
+ <ul>
+ <li
+ aria-label="autocomplete query status"
+ class="query-status loading"
+ tabindex="-1"
+ >
+ <span>Loading...</span>
+ <span class="label"></span>
+ </li>
+ </ul>
+ </div>
+ `
+ );
+ });
+
test('escape key close dropdown with error', async () => {
const closeSpy = sinon.spy(element, 'close');
pressKey(element, Key.ESC);
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
index ab74e8b..db0d236 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
@@ -7,7 +7,11 @@
import '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
import '../gr-cursor-manager/gr-cursor-manager';
import '../../../styles/shared-styles';
-import {GrAutocompleteDropdown} from '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
+import {
+ AutocompleteQueryStatus,
+ AutocompleteQueryStatusType,
+ GrAutocompleteDropdown,
+} from '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
import {fire, fireEvent} from '../../../utils/event-util';
import {
debounce,
@@ -166,7 +170,7 @@
@state() suggestions: AutocompleteSuggestion[] = [];
- @state() queryErrorMessage?: string;
+ @state() queryStatus?: AutocompleteQueryStatus;
@state() index: number | null = null;
@@ -179,8 +183,23 @@
@state() selected: HTMLElement | null = null;
+ /**
+ * The query id that status or suggestions correspond to.
+ */
+ private activeQueryId = 0;
+
+ /**
+ * Last scheduled update suggestions task.
+ */
private updateSuggestionsTask?: DelayedTask;
+ // Generate ids for scheduled suggestion queries to easily distinguish them.
+ private static NEXT_QUERY_ID = 1;
+
+ private static getNextQueryId() {
+ return GrAutocomplete.NEXT_QUERY_ID++;
+ }
+
/**
* @return Promise that resolves when suggestions are update.
*/
@@ -266,7 +285,7 @@
}
if (
changedProperties.has('suggestions') ||
- changedProperties.has('queryErrorMessage')
+ changedProperties.has('queryStatus')
) {
this.updateDropdownVisibility();
}
@@ -310,7 +329,7 @@
@item-selected=${this.handleItemSelect}
@dropdown-closed=${this.focusWithoutDisplayingSuggestions}
.suggestions=${this.suggestions}
- .errorMessage=${this.queryErrorMessage}
+ .queryStatus=${this.queryStatus}
role="listbox"
.index=${this.index}
>
@@ -412,8 +431,7 @@
// Reset suggestions for every update
// This will also prevent from carrying over suggestions:
// @see Issue 12039
- this.suggestions = [];
- this.queryErrorMessage = undefined;
+ this.resetQueryOutput();
// TODO(taoalpha): Also skip if text has not changed
@@ -421,8 +439,7 @@
return;
}
- const query = this.query;
- if (!query) {
+ if (!this.query) {
return;
}
@@ -435,49 +452,69 @@
return;
}
- const update = () => {
- query(this.text)
- .then(suggestions => {
- if (this.text !== this.text) {
- // Late response.
- return;
- }
- for (const suggestion of suggestions) {
- suggestion.text = suggestion?.name ?? '';
- }
- this.suggestions = suggestions;
- if (this.index === -1) {
- this.value = '';
- }
- })
- .catch(e => {
- this.value = '';
- if (typeof e === 'string') {
- this.queryErrorMessage = e;
- } else if (e instanceof Error) {
- this.queryErrorMessage = e.message;
- }
- });
- };
-
+ const queryId = GrAutocomplete.getNextQueryId();
+ this.activeQueryId = queryId;
+ this.setQueryStatus({
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
this.updateSuggestionsTask = debounce(
this.updateSuggestionsTask,
- update,
+ this.createUpdateTask(queryId, this.query, this.text),
DEBOUNCE_WAIT_MS
);
}
+ private createUpdateTask(
+ queryId: number,
+ query: AutocompleteQuery,
+ text: string
+ ): () => Promise<void> {
+ return async () => {
+ let suggestions: AutocompleteSuggestion[];
+ try {
+ suggestions = await query(text);
+ } catch (e) {
+ this.value = '';
+ if (typeof e === 'string') {
+ this.setQueryStatus({
+ type: AutocompleteQueryStatusType.ERROR,
+ message: e,
+ });
+ } else if (e instanceof Error) {
+ this.setQueryStatus({
+ type: AutocompleteQueryStatusType.ERROR,
+ message: e.message,
+ });
+ }
+ return;
+ }
+ if (queryId !== this.activeQueryId) {
+ // Late response.
+ return;
+ }
+ for (const suggestion of suggestions) {
+ suggestion.text = suggestion?.name ?? '';
+ }
+ this.setSuggestions(suggestions);
+ if (this.index === -1) {
+ this.value = '';
+ }
+ };
+ }
+
setFocus(focused: boolean) {
if (focused === this.focused) return;
this.focused = focused;
this.updateDropdownVisibility();
}
+ private shouldShowDropdown() {
+ return (this.suggestions.length > 0 || this.queryStatus) && this.focused;
+ }
+
updateDropdownVisibility() {
- if (
- (this.suggestions.length > 0 || this.queryErrorMessage) &&
- this.focused
- ) {
+ if (this.shouldShowDropdown()) {
this.suggestionsDropdown?.open();
return;
}
@@ -510,10 +547,26 @@
this.cancel();
break;
case 'Tab':
- if (this.suggestions.length > 0 && this.tabComplete) {
+ if (
+ this.queryStatus?.type === AutocompleteQueryStatusType.LOADING &&
+ this.tabComplete
+ ) {
e.preventDefault();
+ // Queue tab on load.
+ this.queryStatus = {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Tab on load)',
+ };
+ const queryId = this.activeQueryId;
+ this.latestSuggestionUpdateComplete?.then(() => {
+ if (queryId === this.activeQueryId) {
+ this.handleInputCommit(/* _tabComplete=*/ true);
+ }
+ });
+ } else if (this.suggestions.length > 0 && this.tabComplete) {
+ e.preventDefault();
+ this.handleInputCommit(/* _tabComplete=*/ true);
this.focus();
- this.handleInputCommit(true);
} else {
this.setFocus(false);
}
@@ -522,12 +575,24 @@
if (modifierPressed(e)) {
break;
}
- if (this.suggestions.length > 0) {
+ e.preventDefault();
+ if (this.queryStatus?.type === AutocompleteQueryStatusType.LOADING) {
+ // Queue enter on load.
+ this.queryStatus = {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Enter on load)',
+ };
+ const queryId = this.activeQueryId;
+ this.latestSuggestionUpdateComplete?.then(() => {
+ if (queryId === this.activeQueryId) {
+ this.handleItemSelectEnter(e);
+ }
+ });
+ } else if (this.suggestions.length > 0) {
// If suggestions are shown, act as if the keypress is in dropdown.
// suggestions length is 0 if error is shown.
this.handleItemSelectEnter(e);
} else {
- e.preventDefault();
this.handleInputCommit();
}
break;
@@ -540,7 +605,8 @@
// been based on a previous input. Clear them. This prevents an
// outdated suggestion from being used if the input keystroke is
// immediately followed by a commit keystroke. @see Issue 8655
- this.suggestions = [];
+ this.resetQueryOutput();
+ this.activeQueryId = 0;
}
this.dispatchEvent(
new CustomEvent('input-keydown', {
@@ -552,9 +618,11 @@
}
cancel() {
- if (this.suggestions.length || this.queryErrorMessage) {
- this.suggestions = [];
- this.queryErrorMessage = undefined;
+ if (this.shouldShowDropdown()) {
+ this.resetQueryOutput();
+ // If query is in flight by setting id to 0 we indicate that the results
+ // are outdated.
+ this.activeQueryId = 0;
this.requestUpdate();
} else {
fireEvent(this, 'cancel');
@@ -565,7 +633,7 @@
// Nothing to do if no suggestions.
if (
!this.allowNonSuggestedValues &&
- (this.suggestionsDropdown?.isHidden || this.queryErrorMessage)
+ (this.suggestionsDropdown?.isHidden || this.suggestions.length === 0)
) {
return;
}
@@ -607,6 +675,7 @@
}
}
this.setFocus(false);
+ this.activeQueryId = 0;
};
/**
@@ -643,8 +712,7 @@
}
}
- this.suggestions = [];
- this.queryErrorMessage = undefined;
+ this.resetQueryOutput();
// we need willUpdate to send text-changed event before we can send the
// 'commit' event
await this.updateComplete;
@@ -658,6 +726,23 @@
);
}
}
+
+ // resetQueryOutput, setSuggestions and setQueryStatus insure that suggestions
+ // and queryStatus are never set at the same time.
+ private resetQueryOutput() {
+ this.suggestions = [];
+ this.queryStatus = undefined;
+ }
+
+ private setSuggestions(suggestions: AutocompleteSuggestion[]) {
+ this.suggestions = suggestions;
+ this.queryStatus = undefined;
+ }
+
+ private setQueryStatus(queryStatus: AutocompleteQueryStatus) {
+ this.suggestions = [];
+ this.queryStatus = queryStatus;
+ }
}
/**
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.ts
index 81949c7..c59b8e8 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.ts
@@ -8,11 +8,15 @@
import {AutocompleteSuggestion, GrAutocomplete} from './gr-autocomplete';
import {
assertFails,
+ mockPromise,
pressKey,
queryAndAssert,
waitUntil,
} from '../../../test/test-utils';
-import {GrAutocompleteDropdown} from '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
+import {
+ AutocompleteQueryStatusType,
+ GrAutocompleteDropdown,
+} from '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
import {PaperInputElement} from '@polymer/paper-input/paper-input';
import {fixture, html, assert} from '@open-wc/testing';
import {Key, Modifier} from '../../../utils/dom-util';
@@ -30,9 +34,7 @@
const inputEl = () => queryAndAssert<HTMLInputElement>(element, '#input');
setup(async () => {
- element = await fixture(
- html`<gr-autocomplete no-debounce></gr-autocomplete>`
- );
+ element = await fixture(html`<gr-autocomplete></gr-autocomplete>`);
});
test('renders', () => {
@@ -151,7 +153,10 @@
],
}
);
- assert.equal(element.suggestionsDropdown?.errorMessage, 'blah not allowed');
+ assert.equal(
+ element.suggestionsDropdown?.queryStatus?.message,
+ 'blah not allowed'
+ );
});
test('cursor starts on suggestions', async () => {
@@ -240,17 +245,21 @@
await element.updateComplete;
return assertFails(promise).then(async () => {
+ await element.latestSuggestionUpdateComplete;
await waitUntil(() => !suggestionsEl().isHidden);
const cancelHandler = sinon.spy();
element.addEventListener('cancel', cancelHandler);
- assert.equal(element.queryErrorMessage, 'Test error');
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Test error',
+ });
pressKey(inputEl(), Key.ESC);
await waitUntil(() => suggestionsEl().isHidden);
assert.isFalse(cancelHandler.called);
- assert.isUndefined(element.queryErrorMessage);
+ assert.isUndefined(element.queryStatus);
pressKey(inputEl(), Key.ESC);
await element.updateComplete;
@@ -260,16 +269,14 @@
});
test('emits commit and handles cursor movement', async () => {
- let promise: Promise<AutocompleteSuggestion[]> = Promise.resolve([]);
- const queryStub = sinon.spy(
- (input: string) =>
- (promise = Promise.resolve([
- {name: input + ' 0', value: '0'},
- {name: input + ' 1', value: '1'},
- {name: input + ' 2', value: '2'},
- {name: input + ' 3', value: '3'},
- {name: input + ' 4', value: '4'},
- ] as AutocompleteSuggestion[]))
+ const queryStub = sinon.spy((input: string) =>
+ Promise.resolve([
+ {name: input + ' 0', value: '0'},
+ {name: input + ' 1', value: '1'},
+ {name: input + ' 2', value: '2'},
+ {name: input + ' 3', value: '3'},
+ {name: input + ' 4', value: '4'},
+ ] as AutocompleteSuggestion[])
);
element.query = queryStub;
await element.updateComplete;
@@ -280,7 +287,7 @@
element.text = 'blah';
await element.updateComplete;
- return promise.then(async () => {
+ return element.latestSuggestionUpdateComplete!.then(async () => {
await waitUntil(() => !suggestionsEl().isHidden);
const commitHandler = sinon.spy();
@@ -456,24 +463,22 @@
});
test('suggestions should not carry over', async () => {
- let promise: Promise<AutocompleteSuggestion[]> = Promise.resolve([]);
const queryStub = sinon
.stub()
- .returns(
- (promise = Promise.resolve([
- {name: 'suggestion', value: '0'},
- ] as AutocompleteSuggestion[]))
- );
+ .resolves([{name: 'suggestion', value: '0'}] as AutocompleteSuggestion[]);
element.query = queryStub;
focusOnInput();
element.text = 'bla';
await element.updateComplete;
- return promise.then(async () => {
+ return element.latestSuggestionUpdateComplete!.then(async () => {
await waitUntil(() => element.suggestions.length > 0);
assert.equal(element.suggestions.length, 1);
+
+ queryStub.resolves([] as AutocompleteSuggestion[]);
element.text = '';
element.threshold = 0;
await element.updateComplete;
+ await element.latestSuggestionUpdateComplete;
assert.equal(element.suggestions.length, 0);
});
});
@@ -488,11 +493,15 @@
element.text = 'bla';
await element.updateComplete;
return assertFails(promise).then(async () => {
- await waitUntil(() => element.queryErrorMessage === 'Test error');
+ await element.latestSuggestionUpdateComplete;
+ await waitUntil(() => element.queryStatus?.message === 'Test error');
+
+ queryStub.resolves([] as AutocompleteSuggestion[]);
element.text = '';
element.threshold = 0;
await element.updateComplete;
- assert.isUndefined(element.queryErrorMessage);
+ await element.latestSuggestionUpdateComplete;
+ assert.isUndefined(element.queryStatus);
});
});
@@ -514,6 +523,7 @@
return promise.then(async () => {
const commitHandler = sinon.spy();
element.addEventListener('commit', commitHandler);
+ await element.latestSuggestionUpdateComplete;
await waitUntil(() => element.suggestionsDropdown?.isHidden === false);
pressKey(inputEl(), Key.ENTER);
@@ -524,15 +534,24 @@
});
test('tabComplete flag functions', async () => {
+ element.query = sinon
+ .stub()
+ .resolves([
+ {name: 'tunnel snakes rule!', value: 'snakes'},
+ ] as AutocompleteSuggestion[]);
+
// commitHandler checks for the commit event, whereas commitSpy checks for
// the _commit function of the element.
const commitHandler = sinon.spy();
element.addEventListener('commit', commitHandler);
const commitSpy = sinon.spy(element, '_commit');
element.setFocus(true);
-
- element.suggestions = [{text: 'tunnel snakes rule!', name: ''}];
element.tabComplete = false;
+ element.text = 'text1';
+ await element.updateComplete;
+
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
pressKey(inputEl(), Key.TAB);
await element.updateComplete;
@@ -540,9 +559,12 @@
assert.isFalse(commitSpy.called);
assert.isFalse(element.focused);
- element.tabComplete = true;
- await element.updateComplete;
element.setFocus(true);
+ element.tabComplete = true;
+ element.text = 'text2';
+ await element.updateComplete;
+
+ await element.latestSuggestionUpdateComplete;
await element.updateComplete;
pressKey(inputEl(), Key.TAB);
@@ -597,7 +619,11 @@
' allowNonSuggestedValues',
() => {
const commitStub = sinon.stub(element, '_commit');
- element.queryErrorMessage = 'Error';
+ element.queryStatus = {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Error',
+ };
+ element.suggestions = [];
element.handleInputCommit();
assert.isFalse(commitStub.called);
}
@@ -620,7 +646,11 @@
() => {
const commitStub = sinon.stub(element, '_commit');
element.allowNonSuggestedValues = true;
- element.queryErrorMessage = 'Error';
+ element.queryStatus = {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Error',
+ };
+ element.suggestions = [];
element.handleInputCommit();
assert.isTrue(commitStub.called);
}
@@ -629,6 +659,7 @@
test('handleInputCommit with autocomplete open calls commit', () => {
const commitStub = sinon.stub(element, '_commit');
suggestionsEl().isHidden = false;
+ element.suggestions = [{name: 'first suggestion'}];
element.handleInputCommit();
assert.isTrue(commitStub.calledOnce);
});
@@ -671,6 +702,215 @@
assert.equal(element.text, 'file:x');
});
+ test('render loading replace with suggestions when done', async () => {
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ queryPromise.resolve([{name: 'suggestion 1'}] as AutocompleteSuggestion[]);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ assert.equal(element.suggestions.length, 1);
+ assert.isUndefined(element.queryStatus);
+ });
+
+ test('render loading replace with error when done', async () => {
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ queryPromise.reject(new Error('Test error'));
+ await assertFails(queryPromise);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ assert.equal(element.suggestions.length, 0);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Test error',
+ });
+ });
+
+ test('render loading esc cancels', async () => {
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ const cancelHandler = sinon.spy();
+ element.addEventListener('cancel', cancelHandler);
+ pressKey(inputEl(), Key.ESC);
+ await waitUntil(() => suggestionsEl().isHidden);
+
+ assert.isFalse(cancelHandler.called);
+ assert.isUndefined(element.queryStatus);
+
+ pressKey(inputEl(), Key.ESC);
+ await element.updateComplete;
+
+ assert.isTrue(cancelHandler.called);
+ });
+
+ test('while loading queue enter commits', async () => {
+ const commitHandler = sinon.stub();
+ element.addEventListener('commit', commitHandler);
+ let resolvePromise: (value: AutocompleteSuggestion[]) => void;
+ const blockingPromise = new Promise<AutocompleteSuggestion[]>(resolve => {
+ resolvePromise = resolve;
+ });
+ element.query = (_: string) => blockingPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ pressKey(inputEl(), Key.ENTER);
+ await element.updateComplete;
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Enter on load)',
+ });
+
+ resolvePromise!([{name: 'suggestion 1'}] as AutocompleteSuggestion[]);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ assert.equal(element.suggestions.length, 0);
+ assert.isUndefined(element.queryStatus);
+ assert.isTrue(commitHandler.called);
+ });
+
+ test('while loading queue tab completes', async () => {
+ element.tabComplete = true;
+ const commitHandler = sinon.stub();
+ element.addEventListener('commit', commitHandler);
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ pressKey(inputEl(), Key.TAB);
+ await element.updateComplete;
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Tab on load)',
+ });
+
+ queryPromise.resolve([{name: 'suggestion 1'}] as AutocompleteSuggestion[]);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ assert.equal(element.suggestions.length, 0);
+ assert.isUndefined(element.queryStatus);
+ assert.isFalse(commitHandler.called);
+ assert.equal(element.text, 'suggestion 1');
+ });
+
+ test('while loading and queued update text cancels', async () => {
+ const commitHandler = sinon.stub();
+ element.addEventListener('commit', commitHandler);
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ pressKey(inputEl(), Key.ENTER);
+ await element.updateComplete;
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Enter on load)',
+ });
+
+ element.text = 'more blah';
+ await element.updateComplete;
+
+ queryPromise.resolve([{name: 'suggestion 1'}] as AutocompleteSuggestion[]);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ // Commit for stale request is not called.
+ assert.isFalse(commitHandler.called);
+ });
+
+ test('while loading and queued esc cancels', async () => {
+ const commitHandler = sinon.stub();
+ element.addEventListener('commit', commitHandler);
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ pressKey(inputEl(), Key.ENTER);
+ await element.updateComplete;
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Enter on load)',
+ });
+
+ pressKey(inputEl(), Key.ESC);
+ await element.updateComplete;
+
+ queryPromise.resolve([{name: 'suggestion 1'}] as AutocompleteSuggestion[]);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ // Commit for stale request is not called.
+ assert.isFalse(commitHandler.called);
+ // Query results and status are cleared
+ assert.equal(element.suggestions.length, 0);
+ assert.isUndefined(element.queryStatus);
+ });
+
suite('focus', () => {
let commitSpy: sinon.SinonSpy;
let focusSpy: sinon.SinonSpy;
@@ -688,13 +928,16 @@
await element.updateComplete;
assert.equal(element.suggestions.length, 0);
- assert.isUndefined(element.queryErrorMessage);
+ assert.isUndefined(element.queryStatus);
assert.isTrue(suggestionsEl().isHidden);
});
test('enter in input does not re-render error', async () => {
element.allowNonSuggestedValues = true;
- element.queryErrorMessage = 'Error message';
+ element.queryStatus = {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Error message',
+ };
pressKey(inputEl(), Key.ENTER);
@@ -702,7 +945,7 @@
await element.updateComplete;
assert.equal(element.suggestions.length, 0);
- assert.isUndefined(element.queryErrorMessage);
+ assert.isUndefined(element.queryStatus);
assert.isTrue(suggestionsEl().isHidden);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
index d2b9e2d..073d9f1 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
@@ -7,7 +7,6 @@
import '../gr-tooltip-content/gr-tooltip-content';
import '../../../styles/shared-styles';
import {ChangeInfo} from '../../../types/common';
-import {ParsedChangeInfo} from '../../../types/types';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, html, css} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
@@ -50,9 +49,6 @@
@property({type: Boolean, reflect: true})
flat = false;
- @property({type: Object})
- change?: ChangeInfo | ParsedChangeInfo;
-
@property({type: String})
status?: ChangeStates;
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.ts b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.ts
index d916118..3bb058e 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.ts
@@ -295,7 +295,10 @@
suggestions = [{name: 'value text 1'}, {name: 'value text 2'}];
await element.open();
+ // Waiting until dropdown not hidden, will ensure dialog is open and input
+ // is focused, but not that the suggestion has loaded.
await waitUntil(() => !autocomplete.suggestionsDropdown!.isHidden);
+ await autocomplete.latestSuggestionUpdateComplete;
pressKey(autocomplete.input!, Key.ENTER);
@@ -312,7 +315,11 @@
test('autocomplete suggestions closed enter saves suggestion', async () => {
suggestions = [{name: 'value text 1'}, {name: 'value text 2'}];
await element.open();
+ // Waiting until dropdown not hidden, will ensure dialog is open and input
+ // is focused, but not that the suggestion has loaded.
await waitUntil(() => !autocomplete.suggestionsDropdown!.isHidden);
+ await autocomplete.latestSuggestionUpdateComplete;
+
// Press enter to close suggestions.
pressKey(autocomplete.input!, Key.ENTER);
diff --git a/polygerrit-ui/app/models/accounts-model/accounts-model.ts b/polygerrit-ui/app/models/accounts-model/accounts-model.ts
index 2bf6068..1c67857 100644
--- a/polygerrit-ui/app/models/accounts-model/accounts-model.ts
+++ b/polygerrit-ui/app/models/accounts-model/accounts-model.ts
@@ -8,11 +8,14 @@
import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
import {UserId} from '../../types/common';
import {getUserId, isDetailedAccount} from '../../utils/account-util';
+import {hasOwnProperty} from '../../utils/common-util';
import {define} from '../dependency';
import {Model} from '../model';
export interface AccountsState {
- accounts: {[id: UserId]: AccountDetailInfo};
+ accounts: {
+ [id: UserId]: AccountDetailInfo | AccountInfo;
+ };
}
export const accountsModelToken = define<AccountsModel>('accounts-model');
@@ -24,33 +27,36 @@
});
}
- private updateStateAccount(id: UserId, account?: AccountDetailInfo) {
+ private updateStateAccount(
+ id: UserId,
+ account: AccountDetailInfo | AccountInfo
+ ) {
if (!account) return;
const current = {...this.getState()};
current.accounts = {...current.accounts, [id]: account};
this.setState(current);
}
- async getAccount(partialAccount: AccountInfo) {
+ async getAccount(
+ partialAccount: AccountInfo
+ ): Promise<AccountDetailInfo | AccountInfo> {
const current = this.getState();
const id = getUserId(partialAccount);
- if (current.accounts[id]) return current.accounts[id];
+ if (hasOwnProperty(current.accounts, id)) return current.accounts[id];
// It is possible to add emails to CC when they don't have a Gerrit
- // account. In this case getAccountDetails will return a 404 error hence
- // pass an empty error function to handle that.
+ // account. In this case getAccountDetails will return a 404 error then
+ // we at least use what is in partialAccount.
const account = await this.restApiService.getAccountDetails(id, () => {
- this.updateStateAccount(id, partialAccount as AccountDetailInfo);
+ this.updateStateAccount(id, partialAccount);
return;
});
if (account) this.updateStateAccount(id, account);
- return account;
+ return account ?? partialAccount;
}
async fillDetails(account: AccountInfo) {
if (!isDetailedAccount(account)) {
- if (account.email) return await this.getAccount({email: account.email});
- else if (account._account_id)
- return await this.getAccount({_account_id: account._account_id});
+ return await this.getAccount(account);
}
return account;
}
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 29e9259..7488e79 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -19,5 +19,4 @@
PUSH_NOTIFICATIONS_DEVELOPER = 'UiFeature__push_notifications_developer',
PUSH_NOTIFICATIONS = 'UiFeature__push_notifications',
SUGGEST_EDIT = 'UiFeature__suggest_edit',
- REBASE_CHAIN = 'UiFeature__rebase_chain',
}
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index 0d0c88f..610d8f3 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -769,7 +769,7 @@
userId: AccountId | EmailAddress,
errFn?: ErrorCallback
): Promise<AccountDetailInfo | undefined> {
- return this._restApiHelper.fetchJSON({
+ return this._fetchSharedCacheURL({
url: `/accounts/${encodeURIComponent(userId)}/detail`,
anonymizedUrl: '/accounts/*/detail',
errFn,
diff --git a/polygerrit-ui/app/utils/async-util.ts b/polygerrit-ui/app/utils/async-util.ts
index 752de62..6ac3211 100644
--- a/polygerrit-ui/app/utils/async-util.ts
+++ b/polygerrit-ui/app/utils/async-util.ts
@@ -62,6 +62,8 @@
/**
* Promise that is resolved after the callback is run or the task is
* cancelled.
+ *
+ * If callback returns a Promise this resolves after the promise is settled.
*/
public readonly promise: Promise<ResolvedDelayedTaskStatus>;
@@ -69,14 +71,28 @@
value: ResolvedDelayedTaskStatus | PromiseLike<ResolvedDelayedTaskStatus>
) => void;
- constructor(private callback: () => void, waitMs = 0) {
+ private callCallbackAndResolveOnCompletion() {
+ let callbackResult;
+ if (this.callback) callbackResult = this.callback();
+ if (callbackResult instanceof Promise) {
+ callbackResult.finally(() => {
+ this.resolvePromise!(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED);
+ });
+ } else {
+ this.resolvePromise!(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED);
+ }
+ }
+
+ constructor(
+ private readonly callback: () => void | Promise<void>,
+ waitMs = 0
+ ) {
this.promise = new Promise(resolve => {
this.resolvePromise = resolve;
this.timerId = window.setTimeout(() => {
if (this.timerId) _testOnly_allTasks.delete(this.timerId);
this.timerId = undefined;
- if (this.callback) this.callback();
- resolve(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED);
+ this.callCallbackAndResolveOnCompletion();
}, waitMs);
_testOnly_allTasks.set(this.timerId, this);
});
@@ -98,8 +114,7 @@
flush() {
if (this.isActive()) {
this.cancelTimer();
- if (this.callback) this.callback();
- this.resolvePromise?.(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED);
+ this.callCallbackAndResolveOnCompletion();
}
}
diff --git a/polygerrit-ui/app/utils/async-util_test.ts b/polygerrit-ui/app/utils/async-util_test.ts
index 9f029b8..ee4f73a 100644
--- a/polygerrit-ui/app/utils/async-util_test.ts
+++ b/polygerrit-ui/app/utils/async-util_test.ts
@@ -6,8 +6,8 @@
import {assert} from '@open-wc/testing';
import {SinonFakeTimers} from 'sinon';
import '../test/common-test-setup';
-import {waitEventLoop} from '../test/test-utils';
-import {asyncForeach, debounceP} from './async-util';
+import {mockPromise, waitEventLoop, waitUntil} from '../test/test-utils';
+import {asyncForeach, debounceP, DelayedTask} from './async-util';
suite('async-util tests', () => {
suite('asyncForeach', () => {
@@ -205,4 +205,16 @@
await waitEventLoop();
});
});
+
+ test('DelayedTask promise resolved when callback is done', async () => {
+ const callbackPromise = mockPromise<void>();
+ const task = new DelayedTask(() => callbackPromise);
+ let completed = false;
+ task.promise.then(() => (completed = true));
+ await waitUntil(() => !task.isActive());
+
+ assert.isFalse(completed);
+ callbackPromise.resolve();
+ await waitUntil(() => completed);
+ });
});
