Merge "Add assertIsDefined util method"
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index 7f434ca..871d8d2 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -35,6 +35,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Change.Status;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
@@ -493,12 +494,29 @@
logger.atFine().log("Calculated to merge %s", indexBackedChangeSet);
// Reload ChangeSet so that we don't rely on (potentially) stale index data for merging
- ChangeSet cs = reloadChanges(indexBackedChangeSet);
+ ChangeSet noteDbChangeSet = reloadChanges(indexBackedChangeSet);
+
+ // At this point, any change that isn't new can be filtered out since they were only here
+ // in the first place due to stale index.
+ List<ChangeData> filteredChanges = new ArrayList<>();
+ for (ChangeData changeData : noteDbChangeSet.changes()) {
+ if (!changeData.change().getStatus().equals(Status.NEW)) {
+ logger.atFine().log(
+ "Change %s has status %s due to stale index, so it is skipped during submit",
+ changeData.getId().toString(), changeData.change().getStatus().name());
+ continue;
+ }
+ filteredChanges.add(changeData);
+ }
+
+ // There are no hidden changes (or else we would have thrown AuthException above).
+ ChangeSet filteredNoteDbChangeSet =
+ new ChangeSet(filteredChanges, /* hiddenChanges= */ ImmutableList.of());
// Count cross-project submissions outside of the retry loop. The chance of a single project
// failing increases with the number of projects, so the failure count would be inflated if
// this metric were incremented inside of integrateIntoHistory.
- int projects = cs.projects().size();
+ int projects = filteredNoteDbChangeSet.projects().size();
if (projects > 1) {
topicMetrics.topicSubmissions.increment();
}
@@ -517,22 +535,22 @@
this.ts = TimeUtil.nowTs();
openRepoManager();
}
- this.commitStatus = new CommitStatus(cs, isRetry);
+ this.commitStatus = new CommitStatus(filteredNoteDbChangeSet, isRetry);
if (checkSubmitRules) {
logger.atFine().log("Checking submit rules and state");
- checkSubmitRulesAndState(cs, isRetry);
+ checkSubmitRulesAndState(filteredNoteDbChangeSet, isRetry);
} else {
logger.atFine().log("Bypassing submit rules");
- bypassSubmitRules(cs, isRetry);
+ bypassSubmitRules(filteredNoteDbChangeSet, isRetry);
}
- integrateIntoHistory(cs, submissionExecutor);
+ integrateIntoHistory(filteredNoteDbChangeSet, submissionExecutor);
return null;
})
.listener(retryTracker)
// Up to the entire submit operation is retried, including possibly many projects.
// Multiply the timeout by the number of projects we're actually attempting to
// submit.
- .defaultTimeoutMultiplier(cs.projects().size())
+ .defaultTimeoutMultiplier(filteredNoteDbChangeSet.projects().size())
// By default, we only retry lock failures. Here it's better to also retry unexpected
// runtime exceptions.
.retryOn(t -> t instanceof RuntimeException)
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 9a6b82b..7ab7ae9 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -74,6 +74,7 @@
"//lib:jgit",
"//lib:jgit-junit",
"//lib:protobuf",
+ "//lib:soy",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
"//lib/flogger:api",
diff --git a/javatests/com/google/gerrit/server/mail/send/MailSoySauceProviderTest.java b/javatests/com/google/gerrit/server/mail/send/MailSoySauceProviderTest.java
new file mode 100644
index 0000000..2ec5e4d
--- /dev/null
+++ b/javatests/com/google/gerrit/server/mail/send/MailSoySauceProviderTest.java
@@ -0,0 +1,48 @@
+// 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.mail.send;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.plugincontext.PluginContext.PluginMetrics;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
+import com.google.template.soy.shared.SoyAstCache;
+import java.nio.file.Paths;
+import org.junit.Before;
+import org.junit.Test;
+
+public class MailSoySauceProviderTest {
+
+ private SitePaths sitePaths;
+ private DynamicSet<MailSoyTemplateProvider> set;
+
+ @Before
+ public void setUp() throws Exception {
+ sitePaths = new SitePaths(Paths.get("."));
+ set = new DynamicSet<>();
+ }
+
+ @Test
+ public void soyCompilation() {
+ MailSoySauceProvider provider =
+ new MailSoySauceProvider(
+ sitePaths,
+ new SoyAstCache(),
+ new PluginSetContext<>(set, PluginMetrics.DISABLED_INSTANCE));
+ assertThat(provider.get()).isNotNull(); // should not throw
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index 72ed6e6..4af51a9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -40,6 +40,7 @@
iconForCategory,
iconForStatus,
isRunning,
+ isRunningOrHasCompleted,
} from '../../../services/checks/checks-util';
import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
import {
@@ -288,6 +289,9 @@
:host.new-change-summary-true {
margin-bottom: var(--spacing-m);
}
+ .zeroState {
+ color: var(--primary-text-color);
+ }
td.key {
padding-right: var(--spacing-l);
padding-bottom: var(--spacing-m);
@@ -312,6 +316,11 @@
];
}
+ renderChecksZeroState() {
+ if (this.runs.some(isRunningOrHasCompleted)) return;
+ return html`<span class="font-small zeroState">No results</span>`;
+ }
+
renderChecksChipForCategory(category: Category) {
const icon = iconForCategory(category);
const runs = this.runs.filter(run => hasResultsOf(run, category));
@@ -395,7 +404,7 @@
<tr ?hidden=${!this.showChecksSummary}>
<td class="key">Checks</td>
<td class="value">
- ${this.renderChecksChipForCategory(
+ ${this.renderChecksZeroState()}${this.renderChecksChipForCategory(
Category.ERROR
)}${this.renderChecksChipForCategory(
Category.WARNING
@@ -410,13 +419,13 @@
<tr ?hidden=${!this.newChangeSummaryUiEnabled}>
<td class="key">Comments</td>
<td class="value">
- <gr-summary-chip
- styleType=${SummaryChipStyles.INFO}
+ <span
+ class="font-small zeroState"
?hidden=${!!countResolvedComments ||
!!draftCount ||
!!countUnresolvedComments}
>
- No Comments</gr-summary-chip
+ No Comments</span
><gr-summary-chip
styleType=${SummaryChipStyles.WARNING}
category=${CommentTabState.DRAFTS}
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 f70ff46..8f74f76 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
@@ -576,7 +576,6 @@
[Shortcut.UP_TO_DASHBOARD]: '_handleUpToDashboard',
[Shortcut.EXPAND_ALL_MESSAGES]: '_handleExpandAllMessages',
[Shortcut.COLLAPSE_ALL_MESSAGES]: '_handleCollapseAllMessages',
- [Shortcut.EXPAND_ALL_DIFF_CONTEXT]: '_expandAllDiffs',
[Shortcut.OPEN_DIFF_PREFS]: '_handleOpenDiffPrefsShortcut',
[Shortcut.EDIT_TOPIC]: '_handleEditTopic',
[Shortcut.DIFF_AGAINST_BASE]: '_handleDiffAgainstBase',
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
index 23627ae..fbbef12 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
@@ -60,26 +60,12 @@
return [
sharedStyles,
css`
- .title {
- font-weight: var(--font-weight-bold);
- color: var(--deemphasized-text-color);
- padding-left: var(--metadata-horizontal-padding);
- }
- h4 {
- display: flex;
- }
- /* This is a hacky solution from old gr-related-change-list
- * TODO(milutin): find layout without needing it
- */
- h4:before,
- gr-related-change:before {
- content: ' ';
- flex-shrink: 0;
- width: 1.2em;
- }
.note {
color: var(--error-text-color);
}
+ section {
+ margin-bottom: var(--spacing-m);
+ }
`,
];
}
@@ -101,8 +87,10 @@
class="relatedChanges"
?hidden=${!relatedChanges.length}
>
- <h4 class="title">Relation chain</h4>
- <gr-related-collapse .length=${relatedChanges.length}>
+ <gr-related-collapse
+ title="Relation chain"
+ .length=${relatedChanges.length}
+ >
${relatedChanges.map(
(change, index) =>
html`<gr-related-change
@@ -140,8 +128,10 @@
?hidden=${!submittedTogetherChanges?.length &&
!this._submittedTogether?.non_visible_changes}
>
- <h4 class="title">Submitted together</h4>
- <gr-related-collapse .length=${submittedTogetherChanges.length}>
+ <gr-related-collapse
+ title="Submitted together"
+ .length=${submittedTogetherChanges.length}
+ >
${submittedTogetherChanges.map(
(change, index) =>
html`<gr-related-change
@@ -277,6 +267,9 @@
@customElement('gr-related-collapse')
export class GrRelatedCollapse extends GrLitElement {
@property()
+ title = '';
+
+ @property()
showAll = false;
@property()
@@ -286,10 +279,24 @@
return [
sharedStyles,
css`
+ .title {
+ font-weight: var(--font-weight-bold);
+ color: var(--deemphasized-text-color);
+ padding-left: var(--metadata-horizontal-padding);
+ }
+ h4 {
+ display: flex;
+ align-self: flex-end;
+ }
gr-button {
display: flex;
}
- gr-button:before {
+ /* This is a hacky solution from old gr-related-change-list
+ * TODO(milutin): find layout without needing it
+ */
+ h4:before,
+ gr-button:before,
+ ::slotted(gr-related-change):before {
content: ' ';
flex-shrink: 0;
width: 1.2em;
@@ -303,31 +310,46 @@
::slotted(gr-related-change) {
display: flex;
}
+ gr-button iron-icon {
+ color: inherit;
+ --iron-icon-height: 18px;
+ --iron-icon-width: 18px;
+ }
+ .container {
+ justify-content: space-between;
+ display: flex;
+ margin-bottom: var(--spacing-s);
+ }
`,
];
}
render() {
+ const title = html`<h4 class="title">${this.title}</h4>`;
+
const collapsible = this.length > MAX_CHANGES_WHEN_COLLAPSED;
const items = html` <div
class="${!this.showAll && collapsible ? 'collapsed' : ''}"
>
<slot></slot>
</div>`;
+
let button = nothing;
if (collapsible) {
if (this.showAll) {
button = html`<gr-button link="" @click="${this.toggle}"
- >Show less</gr-button
- >`;
+ >Show less<iron-icon icon="gr-icons:expand-less"></iron-icon
+ ></gr-button>`;
} else {
button = html`<gr-button link="" @click="${this.toggle}"
- >+ ${this.length - MAX_CHANGES_WHEN_COLLAPSED} more</gr-button
- >`;
+ >Show all (${this.length})
+ <iron-icon icon="gr-icons:expand-more"></iron-icon
+ ></gr-button>`;
}
}
- return html`${items}${button}`;
+ return html`<div class="container">${title}${button}</div>
+ ${items}`;
}
private toggle(e: MouseEvent) {
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 b4b6d31..5705c4f 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
@@ -451,12 +451,14 @@
?.getElementsByClassName('arrowToCurrentChange')[0]
?.nextElementSibling?.nextElementSibling?.getElementsByTagName('a')[0];
- if (!target || !currentChange) return;
+ if (!target) return;
this.reportingService.reportInteraction('related-change-click', {
sectionName,
index: sectionLinks.indexOf(target) + 1,
countChanges: sectionLinks.length,
- currentChangeIndex: sectionLinks.indexOf(currentChange) + 1,
+ currentChangeIndex: !currentChange
+ ? undefined
+ : sectionLinks.indexOf(currentChange) + 1,
});
}
}
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index bc72b2a..df881f3 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -21,8 +21,10 @@
import {sharedStyles} from '../../styles/shared-styles';
import {RunResult} from '../../services/checks/checks-model';
import {
+ hasCompleted,
hasCompletedWithoutResults,
iconForCategory,
+ isRunning,
} from '../../services/checks/checks-util';
@customElement('gr-result-row')
@@ -276,6 +278,9 @@
.categoryHeader iron-icon.success {
color: var(--success-foreground);
}
+ .noCompleted {
+ margin-top: var(--spacing-l);
+ }
table.resultsTable {
width: 100%;
max-width: 1280px;
@@ -295,12 +300,21 @@
render() {
return html`
<div><h2 class="heading-2">Results</h2></div>
- ${this.renderSection(Category.ERROR)}
+ ${this.renderNoCompleted()} ${this.renderSection(Category.ERROR)}
${this.renderSection(Category.WARNING)}
${this.renderSection(Category.INFO)} ${this.renderSuccess()}
`;
}
+ renderNoCompleted() {
+ if (this.runs.some(hasCompleted)) return;
+ let text = 'No results';
+ if (this.runs.some(isRunning)) {
+ text = 'Checks are running ...';
+ }
+ return html`<div class="noCompleted">${text}</div>`;
+ }
+
renderSection(category: Category) {
const catString = category.toString().toLowerCase();
const runs = this.runs.filter(r =>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index a3a5a22..47b4a1f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -565,8 +565,8 @@
this.$.diff.clearDiffContent();
}
- expandAllContext() {
- this.$.diff.expandAllContext();
+ toggleAllContext() {
+ this.$.diff.toggleAllContext();
}
_getLoggedIn() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index 99486ad..01c78f0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -796,9 +796,9 @@
assert.equal(stub.lastCall.args.length, 0);
});
- test('delegates expandAllContext()', () => {
- const stub = sinon.stub(element.$.diff, 'expandAllContext');
- element.expandAllContext();
+ test('delegates toggleAllContext()', () => {
+ const stub = sinon.stub(element.$.diff, 'toggleAllContext');
+ element.toggleAllContext();
assert.isTrue(stub.calledOnce);
assert.equal(stub.lastCall.args.length, 0);
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index ff8fa69..088d9cf 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -292,7 +292,7 @@
[Shortcut.OPEN_DIFF_PREFS]: '_handleCommaKey',
[Shortcut.TOGGLE_DIFF_MODE]: '_handleToggleDiffMode',
[Shortcut.TOGGLE_FILE_REVIEWED]: '_throttledToggleFileReviewed',
- [Shortcut.EXPAND_ALL_DIFF_CONTEXT]: '_handleExpandAllDiffContext',
+ [Shortcut.TOGGLE_ALL_DIFF_CONTEXT]: '_handleToggleAllDiffContext',
[Shortcut.NEXT_UNREVIEWED_FILE]: '_handleNextUnreviewedFile',
[Shortcut.TOGGLE_BLAME]: '_handleToggleBlame',
[Shortcut.TOGGLE_HIDE_ALL_COMMENT_THREADS]:
@@ -1745,10 +1745,10 @@
return '';
}
- _handleExpandAllDiffContext(e: CustomKeyboardEvent) {
+ _handleToggleAllDiffContext(e: CustomKeyboardEvent) {
if (this.shouldSuppressKeyboardShortcut(e)) return;
- this.$.diffHost.expandAllContext();
+ this.$.diffHost.toggleAllContext();
}
_computeDiffPrefsDisabled(disableDiffPrefs?: boolean, loggedIn?: boolean) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 46a670d..b5473a3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -63,7 +63,7 @@
kb.bindShortcut(Shortcut.OPEN_DIFF_PREFS, ',');
kb.bindShortcut(Shortcut.TOGGLE_DIFF_MODE, 'm');
kb.bindShortcut(Shortcut.TOGGLE_FILE_REVIEWED, 'r');
- kb.bindShortcut(Shortcut.EXPAND_ALL_DIFF_CONTEXT, 'shift+x');
+ kb.bindShortcut(Shortcut.TOGGLE_ALL_DIFF_CONTEXT, 'shift+x');
kb.bindShortcut(Shortcut.EXPAND_ALL_COMMENT_THREADS, 'e');
kb.bindShortcut(Shortcut.TOGGLE_HIDE_ALL_COMMENT_THREADS, 'h');
kb.bindShortcut(Shortcut.COLLAPSE_ALL_COMMENT_THREADS, 'shift+e');
@@ -508,11 +508,11 @@
assert.isTrue(diffChangeStub.called);
});
- test('shift+x shortcut expands all diff context', () => {
- const expandStub = sinon.stub(element.$.diffHost, 'expandAllContext');
+ test('shift+x shortcut toggles all diff context', () => {
+ const toggleStub = sinon.stub(element.$.diffHost, 'toggleAllContext');
MockInteractions.pressAndReleaseKeyOn(element, 88, 'shift', 'x');
flush();
- assert.isTrue(expandStub.called);
+ assert.isTrue(toggleStub.called);
});
test('diff against base', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 902bfd7..6974a76 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -55,7 +55,11 @@
PolymerDomWrapper,
} from '../../../types/types';
import {CommentRangeLayer} from '../gr-ranged-comment-layer/gr-ranged-comment-layer';
-import {DiffViewMode, Side} from '../../../constants/constants';
+import {
+ createDefaultDiffPrefs,
+ DiffViewMode,
+ Side,
+} from '../../../constants/constants';
import {KeyLocations} from '../gr-diff-processor/gr-diff-processor';
import {FlattenedNodesObserver} from '@polymer/polymer/lib/utils/flattened-nodes-observer';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
@@ -78,7 +82,6 @@
const LARGE_DIFF_THRESHOLD_LINES = 10000;
const FULL_CONTEXT = -1;
-const LIMITED_CONTEXT = 10;
const COMMIT_MSG_PATH = '/COMMIT_MSG';
/**
@@ -969,8 +972,12 @@
this._debounceRenderDiffTable();
}
- _handleLimitedBypass() {
- this._safetyBypass = LIMITED_CONTEXT;
+ _collapseContext() {
+ // Uses the default context amount if the preference is for the entire file.
+ this._safetyBypass =
+ this.prefs?.context && this.prefs.context >= 0
+ ? null
+ : createDefaultDiffPrefs().context;
this._debounceRenderDiffTable();
}
@@ -982,8 +989,15 @@
return errorMessage ? 'showError' : '';
}
- expandAllContext() {
- this._handleFullBypass();
+ toggleAllContext() {
+ if (!this.prefs) {
+ return;
+ }
+ if (this._getBypassPrefs(this.prefs).context < 0) {
+ this._collapseContext();
+ } else {
+ this._handleFullBypass();
+ }
}
_computeNewlineWarning(warnLeft: boolean, warnRight: boolean) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
index 4b477cd..4d0e566 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
@@ -623,7 +623,7 @@
Prevented render because "Whole file" is enabled and this diff is very
large (about [[_diffLength]] lines).
</p>
- <gr-button on-click="_handleLimitedBypass">
+ <gr-button on-click="_collapseContext">
Render with limited context
</gr-button>
<gr-button on-click="_handleFullBypass">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
index 2215d5b..6fef223 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
@@ -794,6 +794,43 @@
element.addEventListener('render', rendered);
element._renderDiffTable();
});
+
+ test('toggles expand context using bypass', async () => {
+ element.prefs = {...MINIMAL_PREFS, context: 3};
+
+ element.toggleAllContext();
+ element._renderDiffTable();
+ await flush();
+
+ assert.equal(element.prefs.context, 3);
+ assert.equal(element._safetyBypass, -1);
+ assert.equal(renderStub.firstCall.lastArg.context, -1);
+ });
+
+ test('toggles collapse context from bypass', async () => {
+ element.prefs = {...MINIMAL_PREFS, context: 3};
+ element._safetyBypass = -1;
+
+ element.toggleAllContext();
+ element._renderDiffTable();
+ await flush();
+
+ assert.equal(element.prefs.context, 3);
+ assert.isNull(element._safetyBypass);
+ assert.equal(renderStub.firstCall.lastArg.context, 3);
+ });
+
+ test('toggles collapse context from pref using default', async () => {
+ element.prefs = {...MINIMAL_PREFS, context: -1};
+
+ element.toggleAllContext();
+ element._renderDiffTable();
+ await flush();
+
+ assert.equal(element.prefs.context, -1);
+ assert.equal(element._safetyBypass, 10);
+ assert.equal(renderStub.firstCall.lastArg.context, 10);
+ });
});
suite('blame', () => {
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index cdc9f98..e4d72e1 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -392,7 +392,7 @@
}
this.bindShortcut(Shortcut.NEXT_CHUNK, 'n');
this.bindShortcut(Shortcut.PREV_CHUNK, 'p');
- this.bindShortcut(Shortcut.EXPAND_ALL_DIFF_CONTEXT, 'shift+x');
+ this.bindShortcut(Shortcut.TOGGLE_ALL_DIFF_CONTEXT, 'shift+x');
this.bindShortcut(Shortcut.NEXT_COMMENT_THREAD, 'shift+n');
this.bindShortcut(Shortcut.PREV_COMMENT_THREAD, 'shift+p');
this.bindShortcut(
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index f3be790..ec59ddc 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -2234,13 +2234,13 @@
if (!basePatchNum && !patchNum && !path) {
return this._getDiffComments(changeNum, '/comments', {
'enable-context': true,
- 'context-padding': 5,
+ 'context-padding': 3,
});
}
return this._getDiffComments(
changeNum,
'/comments',
- {'enable-context': true, 'context-padding': 5},
+ {'enable-context': true, 'context-padding': 3},
basePatchNum,
patchNum,
path
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
index 9bd2f5a..7050cbe 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
@@ -176,7 +176,7 @@
VISIBLE_LINE = 'VISIBLE_LINE',
NEXT_CHUNK = 'NEXT_CHUNK',
PREV_CHUNK = 'PREV_CHUNK',
- EXPAND_ALL_DIFF_CONTEXT = 'EXPAND_ALL_DIFF_CONTEXT',
+ TOGGLE_ALL_DIFF_CONTEXT = 'TOGGLE_ALL_DIFF_CONTEXT',
NEXT_COMMENT_THREAD = 'NEXT_COMMENT_THREAD',
PREV_COMMENT_THREAD = 'PREV_COMMENT_THREAD',
EXPAND_ALL_COMMENT_THREADS = 'EXPAND_ALL_COMMENT_THREADS',
@@ -404,9 +404,9 @@
'Go to previous diff chunk'
);
_describe(
- Shortcut.EXPAND_ALL_DIFF_CONTEXT,
+ Shortcut.TOGGLE_ALL_DIFF_CONTEXT,
ShortcutSection.DIFFS,
- 'Expand all diff context'
+ 'Toggle all diff context'
);
_describe(
Shortcut.NEXT_COMMENT_THREAD,
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 61456a2..de12a2a 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -289,6 +289,8 @@
}
export function computeDiffFromContext(context: ContextLine[], path: string) {
+ // do not render more than 20 lines of context
+ context = context.slice(0, 20);
const diff: DiffInfo = {
meta_a: {
name: '',
