Merge "Submit Requirements - Refine gr-account chip"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index a1dc27c..ace6995 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2216,6 +2216,9 @@
other project managed by the running server. The name is
relative to `gerrit.basePath`.
+
+The link:#cache_names[persisted_projects cache] must be
+flushed after this setting is changed.
++
Defaults to `All-Projects` if not set.
[[gerrit.defaultBranch]]gerrit.defaultBranch::
diff --git a/Documentation/user-attention-set.txt b/Documentation/user-attention-set.txt
index 1f67fc7..512f784 100644
--- a/Documentation/user-attention-set.txt
+++ b/Documentation/user-attention-set.txt
@@ -2,8 +2,6 @@
Report a bug or send feedback using
link:https://bugs.chromium.org/p/gerrit/issues/entry?template=Attention+Set[this Monorail template].
-You can also report a bug through the bug icon in the user hovercard and in the
-reply dialog.
[[whose-turn]]
== Whose turn is it?
@@ -133,24 +131,6 @@
Gerrit-Attention: Marian Harbach <mharbach@google.com>
----
-=== Assignee
-
-While the "Assignee" feature can still be used together with the attention set,
-we do not recommend doing so. Using both features is likely confusing. The
-distinct feature of the "Assignee" compared to the attention set is that only
-one user can be the assignee at the same time. So the assignee can be used to
-single out one person or escalate, if there are multiple reviewers. Since
-*every* reviewer in the attention set is expected to take action, singling out
-is not likely to be important and also still achievable with the attention set.
-Otherwise "Assignee" and "Attention Set" are very much overlapping, so we
-recommend to only use one of them.
-
-If you don't expect action from reviewers, then consider adding them to CC
-instead.
-
-The "Assignee" feature can be turned on/off with the
-link:config-gerrit.html#change.enableAssignee[enableAssignee] config option.
-
=== Bold Changes / Mark Reviewed
Before the attention set feature, changes were bolded in the dashboard when
diff --git a/java/com/google/gerrit/entities/AccessSection.java b/java/com/google/gerrit/entities/AccessSection.java
index 69a234a..8ae0a5d 100644
--- a/java/com/google/gerrit/entities/AccessSection.java
+++ b/java/com/google/gerrit/entities/AccessSection.java
@@ -18,12 +18,14 @@
import static java.util.Objects.requireNonNull;
import com.google.auto.value.AutoValue;
+import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;
+import java.util.regex.Pattern;
/** Portion of a {@link Project} describing access rules. */
@AutoValue
@@ -42,6 +44,20 @@
/** Name of the access section. It could be a ref pattern or something else. */
public abstract String getName();
+ /**
+ * A compiled regular expression in case {@link #getName()} is a regular expression. This is
+ * memoized to save callers from compiling patterns for every use.
+ */
+ @Memoized
+ public Optional<Pattern> getNamePattern() {
+ if (isValidRefSectionName(getName())
+ && getName().startsWith(REGEX_PREFIX)
+ && !getName().contains("${")) {
+ return Optional.of(Pattern.compile(getName()));
+ }
+ return Optional.empty();
+ }
+
public abstract ImmutableList<Permission> getPermissions();
public static AccessSection create(String name) {
diff --git a/java/com/google/gerrit/entities/CachedProjectConfig.java b/java/com/google/gerrit/entities/CachedProjectConfig.java
index cd65efc..be4a1cf 100644
--- a/java/com/google/gerrit/entities/CachedProjectConfig.java
+++ b/java/com/google/gerrit/entities/CachedProjectConfig.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedMap;
import com.google.common.flogger.FluentLogger;
import java.util.Collection;
import java.util.List;
@@ -69,7 +70,7 @@
public abstract AccountsSection getAccountsSection();
/** Returns a map of {@link AccessSection}s keyed by their name. */
- public abstract ImmutableMap<String, AccessSection> getAccessSections();
+ public abstract ImmutableSortedMap<String, AccessSection> getAccessSections();
/** Returns the {@link AccessSection} with to the given name. */
public Optional<AccessSection> getAccessSection(String refName) {
@@ -235,7 +236,7 @@
protected abstract ImmutableMap.Builder<AccountGroup.UUID, GroupReference> groupsBuilder();
- protected abstract ImmutableMap.Builder<String, AccessSection> accessSectionsBuilder();
+ protected abstract ImmutableSortedMap.Builder<String, AccessSection> accessSectionsBuilder();
protected abstract ImmutableMap.Builder<String, ContributorAgreement>
contributorAgreementsBuilder();
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index 27c6793..1191db8 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -173,7 +173,13 @@
return ref(notes.getChange().getDest()).change(notes);
}
- /** Verify scoped user can {@code perm}, throwing if denied. */
+ /**
+ * Verify scoped user can {@code perm}, throwing if denied.
+ *
+ * <p>Should be used in REST API handlers where the thrown {@link AuthException} can be
+ * propagated. In business logic, where the exception would have to be caught, prefer using
+ * {@link #test(GlobalOrPluginPermission)}.
+ */
public abstract void check(GlobalOrPluginPermission perm)
throws AuthException, PermissionBackendException;
@@ -280,7 +286,13 @@
return ref(notes.getChange().getDest().branch()).change(notes);
}
- /** Verify scoped user can {@code perm}, throwing if denied. */
+ /**
+ * Verify scoped user can {@code perm}, throwing if denied.
+ *
+ * <p>Should be used in REST API handlers where the thrown {@link AuthException} can be
+ * propagated. In business logic, where the exception would have to be caught, prefer using
+ * {@link #test(CoreOrPluginProjectPermission)}.
+ */
public abstract void check(CoreOrPluginProjectPermission perm)
throws AuthException, PermissionBackendException;
@@ -368,7 +380,13 @@
/** Returns an instance scoped to change. */
public abstract ForChange change(ChangeNotes notes);
- /** Verify scoped user can {@code perm}, throwing if denied. */
+ /**
+ * Verify scoped user can {@code perm}, throwing if denied.
+ *
+ * <p>Should be used in REST API handlers where the thrown {@link AuthException} can be
+ * propagated. In business logic, where the exception would have to be caught, prefer using
+ * {@link #test(RefPermission)}.
+ */
public abstract void check(RefPermission perm) throws AuthException, PermissionBackendException;
/** Filter {@code permSet} to permissions scoped user might be able to perform. */
@@ -406,7 +424,13 @@
/** Returns the fully qualified resource path that this instance is scoped to. */
public abstract String resourcePath();
- /** Verify scoped user can {@code perm}, throwing if denied. */
+ /**
+ * Verify scoped user can {@code perm}, throwing if denied.
+ *
+ * <p>Should be used in REST API handlers where the thrown {@link AuthException} can be
+ * propagated. In business logic, where the exception would have to be caught, prefer using
+ * {@link #test(ChangePermissionOrLabel)}.
+ */
public abstract void check(ChangePermissionOrLabel perm)
throws AuthException, PermissionBackendException;
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 31bbff5..3aa3783 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -130,7 +130,7 @@
.keySerializer(new ProtobufSerializer<>(Cache.ProjectCacheKeyProto.parser()))
.valueSerializer(PersistedProjectConfigSerializer.INSTANCE)
.diskLimit(1 << 30) // 1 GiB
- .version(3)
+ .version(4)
.maximumWeight(0);
cache(CACHE_LIST, ListKey.class, new TypeLiteral<ImmutableSortedSet<Project.NameKey>>() {})
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 4a17b5c..123a14c 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -224,7 +224,8 @@
projectName,
projectName.equals(allProjectsName)
? allProjectsConfigProvider.get(allProjectsName)
- : Optional.empty());
+ : Optional.empty(),
+ allProjectsName);
}
public ProjectConfig read(MetaDataUpdate update) throws IOException, ConfigInvalidException {
@@ -250,6 +251,7 @@
}
private final Optional<StoredConfig> baseConfig;
+ private final AllProjectsName allProjectsName;
private Project project;
private AccountsSection accountsSection;
@@ -287,7 +289,6 @@
.setCheckReceivedObjects(checkReceivedObjects)
.setExtensionPanelSections(extensionPanelSections);
groupList.byUUID().values().forEach(g -> builder.addGroup(g));
- accessSections.values().forEach(a -> builder.addAccessSection(a));
contributorAgreements.values().forEach(c -> builder.addContributorAgreement(c));
notifySections.values().forEach(n -> builder.addNotifySection(n));
subscribeSections.values().forEach(s -> builder.addSubscribeSection(s));
@@ -300,6 +301,28 @@
projectLevelConfigs
.entrySet()
.forEach(c -> builder.addProjectLevelConfig(c.getKey(), c.getValue().toText()));
+
+ if (projectName.equals(allProjectsName)) {
+ // Filter out permissions that aren't allowed to be set on All-Projects
+ accessSections
+ .values()
+ .forEach(
+ a -> {
+ List<Permission.Builder> copy = new ArrayList<>();
+ for (Permission p : a.getPermissions()) {
+ if (Permission.canBeOnAllProjects(a.getName(), p.getName())) {
+ copy.add(p.toBuilder());
+ }
+ }
+ AccessSection section =
+ AccessSection.builder(a.getName())
+ .modifyPermissions(permissions -> permissions.addAll(copy))
+ .build();
+ builder.addAccessSection(section);
+ });
+ } else {
+ accessSections.values().forEach(a -> builder.addAccessSection(a));
+ }
return builder.build();
}
@@ -355,9 +378,13 @@
requireNonNull(commentLinkSections.remove(name));
}
- private ProjectConfig(Project.NameKey projectName, Optional<StoredConfig> baseConfig) {
+ private ProjectConfig(
+ Project.NameKey projectName,
+ Optional<StoredConfig> baseConfig,
+ AllProjectsName allProjectsName) {
this.projectName = projectName;
this.baseConfig = baseConfig;
+ this.allProjectsName = allProjectsName;
}
public void load(Repository repo) throws IOException, ConfigInvalidException {
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index 69e6036..6352f66 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -15,9 +15,7 @@
package com.google.gerrit.server.project;
import static com.google.common.base.Preconditions.checkState;
-import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.entities.PermissionRule.Action.ALLOW;
-import static java.util.Comparator.comparing;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.FluentIterable;
@@ -268,35 +266,18 @@
/** Get the sections that pertain only to this project. */
List<SectionMatcher> getLocalAccessSections() {
- List<SectionMatcher> sm = localAccessSections;
- if (sm == null) {
- ImmutableList<AccessSection> fromConfig =
- cachedConfig.getAccessSections().values().stream()
- .sorted(comparing(AccessSection::getName))
- .collect(toImmutableList());
- sm = new ArrayList<>(fromConfig.size());
- for (AccessSection section : fromConfig) {
- if (isAllProjects) {
- List<Permission.Builder> copy = new ArrayList<>();
- for (Permission p : section.getPermissions()) {
- if (Permission.canBeOnAllProjects(section.getName(), p.getName())) {
- copy.add(p.toBuilder());
- }
- }
- section =
- AccessSection.builder(section.getName())
- .modifyPermissions(permissions -> permissions.addAll(copy))
- .build();
- }
-
- SectionMatcher matcher = SectionMatcher.wrap(getNameKey(), section);
- if (matcher != null) {
- sm.add(matcher);
- }
- }
- localAccessSections = sm;
+ if (localAccessSections != null) {
+ return localAccessSections;
}
- return sm;
+ List<SectionMatcher> sm = new ArrayList<>(cachedConfig.getAccessSections().values().size());
+ for (AccessSection section : cachedConfig.getAccessSections().values()) {
+ SectionMatcher matcher = SectionMatcher.wrap(getNameKey(), section);
+ if (matcher != null) {
+ sm.add(matcher);
+ }
+ }
+ localAccessSections = sm;
+ return localAccessSections;
}
/**
diff --git a/java/com/google/gerrit/server/project/RefPatternMatcher.java b/java/com/google/gerrit/server/project/RefPatternMatcher.java
index b9076b3..be840b5 100644
--- a/java/com/google/gerrit/server/project/RefPatternMatcher.java
+++ b/java/com/google/gerrit/server/project/RefPatternMatcher.java
@@ -22,6 +22,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
import com.google.gerrit.common.data.ParameterizedString;
+import com.google.gerrit.entities.AccessSection;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.server.CurrentUser;
@@ -32,6 +33,13 @@
import java.util.stream.Stream;
public abstract class RefPatternMatcher {
+ public static RefPatternMatcher getMatcher(AccessSection section) {
+ if (section.getNamePattern().isPresent()) {
+ return new Regexp(section.getNamePattern().get());
+ }
+ return getMatcher(section.getName());
+ }
+
public static RefPatternMatcher getMatcher(String pattern) {
if (containsParameters(pattern)) {
return new ExpandParameters(pattern);
@@ -79,6 +87,10 @@
pattern = Pattern.compile(re);
}
+ Regexp(Pattern re) {
+ pattern = re;
+ }
+
@Override
public boolean match(String ref, CurrentUser user) {
return pattern.matcher(ref).matches() || (isRE(ref) && pattern.pattern().equals(ref));
diff --git a/java/com/google/gerrit/server/project/SectionMatcher.java b/java/com/google/gerrit/server/project/SectionMatcher.java
index 763957e..3d7175f 100644
--- a/java/com/google/gerrit/server/project/SectionMatcher.java
+++ b/java/com/google/gerrit/server/project/SectionMatcher.java
@@ -28,7 +28,7 @@
static SectionMatcher wrap(Project.NameKey project, AccessSection section) {
String ref = section.getName();
if (AccessSection.isValidRefSectionName(ref)) {
- return new SectionMatcher(project, section, getMatcher(ref));
+ return new SectionMatcher(project, section, getMatcher(section));
}
return null;
}
diff --git a/polygerrit-ui/app/api/rest-api.ts b/polygerrit-ui/app/api/rest-api.ts
index 5cbae90..9eb8aa4 100644
--- a/polygerrit-ui/app/api/rest-api.ts
+++ b/polygerrit-ui/app/api/rest-api.ts
@@ -558,7 +558,7 @@
export declare interface ContributorAgreementInfo {
name: string;
description: string;
- url: string;
+ url?: string;
auto_verify_group?: GroupInfo;
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirement/gr-change-list-column-requirement.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirement/gr-change-list-column-requirement.ts
new file mode 100644
index 0000000..c832f13
--- /dev/null
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirement/gr-change-list-column-requirement.ts
@@ -0,0 +1,97 @@
+/**
+ * @license
+ * Copyright (C) 2022 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.
+ */
+
+import '../../change/gr-submit-requirement-dashboard-hovercard/gr-submit-requirement-dashboard-hovercard';
+import '../../shared/gr-change-status/gr-change-status';
+import {LitElement, css, html} from 'lit';
+import {customElement, property} from 'lit/decorators';
+import {ChangeInfo, SubmitRequirementStatus} from '../../../api/rest-api';
+import {submitRequirementsStyles} from '../../../styles/gr-submit-requirements-styles';
+import {getRequirements, iconForStatus} from '../../../utils/label-util';
+import {sharedStyles} from '../../../styles/shared-styles';
+
+@customElement('gr-change-list-column-requirement')
+export class GrChangeListColumnRequirement extends LitElement {
+ @property({type: Object})
+ change?: ChangeInfo;
+
+ @property()
+ labelName?: string;
+
+ static override get styles() {
+ return [
+ submitRequirementsStyles,
+ sharedStyles,
+ css`
+ iron-icon {
+ vertical-align: top;
+ }
+ .container.not-applicable {
+ background-color: var(--table-header-background-color);
+ height: calc(var(--line-height-normal) + var(--spacing-m));
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html`<div class="container ${this.computeClass()}">
+ ${this.renderContent()}
+ </div>`;
+ }
+
+ private renderContent() {
+ if (!this.labelName) return;
+ const requirements = this.getRequirement(this.labelName);
+ if (requirements.length === 0) return;
+
+ const icon = iconForStatus(
+ requirements[0].status ?? SubmitRequirementStatus.ERROR
+ );
+ return html`<iron-icon
+ class="${icon}"
+ icon="gr-icons:${icon}"
+ ></iron-icon>`;
+ }
+
+ private computeClass(): string {
+ if (!this.labelName) return '';
+ const requirements = this.getRequirement(this.labelName);
+ if (requirements.length === 0) {
+ return 'not-applicable';
+ }
+ return '';
+ }
+
+ private getRequirement(labelName: string) {
+ const requirements = getRequirements(this.change).filter(
+ sr => sr.name === labelName
+ );
+ // TODO(milutin): Remove this after migration from legacy requirements.
+ if (requirements.length > 1) {
+ return requirements.filter(sr => !sr.is_legacy);
+ } else {
+ return requirements;
+ }
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-change-list-column-requirement': GrChangeListColumnRequirement;
+ }
+}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirement/gr-change-list-column-requirement_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirement/gr-change-list-column-requirement_test.ts
new file mode 100644
index 0000000..01fcd2c
--- /dev/null
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirement/gr-change-list-column-requirement_test.ts
@@ -0,0 +1,70 @@
+/**
+ * @license
+ * Copyright (C) 2022 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.
+ */
+
+import '../../../test/common-test-setup-karma';
+import {fixture} from '@open-wc/testing-helpers';
+import {html} from 'lit';
+import './gr-change-list-column-requirement';
+import {GrChangeListColumnRequirement} from './gr-change-list-column-requirement';
+import {
+ createSubmitRequirementExpressionInfo,
+ createSubmitRequirementResultInfo,
+ createNonApplicableSubmitRequirementResultInfo,
+ createChange,
+} from '../../../test/test-data-generators';
+import {ChangeInfo, SubmitRequirementResultInfo} from '../../../api/rest-api';
+import {StandardLabels} from '../../../utils/label-util';
+
+suite('gr-change-list-column-requirement tests', () => {
+ let element: GrChangeListColumnRequirement;
+ let change: ChangeInfo;
+ setup(() => {
+ const submitRequirement: SubmitRequirementResultInfo = {
+ ...createSubmitRequirementResultInfo(),
+ name: StandardLabels.CODE_REVIEW,
+ submittability_expression_result: {
+ ...createSubmitRequirementExpressionInfo(),
+ expression: 'label:Verified=MAX -label:Verified=MIN',
+ },
+ };
+ change = {
+ ...createChange(),
+ submit_requirements: [
+ submitRequirement,
+ createNonApplicableSubmitRequirementResultInfo(),
+ ],
+ unresolved_comment_count: 1,
+ };
+ });
+
+ test('renders', async () => {
+ element = await fixture<GrChangeListColumnRequirement>(
+ html`<gr-change-list-column-requirement
+ .change=${change}
+ .labelName=${StandardLabels.CODE_REVIEW}
+ >
+ </gr-change-list-column-requirement>`
+ );
+ expect(element).shadowDom.to.equal(`<div class="container">
+ <iron-icon
+ class="check-circle-filled"
+ icon="gr-icons:check-circle-filled"
+ >
+ </iron-icon>
+ </div>`);
+ });
+});
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 a09466b..73b043a 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
@@ -25,6 +25,7 @@
import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
import '../../plugins/gr-endpoint-param/gr-endpoint-param';
import '../gr-change-list-column-requirements-summary/gr-change-list-column-requirements-summary';
+import '../gr-change-list-column-requirement/gr-change-list-column-requirement';
import '../../shared/gr-tooltip-content/gr-tooltip-content';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {getDisplayName} from '../../../utils/display-name-util';
@@ -44,17 +45,14 @@
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
import {pluralize} from '../../../utils/string-util';
-import {
- getRequirements,
- iconForStatus,
- showNewSubmitRequirements,
-} from '../../../utils/label-util';
+import {showNewSubmitRequirements} from '../../../utils/label-util';
import {changeListStyles} from '../../../styles/gr-change-list-styles';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, css, html} from 'lit';
import {customElement, property, state} from 'lit/decorators';
import {submitRequirementsStyles} from '../../../styles/gr-submit-requirements-styles';
import {ifDefined} from 'lit/directives/if-defined';
+import {KnownExperimentId} from '../../../services/flags/flags';
enum ChangeSize {
XS = 10,
@@ -250,12 +248,20 @@
.placeholder {
color: var(--deemphasized-text-color);
}
+ .cell.selection input {
+ vertical-align: middle;
+ }
.cell.label {
font-weight: var(--font-weight-normal);
}
.cell.label iron-icon {
vertical-align: top;
}
+ /* Requirement child needs whole area */
+ .cell.requirement {
+ padding: 0;
+ margin: 0;
+ }
@media only screen and (max-width: 50em) {
:host {
display: flex;
@@ -269,13 +275,14 @@
const changeUrl = this.computeChangeURL();
return html`
<td aria-hidden="true" class="cell leftPadding"></td>
- ${this.renderCellStar()} ${this.renderCellNumber(changeUrl)}
- ${this.renderCellSubject(changeUrl)} ${this.renderCellStatus()}
- ${this.renderCellOwner()} ${this.renderCellReviewers()}
- ${this.renderCellComments()} ${this.renderCellRepo()}
- ${this.renderCellBranch()} ${this.renderCellUpdated()}
- ${this.renderCellSubmitted()} ${this.renderCellWaiting()}
- ${this.renderCellSize()} ${this.renderCellRequirements()}
+ ${this.renderCellSelectionBox()} ${this.renderCellStar()}
+ ${this.renderCellNumber(changeUrl)} ${this.renderCellSubject(changeUrl)}
+ ${this.renderCellStatus()} ${this.renderCellOwner()}
+ ${this.renderCellReviewers()} ${this.renderCellComments()}
+ ${this.renderCellRepo()} ${this.renderCellBranch()}
+ ${this.renderCellUpdated()} ${this.renderCellSubmitted()}
+ ${this.renderCellWaiting()} ${this.renderCellSize()}
+ ${this.renderCellRequirements()}
${this.labelNames?.map(labelNames => this.renderChangeLabels(labelNames))}
${this.dynamicCellEndpoints?.map(pluginEndpointName =>
this.renderChangePluginEndpoint(pluginEndpointName)
@@ -283,6 +290,15 @@
`;
}
+ private renderCellSelectionBox() {
+ if (!this.flagsService.isEnabled(KnownExperimentId.BULK_ACTIONS)) return;
+ return html`
+ <td class="cell selection">
+ <input type="checkbox" />
+ </td>
+ `;
+ }
+
private renderCellStar() {
if (!this.showStar) return;
@@ -535,6 +551,15 @@
}
private renderChangeLabels(labelName: string) {
+ if (showNewSubmitRequirements(this.flagsService, this.change)) {
+ return html` <td class="cell label requirement">
+ <gr-change-list-column-requirement
+ .change=${this.change}
+ .labelName=${labelName}
+ >
+ </gr-change-list-column-requirement>
+ </td>`;
+ }
return html`
<td
title="${this.computeLabelTitle(labelName)}"
@@ -546,16 +571,6 @@
}
private renderChangeHasLabelIcon(labelName: string) {
- if (showNewSubmitRequirements(this.flagsService, this.change)) {
- const requirements = this.getRequirement(labelName);
- if (requirements.length === 1) {
- const icon = iconForStatus(requirements[0].status);
- return html`<iron-icon
- class="${icon}"
- icon="gr-icons:${icon}"
- ></iron-icon>`;
- }
- }
if (this.computeLabelIcon(labelName) === '')
return html`<span>${this.computeLabelValue(labelName)}</span>`;
@@ -611,14 +626,6 @@
// private but used in test
computeLabelClass(labelName: string) {
const classes = ['cell', 'label'];
- if (showNewSubmitRequirements(this.flagsService, this.change)) {
- const requirements = this.getRequirement(labelName);
- if (requirements.length === 1) {
- classes.push('requirement');
- // Do not add label category classes.
- return classes.sort().join(' ');
- }
- }
const category = this.computeLabelCategory(labelName);
switch (category) {
case LabelCategory.NOT_APPLICABLE:
@@ -900,16 +907,4 @@
const isLast = index === primaryCount - 1;
return isLast && additionalCount === 0;
}
-
- private getRequirement(labelName: string) {
- const requirements = getRequirements(this.change).filter(
- sr => sr.name === labelName
- );
- // TODO(milutin): Remove this after migration from legacy requirements.
- if (requirements.length > 1) {
- return requirements.filter(sr => !sr.is_legacy);
- } else {
- return requirements;
- }
- }
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
index ab8d2d7..cd43151 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
@@ -27,7 +27,12 @@
createSubmitRequirementResultInfo,
createNonApplicableSubmitRequirementResultInfo,
} from '../../../test/test-data-generators';
-import {query, queryAndAssert, stubRestApi} from '../../../test/test-utils';
+import {
+ query,
+ queryAndAssert,
+ stubRestApi,
+ stubFlags,
+} from '../../../test/test-utils';
import {
AccountId,
BranchName,
@@ -288,6 +293,14 @@
}
});
+ test('selection checkbox is only shown if experiment is enabled', async () => {
+ assert.isNotOk(query(element, '.selection'));
+ stubFlags('isEnabled').returns(true);
+ element = basicFixture.instantiate();
+ await element.updateComplete;
+ assert.isOk(query(element, '.selection'));
+ });
+
test('repo column hidden', async () => {
element.visibleChangeTableColumns = [
'Subject',
@@ -498,9 +511,7 @@
);
const requirement = queryAndAssert(element, '.requirement');
- expect(requirement).dom.to.equal(`<iron-icon
- class="check-circle-filled"
- icon="gr-icons:check-circle-filled">
- </iron-icon>`);
+ expect(requirement).dom.to.equal(`<gr-change-list-column-requirement>
+ </gr-change-list-column-requirement>`);
});
});
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index f276260..f8b1a2b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -252,6 +252,11 @@
`;
}
+ private renderSelectionHeader() {
+ if (!this.flagsService.isEnabled(KnownExperimentId.BULK_ACTIONS)) return;
+ return html`<td aria-hidden="true" class="selection"></td>`;
+ }
+
private renderSectionHeader(
changeSection: ChangeListSection,
labelNames: string[]
@@ -262,6 +267,7 @@
<tbody>
<tr class="groupHeader">
<td aria-hidden="true" class="leftPadding"></td>
+ ${this.renderSelectionHeader()}
<td aria-hidden="true" class="star" ?hidden=${!this.showStar}></td>
<td
class="cell"
@@ -317,6 +323,7 @@
return html`
<tr class="groupTitle">
<td class="leftPadding" aria-hidden="true"></td>
+ ${this.renderSelectionHeader()}
<td
class="star"
aria-label="Star status column"
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
index 50708c0..29204b6 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
@@ -292,6 +292,41 @@
});
});
+ test('selection checkbox is only shown if experiment is enabled', async () => {
+ function propertiesSetup(element: GrChangeList) {
+ element.sections = [{results: [{...createChange()}]}];
+ element.account = {_account_id: 1001 as AccountId};
+ element.preferences = {
+ legacycid_in_change_table: true,
+ time_format: TimeFormat.HHMM_12,
+ change_table: [
+ 'Subject',
+ 'Status',
+ 'Owner',
+ 'Reviewers',
+ 'Comments',
+ 'Repo',
+ 'Branch',
+ 'Updated',
+ 'Size',
+ ' Status ',
+ ],
+ };
+ element.config = createServerInfo();
+ }
+
+ element = basicFixture.instantiate();
+ propertiesSetup(element);
+ await element.updateComplete;
+ assert.isNotOk(query(element, '.selection'));
+
+ stubFlags('isEnabled').returns(true);
+ element = basicFixture.instantiate();
+ propertiesSetup(element);
+ await element.updateComplete;
+ assert.isOk(query(element, '.selection'));
+ });
+
suite('empty column preference', () => {
let element: GrChangeList;
diff --git a/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores.ts b/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores.ts
index 9492fa9..d01ae02 100644
--- a/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores.ts
+++ b/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores.ts
@@ -14,14 +14,17 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+import '../gr-trigger-vote/gr-trigger-vote';
import {LitElement, css, html} from 'lit';
import {customElement, property} from 'lit/decorators';
+import {ChangeInfo} from '../../../api/rest-api';
import {
ChangeMessage,
LabelExtreme,
PATCH_SET_PREFIX_PATTERN,
} from '../../../utils/comment-util';
import {hasOwnProperty} from '../../../utils/common-util';
+import {getTriggerVotes} from '../../../utils/label-util';
const VOTE_RESET_TEXT = '0 (vote reset)';
@@ -41,16 +44,22 @@
@property({type: Object})
message?: ChangeMessage;
+ @property({type: Object})
+ change?: ChangeInfo;
+
static override get styles() {
return css`
+ .score,
+ gr-trigger-vote {
+ padding: 0 var(--spacing-s);
+ margin-right: var(--spacing-s);
+ display: inline-block;
+ }
.score {
box-sizing: border-box;
border-radius: var(--border-radius);
color: var(--vote-text-color);
- display: inline-block;
- padding: 0 var(--spacing-s);
text-align: center;
- margin-right: var(--spacing-s);
min-width: 115px;
}
.score.removed {
@@ -93,10 +102,22 @@
override render() {
const scores = this._getScores(this.message, this.labelExtremes);
- return scores.map(score => this.renderScore(score));
+ const triggerVotes = getTriggerVotes(this.change);
+ return scores.map(score => this.renderScore(score, triggerVotes));
}
- private renderScore(score: Score) {
+ private renderScore(score: Score, triggerVotes: string[]) {
+ if (score.label && triggerVotes.includes(score.label)) {
+ const labels = this.change?.labels ?? {};
+ return html`<gr-trigger-vote
+ .label="${score.label}"
+ .labelInfo="${labels[score.label]}"
+ .change="${this.change}"
+ .mutable="${false}"
+ disable-hovercards
+ >
+ </gr-trigger-vote>`;
+ }
return html`<span
class="score ${this._computeScoreClass(score, this.labelExtremes)}"
>
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
index 628af83..70e6381 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
@@ -190,6 +190,7 @@
<gr-message-scores
label-extremes="[[labelExtremes]]"
message="[[message]]"
+ change="[[change]]"
></gr-message-scores>
</div>
<template is="dom-if" if="[[_commentCountText]]">
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
index 8fa2fa4..c5e8b8c 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
@@ -356,6 +356,7 @@
.change="${this.change}"
.account="${this.account}"
.mutable="${this.mutable ?? false}"
+ .disableHovercards=${this.disableHovercards}
></gr-trigger-vote>`
)}
</section>`;
diff --git a/polygerrit-ui/app/elements/change/gr-trigger-vote/gr-trigger-vote.ts b/polygerrit-ui/app/elements/change/gr-trigger-vote/gr-trigger-vote.ts
index d920b46..a00de22 100644
--- a/polygerrit-ui/app/elements/change/gr-trigger-vote/gr-trigger-vote.ts
+++ b/polygerrit-ui/app/elements/change/gr-trigger-vote/gr-trigger-vote.ts
@@ -48,6 +48,9 @@
@property({type: Boolean})
mutable?: boolean;
+ @property({type: Boolean, attribute: 'disable-hovercards'})
+ disableHovercards = false;
+
static override get styles() {
return css`
:host {
@@ -84,26 +87,31 @@
if (!this.labelInfo) return;
return html`
<div class="container">
- <gr-trigger-vote-hovercard
- .labelName=${this.label}
- .labelInfo=${this.labelInfo}
- >
- <gr-label-info
- slot="label-info"
- .change=${this.change}
- .account=${this.account}
- .mutable=${this.mutable}
- .label=${this.label}
- .labelInfo=${this.labelInfo}
- .showAllReviewers=${false}
- ></gr-label-info>
- </gr-trigger-vote-hovercard>
+ ${this.renderHovercard()}
<span class="label">${this.label}</span>
${this.renderVotes()}
</div>
`;
}
+ private renderHovercard() {
+ if (this.disableHovercards) return;
+ return html`<gr-trigger-vote-hovercard
+ .labelName=${this.label}
+ .labelInfo=${this.labelInfo}
+ >
+ <gr-label-info
+ slot="label-info"
+ .change=${this.change}
+ .account=${this.account}
+ .mutable=${this.mutable}
+ .label=${this.label}
+ .labelInfo=${this.labelInfo}
+ .showAllReviewers=${false}
+ ></gr-label-info>
+ </gr-trigger-vote-hovercard>`;
+ }
+
private renderVotes() {
const {labelInfo} = this;
if (!labelInfo) return;
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 64771f5..2680a31 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -265,7 +265,7 @@
// custom element having the id "app", but it is made explicit here.
// If you move this code to other place, please update comment about
// gr-router and gr-app in the PolyGerritIndexHtml.soy file if needed
-const app = document.querySelector('#app');
+const app = document.querySelector('gr-app');
if (!app) {
console.info('No gr-app found (running tests)');
}
diff --git a/polygerrit-ui/app/elements/settings/gr-agreements-list/gr-agreements-list.ts b/polygerrit-ui/app/elements/settings/gr-agreements-list/gr-agreements-list.ts
index 43aefdc..2db7c76 100644
--- a/polygerrit-ui/app/elements/settings/gr-agreements-list/gr-agreements-list.ts
+++ b/polygerrit-ui/app/elements/settings/gr-agreements-list/gr-agreements-list.ts
@@ -62,7 +62,7 @@
return html`
<tr>
<td class="nameColumn">
- <a href="${this.getUrlBase(agreement.url)}" rel="external">
+ <a href="${this.getUrlBase(agreement?.url)}" rel="external">
${agreement.name}
</a>
</td>
@@ -94,8 +94,8 @@
return `${getBaseUrl()}/settings/new-agreement`;
}
- getUrlBase(item: string) {
- return `${getBaseUrl()}/${item}`;
+ getUrlBase(item?: string) {
+ return item ? `${getBaseUrl()}/${item}` : '';
}
}
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
index 92d984d..677e0c1 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
@@ -196,4 +196,10 @@
? 'hideAgreementsTextBox'
: '';
}
+
+ _computeAgreements(serverConfig?: ServerInfo) {
+ return (serverConfig?.auth.contributor_agreements ?? []).filter(
+ agreement => agreement.url
+ );
+ }
}
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_html.ts b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_html.ts
index ce95ccb..564297fa 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_html.ts
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_html.ts
@@ -66,10 +66,7 @@
<main>
<h1 class="heading-1">New Contributor Agreement</h1>
<h3 class="heading-3">Select an agreement type:</h3>
- <template
- is="dom-repeat"
- items="[[_serverConfig.auth.contributor_agreements]]"
- >
+ <template is="dom-repeat" items="[[_computeAgreements(_serverConfig)]]">
<span class="contributorAgreementButton">
<input
id$="claNewAgreementsInput[[item.name]]"
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index 417d7fb..c6f44af 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -121,6 +121,7 @@
border-left-width: var(--spacing-s);
margin: var(--spacing-m) 0;
padding: var(--spacing-s) var(--spacing-m);
+ overflow-x: scroll;
}
li {
list-style-type: disc;
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
index 9adb8ae..c283876 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
@@ -61,14 +61,6 @@
white-space: var(--linked-text-white-space, pre-wrap);
word-wrap: var(--linked-text-word-wrap, break-word);
}
- :host([disabled]) ::slotted(a) {
- color: inherit;
- text-decoration: none;
- pointer-events: none;
- }
- ::slotted(a) {
- color: var(--link-color);
- }
`;
}
@@ -97,6 +89,8 @@
override updated(changedProperties: PropertyValues): void {
if (changedProperties.has('content') || changedProperties.has('config')) {
this._contentOrConfigChanged();
+ } else if (changedProperties.has('disabled')) {
+ this.styleLinks();
}
}
@@ -131,7 +125,7 @@
// Ensure links to the same host originating from commentlink configs
// open in the same tab. When target is not set - default is _self
// @see Issue 4616
- this.outputElement!.querySelectorAll('a').forEach(anchor => {
+ this.outputElement.querySelectorAll('a').forEach(anchor => {
if (anchor.hostname === window.location.hostname) {
anchor.removeAttribute('target');
} else {
@@ -139,6 +133,30 @@
}
anchor.setAttribute('rel', 'noopener');
});
+
+ this.styleLinks();
+ }
+
+ /**
+ * Styles the links based on whether gr-linked-text is disabled or not
+ */
+ private styleLinks() {
+ assertIsDefined(this.outputElement);
+ this.outputElement.querySelectorAll('a').forEach(anchor => {
+ anchor.setAttribute('style', this.computeLinkStyle());
+ });
+ }
+
+ private computeLinkStyle() {
+ if (this.disabled) {
+ return `
+ color: inherit;
+ text-decoration: none;
+ pointer-events: none;
+ `;
+ } else {
+ return 'color: var(--link-color)';
+ }
}
/**
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index dd056c8..515dab8 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -29,6 +29,7 @@
NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui',
CHECKS_DEVELOPER = 'UiFeature__checks_developer',
SUBMIT_REQUIREMENTS_UI = 'UiFeature__submit_requirements_ui',
+ BULK_ACTIONS = 'UiFeature__bulk_actions_dashboard',
CHECK_RESULTS_IN_DIFFS = 'UiFeature__check_results_in_diffs',
DIFF_RENDERING_LIT = 'UiFeature__diff_rendering_lit',
}
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 7c922f6..d9c102b 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -177,6 +177,8 @@
--not-working-hours-icon-background-color: var(--purple-50);
--not-working-hours-icon-color: var(--purple-700);
--unavailability-icon-color: var(--gray-700);
+ --unavailability-chip-icon-color: var(--orange-900);
+ --unavailability-chip-background-color: var(--yellow-50);
/* text colors */
--primary-text-color: var(--gray-900);
