Merge "Refactor show-alert events to use event-util"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index d128613..43c3b9e 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -216,15 +216,18 @@
[[labels]]
--
* `LABELS`: a summary of each label required for submit, and
- approvers that have granted (or rejected) with that label.
+ approvers that have granted (or rejected) with that label
+ as well as all reviewers by state, and reviewers that may
+ be removed by the current user.
--
[[detailed-labels]]
--
* `DETAILED_LABELS`: detailed label information, including numeric
values of all existing approvals, recognized label values, values
- permitted to be set by the current user, all reviewers by state, and
- reviewers that may be removed by the current user.
+ permitted to be set by any reviewer and the change owner, all
+ reviewers by state, and reviewers that may be removed by the
+ current user.
--
[[current-revision]]
@@ -6412,7 +6415,8 @@
|`removable_reviewers`|optional|
The reviewers that can be removed by the calling user as a list of
link:rest-api-accounts.html#account-info[AccountInfo] entities. +
-Only set if link:#detailed-labels[detailed labels] are requested.
+Only set if link:#labels[labels] or
+link:#detailed-labels[detailed labels] are requested.
|`reviewers` |optional|
The reviewers as a map that maps a reviewer state to a list of
link:rest-api-accounts.html#account-info[AccountInfo] entities.
@@ -6421,13 +6425,15 @@
`CC`: Users that were added to the change, but have not voted. +
`REMOVED`: Users that were previously reviewers on the change, but have
been removed. +
-Only set if link:#detailed-labels[detailed labels] are requested.
+Only set if link:#labels[labels] or
+link:#detailed-labels[detailed labels] are requested.
|`pending_reviewers` |optional|
Updates to `reviewers` that have been made while the change was in the
WIP state. Only present on WIP changes and only if there are pending
reviewer updates to report. These are reviewers who have not yet been
notified about being added to or removed from the change. +
-Only set if link:#detailed-labels[detailed labels] are requested.
+Only set if link:#labels[labels] or
+link:#detailed-labels[detailed labels] are requested.
|`reviewer_updates`|optional|
Updates to reviewers set for the change as
link:#review-update-info[ReviewerUpdateInfo] entities.
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 0cfc6cb..6ab0c61 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -602,7 +602,9 @@
? labelsJson.permittedLabels(user.getAccountId(), cd)
: ImmutableMap.of();
}
+ }
+ if (has(LABELS) || has(DETAILED_LABELS)) {
out.reviewers = reviewerMap(cd.reviewers(), cd.reviewersByEmail(), false);
out.pendingReviewers = reviewerMap(cd.pendingReviewers(), cd.pendingReviewersByEmail(), true);
out.removableReviewers = removableReviewers(cd, out);
diff --git a/polygerrit-ui/FE_Style_Guide.md b/polygerrit-ui/FE_Style_Guide.md
index c9a5d9b..a636119 100644
--- a/polygerrit-ui/FE_Style_Guide.md
+++ b/polygerrit-ui/FE_Style_Guide.md
@@ -10,11 +10,20 @@
Additionally to the rules above, Gerrit frontend uses the following rules (some of them have automated checks,
some don't):
+- [Prefer null over undefined](#prefer-null)
- [Use destructuring imports only](#destructuring-imports-only)
- [Use classes and services for storing and manipulating global state](#services-for-global-state)
- [Pass required services in the constructor for plain classes](#pass-dependencies-in-constructor)
- [Assign required services in a HTML/Polymer element constructor](#assign-dependencies-in-html-element-constructor)
+## <a name="prefer-undefined"></a>Prefer `undefined` over `null`
+
+It is more confusing than helpful to work with both `null` and `undefined`. We prefer to only use `undefined` in
+our code base. Try to avoid `null`.
+
+Some browser and library APIs are using `null`, so we cannot remove `null` completely from our code base. But even
+then try to convert return values and leak as few `nulls` as possible.
+
## <a name="destructuring-imports-only"></a>Use destructuring imports only
Always use destructuring import statement and specify all required names explicitly (e.g. `import {a,b,c} from '...'`)
where possible.
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index d70e891..64342f4 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -152,15 +152,19 @@
if (!label || category === LabelCategory.NOT_APPLICABLE) {
return 'Label not applicable';
}
+ const titleParts: string[] = [];
if (category === LabelCategory.UNRESOLVED_COMMENTS) {
const num = change?.unresolved_comment_count ?? 0;
const plural = num > 1 ? 's' : '';
- return `${num} unresolved comment${plural}`;
+ titleParts.push(`${num} unresolved comment${plural}`);
}
const significantLabel =
label.rejected || label.approved || label.disliked || label.recommended;
- if (significantLabel && significantLabel.name) {
- return `${labelName}\nby ${significantLabel.name}`;
+ if (significantLabel?.name) {
+ titleParts.push(`${labelName} by ${significantLabel.name}`);
+ }
+ if (titleParts.length > 0) {
+ return titleParts.join(',\n');
}
return labelName;
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js
index d3274f3..38cc772 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js
@@ -88,35 +88,35 @@
'Label not applicable');
assert.equal(element._computeLabelTitle(
{labels: {Verified: {approved: {name: 'Diffy'}}}}, 'Verified'),
- 'Verified\nby Diffy');
+ 'Verified by Diffy');
assert.equal(element._computeLabelTitle(
{labels: {Verified: {approved: {name: 'Diffy'}}}}, 'Code-Review'),
'Label not applicable');
assert.equal(element._computeLabelTitle(
{labels: {Verified: {rejected: {name: 'Diffy'}}}}, 'Verified'),
- 'Verified\nby Diffy');
+ 'Verified by Diffy');
assert.equal(element._computeLabelTitle(
{labels: {'Code-Review': {disliked: {name: 'Diffy'}, value: -1}}},
- 'Code-Review'), 'Code-Review\nby Diffy');
+ 'Code-Review'), 'Code-Review by Diffy');
assert.equal(element._computeLabelTitle(
{labels: {'Code-Review': {recommended: {name: 'Diffy'}, value: 1}}},
- 'Code-Review'), 'Code-Review\nby Diffy');
+ 'Code-Review'), 'Code-Review by Diffy');
assert.equal(element._computeLabelTitle(
{labels: {'Code-Review': {recommended: {name: 'Diffy'},
rejected: {name: 'Admin'}}}}, 'Code-Review'),
- 'Code-Review\nby Admin');
+ 'Code-Review by Admin');
assert.equal(element._computeLabelTitle(
{labels: {'Code-Review': {approved: {name: 'Diffy'},
rejected: {name: 'Admin'}}}}, 'Code-Review'),
- 'Code-Review\nby Admin');
+ 'Code-Review by Admin');
assert.equal(element._computeLabelTitle(
{labels: {'Code-Review': {recommended: {name: 'Diffy'},
disliked: {name: 'Admin'}, value: -1}}}, 'Code-Review'),
- 'Code-Review\nby Admin');
+ 'Code-Review by Admin');
assert.equal(element._computeLabelTitle(
{labels: {'Code-Review': {approved: {name: 'Diffy'},
disliked: {name: 'Admin'}, value: -1}}}, 'Code-Review'),
- 'Code-Review\nby Diffy');
+ 'Code-Review by Diffy');
assert.equal(element._computeLabelTitle(
{
labels: {'Code-Review': {approved: true, value: 1}},
@@ -125,6 +125,12 @@
'1 unresolved comment');
assert.equal(element._computeLabelTitle(
{
+ labels: {'Code-Review': {approved: {name: 'Diffy'}, value: 1}},
+ unresolved_comment_count: 1,
+ }, 'Code-Review'),
+ '1 unresolved comment,\nCode-Review by Diffy');
+ assert.equal(element._computeLabelTitle(
+ {
labels: {'Code-Review': {approved: true, value: 1}},
unresolved_comment_count: 2,
}, 'Code-Review'),
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 e5107f1..108f9ed 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
@@ -998,6 +998,7 @@
return;
}
e.preventDefault();
+ this.classList.remove('hideComments');
this.$.diffCursor.createCommentInPlace();
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
index b7181a3..5090462 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -139,6 +139,16 @@
window.removeEventListener('scroll', this._boundHandleWindowScroll);
}
+ // Don't remove - used by clients embedding gr-diff outside of Gerrit.
+ isAtStart() {
+ return this.$.cursorManager.isAtStart();
+ }
+
+ // Don't remove - used by clients embedding gr-diff outside of Gerrit.
+ isAtEnd() {
+ return this.$.cursorManager.isAtEnd();
+ }
+
moveLeft() {
this.side = Side.LEFT;
if (this._isTargetBlank()) {
@@ -155,21 +165,21 @@
moveDown() {
if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
- this.$.cursorManager.next({
+ return this.$.cursorManager.next({
filter: (row: Element) => this._rowHasSide(row),
});
} else {
- this.$.cursorManager.next();
+ return this.$.cursorManager.next();
}
}
moveUp() {
if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
- this.$.cursorManager.previous({
+ return this.$.cursorManager.previous({
filter: (row: Element) => this._rowHasSide(row),
});
} else {
- this.$.cursorManager.previous();
+ return this.$.cursorManager.previous();
}
}
@@ -183,7 +193,10 @@
}
}
- moveToNextChunk(clipToTop?: boolean, navigateToNextFile?: boolean) {
+ moveToNextChunk(
+ clipToTop?: boolean,
+ navigateToNextFile?: boolean
+ ): CursorMoveResult {
const result = this.$.cursorManager.next({
filter: (row: HTMLElement) => this._isFirstRowOfChunk(row),
getTargetHeight: target =>
@@ -198,7 +211,7 @@
if (
navigateToNextFile &&
result === CursorMoveResult.CLIPPED &&
- this.$.cursorManager.isAtEnd()
+ this.isAtEnd()
) {
if (
this.lastDisplayedNavigateToNextFileToast &&
@@ -223,27 +236,31 @@
}
this._fixSide();
+ return result;
}
- moveToPreviousChunk() {
- this.$.cursorManager.previous({
+ moveToPreviousChunk(): CursorMoveResult {
+ const result = this.$.cursorManager.previous({
filter: (row: HTMLElement) => this._isFirstRowOfChunk(row),
});
this._fixSide();
+ return result;
}
- moveToNextCommentThread() {
- this.$.cursorManager.next({
+ moveToNextCommentThread(): CursorMoveResult {
+ const result = this.$.cursorManager.next({
filter: (row: HTMLElement) => this._rowHasThread(row),
});
this._fixSide();
+ return result;
}
- moveToPreviousCommentThread() {
- this.$.cursorManager.previous({
+ moveToPreviousCommentThread(): CursorMoveResult {
+ const result = this.$.cursorManager.previous({
filter: (row: HTMLElement) => this._rowHasThread(row),
});
this._fixSide();
+ return result;
}
moveToLineNumber(number: number, side: Side, path?: string) {
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 5e90025..9d24b3e 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
@@ -600,6 +600,7 @@
if (this.modifierPressed(e)) return;
e.preventDefault();
+ this.classList.remove('hideComments');
this.$.cursor.createCommentInPlace();
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api-types.ts b/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api-types.ts
new file mode 100644
index 0000000..f587da1
--- /dev/null
+++ b/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api-types.ts
@@ -0,0 +1,370 @@
+/**
+ * @license
+ * Copyright (C) 2020 The Android Open Source Settings
+ *
+ * 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.
+ */
+
+// IMPORTANT !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+// The entire API is currently in DRAFT state.
+// Changes to all type and interfaces are expected.
+// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+
+export interface GrChecksApiInterface {
+ /**
+ * Must only be called once. You cannot register twice. You cannot unregister.
+ */
+ register(provider: ChecksProvider, config?: ChecksApiConfig): void;
+
+ /**
+ * Forces Gerrit to call fetch() on the registered provider. Can be called
+ * when the provider has gotten an update and does not want wait for the next
+ * polling interval to pass.
+ */
+ announceUpdate(): void;
+}
+
+export interface ChecksApiConfig {
+ /**
+ * How often should the provider be called for new CheckData while the user
+ * navigates change related pages and the browser tab remains visible?
+ * Set to 0 to disable polling. Default is 60 seconds.
+ */
+ fetchPollingIntervalSeconds: number;
+}
+
+export interface ChecksProvider {
+ /**
+ * Gerrit calls this method when ...
+ * - ... the change or diff page is loaded.
+ * - ... the user switches back to a Gerrit tab with a change or diff page.
+ * - ... while the tab is visible in a regular polling interval, see
+ * ChecksApiConfig.
+ */
+ fetch(change: number, patchset: number): Promise<FetchResponse>;
+}
+
+export interface FetchResponse {
+ responseCode: ResponseCode;
+
+ /** Only relevant when the responseCode is ERROR. */
+ errorMessage?: string;
+
+ /**
+ * Only relevant when the responseCode is NOT_LOGGED_IN.
+ * Gerrit displays a "Login" button and calls this callback when the user
+ * clicks on the button.
+ */
+ loginCallback?: () => void;
+
+ runs: CheckRun[];
+ results: CheckResult[];
+}
+
+export enum ResponseCode {
+ OK,
+ ERROR,
+ NOT_LOGGED_IN,
+}
+
+/**
+ * A CheckRun models an entity that has start/end timestamps and can be in
+ * either of the states RUNNABLE, RUNNING, COMPLETED. By itself it cannot model
+ * an error, neither can it be failed or successful by itself. A run can be
+ * associated with 0 to n results (see below). So until runs are completed the
+ * runs are more interesting for the user: What is going on at the moment? When
+ * runs are completed the users' interest shifts to results: What do I have to
+ * fix? The only actions that can be associated with runs are RUN and CANCEL.
+ */
+export interface CheckRun {
+ /**
+ * Gerrit requests check runs and results from the plugin by change number and
+ * patchset number. So these two properties can as well be left empty when
+ * returning results to the Gerrit UI and are thus optional.
+ */
+ change?: number;
+ /**
+ * Typically only runs for the latest patchset are requested and presented.
+ * Older runs and their results are only available on request, e.g. by
+ * switching to another patchset in a dropdown
+ *
+ * TBD: CI data providers may decide that runs and results are applicable to a
+ * newer patchset, even if they were produced for an older, e.g. because only
+ * the commit message was changed. Maybe that warrants the addition of another
+ * optional field, e.g. `original_patchset`.
+ */
+ patchset?: number;
+ /**
+ * The UI will focus on just the latest attempt per run. Former attempts are
+ * accessible, but initially collapsed/hidden. Lower number means older
+ * attempt. Every run has its own attempt numbering, so attempt 3 of run A is
+ * not directly related to attempt 3 of run B.
+ *
+ * RUNNABLE runs must use `undefined` as attempt.
+ * COMPLETED and RUNNING runs must use an attempt number >=0.
+ *
+ * TBD: Optionally providing aggregate information about former attempts will
+ * probably be a useful feature, but we are deferring the exact data modeling
+ * of that to later.
+ */
+ attempt?: number;
+
+ /**
+ * An optional opaque identifier not used by Gerrit directly, but might be
+ * used by plugin extensions and callbacks.
+ */
+ externalId?: string;
+
+ // The following 3 properties are independent of this run *instance*. They
+ // just describe what the check is about and will be identical for other
+ // attempts or patchsets or changes.
+
+ /**
+ * The unique name of the check. There can’t be two runs with the same
+ * change/patchset/attempt/checkName combination.
+ * Multiple attempts of the same run must have the same checkName.
+ * It should be expected that this string is cut off at ~30 chars in the UI.
+ * The full name will then be shown in a tooltip.
+ */
+ checkName: string;
+ /**
+ * Optional description of the check. Only shown as a tooltip or in a
+ * hovercard.
+ */
+ checkDescription?: string;
+ /**
+ * Optional http link to an external page with more detailed information about
+ * this run. Must begin with 'http'.
+ */
+ checkLink?: string;
+
+ /**
+ * RUNNABLE: Not run (yet). Mostly useful for runs that the user can trigger
+ * (see actions). Cannot contain results.
+ * RUNNING: Subsumes "scheduled".
+ * COMPLETED: The attempt of the run has finished. Does not indicate at all
+ * whether the run was successful or not. Outcomes can and should
+ * be modeled using the CheckResult entity.
+ */
+ status: RunStatus;
+ /**
+ * Optional short description of the run status. This is a plain string
+ * without styling or formatting options. It will only be shown as a tooltip
+ * or in a hovercard.
+ *
+ * Examples:
+ * "40 tests running, 30 completed: 0 failing so far",
+ * "Scheduled 5 minutes ago, not running yet".
+ */
+ statusDescription?: string;
+ /**
+ * Optional http link to an external page with more detailed information about
+ * the run status. Must begin with 'http'.
+ */
+ statusLink?: string;
+
+ /**
+ * Optional reference to a Gerrit label (e.g. "Verified") that this result
+ * influences. Allows the user to understand and navigate the relationship
+ * between CI results and submit requirements,
+ * see also https://gerrit-review.googlesource.com/c/homepage/+/279176.
+ */
+ labelName?: string;
+
+ /**
+ * Optional callbacks to the CI plugin. Must be implemented individually by
+ * each plugin. The most important actions (which get special UI treatment)
+ * are:
+ * "Run" for RUNNABLE and COMPLETED runs.
+ * "Cancel" for RUNNING runs.
+ */
+ actions: Action[];
+
+ scheduledTimestamp?: Date;
+ startedTimestamp?: Date;
+ finishedTimestamp?: Date;
+
+ /**
+ * List of results produced by this run.
+ * RUNNABLE runs must not have results.
+ * RUNNING runs can contain (intermediate) results.
+ * Nesting the results in runs enforces that:
+ * - A run can have 0-n results.
+ * - A result is associated with exactly one run.
+ */
+ results: CheckResult[];
+}
+
+export interface Action {
+ name: string;
+ tooltip?: string;
+ /**
+ * Primary actions will get a more prominent treatment in the UI. For example
+ * primary actions might be rendered as buttons versus just menu entries in
+ * an overflow menu.
+ */
+ primary: boolean;
+ callback: ActionCallback;
+}
+
+export type ActionCallback = (
+ change: number,
+ patchset: number,
+ attempt: number,
+ externalId: string,
+ /** Identical to 'checkName' property of CheckRun. */
+ checkName: string,
+ /** Identical to 'name' property of Action entity. */
+ actionName: string
+) => Promise<ActionResult>;
+
+export interface ActionResult {
+ /** An empty errorMessage means success. */
+ errorMessage?: string;
+}
+
+export enum RunStatus {
+ RUNNABLE,
+ RUNNING,
+ COMPLETED,
+}
+
+export interface CheckResult {
+ /**
+ * An optional opaque identifier not used by Gerrit directly, but might be
+ * used by plugin extensions and callbacks.
+ */
+ externalId?: string;
+
+ /**
+ * INFO: The user will typically not bother to look into this category,
+ * only for looking up something that they are searching for. Can be
+ * used for reporting secondary metrics and analysis, or a wider
+ * range of artifacts produced by the CI system.
+ * WARNING: A warning is something that should be read before submitting the
+ * change. The user should not ignore it, but it is also not blocking
+ * submit. It has a similar level of importance as an unresolved
+ * comment.
+ * ERROR: An error indicates that the change must not or cannot be submitted
+ * without fixing the problem. Errors will be visualized very
+ * prominently to the user.
+ *
+ * The ‘tags’ field below can be used for further categorization, e.g. for
+ * distinguishing FAILED vs TIMED_OUT.
+ */
+ category: Category;
+
+ /**
+ * Short description of the check result.
+ *
+ * It should be expected that this string might be cut off at ~80 chars in the
+ * UI. The full description will then be shown in a tooltip.
+ * This is a plain string without styling or formatting options.
+ *
+ * Examples:
+ * MessageConverterTest failed with: 'kermit' expected, but got 'ernie'.
+ * Binary size of javascript bundle has increased by 27%.
+ */
+ summary: string;
+
+ /**
+ * Exhaustive optional message describing the check result.
+ * Will be initially collapsed. Might potentially be very long, e.g. a log of
+ * MB size. The UI is not limiting this. CI data providers are responsible for
+ * not killing the browser. :-)
+ *
+ * For now this is just a plain unformatted string. The only formatting
+ * applied is the one that Gerrit also applies to human comments. TBD: Both
+ * human comments and check result messages should get richer formatting
+ * options.
+ */
+ message?: string;
+
+ /**
+ * Tags allow a CI System to further categorize a result, e.g. making a list
+ * of results filterable by the end-user.
+ * The name is free-form, but there is a predefined set of TagColors to
+ * choose from with a recommendation of color for common tags, see below.
+ *
+ * Examples:
+ * PASS, FAIL, SCHEDULED, OBSOLETE, SKIPPED, TIMED_OUT, INFRA_ERROR, FLAKY
+ * WIN, MAC, LINUX
+ * BUILD, TEST, LINT
+ * INTEGRATION, E2E, SCREENSHOT
+ */
+ tags: Tag[];
+
+ /**
+ * Links provide an opportunity for the end-user to easily access details and
+ * artifacts. Links are displayed by an icon+tooltip only. They don’t have a
+ * name, making them clearly distinguishable from tags and actions.
+ *
+ * There is a fixed set of LinkIcons to choose from, see below.
+ *
+ * Examples:
+ * Link to test log.
+ * Link to result artifacts such as images and screenshots.
+ * Link to downloadable artifacts such as ZIP or APK files.
+ */
+ links: Link[];
+
+ /**
+ * Callbacks to the CI plugin. Must be implemented individually by each
+ * plugin. Actions are rendered as buttons. If there are more than two actions
+ * per result, then further actions are put into an overflow menu. Sort order
+ * is defined by the data provider.
+ *
+ * Examples:
+ * Acknowledge/Dismiss, Delete, Report a bug, Report as not useful,
+ * Make blocking, Downgrade severity.
+ */
+ actions: Action[];
+}
+
+export enum Category {
+ INFO,
+ WARNING,
+ ERROR,
+}
+
+export interface Tag {
+ name: string;
+ tooltip?: string;
+ color?: TagColor;
+}
+
+// TBD: Add more ...
+// TBD: Clarify standard colors for common tags.
+export enum TagColor {
+ GRAY,
+ GREEN,
+}
+
+export interface Link {
+ /** Must begin with 'http'. */
+ url: string;
+ tooltip?: string;
+ /**
+ * Primary links will get a more prominent treatment in the UI, e.g. being
+ * always visible in the results table or also showing up in the change page
+ * summary of checks.
+ */
+ primary: boolean;
+ icon: LinkIcon;
+}
+
+// TBD: Add more ...
+export enum LinkIcon {
+ EXTERNAL,
+ DOWNLOAD,
+}
diff --git a/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts b/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts
index bfcff33..50b9222 100644
--- a/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts
@@ -15,7 +15,60 @@
* limitations under the License.
*/
import {PluginApi} from '../gr-plugin-types';
+import {
+ ChecksApiConfig,
+ ChecksProvider,
+ GrChecksApiInterface,
+ ResponseCode,
+} from './gr-checks-api-types';
-export class GrChecksApi {
+const DEFAULT_CONFIG: ChecksApiConfig = {
+ fetchPollingIntervalSeconds: 60,
+};
+
+enum State {
+ NOT_REGISTERED,
+ REGISTERED,
+ FETCHING,
+}
+
+/**
+ * Plugin API for checks.
+ *
+ * This object is created/returned to plugins that want to provide check data.
+ * Plugins normally just call register() once at startup and then wait for
+ * fetch() being called on the provider interface.
+ */
+export class GrChecksApi implements GrChecksApiInterface {
+ private provider?: ChecksProvider;
+
+ config?: ChecksApiConfig;
+
+ private state = State.NOT_REGISTERED;
+
constructor(readonly plugin: PluginApi) {}
+
+ announceUpdate() {
+ // TODO(brohlfs): Implement!
+ }
+
+ register(provider: ChecksProvider, config?: ChecksApiConfig): void {
+ if (this.state !== State.NOT_REGISTERED || this.provider)
+ throw new Error('Only one provider can be registered per plugin.');
+ this.state = State.REGISTERED;
+ this.provider = provider;
+ this.config = config ?? DEFAULT_CONFIG;
+ }
+
+ async fetch(change: number, patchset: number) {
+ if (this.state === State.NOT_REGISTERED || !this.provider)
+ throw new Error('Cannot fetch checks without a registered provider.');
+ if (this.state === State.FETCHING) return;
+ this.state = State.FETCHING;
+ const response = await this.provider.fetch(change, patchset);
+ this.state = State.REGISTERED;
+ if (response.responseCode === ResponseCode.OK) {
+ // TODO(brohlfs): Do something with the response.
+ }
+ }
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
index c021461..c3125c1 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
@@ -249,12 +249,17 @@
<div id="container" class="container">
<div class="header" id="header" on-click="_handleToggleCollapsed">
<div class="headerLeft">
- <gr-account-label
- account="[[_getAuthor(comment, _selfAccount)]]"
- class$="[[_computeAccountLabelClass(draft)]]"
- hide-status=""
- >
- </gr-account-label>
+ <template is="dom-if" if="[[comment.robot_id]]">
+ <span class="robotName"> [[comment.robot_id]] </span>
+ </template>
+ <template is="dom-if" if="[[!comment.robot_id]]">
+ <gr-account-label
+ account="[[_getAuthor(comment, _selfAccount)]]"
+ class$="[[_computeAccountLabelClass(draft)]]"
+ hide-status=""
+ >
+ </gr-account-label>
+ </template>
<gr-tooltip-content
class="draftTooltip"
has-tooltip=""
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js
index 10925af..e8d74f0 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js
@@ -695,9 +695,8 @@
assert.isTrue(runDetailsLink.href.indexOf(element.comment.url) !== -1);
const robotServiceName = element.shadowRoot
- .querySelector('gr-account-label')
- .shadowRoot.querySelector('span.name');
- assert.equal(robotServiceName.textContent.trim(), 'Display name Robot');
+ .querySelector('.robotName');
+ assert.equal(robotServiceName.textContent.trim(), 'happy_robot_id');
const authorName = element.shadowRoot
.querySelector('.robotId');
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
index 352bc58..2fbbd7c 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
@@ -261,14 +261,14 @@
this._targetHeight = null;
}
- isAtStart() {
- return this.index === 0;
+ /** Returns true if there are no stops, or we are on the first stop. */
+ isAtStart(): boolean {
+ return this.stops.length === 0 || this.index === 0;
}
- isAtEnd() {
- // Unset cursor should not be considered "at end", even when there are no
- // cursor stops.
- return this.index !== -1 && this.index === this.stops.length - 1;
+ /** Returns true if there are no stops, or we are on the last stop. */
+ isAtEnd(): boolean {
+ return this.stops.length === 0 || this.index === this.stops.length - 1;
}
moveToStart() {
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js
index 33aeafc..6f74d6b 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js
@@ -117,6 +117,16 @@
assert.equal(element.index, -1);
});
+ test('isAtStart() returns true when there are no stops', () => {
+ element.stops = [];
+ assert.isTrue(element.isAtStart());
+ });
+
+ test('isAtEnd() returns true when there are no stops', () => {
+ element.stops = [];
+ assert.isTrue(element.isAtEnd());
+ });
+
test('next() goes to first element when no cursor is set', () => {
element.stops = [...list.querySelectorAll('li')];
const result = element.next();
@@ -137,8 +147,6 @@
assert.equal(element.index, -1);
assert.isNotOk(element.target);
assert.isFalse(list.children[1].classList.contains('targeted'));
- assert.isFalse(element.isAtStart());
- assert.isFalse(element.isAtEnd());
});
test('previous() goes to last element when no cursor is set', () => {
@@ -162,8 +170,6 @@
assert.equal(element.index, -1);
assert.isNotOk(element.target);
assert.isFalse(list.children[1].classList.contains('targeted'));
- assert.isFalse(element.isAtStart());
- assert.isFalse(element.isAtEnd());
});
test('_moveCursor', () => {
