Merge "Return 500 when a request is canceled due to an exceeded server deadline"
diff --git a/.settings/org.eclipse.jdt.core.prefs b/.settings/org.eclipse.jdt.core.prefs
index 09ce63b..fae2a87 100644
--- a/.settings/org.eclipse.jdt.core.prefs
+++ b/.settings/org.eclipse.jdt.core.prefs
@@ -1,4 +1,5 @@
eclipse.preferences.version=1
+org.eclipse.jdt.core.builder.annotationPath.allLocations=disabled
org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations=disabled
org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation=ignore
org.eclipse.jdt.core.compiler.annotation.nonnull=org.eclipse.jdt.annotation.NonNull
@@ -102,7 +103,7 @@
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.unclosedCloseable=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=ignore
-org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
+org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=ignore
org.eclipse.jdt.core.compiler.problem.unlikelyCollectionMethodArgumentType=warning
org.eclipse.jdt.core.compiler.problem.unlikelyCollectionMethodArgumentTypeStrict=disabled
org.eclipse.jdt.core.compiler.problem.unlikelyEqualsArgumentType=warning
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 4c666b9..3cc5a5a 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1791,7 +1791,7 @@
----
[commentlink "changeid"]
match = (I[0-9a-f]{8,40})
- link = "#/q/$1"
+ link = "/q/$1"
[commentlink "bugzilla"]
match = "(^|\\s)(bug\\s+#?)(\\d+)($|\\s)"
@@ -5545,18 +5545,6 @@
+
By default, false.
-[[tracing.exportPerformanceMetrics]]tracing.exportPerformanceMetrics::
-+
-Whether to export performance metrics.
-+
-Performace logged when link:#tracing.performanceLogging[`performanceLogging`] is
-enabled, can be exported as metrics.
-+
-NOTE: Since the payload returned could be of tens of thousands metrics,
-assess the latency of the metrics endpoint before enabling this option.
-+
-By default, false.
-
[[tracing.traceid]]
==== Subsection tracing.<trace-id>
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 6c9dfef..2f43538 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -73,30 +73,6 @@
** `cancellation_type`:
The cancellation type (graceful or forceful).
-[[performance]]
-=== Performance
-
-* `performance/operations`: Latency of performing operations
-** `operation_name`:
- The operation that was performed.
-** `request`:
- The request for which the operation was performed (format = '<request-type>
- <redacted-request-uri>').
-** `plugin`:
- The name of the plugin that performed the operation.
-* `performance/operations_count`: Number of performed operations
-** `operation_name`:
- The operation that was performed.
-** `request`:
- The request for which the operation was performed (format = '<request-type>
- <redacted-request-uri>').
-** `plugin`:
- The name of the plugin that performed the operation.
-
-Performance metrics can be enabled via the
-link:config.gerrit.html#tracing.exportPerformanceMetrics[`tracing.exportPerformanceMetrics`]
-setting.
-
=== Pushes
* `receivecommits/changes`: histogram of number of changes processed in a single
diff --git a/java/com/google/gerrit/gpg/PublicKeyChecker.java b/java/com/google/gerrit/gpg/PublicKeyChecker.java
index 5347398..946fee3 100644
--- a/java/com/google/gerrit/gpg/PublicKeyChecker.java
+++ b/java/com/google/gerrit/gpg/PublicKeyChecker.java
@@ -237,7 +237,6 @@
List<PGPSignature> revocations,
Map<Long, RevocationKey> revokers)
throws PGPException {
- @SuppressWarnings("unchecked")
Iterator<PGPSignature> allSigs = key.getSignatures();
while (allSigs.hasNext()) {
PGPSignature sig = allSigs.next();
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 0ace77e..4e11346 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -519,7 +519,6 @@
(RestReadView<RestResource>) viewData.view,
rsrc);
} else if (viewData.view instanceof RestModifyView<?, ?>) {
- @SuppressWarnings("unchecked")
RestModifyView<RestResource, Object> m =
(RestModifyView<RestResource, Object>) viewData.view;
@@ -535,7 +534,6 @@
}
}
} else if (viewData.view instanceof RestCollectionCreateView<?, ?, ?>) {
- @SuppressWarnings("unchecked")
RestCollectionCreateView<RestResource, RestResource, Object> m =
(RestCollectionCreateView<RestResource, RestResource, Object>) viewData.view;
@@ -550,7 +548,6 @@
}
}
} else if (viewData.view instanceof RestCollectionDeleteMissingView<?, ?, ?>) {
- @SuppressWarnings("unchecked")
RestCollectionDeleteMissingView<RestResource, RestResource, Object> m =
(RestCollectionDeleteMissingView<RestResource, RestResource, Object>)
viewData.view;
@@ -566,7 +563,6 @@
}
}
} else if (viewData.view instanceof RestCollectionModifyView<?, ?, ?>) {
- @SuppressWarnings("unchecked")
RestCollectionModifyView<RestResource, RestResource, Object> m =
(RestCollectionModifyView<RestResource, RestResource, Object>) viewData.view;
diff --git a/java/com/google/gerrit/server/PerformanceMetrics.java b/java/com/google/gerrit/server/PerformanceMetrics.java
deleted file mode 100644
index 845ed80..0000000
--- a/java/com/google/gerrit/server/PerformanceMetrics.java
+++ /dev/null
@@ -1,91 +0,0 @@
-// Copyright (C) 2021 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server;
-
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.metrics.Counter3;
-import com.google.gerrit.metrics.Description;
-import com.google.gerrit.metrics.Field;
-import com.google.gerrit.metrics.MetricMaker;
-import com.google.gerrit.metrics.Timer3;
-import com.google.gerrit.server.logging.Metadata;
-import com.google.gerrit.server.logging.PerformanceLogger;
-import com.google.gerrit.server.logging.TraceContext;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import java.util.concurrent.TimeUnit;
-
-/** Performance logger that records the execution times as a metric. */
-@Singleton
-public class PerformanceMetrics implements PerformanceLogger {
- private static final String OPERATION_LATENCY_METRIC_NAME = "performance/operations";
- private static final String OPERATION_COUNT_METRIC_NAME = "performance/operations_count";
-
- public final Timer3<String, String, String> operationsLatency;
- public final Counter3<String, String, String> operationsCounter;
-
- @Inject
- PerformanceMetrics(MetricMaker metricMaker) {
- Field<String> operationNameField =
- Field.ofString(
- "operation_name",
- (metadataBuilder, fieldValue) -> metadataBuilder.operationName(fieldValue))
- .description("The operation that was performed.")
- .build();
- Field<String> requestField =
- Field.ofString("request", (metadataBuilder, fieldValue) -> {})
- .description(
- "The request for which the operation was performed"
- + " (format = '<request-type> <redacted-request-uri>').")
- .build();
- Field<String> pluginField =
- Field.ofString(
- "plugin", (metadataBuilder, fieldValue) -> metadataBuilder.pluginName(fieldValue))
- .description("The name of the plugin that performed the operation.")
- .build();
-
- this.operationsLatency =
- metricMaker
- .newTimer(
- OPERATION_LATENCY_METRIC_NAME,
- new Description("Latency of performing operations")
- .setCumulative()
- .setUnit(Description.Units.MILLISECONDS),
- operationNameField,
- requestField,
- pluginField)
- .suppressLogging();
- this.operationsCounter =
- metricMaker.newCounter(
- OPERATION_COUNT_METRIC_NAME,
- new Description("Number of performed operations").setRate(),
- operationNameField,
- requestField,
- pluginField);
- }
-
- @Override
- public void log(String operation, long durationMs) {
- log(operation, durationMs, /* metadata= */ null);
- }
-
- @Override
- public void log(String operation, long durationMs, @Nullable Metadata metadata) {
- String requestTag = TraceContext.getTag(TraceRequestListener.TAG_REQUEST).orElse("");
- String pluginTag = TraceContext.getPluginTag().orElse("");
- operationsLatency.record(operation, requestTag, pluginTag, durationMs, TimeUnit.MILLISECONDS);
- operationsCounter.increment(operation, requestTag, pluginTag);
- }
-}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 3373860..e9912f5 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -89,7 +89,6 @@
import com.google.gerrit.server.ExceptionHookImpl;
import com.google.gerrit.server.ExternalUser;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.PerformanceMetrics;
import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.TraceRequestListener;
import com.google.gerrit.server.account.AccountCacheImpl;
@@ -446,9 +445,6 @@
DynamicSet.setOf(binder(), SubmitRequirement.class);
DynamicSet.setOf(binder(), QuotaEnforcer.class);
DynamicSet.setOf(binder(), PerformanceLogger.class);
- if (cfg.getBoolean("tracing", "exportPerformanceMetrics", false)) {
- DynamicSet.bind(binder(), PerformanceLogger.class).to(PerformanceMetrics.class);
- }
DynamicSet.setOf(binder(), RequestListener.class);
DynamicSet.bind(binder(), RequestListener.class).to(TraceRequestListener.class);
DynamicSet.setOf(binder(), ChangeETagComputation.class);
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 93521f3..6f2bfdd 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -248,7 +248,6 @@
.build();
/** Upgrade Lucene to 8.x requires reindexing. */
- @SuppressWarnings("deprecation")
static final Schema<ChangeData> V83 = schema(V82);
/**
diff --git a/java/com/google/gerrit/server/mail/EmailModule.java b/java/com/google/gerrit/server/mail/EmailModule.java
index 340a103..92777e4 100644
--- a/java/com/google/gerrit/server/mail/EmailModule.java
+++ b/java/com/google/gerrit/server/mail/EmailModule.java
@@ -114,9 +114,8 @@
AttentionSetChange attentionSetChange, ChangeEmail changeEmail) {
if (attentionSetChange.equals(AttentionSetChange.USER_ADDED)) {
return outgoingEmailFactory.create("addToAttentionSet", changeEmail);
- } else {
- return outgoingEmailFactory.create("removeFromAttentionSet", changeEmail);
}
+ return outgoingEmailFactory.create("removeFromAttentionSet", changeEmail);
}
}
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 0b909cd..00c73a0 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -107,7 +107,7 @@
void populateEmailContent() throws EmailException;
/** If returns false email is not sent to any recipients. */
- default boolean shouldSendMessage() throws EmailException {
+ default boolean shouldSendMessage() {
return true;
}
}
@@ -195,7 +195,7 @@
}
@Override
- public boolean shouldSendMessage() throws EmailException {
+ public boolean shouldSendMessage() {
return changeEmailDecorator.shouldSendMessage();
}
diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index a1db847..afdcbad 100644
--- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -88,16 +88,28 @@
void populateEmailContent() throws EmailException;
/** If returns false email is not sent to any recipients. */
- default boolean shouldSendMessage() throws EmailException {
+ default boolean shouldSendMessage() {
return true;
}
- /** Evaluates whether account can be added to the list of recipients. */
+ /**
+ * Evaluates whether account can be added to the list of recipients.
+ *
+ * @param rcpt the recipient for which it should be checker whether it can be added to the list
+ * of recipients
+ * @throws PermissionBackendException thrown if checking permissions fails
+ */
default boolean isRecipientAllowed(Account.Id rcpt) throws PermissionBackendException {
return true;
}
- /** Evaluates whether email can be added to the list of recipients. */
+ /**
+ * Evaluates whether email can be added to the list of recipients.
+ *
+ * @param rcpt the recipient for which it should be checker whether it can be added to the list
+ * of recipients
+ * @throws PermissionBackendException thrown if checking permissions fails
+ */
default boolean isRecipientAllowed(Address rcpt) throws PermissionBackendException {
return true;
}
@@ -108,7 +120,7 @@
private String messageClass;
private final Set<Account.Id> rcptTo = new HashSet<>();
- private final Map<String, EmailHeader> headers = new LinkedHashMap<>();;
+ private final Map<String, EmailHeader> headers = new LinkedHashMap<>();
private final Set<Address> smtpRcptTo = new HashSet<>();
private final Set<Address> smtpBccRcptTo = new HashSet<>();
private Address smtpFromAddress;
@@ -564,7 +576,7 @@
return accountState.get().userName().orElse(null);
}
- private boolean shouldSendMessage() throws EmailException {
+ private boolean shouldSendMessage() {
if (textBody.length() == 0) {
// If we have no message body, don't send.
logger.atFine().log("Not sending '%s': No message body", messageClass);
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 95e528b..5c84589 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -730,7 +730,7 @@
}
}
- private void parseCustomKeyedValues(ChangeNotesCommit commit) throws ConfigInvalidException {
+ private void parseCustomKeyedValues(ChangeNotesCommit commit) {
for (String customKeyedValueLine : commit.getFooterLineValues(FOOTER_CUSTOM_KEYED_VALUE)) {
String[] parts = customKeyedValueLine.split("=", 2);
String key = parts[0];
diff --git a/javatests/com/google/gerrit/gpg/PublicKeyCheckerTest.java b/javatests/com/google/gerrit/gpg/PublicKeyCheckerTest.java
index 8bafafe..c360b2f 100644
--- a/javatests/com/google/gerrit/gpg/PublicKeyCheckerTest.java
+++ b/javatests/com/google/gerrit/gpg/PublicKeyCheckerTest.java
@@ -279,7 +279,6 @@
private PGPPublicKeyRing removeRevokers(PGPPublicKeyRing kr) {
PGPPublicKey k = kr.getPublicKey();
- @SuppressWarnings("unchecked")
Iterator<PGPSignature> sigs = k.getSignaturesOfType(DIRECT_KEY);
while (sigs.hasNext()) {
PGPSignature sig = sigs.next();
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index d04964b..04ff4fc 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -340,8 +340,7 @@
newBuilder()
.customKeyedValues(
ImmutableList.of(
- new SimpleEntry<String, String>("key1", "value1"),
- new SimpleEntry<String, String>("key2", "value2")))
+ new SimpleEntry<>("key1", "value1"), new SimpleEntry<>("key2", "value2")))
.build(),
ChangeNotesStateProto.newBuilder()
.setMetaId(SHA_BYTES)
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index d778546..b1c4c8e4 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -710,6 +710,7 @@
changeNum: change._number,
repo: change.project,
patchNum: patchset,
+ checksPatchset: patchset,
diffView: {path, lineNum: line},
}),
primary: true,
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 6892188..864f559 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -1446,6 +1446,10 @@
diffView: {path: ctx.params[8]},
};
const queryMap = new URLSearchParams(ctx.querystring);
+ const checksPatchset = Number(queryMap.get('checksPatchset'));
+ if (Number.isInteger(checksPatchset) && checksPatchset > 0) {
+ state.checksPatchset = checksPatchset as PatchSetNumber;
+ }
if (queryMap.has('forceReload')) state.forceReload = true;
const address = this.parseLineAddress(ctx.hash);
if (address) {
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 64dff29..d43dadc 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
@@ -74,6 +74,7 @@
import {createChangeUrl, createDiffUrl} from '../../../models/views/change';
import {userModelToken} from '../../../models/user/user-model';
import {highlightServiceToken} from '../../../services/highlight/highlight-service';
+import {waitUntil} from '../../../utils/async-util';
declare global {
interface HTMLElementEventMap {
@@ -112,6 +113,9 @@
@query('.comment-box')
commentBox?: HTMLElement;
+ @query('gr-comment.draft')
+ draftElement?: GrComment;
+
@queryAll('gr-comment')
commentElements?: NodeList;
@@ -495,6 +499,7 @@
: !this.unresolved);
return html`
<gr-comment
+ class=${classMap({draft: isDraft(comment)})}
.comment=${comment}
.comments=${this.thread!.comments}
?initially-collapsed=${initiallyCollapsed}
@@ -646,6 +651,15 @@
}, 500);
});
}
+ if (this.thread && isDraft(this.getFirstComment())) {
+ const msg = this.getFirstComment()?.message ?? '';
+ if (msg.length === 0) this.editDraft();
+ }
+ }
+
+ private async editDraft() {
+ await waitUntil(() => !!this.draftElement);
+ this.draftElement!.edit();
}
private isDraft() {
@@ -797,6 +811,7 @@
const newReply = createNewReply(replyingTo, content, unresolved);
if (userWantsToEdit) {
this.getCommentsModel().addNewDraft(newReply);
+ this.editDraft();
} else {
try {
this.saving = true;
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index 1027a62..14ed24f 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -120,7 +120,11 @@
robot-button-disabled=""
show-patchset=""
></gr-comment>
- <gr-comment robot-button-disabled="" show-patchset=""></gr-comment>
+ <gr-comment
+ class="draft"
+ robot-button-disabled=""
+ show-patchset=""
+ ></gr-comment>
</div>
</div>
`
@@ -145,7 +149,11 @@
<div id="container">
<h3 class="assistive-tech-only">Draft Comment thread by Yoda</h3>
<div class="comment-box" tabindex="0">
- <gr-comment robot-button-disabled="" show-patchset=""></gr-comment>
+ <gr-comment
+ class="draft"
+ robot-button-disabled=""
+ show-patchset=""
+ ></gr-comment>
</div>
</div>
`
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 74c5806..158799d 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -924,13 +924,6 @@
if (this.permanentEditingMode) {
this.edit();
}
- if (
- isDraft(this.comment) &&
- isNew(this.comment) &&
- !isSaving(this.comment)
- ) {
- this.edit();
- }
if (isDraft(this.comment)) {
this.collapsed = false;
} else {
@@ -941,6 +934,10 @@
override updated(changed: PropertyValues) {
if (changed.has('editing')) {
if (this.editing && !this.permanentEditingMode) {
+ // Note that this is a bit fragile, because we are relying on the
+ // comment to become visible soonish. If that does not happen, then we
+ // will be waiting indefinitely and grab focus at some point in the
+ // distant future.
whenVisible(this, () => this.textarea?.putCursorAtEnd());
}
}
@@ -981,7 +978,7 @@
}
/** Enter editing mode. */
- private edit() {
+ edit() {
assert(isDraft(this.comment), 'only drafts are editable');
if (this.editing) return;
this.editing = true;
@@ -1065,6 +1062,8 @@
}
override focus() {
+ // Note that this may not work as intended, because the textarea is not
+ // rendered yet.
this.textarea?.focus();
}
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
index 7779fff..95f1b8a 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
@@ -295,6 +295,8 @@
}
override focus() {
+ // Note that this may not work as intended, because the textarea is not
+ // rendered yet.
this.textarea?.textarea.focus();
}
@@ -627,7 +629,7 @@
this.currentSearchString = '';
this.closeDropdown();
this.specialCharIndex = -1;
- this.textarea?.textarea.focus();
+ this.focus();
}
private fireChangedEvents() {
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index 713d401..127f77b 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -204,25 +204,33 @@
childView: ChangeChildView.DIFF,
});
- const path = `/${encodeURL(state.diffView?.path ?? '')}`;
-
- let suffix = '';
+ let path = `/${encodeURL(state.diffView?.path ?? '')}`;
// TODO: Move creating of comment URLs to a separate function. We are
// "abusing" the `commentId` property, which should only be used for pointing
// to comment in the COMMENTS tab of the OVERVIEW page.
if (state.commentId) {
- suffix += `comment/${state.commentId}/`;
+ path += `comment/${state.commentId}/`;
}
+ let queryParams = '';
+ const params = [];
+ if (state.checksPatchset && state.checksPatchset > 0) {
+ params.push(`checksPatchset=${state.checksPatchset}`);
+ }
+ if (params.length > 0) {
+ queryParams = '?' + params.join('&');
+ }
+
+ let hash = '';
if (state.diffView?.lineNum) {
- suffix += '#';
+ hash += '#';
if (state.diffView?.leftSide) {
- suffix += 'b';
+ hash += 'b';
}
- suffix += state.diffView.lineNum;
+ hash += state.diffView.lineNum;
}
- return `${createChangeUrlCommon(state)}${path}${suffix}`;
+ return `${createChangeUrlCommon(state)}${path}${queryParams}${hash}`;
}
export function createEditUrl(
diff --git a/polygerrit-ui/app/models/views/change_test.ts b/polygerrit-ui/app/models/views/change_test.ts
index 837e362..8e671d1 100644
--- a/polygerrit-ui/app/models/views/change_test.ts
+++ b/polygerrit-ui/app/models/views/change_test.ts
@@ -6,6 +6,7 @@
import {assert} from '@open-wc/testing';
import {
BasePatchSetNum,
+ PatchSetNumber,
RepoName,
RevisionPatchSetNum,
} from '../../api/rest-api';
@@ -77,59 +78,97 @@
assert.equal(createChangeUrl(state), '/c/x%252B/y%252B/z%252B/w/+/42');
});
- test('createDiffUrl', () => {
- const params: ChangeViewState = {
- ...createDiffViewState(),
- patchNum: 12 as RevisionPatchSetNum,
- diffView: {path: 'x+y/path.cpp'},
- };
- assert.equal(
- createDiffUrl(params),
- '/c/test-project/+/42/12/x%252By/path.cpp'
- );
+ suite('createDiffUrl', () => {
+ let params: ChangeViewState;
+ setup(() => {
+ params = {
+ ...createDiffViewState(),
+ patchNum: 12 as RevisionPatchSetNum,
+ diffView: {path: 'x+y/path.cpp'},
+ };
+ });
- window.CANONICAL_PATH = '/base';
- assert.equal(createDiffUrl(params).substring(0, 5), '/base');
- window.CANONICAL_PATH = undefined;
+ test('CANONICAL_PATH', () => {
+ window.CANONICAL_PATH = '/base';
+ assert.equal(createDiffUrl(params).substring(0, 5), '/base');
+ window.CANONICAL_PATH = undefined;
+ });
- params.repo = 'test' as RepoName;
- assert.equal(createDiffUrl(params), '/c/test/+/42/12/x%252By/path.cpp');
+ test('basic', () => {
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/12/x%252By/path.cpp'
+ );
+ });
- params.basePatchNum = 6 as BasePatchSetNum;
- assert.equal(createDiffUrl(params), '/c/test/+/42/6..12/x%252By/path.cpp');
+ test('repo', () => {
+ params.repo = 'test' as RepoName;
+ assert.equal(createDiffUrl(params), '/c/test/+/42/12/x%252By/path.cpp');
+ });
- params.diffView = {
- path: 'foo bar/my+file.txt%',
- };
- params.patchNum = 2 as RevisionPatchSetNum;
- delete params.basePatchNum;
- assert.equal(
- createDiffUrl(params),
- '/c/test/+/42/2/foo+bar/my%252Bfile.txt%2525'
- );
+ test('checks patchset', () => {
+ params.checksPatchset = 4 as PatchSetNumber;
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/12/x%252By/path.cpp?checksPatchset=4'
+ );
+ });
- params.diffView = {
- path: 'file.cpp',
- lineNum: 123,
- };
- assert.equal(createDiffUrl(params), '/c/test/+/42/2/file.cpp#123');
+ test('base patchset', () => {
+ params.basePatchNum = 6 as BasePatchSetNum;
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/6..12/x%252By/path.cpp'
+ );
+ });
- params.diffView = {
- path: 'file.cpp',
- lineNum: 123,
- leftSide: true,
- };
- assert.equal(createDiffUrl(params), '/c/test/+/42/2/file.cpp#b123');
- });
+ test('percent', () => {
+ params.diffView = {
+ path: 'foo bar/my+file.txt%',
+ };
+ params.patchNum = 2 as RevisionPatchSetNum;
+ delete params.basePatchNum;
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/2/foo+bar/my%252Bfile.txt%2525'
+ );
+ });
- test('diff with repo name encoding', () => {
- const params: ChangeViewState = {
- ...createDiffViewState(),
- patchNum: 12 as RevisionPatchSetNum,
- repo: 'x+/y' as RepoName,
- diffView: {path: 'x+y/path.cpp'},
- };
- assert.equal(createDiffUrl(params), '/c/x%252B/y/+/42/12/x%252By/path.cpp');
+ test('line right', () => {
+ params.diffView = {
+ path: 'file.cpp',
+ lineNum: 123,
+ };
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/12/file.cpp#123'
+ );
+ });
+
+ test('line left', () => {
+ params.diffView = {
+ path: 'file.cpp',
+ lineNum: 123,
+ leftSide: true,
+ };
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/12/file.cpp#b123'
+ );
+ });
+
+ test('diff with repo name encoding', () => {
+ const params: ChangeViewState = {
+ ...createDiffViewState(),
+ patchNum: 12 as RevisionPatchSetNum,
+ repo: 'x+/y' as RepoName,
+ diffView: {path: 'x+y/path.cpp'},
+ };
+ assert.equal(
+ createDiffUrl(params),
+ '/c/x%252B/y/+/42/12/x%252By/path.cpp'
+ );
+ });
});
test('createEditUrl', () => {
