Merge "Respect the show_file_comment_button pref in lit diff"
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 565c491..e12c27c 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -43,6 +43,17 @@
For more predictable results, use explicit search operators as described
in the following section.
+[IMPORTANT]
+--
+The change search API is backed by a secondary index and might sometimes return
+stale results if the re-indexing operation failed for a change update.
+
+Please also note that changes are not re-indexed if the project configuration
+is updated with newly added or modified
+link:config-submit-requirements.html[submit requirements].
+--
+
+
[[search-operators]]
== Search Operators
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
index 8d6d89a..3599224 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
@@ -25,7 +25,7 @@
RepoState,
SubmitType,
} from '../../../constants/constants';
-import {hasOwnProperty} from '../../../utils/common-util';
+import {assertIsDefined, hasOwnProperty} from '../../../utils/common-util';
import {firePageError, fireTitleChange} from '../../../utils/event-util';
import {getAppContext} from '../../../services/app-context';
import {WebLinkInfo} from '../../../types/diff';
@@ -36,8 +36,9 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {BindValueChangeEvent} from '../../../types/events';
import {deepClone} from '../../../utils/deep-util';
-import {LitElement, PropertyValues, css, html} from 'lit';
+import {LitElement, PropertyValues, css, html, nothing} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
+import {when} from 'lit/directives/when.js';
import {subscribe} from '../../lit/subscription-controller';
import {createSearchUrl} from '../../../models/views/search';
import {userModelToken} from '../../../models/user/user-model';
@@ -150,16 +151,6 @@
color: var(--deemphasized-text-color);
content: ' *';
}
- .loading,
- .hide {
- display: none;
- }
- #loading.loading {
- display: block;
- }
- #loading:not(.loading) {
- display: none;
- }
#options .repositorySettings {
display: none;
}
@@ -187,49 +178,48 @@
>
</div>
</div>
- <div id="loading" class=${this.loading ? 'loading' : ''}>
- Loading...
- </div>
- <div id="loadedContent" class=${this.loading ? 'loading' : ''}>
- ${this.renderDownloadCommands()}
- <h2
- id="configurations"
- class="heading-2 ${configChanged ? 'edited' : ''}"
- >
- Configurations
- </h2>
- <div id="form">
- <fieldset>
- ${this.renderDescription()} ${this.renderRepoOptions()}
- ${this.renderPluginConfig()}
- <gr-button
- ?disabled=${this.readOnly || !configChanged}
- @click=${this.handleSaveRepoConfig}
- >Save changes</gr-button
- >
- </fieldset>
- <gr-endpoint-decorator name="repo-config">
- <gr-endpoint-param
- name="repoName"
- .value=${this.repo}
- ></gr-endpoint-param>
- <gr-endpoint-param
- name="readOnly"
- .value=${this.readOnly}
- ></gr-endpoint-param>
- </gr-endpoint-decorator>
- </div>
- </div>
+ ${when(
+ this.loading || !this.repoConfig,
+ () => html`<div id="loading">Loading...</div>`,
+ () => html`<div id="loadedContent">
+ ${this.renderDownloadCommands()}
+ <h2
+ id="configurations"
+ class="heading-2 ${configChanged ? 'edited' : ''}"
+ >
+ Configurations
+ </h2>
+ <div id="form">
+ <fieldset>
+ ${this.renderDescription()} ${this.renderRepoOptions()}
+ ${this.renderPluginConfig()}
+ <gr-button
+ ?disabled=${this.readOnly || !configChanged}
+ @click=${this.handleSaveRepoConfig}
+ >Save changes</gr-button
+ >
+ </fieldset>
+ <gr-endpoint-decorator name="repo-config">
+ <gr-endpoint-param
+ name="repoName"
+ .value=${this.repo}
+ ></gr-endpoint-param>
+ <gr-endpoint-param
+ name="readOnly"
+ .value=${this.readOnly}
+ ></gr-endpoint-param>
+ </gr-endpoint-decorator>
+ </div>
+ </div>`
+ )}
</div>
`;
}
private renderDownloadCommands() {
+ if (!this.schemes.length) return nothing;
return html`
- <div
- id="downloadContent"
- class=${!this.schemes || !this.schemes.length ? 'hide' : ''}
- >
+ <div id="downloadContent">
<h2 id="download" class="heading-2">Download</h2>
<fieldset>
<gr-download-commands
@@ -252,6 +242,7 @@
}
private renderDescription() {
+ assertIsDefined(this.repoConfig, 'repoConfig');
return html`
<h3 id="Description" class="heading-3">Description</h3>
<fieldset>
@@ -263,7 +254,7 @@
rows="4"
monospace
?disabled=${this.readOnly}
- .text=${this.repoConfig?.description ?? ''}
+ .text=${this.repoConfig.description ?? ''}
@text-changed=${this.handleDescriptionTextChanged}
></gr-textarea>
</fieldset>
@@ -725,8 +716,9 @@
private renderPluginConfig() {
const pluginData = this.computePluginData();
+ if (!pluginData.length) return nothing;
return html` <div
- class="pluginConfig ${!pluginData || !pluginData.length ? 'hide' : ''}"
+ class="pluginConfig"
@plugin-config-changed=${this.handlePluginConfigChanged}
>
<h3 class="heading-3">Plugins</h3>
@@ -762,6 +754,12 @@
// private but used in test
async loadRepo() {
if (!this.repo) return Promise.resolve();
+ this.repoConfig = undefined;
+ this.originalConfig = undefined;
+ this.loading = true;
+ this.weblinks = [];
+ this.schemesObj = undefined;
+ this.readOnly = true;
const promises = [];
@@ -1121,6 +1119,7 @@
private handleDescriptionTextChanged(e: BindValueChangeEvent) {
if (!this.repoConfig || this.loading) return;
+ if (this.repoConfig.description === e.detail.value) return;
this.repoConfig = {
...this.repoConfig,
description: e.detail.value,
@@ -1130,6 +1129,7 @@
private handleStateSelectBindValueChanged(e: BindValueChangeEvent) {
if (!this.repoConfig || this.loading) return;
+ if (this.repoConfig.state === e.detail.value) return;
this.repoConfig = {
...this.repoConfig,
state: e.detail.value as RepoState,
@@ -1139,6 +1139,7 @@
private handleSubmitTypeSelectBindValueChanged(e: BindValueChangeEvent) {
if (!this.repoConfig || this.loading) return;
+ if (this.repoConfig.submit_type === e.detail.value) return;
this.repoConfig = {
...this.repoConfig,
submit_type: e.detail.value as SubmitType,
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts
index c013c9e..4deb99a 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts
@@ -157,14 +157,17 @@
element = await fixture(html`<gr-repo></gr-repo>`);
});
- test('render', () => {
+ test('render', async () => {
+ element.repo = REPO as RepoName;
+ await element.loadRepo();
+ await element.updateComplete;
// prettier and shadowDom assert do not agree about span.title wrapping
assert.shadowDom.equal(
element,
/* prettier-ignore */ /* HTML */ `
<div class="gr-form-styles main read-only">
<div class="info">
- <h1 class="heading-1" id="Title"></h1>
+ <h1 class="heading-1" id="Title">test-repo</h1>
<hr />
<div>
<a href="">
@@ -178,7 +181,7 @@
Browse
</gr-button>
</a>
- <a href="">
+ <a href="/q/project:test-repo">
<gr-button
aria-disabled="false"
link=""
@@ -190,15 +193,7 @@
</a>
</div>
</div>
- <div class="loading" id="loading">Loading...</div>
- <div class="loading" id="loadedContent">
- <div class="hide" id="downloadContent">
- <h2 class="heading-2" id="download">Download</h2>
- <fieldset>
- <gr-download-commands id="downloadCommands">
- </gr-download-commands>
- </fieldset>
- </div>
+ <div id="loadedContent">
<h2 class="heading-2" id="configurations">Configurations</h2>
<div id="form">
<fieldset>
@@ -266,7 +261,7 @@
</span>
</section>
<section
- class="repositorySettings"
+ class="repositorySettings showConfig"
id="enableSignedPushSettings"
>
<span class="title"> Enable signed push </span>
@@ -277,7 +272,7 @@
</span>
</section>
<section
- class="repositorySettings"
+ class="repositorySettings showConfig"
id="requireSignedPushSettings"
>
<span class="title"> Require signed push </span>
@@ -379,9 +374,6 @@
</span>
</section>
</fieldset>
- <div class="hide pluginConfig">
- <h3 class="heading-3">Plugins</h3>
- </div>
<gr-button
aria-disabled="true"
disabled=""
@@ -398,7 +390,51 @@
</div>
</div>
</div>
- `
+ `,
+ {ignoreTags: ['option']}
+ );
+ });
+
+ test('render loading', async () => {
+ element.repo = REPO as RepoName;
+ element.loading = true;
+ await element.updateComplete;
+ // prettier and shadowDom assert do not agree about span.title wrapping
+ assert.shadowDom.equal(
+ element,
+ /* prettier-ignore */ /* HTML */ `
+ <div class="gr-form-styles main read-only">
+ <div class="info">
+ <h1 class="heading-1" id="Title">test-repo</h1>
+ <hr />
+ <div>
+ <a href="">
+ <gr-button
+ aria-disabled="true"
+ disabled=""
+ link=""
+ role="button"
+ tabindex="-1"
+ >
+ Browse
+ </gr-button>
+ </a>
+ <a href="/q/project:test-repo">
+ <gr-button
+ aria-disabled="false"
+ link=""
+ role="button"
+ tabindex="0"
+ >
+ View Changes
+ </gr-button>
+ </a>
+ </div>
+ </div>
+ <div id="loading">Loading...</div>
+ </div>
+ `,
+ {ignoreTags: ['option']}
);
});
@@ -451,55 +487,22 @@
assert.isTrue(requestUpdateStub.called);
});
- test('loading displays before repo config is loaded', () => {
- assert.isTrue(
- queryAndAssert<HTMLDivElement>(element, '#loading').classList.contains(
- 'loading'
- )
- );
- assert.isFalse(
- getComputedStyle(queryAndAssert<HTMLDivElement>(element, '#loading'))
- .display === 'none'
- );
- assert.isTrue(
- queryAndAssert<HTMLDivElement>(
- element,
- '#loadedContent'
- ).classList.contains('loading')
- );
- assert.isTrue(
- getComputedStyle(
- queryAndAssert<HTMLDivElement>(element, '#loadedContent')
- ).display === 'none'
- );
- });
-
- test('download commands visibility', async () => {
- element.loading = false;
- await element.updateComplete;
- assert.isTrue(
- queryAndAssert<HTMLDivElement>(
- element,
- '#downloadContent'
- ).classList.contains('hide')
- );
- assert.isTrue(
- getComputedStyle(
- queryAndAssert<HTMLDivElement>(element, '#downloadContent')
- ).display === 'none'
- );
+ test('render download commands', async () => {
+ element.repo = REPO as RepoName;
+ await element.loadRepo();
element.schemesObj = SCHEMES;
await element.updateComplete;
- assert.isFalse(
- queryAndAssert<HTMLDivElement>(
- element,
- '#downloadContent'
- ).classList.contains('hide')
- );
- assert.isFalse(
- getComputedStyle(
- queryAndAssert<HTMLDivElement>(element, '#downloadContent')
- ).display === 'none'
+ const content = queryAndAssert<HTMLDivElement>(element, '#downloadContent');
+ assert.dom.equal(
+ content,
+ /* HTML */ `
+ <div id="downloadContent">
+ <h2 class="heading-2" id="download">Download</h2>
+ <fieldset>
+ <gr-download-commands id="downloadCommands"></gr-download-commands>
+ </fieldset>
+ </div>
+ `
);
});
@@ -715,9 +718,9 @@
Promise.resolve(new Response())
);
- const button = queryAll<GrButton>(element, 'gr-button')[2];
-
await element.loadRepo();
+
+ const button = queryAll<GrButton>(element, 'gr-button')[2];
assert.isTrue(button.hasAttribute('disabled'));
assert.isFalse(
queryAndAssert<HTMLHeadingElement>(
diff --git a/polygerrit-ui/app/utils/attention-set-util.ts b/polygerrit-ui/app/utils/attention-set-util.ts
index 77834bd..4404e59 100644
--- a/polygerrit-ui/app/utils/attention-set-util.ts
+++ b/polygerrit-ui/app/utils/attention-set-util.ts
@@ -3,7 +3,12 @@
* Copyright 2020 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {AccountInfo, ChangeInfo, ServerInfo} from '../types/common';
+import {
+ AccountInfo,
+ ChangeInfo,
+ DetailedLabelInfo,
+ ServerInfo,
+} from '../types/common';
import {ParsedChangeInfo} from '../types/types';
import {
getAccountTemplate,
@@ -13,6 +18,7 @@
} from './account-util';
import {CommentThread, isMentionedThread, isUnresolved} from './comment-util';
import {hasOwnProperty} from './common-util';
+import {getCodeReviewLabel} from './label-util';
export function canHaveAttention(account?: AccountInfo): boolean {
return !!account?._account_id && !isServiceUser(account);
@@ -101,9 +107,10 @@
/**
* Sort order:
* 1. The user themselves
- * 2. Human users in the attention set.
- * 3. Other human users.
- * 4. Service users.
+ * 2. Users in the attention set first.
+ * 3. Human users first.
+ * 4. Users that have voted first in this order of vote values:
+ * -2, -1, +2, +1, 0 or no vote.
*/
export function sortReviewers(
r1: AccountInfo,
@@ -117,7 +124,22 @@
}
const a1 = hasAttention(r1, change) ? 1 : 0;
const a2 = hasAttention(r2, change) ? 1 : 0;
- const s1 = isServiceUser(r1) ? -2 : 0;
- const s2 = isServiceUser(r2) ? -2 : 0;
- return a2 - a1 + s2 - s1;
+ if (a2 - a1 !== 0) return a2 - a1;
+
+ const s1 = isServiceUser(r1) ? -1 : 0;
+ const s2 = isServiceUser(r2) ? -1 : 0;
+ if (s2 - s1 !== 0) return s2 - s1;
+
+ const crLabel = getCodeReviewLabel(change?.labels ?? {}) as DetailedLabelInfo;
+ let v1 =
+ crLabel?.all?.find(vote => vote._account_id === r1._account_id)?.value ?? 0;
+ let v2 =
+ crLabel?.all?.find(vote => vote._account_id === r2._account_id)?.value ?? 0;
+ // We want negative votes getting a higher score than positive votes, so
+ // we choose 10 as a random number that is higher than all positive votes that
+ // are in use, and then add the absolute value of the vote to that.
+ // So -2 becomes 12.
+ if (v1 < 0) v1 = 10 - v1;
+ if (v2 < 0) v2 = 10 - v2;
+ return v2 - v1;
}
diff --git a/polygerrit-ui/app/utils/attention-set-util_test.ts b/polygerrit-ui/app/utils/attention-set-util_test.ts
index 8092a6e..5bd1924 100644
--- a/polygerrit-ui/app/utils/attention-set-util_test.ts
+++ b/polygerrit-ui/app/utils/attention-set-util_test.ts
@@ -6,9 +6,11 @@
import '../test/common-test-setup';
import {
createAccountDetailWithIdNameAndEmail,
+ createAccountWithId,
createChange,
createComment,
createCommentThread,
+ createParsedChange,
createServerInfo,
} from '../test/test-data-generators';
import {
@@ -22,9 +24,10 @@
getMentionedReason,
getReason,
hasAttention,
+ sortReviewers,
} from './attention-set-util';
import {DefaultDisplayNameConfig} from '../api/rest-api';
-import {AccountsVisibility} from '../constants/constants';
+import {AccountsVisibility, AccountTag} from '../constants/constants';
import {assert} from '@open-wc/testing';
const KERMIT: AccountInfo = {
@@ -101,6 +104,45 @@
assert.equal(getReason(config, OTHER_ACCOUNT, change), 'Added by kermit');
});
+ test('sortReviewers', () => {
+ const a1 = createAccountWithId(1);
+ a1.tags = [AccountTag.SERVICE_USER];
+ const a2 = createAccountWithId(2);
+ a2.tags = [AccountTag.SERVICE_USER];
+ const a3 = createAccountWithId(3);
+ const a4 = createAccountWithId(4);
+ const a5 = createAccountWithId(5);
+ const a6 = createAccountWithId(6);
+ const a7 = createAccountWithId(7);
+
+ const reviewers = [a1, a2, a3, a4, a5, a6, a7];
+ const change = {
+ ...createParsedChange(),
+ attention_set: {'6': {account: a6}},
+ labels: {
+ 'Code-Review': {
+ all: [
+ {...a2, value: 1},
+ {...a4, value: 1},
+ {...a5, value: -1},
+ ],
+ },
+ },
+ };
+ assert.sameOrderedMembers(
+ reviewers.sort((r1, r2) => sortReviewers(r1, r2, change, a7)),
+ [
+ a7, // self
+ a6, // is in the attention set
+ a5, // human user, has voted -1
+ a4, // human user, has voted +1
+ a3, // human user, has not voted
+ a2, // service user, has voted
+ a1, // service user, has not voted
+ ]
+ );
+ });
+
test('getMentionReason', () => {
let comment = {
...createComment(),
