Merge "Revert: Do not check the account visibility of reviewers/CCs"
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index ca72f8b..33c5bbd 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -533,6 +533,24 @@
Certain operations in Gerrit can be validated by plugins by
implementing the corresponding link:config-validation.html[listeners].
+[[taskListeners]]
+== WorkQueue.TaskListeners
+
+It is possible for plugins to listen to
+`com.google.gerrit.server.git.WorkQueue$Task`s directly before they run, and
+directly after they complete. This may be used to delay task executions based
+on custom criteria by blocking, likely on a lock or semaphore, inside
+onStart(), and a lock/semaphore release in onStop(). Plugins may listen to
+tasks by implementing a `com.google.gerrit.server.git.WorkQueue$TaskListener`
+and registering the new listener like this:
+
+[source,java]
+----
+bind(TaskListener.class)
+ .annotatedWith(Exports.named("MyListener"))
+ .to(MyListener.class);
+----
+
[[change-message-modifier]]
== Change Message Modifier
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index efc746a..685f9d9 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -3804,6 +3804,7 @@
|`enabled` |optional|Whether the commentlink is enabled, as documented
in link:config-gerrit.html#commentlink.name.enabled[
commentlink.name.enabled]. If not set the commentlink is enabled.
+|==================================================
[[commentlink-input]]
=== CommentLinkInput
diff --git a/java/com/google/gerrit/pgm/Reindex.java b/java/com/google/gerrit/pgm/Reindex.java
index c4e185d..762d988 100644
--- a/java/com/google/gerrit/pgm/Reindex.java
+++ b/java/com/google/gerrit/pgm/Reindex.java
@@ -36,6 +36,7 @@
import com.google.gerrit.server.cache.CacheInfo;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.WorkQueue.WorkQueueModule;
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.index.options.AutoFlush;
@@ -175,6 +176,8 @@
}
boolean replica = ReplicaUtil.isReplica(globalConfig);
List<Module> modules = new ArrayList<>();
+ modules.add(new WorkQueueModule());
+
Module indexModule;
IndexType indexType = IndexModule.getIndexType(dbInjector);
if (indexType.isLucene()) {
diff --git a/java/com/google/gerrit/server/git/WorkQueue.java b/java/com/google/gerrit/server/git/WorkQueue.java
index 3032bfe..715cb30 100644
--- a/java/com/google/gerrit/server/git/WorkQueue.java
+++ b/java/com/google/gerrit/server/git/WorkQueue.java
@@ -20,6 +20,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.MetricMaker;
@@ -27,6 +28,7 @@
import com.google.gerrit.server.config.ScheduleConfig.Schedule;
import com.google.gerrit.server.logging.LoggingContext;
import com.google.gerrit.server.logging.LoggingContextAwareRunnable;
+import com.google.gerrit.server.plugincontext.PluginMapContext;
import com.google.gerrit.server.util.IdGenerator;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -58,6 +60,30 @@
public class WorkQueue {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ /**
+ * To register a TaskListener, which will be called directly before Tasks run, and directly after
+ * they complete, bind the TaskListener like this:
+ *
+ * <p><code>
+ * bind(TaskListener.class)
+ * .annotatedWith(Exports.named("MyListener"))
+ * .to(MyListener.class);
+ * </code>
+ */
+ public interface TaskListener {
+ public static class NoOp implements TaskListener {
+ @Override
+ public void onStart(Task<?> task) {}
+
+ @Override
+ public void onStop(Task<?> task) {}
+ }
+
+ void onStart(Task<?> task);
+
+ void onStop(Task<?> task);
+ }
+
public static class Lifecycle implements LifecycleListener {
private final WorkQueue workQueue;
@@ -78,6 +104,7 @@
public static class WorkQueueModule extends LifecycleModule {
@Override
protected void configure() {
+ DynamicMap.mapOf(binder(), WorkQueue.TaskListener.class);
bind(WorkQueue.class);
listener().to(Lifecycle.class);
}
@@ -87,18 +114,32 @@
private final IdGenerator idGenerator;
private final MetricMaker metrics;
private final CopyOnWriteArrayList<Executor> queues;
+ private final PluginMapContext<TaskListener> listeners;
@Inject
- WorkQueue(IdGenerator idGenerator, @GerritServerConfig Config cfg, MetricMaker metrics) {
- this(idGenerator, Math.max(cfg.getInt("execution", "defaultThreadPoolSize", 2), 2), metrics);
+ WorkQueue(
+ IdGenerator idGenerator,
+ @GerritServerConfig Config cfg,
+ MetricMaker metrics,
+ PluginMapContext<TaskListener> listeners) {
+ this(
+ idGenerator,
+ Math.max(cfg.getInt("execution", "defaultThreadPoolSize", 2), 2),
+ metrics,
+ listeners);
}
/** Constructor to allow binding the WorkQueue more explicitly in a vhost setup. */
- public WorkQueue(IdGenerator idGenerator, int defaultThreadPoolSize, MetricMaker metrics) {
+ public WorkQueue(
+ IdGenerator idGenerator,
+ int defaultThreadPoolSize,
+ MetricMaker metrics,
+ PluginMapContext<TaskListener> listeners) {
this.idGenerator = idGenerator;
this.metrics = metrics;
this.queues = new CopyOnWriteArrayList<>();
this.defaultQueue = createQueue(defaultThreadPoolSize, "WorkQueue", true);
+ this.listeners = listeners;
}
/** Get the default work queue, for miscellaneous tasks. */
@@ -438,6 +479,14 @@
Collection<Task<?>> getTasks() {
return all.values();
}
+
+ public void onStart(Task<?> task) {
+ listeners.runEach(extension -> extension.getProvider().get().onStart(task));
+ }
+
+ public void onStop(Task<?> task) {
+ listeners.runEach(extension -> extension.getProvider().get().onStop(task));
+ }
}
private static void logUncaughtException(Thread t, Throwable e) {
@@ -608,10 +657,12 @@
if (running.compareAndSet(false, true)) {
String oldThreadName = Thread.currentThread().getName();
try {
+ executor.onStart(this);
Thread.currentThread().setName(oldThreadName + "[" + task.toString() + "]");
task.run();
} finally {
Thread.currentThread().setName(oldThreadName);
+ executor.onStop(this);
if (isPeriodic()) {
running.set(false);
} else {
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index b00cadb..781965e 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -79,6 +79,7 @@
import com.google.gerrit.server.git.PerThreadRequestScope;
import com.google.gerrit.server.git.SearchingChangeCacheImpl.SearchingChangeCacheImplModule;
import com.google.gerrit.server.git.WorkQueue;
+import com.google.gerrit.server.git.WorkQueue.WorkQueueModule;
import com.google.gerrit.server.group.testing.TestGroupBackend;
import com.google.gerrit.server.index.account.AccountSchemaDefinitions;
import com.google.gerrit.server.index.account.AllAccountsIndexer;
@@ -195,6 +196,7 @@
install(new AuditModule());
install(new SubscriptionGraphModule());
install(new SuperprojectUpdateSubmissionListenerModule());
+ install(new WorkQueueModule());
bindScope(RequestScoped.class, PerThreadRequestScope.REQUEST);
diff --git a/javatests/com/google/gerrit/acceptance/server/util/TaskListenerIT.java b/javatests/com/google/gerrit/acceptance/server/util/TaskListenerIT.java
new file mode 100644
index 0000000..b3094ac
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/util/TaskListenerIT.java
@@ -0,0 +1,255 @@
+// 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.acceptance.server.util;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.server.git.WorkQueue;
+import com.google.gerrit.server.git.WorkQueue.Task;
+import com.google.gerrit.server.git.WorkQueue.TaskListener;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TaskListenerIT extends AbstractDaemonTest {
+ /**
+ * Use a LatchedMethod in a method to allow another thread to await the method's call. Once
+ * called, the Latch.call() method will block until another thread calls its LatchedMethods's
+ * complete() method.
+ */
+ private static class LatchedMethod {
+ private static final int AWAIT_TIMEOUT = 20;
+ private static final TimeUnit AWAIT_TIMEUNIT = TimeUnit.MILLISECONDS;
+
+ /** API class meant be used by the class whose method is being latched */
+ private class Latch {
+ /** Ensure that the latched method calls this on entry */
+ public void call() {
+ called.countDown();
+ await(complete);
+ }
+ }
+
+ public Latch latch = new Latch();
+
+ private CountDownLatch called = new CountDownLatch(1);
+ private CountDownLatch complete = new CountDownLatch(1);
+
+ /** Assert that the Latch's call() method has not yet been called */
+ public void assertUncalled() {
+ assertThat(called.getCount()).isEqualTo(1);
+ }
+
+ /**
+ * Assert that a timeout does not occur while awaiting Latch's call() method to be called. Fails
+ * if the waiting time elapses before Latch's call() method is called, otherwise passes.
+ */
+ public void assertAwait() {
+ assertThat(await(called)).isEqualTo(true);
+ }
+
+ /** Unblock the Latch's call() method so that it can complete */
+ public void complete() {
+ complete.countDown();
+ }
+
+ private static boolean await(CountDownLatch latch) {
+ try {
+ return latch.await(AWAIT_TIMEOUT, AWAIT_TIMEUNIT);
+ } catch (InterruptedException e) {
+ return false;
+ }
+ }
+ }
+
+ private static class LatchedRunnable implements Runnable {
+ public LatchedMethod run = new LatchedMethod();
+
+ @Override
+ public void run() {
+ run.latch.call();
+ }
+ }
+
+ private static class ForwardingListener implements TaskListener {
+ public volatile TaskListener delegate;
+ public volatile Task task;
+
+ public void resetDelegate(TaskListener listener) {
+ delegate = listener;
+ task = null;
+ }
+
+ @Override
+ public void onStart(Task<?> task) {
+ if (delegate != null) {
+ if (this.task == null || this.task == task) {
+ this.task = task;
+ delegate.onStart(task);
+ }
+ }
+ }
+
+ @Override
+ public void onStop(Task<?> task) {
+ if (delegate != null) {
+ if (this.task == task) {
+ delegate.onStop(task);
+ }
+ }
+ }
+ }
+
+ private static class LatchedListener implements TaskListener {
+ public LatchedMethod onStart = new LatchedMethod();
+ public LatchedMethod onStop = new LatchedMethod();
+
+ @Override
+ public void onStart(Task<?> task) {
+ onStart.latch.call();
+ }
+
+ @Override
+ public void onStop(Task<?> task) {
+ onStop.latch.call();
+ }
+ }
+
+ private static ForwardingListener forwarder;
+
+ @Inject private WorkQueue workQueue;
+ private ScheduledExecutorService executor;
+
+ private LatchedListener listener = new LatchedListener();
+ private LatchedRunnable runnable = new LatchedRunnable();
+
+ @Override
+ public Module createModule() {
+ return new AbstractModule() {
+ @Override
+ public void configure() {
+ // Forwarder.delegate is empty on start to protect test listener from non test tasks
+ // (such as the "Log File Compressor") interference
+ forwarder = new ForwardingListener(); // Only gets bound once for all tests
+ bind(TaskListener.class).annotatedWith(Exports.named("listener")).toInstance(forwarder);
+ }
+ };
+ }
+
+ @Before
+ public void setupExecutorAndForwarder() throws InterruptedException {
+ executor = workQueue.createQueue(1, "TaskListeners");
+
+ // "Log File Compressor"s are likely running and will interfere with tests
+ while (0 != workQueue.getTasks().size()) {
+ for (Task<?> t : workQueue.getTasks()) {
+ t.cancel(true);
+ }
+ TimeUnit.MILLISECONDS.sleep(1);
+ }
+
+ forwarder.resetDelegate(listener);
+
+ assertQueueSize(0);
+ assertThat(forwarder.task).isEqualTo(null);
+ listener.onStart.assertUncalled();
+ runnable.run.assertUncalled();
+ listener.onStop.assertUncalled();
+ }
+
+ @Test
+ public void onStartThenRunThenOnStopAreCalled() throws Exception {
+ int size = assertQueueBlockedOnExecution(runnable);
+
+ // onStartThenRunThenOnStopAreCalled -> onStart...Called
+ listener.onStart.assertAwait();
+ assertQueueSize(size);
+ runnable.run.assertUncalled();
+ listener.onStop.assertUncalled();
+
+ listener.onStart.complete();
+ // onStartThenRunThenOnStopAreCalled -> ...ThenRun...Called
+ runnable.run.assertAwait();
+ listener.onStop.assertUncalled();
+
+ runnable.run.complete();
+ // onStartThenRunThenOnStopAreCalled -> ...ThenOnStop...Called
+ listener.onStop.assertAwait();
+ assertQueueSize(size);
+
+ listener.onStop.complete();
+ assertAwaitQueueSize(--size);
+ }
+
+ @Test
+ public void firstBlocksSecond() throws Exception {
+ int size = assertQueueBlockedOnExecution(runnable);
+
+ // firstBlocksSecond -> first...
+ listener.onStart.assertAwait();
+ assertQueueSize(size);
+
+ LatchedRunnable runnable2 = new LatchedRunnable();
+ size = assertQueueBlockedOnExecution(runnable2);
+
+ // firstBlocksSecond -> ...BlocksSecond
+ runnable2.run.assertUncalled();
+ assertQueueSize(size); // waiting on first
+
+ listener.onStart.complete();
+ runnable.run.assertAwait();
+ assertQueueSize(size); // waiting on first
+ runnable2.run.assertUncalled();
+
+ runnable.run.complete();
+ listener.onStop.assertAwait();
+ assertQueueSize(size); // waiting on first
+ runnable2.run.assertUncalled();
+
+ listener.onStop.complete();
+ runnable2.run.assertAwait();
+ assertQueueSize(--size);
+
+ runnable2.run.complete();
+ assertAwaitQueueSize(--size);
+ }
+
+ private int assertQueueBlockedOnExecution(Runnable runnable) {
+ int expectedSize = workQueue.getTasks().size() + 1;
+ executor.execute(runnable);
+ assertQueueSize(expectedSize);
+ return expectedSize;
+ }
+
+ private void assertQueueSize(int size) {
+ assertThat(workQueue.getTasks().size()).isEqualTo(size);
+ }
+
+ /** Fails if the waiting time elapses before the count is reached, otherwise passes */
+ private void assertAwaitQueueSize(int size) throws InterruptedException {
+ long i = 0;
+ do {
+ TimeUnit.NANOSECONDS.sleep(10);
+ assertThat(i++).isLessThan(100);
+ } while (size != workQueue.getTasks().size());
+ }
+}
diff --git a/polygerrit-ui/app/api/rest-api.ts b/polygerrit-ui/app/api/rest-api.ts
index a7af005..3c06eb0 100644
--- a/polygerrit-ui/app/api/rest-api.ts
+++ b/polygerrit-ui/app/api/rest-api.ts
@@ -453,6 +453,9 @@
export declare interface CommentLinkInfo {
match: string;
link?: string;
+ prefix?: string;
+ suffix?: string;
+ text?: string;
enabled?: boolean;
html?: string;
}
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 76ec316..8293daa 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
@@ -15,7 +15,7 @@
import '../../shared/gr-change-star/gr-change-star';
import '../../shared/gr-change-status/gr-change-status';
import '../../shared/gr-editable-content/gr-editable-content';
-import '../../shared/gr-formatted-text/gr-formatted-text';
+import '../../shared/gr-linked-text/gr-linked-text';
import '../../shared/gr-overlay/gr-overlay';
import '../../shared/gr-tooltip-content/gr-tooltip-content';
import '../gr-change-actions/gr-change-actions';
@@ -191,6 +191,7 @@
const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
+const REVIEWERS_REGEX = /^(R|CC)=/gm;
const MIN_CHECK_INTERVAL_SECS = 0;
const REPLY_REFIT_DEBOUNCE_INTERVAL_MS = 500;
@@ -958,7 +959,7 @@
/* Account for border and padding and rounding errors. */
max-width: calc(72ch + 2px + 2 * var(--spacing-m) + 0.4px);
}
- .commitMessage gr-formatted-text {
+ .commitMessage gr-linked-text {
word-break: break-word;
}
#commitMessageEditor {
@@ -1459,10 +1460,12 @@
.commitCollapsible=${this.computeCommitCollapsible()}
remove-zero-width-space=""
>
- <gr-formatted-text
- .markdown=${false}
- .content=${this.latestCommitMessage ?? ''}
- ></gr-formatted-text>
+ <gr-linked-text
+ pre=""
+ .content=${this.latestCommitMessage}
+ .config=${this.projectConfig?.commentlinks}
+ remove-zero-width-space=""
+ ></gr-linked-text>
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">Comments and Checks Summary</h3>
@@ -1821,7 +1824,7 @@
return;
}
- this.latestCommitMessage = message;
+ this.latestCommitMessage = this.prepareCommitMsgForLinkify(message);
this.editingCommitMessage = false;
this.reloadWindow();
})
@@ -2249,8 +2252,6 @@
if (tab === Tab.CHECKS) {
const state: ChecksTabState = {};
detail.tabState = {checksTab: state};
- if (this.viewState?.filter) state.filter = this.viewState.filter;
- if (this.viewState?.attempt) state.attempt = this.viewState.attempt;
}
this.setActiveTab(
@@ -2671,6 +2672,14 @@
this.changeViewAriaHidden = true;
}
+ // Private but used in tests.
+ prepareCommitMsgForLinkify(msg: string) {
+ // TODO(wyatta) switch linkify sequence, see issue 5526.
+ // This is a zero-with space. It is added to prevent the linkify library
+ // from including R= or CC= as part of the email address.
+ return msg.replace(REVIEWERS_REGEX, '$1=\u200B');
+ }
+
/**
* Utility function to make the necessary modifications to a change in the
* case an edit exists.
@@ -2800,7 +2809,9 @@
throw new Error('Could not find latest Revision Sha');
const currentRevision = this.change.revisions[latestRevisionSha];
if (currentRevision.commit && currentRevision.commit.message) {
- this.latestCommitMessage = currentRevision.commit.message;
+ this.latestCommitMessage = this.prepareCommitMsgForLinkify(
+ currentRevision.commit.message
+ );
} else {
this.latestCommitMessage = null;
}
@@ -2853,7 +2864,9 @@
.getChangeCommitInfo(this.changeNum, lastpatchNum)
.then(commitInfo => {
if (!commitInfo) return;
- this.latestCommitMessage = commitInfo.message;
+ this.latestCommitMessage = this.prepareCommitMsgForLinkify(
+ commitInfo.message
+ );
});
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index e0c09e2..ad84fb0 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -433,7 +433,9 @@
id="commitMessageEditor"
remove-zero-width-space=""
>
- <gr-formatted-text></gr-formatted-text>
+ <gr-linked-text pre="" remove-zero-width-space="">
+ <span id="output" slot="insert"></span>
+ </gr-linked-text>
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">
@@ -1407,6 +1409,20 @@
assert.isTrue(overlayOpenStub.called);
});
+ test('prepareCommitMsgForLinkify', () => {
+ let commitMessage = 'R=test@google.com';
+ let result = element.prepareCommitMsgForLinkify(commitMessage);
+ assert.equal(result, 'R=\u200Btest@google.com');
+
+ commitMessage = 'R=test@google.com\nR=test@google.com';
+ result = element.prepareCommitMsgForLinkify(commitMessage);
+ assert.equal(result, 'R=\u200Btest@google.com\nR=\u200Btest@google.com');
+
+ commitMessage = 'CC=test@google.com';
+ result = element.prepareCommitMsgForLinkify(commitMessage);
+ assert.equal(result, 'CC=\u200Btest@google.com');
+ });
+
test('_isSubmitEnabled', () => {
assert.isFalse(element.isSubmitEnabled());
element.currentRevisionActions = {submit: {}};
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index a0e4c89..731f227 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -882,7 +882,7 @@
override render() {
this.classList.toggle('editMode', this.editMode);
const patchChange = this.calculatePatchChange();
- return this.patched.html`
+ return html`
<h3 class="assistive-tech-only">File list</h3>
${this.renderContainer()} ${this.renderChangeTotals(patchChange)}
${this.renderBinaryTotals(patchChange)} ${this.renderControlRow()}
@@ -895,7 +895,7 @@
}
private renderContainer() {
- return this.patched.html`
+ return html`
<div
id="container"
@click=${(e: MouseEvent) => this.handleFileListClick(e)}
@@ -1013,7 +1013,7 @@
this.reportRenderedRow(index);
const previousFileName = this.shownFiles[index - 1]?.__path;
const patchSetFile = this.computePatchSetFile(file);
- return this.patched.html` <div class="stickyArea">
+ return html` <div class="stickyArea">
<div
class=${`file-row row ${this.computePathClass(file.__path)}`}
data-file=${JSON.stringify(patchSetFile)}
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index a4e026b..11fe0e2 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -28,8 +28,8 @@
import {pluralize} from '../../../utils/string-util';
import {
changeIsOpen,
+ getChangeNumber,
getRevisionKey,
- isChangeInfo,
} from '../../../utils/change-util';
import {DEFALT_NUM_CHANGES_WHEN_COLLAPSED} from './gr-related-collapse';
import {createChangeUrl} from '../../../models/views/change';
@@ -639,29 +639,12 @@
a?: ChangeInfo | RelatedChangeAndCommitInfo,
b?: ChangeInfo | ParsedChangeInfo | RelatedChangeAndCommitInfo
) {
- const aNum = this._getChangeNumber(a);
- const bNum = this._getChangeNumber(b);
+ if (!a || !b) return false;
+ const aNum = getChangeNumber(a);
+ const bNum = getChangeNumber(b);
return aNum === bNum;
}
- /**
- * Get the change number from either a ChangeInfo (such as those included in
- * SubmittedTogetherInfo responses) or get the change number from a
- * RelatedChangeAndCommitInfo (such as those included in a
- * RelatedChangesInfo response).
- */
- _getChangeNumber(
- change?: ChangeInfo | ParsedChangeInfo | RelatedChangeAndCommitInfo
- ) {
- // Default to 0 if change property is not defined.
- if (!change) return 0;
-
- if (isChangeInfo(change)) {
- return change._number;
- }
- return change._change_number;
- }
-
/*
* A list of commit ids connected to change to understand if other change
* is direct or indirect ancestor / descendant.
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
index 09cdf5f..dcc6039 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
@@ -36,6 +36,7 @@
SubmittedTogetherInfo,
} from '../../../types/common';
import {ParsedChangeInfo} from '../../../types/types';
+import {getChangeNumber} from '../../../utils/change-util';
import {GrEndpointDecorator} from '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import './gr-related-changes-list';
@@ -359,8 +360,8 @@
change_id: '456' as ChangeId,
_number: 1 as NumericChangeId,
};
- assert.equal(element._getChangeNumber(change1), 0);
- assert.equal(element._getChangeNumber(change2), 1);
+ assert.equal(getChangeNumber(change1), 0);
+ assert.equal(getChangeNumber(change2), 1);
});
suite('get conflicts tests', () => {
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 b540c89..22f1325 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
@@ -257,9 +257,6 @@
@state() serverConfig?: ServerInfo;
@state()
- patchsetLevelDraftMessage = '';
-
- @state()
filterReviewerSuggestion: (input: Suggestion) => boolean;
@state()
@@ -382,7 +379,7 @@
patchsetLevelDraftIsResolved = true;
@state()
- patchsetLevelComment?: UnsavedInfo | DraftInfo;
+ patchsetLevelComment: UnsavedInfo | DraftInfo = this.createDraft('');
private readonly restApiService: RestApiService =
getAppContext().restApiService;
@@ -673,7 +670,9 @@
subscribe(
this,
() => this.getCommentsModel().patchsetLevelDrafts$,
- x => (this.patchsetLevelComment = x[0])
+ x => {
+ if (x.length > 0) this.patchsetLevelComment = x[0];
+ }
);
subscribe(
this,
@@ -764,7 +763,7 @@
changedProperties.has('mentionedUsersInUnresolvedDrafts') ||
changedProperties.has('includeComments') ||
changedProperties.has('labelsChanged') ||
- changedProperties.has('patchsetLevelDraftMessage') ||
+ changedProperties.has('patchsetLevelComment') ||
changedProperties.has('mentionedCCs')
) {
this.computeNewAttention();
@@ -916,10 +915,11 @@
}
// TODO: move to comment-util
- private createDraft(): UnsavedInfo {
+ // Private but used in tests.
+ createDraft(message: string): UnsavedInfo {
return {
patch_set: this.latestPatchNum,
- message: this.patchsetLevelDraftMessage,
+ message,
unresolved: !this.patchsetLevelDraftIsResolved,
path: SpecialFilePath.PATCHSET_LEVEL_COMMENTS,
__unsaved: true,
@@ -927,8 +927,6 @@
}
private renderPatchsetLevelComment() {
- if (!this.patchsetLevelComment)
- this.patchsetLevelComment = this.createDraft();
return html`
<gr-comment
id="patchsetLevelComment"
@@ -938,7 +936,10 @@
this.patchsetLevelDraftIsResolved = !e.detail.value;
}}
@comment-text-changed=${(e: ValueChangedEvent<string>) => {
- this.patchsetLevelDraftMessage = e.detail.value;
+ const newMessage = e.detail.value;
+ if (this.patchsetLevelComment.message === newMessage) return;
+ this.patchsetLevelComment.message = newMessage;
+ this.requestUpdate('patchsetLevelComment');
}}
.messagePlaceholder=${this.messagePlaceholder}
hide-header
@@ -1268,7 +1269,7 @@
this.focusOn(focusTarget);
if (quote?.length) {
// If a reply quote has been provided, use it.
- this.patchsetLevelDraftMessage = quote;
+ this.patchsetLevelComment = this.createDraft(quote);
}
if (this.restApiService.hasPendingDiffDrafts()) {
this.savingComments = true;
@@ -1281,7 +1282,7 @@
hasDrafts() {
return (
- this.patchsetLevelDraftMessage.length > 0 ||
+ !!this.patchsetLevelComment.message?.length ||
this.draftCommentThreads.length > 0
);
}
@@ -1469,8 +1470,8 @@
return;
}
- this.patchsetLevelDraftMessage = '';
this.includeComments = true;
+ this.patchsetLevelComment = this.createDraft('');
this.dispatchEvent(
new CustomEvent('send', {
composed: true,
@@ -2031,7 +2032,6 @@
computeSendButtonDisabled() {
if (
this.canBeStarted === undefined ||
- this.patchsetLevelDraftMessage === undefined ||
this.reviewersMutated === undefined ||
this.labelsChanged === undefined ||
this.includeComments === undefined ||
@@ -2055,13 +2055,8 @@
const revotingOrNewVote = this.labelsChanged || existingVote;
const hasDrafts =
(this.includeComments && this.draftCommentThreads.length > 0) ||
- this.patchsetLevelDraftMessage.length > 0;
- return (
- !hasDrafts &&
- !this.patchsetLevelDraftMessage.length &&
- !this.reviewersMutated &&
- !revotingOrNewVote
- );
+ !!this.patchsetLevelComment?.message?.length;
+ return !hasDrafts && !this.reviewersMutated && !revotingOrNewVote;
}
computePatchSetWarning() {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
index acd1755..cef787a 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
@@ -327,7 +327,9 @@
// Async tick is needed because iron-selector content is distributed and
// distributed content requires an observer to be set up.
await element.updateComplete;
- element.patchsetLevelDraftMessage = 'I wholeheartedly disapprove';
+ element.patchsetLevelComment = element.createDraft(
+ 'I wholeheartedly disapprove'
+ );
element.draftCommentThreads = [createCommentThread([createComment()])];
element.includeComments = true;
@@ -1054,7 +1056,9 @@
});
test('label picker', async () => {
- element.patchsetLevelDraftMessage = 'I wholeheartedly disapprove';
+ element.patchsetLevelComment = element.createDraft(
+ 'I wholeheartedly disapprove'
+ );
element.draftCommentThreads = [createCommentThread([createComment()])];
const saveReviewPromise = interceptSaveReview();
@@ -1075,7 +1079,7 @@
const review = await saveReviewPromise;
await element.updateComplete;
await waitUntil(() => element.disabled === false);
- assert.equal(element.patchsetLevelDraftMessage.length, 0);
+ assert.equal(element.patchsetLevelComment.message?.length, 0);
assert.deepEqual(review, {
drafts: 'PUBLISH_ALL_REVISIONS',
labels: {
@@ -1101,7 +1105,9 @@
// Async tick is needed because iron-selector content is distributed and
// distributed content requires an observer to be set up.
await element.updateComplete;
- element.patchsetLevelDraftMessage = 'I wholeheartedly disapprove';
+ element.patchsetLevelComment = element.createDraft(
+ 'I wholeheartedly disapprove'
+ );
const saveReviewPromise = interceptSaveReview();
@@ -1616,7 +1622,7 @@
assert.isFalse(element.attentionExpanded);
- element.patchsetLevelDraftMessage = 'a test comment';
+ element.patchsetLevelComment = element.createDraft('a test comment');
await element.updateComplete;
modifyButton.click();
@@ -2066,11 +2072,11 @@
const expectedError = new Error('test');
setup(() => {
- element.patchsetLevelDraftMessage = expectedDraft;
+ element.patchsetLevelComment = element.createDraft(expectedDraft);
});
function assertDialogOpenAndEnabled() {
- assert.strictEqual(expectedDraft, element.patchsetLevelDraftMessage);
+ assert.strictEqual(expectedDraft, element.patchsetLevelComment.message);
assert.isFalse(element.disabled);
}
@@ -2116,7 +2122,7 @@
// Mock canBeStarted
element.canBeStarted = true;
element.draftCommentThreads = [];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2130,7 +2136,7 @@
// Mock everything false
element.canBeStarted = false;
element.draftCommentThreads = [];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2144,7 +2150,7 @@
// Mock nonempty comment draft array; with sending comments.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = true;
@@ -2158,7 +2164,7 @@
// Mock nonempty comment draft array; without sending comments.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2173,7 +2179,7 @@
// Mock nonempty change message.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = 'test';
+ element.patchsetLevelComment = element.createDraft('test');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2188,7 +2194,7 @@
// Mock reviewers mutated.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = true;
element.labelsChanged = false;
element.includeComments = false;
@@ -2203,7 +2209,7 @@
// Mock labels changed.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = true;
element.includeComments = false;
@@ -2218,7 +2224,7 @@
// Whole dialog is disabled.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = true;
element.includeComments = false;
@@ -2236,7 +2242,7 @@
).all = [account];
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelDraftMessage = '';
+ element.patchsetLevelComment = element.createDraft('');
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2305,13 +2311,13 @@
queryAndAssert<GrComment>(element, '#patchsetLevelComment').messageText =
'hello';
- await waitUntil(() => element.patchsetLevelDraftMessage === 'hello');
+ await waitUntil(() => element.patchsetLevelComment.message === 'hello');
assert.isFalse(element.computeSendButtonDisabled());
queryAndAssert<GrComment>(element, '#patchsetLevelComment').messageText =
'';
- await waitUntil(() => element.patchsetLevelDraftMessage === '');
+ await waitUntil(() => element.patchsetLevelComment.message === '');
assert.isTrue(element.computeSendButtonDisabled());
});
@@ -2327,7 +2333,7 @@
patchsetLevelComment.messageText = 'hello world';
await waitUntil(
- () => element.patchsetLevelDraftMessage === 'hello world'
+ () => element.patchsetLevelComment.message === 'hello world'
);
const saveReviewPromise = interceptSaveReview();
@@ -2367,7 +2373,7 @@
patchsetLevelComment.messageText = 'hello world';
await waitUntil(
- () => element.patchsetLevelDraftMessage === 'hello world'
+ () => element.patchsetLevelComment.message === 'hello world'
);
assert.deepEqual(autoSaveStub.callCount, 0);
@@ -2390,7 +2396,7 @@
await waitUntil(() => element.draftCommentThreads.length === 1);
// patchset level draft as a reply is not loaded in patchsetLevel comment
- assert.equal(element.patchsetLevelDraftMessage, '');
+ assert.equal(element.patchsetLevelComment.message, '');
assert.deepEqual(element.draftCommentThreads[0].comments[0], draft);
});
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 26fa0cb..a4e26fe 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -1246,13 +1246,16 @@
}
private onPatchsetSelected(e: CustomEvent<{value: string}>) {
- const patchset = Number(e.detail.value);
- assert(!isNaN(patchset), 'selected patchset must be a number');
- this.getChecksModel().setPatchset(patchset as PatchSetNumber);
+ let patchset: number | undefined = Number(e.detail.value);
+ assert(Number.isInteger(patchset), `patchset must be integer: ${patchset}`);
+ if (patchset === this.latestPatchsetNumber) patchset = undefined;
+ this.getChecksModel().updateStateSetPatchset(
+ patchset as PatchSetNumber | undefined
+ );
}
private goToLatestPatchset() {
- this.getChecksModel().setPatchset(undefined);
+ this.getChecksModel().updateStateSetPatchset(undefined);
}
private createAttemptDropdownItems() {
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
index c72bccb..82893bc 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -3,7 +3,7 @@
* Copyright 2020 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {LitElement, css, html, PropertyValues} from 'lit';
+import {LitElement, css, html} from 'lit';
import {customElement, property, query, state} from 'lit/decorators.js';
import {
CheckResult,
@@ -22,7 +22,6 @@
import {Interaction} from '../../constants/reporting';
import {resolve} from '../../models/dependency';
import {GrChecksRuns} from './gr-checks-runs';
-import {LATEST_ATTEMPT} from '../../models/checks/checks-util';
/**
* The "Checks" tab on the Gerrit change page. Gets its data from plugins that
@@ -147,18 +146,6 @@
`;
}
- protected override updated(changedProperties: PropertyValues) {
- super.updated(changedProperties);
- if (changedProperties.has('tabState')) this.tabStateUpdated();
- }
-
- private tabStateUpdated() {
- if (!this.tabState?.checksTab) return;
- const {attempt, filter} = this.tabState.checksTab;
- this.getChecksModel().updateStateSetAttempt(attempt ?? LATEST_ATTEMPT);
- this.getChecksModel().updateStateSetRunFilter(filter ?? '');
- }
-
handleRunSelected(e: RunSelectedEvent) {
this.reporting.reportInteraction(Interaction.CHECKS_RUN_SELECTED, {
checkName: e.detail.checkName,
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 762f6b7..a35ca40 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -21,6 +21,7 @@
RepoName,
UrlEncodedCommentId,
PARENT,
+ PatchSetNumber,
} from '../../../types/common';
import {AppElement, AppElementParams} from '../../gr-app-types';
import {LocationChangeEventDetail} from '../../../types/events';
@@ -59,7 +60,11 @@
GroupViewState,
} from '../../../models/views/group';
import {DiffViewModel, DiffViewState} from '../../../models/views/diff';
-import {ChangeViewModel, ChangeViewState} from '../../../models/views/change';
+import {
+ ChangeViewModel,
+ ChangeViewState,
+ createChangeUrl,
+} from '../../../models/views/change';
import {EditViewModel, EditViewState} from '../../../models/views/edit';
import {
DashboardViewModel,
@@ -80,6 +85,7 @@
import {PluginViewModel, PluginViewState} from '../../../models/views/plugin';
import {SearchViewModel, SearchViewState} from '../../../models/views/search';
import {DashboardSection} from '../../../utils/dashboard-util';
+import {Subscription} from 'rxjs';
const RoutePattern = {
ROOT: '/',
@@ -279,6 +285,10 @@
// and for first navigation in app after loaded from server (true).
_isInitialLoad = true;
+ private subscriptions: Subscription[] = [];
+
+ private view?: GerritView;
+
constructor(
private readonly reporting: ReportingService,
private readonly routerModel: RouterModel,
@@ -295,9 +305,36 @@
private readonly repoViewModel: RepoViewModel,
private readonly searchViewModel: SearchViewModel,
private readonly settingsViewModel: SettingsViewModel
- ) {}
+ ) {
+ this.subscriptions = [
+ // TODO: Do the same for other view models.
+ // We want to make sure that the current view model state is always
+ // reflected back into the URL bar.
+ this.changeViewModel.state$.subscribe(state => {
+ if (!state) return;
+ // Note that router model view must be updated before view model state.
+ // So this check is slightly fragile, but should work.
+ if (this.view !== GerritView.CHANGE) return;
+ const browserUrl = window.location.toString();
+ const stateUrl = new URL(createChangeUrl(state), browserUrl).toString();
+ if (browserUrl !== stateUrl) {
+ page.replace(
+ stateUrl,
+ null,
+ /* init: */ false,
+ /* dispatch: */ false
+ );
+ }
+ }),
+ this.routerModel.routerView$.subscribe(view => (this.view = view)),
+ ];
+ }
- finalize(): void {}
+ finalize(): void {
+ for (const subscription of this.subscriptions) {
+ subscription.unsubscribe();
+ }
+ }
start() {
if (!this._app) {
@@ -940,6 +977,7 @@
view: GerritView.DASHBOARD,
user: ctx.params[0],
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.dashboardViewModel.setState(state);
}
@@ -976,6 +1014,7 @@
sections,
title,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.dashboardViewModel.setState(state);
return Promise.resolve();
@@ -988,6 +1027,7 @@
project,
dashboard: decodeURIComponent(ctx.params[1]) as DashboardId,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.dashboardViewModel.setState(state);
this.reporting.setRepoName(project);
@@ -1010,6 +1050,7 @@
view: GerritView.GROUP,
groupId: ctx.params[0] as GroupId,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.groupViewModel.setState(state);
}
@@ -1020,6 +1061,7 @@
detail: GroupDetailView.LOG,
groupId: ctx.params[0] as GroupId,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.groupViewModel.setState(state);
}
@@ -1030,6 +1072,7 @@
detail: GroupDetailView.MEMBERS,
groupId: ctx.params[0] as GroupId,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.groupViewModel.setState(state);
}
@@ -1042,6 +1085,7 @@
filter: null,
openCreateModal: ctx.hash === 'create',
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.adminViewModel.setState(state);
}
@@ -1053,6 +1097,7 @@
offset: ctx.params['offset'],
filter: ctx.params['filter'],
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.adminViewModel.setState(state);
}
@@ -1063,6 +1108,7 @@
adminView: AdminChildView.GROUPS,
filter: ctx.params['filter'] || null,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.adminViewModel.setState(state);
}
@@ -1086,6 +1132,7 @@
detail: RepoDetailView.COMMANDS,
repo,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
this.reporting.setRepoName(repo);
@@ -1098,6 +1145,7 @@
detail: RepoDetailView.GENERAL,
repo,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
this.reporting.setRepoName(repo);
@@ -1110,6 +1158,7 @@
detail: RepoDetailView.ACCESS,
repo,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
this.reporting.setRepoName(repo);
@@ -1122,6 +1171,7 @@
detail: RepoDetailView.DASHBOARDS,
repo,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
this.reporting.setRepoName(repo);
@@ -1135,6 +1185,7 @@
offset: ctx.params[2] || 0,
filter: null,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
}
@@ -1147,6 +1198,7 @@
offset: ctx.params['offset'],
filter: ctx.params['filter'],
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
}
@@ -1158,6 +1210,7 @@
repo: ctx.params['repo'] as RepoName,
filter: ctx.params['filter'] || null,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
}
@@ -1170,6 +1223,7 @@
offset: ctx.params[2] || 0,
filter: null,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
}
@@ -1182,6 +1236,7 @@
offset: ctx.params['offset'],
filter: ctx.params['filter'],
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
}
@@ -1193,6 +1248,7 @@
repo: ctx.params['repo'] as RepoName,
filter: ctx.params['filter'] || null,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
}
@@ -1205,6 +1261,7 @@
filter: null,
openCreateModal: ctx.hash === 'create',
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.adminViewModel.setState(state);
}
@@ -1216,6 +1273,7 @@
offset: ctx.params['offset'],
filter: ctx.params['filter'],
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.adminViewModel.setState(state);
}
@@ -1226,6 +1284,7 @@
adminView: AdminChildView.REPOS,
filter: ctx.params['filter'] || null,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.adminViewModel.setState(state);
}
@@ -1253,6 +1312,7 @@
offset: ctx.params[1] || 0,
filter: null,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.adminViewModel.setState(state);
}
@@ -1264,6 +1324,7 @@
offset: ctx.params['offset'],
filter: ctx.params['filter'],
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.adminViewModel.setState(state);
}
@@ -1274,6 +1335,7 @@
adminView: AdminChildView.PLUGINS,
filter: ctx.params['filter'] || null,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.adminViewModel.setState(state);
}
@@ -1283,6 +1345,7 @@
view: GerritView.ADMIN,
adminView: AdminChildView.PLUGINS,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.adminViewModel.setState(state);
}
@@ -1293,6 +1356,7 @@
query: ctx.params[0],
offset: ctx.params[2],
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.searchViewModel.setState(state);
}
@@ -1305,6 +1369,7 @@
view: GerritView.SEARCH,
query: ctx.params[0],
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.searchViewModel.setState(state);
}
@@ -1329,26 +1394,15 @@
};
const queryMap = new URLSearchParams(ctx.querystring);
- if (queryMap.has('forceReload')) {
- state.forceReload = true;
- history.replaceState(
- null,
- '',
- location.href.replace(/[?&]forceReload=true/, '')
- );
- }
-
- if (queryMap.has('openReplyDialog')) {
- state.openReplyDialog = true;
- history.replaceState(
- null,
- '',
- location.href.replace(/[?&]openReplyDialog=true/, '')
- );
- }
+ if (queryMap.has('forceReload')) state.forceReload = true;
+ if (queryMap.has('openReplyDialog')) state.openReplyDialog = true;
const tab = queryMap.get('tab');
if (tab) state.tab = tab;
+ const checksPatchset = Number(queryMap.get('checksPatchset'));
+ if (Number.isInteger(checksPatchset) && checksPatchset > 0) {
+ state.checksPatchset = checksPatchset as PatchSetNumber;
+ }
const filter = queryMap.get('filter');
if (filter) state.filter = filter;
const attempt = stringToAttemptChoice(queryMap.get('attempt'));
@@ -1358,6 +1412,7 @@
this.reporting.setRepoName(state.project);
this.reporting.setChangeId(changeNum);
this.normalizePatchRangeParams(state);
+ // Note that router model view must be updated before view models.
this.setState(state);
this.changeViewModel.setState(state);
}
@@ -1374,6 +1429,7 @@
this.reporting.setRepoName(state.project ?? '');
this.reporting.setChangeId(changeNum);
this.normalizePatchRangeParams(state);
+ // Note that router model view must be updated before view models.
this.setState(state);
this.diffViewModel.setState(state);
}
@@ -1390,6 +1446,7 @@
this.reporting.setRepoName(state.project);
this.reporting.setChangeId(changeNum);
this.normalizePatchRangeParams(state);
+ // Note that router model view must be updated before view models.
this.setState(state);
this.changeViewModel.setState(state);
}
@@ -1413,6 +1470,7 @@
this.reporting.setRepoName(state.project ?? '');
this.reporting.setChangeId(changeNum);
this.normalizePatchRangeParams(state);
+ // Note that router model view must be updated before view models.
this.setState(state);
this.diffViewModel.setState(state);
}
@@ -1452,6 +1510,7 @@
view: GerritView.EDIT,
};
this.normalizePatchRangeParams(state);
+ // Note that router model view must be updated before view models.
this.setState(state);
this.editViewModel.setState(state);
this.reporting.setRepoName(project);
@@ -1480,6 +1539,7 @@
);
}
this.normalizePatchRangeParams(state);
+ // Note that router model view must be updated before view models.
this.setState(state);
this.changeViewModel.setState(state);
this.reporting.setRepoName(project);
@@ -1494,6 +1554,7 @@
const state: AgreementViewState = {
view: GerritView.AGREEMENTS,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.agreementViewModel.setState(state);
}
@@ -1507,12 +1568,14 @@
view: GerritView.SETTINGS,
emailToken: token,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.settingsViewModel.setState(state);
}
handleSettingsRoute(_: PageContext) {
const state: SettingsViewState = {view: GerritView.SETTINGS};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.settingsViewModel.setState(state);
}
@@ -1558,6 +1621,7 @@
plugin: ctx.params[0],
screen: ctx.params[1],
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.pluginViewModel.setState(state);
}
@@ -1567,6 +1631,7 @@
view: GerritView.DOCUMENTATION_SEARCH,
filter: ctx.params['filter'] || null,
};
+ // Note that router model view must be updated before view models.
this.setState(state);
this.documentationViewModel.setState(state);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index eb9dc41..78a1509 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -452,7 +452,7 @@
unresolved: this.unresolved,
saving: this.saving,
};
- return this.patched.html`
+ return html`
${this.renderFilePath()}
<div id="container">
<h3 class="assistive-tech-only">${this.computeAriaHeading()}</h3>
@@ -504,7 +504,7 @@
// because we ran into spurious issues with <gr-comment> being destroyed
// and re-created when an unsaved draft transitions to 'saved' state.
const draftComment = this.renderComment(this.getDraftOrUnsaved());
- return this.patched.html`${publishedComments}${draftComment}`;
+ return html`${publishedComments}${draftComment}`;
}
private renderComment(comment?: Comment) {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index ad2daf5..de3d4604 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -508,14 +508,22 @@
if (isUnsaved(this.comment) && !this.editing) return;
const classes = {container: true, draft: isDraftOrUnsaved(this.comment)};
return html`
- <div id="container" class=${classMap(classes)}>
- ${this.renderHeader()}
- <div class="body">
- ${this.renderRobotAuthor()} ${this.renderEditingTextarea()}
- ${this.renderCommentMessage()} ${this.renderHumanActions()}
- ${this.renderRobotActions()} ${this.renderSuggestEditActions()}
+ <gr-endpoint-decorator name="comment">
+ <gr-endpoint-param name="comment" .value=${this.comment}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="editing" .value=${this.editing}>
+ </gr-endpoint-param>
+ <div id="container" class=${classMap(classes)}>
+ ${this.renderHeader()}
+ <div class="body">
+ ${this.renderRobotAuthor()} ${this.renderEditingTextarea()}
+ ${this.renderCommentMessage()}
+ <gr-endpoint-slot name="above-actions"></gr-endpoint-slot>
+ ${this.renderHumanActions()} ${this.renderRobotActions()}
+ ${this.renderSuggestEditActions()}
+ </div>
</div>
- </div>
+ </gr-endpoint-decorator>
${this.renderConfirmDialog()}
`;
}
@@ -968,8 +976,16 @@
override willUpdate(changed: PropertyValues) {
this.firstWillUpdate();
+ if (changed.has('editing') || changed.has('comment')) {
+ this.reflectCommentToInternalFields();
+ }
if (changed.has('editing')) {
- this.onEditingChanged();
+ // Parent components such as the reply dialog might be interested in whether
+ // come of their child components are in editing mode.
+ fire(this, 'comment-editing-changed', {
+ editing: this.editing,
+ path: this.comment?.path ?? '',
+ });
}
if (changed.has('unresolved')) {
// The <gr-comment-thread> component wants to change its color based on
@@ -1047,22 +1063,14 @@
throw new Error('unable to create preview fix event');
}
- private onEditingChanged() {
- if (this.editing) {
- this.collapsed = false;
- this.messageText = this.comment?.message ?? '';
- this.unresolved = this.comment?.unresolved ?? true;
- this.originalMessage = this.messageText;
- this.originalUnresolved = this.unresolved;
- setTimeout(() => this.textarea?.putCursorAtEnd(), 1);
- }
-
- // Parent components such as the reply dialog might be interested in whether
- // come of their child components are in editing mode.
- fire(this, 'comment-editing-changed', {
- editing: this.editing,
- path: this.comment?.path ?? '',
- });
+ private reflectCommentToInternalFields() {
+ if (!this.editing) return;
+ this.collapsed = false;
+ this.messageText = this.comment?.message ?? '';
+ this.unresolved = this.comment?.unresolved ?? true;
+ this.originalMessage = this.messageText;
+ this.originalUnresolved = this.unresolved;
+ setTimeout(() => this.textarea?.putCursorAtEnd(), 1);
}
// private, but visible for testing
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 5686c6b..1b48658 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -92,26 +92,32 @@
assert.shadowDom.equal(
initiallyCollapsedElement,
/* HTML */ `
- <div class="container" id="container">
- <div class="header" id="header">
- <div class="headerLeft">
- <gr-account-label deselected=""></gr-account-label>
+ <gr-endpoint-decorator name="comment">
+ <gr-endpoint-param name="comment"></gr-endpoint-param>
+ <gr-endpoint-param name="editing"></gr-endpoint-param>
+ <div class="container" id="container">
+ <div class="header" id="header">
+ <div class="headerLeft">
+ <gr-account-label deselected=""></gr-account-label>
+ </div>
+ <div class="headerMiddle">
+ <span class="collapsedContent">
+ This is the test comment message.
+ </span>
+ </div>
+ <span class="patchset-text">Patchset 1</span>
+ <div class="show-hide" tabindex="0">
+ <label aria-label="Expand" class="show-hide">
+ <input checked="" class="show-hide" type="checkbox" />
+ <gr-icon id="icon" icon="expand_more"></gr-icon>
+ </label>
+ </div>
</div>
- <div class="headerMiddle">
- <span class="collapsedContent">
- This is the test comment message.
- </span>
- </div>
- <span class="patchset-text">Patchset 1</span>
- <div class="show-hide" tabindex="0">
- <label aria-label="Expand" class="show-hide">
- <input checked="" class="show-hide" type="checkbox" />
- <gr-icon id="icon" icon="expand_more"></gr-icon>
- </label>
+ <div class="body">
+ <gr-endpoint-slot name="above-actions"></gr-endpoint-slot>
</div>
</div>
- <div class="body"></div>
- </div>
+ </gr-endpoint-decorator>
`
);
});
@@ -122,28 +128,33 @@
assert.shadowDom.equal(
element,
/* HTML */ `
- <div class="container" id="container">
- <div class="header" id="header">
- <div class="headerLeft">
- <gr-account-label deselected=""></gr-account-label>
+ <gr-endpoint-decorator name="comment">
+ <gr-endpoint-param name="comment"></gr-endpoint-param>
+ <gr-endpoint-param name="editing"></gr-endpoint-param>
+ <div class="container" id="container">
+ <div class="header" id="header">
+ <div class="headerLeft">
+ <gr-account-label deselected=""></gr-account-label>
+ </div>
+ <div class="headerMiddle"></div>
+ <span class="patchset-text">Patchset 1</span>
+ <span class="separator"></span>
+ <span class="date" tabindex="0">
+ <gr-date-formatter withtooltip=""></gr-date-formatter>
+ </span>
+ <div class="show-hide" tabindex="0">
+ <label aria-label="Collapse" class="show-hide">
+ <input class="show-hide" type="checkbox" />
+ <gr-icon id="icon" icon="expand_less"></gr-icon>
+ </label>
+ </div>
</div>
- <div class="headerMiddle"></div>
- <span class="patchset-text">Patchset 1</span>
- <span class="separator"></span>
- <span class="date" tabindex="0">
- <gr-date-formatter withtooltip=""></gr-date-formatter>
- </span>
- <div class="show-hide" tabindex="0">
- <label aria-label="Collapse" class="show-hide">
- <input class="show-hide" type="checkbox" />
- <gr-icon id="icon" icon="expand_less"></gr-icon>
- </label>
+ <div class="body">
+ <gr-formatted-text class="message"></gr-formatted-text>
+ <gr-endpoint-slot name="above-actions"></gr-endpoint-slot>
</div>
</div>
- <div class="body">
- <gr-formatted-text class="message"></gr-formatted-text>
- </div>
- </div>
+ </gr-endpoint-decorator>
`
);
});
@@ -155,60 +166,65 @@
assert.shadowDom.equal(
element,
/* HTML */ `
- <div class="container" id="container">
- <div class="header" id="header">
- <div class="headerLeft">
- <span class="robotName">robot-id-123</span>
+ <gr-endpoint-decorator name="comment">
+ <gr-endpoint-param name="comment"></gr-endpoint-param>
+ <gr-endpoint-param name="editing"></gr-endpoint-param>
+ <div class="container" id="container">
+ <div class="header" id="header">
+ <div class="headerLeft">
+ <span class="robotName">robot-id-123</span>
+ </div>
+ <div class="headerMiddle"></div>
+ <span class="patchset-text">Patchset 1</span>
+ <span class="separator"></span>
+ <span class="date" tabindex="0">
+ <gr-date-formatter withtooltip=""></gr-date-formatter>
+ </span>
+ <div class="show-hide" tabindex="0">
+ <label aria-label="Collapse" class="show-hide">
+ <input class="show-hide" type="checkbox" />
+ <gr-icon id="icon" icon="expand_less"></gr-icon>
+ </label>
+ </div>
</div>
- <div class="headerMiddle"></div>
- <span class="patchset-text">Patchset 1</span>
- <span class="separator"></span>
- <span class="date" tabindex="0">
- <gr-date-formatter withtooltip=""></gr-date-formatter>
- </span>
- <div class="show-hide" tabindex="0">
- <label aria-label="Collapse" class="show-hide">
- <input class="show-hide" type="checkbox" />
- <gr-icon id="icon" icon="expand_less"></gr-icon>
- </label>
+ <div class="body">
+ <div class="robotId"></div>
+ <gr-formatted-text class="message"></gr-formatted-text>
+ <gr-endpoint-slot name="above-actions"></gr-endpoint-slot>
+ <div class="robotActions">
+ <gr-icon
+ icon="link"
+ class="copy link-icon"
+ role="button"
+ tabindex="0"
+ title="Copy link to this comment"
+ ></gr-icon>
+ <gr-endpoint-decorator name="robot-comment-controls">
+ <gr-endpoint-param name="comment"></gr-endpoint-param>
+ </gr-endpoint-decorator>
+ <gr-button
+ aria-disabled="false"
+ class="action show-fix"
+ link=""
+ role="button"
+ secondary=""
+ tabindex="0"
+ >
+ Show Fix
+ </gr-button>
+ <gr-button
+ aria-disabled="false"
+ class="action fix"
+ link=""
+ role="button"
+ tabindex="0"
+ >
+ Please Fix
+ </gr-button>
+ </div>
</div>
</div>
- <div class="body">
- <div class="robotId"></div>
- <gr-formatted-text class="message"></gr-formatted-text>
- <div class="robotActions">
- <gr-icon
- icon="link"
- class="copy link-icon"
- role="button"
- tabindex="0"
- title="Copy link to this comment"
- ></gr-icon>
- <gr-endpoint-decorator name="robot-comment-controls">
- <gr-endpoint-param name="comment"></gr-endpoint-param>
- </gr-endpoint-decorator>
- <gr-button
- aria-disabled="false"
- class="action show-fix"
- link=""
- role="button"
- secondary=""
- tabindex="0"
- >
- Show Fix
- </gr-button>
- <gr-button
- aria-disabled="false"
- class="action fix"
- link=""
- role="button"
- tabindex="0"
- >
- Please Fix
- </gr-button>
- </div>
- </div>
- </div>
+ </gr-endpoint-decorator>
`
);
});
@@ -242,64 +258,69 @@
assert.shadowDom.equal(
element,
/* HTML */ `
- <div class="container draft" id="container">
- <div class="header" id="header">
- <div class="headerLeft">
- <gr-tooltip-content
- class="draftTooltip"
- has-tooltip=""
- max-width="20em"
- title="This draft is only visible to you. To publish drafts, click the 'Reply' or 'Start review' button at the top of the change or press the 'a' key."
- >
- <gr-icon filled icon="rate_review"></gr-icon>
- <span class="draftLabel">Draft</span>
- </gr-tooltip-content>
- </div>
- <div class="headerMiddle"></div>
- <span class="patchset-text">Patchset 1</span>
- <span class="separator"></span>
- <span class="date" tabindex="0">
- <gr-date-formatter withtooltip=""></gr-date-formatter>
- </span>
- <div class="show-hide" tabindex="0">
- <label aria-label="Collapse" class="show-hide">
- <input class="show-hide" type="checkbox" />
- <gr-icon id="icon" icon="expand_less"></gr-icon>
- </label>
- </div>
- </div>
- <div class="body">
- <gr-formatted-text class="message"></gr-formatted-text>
- <div class="actions">
- <div class="action resolve">
- <label>
- <input checked="" id="resolvedCheckbox" type="checkbox" />
- Resolved
+ <gr-endpoint-decorator name="comment">
+ <gr-endpoint-param name="comment"></gr-endpoint-param>
+ <gr-endpoint-param name="editing"></gr-endpoint-param>
+ <div class="container draft" id="container">
+ <div class="header" id="header">
+ <div class="headerLeft">
+ <gr-tooltip-content
+ class="draftTooltip"
+ has-tooltip=""
+ max-width="20em"
+ title="This draft is only visible to you. To publish drafts, click the 'Reply' or 'Start review' button at the top of the change or press the 'a' key."
+ >
+ <gr-icon filled icon="rate_review"></gr-icon>
+ <span class="draftLabel">Draft</span>
+ </gr-tooltip-content>
+ </div>
+ <div class="headerMiddle"></div>
+ <span class="patchset-text">Patchset 1</span>
+ <span class="separator"></span>
+ <span class="date" tabindex="0">
+ <gr-date-formatter withtooltip=""></gr-date-formatter>
+ </span>
+ <div class="show-hide" tabindex="0">
+ <label aria-label="Collapse" class="show-hide">
+ <input class="show-hide" type="checkbox" />
+ <gr-icon id="icon" icon="expand_less"></gr-icon>
</label>
</div>
- <div class="rightActions">
- <gr-button
- aria-disabled="false"
- class="action discard"
- link=""
- role="button"
- tabindex="0"
- >
- Discard
- </gr-button>
- <gr-button
- aria-disabled="false"
- class="action edit"
- link=""
- role="button"
- tabindex="0"
- >
- Edit
- </gr-button>
+ </div>
+ <div class="body">
+ <gr-formatted-text class="message"></gr-formatted-text>
+ <gr-endpoint-slot name="above-actions"></gr-endpoint-slot>
+ <div class="actions">
+ <div class="action resolve">
+ <label>
+ <input checked="" id="resolvedCheckbox" type="checkbox" />
+ Resolved
+ </label>
+ </div>
+ <div class="rightActions">
+ <gr-button
+ aria-disabled="false"
+ class="action discard"
+ link=""
+ role="button"
+ tabindex="0"
+ >
+ Discard
+ </gr-button>
+ <gr-button
+ aria-disabled="false"
+ class="action edit"
+ link=""
+ role="button"
+ tabindex="0"
+ >
+ Edit
+ </gr-button>
+ </div>
</div>
</div>
</div>
- </div>
+ </gr-endpoint-decorator>
`
);
});
@@ -312,72 +333,77 @@
assert.shadowDom.equal(
element,
/* HTML */ `
- <div class="container draft" id="container">
- <div class="header" id="header">
- <div class="headerLeft">
- <gr-tooltip-content
- class="draftTooltip"
- has-tooltip=""
- max-width="20em"
- title="This draft is only visible to you. To publish drafts, click the 'Reply' or 'Start review' button at the top of the change or press the 'a' key."
- >
- <gr-icon filled icon="rate_review"></gr-icon>
- <span class="draftLabel">Draft</span>
- </gr-tooltip-content>
- </div>
- <div class="headerMiddle"></div>
- <span class="patchset-text">Patchset 1</span>
- <span class="separator"></span>
- <span class="date" tabindex="0">
- <gr-date-formatter withtooltip=""></gr-date-formatter>
- </span>
- <div class="show-hide" tabindex="0">
- <label aria-label="Collapse" class="show-hide">
- <input class="show-hide" type="checkbox" />
- <gr-icon id="icon" icon="expand_less"></gr-icon>
- </label>
- </div>
- </div>
- <div class="body">
- <gr-textarea
- autocomplete="on"
- class="code editMessage"
- code=""
- id="editTextarea"
- rows="4"
- text="This is the test comment message."
- >
- </gr-textarea>
- <div class="actions">
- <div class="action resolve">
- <label>
- <input checked="" id="resolvedCheckbox" type="checkbox" />
- Resolved
+ <gr-endpoint-decorator name="comment">
+ <gr-endpoint-param name="comment"></gr-endpoint-param>
+ <gr-endpoint-param name="editing"></gr-endpoint-param>
+ <div class="container draft" id="container">
+ <div class="header" id="header">
+ <div class="headerLeft">
+ <gr-tooltip-content
+ class="draftTooltip"
+ has-tooltip=""
+ max-width="20em"
+ title="This draft is only visible to you. To publish drafts, click the 'Reply' or 'Start review' button at the top of the change or press the 'a' key."
+ >
+ <gr-icon filled icon="rate_review"></gr-icon>
+ <span class="draftLabel">Draft</span>
+ </gr-tooltip-content>
+ </div>
+ <div class="headerMiddle"></div>
+ <span class="patchset-text">Patchset 1</span>
+ <span class="separator"></span>
+ <span class="date" tabindex="0">
+ <gr-date-formatter withtooltip=""></gr-date-formatter>
+ </span>
+ <div class="show-hide" tabindex="0">
+ <label aria-label="Collapse" class="show-hide">
+ <input class="show-hide" type="checkbox" />
+ <gr-icon id="icon" icon="expand_less"></gr-icon>
</label>
</div>
- <div class="rightActions">
- <gr-button
- aria-disabled="false"
- class="action cancel"
- link=""
- role="button"
- tabindex="0"
- >
- Cancel
- </gr-button>
- <gr-button
- aria-disabled="false"
- class="action save"
- link=""
- role="button"
- tabindex="0"
- >
- Save
- </gr-button>
+ </div>
+ <div class="body">
+ <gr-textarea
+ autocomplete="on"
+ class="code editMessage"
+ code=""
+ id="editTextarea"
+ rows="4"
+ text="This is the test comment message."
+ >
+ </gr-textarea>
+ <gr-endpoint-slot name="above-actions"></gr-endpoint-slot>
+ <div class="actions">
+ <div class="action resolve">
+ <label>
+ <input checked="" id="resolvedCheckbox" type="checkbox" />
+ Resolved
+ </label>
+ </div>
+ <div class="rightActions">
+ <gr-button
+ aria-disabled="false"
+ class="action cancel"
+ link=""
+ role="button"
+ tabindex="0"
+ >
+ Cancel
+ </gr-button>
+ <gr-button
+ aria-disabled="false"
+ class="action save"
+ link=""
+ role="button"
+ tabindex="0"
+ >
+ Save
+ </gr-button>
+ </div>
</div>
</div>
</div>
- </div>
+ </gr-endpoint-decorator>
`
);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
index 8b651f1..d6a4d94 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
@@ -370,7 +370,10 @@
content = this.content || '';
}
- this.newContent = content;
+ // TODO(wyatta) switch linkify sequence, see issue 5526.
+ this.newContent = this.removeZeroWidthSpace
+ ? content.replace(/^R=\u200B/gm, 'R=')
+ : content;
}
computeSaveDisabled(): boolean {
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index da90b17..6ab0ec4 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -154,7 +154,17 @@
// for this.
// 4. Rewrite plain text ("text") to apply linking and other config-based
// rewrites. Text within code blocks is not passed here.
+ // 5. Open links in a new tab by rendering with target="_blank" attribute.
function customRenderer(renderer: {[type: string]: Function}) {
+ renderer['link'] = (href: string, title: string, text: string) =>
+ /* HTML */
+ `<a
+ href="${href}"
+ target="_blank"
+ ${title ? `title="${title}"` : ''}
+ rel="noopener"
+ >${text}</a
+ >`;
renderer['image'] = (href: string, _title: string, text: string) =>
`![${text}](${href})`;
renderer['codespan'] = (text: string) =>
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
index f3c9d9c..6391347 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
@@ -51,7 +51,15 @@
match: 'HTMLRewriteMe',
html: '<div>HTMLRewritten</div>',
},
+ complexLinkRewrite: {
+ match: '(^|\\s)A Link (\\d+)($|\\s)',
+ link: '/page?id=$2',
+ text: 'Link $2',
+ prefix: '$1A ',
+ suffix: '$3',
+ },
});
+ self.CANONICAL_PATH = 'http://localhost';
element = (
await fixture(
wrapInProvider(
@@ -72,6 +80,7 @@
test('renders text with links and rewrites', async () => {
element.content = `text with plain link: google.com
\ntext with config link: LinkRewriteMe
+ \ntext with complex link: A Link 12
\ntext with config html: HTMLRewriteMe`;
await element.updateComplete;
@@ -91,6 +100,14 @@
>
LinkRewriteMe
</a>
+ text with complex link: A
+ <a
+ href="http://localhost/page?id=12"
+ rel="noopener"
+ target="_blank"
+ >
+ Link 12
+ </a>
text with config html:
<div>HTMLRewritten</div>
</pre>
@@ -129,6 +146,8 @@
element.content = `text
\ntext with plain link: google.com
\ntext with config link: LinkRewriteMe
+ \ntext without a link: NotA Link 15 cats
+ \ntext with complex link: A Link 12
\ntext with config html: HTMLRewriteMe`;
await element.updateComplete;
@@ -154,6 +173,17 @@
LinkRewriteMe
</a>
</p>
+ <p>text without a link: NotA Link 15 cats</p>
+ <p>
+ text with complex link: A
+ <a
+ href="http://localhost/page?id=12"
+ rel="noopener"
+ target="_blank"
+ >
+ Link 12
+ </a>
+ </p>
<p>text with config html:</p>
<div>HTMLRewritten</div>
<p></p>
@@ -302,7 +332,13 @@
<div slot="markdown-html">
<p>
@
- <a href="mailto:someone@google.com"> someone@google.com </a>
+ <a
+ href="mailto:someone@google.com"
+ rel="noopener"
+ target="_blank"
+ >
+ someone@google.com
+ </a>
</p>
</div>
</marked-element>
@@ -353,7 +389,13 @@
<div slot="markdown-html">
<p>
<code>@</code>
- <a href="mailto:someone@google.com"> someone@google.com </a>
+ <a
+ href="mailto:someone@google.com"
+ rel="noopener"
+ target="_blank"
+ >
+ someone@google.com
+ </a>
</p>
</div>
</marked-element>
@@ -371,7 +413,9 @@
<marked-element>
<div slot="markdown-html">
<p>
- <a href="https://www.google.com">myLink</a>
+ <a href="https://www.google.com" rel="noopener" target="_blank"
+ >myLink</a
+ >
</p>
</div>
</marked-element>
@@ -452,7 +496,9 @@
<p>block quote ${escapedDiv}</p>
</blockquote>
<p>
- <a href="http://google.com">inline link ${escapedDiv}</a>
+ <a href="http://google.com" rel="noopener" target="_blank"
+ >inline link ${escapedDiv}</a
+ >
</p>
</div>
</marked-element>
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
new file mode 100644
index 0000000..16a60e7
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
@@ -0,0 +1,178 @@
+/**
+ * @license
+ * Copyright 2015 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../styles/shared-styles';
+import {GrLinkTextParser, LinkTextParserConfig} from './link-text-parser';
+import {LitElement, css, html, PropertyValues} from 'lit';
+import {customElement, property} from 'lit/decorators.js';
+import {assertIsDefined} from '../../../utils/common-util';
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-linked-text': GrLinkedText;
+ }
+}
+
+@customElement('gr-linked-text')
+export class GrLinkedText extends LitElement {
+ private outputElement?: HTMLSpanElement;
+
+ @property({type: Boolean, attribute: 'remove-zero-width-space'})
+ removeZeroWidthSpace?: boolean;
+
+ @property({type: String})
+ content = '';
+
+ @property({type: Boolean, attribute: true})
+ pre = false;
+
+ @property({type: Boolean, attribute: true})
+ disabled = false;
+
+ @property({type: Boolean, attribute: true})
+ inline = false;
+
+ @property({type: Object})
+ config?: LinkTextParserConfig;
+
+ static override get styles() {
+ return css`
+ :host {
+ display: block;
+ }
+ :host([inline]) {
+ display: inline;
+ }
+ :host([pre]) ::slotted(span) {
+ white-space: var(--linked-text-white-space, pre-wrap);
+ word-wrap: var(--linked-text-word-wrap, break-word);
+ }
+ `;
+ }
+
+ override render() {
+ return html`<slot name="insert"></slot>`;
+ }
+
+ // NOTE: LinkTextParser dynamically creates HTML fragments based on backend
+ // configuration commentLinks. These commentLinks can contain arbitrary HTML
+ // fragments. This means that arbitrary HTML needs to be injected into the
+ // DOM-tree, where this HTML is is controlled on the server-side in the
+ // server-configuration rather than by arbitrary users.
+ // To enable this injection of 'unsafe' HTML, LinkTextParser generates
+ // HTML fragments. Lit does not support inserting html fragments directly
+ // into its DOM-tree as it controls the DOM-tree that it generates.
+ // Therefore, to get around this we create a single element that we slot into
+ // the Lit-owned DOM. This element will not be part of this LitElement as
+ // it's slotted in and thus can be modified on the fly by handleParseResult.
+ override firstUpdated(_changedProperties: PropertyValues): void {
+ this.outputElement = document.createElement('span');
+ this.outputElement.id = 'output';
+ this.outputElement.slot = 'insert';
+ this.append(this.outputElement);
+ }
+
+ override updated(changedProperties: PropertyValues): void {
+ if (changedProperties.has('content') || changedProperties.has('config')) {
+ this._contentOrConfigChanged();
+ } else if (changedProperties.has('disabled')) {
+ this.styleLinks();
+ }
+ }
+
+ /**
+ * Because either the source text or the linkification config has changed,
+ * the content should be re-parsed.
+ * Private but used in tests.
+ *
+ * @param content The raw, un-linkified source string to parse.
+ * @param config The server config specifying commentLink patterns
+ */
+ _contentOrConfigChanged() {
+ if (!this.config) {
+ assertIsDefined(this.outputElement);
+ this.outputElement.textContent = this.content;
+ return;
+ }
+
+ assertIsDefined(this.outputElement);
+ this.outputElement.textContent = '';
+ const parser = new GrLinkTextParser(
+ this.config,
+ (text: string | null, href: string | null, fragment?: DocumentFragment) =>
+ this.handleParseResult(text, href, fragment),
+ this.removeZeroWidthSpace
+ );
+ parser.parse(this.content);
+
+ // Ensure that external links originating from HTML commentlink configs
+ // open in a new tab. @see Issue 5567
+ // Ensure links to the same host originating from commentlink configs
+ // open in the same tab. When target is not set - default is _self
+ // @see Issue 4616
+ this.outputElement.querySelectorAll('a').forEach(anchor => {
+ if (anchor.hostname === window.location.hostname) {
+ anchor.removeAttribute('target');
+ } else {
+ anchor.setAttribute('target', '_blank');
+ }
+ anchor.setAttribute('rel', 'noopener');
+ });
+
+ this.styleLinks();
+ }
+
+ /**
+ * Styles the links based on whether gr-linked-text is disabled or not
+ */
+ private styleLinks() {
+ assertIsDefined(this.outputElement);
+ this.outputElement.querySelectorAll('a').forEach(anchor => {
+ anchor.setAttribute('style', this.computeLinkStyle());
+ });
+ }
+
+ private computeLinkStyle() {
+ if (this.disabled) {
+ return `
+ color: inherit;
+ text-decoration: none;
+ pointer-events: none;
+ `;
+ } else {
+ return 'color: var(--link-color)';
+ }
+ }
+
+ /**
+ * This method is called when the GrLikTextParser emits a partial result
+ * (used as the "callback" parameter). It will be called in either of two
+ * ways:
+ * - To create a link: when called with `text` and `href` arguments, a link
+ * element should be created and attached to the resulting DOM.
+ * - To attach an arbitrary fragment: when called with only the `fragment`
+ * argument, the fragment should be attached to the resulting DOM as is.
+ */
+ private handleParseResult(
+ text: string | null,
+ href: string | null,
+ fragment?: DocumentFragment
+ ) {
+ assertIsDefined(this.outputElement);
+ const output = this.outputElement;
+ if (href) {
+ const a = document.createElement('a');
+ a.setAttribute('href', href);
+ // GrLinkTextParser either pass text and href together or
+ // only DocumentFragment - see LinkTextParserCallback
+ a.textContent = text!;
+ a.target = '_blank';
+ a.setAttribute('rel', 'noopener');
+ output.appendChild(a);
+ } else if (fragment) {
+ output.appendChild(fragment);
+ }
+ }
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
new file mode 100644
index 0000000..00e0313
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
@@ -0,0 +1,471 @@
+/**
+ * @license
+ * Copyright 2015 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+import './gr-linked-text';
+import {fixture, html, assert} from '@open-wc/testing';
+import {GrLinkedText} from './gr-linked-text';
+import {queryAndAssert} from '../../../test/test-utils';
+
+suite('gr-linked-text tests', () => {
+ let element: GrLinkedText;
+
+ let originalCanonicalPath: string | undefined;
+
+ setup(async () => {
+ originalCanonicalPath = window.CANONICAL_PATH;
+ element = await fixture<GrLinkedText>(html`
+ <gr-linked-text>
+ <div id="output"></div>
+ </gr-linked-text>
+ `);
+
+ element.config = {
+ ph: {
+ match: '([Bb]ug|[Ii]ssue)\\s*#?(\\d+)',
+ link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
+ },
+ prefixsameinlinkandpattern: {
+ match: '([Hh][Tt][Tt][Pp]example)\\s*#?(\\d+)',
+ link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
+ },
+ changeid: {
+ match: '(I[0-9a-f]{8,40})',
+ link: '#/q/$1',
+ },
+ changeid2: {
+ match: 'Change-Id: +(I[0-9a-f]{8,40})',
+ link: '#/q/$1',
+ },
+ googlesearch: {
+ match: 'google:(.+)',
+ link: 'https://bing.com/search?q=$1', // html should supersede link.
+ html: '<a href="https://google.com/search?q=$1">$1</a>',
+ },
+ hashedhtml: {
+ match: 'hash:(.+)',
+ html: '<a href="#/awesomesauce">$1</a>',
+ },
+ baseurl: {
+ match: 'test (.+)',
+ html: '<a href="/r/awesomesauce">$1</a>',
+ },
+ anotatstartwithbaseurl: {
+ match: 'a test (.+)',
+ html: '[Lookup: <a href="/r/awesomesauce">$1</a>]',
+ },
+ disabledconfig: {
+ match: 'foo:(.+)',
+ link: 'https://google.com/search?q=$1',
+ enabled: false,
+ },
+ };
+ });
+
+ teardown(() => {
+ window.CANONICAL_PATH = originalCanonicalPath;
+ });
+
+ test('render', async () => {
+ element.content =
+ 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
+ await element.updateComplete;
+ assert.lightDom.equal(
+ element,
+ /* HTML */ `
+ <div id="output"></div>
+ <span id="output" slot="insert">
+ <a
+ href="https://bugs.chromium.org/p/gerrit/issues/detail?id=3650"
+ rel="noopener"
+ style="color: var(--link-color)"
+ target="_blank"
+ >
+ https://bugs.chromium.org/p/gerrit/issues/detail?id=3650
+ </a>
+ </span>
+ `
+ );
+ });
+
+ test('URL pattern was parsed and linked.', async () => {
+ // Regular inline link.
+ const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
+ element.content = url;
+ await element.updateComplete;
+
+ const linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[0] as HTMLAnchorElement;
+ assert.equal(linkEl.target, '_blank');
+ assert.equal(linkEl.rel, 'noopener');
+ assert.equal(linkEl.href, url);
+ assert.equal(linkEl.textContent, url);
+ });
+
+ test('Bug pattern was parsed and linked', async () => {
+ // "Issue/Bug" pattern.
+ element.content = 'Issue 3650';
+ await element.updateComplete;
+
+ let linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[0] as HTMLAnchorElement;
+ const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
+ assert.equal(linkEl.target, '_blank');
+ assert.equal(linkEl.href, url);
+ assert.equal(linkEl.textContent, 'Issue 3650');
+
+ element.content = 'Bug 3650';
+ await element.updateComplete;
+
+ linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[0] as HTMLAnchorElement;
+ assert.equal(linkEl.target, '_blank');
+ assert.equal(linkEl.rel, 'noopener');
+ assert.equal(linkEl.href, url);
+ assert.equal(linkEl.textContent, 'Bug 3650');
+ });
+
+ test('Pattern with same prefix as link was correctly parsed', async () => {
+ // Pattern starts with the same prefix (`http`) as the url.
+ element.content = 'httpexample 3650';
+ await element.updateComplete;
+
+ assert.equal(queryAndAssert(element, 'span#output').childNodes.length, 1);
+ const linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[0] as HTMLAnchorElement;
+ const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
+ assert.equal(linkEl.target, '_blank');
+ assert.equal(linkEl.href, url);
+ assert.equal(linkEl.textContent, 'httpexample 3650');
+ });
+
+ test('Change-Id pattern was parsed and linked', async () => {
+ // "Change-Id:" pattern.
+ const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
+ const prefix = 'Change-Id: ';
+ element.content = prefix + changeID;
+ await element.updateComplete;
+
+ const textNode = queryAndAssert(element, 'span#output').childNodes[0];
+ const linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[1] as HTMLAnchorElement;
+ assert.equal(textNode.textContent, prefix);
+ const url = '/q/' + changeID;
+ assert.isFalse(linkEl.hasAttribute('target'));
+ // Since url is a path, the host is added automatically.
+ assert.isTrue(linkEl.href.endsWith(url));
+ assert.equal(linkEl.textContent, changeID);
+ });
+
+ test('Change-Id pattern was parsed and linked with base url', async () => {
+ window.CANONICAL_PATH = '/r';
+
+ // "Change-Id:" pattern.
+ const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
+ const prefix = 'Change-Id: ';
+ element.content = prefix + changeID;
+ await element.updateComplete;
+
+ const textNode = queryAndAssert(element, 'span#output').childNodes[0];
+ const linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[1] as HTMLAnchorElement;
+ assert.equal(textNode.textContent, prefix);
+ const url = '/r/q/' + changeID;
+ assert.isFalse(linkEl.hasAttribute('target'));
+ // Since url is a path, the host is added automatically.
+ assert.isTrue(linkEl.href.endsWith(url));
+ assert.equal(linkEl.textContent, changeID);
+ });
+
+ test('Multiple matches', async () => {
+ element.content = 'Issue 3650\nIssue 3450';
+ await element.updateComplete;
+
+ const linkEl1 = queryAndAssert(element, 'span#output')
+ .childNodes[0] as HTMLAnchorElement;
+ const linkEl2 = queryAndAssert(element, 'span#output')
+ .childNodes[2] as HTMLAnchorElement;
+
+ assert.equal(linkEl1.target, '_blank');
+ assert.equal(
+ linkEl1.href,
+ 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650'
+ );
+ assert.equal(linkEl1.textContent, 'Issue 3650');
+
+ assert.equal(linkEl2.target, '_blank');
+ assert.equal(
+ linkEl2.href,
+ 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3450'
+ );
+ assert.equal(linkEl2.textContent, 'Issue 3450');
+ });
+
+ test('Change-Id pattern parsed before bug pattern', async () => {
+ // "Change-Id:" pattern.
+ const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
+ const prefix = 'Change-Id: ';
+
+ // "Issue/Bug" pattern.
+ const bug = 'Issue 3650';
+
+ const changeUrl = '/q/' + changeID;
+ const bugUrl = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
+
+ element.content = prefix + changeID + bug;
+ await element.updateComplete;
+
+ const textNode = queryAndAssert(element, 'span#output').childNodes[0];
+ const changeLinkEl = queryAndAssert(element, 'span#output')
+ .childNodes[1] as HTMLAnchorElement;
+ const bugLinkEl = queryAndAssert(element, 'span#output')
+ .childNodes[2] as HTMLAnchorElement;
+
+ assert.equal(textNode.textContent, prefix);
+
+ assert.isFalse(changeLinkEl.hasAttribute('target'));
+ assert.isTrue(changeLinkEl.href.endsWith(changeUrl));
+ assert.equal(changeLinkEl.textContent, changeID);
+
+ assert.equal(bugLinkEl.target, '_blank');
+ assert.equal(bugLinkEl.href, bugUrl);
+ assert.equal(bugLinkEl.textContent, 'Issue 3650');
+ });
+
+ test('html field in link config', async () => {
+ element.content = 'google:do a barrel roll';
+ await element.updateComplete;
+
+ const linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[0] as HTMLAnchorElement;
+ assert.equal(
+ linkEl.getAttribute('href'),
+ 'https://google.com/search?q=do a barrel roll'
+ );
+ assert.equal(linkEl.textContent, 'do a barrel roll');
+ });
+
+ test('removing hash from links', async () => {
+ element.content = 'hash:foo';
+ await element.updateComplete;
+
+ const linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[0] as HTMLAnchorElement;
+ assert.isTrue(linkEl.href.endsWith('/awesomesauce'));
+ assert.equal(linkEl.textContent, 'foo');
+ });
+
+ test('html with base url', async () => {
+ window.CANONICAL_PATH = '/r';
+
+ element.content = 'test foo';
+ await element.updateComplete;
+
+ const linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[0] as HTMLAnchorElement;
+ assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
+ assert.equal(linkEl.textContent, 'foo');
+ });
+
+ test('a is not at start', async () => {
+ window.CANONICAL_PATH = '/r';
+
+ element.content = 'a test foo';
+ await element.updateComplete;
+
+ const linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[1] as HTMLAnchorElement;
+ assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
+ assert.equal(linkEl.textContent, 'foo');
+ });
+
+ test('hash html with base url', async () => {
+ window.CANONICAL_PATH = '/r';
+
+ element.content = 'hash:foo';
+ await element.updateComplete;
+
+ const linkEl = queryAndAssert(element, 'span#output')
+ .childNodes[0] as HTMLAnchorElement;
+ assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
+ assert.equal(linkEl.textContent, 'foo');
+ });
+
+ test('disabled config', async () => {
+ element.content = 'foo:baz';
+ await element.updateComplete;
+
+ assert.equal(queryAndAssert(element, 'span#output').innerHTML, 'foo:baz');
+ });
+
+ test('R=email labels link correctly', async () => {
+ element.removeZeroWidthSpace = true;
+ element.content = 'R=\u200Btest@google.com';
+ await element.updateComplete;
+
+ assert.equal(
+ queryAndAssert(element, 'span#output').textContent,
+ 'R=test@google.com'
+ );
+ assert.equal(
+ queryAndAssert(element, 'span#output').innerHTML.match(/(R=<a)/g)!.length,
+ 1
+ );
+ });
+
+ test('CC=email labels link correctly', async () => {
+ element.removeZeroWidthSpace = true;
+ element.content = 'CC=\u200Btest@google.com';
+ await element.updateComplete;
+
+ assert.equal(
+ queryAndAssert(element, 'span#output').textContent,
+ 'CC=test@google.com'
+ );
+ assert.equal(
+ queryAndAssert(element, 'span#output').innerHTML.match(/(CC=<a)/g)!
+ .length,
+ 1
+ );
+ });
+
+ test('only {http,https,mailto} protocols are linkified', async () => {
+ element.content = 'xx mailto:test@google.com yy';
+ await element.updateComplete;
+
+ let links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
+ assert.equal(links[0].innerHTML, 'mailto:test@google.com');
+
+ element.content = 'xx http://google.com yy';
+ await element.updateComplete;
+
+ links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'http://google.com');
+ assert.equal(links[0].innerHTML, 'http://google.com');
+
+ element.content = 'xx https://google.com yy';
+ await element.updateComplete;
+
+ links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'https://google.com');
+ assert.equal(links[0].innerHTML, 'https://google.com');
+
+ element.content = 'xx ssh://google.com yy';
+ await element.updateComplete;
+
+ links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 0);
+
+ element.content = 'xx ftp://google.com yy';
+ await element.updateComplete;
+
+ links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 0);
+ });
+
+ test('links without leading whitespace are linkified', async () => {
+ element.content = 'xx abcmailto:test@google.com yy';
+ await element.updateComplete;
+
+ assert.equal(
+ queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
+ 'xx abc'
+ );
+ let links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
+ assert.equal(links[0].innerHTML, 'mailto:test@google.com');
+
+ element.content = 'xx defhttp://google.com yy';
+ await element.updateComplete;
+
+ assert.equal(
+ queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
+ 'xx def'
+ );
+ links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'http://google.com');
+ assert.equal(links[0].innerHTML, 'http://google.com');
+
+ element.content = 'xx qwehttps://google.com yy';
+ await element.updateComplete;
+
+ assert.equal(
+ queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
+ 'xx qwe'
+ );
+ links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'https://google.com');
+ assert.equal(links[0].innerHTML, 'https://google.com');
+
+ // Non-latin character
+ element.content = 'xx абвhttps://google.com yy';
+ await element.updateComplete;
+
+ assert.equal(
+ queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
+ 'xx абв'
+ );
+ links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'https://google.com');
+ assert.equal(links[0].innerHTML, 'https://google.com');
+
+ element.content = 'xx ssh://google.com yy';
+ await element.updateComplete;
+
+ links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 0);
+
+ element.content = 'xx ftp://google.com yy';
+ await element.updateComplete;
+
+ links = queryAndAssert(element, 'span#output').querySelectorAll('a');
+ assert.equal(links.length, 0);
+ });
+
+ test('overlapping links', async () => {
+ element.config = {
+ b1: {
+ match: '(B:\\s*)(\\d+)',
+ html: '$1<a href="ftp://foo/$2">$2</a>',
+ },
+ b2: {
+ match: '(B:\\s*\\d+\\s*,\\s*)(\\d+)',
+ html: '$1<a href="ftp://foo/$2">$2</a>',
+ },
+ };
+ element.content = '- B: 123, 45';
+ await element.updateComplete;
+
+ const links = element.querySelectorAll('a');
+
+ assert.equal(links.length, 2);
+ assert.equal(
+ queryAndAssert<HTMLSpanElement>(element, 'span').textContent,
+ '- B: 123, 45'
+ );
+
+ assert.equal(links[0].href, 'ftp://foo/123');
+ assert.equal(links[0].textContent, '123');
+
+ assert.equal(links[1].href, 'ftp://foo/45');
+ assert.equal(links[1].textContent, '45');
+ });
+
+ test('_contentOrConfigChanged called with config', async () => {
+ const contentConfigStub = sinon.stub(element, '_contentOrConfigChanged');
+ element.content = 'some text';
+ await element.updateComplete;
+
+ assert.isTrue(contentConfigStub.called);
+ });
+});
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
new file mode 100644
index 0000000..73cf58b
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
@@ -0,0 +1,415 @@
+/**
+ * @license
+ * Copyright 2015 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import 'ba-linkify/ba-linkify';
+import {getBaseUrl} from '../../../utils/url-util';
+import {CommentLinkInfo} from '../../../types/common';
+
+/**
+ * Pattern describing URLs with supported protocols.
+ */
+const URL_PROTOCOL_PATTERN = /^(.*)(https?:\/\/|mailto:)/;
+
+export type LinkTextParserCallback = ((text: string, href: string) => void) &
+ ((text: null, href: null, fragment: DocumentFragment) => void);
+
+export interface CommentLinkItem {
+ position: number;
+ length: number;
+ html: HTMLAnchorElement | DocumentFragment;
+}
+
+export type LinkTextParserConfig = {[name: string]: CommentLinkInfo};
+
+export class GrLinkTextParser {
+ private readonly baseUrl = getBaseUrl();
+
+ /**
+ * Construct a parser for linkifying text. Will linkify plain URLs that appear
+ * in the text as well as custom links if any are specified in the linkConfig
+ * parameter.
+ *
+ * @param linkConfig Comment links as specified by the commentlinks field on a
+ * project config.
+ * @param callback The callback to be fired when an intermediate parse result
+ * is emitted. The callback is passed text and href strings if a link is to
+ * be created, or a document fragment otherwise.
+ * @param removeZeroWidthSpace If true, zero-width spaces will be removed from
+ * R=<email> and CC=<email> expressions.
+ */
+ constructor(
+ private readonly linkConfig: LinkTextParserConfig,
+ private readonly callback: LinkTextParserCallback,
+ private readonly removeZeroWidthSpace?: boolean
+ ) {
+ Object.preventExtensions(this);
+ }
+
+ /**
+ * Emit a callback to create a link element.
+ *
+ * @param text The text of the link.
+ * @param href The URL to use as the href of the link.
+ */
+ addText(text: string, href: string) {
+ if (!text) {
+ return;
+ }
+ this.callback(text, href);
+ }
+
+ /**
+ * Given the source text and a list of CommentLinkItem objects that were
+ * generated by the commentlinks config, emit parsing callbacks.
+ *
+ * @param text The chuml of source text over which the outputArray items range.
+ * @param outputArray The list of items to add resulting from commentlink
+ * matches.
+ */
+ processLinks(text: string, outputArray: CommentLinkItem[]) {
+ this.sortArrayReverse(outputArray);
+ const fragment = document.createDocumentFragment();
+ let cursor = text.length;
+
+ // Start inserting linkified URLs from the end of the String. That way, the
+ // string positions of the items don't change as we iterate through.
+ outputArray.forEach(item => {
+ // Add any text between the current linkified item and the item added
+ // before if it exists.
+ if (item.position + item.length !== cursor) {
+ fragment.insertBefore(
+ document.createTextNode(
+ text.slice(item.position + item.length, cursor)
+ ),
+ fragment.firstChild
+ );
+ }
+ fragment.insertBefore(item.html, fragment.firstChild);
+ cursor = item.position;
+ });
+
+ // Add the beginning portion at the end.
+ if (cursor !== 0) {
+ fragment.insertBefore(
+ document.createTextNode(text.slice(0, cursor)),
+ fragment.firstChild
+ );
+ }
+
+ this.callback(null, null, fragment);
+ }
+
+ /**
+ * Sort the given array of CommentLinkItems such that the positions are in
+ * reverse order.
+ */
+ sortArrayReverse(outputArray: CommentLinkItem[]) {
+ outputArray.sort((a, b) => b.position - a.position);
+ }
+
+ addItem(
+ text: string,
+ href: string,
+ html: null,
+ position: number,
+ length: number,
+ outputArray: CommentLinkItem[]
+ ): void;
+
+ addItem(
+ text: null,
+ href: null,
+ html: string,
+ position: number,
+ length: number,
+ outputArray: CommentLinkItem[]
+ ): void;
+
+ /**
+ * Create a CommentLinkItem and append it to the given output array. This
+ * method can be called in either of two ways:
+ * - With `text` and `href` parameters provided, and the `html` parameter
+ * passed as `null`. In this case, the new CommentLinkItem will be a link
+ * element with the given text and href value.
+ * - With the `html` paremeter provided, and the `text` and `href` parameters
+ * passed as `null`. In this case, the string of HTML will be parsed and the
+ * first resulting node will be used as the resulting content.
+ *
+ * @param text The text to use if creating a link.
+ * @param href The href to use as the URL if creating a link.
+ * @param html The html to parse and use as the result.
+ * @param position The position inside the source text where the item
+ * starts.
+ * @param length The number of characters in the source text
+ * represented by the item.
+ * @param outputArray The array to which the
+ * new item is to be appended.
+ */
+ addItem(
+ text: string | null,
+ href: string | null,
+ html: string | null,
+ position: number,
+ length: number,
+ outputArray: CommentLinkItem[]
+ ): void {
+ if (href) {
+ const a = document.createElement('a');
+ a.setAttribute('href', href);
+ a.textContent = text;
+ a.target = '_blank';
+ a.rel = 'noopener';
+ outputArray.push({
+ html: a,
+ position,
+ length,
+ });
+ } else if (html) {
+ // addItem has 2 overloads. If href is null, then html
+ // can't be null.
+ // TODO(TS): remove if(html) and keep else block without condition
+ const fragment = document.createDocumentFragment();
+ // Create temporary div to hold the nodes in.
+ const div = document.createElement('div');
+ div.innerHTML = html;
+ while (div.firstChild) {
+ fragment.appendChild(div.firstChild);
+ }
+ outputArray.push({
+ html: fragment,
+ position,
+ length,
+ });
+ }
+ }
+
+ /**
+ * Create a CommentLinkItem for a link and append it to the given output
+ * array.
+ *
+ * @param text The text for the link.
+ * @param href The href to use as the URL of the link.
+ * @param position The position inside the source text where the link
+ * starts.
+ * @param length The number of characters in the source text
+ * represented by the link.
+ * @param outputArray The array to which the
+ * new item is to be appended.
+ */
+ addLink(
+ text: string,
+ href: string,
+ position: number,
+ length: number,
+ outputArray: CommentLinkItem[]
+ ) {
+ // TODO(TS): remove !test condition
+ if (!text || this.hasOverlap(position, length, outputArray)) {
+ return;
+ }
+ if (
+ !!this.baseUrl &&
+ href.startsWith('/') &&
+ !href.startsWith(this.baseUrl)
+ ) {
+ href = this.baseUrl + href;
+ }
+ this.addItem(text, href, null, position, length, outputArray);
+ }
+
+ /**
+ * Create a CommentLinkItem specified by an HTMl string and append it to the
+ * given output array.
+ *
+ * @param html The html to parse and use as the result.
+ * @param position The position inside the source text where the item
+ * starts.
+ * @param length The number of characters in the source text
+ * represented by the item.
+ * @param outputArray The array to which the
+ * new item is to be appended.
+ */
+ addHTML(
+ html: string,
+ position: number,
+ length: number,
+ outputArray: CommentLinkItem[]
+ ) {
+ if (this.hasOverlap(position, length, outputArray)) {
+ return;
+ }
+ if (
+ !!this.baseUrl &&
+ html.match(/<a href="\//g) &&
+ !new RegExp(`<a href="${this.baseUrl}`, 'g').test(html)
+ ) {
+ html = html.replace(/<a href="\//g, `<a href="${this.baseUrl}/`);
+ }
+ this.addItem(null, null, html, position, length, outputArray);
+ }
+
+ /**
+ * Does the given range overlap with anything already in the item list.
+ */
+ hasOverlap(position: number, length: number, outputArray: CommentLinkItem[]) {
+ const endPosition = position + length;
+ for (let i = 0; i < outputArray.length; i++) {
+ const arrayItemStart = outputArray[i].position;
+ const arrayItemEnd = outputArray[i].position + outputArray[i].length;
+ if (
+ (position >= arrayItemStart && position < arrayItemEnd) ||
+ (endPosition > arrayItemStart && endPosition <= arrayItemEnd) ||
+ (position === arrayItemStart && position === arrayItemEnd)
+ ) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Parse the given source text and emit callbacks for the items that are
+ * parsed.
+ */
+ parse(text?: string | null) {
+ if (text) {
+ window.linkify(text, {
+ callback: (text: string, href?: string) => this.parseChunk(text, href),
+ });
+ }
+ }
+
+ /**
+ * Callback that is pased into the linkify function. ba-linkify will call this
+ * method in either of two ways:
+ * - With both a `text` and `href` parameter provided: this indicates that
+ * ba-linkify has found a plain URL and wants it linkified.
+ * - With only a `text` parameter provided: this represents the non-link
+ * content that lies between the links the library has found.
+ *
+ */
+ parseChunk(text: string, href?: string) {
+ // TODO(wyatta) switch linkify sequence, see issue 5526.
+ if (this.removeZeroWidthSpace) {
+ // Remove the zero-width space added in gr-change-view.
+ text = text.replace(/^(CC|R)=\u200B/gm, '$1=');
+ }
+
+ // If the href is provided then ba-linkify has recognized it as a URL. If
+ // the source text does not include a protocol, the protocol will be added
+ // by ba-linkify. Create the link if the href is provided and its protocol
+ // matches the expected pattern.
+ if (href) {
+ const result = URL_PROTOCOL_PATTERN.exec(href);
+ if (result) {
+ const prefixText = result[1];
+ if (prefixText.length > 0) {
+ // Fix for simple cases from
+ // https://bugs.chromium.org/p/gerrit/issues/detail?id=11697
+ // When leading whitespace is missed before link,
+ // linkify add this text before link as a schema name to href.
+ // We suppose, that prefixText just a single word
+ // before link and add this word as is, without processing
+ // any patterns in it.
+ this.parseLinks(prefixText, {});
+ text = text.substring(prefixText.length);
+ href = href.substring(prefixText.length);
+ }
+ this.addText(text, href);
+ return;
+ }
+ }
+ // For the sections of text that lie between the links found by
+ // ba-linkify, we search for the project-config-specified link patterns.
+ this.parseLinks(text, this.linkConfig);
+ }
+
+ /**
+ * Walk over the given source text to find matches for comemntlink patterns
+ * and emit parse result callbacks.
+ *
+ * @param text The raw source text.
+ * @param config A comment links specification object.
+ */
+ parseLinks(text: string, config: LinkTextParserConfig) {
+ // The outputArray is used to store all of the matches found for all
+ // patterns.
+ const outputArray: CommentLinkItem[] = [];
+ for (const [configName, linkInfo] of Object.entries(config)) {
+ // TODO(TS): it seems, the following line can be rewritten as:
+ // if(enabled === false || enabled === 0 || enabled === '')
+ // Should be double-checked before update
+ // eslint-disable-next-line eqeqeq
+ if (linkInfo.enabled != null && linkInfo.enabled == false) {
+ continue;
+ }
+ // PolyGerrit doesn't use hash-based navigation like the GWT UI.
+ // Account for this.
+ const html = linkInfo.html;
+ const link = linkInfo.link;
+ if (html) {
+ linkInfo.html = html.replace(/<a href="#\//g, '<a href="/');
+ } else if (link) {
+ if (link[0] === '#') {
+ linkInfo.link = link.substr(1);
+ }
+ }
+
+ const pattern = new RegExp(linkInfo.match, 'g');
+
+ let match;
+ let textToCheck = text;
+ let susbtrIndex = 0;
+
+ while ((match = pattern.exec(textToCheck))) {
+ textToCheck = textToCheck.substr(match.index + match[0].length);
+ let result = match[0].replace(
+ pattern,
+ // Either html or link has a value. Otherwise an exception is thrown
+ // in the code below.
+ (linkInfo.html || linkInfo.link)!
+ );
+
+ if (linkInfo.html) {
+ let i;
+ // Skip portion of replacement string that is equal to original to
+ // allow overlapping patterns.
+ for (i = 0; i < result.length; i++) {
+ if (result[i] !== match[0][i]) {
+ break;
+ }
+ }
+ result = result.slice(i);
+
+ this.addHTML(
+ result,
+ susbtrIndex + match.index + i,
+ match[0].length - i,
+ outputArray
+ );
+ } else if (linkInfo.link) {
+ this.addLink(
+ match[0],
+ result,
+ susbtrIndex + match.index,
+ match[0].length,
+ outputArray
+ );
+ } else {
+ throw Error(
+ 'linkconfig entry ' +
+ configName +
+ ' doesn’t contain a link or html attribute.'
+ );
+ }
+
+ // Update the substring location so we know where we are in relation to
+ // the initial full text string.
+ susbtrIndex = susbtrIndex + match.index + match[0].length;
+ }
+ }
+ this.processLinks(text, outputArray);
+ }
+}
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 b58fc3f..f706712 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
@@ -22,8 +22,11 @@
ReviewInput,
ReviewerInput,
AttentionSetInput,
+ RelatedChangeAndCommitInfo,
} from '../../types/common';
import {getUserId} from '../../utils/account-util';
+import {getChangeNumber} from '../../utils/change-util';
+import {deepEqual} from '../../utils/deep-util';
export const bulkActionsModelToken =
define<BulkActionsModel>('bulk-actions-model');
@@ -136,12 +139,12 @@
return Promise.resolve(new Response());
}
return this.restApiService.executeChangeAction(
- change._number,
+ getChangeNumber(change),
change.actions!.abandon!.method,
'/abandon',
undefined,
{message: reason ?? ''},
- () => errFn && errFn(change._number)
+ () => errFn && errFn(getChangeNumber(change))
);
});
}
@@ -152,7 +155,7 @@
const change = current.allChanges.get(changeNum)!;
if (!change) throw new Error('invalid change id');
return this.restApiService.saveChangeReview(
- change._number,
+ getChangeNumber(change),
'current',
reviewInput,
() => {
@@ -196,7 +199,7 @@
add_to_attention_set: attentionSetUpdates,
};
return this.restApiService.saveChangeReview(
- change._number,
+ getChangeNumber(change),
'current',
reviewInput
);
@@ -233,13 +236,13 @@
);
}
- async sync(changes: ChangeInfo[]) {
- const basicChanges = new Map(changes.map(c => [c._number, c]));
+ async sync(changes: (ChangeInfo | RelatedChangeAndCommitInfo)[]) {
+ const basicChanges = new Map(changes.map(c => [getChangeNumber(c), c]));
let currentState = this.getState();
const selectedChangeNums = currentState.selectedChangeNums.filter(
changeNum => basicChanges.has(changeNum)
);
- const selectableChangeNums = changes.map(c => c._number);
+ const selectableChangeNums = changes.map(c => getChangeNumber(c));
this.updateState({
loadingState: LoadingState.LOADING,
selectedChangeNums,
@@ -252,18 +255,16 @@
}
const changeDetails =
await this.restApiService.getDetailedChangesWithActions(
- changes.map(c => c._number)
+ changes.map(c => getChangeNumber(c))
);
currentState = this.getState();
// Return early if sync has been called again since starting the load.
- if (selectableChangeNums !== currentState.selectableChangeNums) return;
+ if (!deepEqual(selectableChangeNums, currentState.selectableChangeNums)) {
+ return;
+ }
const allDetailedChanges: Map<NumericChangeId, ChangeInfo> = new Map();
for (const detailedChange of changeDetails ?? []) {
- const basicChange = basicChanges.get(detailedChange._number)!;
- allDetailedChanges.set(
- detailedChange._number,
- this.mergeOldAndDetailedChangeInfos(basicChange, detailedChange)
- );
+ allDetailedChanges.set(detailedChange._number, detailedChange);
}
this.setState({
...currentState,
@@ -272,17 +273,6 @@
});
}
- private mergeOldAndDetailedChangeInfos(
- originalChange: ChangeInfo,
- newData: ChangeInfo
- ) {
- return {
- ...originalChange,
- ...newData,
- reviewers: originalChange.reviewers,
- };
- }
-
private getNewReviewersToChange(
change: ChangeInfo,
state: ReviewerState,
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 7bc37d6..e2f75f7 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
@@ -14,10 +14,8 @@
NumericChangeId,
ChangeStatus,
HttpMethod,
- SubmitRequirementStatus,
AccountInfo,
ReviewerState,
- AccountId,
GroupInfo,
Hashtag,
} from '../../api/rest-api';
@@ -491,60 +489,6 @@
);
});
- test('sync retains keys from original change including reviewers', async () => {
- const c1: ChangeInfo = {
- ...createChange(),
- _number: 1 as NumericChangeId,
- submit_requirements: [
- {
- name: 'a',
- status: SubmitRequirementStatus.FORCED,
- submittability_expression_result: {
- expression: 'b',
- },
- },
- ],
- reviewers: {
- REVIEWER: [{_account_id: 1 as AccountId, display_name: 'MyName'}],
- },
- };
-
- stubRestApi('getDetailedChangesWithActions').callsFake(() => {
- const change: ChangeInfo = {
- ...createChange(),
- _number: 1 as NumericChangeId,
- actions: {abandon: {}},
- // detailed data will be missing names
- reviewers: {REVIEWER: [createAccountWithIdNameAndEmail()]},
- };
- assert.isNotOk(change.submit_requirements);
- return Promise.resolve([change]);
- });
-
- bulkActionsModel.sync([c1]);
-
- await waitUntilObserved(
- bulkActionsModel.loadingState$,
- s => s === LoadingState.LOADED
- );
-
- const changeAfterSync = bulkActionsModel
- .getState()
- .allChanges.get(1 as NumericChangeId);
- assert.deepEqual(changeAfterSync!.submit_requirements, [
- {
- name: 'a',
- status: SubmitRequirementStatus.FORCED,
- submittability_expression_result: {
- expression: 'b',
- },
- },
- ]);
- assert.deepEqual(changeAfterSync!.actions, {abandon: {}});
- // original reviewers are kept, which includes more details than loaded ones
- assert.deepEqual(changeAfterSync!.reviewers, c1.reviewers);
- });
-
test('sync ignores outdated fetch responses', async () => {
const c1 = createChange();
c1._number = 1 as NumericChangeId;
@@ -556,7 +500,7 @@
const getChangesStub = stubRestApi(
'getDetailedChangesWithActions'
).callsFake(() => promise);
- bulkActionsModel.sync([c1, c2]);
+ bulkActionsModel.sync([c1]);
assert.strictEqual(getChangesStub.callCount, 1);
await waitUntilObserved(
bulkActionsModel.loadingState$,
@@ -590,7 +534,6 @@
// Resolve the old promise.
responsePromise1.resolve([
{...createChange(), _number: 1, subject: 'Subject 1-old'},
- {...createChange(), _number: 2, subject: 'Subject 2-old'},
] as ChangeInfo[]);
await waitEventLoop();
const model2 = bulkActionsModel.getState();
diff --git a/polygerrit-ui/app/models/checks/checks-model.ts b/polygerrit-ui/app/models/checks/checks-model.ts
index 8e058aa..2b92529 100644
--- a/polygerrit-ui/app/models/checks/checks-model.ts
+++ b/polygerrit-ui/app/models/checks/checks-model.ts
@@ -62,6 +62,7 @@
ChecksUpdate,
PluginsModel,
} from '../plugins/plugins-model';
+import {ChangeViewModel} from '../views/change';
/**
* The checks model maintains the state of checks for two patchsets: the latest
@@ -137,20 +138,6 @@
}
interface ChecksState {
- /**
- * This is the patchset number selected by the user. The *latest* patchset
- * can be picked up from the change model.
- */
- patchsetNumberSelected?: PatchSetNumber;
- /**
- * This is the attempt number selected by the user. If this is `undefined`
- * (default), then for each run the latest attempt is displayed.
- */
- attemptNumberSelected: AttemptChoice;
- /**
- * Current filter set by the user in the runs panel or via URL.
- */
- runFilterRegexp: string;
/** Checks data for the latest patchset. */
pluginStateLatest: {
[name: string]: ChecksProviderState;
@@ -210,24 +197,30 @@
private subscriptions: Subscription[] = [];
+ // TODO: Maybe consider migrating usages to `changeViewModel` directly.
public checksSelectedPatchsetNumber$ = select(
- this.state$,
- state => state.patchsetNumberSelected
+ this.changeViewModel.checksPatchset$,
+ ps => ps
);
public checksSelectedAttemptNumber$ = select(
- this.state$,
- state => state.attemptNumberSelected
+ this.changeViewModel.attempt$,
+ attempt => attempt ?? LATEST_ATTEMPT
);
- public runFilterRegexp$ = select(this.state$, state => state.runFilterRegexp);
+ public runFilterRegexp$ = select(
+ this.changeViewModel.filter$,
+ filter => filter ?? ''
+ );
public checksLatest$ = select(this.state$, state => state.pluginStateLatest);
- public checksSelected$ = select(this.state$, state =>
- state.patchsetNumberSelected
- ? state.pluginStateSelected
- : state.pluginStateLatest
+ public checksSelected$ = select(
+ combineLatest([this.state$, this.changeViewModel.checksPatchset$]),
+ ([state, ps]) => {
+ const checksPs = ps ? ChecksPatchset.SELECTED : ChecksPatchset.LATEST;
+ return this.getPluginState(state, checksPs);
+ }
);
public aPluginHasRegistered$ = select(
@@ -379,14 +372,12 @@
constructor(
readonly routerModel: RouterModel,
+ readonly changeViewModel: ChangeViewModel,
readonly changeModel: ChangeModel,
readonly reporting: ReportingService,
readonly pluginsModel: PluginsModel
) {
super({
- patchsetNumberSelected: undefined,
- attemptNumberSelected: LATEST_ATTEMPT,
- runFilterRegexp: '',
pluginStateLatest: {},
pluginStateSelected: {},
});
@@ -405,19 +396,6 @@
this.checkToPluginMap$.subscribe(map => {
this.checkToPluginMap = map;
}),
- combineLatest([
- this.routerModel.routerPatchNum$,
- this.changeModel.latestPatchNum$,
- ]).subscribe(([routerPs, latestPs]) => {
- this.latestPatchNum = latestPs;
- if (latestPs === undefined) {
- this.setPatchset(undefined);
- } else if (typeof routerPs === 'number') {
- this.setPatchset(routerPs);
- } else {
- this.setPatchset(latestPs);
- }
- }),
this.firstLoadCompleted$
.pipe(
filter(completed => !!completed),
@@ -655,20 +633,18 @@
this.setState(nextState);
}
- updateStateSetPatchset(patchsetNumberSelected?: PatchSetNumber) {
- this.updateState({patchsetNumberSelected});
+ updateStateSetPatchset(num?: PatchSetNumber) {
+ this.changeViewModel.updateState({
+ checksPatchset: num === this.latestPatchNum ? undefined : num,
+ });
}
updateStateSetAttempt(attemptNumberSelected: AttemptChoice) {
- this.updateState({attemptNumberSelected});
+ this.changeViewModel.updateState({attempt: attemptNumberSelected});
}
updateStateSetRunFilter(runFilterRegexp: string) {
- this.updateState({runFilterRegexp});
- }
-
- setPatchset(num?: PatchSetNumber) {
- this.updateStateSetPatchset(num === this.latestPatchNum ? undefined : num);
+ this.changeViewModel.updateState({filter: runFilterRegexp});
}
reload(pluginName: string) {
diff --git a/polygerrit-ui/app/models/checks/checks-model_test.ts b/polygerrit-ui/app/models/checks/checks-model_test.ts
index 88fbebc..83ed464 100644
--- a/polygerrit-ui/app/models/checks/checks-model_test.ts
+++ b/polygerrit-ui/app/models/checks/checks-model_test.ts
@@ -21,6 +21,7 @@
import {changeModelToken} from '../change/change-model';
import {assert} from '@open-wc/testing';
import {testResolver} from '../../test/common-test-setup';
+import {changeViewModelToken} from '../views/change';
const PLUGIN_NAME = 'test-plugin';
@@ -63,6 +64,7 @@
setup(() => {
model = new ChecksModel(
getAppContext().routerModel,
+ testResolver(changeViewModelToken),
testResolver(changeModelToken),
getAppContext().reportingService,
getAppContext().pluginsModel
diff --git a/polygerrit-ui/app/models/model.ts b/polygerrit-ui/app/models/model.ts
index 7cd8b2a..2e90a5e 100644
--- a/polygerrit-ui/app/models/model.ts
+++ b/polygerrit-ui/app/models/model.ts
@@ -5,6 +5,7 @@
*/
import {BehaviorSubject, Observable} from 'rxjs';
import {Finalizable} from '../services/registry';
+import {deepEqual} from '../utils/deep-util';
/**
* A Model stores a value <T> and controls changes to that value via `subject$`
@@ -35,6 +36,7 @@
}
setState(state: T) {
+ if (deepEqual(state, this.getState())) return;
this.subject$.next(state);
}
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index b8451a0..826ec66 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -9,9 +9,11 @@
RevisionPatchSetNum,
BasePatchSetNum,
ChangeInfo,
+ PatchSetNumber,
} from '../../api/rest-api';
import {GerritView} from '../../services/router/router-model';
import {UrlEncodedCommentId} from '../../types/common';
+import {select} from '../../utils/observable-util';
import {
encodeURL,
getBaseUrl,
@@ -24,22 +26,34 @@
export interface ChangeViewState extends ViewState {
view: GerritView.CHANGE;
+
changeNum: NumericChangeId;
project: RepoName;
edit?: boolean;
patchNum?: RevisionPatchSetNum;
basePatchNum?: BasePatchSetNum;
commentId?: UrlEncodedCommentId;
- forceReload?: boolean;
- openReplyDialog?: boolean;
tab?: string;
+
+ /** Checks related view state */
+
+ /** selected patchset for check runs (undefined=latest) */
+ checksPatchset?: PatchSetNumber;
/** regular expression for filtering check runs */
filter?: string;
- /** selected attempt for selected check runs */
+ /** selected attempt for check runs (undefined=latest) */
attempt?: AttemptChoice;
+ /** State properties that trigger one-time actions */
+
+ /** for scrolling a Change Log message into view in gr-change-view */
messageHash?: string;
+ /** for logging where the user came from */
usp?: string;
+ /** triggers all change related data to be reloaded */
+ forceReload?: boolean;
+ /** triggers opening the reply dialog */
+ openReplyDialog?: boolean;
}
/**
@@ -84,6 +98,15 @@
}
let suffix = `${range}`;
const queries = [];
+ if (state.checksPatchset && state.checksPatchset > 0) {
+ queries.push(`checksPatchset=${state.checksPatchset}`);
+ }
+ if (state.attempt) {
+ if (state.attempt !== 'latest') queries.push(`attempt=${state.attempt}`);
+ }
+ if (state.filter) {
+ queries.push(`filter=${state.filter}`);
+ }
if (state.forceReload) {
queries.push('forceReload=true');
}
@@ -117,7 +140,27 @@
define<ChangeViewModel>('change-view-model');
export class ChangeViewModel extends Model<ChangeViewState | undefined> {
+ public readonly tab$ = select(this.state$, state => state?.tab);
+
+ public readonly checksPatchset$ = select(
+ this.state$,
+ state => state?.checksPatchset
+ );
+
+ public readonly attempt$ = select(this.state$, state => state?.attempt);
+
+ public readonly filter$ = select(this.state$, state => state?.filter);
+
constructor() {
super(undefined);
+ this.state$.subscribe(s => {
+ if (s?.usp || s?.forceReload || s?.openReplyDialog) {
+ this.updateState({
+ usp: undefined,
+ forceReload: undefined,
+ openReplyDialog: undefined,
+ });
+ }
+ });
}
}
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 9f7a0c0..2e1b817 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -186,6 +186,7 @@
const checksModel = new ChecksModel(
appContext.routerModel,
+ changeViewModel,
changeModel,
appContext.reportingService,
appContext.pluginsModel
diff --git a/polygerrit-ui/app/services/router/router-model.ts b/polygerrit-ui/app/services/router/router-model.ts
index 0699554..99b9870 100644
--- a/polygerrit-ui/app/services/router/router-model.ts
+++ b/polygerrit-ui/app/services/router/router-model.ts
@@ -29,6 +29,7 @@
}
export interface RouterState {
+ // Note that this router model view must be updated before view model state.
view?: GerritView;
changeNum?: NumericChangeId;
patchNum?: RevisionPatchSetNum;
diff --git a/polygerrit-ui/app/test/test-app-context-init.ts b/polygerrit-ui/app/test/test-app-context-init.ts
index e34773e..2cdd4a5 100644
--- a/polygerrit-ui/app/test/test-app-context-init.ts
+++ b/polygerrit-ui/app/test/test-app-context-init.ts
@@ -207,6 +207,7 @@
const checksModelCreator = () =>
new ChecksModel(
appContext.routerModel,
+ resolver(changeViewModelToken),
resolver(changeModelToken),
appContext.reportingService,
appContext.pluginsModel
diff --git a/polygerrit-ui/app/types/events.ts b/polygerrit-ui/app/types/events.ts
index 23acff4..597bb6f 100644
--- a/polygerrit-ui/app/types/events.ts
+++ b/polygerrit-ui/app/types/events.ts
@@ -8,7 +8,6 @@
import {FetchRequest} from './types';
import {LineNumberEventDetail, MovedLinkClickedEventDetail} from '../api/diff';
import {Category, RunStatus} from '../api/checks';
-import {AttemptChoice} from '../models/checks/checks-util';
export enum EventType {
BIND_VALUE_CHANGED = 'bind-value-changed',
@@ -231,10 +230,6 @@
export interface ChecksTabState {
statusOrCategory?: RunStatus | Category;
checkName?: string;
- /** regular expression for filtering runs */
- filter?: string;
- /** selected attempt for selected runs */
- attempt?: AttemptChoice;
}
export type SwitchTabEvent = CustomEvent<SwitchTabEventDetail>;
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index ad5d48a..07b63d6 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -137,6 +137,20 @@
) {
return change?.status === ChangeStatus.ABANDONED;
}
+/**
+ * Get the change number from either a ChangeInfo (such as those included in
+ * SubmittedTogetherInfo responses) or get the change number from a
+ * RelatedChangeAndCommitInfo (such as those included in a
+ * RelatedChangesInfo response).
+ */
+export function getChangeNumber(
+ change: ChangeInfo | ParsedChangeInfo | RelatedChangeAndCommitInfo
+): NumericChangeId {
+ if (isChangeInfo(change)) {
+ return change._number;
+ }
+ return change._change_number!;
+}
export function changeStatuses(
change: ChangeInfo,
diff --git a/polygerrit-ui/app/utils/link-util.ts b/polygerrit-ui/app/utils/link-util.ts
index fd5965b..9079f4c 100644
--- a/polygerrit-ui/app/utils/link-util.ts
+++ b/polygerrit-ui/app/utils/link-util.ts
@@ -18,7 +18,7 @@
const parts: string[] = [];
window.linkify(baseWithZeroWidthSpace, {
callback: (text, href) => {
- const result = href ? createLinkTemplate(text, href) : text;
+ const result = href ? createLinkTemplate(href, text) : text;
const resultWithoutZeroWidthSpace = result.replace(/\u200B/g, '');
parts.push(resultWithoutZeroWidthSpace);
},
@@ -39,7 +39,12 @@
: rewrite.link!;
return {
match: new RegExp(rewrite.match, 'g'),
- replace: createLinkTemplate('$&', replacementHref),
+ replace: createLinkTemplate(
+ replacementHref,
+ rewrite.text ?? '$&',
+ rewrite.prefix,
+ rewrite.suffix
+ ),
};
});
return applyRewrites(base, rewrites);
@@ -71,6 +76,15 @@
);
}
-function createLinkTemplate(displayText: string, href: string) {
- return `<a href="${href}" rel="noopener" target="_blank">${displayText}</a>`;
+function createLinkTemplate(
+ href: string,
+ displayText: string,
+ prefix?: string,
+ suffix?: string
+) {
+ return `${
+ prefix ?? ''
+ }<a href="${href}" rel="noopener" target="_blank">${displayText}</a>${
+ suffix ?? ''
+ }`;
}
diff --git a/polygerrit-ui/app/utils/link-util_test.ts b/polygerrit-ui/app/utils/link-util_test.ts
index c491e35..a1ec2fa 100644
--- a/polygerrit-ui/app/utils/link-util_test.ts
+++ b/polygerrit-ui/app/utils/link-util_test.ts
@@ -30,11 +30,13 @@
'<h1>Change 12345 is the best change</h1> <div>FOO</div>'
);
});
+
test('applyLinkRewritesFromConfig', () => {
const linkedNumber = link('#12345', 'google.com/12345');
const linkedFoo = link('foo', 'foo.gov');
+ const linkedBar = link('Bar Page: 300', 'bar.com/page?id=300');
assert.equal(
- applyLinkRewritesFromConfig('#12345 foo', {
+ applyLinkRewritesFromConfig('#12345 foo crowbar:12 bar:300', {
'number-linker': {
match: '#(\\d+)',
link: 'google.com/$1',
@@ -43,8 +45,15 @@
match: 'foo',
link: 'foo.gov',
},
+ 'advanced-link': {
+ match: '(^|\\s)bar:(\\d+)($|\\s)',
+ link: 'bar.com/page?id=$2',
+ text: 'Bar Page: $2',
+ prefix: '$1',
+ suffix: '$3',
+ },
}),
- `${linkedNumber} ${linkedFoo}`
+ `${linkedNumber} ${linkedFoo} crowbar:12 ${linkedBar}`
);
});
diff --git a/polygerrit-ui/app/utils/page-wrapper-utils.ts b/polygerrit-ui/app/utils/page-wrapper-utils.ts
index 7096423..78e78ed 100644
--- a/polygerrit-ui/app/utils/page-wrapper-utils.ts
+++ b/polygerrit-ui/app/utils/page-wrapper-utils.ts
@@ -14,6 +14,7 @@
(pageCallback: PageCallback): void;
show(url: string): void;
redirect(url: string): void;
+ replace(path: string, state: null, init: boolean, dispatch: boolean): void;
base(url: string): void;
start(): void;
exit(pattern: string | RegExp, ...pageCallback: PageCallback[]): void;