Merge "Remove references to replacement mode"
diff --git a/owners/src/main/resources/Documentation/files-owners-status-not-ready-hover.png b/owners/src/main/resources/Documentation/files-owners-status-not-ready-hover.png
new file mode 100644
index 0000000..8c43565
--- /dev/null
+++ b/owners/src/main/resources/Documentation/files-owners-status-not-ready-hover.png
Binary files differ
diff --git a/owners/src/main/resources/Documentation/files-owners-status-ready-hover.png b/owners/src/main/resources/Documentation/files-owners-status-ready-hover.png
new file mode 100644
index 0000000..0d57ca6
--- /dev/null
+++ b/owners/src/main/resources/Documentation/files-owners-status-ready-hover.png
Binary files differ
diff --git a/owners/src/main/resources/Documentation/files-owners-status.png b/owners/src/main/resources/Documentation/files-owners-status.png
new file mode 100644
index 0000000..afdae26
--- /dev/null
+++ b/owners/src/main/resources/Documentation/files-owners-status.png
Binary files differ
diff --git a/owners/src/main/resources/Documentation/how-to-use.md b/owners/src/main/resources/Documentation/how-to-use.md
new file mode 100644
index 0000000..aa9191a
--- /dev/null
+++ b/owners/src/main/resources/Documentation/how-to-use.md
@@ -0,0 +1,125 @@
+# How to use
+
+## Intro
+
+The `@PLUGIN@` plugin allows defining owners (individual accounts or groups)
+of directories, sub-directories and files that require approval when changes
+modify them.
+
+The rules defining which user/group needs to approve which file are specified
+in the `OWNERS` file and are covered in the
+[@PLUGIN@ configuration](config.html) guide.
+
+## Context
+
+The owners plugin can be used in three different modes:
+1. Prolog rules only(deprecated since Gerrit 3.6).
+2. With plugin-provided submit requirements.
+3. With user defined custom submit requirements.
+
+When using the plugin in mode 1. the functionality is limited to the generation
+of a Prolog-based submit rule with no extra UI features.
+
+On top of providing significantly better and more predictable performances,
+using the plugin in either mode 2. or 3. provides extra capabilities like:
+- A REST-api that exposes the owners approval status with single file granularity.
+- Enhanced UI experience with much clearer explanation of who owns what files,
+ as explained below.
+
+## <a id="ownerStatus">Owner status on change page
+
+### <a id="ownerStatus.submitRequirements">Owners status for submit requirements
+
+When either `has:approval_owners` predicate is used in a submit requirement or
+[enableSubmitRequirement](config.html#owners.enableSubmitRequirement) is enabled for a
+project or any of its parents and, if applicable, owners status is displayed in two ways:
+
+* As an icon prepending the submit requirement and as a text next to it
+\
+
+
+Which then displays more detail information in a tooltip when hovered over.
+
+
+
+* Next to all owned files, even if they are owned by someone else, (when the
+newest patchset is selected)
+\
+
+
+#### <a id="ownersStatus.submitRequirement.files">Per file owners statuses
+
+The @PLUGIN@ plugin also shows the owners statuses per file in the file list.
+For each file, the owners status is shown as an icon. One can **hover over
+the icon** to get additional information displayed as a tooltip.
+
+- **Needs Owners' Approval**\
+ A file owner either has not voted or voted negatively or not with sufficient
+ score.
+\
+
+
+
+ > **Notes:**
+ >
+ > The dialog contains up to **5** first file owners returned by the
+ > @PLUGIN@ plugin's REST API.
+ >
+ > Apart from the owner's name, their votes (if cast) and
+ > `Copy owner's email to clipboard` button are shown.
+ >
+ > Hovering over the owner's name results in their details being displayed
+
+
+ 
+
+- **Approved by Owners**\
+ A file owner has approved the change.
+\
+
+
+
+ > Note that again, apart from the owner's name, their vote and
+ > `Copy owner's email to clipboard` button are shown, and owner's details are
+ > displayed when one hovers over the name.
+
+### <a id="ownerStatus.submitRule">Owners status for submit rule
+
+ `OWNERS` file can still be evaluated (without a need for the prolog
+predicate being added to the project) by enabling the
+[default submit requirements](config.html#owners.enableSubmitRequirement).
+In this way, results of the `OWNERS` file evaluation are provided to
+Gerrit's change through submit rule (as
+[submit record](/Documentation/rest-api-changes.html#submit-record)) and, if
+applicable, owners status is displayed:
+
+* As an icon prepending the `Code Review from owners` submit requirement
+\
+
+
+
+ > Note that when default submit requirements are enabled, owner's votes are
+ > displayed next to the `Code Review from owners` submit requirement instead
+ > of its status. It is a Gerrit's core representation of submit rule.
+
+* Next to all owned files (when the newest patchset is selected)
+\
+
+
+
+ > There is no difference in this aspect between modes.
+
+#### <a id="ownersStatus.submitRule.owners">`Code Review from owners` submit requirement
+
+The `Code Review from owners` (i.e. the
+[default submit requirement](config.html#owners.enableSubmitRequirement)) submit
+requirement provides general information if all owned files were approved
+(requirement is satisfied). When hovered, a detailed description is shown
+
+
+
+#### <a id="ownersStatus.submitRule.files">Per file owners statuses
+
+The owners status per file when [default submit
+requirements](config.html#owners.enableSubmitRequirement) are enabled doesn't
+differ from [submit requirements mode](#ownersStatus.submitRequirement.files).
diff --git a/owners/src/main/resources/Documentation/owner-details.png b/owners/src/main/resources/Documentation/owner-details.png
new file mode 100644
index 0000000..f5ba6fd
--- /dev/null
+++ b/owners/src/main/resources/Documentation/owner-details.png
Binary files differ
diff --git a/owners/src/main/resources/Documentation/submit-requirement-hover.png b/owners/src/main/resources/Documentation/submit-requirement-hover.png
new file mode 100644
index 0000000..15b5f9c
--- /dev/null
+++ b/owners/src/main/resources/Documentation/submit-requirement-hover.png
Binary files differ
diff --git a/owners/src/main/resources/Documentation/submit-requirement.png b/owners/src/main/resources/Documentation/submit-requirement.png
new file mode 100644
index 0000000..a9c27de
--- /dev/null
+++ b/owners/src/main/resources/Documentation/submit-requirement.png
Binary files differ
diff --git a/owners/src/main/resources/Documentation/submit-rule-hover.png b/owners/src/main/resources/Documentation/submit-rule-hover.png
new file mode 100644
index 0000000..ab5995c
--- /dev/null
+++ b/owners/src/main/resources/Documentation/submit-rule-hover.png
Binary files differ
diff --git a/owners/src/main/resources/Documentation/submit-rule.png b/owners/src/main/resources/Documentation/submit-rule.png
new file mode 100644
index 0000000..b71e62f
--- /dev/null
+++ b/owners/src/main/resources/Documentation/submit-rule.png
Binary files differ
diff --git a/owners/web/gr-owned-files.ts b/owners/web/gr-owned-files.ts
index 96ea373..29f8b7a 100644
--- a/owners/web/gr-owned-files.ts
+++ b/owners/web/gr-owned-files.ts
@@ -15,7 +15,15 @@
* limitations under the License.
*/
-import {css, html, LitElement, PropertyValues, nothing, CSSResult} from 'lit';
+import {
+ css,
+ html,
+ LitElement,
+ PropertyValues,
+ nothing,
+ CSSResult,
+ unsafeCSS,
+} from 'lit';
import {ifDefined} from 'lit/directives/if-defined.js';
import {OwnersMixin} from './owners-mixin';
import {customElement, property} from 'lit/decorators';
@@ -42,18 +50,23 @@
truncatePath,
} from './utils';
-const STATUS_CODE = {
- MISSING: 'missing',
- APPROVED: 'approved',
-};
+export enum FileStatus {
+ NEEDS_APPROVAL = 'missing',
+ APPROVED = 'approved',
+}
const STATUS_ICON = {
- [STATUS_CODE.MISSING]: 'schedule',
- [STATUS_CODE.APPROVED]: 'check',
+ [FileStatus.NEEDS_APPROVAL]: 'schedule',
+ [FileStatus.APPROVED]: 'check',
};
+interface OwnedFileInfo {
+ status: FileStatus;
+ file: string;
+}
+
export interface OwnedFilesInfo {
- ownedFiles: string[];
+ ownedFiles: OwnedFileInfo[];
numberOfPending: number;
numberOfApproved: number;
}
@@ -79,7 +92,13 @@
}
static commonStyles(): CSSResult[] {
- return [window?.Gerrit?.styles.font as CSSResult];
+ const template = document.querySelector(
+ 'dom-module#shared-styles > template'
+ ) as HTMLTemplateElement;
+ const sharedStyles = css`
+ ${unsafeCSS(template?.content?.querySelector('style')?.textContent)}
+ `;
+ return [sharedStyles, window?.Gerrit?.styles.font as CSSResult];
}
private computeOwnedFiles() {
@@ -91,7 +110,7 @@
@customElement(OWNED_FILES_TAB_HEADER)
export class OwnedFilesTabHeader extends OwnedFilesCommon {
@property({type: String, reflect: true, attribute: 'files-status'})
- filesStatus?: string;
+ filesStatus?: FileStatus;
private ownedFiles: number | undefined;
@@ -125,8 +144,8 @@
} else {
[this.filesStatus, this.ownedFiles] =
this.ownedFilesInfo.numberOfPending > 0
- ? [STATUS_CODE.MISSING, this.ownedFilesInfo.numberOfPending]
- : [STATUS_CODE.APPROVED, this.ownedFilesInfo.numberOfApproved];
+ ? [FileStatus.NEEDS_APPROVAL, this.ownedFilesInfo.numberOfPending]
+ : [FileStatus.APPROVED, this.ownedFilesInfo.numberOfApproved];
}
}
@@ -184,7 +203,7 @@
}
private buildInfoAndSummary(
- filesStatus: string,
+ filesStatus: FileStatus,
filesApproved: number,
filesPending: number
): [string, string] {
@@ -201,12 +220,12 @@
} already approved.`
: '';
const info = `${
- STATUS_CODE.APPROVED === filesStatus
+ FileStatus.APPROVED === filesStatus
? approvedInfo
: `${pendingInfo}${filesApproved > 0 ? ` and ${approvedInfo}` : '.'}`
}`;
const summary = `${
- STATUS_CODE.APPROVED === filesStatus ? 'Approved' : 'Missing'
+ FileStatus.APPROVED === filesStatus ? 'Approved' : 'Missing'
}`;
return [info, summary];
}
@@ -221,7 +240,7 @@
revision?: RevisionInfo;
@property({type: Array})
- ownedFiles?: string[];
+ ownedFiles?: OwnedFileInfo[];
static override get styles() {
return [
@@ -238,9 +257,11 @@
padding: var(--spacing-xs) var(--spacing-l);
}
.header-row {
+ padding-left: var(--spacing-m);
background-color: var(--background-color-secondary);
}
.file-row {
+ padding-left: var(--spacing-m);
cursor: pointer;
}
.file-row:hover {
@@ -249,13 +270,23 @@
.file-row.selected {
background-color: var(--selection-background-color);
}
+ .status {
+ padding: inherit;
+ }
.path {
+ padding: inherit;
cursor: pointer;
flex: 1;
/* Wrap it into multiple lines if too long. */
white-space: normal;
word-break: break-word;
}
+ gr-icon.status.approved {
+ color: var(--positive-green-text-color);
+ }
+ gr-icon.status.missing {
+ color: #ffa62f;
+ }
.matchingFilePath {
color: var(--deemphasized-text-color);
}
@@ -304,24 +335,39 @@
private renderOwnedFilesHeaderRow() {
return html`
<div class="header-row row" role="row">
+ <div class="status" role="columnheader">Status</div>
<div class="path" role="columnheader">File</div>
</div>
`;
}
- private renderOwnedFileRow(ownedFile: string, index: number) {
+ private renderOwnedFileRow(ownedFile: OwnedFileInfo, index: number) {
return html`
<div
class="file-row row"
tabindex="-1"
role="row"
- aria-label=${ownedFile}
+ aria-label=${ownedFile.file}
>
- ${this.renderFilePath(ownedFile, index)}
+ ${this.renderFileStatus(ownedFile.status)}
+ ${this.renderFilePath(ownedFile.file, index)}
</div>
`;
}
+ private renderFileStatus(status: FileStatus) {
+ const icon = STATUS_ICON[status];
+ return html`
+ <span class="status" role="gridcell">
+ <gr-icon
+ class="status ${status}"
+ icon=${icon}
+ aria-hidden="true"
+ ></gr-icon>
+ </span>
+ `;
+ }
+
private renderFilePath(file: string, index: number) {
const displayPath = computeDisplayPath(file);
const previousFile = (this.ownedFiles ?? [])[index - 1];
@@ -332,7 +378,7 @@
href=${ifDefined(computeDiffUrl(file, this.change, this.revision))}
>
<span title=${displayPath} class="fullFileName">
- ${this.renderStyledPath(file, previousFile)}
+ ${this.renderStyledPath(file, previousFile?.file)}
</span>
<span title=${displayPath} class="truncatedFileName">
${truncatePath(displayPath)}
@@ -392,7 +438,7 @@
change?: ChangeInfo,
revision?: RevisionInfo,
user?: User,
- ownedFiles?: string[]
+ ownedFiles?: OwnedFileInfo[]
) {
// don't show owned files when no change or change is abandoned/merged or being edited or viewing not current PS
if (
@@ -421,8 +467,9 @@
owner: AccountInfo,
groupPrefix: string,
files: OwnedFiles,
+ fileStatus: FileStatus,
emailWithoutDomain?: string
-): string[] {
+): OwnedFileInfo[] {
const ownedFiles = [];
for (const file of Object.keys(files ?? [])) {
if (
@@ -438,7 +485,7 @@
);
})
) {
- ownedFiles.push(file);
+ ownedFiles.push({file, status: fileStatus} as OwnedFileInfo);
}
}
@@ -463,12 +510,14 @@
owner,
groupPrefix,
filesOwners.files,
+ FileStatus.NEEDS_APPROVAL,
emailWithoutDomain
);
const approvedFiles = collectOwnedFiles(
owner,
groupPrefix,
filesOwners.files_approved,
+ FileStatus.APPROVED,
emailWithoutDomain
);
return {
diff --git a/owners/web/gr-owned-files_test.ts b/owners/web/gr-owned-files_test.ts
index d7bef24..fd37f67 100644
--- a/owners/web/gr-owned-files_test.ts
+++ b/owners/web/gr-owned-files_test.ts
@@ -26,7 +26,12 @@
EDIT,
SubmitRequirementResultInfo,
} from '@gerritcodereview/typescript-api/rest-api';
-import {ownedFiles, OwnedFilesInfo, shouldHide} from './gr-owned-files';
+import {
+ FileStatus,
+ ownedFiles,
+ OwnedFilesInfo,
+ shouldHide,
+} from './gr-owned-files';
import {FilesOwners, GroupOwner, Owner} from './owners-service';
import {deepEqual} from './utils';
import {User, UserRole} from './owners-model';
@@ -77,7 +82,10 @@
test('ownedFiles - should return owned files', () => {
assert.equal(
deepEqual(ownedFiles(owner, filesOwners), {
- ownedFiles: [ownedFile, ownedApprovedFile],
+ ownedFiles: [
+ {file: ownedFile, status: FileStatus.NEEDS_APPROVAL},
+ {file: ownedApprovedFile, status: FileStatus.APPROVED},
+ ],
numberOfApproved: 1,
numberOfPending: 1,
}),
@@ -94,7 +102,7 @@
} as unknown as FilesOwners;
assert.equal(
deepEqual(ownedFiles(owner, filesOwners), {
- ownedFiles: [ownedFile],
+ ownedFiles: [{file: ownedFile, status: FileStatus.NEEDS_APPROVAL}],
numberOfApproved: 0,
numberOfPending: 1,
}),
@@ -111,7 +119,7 @@
} as unknown as FilesOwners;
assert.equal(
deepEqual(ownedFiles(owner, filesOwners), {
- ownedFiles: [ownedFile],
+ ownedFiles: [{file: ownedFile, status: FileStatus.APPROVED}],
numberOfApproved: 1,
numberOfPending: 0,
}),
@@ -142,7 +150,7 @@
commit: {commit: current_revision},
} as unknown as RevisionInfo;
const user = {account: account(1), role: UserRole.OTHER};
- const ownedFiles = ['README.md'];
+ const ownedFiles = [{file: 'README.md', status: FileStatus.NEEDS_APPROVAL}];
test('shouldHide - should be `true` when change is `undefined`', () => {
const undefinedChange = undefined;
