Merge branch 'stable-3.7' into stable-3.8 * stable-3.7: Fix misleading log message when adding reviewers Change-Id: Ie94419ca4e2b3a49a5b88a55b4be020bd0841a61
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java index 9a8edb6..16c720a 100644 --- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java +++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
@@ -90,6 +90,7 @@ private final AutoAssignConfig cfg; private final PathOwnersEntriesCache cache; + private final PluginSettings pluginSettings; @Inject public GitRefListener( @@ -103,7 +104,8 @@ Provider<CurrentUser> currentUserProvider, ChangeNotes.Factory notesFactory, AutoAssignConfig cfg, - PathOwnersEntriesCache cache) { + PathOwnersEntriesCache cache, + PluginSettings pluginSettings) { this.api = api; this.patchListCache = patchListCache; this.projectCache = projectCache; @@ -115,6 +117,7 @@ this.notesFactory = notesFactory; this.cfg = cfg; this.cache = cache; + this.pluginSettings = pluginSettings; } @Override @@ -241,7 +244,8 @@ patchList, cfg.expandGroups(), projectNameKey.get(), - cache); + cache, + pluginSettings.globalLabel()); Set<Account.Id> allReviewers = Sets.newHashSet(); allReviewers.addAll(owners.get().values()); allReviewers.addAll(owners.getReviewers().values());
diff --git a/owners-autoassign/src/main/resources/Documentation/config.md b/owners-autoassign/src/main/resources/Documentation/config.md index a99e3b1..1caf92f 100644 --- a/owners-autoassign/src/main/resources/Documentation/config.md +++ b/owners-autoassign/src/main/resources/Documentation/config.md
@@ -26,10 +26,19 @@ ## Project configuration -The project configuration `autoAssignWip` controls the automatic -assignment of reviewers based on the OWNERS file on WIP changes. +There are 2 project specific configurations: `autoAssignWip` and +`autoAssignField`. `autoAssignWip` controls the automatic assignment of +reviewers based on the OWNERS file on WIP changes, while `autoAssignField` +controls which field these OWNERS will be assigned too. It can be one of +`REVIEWER` or `CC`. e.g.: -The setting can be inherited from the parent project by setting the value +``` +[plugin "owners-autoassign"] + autoAssignField = CC + autoAssignWip = TRUE +``` + +Both settings can be inherited from the parent project by setting the value to `INHERIT`. By default, all changes are subject to auto-assignment, unless the project
diff --git a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/GitRefListenerTest.java b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/GitRefListenerTest.java index b651572..846f583 100644 --- a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/GitRefListenerTest.java +++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/GitRefListenerTest.java
@@ -47,7 +47,8 @@ Provider<CurrentUser> currentUserProvider, ChangeNotes.Factory notesFactory, AutoAssignConfig cfg, - PathOwnersEntriesCache cache) { + PathOwnersEntriesCache cache, + PluginSettings pluginSettings) { super( api, patchListCache, @@ -59,7 +60,8 @@ currentUserProvider, notesFactory, cfg, - cache); + cache, + pluginSettings); } @Override
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java index e7e437a..1e5023b 100644 --- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java +++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
@@ -103,7 +103,8 @@ Map<String, FileDiffOutput> fileDiffMap, boolean expandGroups, String project, - PathOwnersEntriesCache cache) + PathOwnersEntriesCache cache, + Optional<LabelDefinition> globalLabel) throws InvalidOwnersFileException { this( accounts, @@ -114,7 +115,8 @@ getModifiedPaths(fileDiffMap), expandGroups, project, - cache); + cache, + globalLabel); } public PathOwners( @@ -126,7 +128,8 @@ DiffSummary diffSummary, boolean expandGroups, String project, - PathOwnersEntriesCache cache) + PathOwnersEntriesCache cache, + Optional<LabelDefinition> globalLabel) throws InvalidOwnersFileException { this( accounts, @@ -137,7 +140,8 @@ ImmutableSet.copyOf(diffSummary.getPaths()), expandGroups, project, - cache); + cache, + globalLabel); } public PathOwners( @@ -149,7 +153,8 @@ Set<String> modifiedPaths, boolean expandGroups, String project, - PathOwnersEntriesCache cache) + PathOwnersEntriesCache cache, + Optional<LabelDefinition> globalLabel) throws InvalidOwnersFileException { this.repositoryManager = repositoryManager; this.repository = repository; @@ -170,7 +175,7 @@ matchers = map.getMatchers(); fileOwners = map.getFileOwners(); fileGroupOwners = map.getFileGroupOwners(); - label = map.getLabel(); + label = globalLabel.or(map::getLabel); } /** * Returns a read only view of the paths to owners mapping.
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java index 3af7a58..c882414 100644 --- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java +++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java
@@ -20,6 +20,7 @@ import com.google.common.cache.LoadingCache; import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimap; +import com.google.gerrit.entities.RefNames; import com.google.inject.Inject; import com.google.inject.Singleton; import com.google.inject.name.Named; @@ -88,6 +89,6 @@ } private String indexKey(String project, String branch) { - return new StringBuilder(project).append('@').append(branch).toString(); + return new StringBuilder(project).append('@').append(RefNames.fullName(branch)).toString(); } }
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PluginSettings.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PluginSettings.java index 26b0f62..1fd7403 100644 --- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PluginSettings.java +++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PluginSettings.java
@@ -15,6 +15,7 @@ package com.googlesource.gerrit.owners.common; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableSet; import com.google.gerrit.entities.Project; import com.google.gerrit.extensions.annotations.PluginName; @@ -23,6 +24,8 @@ import com.google.gerrit.server.project.NoSuchProjectException; import com.google.inject.Inject; import com.google.inject.Singleton; +import java.util.Optional; +import java.util.function.Supplier; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; @@ -36,6 +39,8 @@ private final boolean expandGroups; private final boolean enableSubmitRequirement; + private final Supplier<Optional<LabelDefinition>> globalLabel; + @Inject public PluginSettings(PluginConfigFactory configFactory, @PluginName String ownersPluginName) { this.configFactory = configFactory; @@ -48,6 +53,10 @@ this.expandGroups = globalPluginConfig.getBoolean("owners", "expandGroups", true); this.enableSubmitRequirement = globalPluginConfig.getBoolean("owners", "enableSubmitRequirement", false); + + this.globalLabel = + Suppliers.memoize( + () -> LabelDefinition.parse(globalPluginConfig.getString("owners", null, "label"))); } /** @@ -95,6 +104,19 @@ return configFactory.getFromProjectConfigWithInheritance(projectKey, ownersPluginName); } + /** + * Global definition of the review label to use for the owners' plugin. + * + * <p>When the global label is set in the plugin.owners.label settings the value overrides any + * label name defined in the OWNERS files of any repository. + * + * @return the global label definition or {@link Optional#empty()} if there isn't any global label + * definition. + */ + public Optional<LabelDefinition> globalLabel() { + return globalLabel.get(); + } + // Logic copied from JGit's TestRepository private static String normalizeRef(String ref) { if (Constants.HEAD.equals(ref)) {
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java index 542ff6d..7ab294f 100644 --- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java +++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java
@@ -51,7 +51,11 @@ private static final boolean EXPAND_GROUPS = true; private static final boolean DO_NOT_EXPAND_GROUPS = false; private static final String EXPECTED_LABEL = "expected-label"; + private static final Short EXPECTED_LABEL_SCORE = 1; + private static final LabelDefinition EXPECTED_LABEL_DEFINITION = + new LabelDefinition(EXPECTED_LABEL, EXPECTED_LABEL_SCORE); private static final String A_LABEL = "a-label"; + private static final String B_LABEL = "b-label"; private static PathOwnersEntriesCache CACHE_MOCK = new PathOwnersEntriesCacheMock(); public static final String CLASSIC_FILE_TXT = "classic/file.txt"; @@ -80,12 +84,33 @@ Set.of(CLASSIC_FILE_TXT), EXPAND_GROUPS, "foo", - CACHE_MOCK); + CACHE_MOCK, + Optional.empty()); Set<Account.Id> ownersSet = owners.get().get(CLASSIC_OWNERS); assertEquals(2, ownersSet.size()); assertTrue(ownersSet.contains(USER_A_ID)); assertTrue(ownersSet.contains(USER_B_ID)); assertTrue(owners.expandGroups()); + assertThat(owners.getLabel()).isEmpty(); + } + + @Test + public void testGlobalLabel() throws Exception { + mockOwners(USER_A_EMAIL_COM, USER_B_EMAIL_COM); + + PathOwners owners = + new PathOwners( + accounts, + repositoryManager, + repository, + emptyList(), + branch, + Set.of(CLASSIC_FILE_TXT), + EXPAND_GROUPS, + "foo", + CACHE_MOCK, + Optional.of(EXPECTED_LABEL_DEFINITION)); + assertThat(owners.getLabel()).hasValue(EXPECTED_LABEL_DEFINITION); } @Test @@ -102,7 +127,8 @@ Set.of(CLASSIC_FILE_TXT), DO_NOT_EXPAND_GROUPS, "foo", - CACHE_MOCK); + CACHE_MOCK, + Optional.empty()); Set<String> ownersSet = owners.getFileGroupOwners().get(CLASSIC_FILE_TXT); assertEquals(2, ownersSet.size()); assertTrue(ownersSet.contains(USER_A)); @@ -124,7 +150,8 @@ Set.of(CLASSIC_FILE_TXT), EXPAND_GROUPS, "foo", - CACHE_MOCK); + CACHE_MOCK, + Optional.empty()); Set<Account.Id> ownersSet = owners.get().get(CLASSIC_OWNERS); assertEquals(0, ownersSet.size()); } @@ -149,7 +176,8 @@ Set.of("classic/file.txt"), EXPAND_GROUPS, "foo", - CACHE_MOCK); + CACHE_MOCK, + Optional.empty()); Set<Account.Id> ownersSet2 = owners2.get().get(CLASSIC_OWNERS); // in this case we are inheriting the acct3 from /OWNERS @@ -164,6 +192,50 @@ } @Test + public void testClassicWithInheritanceAndGlobalLabel() throws Exception { + expectConfig("OWNERS", createConfig(true, Optional.of(A_LABEL), owners())); + expectConfig(CLASSIC_OWNERS, createConfig(true, Optional.of(B_LABEL), owners())); + + replayAll(); + + PathOwners owners2 = + new PathOwners( + accounts, + repositoryManager, + repository, + emptyList(), + branch, + Set.of("classic/file.txt"), + EXPAND_GROUPS, + "foo", + CACHE_MOCK, + Optional.of(EXPECTED_LABEL_DEFINITION)); + assertThat(owners2.getLabel()).hasValue(EXPECTED_LABEL_DEFINITION); + } + + @Test + public void testClassicWithoutInheritanceAndGlobalLabel() throws Exception { + expectConfig("OWNERS", createConfig(false, Optional.of(A_LABEL), owners())); + expectConfig(CLASSIC_OWNERS, createConfig(false, Optional.of(B_LABEL), owners())); + + replayAll(); + + PathOwners owners2 = + new PathOwners( + accounts, + repositoryManager, + repository, + emptyList(), + branch, + Set.of("classic/file.txt"), + EXPAND_GROUPS, + "foo", + CACHE_MOCK, + Optional.of(EXPECTED_LABEL_DEFINITION)); + assertThat(owners2.getLabel()).hasValue(EXPECTED_LABEL_DEFINITION); + } + + @Test public void testRootInheritFromProject() throws Exception { expectConfig("OWNERS", "master", createConfig(true, owners())); expectConfig( @@ -188,7 +260,8 @@ Set.of(fileName), EXPAND_GROUPS, "foo", - CACHE_MOCK); + CACHE_MOCK, + Optional.empty()); Map<String, Set<Account.Id>> fileOwners = owners.getFileOwners(); assertEquals(1, fileOwners.size()); @@ -233,7 +306,8 @@ Set.of(fileName), EXPAND_GROUPS, "foo", - CACHE_MOCK); + CACHE_MOCK, + Optional.empty()); Map<String, Set<Account.Id>> fileOwners = owners.getFileOwners(); assertEquals(fileOwners.size(), 1); @@ -282,7 +356,8 @@ Set.of(sqlFileName, javaFileName), EXPAND_GROUPS, "foo", - CACHE_MOCK); + CACHE_MOCK, + Optional.empty()); Map<String, Set<Account.Id>> fileOwners = owners.getFileOwners(); assertEquals(fileOwners.size(), 2); @@ -326,7 +401,8 @@ Set.of("dir/subdir/file.txt"), EXPAND_GROUPS, "foo", - CACHE_MOCK); + CACHE_MOCK, + Optional.empty()); Set<Account.Id> ownersSet = owners.get().get("dir/subdir/OWNERS"); assertEquals(3, ownersSet.size()); @@ -406,7 +482,8 @@ Set.of("dir/subdir/file.txt"), EXPAND_GROUPS, "foo", - cacheMock); + cacheMock, + Optional.empty()); assertThat(owners.getFileOwners()).isNotEmpty(); int expectedCacheCalls =
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PluginSettingsTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PluginSettingsTest.java index c4c75d4..2aecd88 100644 --- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PluginSettingsTest.java +++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PluginSettingsTest.java
@@ -18,6 +18,7 @@ import static org.mockito.Mockito.when; import com.google.gerrit.server.config.PluginConfigFactory; +import java.util.Optional; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; import org.junit.Test; @@ -81,4 +82,21 @@ assertThat(pluginSettings.isBranchDisabled(branchName1)).isTrue(); assertThat(pluginSettings.isBranchDisabled(branchName2)).isTrue(); } + + @Test + public void globalLabelIsEmptyByDefault() { + setupMocks(new Config()); + + assertThat(pluginSettings.globalLabel()).isEqualTo(Optional.empty()); + } + + @Test + public void globalLabelSetByConfig() { + LabelDefinition globalLabelName = new LabelDefinition("Custom-Label", (short) 2); + Config pluginConfig = new Config(); + pluginConfig.setString("owners", null, "label", "Custom-Label,2"); + setupMocks(pluginConfig); + + assertThat(pluginSettings.globalLabel()).isEqualTo(Optional.of(globalLabelName)); + } }
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java index a035d93..821db6a 100644 --- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java +++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java
@@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import org.junit.Before; @@ -156,7 +157,8 @@ "project/file.sql"), // only .sql matching b,c EXPAND_GROUPS, "foo", - new PathOwnersEntriesCacheMock()); + new PathOwnersEntriesCacheMock(), + Optional.empty()); // assertions on classic owners Set<Account.Id> ownersSet = owners.get().get("project/OWNERS"); @@ -263,7 +265,8 @@ Set.of("project/file.sql", "another.txt"), EXPAND_GROUPS, "foo", - new PathOwnersEntriesCacheMock()); + new PathOwnersEntriesCacheMock(), + Optional.empty()); Set<String> ownedFiles = owners.getFileOwners().keySet(); assertThat(ownedFiles).containsExactly("project/file.sql");
diff --git a/owners/BUILD b/owners/BUILD index f08ec84..c1dac46 100644 --- a/owners/BUILD +++ b/owners/BUILD
@@ -38,7 +38,9 @@ "Implementation-URL: https://gerrit.googlesource.com/plugins/owners", "Gerrit-PluginName: owners", "Gerrit-Module: com.googlesource.gerrit.owners.OwnersModule", + "Gerrit-HttpModule: com.googlesource.gerrit.owners.HttpModule", ], + resource_jars = ["//plugins/owners/web:gr-owners"], resources = glob(["src/main/resources/**/*"]), deps = [ ":gerrit-owners-predicates",
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/HttpModule.java b/owners/src/main/java/com/googlesource/gerrit/owners/HttpModule.java new file mode 100644 index 0000000..10759c0 --- /dev/null +++ b/owners/src/main/java/com/googlesource/gerrit/owners/HttpModule.java
@@ -0,0 +1,26 @@ +// Copyright (C) 2024 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.googlesource.gerrit.owners; + +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.extensions.webui.JavaScriptPlugin; +import com.google.gerrit.extensions.webui.WebUiPlugin; +import com.google.inject.servlet.ServletModule; + +public class HttpModule extends ServletModule { + @Override + protected void configureServlets() { + DynamicSet.bind(binder(), WebUiPlugin.class).toInstance(new JavaScriptPlugin("gr-owners.js")); + } +}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java index c077bb3..ff8bc18 100644 --- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java +++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
@@ -73,7 +73,8 @@ patchList, settings.expandGroups(), projectState.getName(), - cache); + cache, + settings.globalLabel()); } catch (InvalidOwnersFileException e) { // re-throw exception as it is already logged but more importantly it is nicely // handled by the prolog rules evaluator and results in prolog rule error
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java index 0d8dac4..64638bc 100644 --- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java +++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
@@ -216,7 +216,8 @@ getDiff(nameKey, cd.currentPatchSet().commitId()), pluginSettings.expandGroups(), nameKey.get(), - cache); + cache, + pluginSettings.globalLabel()); return pathOwners; }
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java b/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java index 30c9c73..3f6c937 100644 --- a/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java +++ b/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java
@@ -24,11 +24,15 @@ public final Map<String, Set<GroupOwner>> files; public final Map<Integer, Map<String, Integer>> ownersLabels; + public final Map<String, Set<GroupOwner>> filesApproved; public FilesOwnersResponse( - Map<Integer, Map<String, Integer>> ownersLabels, Map<String, Set<GroupOwner>> files) { + Map<Integer, Map<String, Integer>> ownersLabels, + Map<String, Set<GroupOwner>> files, + Map<String, Set<GroupOwner>> filesApproved) { this.ownersLabels = ownersLabels; this.files = files; + this.filesApproved = filesApproved; } @Override @@ -36,16 +40,25 @@ if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; FilesOwnersResponse that = (FilesOwnersResponse) o; - return Objects.equal(files, that.files) && Objects.equal(ownersLabels, that.ownersLabels); + return Objects.equal(files, that.files) + && Objects.equal(ownersLabels, that.ownersLabels) + && Objects.equal(filesApproved, that.filesApproved); } @Override public int hashCode() { - return Objects.hashCode(files, ownersLabels); + return Objects.hashCode(files, ownersLabels, filesApproved); } @Override public String toString() { - return "FilesOwnersResponse{" + "files=" + files + ", ownersLabels=" + ownersLabels + '}'; + return "FilesOwnersResponse{" + + "files=" + + files + + ", ownersLabels=" + + ownersLabels + + ", filesApproved=" + + filesApproved + + '}'; } }
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java index a724239..005e83f 100644 --- a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java +++ b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
@@ -114,7 +114,8 @@ changePaths, pluginSettings.expandGroups(), project.get(), - cache); + cache, + pluginSettings.globalLabel()); Map<String, Set<GroupOwner>> fileExpandedOwners = Maps.transformValues( @@ -144,7 +145,14 @@ !isApprovedByOwner( fileExpandedOwners.get(fileOwnerEntry.getKey()), ownersLabels, label)); - return Response.ok(new FilesOwnersResponse(ownersLabels, filesWithPendingOwners)); + Map<String, Set<GroupOwner>> filesApprovedByOwners = + Maps.filterEntries( + fileToOwners, + (fileOwnerEntry) -> + isApprovedByOwner( + fileExpandedOwners.get(fileOwnerEntry.getKey()), ownersLabels, label)); + + return Response.ok(new FilesOwnersResponse(ownersLabels, filesWithPendingOwners, filesApprovedByOwners)); } catch (InvalidOwnersFileException e) { logger.atSevere().withCause(e).log("Reading/parsing OWNERS file error."); throw new ResourceConflictException(e.getMessage(), e);
diff --git a/owners/src/main/resources/Documentation/config.md b/owners/src/main/resources/Documentation/config.md index 1d157c6..811325b 100644 --- a/owners/src/main/resources/Documentation/config.md +++ b/owners/src/main/resources/Documentation/config.md
@@ -28,6 +28,18 @@ expandGroups = false ``` +owners.label +: Global override for the label and score, separated by a comma, to use by + the owners of changes for approving them. When defined, it overrides any + other label definition set by the OWNERS at any level in any project. + + Example: + + ``` + [owners] + label = Code-Review, 1 + ``` + <a name="owners.enableSubmitRequirement">owners.enableSubmitRequirement</a> : If set to `true` the approvals are evaluated through the owners plugin provided submit rule without a need of prolog predicate being added to a
diff --git a/owners/src/main/resources/Documentation/rest-api.md b/owners/src/main/resources/Documentation/rest-api.md index 68fb463..3505d6c 100644 --- a/owners/src/main/resources/Documentation/rest-api.md +++ b/owners/src/main/resources/Documentation/rest-api.md
@@ -1,7 +1,8 @@ # Rest API -The @PLUGIN@ exposes a Rest API endpoint to list the owners associated to each file that -needs a review, and, for each owner, its current labels and votes: +The @PLUGIN@ exposes a Rest API endpoint to list the owners associated with each file that +needs approval (`file` field), is approved (`files_approved`) and, for each owner, +its current labels and votes (`owners_labels`): ```bash GET /changes/{change-id}/revisions/{revision-id}/owners~files-owners @@ -18,6 +19,12 @@ { "name":"Jack", "id": 1000003 } ] }, + "files_approved": { + "NewBuild.build":[ + { "name":"John", "id": 1000004 }, + { "name":"Release Engineer", "id": 1000001 } + ] + }, "owners_labels" : { "1000002": { "Verified": 1,
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java index c39974c..1633783 100644 --- a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java +++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java
@@ -42,6 +42,7 @@ import com.googlesource.gerrit.owners.entities.Owner; import com.googlesource.gerrit.owners.restapi.GetFilesOwners.LabelNotFoundException; import java.util.Arrays; +import java.util.Map; import javax.servlet.http.HttpServletResponse; import org.apache.commons.compress.utils.Sets; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; @@ -102,7 +103,7 @@ } @Test - public void shouldReturnOwnersLabelsWhenNotApprovedByOwners() throws Exception { + public void shouldReturnEmptyOwnersLabelsWhenNotApprovedByOwners() throws Exception { addOwnerFileToRoot(true); String changeId = createChange().getChangeId(); @@ -116,7 +117,21 @@ } @Test - public void shouldReturnEmptyResponseWhenApprovedByOwners() throws Exception { + public void shouldReturnNonEmptyOwnersLabelsWhenApprovedByOwners() throws Exception { + addOwnerFileToRoot(true); + String changeId = createChange().getChangeId(); + approve(changeId); + + Response<FilesOwnersResponse> resp = + assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); + + assertThat(resp.value().ownersLabels) + .containsExactly(admin.id().get(), Map.of(LabelId.CODE_REVIEW, 2)); + } + + @Test + public void shouldReturnEmptyFilesAndNonEmptyFilesApprovedResponseWhenApprovedByOwners() + throws Exception { addOwnerFileToRoot(true); String changeId = createChange().getChangeId(); approve(changeId); @@ -125,6 +140,8 @@ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); assertThat(resp.value().files).isEmpty(); + assertThat(resp.value().filesApproved) + .containsExactly("a.txt", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get()))); } @Test @@ -138,12 +155,14 @@ assertThat(resp.value().files) .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username()))); + assertThat(resp.value().filesApproved).isEmpty(); } @Test @GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false") - public void shouldReturnEmptyResponseWhenApprovedByOwnersWithUnexpandedFileOwners() - throws Exception { + public void + shouldReturnEmptyFilesAndNonEmptyFilesApprovedResponseWhenApprovedByOwnersWithUnexpandedFileOwners() + throws Exception { addOwnerFileToRoot(true); String changeId = createChange().getChangeId(); approve(changeId); @@ -152,6 +171,8 @@ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); assertThat(resp.value().files).isEmpty(); + assertThat(resp.value().filesApproved) + .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username()))); } @Test @@ -165,6 +186,7 @@ assertThat(resp.value().files) .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username()))); + assertThat(resp.value().filesApproved).isEmpty(); } @Test @@ -192,6 +214,7 @@ addOwnerFileToProjectConfig(allProjects, true, user); resp = assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(projectOwner)); + assertThat(resp.value().filesApproved).isEmpty(); } @Test @@ -265,6 +288,7 @@ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(rootOwner)); + assertThat(resp.value().filesApproved).isEmpty(); } private void assertInheritFromProject(Project.NameKey projectNameKey) throws Exception { @@ -277,6 +301,7 @@ assertThat(resp.value().files) .containsExactly("a.txt", Sets.newHashSet(rootOwner, projectOwner)); + assertThat(resp.value().filesApproved).isEmpty(); } private void addBrokenOwnersFileToRoot() throws Exception {
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java index 8172611..f5c80b7 100644 --- a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java +++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java
@@ -72,6 +72,7 @@ assertThat(resp.value().files) .containsExactly("foo", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get()))); assertThat(resp.value().ownersLabels).isEmpty(); + assertThat(resp.value().filesApproved).isEmpty(); // give CR+1 as requested recommend(changeId); @@ -80,6 +81,8 @@ assertThat(resp.value().files).isEmpty(); assertThat(resp.value().ownersLabels) .containsExactly(admin.id().get(), Map.of(LabelId.CODE_REVIEW, 1)); + assertThat(resp.value().filesApproved) + .containsExactly("foo", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get()))); } @Test @@ -96,6 +99,7 @@ assertThat(resp.value().files) .containsExactly("foo", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get()))); assertThat(resp.value().ownersLabels).isEmpty(); + assertThat(resp.value().filesApproved).isEmpty(); // give LabelFoo+1 as requested gApi.changes().id(changeId).current().review(new ReviewInput().label(label, 1)); @@ -103,6 +107,8 @@ resp = assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); assertThat(resp.value().files).isEmpty(); assertThat(resp.value().ownersLabels).containsEntry(admin.id().get(), Map.of(label, 1)); + assertThat(resp.value().filesApproved) + .containsExactly("foo", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get()))); } private void addOwnerFileToRoot(LabelDefinition label, TestAccount u) throws Exception {
diff --git a/owners/web/.eslintrc.js b/owners/web/.eslintrc.js new file mode 100644 index 0000000..776d84e --- /dev/null +++ b/owners/web/.eslintrc.js
@@ -0,0 +1,20 @@ +/** + * @license + * Copyright (C) 2024 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. + */ +__plugindir = 'owners/web'; +module.exports = { + extends: '../../.eslintrc.js', +}; \ No newline at end of file
diff --git a/owners/web/BUILD b/owners/web/BUILD new file mode 100644 index 0000000..56ab697 --- /dev/null +++ b/owners/web/BUILD
@@ -0,0 +1,68 @@ +load("@npm//@bazel/typescript:index.bzl", "ts_config", "ts_project") +load("//tools/bzl:js.bzl", "gerrit_js_bundle", "karma_test") +load("//tools/js:eslint.bzl", "plugin_eslint") + +package_group( + name = "visibility", + packages = ["//plugins/owners/..."], +) + +package(default_visibility = [":visibility"]) + +ts_config( + name = "tsconfig", + src = "tsconfig.json", + deps = [ + "//plugins:tsconfig-plugins-base.json", + ], +) + +ts_project( + name = "owners-ts", + srcs = glob( + ["**/*.ts"], + exclude = [ + "**/*test*", + "**/test-utils.ts", + ], + ), + incremental = True, + out_dir = "_bazel_ts_out", + tsc = "//tools/node_tools:tsc-bin", + tsconfig = ":tsconfig", + deps = [ + "@plugins_npm//@gerritcodereview/typescript-api", + "@plugins_npm//lit", + "@plugins_npm//rxjs", + ], +) + +ts_project( + name = "owners-ts-tests", + srcs = glob([ + "**/*.ts", + "**/test-utils.ts", + ]), + incremental = True, + out_dir = "_bazel_ts_out_tests", + tsc = "//tools/node_tools:tsc-bin", + tsconfig = ":tsconfig", + deps = [ + "@plugins_npm//:node_modules", + "@ui_dev_npm//:node_modules", + ], +) + +gerrit_js_bundle( + name = "gr-owners", + srcs = [":owners-ts"], + entry_point = "_bazel_ts_out/plugin.js", +) + +karma_test( + name = "karma_test", + srcs = ["karma_test.sh"], + data = [":owners-ts-tests"], +) + +plugin_eslint()
diff --git a/owners/web/gerrit-model.ts b/owners/web/gerrit-model.ts new file mode 100644 index 0000000..da0d42c --- /dev/null +++ b/owners/web/gerrit-model.ts
@@ -0,0 +1,54 @@ +/** + * @license + * Copyright (C) 2024 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 {AccountInfo} from '@gerritcodereview/typescript-api/rest-api'; + +/** + * A provider for a value. Copied from Gerrit core. + */ +export type Provider<T> = () => T; + +/** + * Partial Gerrit's `AccountsModel` interface that defines functions needed in the owners plugin + */ +export interface AccountsModel { + fillDetails(account: AccountInfo): AccountInfo; +} + +/** + * Parital Gerrit's `GrAccountLabel` interface that provides access to the `AccountsModel` + */ +export interface GrAccountLabel extends Element { + getAccountsModel: Provider<AccountsModel>; +} + +/** + * Partial Gerrit's `SpecialFilePath` enum + */ +export enum SpecialFilePath { + COMMIT_MESSAGE = '/COMMIT_MSG', + MERGE_LIST = '/MERGE_LIST', +} + +/** + * Partial Gerrit's `Window` interface defintion so that `getBaseUrl` function can work. + */ +declare global { + interface Window { + CANONICAL_PATH?: string; + } +}
diff --git a/owners/web/gr-files.ts b/owners/web/gr-files.ts new file mode 100644 index 0000000..d0fe189 --- /dev/null +++ b/owners/web/gr-files.ts
@@ -0,0 +1,569 @@ +/** + * @license + * Copyright (C) 2024 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 {css, html, LitElement, nothing, PropertyValues, CSSResult} from 'lit'; +import {customElement, property} from 'lit/decorators'; +import { + AccountInfo, + ApprovalInfo, + ChangeInfo, + ChangeStatus, + GroupInfo, + LabelInfo, + isDetailedLabelInfo, + EmailAddress, + ReviewerState, +} from '@gerritcodereview/typescript-api/rest-api'; +import { + FilesOwners, + isOwner, + OwnersLabels, + hasOwnersSubmitRequirement, +} from './owners-service'; +import { + FileOwnership, + FileStatus, + OwnerOrGroupOwner, + PatchRange, + UserRole, +} from './owners-model'; +import {query} from './utils'; +import {GrAccountLabel} from './gerrit-model'; +import {OwnersMixin} from './owners-mixin'; + +const STATUS_CODE = { + MISSING: 'missing', + APPROVED: 'approved', +}; + +const STATUS_ICON = { + [STATUS_CODE.MISSING]: 'schedule', + [STATUS_CODE.APPROVED]: 'check', +}; + +const FILE_STATUS = { + [FileStatus.NEEDS_APPROVAL]: STATUS_CODE.MISSING, + [FileStatus.APPROVED]: STATUS_CODE.APPROVED, +}; + +const HOVER_HEADING = { + [STATUS_CODE.MISSING]: "Needs Owners' Approval", + [STATUS_CODE.APPROVED]: 'Approved by Owners', +}; + +const DISPLAY_OWNERS_FOR_FILE_LIMIT = 5; +const LIMITED_FILES_OWNERS_TOOLTIP = `File owners limited to first ${DISPLAY_OWNERS_FOR_FILE_LIMIT} accounts.`; + +const common = OwnersMixin(LitElement); + +class FilesCommon extends common { + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + + this.hidden = shouldHide( + this.change, + this.patchRange, + this.filesOwners, + this.user?.role + ); + } + + protected static commonStyles(): CSSResult[] { + return [window?.Gerrit?.styles.font as CSSResult]; + } +} + +export const FILES_OWNERS_COLUMN_HEADER = 'files-owners-column-header'; +@customElement(FILES_OWNERS_COLUMN_HEADER) +export class FilesColumnHeader extends FilesCommon { + static override get styles() { + return [ + ...FilesCommon.commonStyles(), + css` + :host() { + display: block; + padding-right: var(--spacing-m); + width: 4em; + } + :host[hidden] { + display: none; + } + `, + ]; + } + + override render() { + if (this.hidden) return nothing; + return html`<div>Status</div>`; + } +} + +/** + * It has to be part of this file as components defined in dedicated files are not visible + */ +@customElement('gr-owner') +export class GrOwner extends LitElement { + @property({type: Object}) + owner?: AccountInfo; + + @property({type: Object}) + groupOwner?: GroupInfo; + + @property({type: Object}) + approval?: ApprovalInfo; + + @property({type: Object}) + info?: LabelInfo; + + @property({type: String}) + email?: EmailAddress; + + static override get styles() { + return [ + css` + .container { + display: flex; + } + gr-vote-chip { + margin-left: 5px; + --gr-vote-chip-width: 14px; + --gr-vote-chip-height: 14px; + } + gr-tooltip-content { + display: inline-block; + } + `, + ]; + } + + override render() { + if (!this.owner && !this.groupOwner) { + return nothing; + } + + const isAccountOwner = this.owner !== undefined; + const ownerLabel = isAccountOwner + ? this.owner?._account_id + ? html`<gr-account-label .account=${this.owner}></gr-account-label>` + : html`<span>${this.owner?.display_name}</span>` + : html`<span>Group: ${this.groupOwner?.name}</span>`; + + const voteChip = + isAccountOwner && this.approval + ? html` <gr-vote-chip + .vote=${this.approval} + .label=${this.info} + ></gr-vote-chip>` + : nothing; + + // allow user to copy what is available: + // * email or name (if available) for AccountInfo owner type + // * group name for GroupInfo owner type + const [copyText, copyTooltip] = isAccountOwner + ? this.email || this.owner?.display_name + ? [ + this.email ?? this.owner?.display_name, + this.email ? 'email' : 'name', + ] + : [nothing, nothing] + : [this.groupOwner?.name, 'group name']; + const copy = copyText + ? html` <gr-copy-clipboard + .text=${copyText} + hasTooltip + hideinput + buttonTitle="Copy owner's ${copyTooltip} to clipboard" + ></gr-copy-clipboard>` + : nothing; + + return html` + <div class="container">${ownerLabel} ${voteChip} ${copy}</div> + `; + } + + override async updated(changedProperties: PropertyValues) { + super.updated(changedProperties); + + if (changedProperties.has('owner')) { + if (this.owner && !this.email) { + const accountLabel = query<GrAccountLabel>(this, 'gr-account-label'); + if (!accountLabel) { + return; + } + + const updateOwner = await accountLabel + ?.getAccountsModel() + ?.fillDetails(this.owner); + this.email = updateOwner?.email; + } + } + } +} + +export const FILES_OWNERS_COLUMN_CONTENT = 'files-owners-column-content'; +@customElement(FILES_OWNERS_COLUMN_CONTENT) +export class FilesColumnContent extends FilesCommon { + @property({type: String}) + path?: string; + + @property({type: String}) + oldPath?: string; + + @property({type: String, reflect: true, attribute: 'file-status'}) + fileStatus?: string; + + private owners?: OwnerOrGroupOwner[]; + + // taken from Gerrit's common-util.ts + private uniqueId = Math.random().toString(36).substring(2); + + static override get styles() { + return [ + ...FilesCommon.commonStyles(), + css` + :host { + display: flex; + padding-right: var(--spacing-m); + width: 4em; + text-align: center; + } + :host[hidden] { + display: none; + } + gr-icon { + padding: var(--spacing-xs) 0px; + margin-left: 9px; + } + :host([file-status='approved']) gr-icon.status { + color: var(--positive-green-text-color); + } + :host([file-status='missing']) gr-icon.status { + color: #ffa62f; + } + `, + ]; + } + + override render() { + if (this.hidden || !this.fileStatus) { + return nothing; + } + + const icon = STATUS_ICON[this.fileStatus]; + return html` + <gr-icon + id="${this.pathId()}" + class="status" + icon=${icon} + aria-hidden="true" + ></gr-icon> + ${this.renderFileOwners()} + `; + } + + private pathId(): string { + return `path-${this.uniqueId}`; + } + + private renderFileOwners() { + const owners = this.owners ?? []; + const splicedOwners = owners.splice(0, DISPLAY_OWNERS_FOR_FILE_LIMIT); + const showEllipsis = owners.length > DISPLAY_OWNERS_FOR_FILE_LIMIT; + const heading = HOVER_HEADING[this.fileStatus ?? STATUS_CODE.MISSING]; + // inlining <style> here is ugly but an alternative would be to copy the `HovercardMixin` from Gerrit and implement hoover from scratch + return html`<gr-hovercard for="${this.pathId()}"> + <style> + #file-owners-hoovercard { + min-width: 256px; + max-width: 256px; + margin: -10px; + padding: var(--spacing-xl) 0 var(--spacing-m) 0; + } + h3 { + font: inherit; + } + .heading-3 { + margin-left: -2px; + font-family: var(--header-font-family); + font-size: var(--font-size-h3); + font-weight: var(--font-weight-h3); + line-height: var(--line-height-h3); + } + .row { + display: flex; + } + div.section { + margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl); + display: flex; + align-items: center; + } + div.sectionIcon { + flex: 0 0 30px; + } + div.sectionIcon gr-icon { + position: relative; + } + div.sectionContent .row { + margin-left: 2px; + } + div.sectionContent .notLast { + margin-bottom: 2px; + } + div.sectionContent .ellipsis { + margin-left: 5px; + } + </style> + <div id="file-owners-hoovercard"> + <div class="section"> + <div class="sectionIcon"> + <gr-icon class="status" icon="info" aria-hidden="true"></gr-icon> + </div> + <div class="sectionContent"> + <h3 class="name heading-3"> + <span>${heading}</span> + </h3> + </div> + </div> + <div class="section"> + <div class="sectionContent"> + ${splicedOwners.map((owner, idx) => { + const [approval, info] = + computeApprovalAndInfo( + owner, + this.filesOwners?.owners_labels ?? {}, + this.change + ) ?? []; + return html` + <div + class="row ${showEllipsis || idx + 1 < splicedOwners.length + ? 'notLast' + : ''}" + > + <gr-owner + .owner=${owner.owner} + .groupOwner=${owner.groupOwner} + .approval=${approval} + .info=${info} + .email=${owner.owner ? owner.owner.email : nothing} + ></gr-owner> + </div> + `; + })} + ${showEllipsis + ? html` + <gr-tooltip-content + title=${LIMITED_FILES_OWNERS_TOOLTIP} + aria-label="limited-file-onwers" + aria-description=${LIMITED_FILES_OWNERS_TOOLTIP} + has-tooltip + > + <div class="row ellipsis">...</div> + </gt-tooltip-content>` + : nothing} + </div> + </div> + </div> + </gr-hovercard>`; + } + + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + this.computeFileState(); + } + + private computeFileState(): void { + const fileOwnership = getFileOwnership(this.path, this.filesOwners); + if (!fileOwnership || fileOwnership.fileStatus === FileStatus.NOT_OWNED) { + this.fileStatus = undefined; + this.owners = undefined; + return; + } + + this.fileStatus = FILE_STATUS[fileOwnership.fileStatus]; + const accounts = getChangeAccounts(this.change); + + this.owners = (fileOwnership.owners ?? []).map(owner => { + if (isOwner(owner)) { + return { + owner: + accounts.byAccountId.get(owner.id) ?? + ({_account_id: owner.id} as unknown as AccountInfo), + } as unknown as OwnerOrGroupOwner; + } + + const groupPrefix = 'group/'; + if (owner.name.startsWith(groupPrefix)) { + return { + groupOwner: { + name: owner.name.substring(groupPrefix.length), + } as unknown as GroupInfo, + } as unknown as OwnerOrGroupOwner; + } + + // when `owners.expandGroups = false` then neither group nor account + // will be expanded therefore try to match account with change's available + // accounts through email without domain or by full name + // finally construct `AccountInfo` just with a `name` property + const accountOwner = + accounts.byEmailWithoutDomain.get(owner.name) ?? + accounts.byFullName.get(owner.name) ?? + ({display_name: owner.name} as unknown as AccountInfo); + return {owner: accountOwner} as unknown as OwnerOrGroupOwner; + }); + } +} + +export function shouldHide( + change?: ChangeInfo, + patchRange?: PatchRange, + filesOwners?: FilesOwners, + userRole?: UserRole +): boolean { + // don't show owners when no change or change is merged + if (change === undefined || patchRange === undefined) { + return true; + } + if ( + change.status === ChangeStatus.ABANDONED || + change.status === ChangeStatus.MERGED + ) { + return true; + } + + // Note: in some special cases, patchNum is undefined on latest patchset + // like after publishing the edit, still show for them + // TODO: this should be fixed in Gerrit + if (patchRange?.patchNum === undefined) return false; + + // only show if its latest patchset + const latestPatchset = change.revisions![change.current_revision!]; + if (`${patchRange.patchNum}` !== `${latestPatchset._number}`) { + return true; + } + + // show owners when they apply to the change and for logged in user + if ( + hasOwnersSubmitRequirement(change) && + filesOwners && + (filesOwners.files || filesOwners.files_approved) + ) { + return !userRole || userRole === UserRole.ANONYMOUS; + } + return true; +} + +export function getFileOwnership( + path?: string, + filesOwners?: FilesOwners +): FileOwnership | undefined { + if (path === undefined || filesOwners === undefined) { + return undefined; + } + + const fileOwners = (filesOwners.files ?? {})[path]; + const fileApprovers = (filesOwners.files_approved ?? {})[path]; + if (fileApprovers) { + return { + fileStatus: FileStatus.APPROVED, + owners: fileApprovers, + }; + } else if (fileOwners) { + return { + fileStatus: FileStatus.NEEDS_APPROVAL, + owners: fileOwners, + }; + } else { + return {fileStatus: FileStatus.NOT_OWNED}; + } +} + +export function computeApprovalAndInfo( + fileOwner: OwnerOrGroupOwner, + labels: OwnersLabels, + change?: ChangeInfo +): [ApprovalInfo, LabelInfo] | undefined { + if (!change?.labels || !fileOwner?.owner) { + return; + } + const accountId = fileOwner.owner._account_id; + const ownersLabel = labels[`${accountId}`]; + if (!ownersLabel) { + return; + } + + for (const label of Object.keys(ownersLabel)) { + const info = change.labels[label]; + if (!info || !isDetailedLabelInfo(info)) { + return; + } + + const vote = ownersLabel[label]; + if ((info.default_value && info.default_value === vote) || vote === 0) { + // ignore default value + return; + } + + const approval = info.all?.filter(x => x._account_id === accountId)[0]; + return approval ? [approval, info] : undefined; + } + + return; +} + +export interface ChangeAccounts { + byAccountId: Map<number, AccountInfo>; + byEmailWithoutDomain: Map<string, AccountInfo>; + byFullName: Map<string, AccountInfo>; +} + +export function getChangeAccounts(change?: ChangeInfo): ChangeAccounts { + const accounts = { + byAccountId: new Map(), + byEmailWithoutDomain: new Map(), + byFullName: new Map(), + } as unknown as ChangeAccounts; + + if (!change) { + return accounts; + } + + [ + change.owner, + ...(change.submitter ? [change.submitter] : []), + ...(change.reviewers[ReviewerState.REVIEWER] ?? []), + ...(change.reviewers[ReviewerState.CC] ?? []), + ].forEach(account => { + if (account._account_id) { + accounts.byAccountId.set(account._account_id, account); + } + + if (account.email && account.email.indexOf('@') > 0) { + accounts.byEmailWithoutDomain.set( + account.email.substring( + 0, + account.email.indexOf('@') + ) as unknown as string, + account + ); + } + + if (account.name) { + accounts.byFullName.set(account.name, account); + } + }); + return accounts; +}
diff --git a/owners/web/gr-files_test.ts b/owners/web/gr-files_test.ts new file mode 100644 index 0000000..d05f6e7 --- /dev/null +++ b/owners/web/gr-files_test.ts
@@ -0,0 +1,475 @@ +/** + * @license + * Copyright (C) 2024 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 {assert} from '@open-wc/testing'; + +import { + computeApprovalAndInfo, + getChangeAccounts, + getFileOwnership, + shouldHide, +} from './gr-files'; +import { + FileOwnership, + FileStatus, + OwnerOrGroupOwner, + PatchRange, + UserRole, +} from './owners-model'; +import { + AccountInfo, + ApprovalInfo, + ChangeInfo, + ChangeStatus, + DetailedLabelInfo, + SubmitRequirementResultInfo, + ReviewerState, +} from '@gerritcodereview/typescript-api/rest-api'; +import {FilesOwners, OwnersLabels} from './owners-service'; +import {deepEqual} from './utils'; +import {getRandom} from './test-utils'; + +suite('owners status tests', () => { + const path = 'readme.md'; + const approvedPath = 'db.sql'; + const filesOwners = { + files: {[path]: [{name: 'John', id: 1}]}, + files_approved: {[approvedPath]: [{name: 'Merry', id: 2}]}, + } as unknown as FilesOwners; + + suite('shouldHide tests', () => { + const loggedIn = getRandom(UserRole.CHANGE_OWNER, UserRole.OTHER); + + test('shouldHide - should be `true` when change is not defined', () => { + const undefinedChange = undefined; + const definedPatchRange = {} as unknown as PatchRange; + assert.equal( + shouldHide(undefinedChange, definedPatchRange, filesOwners, loggedIn), + true + ); + }); + + test('shouldHide - should be `true` when patch range is not defined', () => { + const definedChange = {} as unknown as ChangeInfo; + const undefinedPatchRange = undefined; + assert.equal( + shouldHide(definedChange, undefinedPatchRange, filesOwners, loggedIn), + true + ); + }); + + test('shouldHide - should be `true` when change is abandoned', () => { + const abandonedChange = { + status: ChangeStatus.ABANDONED, + } as unknown as ChangeInfo; + const definedPatchRange = {} as unknown as PatchRange; + assert.equal( + shouldHide(abandonedChange, definedPatchRange, filesOwners, loggedIn), + true + ); + }); + + test('shouldHide - should be `true` when change is merged', () => { + const mergedChange = { + status: ChangeStatus.MERGED, + } as unknown as ChangeInfo; + const definedPatchRange = {} as unknown as PatchRange; + assert.equal( + shouldHide(mergedChange, definedPatchRange, filesOwners, loggedIn), + true + ); + }); + + test('shouldHide - should be `true` if not on the latest PS', () => { + const changeWithPs2 = { + status: ChangeStatus.NEW, + revisions: { + current_rev: {_number: 2}, + }, + current_revision: 'current_rev', + } as unknown as ChangeInfo; + const patchRangeOnPs1 = {patchNum: 1} as unknown as PatchRange; + assert.equal( + shouldHide(changeWithPs2, patchRangeOnPs1, filesOwners, loggedIn), + true + ); + }); + + const change = { + status: ChangeStatus.NEW, + revisions: { + current_rev: {_number: 1}, + }, + current_revision: 'current_rev', + } as unknown as ChangeInfo; + const patchRange = {patchNum: 1} as unknown as PatchRange; + + test('shouldHide - should be `true` when change has no submit requirements', () => { + assert.equal(shouldHide(change, patchRange, filesOwners, loggedIn), true); + }); + + test('shouldHide - should be `true` when change has no `Owner-Approval` submit requirements', () => { + const changeWithDifferentSubmitReqs = { + ...change, + submit_requirements: [ + {submittability_expression_result: {expression: 'other'}}, + ] as unknown as SubmitRequirementResultInfo[], + }; + assert.equal( + shouldHide( + changeWithDifferentSubmitReqs, + patchRange, + filesOwners, + loggedIn + ), + true + ); + }); + + const submitRequirements = getRandom({ + submit_requirements: [ + {submittability_expression_result: {expression: 'has:approval_owners'}}, + { + submittability_expression_result: { + expression: 'rule:owners~OwnersSubmitRequirement', + }, + }, + ] as unknown as SubmitRequirementResultInfo[], + }); + const changeWithSubmitRequirementOrRecord = { + ...change, + ...submitRequirements, + }; + + test('shouldHide - should be `true` when user is anonymous', () => { + const anonymous = UserRole.ANONYMOUS; + assert.equal( + shouldHide( + changeWithSubmitRequirementOrRecord, + patchRange, + filesOwners, + anonymous + ), + true + ); + }); + + test('shouldHide - should be `true` when files owners are `undefined`', () => { + const undefinedFilesOwners = getRandom( + undefined, + {} + ) as unknown as FilesOwners; + assert.equal( + shouldHide( + changeWithSubmitRequirementOrRecord, + patchRange, + undefinedFilesOwners, + loggedIn + ), + true + ); + }); + + test('shouldHide - should be `false` when change has all files approved', () => { + const allFilesApproved = { + approved_files: {[approvedPath]: [{name: 'Merry', id: 2}]}, + } as unknown as FilesOwners; + assert.equal( + shouldHide( + changeWithSubmitRequirementOrRecord, + patchRange, + allFilesApproved, + loggedIn + ), + true + ); + }); + + test('shouldHide - should be `false` when change has no files approved', () => { + const noApprovedFiles = { + files: {[path]: [{name: 'John', id: 1}]}, + } as unknown as FilesOwners; + assert.equal( + shouldHide( + changeWithSubmitRequirementOrRecord, + patchRange, + noApprovedFiles, + loggedIn + ), + false + ); + }); + + test('shouldHide - should be `false` when change has both approved and not approved files', () => { + assert.equal( + shouldHide( + changeWithSubmitRequirementOrRecord, + patchRange, + filesOwners, + loggedIn + ), + false + ); + }); + + test('shouldHide - should be `false` when in edit mode', () => { + const patchRangeWithoutPatchNum = {} as unknown as PatchRange; + assert.equal( + shouldHide(change, patchRangeWithoutPatchNum, filesOwners, loggedIn), + false + ); + }); + }); + + suite('getFileOwnership tests', () => { + const emptyFilesOwners = {} as unknown as FilesOwners; + + test('getFileOwnership - should be `undefined` when path is `undefined', () => { + const undefinedPath = undefined; + assert.equal( + getFileOwnership(undefinedPath, emptyFilesOwners), + undefined + ); + }); + + test('getFileOwnership - should be `undefined` when file owners are `undefined', () => { + const undefinedFileOwners = undefined; + assert.equal(getFileOwnership(path, undefinedFileOwners), undefined); + }); + + test('getFileOwnership - should return `FileOwnership` with `APPROVED` fileStatus when file is approved', () => { + assert.equal( + deepEqual(getFileOwnership(approvedPath, filesOwners), { + fileStatus: FileStatus.APPROVED, + owners: [{name: 'Merry', id: 2}], + } as FileOwnership), + true + ); + }); + + test('getFileOwnership - should return `FileOwnership` with `NOT_OWNED` fileStatus when file has no owner', () => { + assert.equal( + deepEqual(getFileOwnership(path, emptyFilesOwners), { + fileStatus: FileStatus.NOT_OWNED, + } as FileOwnership), + true + ); + }); + + test('getFileOwnership - should return `FileOwnership` with `NEEDS_APPROVAL` fileStatus when file has owner', () => { + const fileOwnersWithPathOwner = { + files: {[path]: [{name: 'John', id: 1}]}, + } as unknown as FilesOwners; + + assert.equal( + deepEqual(getFileOwnership(path, fileOwnersWithPathOwner), { + fileStatus: FileStatus.NEEDS_APPROVAL, + owners: [{name: 'John', id: 1}], + } as FileOwnership), + true + ); + }); + }); + + suite('computeApprovalAndInfo tests', () => { + const account = 1; + const fileOwner = { + owner: {_account_id: account} as unknown as AccountInfo, + } as unknown as OwnerOrGroupOwner; + const label = 'Code-Review'; + const crPlus1OwnersVote = { + [`${account}`]: {[label]: 1}, + } as unknown as OwnersLabels; + const changeWithLabels = { + labels: { + [label]: { + all: [ + { + value: 1, + date: '2024-10-22 17:26:21.000000000', + permitted_voting_range: { + min: -2, + max: 2, + }, + _account_id: account, + }, + ], + values: { + '-2': 'This shall not be submitted', + '-1': 'I would prefer this is not submitted as is', + ' 0': 'No score', + '+1': 'Looks good to me, but someone else must approve', + '+2': 'Looks good to me, approved', + }, + description: '', + default_value: 0, + }, + }, + } as unknown as ChangeInfo; + + test('computeApprovalAndInfo - should be `undefined` when change is `undefined', () => { + const undefinedChange = undefined; + assert.equal( + computeApprovalAndInfo(fileOwner, crPlus1OwnersVote, undefinedChange), + undefined + ); + }); + + test('computeApprovalAndInfo - should be `undefined` when there is no owners vote', () => { + const emptyOwnersVote = {}; + assert.equal( + computeApprovalAndInfo(fileOwner, emptyOwnersVote, changeWithLabels), + undefined + ); + }); + + test('computeApprovalAndInfo - should be `undefined` for default owners vote', () => { + const defaultOwnersVote = {[label]: 0} as unknown as OwnersLabels; + assert.equal( + computeApprovalAndInfo(fileOwner, defaultOwnersVote, changeWithLabels), + undefined + ); + }); + + test('computeApprovalAndInfo - should be computed for CR+1 owners vote', () => { + const expectedApproval = { + value: 1, + date: '2024-10-22 17:26:21.000000000', + permitted_voting_range: { + min: -2, + max: 2, + }, + _account_id: account, + } as unknown as ApprovalInfo; + const expectedInfo = { + all: [expectedApproval], + values: { + '-2': 'This shall not be submitted', + '-1': 'I would prefer this is not submitted as is', + ' 0': 'No score', + '+1': 'Looks good to me, but someone else must approve', + '+2': 'Looks good to me, approved', + }, + description: '', + default_value: 0, + } as unknown as DetailedLabelInfo; + + assert.equal( + deepEqual( + computeApprovalAndInfo( + fileOwner, + crPlus1OwnersVote, + changeWithLabels + ), + [expectedApproval, expectedInfo] + ), + true + ); + }); + }); + + suite('getChangeAccounts tests', () => { + test('getChangeAccounts - should return empty map when change is `undefined', () => { + const undefinedChange = undefined; + const accounts = getChangeAccounts(undefinedChange); + assert.equal(accounts.byAccountId.size, 0); + assert.equal(accounts.byEmailWithoutDomain.size, 0); + assert.equal(accounts.byFullName.size, 0); + }); + + test('getChangeAccounts - should return map with owner when change has only owner and empty reviewers defined', () => { + const owner = account(1); + const changeWithOwner = { + owner, + reviewers: {}, + } as unknown as ChangeInfo; + const accounts = getChangeAccounts(changeWithOwner); + assert.equal( + deepEqual(accounts.byAccountId, new Map([[1, owner]])), + true + ); + assert.equal( + deepEqual(accounts.byEmailWithoutDomain, new Map([['1_email', owner]])), + true + ); + assert.equal( + deepEqual(accounts.byFullName, new Map([['1_name', owner]])), + true + ); + }); + + test('getChangeAccounts - should return map with owner, submitter and reviewers', () => { + const owner = account(1); + const submitter = account(2); + const reviewer = account(3); + const ccReviewer = account(4); + const change = { + owner, + submitter, + reviewers: { + [ReviewerState.REVIEWER]: [reviewer], + [ReviewerState.CC]: [ccReviewer], + }, + } as unknown as ChangeInfo; + const accounts = getChangeAccounts(change); + assert.equal( + deepEqual( + accounts.byAccountId, + new Map([ + [1, owner], + [2, submitter], + [3, reviewer], + [4, ccReviewer], + ]) + ), + true + ); + assert.equal( + deepEqual( + accounts.byEmailWithoutDomain, + new Map([ + ['1_email', owner], + ['2_email', submitter], + ['3_email', reviewer], + ['4_email', ccReviewer], + ]) + ), + true + ); + assert.equal( + deepEqual( + accounts.byFullName, + new Map([ + ['1_name', owner], + ['2_name', submitter], + ['3_name', reviewer], + ['4_name', ccReviewer], + ]) + ), + true + ); + }); + }); +}); + +function account(id: number) { + return { + _account_id: id, + email: `${id}_email@example.com`, + name: `${id}_name`, + } as unknown as AccountInfo; +}
diff --git a/owners/web/gr-owned-files.ts b/owners/web/gr-owned-files.ts new file mode 100644 index 0000000..96ea373 --- /dev/null +++ b/owners/web/gr-owned-files.ts
@@ -0,0 +1,504 @@ +/** + * @license + * Copyright (C) 2024 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 {css, html, LitElement, PropertyValues, nothing, CSSResult} from 'lit'; +import {ifDefined} from 'lit/directives/if-defined.js'; +import {OwnersMixin} from './owners-mixin'; +import {customElement, property} from 'lit/decorators'; +import { + AccountInfo, + ChangeInfo, + ChangeStatus, + EmailAddress, + RevisionInfo, + EDIT, +} from '@gerritcodereview/typescript-api/rest-api'; +import {User, UserRole} from './owners-model'; +import { + FilesOwners, + hasOwnersSubmitRequirement, + isOwner, + OwnedFiles, +} from './owners-service'; +import { + computeDisplayPath, + diffFilePaths, + encodeURL, + getBaseUrl, + truncatePath, +} from './utils'; + +const STATUS_CODE = { + MISSING: 'missing', + APPROVED: 'approved', +}; + +const STATUS_ICON = { + [STATUS_CODE.MISSING]: 'schedule', + [STATUS_CODE.APPROVED]: 'check', +}; + +export interface OwnedFilesInfo { + ownedFiles: string[]; + numberOfPending: number; + numberOfApproved: number; +} + +const common = OwnersMixin(LitElement); + +class OwnedFilesCommon extends common { + @property({type: Object}) + revision?: RevisionInfo; + + protected ownedFilesInfo?: OwnedFilesInfo; + + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + this.computeOwnedFiles(); + + this.hidden = shouldHide( + this.change, + this.revision, + this.user, + this.ownedFilesInfo?.ownedFiles + ); + } + + static commonStyles(): CSSResult[] { + return [window?.Gerrit?.styles.font as CSSResult]; + } + + private computeOwnedFiles() { + this.ownedFilesInfo = ownedFiles(this.user?.account, this.filesOwners); + } +} + +export const OWNED_FILES_TAB_HEADER = 'owned-files-tab-header'; +@customElement(OWNED_FILES_TAB_HEADER) +export class OwnedFilesTabHeader extends OwnedFilesCommon { + @property({type: String, reflect: true, attribute: 'files-status'}) + filesStatus?: string; + + private ownedFiles: number | undefined; + + static override get styles() { + return [ + ...OwnedFilesCommon.commonStyles(), + css` + [hidden] { + display: none; + } + gr-icon { + padding: var(--spacing-xs) 0px; + margin-left: 3px; + } + :host([files-status='approved']) gr-icon.status { + color: var(--positive-green-text-color); + } + :host([files-status='missing']) gr-icon.status { + color: #ffa62f; + } + `, + ]; + } + + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + + if (this.hidden || this.ownedFilesInfo === undefined) { + this.filesStatus = undefined; + this.ownedFiles = undefined; + } else { + [this.filesStatus, this.ownedFiles] = + this.ownedFilesInfo.numberOfPending > 0 + ? [STATUS_CODE.MISSING, this.ownedFilesInfo.numberOfPending] + : [STATUS_CODE.APPROVED, this.ownedFilesInfo.numberOfApproved]; + } + } + + override render() { + // even if `nothing` is returned Gerrit still shows the pointer and allows + // clicking at it, redirecting to the empty tab when done; traverse through + // the shadowRoots down to the tab and disable/enable it when needed + const tabParent = document + .querySelector('#pg-app') + ?.shadowRoot?.querySelector('#app-element') + ?.shadowRoot?.querySelector('main > gr-change-view') + ?.shadowRoot?.querySelector( + '#tabs > paper-tab[data-name="change-view-tab-header-owners"]' + ); + if (this.hidden) { + if (tabParent && !tabParent.getAttribute('disabled')) { + tabParent.setAttribute('disabled', 'disabled'); + } + return nothing; + } + if (tabParent && tabParent.getAttribute('disabled')) { + tabParent.removeAttribute('disabled'); + } + + if ( + this.ownedFilesInfo === undefined || + this.filesStatus === undefined || + this.ownedFiles === undefined + ) { + return html`<div>Owned Files</div>`; + } + + const icon = STATUS_ICON[this.filesStatus]; + const [info, summary] = this.buildInfoAndSummary( + this.filesStatus, + this.ownedFilesInfo.numberOfApproved, + this.ownedFilesInfo.numberOfPending + ); + return html` <div> + Owned Files + <gr-tooltip-content + title=${info} + aria-label=${summary} + aria-description=${info} + has-tooltip + > + <gr-icon + id="owned-files-status" + class="status" + icon=${icon} + aria-hidden="true" + ></gr-icon> + </gr-tooltip-content> + </div>`; + } + + private buildInfoAndSummary( + filesStatus: string, + filesApproved: number, + filesPending: number + ): [string, string] { + const pendingInfo = + filesPending > 0 + ? `Missing approval for ${filesPending} file${ + filesPending > 1 ? 's' : '' + }` + : ''; + const approvedInfo = + filesApproved > 0 + ? `${filesApproved} file${ + filesApproved > 1 ? 's' : '' + } already approved.` + : ''; + const info = `${ + STATUS_CODE.APPROVED === filesStatus + ? approvedInfo + : `${pendingInfo}${filesApproved > 0 ? ` and ${approvedInfo}` : '.'}` + }`; + const summary = `${ + STATUS_CODE.APPROVED === filesStatus ? 'Approved' : 'Missing' + }`; + return [info, summary]; + } +} + +@customElement('gr-owned-files-list') +export class GrOwnedFilesList extends LitElement { + @property({type: Object}) + change?: ChangeInfo; + + @property({type: Object}) + revision?: RevisionInfo; + + @property({type: Array}) + ownedFiles?: string[]; + + static override get styles() { + return [ + ...OwnedFilesCommon.commonStyles(), + css` + :host { + display: block; + } + .row { + align-items: center; + border-top: 1px solid var(--border-color); + display: flex; + min-height: calc(var(--line-height-normal) + 2 * var(--spacing-s)); + padding: var(--spacing-xs) var(--spacing-l); + } + .header-row { + background-color: var(--background-color-secondary); + } + .file-row { + cursor: pointer; + } + .file-row:hover { + background-color: var(--hover-background-color); + } + .file-row.selected { + background-color: var(--selection-background-color); + } + .path { + cursor: pointer; + flex: 1; + /* Wrap it into multiple lines if too long. */ + white-space: normal; + word-break: break-word; + } + .matchingFilePath { + color: var(--deemphasized-text-color); + } + .newFilePath { + color: var(--primary-text-color); + } + .fileName { + color: var(--link-color); + } + .truncatedFileName { + display: none; + } + .row:focus { + outline: none; + } + .row:hover { + opacity: 100; + } + .pathLink { + display: inline-block; + margin: -2px 0; + padding: var(--spacing-s) 0; + text-decoration: none; + } + .pathLink:hover span.fullFileName, + .pathLink:hover span.truncatedFileName { + text-decoration: underline; + } + `, + ]; + } + + override render() { + if (!this.change || !this.revision || !this.ownedFiles) return nothing; + + return html` + <div id="container" role="grid" aria-label="Owned files list"> + ${this.renderOwnedFilesHeaderRow()} + ${this.ownedFiles?.map((ownedFile, index) => + this.renderOwnedFileRow(ownedFile, index) + )} + </div> + `; + } + + private renderOwnedFilesHeaderRow() { + return html` + <div class="header-row row" role="row"> + <div class="path" role="columnheader">File</div> + </div> + `; + } + + private renderOwnedFileRow(ownedFile: string, index: number) { + return html` + <div + class="file-row row" + tabindex="-1" + role="row" + aria-label=${ownedFile} + > + ${this.renderFilePath(ownedFile, index)} + </div> + `; + } + + private renderFilePath(file: string, index: number) { + const displayPath = computeDisplayPath(file); + const previousFile = (this.ownedFiles ?? [])[index - 1]; + return html` + <span class="path" role="gridcell"> + <a + class="pathLink" + href=${ifDefined(computeDiffUrl(file, this.change, this.revision))} + > + <span title=${displayPath} class="fullFileName"> + ${this.renderStyledPath(file, previousFile)} + </span> + <span title=${displayPath} class="truncatedFileName"> + ${truncatePath(displayPath)} + </span> + </a> + </span> + `; + } + + private renderStyledPath(filePath: string, previousFilePath?: string) { + const {matchingFolders, newFolders, fileName} = diffFilePaths( + filePath, + previousFilePath + ); + return [ + matchingFolders.length > 0 + ? html`<span class="matchingFilePath">${matchingFolders}</span>` + : nothing, + newFolders.length > 0 + ? html`<span class="newFilePath">${newFolders}</span>` + : nothing, + html`<span class="fileName">${fileName}</span>`, + ]; + } +} + +export const OWNED_FILES_TAB_CONTENT = 'owned-files-tab-content'; +@customElement(OWNED_FILES_TAB_CONTENT) +export class OwnedFilesTabContent extends OwnedFilesCommon { + static override get styles() { + return [ + ...OwnedFilesCommon.commonStyles(), + css` + :host { + display: block; + } + `, + ]; + } + + override render() { + if (this.hidden) return nothing; + + return html` + <gr-owned-files-list + id="ownedFilesList" + .change=${this.change} + .revision=${this.revision} + .ownedFiles=${this.ownedFilesInfo?.ownedFiles} + > + </gr-owned-files-list> + `; + } +} + +export function shouldHide( + change?: ChangeInfo, + revision?: RevisionInfo, + user?: User, + ownedFiles?: string[] +) { + // don't show owned files when no change or change is abandoned/merged or being edited or viewing not current PS + if ( + change === undefined || + change.status === ChangeStatus.ABANDONED || + change.status === ChangeStatus.MERGED || + revision === undefined || + revision._number === EDIT || + change.current_revision !== revision.commit?.commit + ) { + return true; + } + + // show owned files if user owns anything + if (hasOwnersSubmitRequirement(change)) { + return ( + !user || + user.role === UserRole.ANONYMOUS || + (ownedFiles ?? []).length === 0 + ); + } + return true; +} + +function collectOwnedFiles( + owner: AccountInfo, + groupPrefix: string, + files: OwnedFiles, + emailWithoutDomain?: string +): string[] { + const ownedFiles = []; + for (const file of Object.keys(files ?? [])) { + if ( + files[file].find(fileOwner => { + if (isOwner(fileOwner)) { + return fileOwner.id === owner._account_id; + } + + return ( + !fileOwner.name?.startsWith(groupPrefix) && + (fileOwner.name === emailWithoutDomain || + fileOwner.name === owner.name) + ); + }) + ) { + ownedFiles.push(file); + } + } + + return ownedFiles; +} + +export function ownedFiles( + owner?: AccountInfo, + filesOwners?: FilesOwners +): OwnedFilesInfo | undefined { + if ( + !owner || + !filesOwners || + (!filesOwners.files && !filesOwners.files_approved) + ) { + return; + } + + const groupPrefix = 'group/'; + const emailWithoutDomain = toEmailWithoutDomain(owner.email); + const pendingFiles = collectOwnedFiles( + owner, + groupPrefix, + filesOwners.files, + emailWithoutDomain + ); + const approvedFiles = collectOwnedFiles( + owner, + groupPrefix, + filesOwners.files_approved, + emailWithoutDomain + ); + return { + ownedFiles: [...pendingFiles, ...approvedFiles], + numberOfPending: pendingFiles.length, + numberOfApproved: approvedFiles.length, + } as OwnedFilesInfo; +} + +export function computeDiffUrl( + file: string, + change?: ChangeInfo, + revision?: RevisionInfo +) { + if (change === undefined || revision?._number === undefined) { + return; + } + + let repo = ''; + if (change.project) repo = `${encodeURL(change.project)}/+/`; + + // TODO it can be a range of patchsets but `PatchRange` is not passed to the `change-view-tab-content` :/ fix in Gerrit? + const range = `/${revision._number}`; + + const path = `/${encodeURL(file)}`; + + return `${getBaseUrl()}/c/${repo}${change._number}${range}${path}`; +} + +function toEmailWithoutDomain(email?: EmailAddress): string | undefined { + const startDomainIndex = email?.indexOf('@'); + return startDomainIndex ? email?.substring(0, startDomainIndex) : undefined; +}
diff --git a/owners/web/gr-owned-files_test.ts b/owners/web/gr-owned-files_test.ts new file mode 100644 index 0000000..d7bef24 --- /dev/null +++ b/owners/web/gr-owned-files_test.ts
@@ -0,0 +1,242 @@ +/** + * @license + * Copyright (C) 2024 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 {assert} from '@open-wc/testing'; +import { + AccountId, + AccountInfo, + ChangeInfo, + ChangeStatus, + CommitId, + RevisionInfo, + EDIT, + SubmitRequirementResultInfo, +} from '@gerritcodereview/typescript-api/rest-api'; +import {ownedFiles, OwnedFilesInfo, shouldHide} from './gr-owned-files'; +import {FilesOwners, GroupOwner, Owner} from './owners-service'; +import {deepEqual} from './utils'; +import {User, UserRole} from './owners-model'; +import {getRandom} from './test-utils'; + +suite('owned files tests', () => { + suite('ownedFiles tests', () => { + const ownerAccountId = 1; + const owner = account(ownerAccountId); + + const ownedFile = 'README.md'; + const ownedApprovedFile = 'db.sql'; + const files = { + [ownedFile]: [fileOwner(1)], + 'some.text': [fileOwner(6)], + }; + const files_approved = { + [ownedApprovedFile]: [fileOwner(1)], + }; + const filesOwners = { + files, + files_approved, + } as unknown as FilesOwners; + const emptyOwnerFilesInfo = { + ownedFiles: [], + numberOfApproved: 0, + numberOfPending: 0, + } as unknown as OwnedFilesInfo; + + test('ownedFiles - should be `undefined` when owner is `undefined`', () => { + const undefinedOwner = undefined; + assert.equal(ownedFiles(undefinedOwner, filesOwners), undefined); + }); + + test('ownedFiles - should be `undefined` when filesOwners are `undefined`', () => { + const undefinedFilesOwners = undefined; + assert.equal(ownedFiles(owner, undefinedFilesOwners), undefined); + }); + + test('ownedFiles - should return empty owned file when no files are owned by user', () => { + const user = account(2); + assert.equal( + deepEqual(ownedFiles(user, filesOwners), emptyOwnerFilesInfo), + true + ); + }); + + test('ownedFiles - should return owned files', () => { + assert.equal( + deepEqual(ownedFiles(owner, filesOwners), { + ownedFiles: [ownedFile, ownedApprovedFile], + numberOfApproved: 1, + numberOfPending: 1, + }), + true + ); + }); + + test('ownedFiles - should match file owner through email without domain name', () => { + const filesOwners = { + files: { + [ownedFile]: [fileOwnerWithNameOnly(`${ownerAccountId}_email`)], + 'some.text': [fileOwnerWithNameOnly('random_joe')], + }, + } as unknown as FilesOwners; + assert.equal( + deepEqual(ownedFiles(owner, filesOwners), { + ownedFiles: [ownedFile], + numberOfApproved: 0, + numberOfPending: 1, + }), + true + ); + }); + + test('ownedFiles - should match file owner through full name', () => { + const filesOwners = { + files_approved: { + [ownedFile]: [fileOwnerWithNameOnly(`${ownerAccountId}_name`)], + 'some.text': [fileOwnerWithNameOnly('random_joe')], + }, + } as unknown as FilesOwners; + assert.equal( + deepEqual(ownedFiles(owner, filesOwners), { + ownedFiles: [ownedFile], + numberOfApproved: 1, + numberOfPending: 0, + }), + true + ); + }); + + test('ownedFiles - should NOT match file owner over email without domain or full name when account id is different', () => { + const notFileOwner = {...owner, _account_id: 2 as unknown as AccountId}; + assert.equal( + deepEqual(ownedFiles(notFileOwner, filesOwners), emptyOwnerFilesInfo), + true + ); + }); + }); + + suite('shouldHide tests', () => { + const current_revision = 'commit-1' as unknown as CommitId; + const change = { + status: ChangeStatus.NEW, + submit_requirements: [ + {submittability_expression_result: {expression: 'has:approval_owners'}}, + ] as unknown as SubmitRequirementResultInfo[], + current_revision, + } as unknown as ChangeInfo; + const revisionInfo = { + _number: 1, + commit: {commit: current_revision}, + } as unknown as RevisionInfo; + const user = {account: account(1), role: UserRole.OTHER}; + const ownedFiles = ['README.md']; + + test('shouldHide - should be `true` when change is `undefined`', () => { + const undefinedChange = undefined; + assert.equal( + shouldHide(undefinedChange, revisionInfo, user, ownedFiles), + true + ); + }); + + test('shouldHide - should be `true` when change is `ABANDONED` or `MERGED`', () => { + const abandonedOrMergedChange = { + ...change, + status: getRandom(ChangeStatus.ABANDONED, ChangeStatus.MERGED), + }; + assert.equal( + shouldHide(abandonedOrMergedChange, revisionInfo, user, ownedFiles), + true + ); + }); + + test('shouldHide - should be `true` when revisionInfo is `undefined` or in `EDIT` mode', () => { + const undefinedOrEditRevisionInfo = getRandom(undefined, { + _number: EDIT, + } as unknown as RevisionInfo); + assert.equal( + shouldHide(change, undefinedOrEditRevisionInfo, user, ownedFiles), + true + ); + }); + + test('shouldHide - should be `true` when NOT current_revision is viewed', () => { + const notCurrentRevisionInfo = { + _number: 0, + commit: {commit: 'not-current' as unknown as CommitId}, + } as unknown as RevisionInfo; + assert.equal( + shouldHide(change, notCurrentRevisionInfo, user, ownedFiles), + true + ); + }); + + test('shouldHide - should be `true` when change has different submit requirements', () => { + const changeWithOtherSubmitRequirements = { + ...change, + submit_requirements: [ + { + submittability_expression_result: { + expression: 'has:other_predicate', + }, + }, + ] as unknown as SubmitRequirementResultInfo[], + }; + assert.equal( + shouldHide( + changeWithOtherSubmitRequirements, + revisionInfo, + user, + ownedFiles + ), + true + ); + }); + + test('shouldHide - should be `true` when user is `undefined` or `ANONYMOUS`', () => { + const undefinedOrAnonymousUser = getRandom(undefined, { + role: UserRole.ANONYMOUS, + } as unknown as User); + assert.equal( + shouldHide(change, revisionInfo, undefinedOrAnonymousUser, ownedFiles), + true + ); + }); + + test('shouldHide - should be `false` when user owns files', () => { + assert.equal(shouldHide(change, revisionInfo, user, ownedFiles), false); + }); + }); +}); + +function account(id: number): AccountInfo { + return { + _account_id: id, + email: `${id}_email@example.com`, + name: `${id}_name`, + } as unknown as AccountInfo; +} + +function fileOwner(id: number): Owner { + return { + id, + name: `${id}_name`, + } as unknown as Owner; +} + +function fileOwnerWithNameOnly(name: string): GroupOwner { + return {name} as unknown as GroupOwner; +}
diff --git a/owners/web/karma_test.sh b/owners/web/karma_test.sh new file mode 100755 index 0000000..7cecdae --- /dev/null +++ b/owners/web/karma_test.sh
@@ -0,0 +1,6 @@ +#!/bin/bash +set -euo pipefail +./$1 start $2 --single-run \ + --root 'plugins/owners/web/_bazel_ts_out_tests/' \ + --test-files '*_test.js' \ + --browsers ${3:-ChromeHeadless}
diff --git a/owners/web/owners-mixin.ts b/owners/web/owners-mixin.ts new file mode 100644 index 0000000..cd9217a --- /dev/null +++ b/owners/web/owners-mixin.ts
@@ -0,0 +1,124 @@ +/** + * @license + * Copyright (C) 2024 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 {Subscription} from 'rxjs'; +import {LitElement, PropertyValues} from 'lit'; +import {property, state} from 'lit/decorators'; +import {ChangeInfo} from '@gerritcodereview/typescript-api/rest-api'; +import {FilesOwners, OwnersService} from './owners-service'; +import {RestPluginApi} from '@gerritcodereview/typescript-api/rest'; +import {ModelLoader, OwnersModel, PatchRange, User} from './owners-model'; + +// Lit mixin definition as described in https://lit.dev/docs/composition/mixins/ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type Constructor<T> = new (...args: any[]) => T; + +export interface OwnersInterface extends LitElement { + change?: ChangeInfo; + patchRange?: PatchRange; + restApi?: RestPluginApi; + user?: User; + filesOwners?: FilesOwners; + + onModelUpdate(): void; +} + +export const OwnersMixin = <T extends Constructor<LitElement>>( + superClass: T +) => { + class Mixin extends superClass { + @property({type: Object}) + change?: ChangeInfo; + + @property({type: Object}) + patchRange?: PatchRange; + + @property({type: Object}) + restApi?: RestPluginApi; + + @state() + user?: User; + + @state() + filesOwners?: FilesOwners; + + private _model?: OwnersModel; + + modelLoader?: ModelLoader; + + private subscriptions: Array<Subscription> = []; + + get model() { + return this._model; + } + + set model(model: OwnersModel | undefined) { + if (this._model === model) return; + for (const s of this.subscriptions) { + s.unsubscribe(); + } + this.subscriptions = []; + this._model = model; + if (!model) return; + + this.subscriptions.push( + model.state$.subscribe(s => { + this.user = s.user; + }) + ); + + this.subscriptions.push( + model.state$.subscribe(s => { + this.filesOwners = s.filesOwners; + }) + ); + + this.onModelUpdate(); + } + + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + + if (changedProperties.has('change') || changedProperties.has('restApi')) { + if (!this.restApi || !this.change) { + this.model = undefined; + this.modelLoader = undefined; + return; + } + const service = OwnersService.getOwnersService( + this.restApi, + this.change + ); + const model = OwnersModel.getModel(this.change); + this.modelLoader = new ModelLoader(service, model); + this.model = model; + } + } + + protected onModelUpdate() { + this.modelLoader?.loadUser(); + this.modelLoader?.loadFilesOwners(); + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + constructor(...args: any[]) { + super(...args); + } + } + + return Mixin as unknown as T & Constructor<OwnersInterface>; +};
diff --git a/owners/web/owners-model.ts b/owners/web/owners-model.ts new file mode 100644 index 0000000..48e7e08 --- /dev/null +++ b/owners/web/owners-model.ts
@@ -0,0 +1,148 @@ +/** + * @license + * Copyright (C) 2024 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 {BehaviorSubject, Observable} from 'rxjs'; +import { + AccountInfo, + BasePatchSetNum, + ChangeInfo, + GroupInfo, + RevisionPatchSetNum, +} from '@gerritcodereview/typescript-api/rest-api'; +import {FileOwner, FilesOwners, OwnersService} from './owners-service'; +import {deepEqual} from './utils'; + +export interface PatchRange { + patchNum: RevisionPatchSetNum; + basePatchNum: BasePatchSetNum; +} + +export enum UserRole { + ANONYMOUS = 'ANONYMOUS', + CHANGE_OWNER = 'CHANGE_OWNER', + OTHER = 'OTHER', +} + +export interface User { + account?: AccountInfo; + role: UserRole; +} + +export interface OwnersState { + user?: User; + filesOwners?: FilesOwners; +} + +export enum FileStatus { + NEEDS_APPROVAL = 'NEEDS_APPROVAL', + APPROVED = 'APPROVED', + NOT_OWNED = 'NOT_OWNED', +} + +export interface FileOwnership { + fileStatus: FileStatus; + owners?: FileOwner[]; +} + +export interface OwnerOrGroupOwner { + owner?: AccountInfo; + groupOwner?: GroupInfo; +} + +let ownersModel: OwnersModel | undefined; + +export class OwnersModel extends EventTarget { + private subject$: BehaviorSubject<OwnersState> = new BehaviorSubject( + {} as OwnersState + ); + + public state$: Observable<OwnersState> = this.subject$.asObservable(); + + constructor(readonly change: ChangeInfo) { + super(); + } + + get state() { + return this.subject$.getValue(); + } + + private setState(state: OwnersState) { + this.subject$.next(Object.freeze(state)); + } + + setUser(user: User) { + const current = this.subject$.getValue(); + if (current.user === user) return; + this.setState({...current, user}); + } + + setFilesOwners(filesOwners: FilesOwners | undefined) { + const current = this.subject$.getValue(); + if (deepEqual(current.filesOwners, filesOwners)) return; + this.setState({...current, filesOwners}); + } + + static getModel(change: ChangeInfo) { + if (!ownersModel || ownersModel.change !== change) { + ownersModel = new OwnersModel(change); + } + return ownersModel; + } +} + +export class ModelLoader { + constructor( + private readonly service: OwnersService, + private readonly model: OwnersModel + ) {} + + async loadUser() { + await this._loadProperty( + 'user', + () => this.service.getLoggedInUser(), + value => this.model.setUser(value) + ); + } + + async loadFilesOwners() { + await this._loadProperty( + 'filesOwners', + () => this.service.getFilesOwners(), + value => this.model.setFilesOwners(value) + ); + } + + private async _loadProperty<K extends keyof OwnersState, T>( + propertyName: K, + propertyLoader: () => Promise<T>, + propertySetter: (value: T) => void + ) { + if (this.model.state[propertyName] !== undefined) return; + let newValue: T; + try { + newValue = await propertyLoader(); + } catch (e) { + console.error(e); + return; + } + // It is possible, that several requests is made in parallel. + // Store only the first result and discard all other results. + // (also, due to the CodeOwnersCacheApi all result must be identical) + if (this.model.state[propertyName] !== undefined) return; + propertySetter(newValue); + } +}
diff --git a/owners/web/owners-service.ts b/owners/web/owners-service.ts new file mode 100644 index 0000000..0a57362 --- /dev/null +++ b/owners/web/owners-service.ts
@@ -0,0 +1,206 @@ +/** + * @license + * Copyright (C) 2024 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 {HttpMethod, RestPluginApi} from '@gerritcodereview/typescript-api/rest'; +import { + AccountDetailInfo, + ChangeInfo, + NumericChangeId, + RepoName, +} from '@gerritcodereview/typescript-api/rest-api'; +import {User, UserRole} from './owners-model'; + +export interface GroupOwner { + name: string; +} + +export interface Owner extends GroupOwner { + id: number; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function isOwner(o: any): o is Owner { + return o && typeof o.id === 'number' && typeof o.name === 'string'; +} + +export type FileOwner = Owner | GroupOwner; + +export interface OwnedFiles { + [fileName: string]: FileOwner[]; +} + +export interface OwnersLabels { + [id: string]: { + [label: string]: number; + }; +} + +export interface FilesOwners { + files: OwnedFiles; + files_approved: OwnedFiles; + owners_labels: OwnersLabels; +} + +const OWNERS_SUBMIT_REQUIREMENT = 'has:approval_owners'; +const OWNERS_SUBMIT_RULE = 'owners~OwnersSubmitRequirement'; + +export function hasOwnersSubmitRequirement(change: ChangeInfo): boolean { + return ( + change.submit_requirements !== undefined && + change.submit_requirements.find(r => { + const expression = r.submittability_expression_result.expression; + return ( + expression.includes(OWNERS_SUBMIT_REQUIREMENT) || + expression.includes(OWNERS_SUBMIT_RULE) + ); + }) !== undefined + ); +} + +class ResponseError extends Error { + constructor(readonly response: Response) { + super(); + } +} + +async function getErrorMessage(response: Response) { + const text = await response.text(); + return text ? `${response.status}: ${text}` : `${response.status}`; +} + +class OwnersApi { + constructor(readonly restApi: RestPluginApi) {} + + async getAccount(): Promise<AccountDetailInfo | undefined> { + const loggedIn = await this.restApi.getLoggedIn(); + if (!loggedIn) return undefined; + return await this.restApi.getAccount(); + } + + /** + * Returns the list of owners associated to each file that needs a review or were approved, + * and, for each owner, its current labels and votes. + * + * @doc + * https://gerrit.googlesource.com/plugins/owners/+/refs/heads/master/owners/src/main/resources/Documentation/rest-api.md + */ + getFilesOwners( + repoName: RepoName, + changeId: NumericChangeId, + revision: String + ): Promise<FilesOwners> { + return this.get( + `/changes/${encodeURIComponent( + repoName + )}~${changeId}/revisions/${revision}/owners~files-owners` + ) as Promise<FilesOwners>; + } + + private async get(url: string): Promise<unknown> { + const errFn = (response?: Response | null, error?: Error) => { + if (error) throw error; + if (response) throw new ResponseError(response); + throw new Error('Generic REST API error'); + }; + try { + return await this.restApi.send(HttpMethod.GET, url, undefined, errFn); + } catch (err) { + if (err instanceof ResponseError && err.response.status === 409) { + getErrorMessage(err.response).then(msg => { + // TODO handle 409 in UI - show banner with error + console.error(`Plugin configuration errot: ${msg}`); + }); + } + throw err; + } + } +} + +/** + * Calls to REST API can be safely cached cuz: + * 1. Gerrit doesn't update the existing change object but obtains and passes new instead + * 2. `OwnersService.getOwnersService` guarantees that whenever new change object is retrieved the new service instance is created (and cache is created with it) + */ +class OwnersApiCache { + private loggedInUser?: Promise<User>; + + private filesOwners?: Promise<FilesOwners | undefined>; + + constructor( + private readonly api: OwnersApi, + private readonly change: ChangeInfo + ) {} + + getLoggedInUser(): Promise<User> { + if (this.loggedInUser === undefined) { + this.loggedInUser = this.getLoggedInUserImpl(); + } + return this.loggedInUser; + } + + getFilesOwners(): Promise<FilesOwners | undefined> { + if (this.filesOwners === undefined) { + this.filesOwners = this.getFilesOwnersImpl(); + } + return this.filesOwners; + } + + private async getLoggedInUserImpl(): Promise<User> { + const account = await this.api.getAccount(); + if (!account) { + return {role: UserRole.ANONYMOUS} as unknown as User; + } + const role = + this.change.owner._account_id === account._account_id + ? UserRole.CHANGE_OWNER + : UserRole.OTHER; + return {account, role} as unknown as User; + } + + private async getFilesOwnersImpl(): Promise<FilesOwners | undefined> { + return this.api.getFilesOwners( + this.change.project, + this.change._number, + this.change.current_revision ?? 'current' + ); + } +} + +let service: OwnersService | undefined; + +export class OwnersService { + private apiCache: OwnersApiCache; + + constructor(readonly restApi: RestPluginApi, readonly change: ChangeInfo) { + this.apiCache = new OwnersApiCache(new OwnersApi(restApi), change); + } + + getLoggedInUser(): Promise<User> { + return this.apiCache.getLoggedInUser(); + } + + getFilesOwners(): Promise<FilesOwners | undefined> { + return this.apiCache.getFilesOwners(); + } + + static getOwnersService(restApi: RestPluginApi, change: ChangeInfo) { + if (!service || service.change !== change) { + service = new OwnersService(restApi, change); + } + return service; + } +}
diff --git a/owners/web/owners-service_test.ts b/owners/web/owners-service_test.ts new file mode 100644 index 0000000..fbb9dc7 --- /dev/null +++ b/owners/web/owners-service_test.ts
@@ -0,0 +1,298 @@ +/** + * @license + * Copyright (C) 2024 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 { + FilesOwners, + hasOwnersSubmitRequirement, + OwnersService, +} from './owners-service'; +import { + RequestPayload, + RestPluginApi, +} from '@gerritcodereview/typescript-api/rest'; +import { + AccountInfo, + ChangeInfo, + ChangeStatus, + HttpMethod, +} from '@gerritcodereview/typescript-api/rest-api'; +import {assert} from '@open-wc/testing'; +import {UserRole} from './owners-model'; +import {deepEqual} from './utils'; + +suite('owners service tests', () => { + const fakeRestApi = {} as unknown as RestPluginApi; + const fakeChange = {} as unknown as ChangeInfo; + + suite('basic api request tests', () => { + test('getOwnersService - same change returns the same instance', () => { + assert.equal( + OwnersService.getOwnersService(fakeRestApi, fakeChange), + OwnersService.getOwnersService(fakeRestApi, fakeChange) + ); + }); + + test('getOwnersService - modified change returns new instance', () => { + assert.notEqual( + OwnersService.getOwnersService(fakeRestApi, {...fakeChange}), + OwnersService.getOwnersService(fakeRestApi, {...fakeChange}) + ); + }); + }); + + suite('user role tests', () => { + test('getLoggedInUser - returns ANONYMOUS when user not logged in', async () => { + const notLoggedInApi = { + getLoggedIn() { + return Promise.resolve(false); + }, + } as unknown as RestPluginApi; + + const service = OwnersService.getOwnersService( + notLoggedInApi, + fakeChange + ); + const user = await service.getLoggedInUser(); + assert.equal(user.role, UserRole.ANONYMOUS); + assert.equal(user.account, undefined); + }); + + test('getLoggedInUser - returns OTHER for logged in user that is NOT change owner', async () => { + const loggedUser = account(2); + const userLoggedInApi = { + getLoggedIn() { + return Promise.resolve(true); + }, + getAccount() { + return Promise.resolve(loggedUser); + }, + } as unknown as RestPluginApi; + const change = {owner: account(1)} as unknown as ChangeInfo; + + const service = OwnersService.getOwnersService(userLoggedInApi, change); + const user = await service.getLoggedInUser(); + assert.equal(user.role, UserRole.OTHER); + assert.equal(user.account, loggedUser); + }); + + test('getLoggedInUser - returns CHANGE_OWNER for logged in user that is a change owner', async () => { + const owner = account(1); + const changeOwnerLoggedInApi = { + getLoggedIn() { + return Promise.resolve(true); + }, + getAccount() { + return Promise.resolve(owner); + }, + } as unknown as RestPluginApi; + const change = {owner} as unknown as ChangeInfo; + + const service = OwnersService.getOwnersService( + changeOwnerLoggedInApi, + change + ); + const user = await service.getLoggedInUser(); + assert.equal(user.role, UserRole.CHANGE_OWNER); + assert.equal(user.account, owner); + }); + + test('getLoggedInUser - should fetch response from plugin only once', async () => { + let calls = 0; + const notLoggedInApi = { + getLoggedIn() { + calls++; + return Promise.resolve(false); + }, + } as unknown as RestPluginApi; + + const service = OwnersService.getOwnersService( + notLoggedInApi, + fakeChange + ); + + await service.getLoggedInUser(); + await service.getLoggedInUser(); + assert.equal(calls, 1); + }); + }); + + suite('files owners tests', () => { + teardown(() => { + sinon.restore(); + }); + + function setupRestApiForLoggedIn(loggedIn: boolean): RestPluginApi { + return { + getLoggedIn() { + return Promise.resolve(loggedIn); + }, + send( + _method: HttpMethod, + _url: string, + _payload?: RequestPayload, + _errFn?: ErrorCallback, + _contentType?: string + ) { + return Promise.resolve({}); + }, + } as unknown as RestPluginApi; + } + + function loggedInRestApiService(acc: number): RestPluginApi { + return { + ...setupRestApiForLoggedIn(true), + getAccount() { + return Promise.resolve(account(acc)); + }, + } as unknown as RestPluginApi; + } + + let service: OwnersService; + let getApiStub: sinon.SinonStub; + + function setup(response = {}) { + const acc = account(1); + const base_change = { + ...fakeChange, + _number: 1, + status: ChangeStatus.NEW, + project: 'test_repo', + owner: acc, + } as unknown as ChangeInfo; + const restApi = loggedInRestApiService(1); + + getApiStub = sinon.stub(restApi, 'send'); + getApiStub + .withArgs( + sinon.match.any, + `/changes/${base_change.project}~${base_change._number}/revisions/current/owners~files-owners`, + sinon.match.any, + sinon.match.any + ) + .returns(Promise.resolve(response)); + service = OwnersService.getOwnersService(restApi, base_change); + } + + test('should call getFilesOwners', async () => { + const expected = { + files: { + 'AJavaFile.java': [{name: 'Bob', id: 1000001}], + 'Aptyhonfileroot.py': [ + {name: 'John', id: 1000002}, + {name: 'Bob', id: 1000001}, + {name: 'Jack', id: 1000003}, + ], + }, + owners_labels: { + '1000002': { + Verified: 1, + 'Code-Review': 0, + }, + '1000001': { + 'Code-Review': 2, + }, + }, + }; + setup(expected); + + const response = await service.getFilesOwners(); + await flush(); + assert.equal(getApiStub.callCount, 1); + assert.equal( + deepEqual(response, {...expected} as unknown as FilesOwners), + true + ); + }); + + test('should fetch response from plugin only once', async () => { + setup(); + + await service.getFilesOwners(); + await flush(); + + await service.getFilesOwners(); + await flush(); + + assert.equal(getApiStub.callCount, 1); + }); + }); + + suite('hasOwnersSubmitRequirement tests', () => { + test('hasOwnersSubmitRequirement - should be `false` when change has no owners submit requirement', () => { + const noSubmitRequirementOrRecordChange = {} as unknown as ChangeInfo; + assert.equal( + hasOwnersSubmitRequirement(noSubmitRequirementOrRecordChange), + false + ); + }); + + test('hasOwnersSubmitRequirement - should be `false` when change has different submit requirements', () => { + const differentSubmitRequirementAndRecord = { + submit_requirements: [ + { + submittability_expression_result: { + expression: 'has:other_predicated', + }, + }, + ], + } as unknown as ChangeInfo; + assert.equal( + hasOwnersSubmitRequirement(differentSubmitRequirementAndRecord), + false + ); + }); + + test('hasOwnersSubmitRequirement - should be `true` when change has owners submit requirement', () => { + const ownersSubmitRequirement = { + submit_requirements: [ + { + submittability_expression_result: { + expression: 'has:approval_owners', + }, + }, + ], + } as unknown as ChangeInfo; + assert.equal(hasOwnersSubmitRequirement(ownersSubmitRequirement), true); + }); + + test('hasOwnersSubmitRequirement - should be `true` when change has owners submit rule', () => { + const ownersSubmitRule = { + submit_requirements: [ + { + submittability_expression_result: { + expression: + 'label:Code-Review from owners\u003downers~OwnersSubmitRequirement', + }, + }, + ], + } as unknown as ChangeInfo; + assert.equal(hasOwnersSubmitRequirement(ownersSubmitRule), true); + }); + }); +}); + +function account(id: number): AccountInfo { + return { + _account_id: id, + } as unknown as AccountInfo; +} + +function flush() { + return new Promise((resolve, _reject) => { + setTimeout(resolve, 0); + }); +}
diff --git a/owners/web/plugin.ts b/owners/web/plugin.ts new file mode 100644 index 0000000..596c8ab --- /dev/null +++ b/owners/web/plugin.ts
@@ -0,0 +1,66 @@ +/** + * @license + * Copyright (C) 2024 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 '@gerritcodereview/typescript-api/gerrit'; +import { + FILES_OWNERS_COLUMN_CONTENT, + FILES_OWNERS_COLUMN_HEADER, + FilesColumnContent, + FilesColumnHeader, +} from './gr-files'; +import { + OWNED_FILES_TAB_CONTENT, + OWNED_FILES_TAB_HEADER, + OwnedFilesTabContent, + OwnedFilesTabHeader, +} from './gr-owned-files'; + +window.Gerrit.install(plugin => { + const restApi = plugin.restApi(); + + plugin + .registerDynamicCustomComponent( + 'change-view-file-list-header-prepend', + FILES_OWNERS_COLUMN_HEADER + ) + .onAttached(view => { + (view as unknown as FilesColumnHeader).restApi = restApi; + }); + plugin + .registerDynamicCustomComponent( + 'change-view-file-list-content-prepend', + FILES_OWNERS_COLUMN_CONTENT + ) + .onAttached(view => { + (view as unknown as FilesColumnContent).restApi = restApi; + }); + plugin + .registerDynamicCustomComponent( + 'change-view-tab-header', + OWNED_FILES_TAB_HEADER + ) + .onAttached(view => { + (view as unknown as OwnedFilesTabHeader).restApi = restApi; + }); + plugin + .registerDynamicCustomComponent( + 'change-view-tab-content', + OWNED_FILES_TAB_CONTENT + ) + .onAttached(view => { + (view as unknown as OwnedFilesTabContent).restApi = restApi; + }); +});
diff --git a/owners/web/test-utils.ts b/owners/web/test-utils.ts new file mode 100644 index 0000000..8f0c8a6 --- /dev/null +++ b/owners/web/test-utils.ts
@@ -0,0 +1,21 @@ +/** + * @license + * Copyright (C) 2024 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. + */ + +export function getRandom<T>(...values: T[]): T { + const idx = Math.floor(Math.random() * values.length); + return values[idx]; +}
diff --git a/owners/web/tsconfig.json b/owners/web/tsconfig.json new file mode 100644 index 0000000..0a15ca9 --- /dev/null +++ b/owners/web/tsconfig.json
@@ -0,0 +1,15 @@ +{ + "extends": "../../tsconfig-plugins-base.json", + "compilerOptions": { + "outDir": "../../../.ts-out/plugins/owners", /* overridden by bazel */ + "lib": [ + "dom", + "dom.iterable", + "es2021", + "webworker" + ], + }, + "include": [ + "**/*.ts" + ], +}
diff --git a/owners/web/utils.ts b/owners/web/utils.ts new file mode 100644 index 0000000..81e0adb --- /dev/null +++ b/owners/web/utils.ts
@@ -0,0 +1,222 @@ +/** + * @license + * Copyright (C) 2024 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 {SpecialFilePath} from './gerrit-model'; + +export function deepEqual<T>(a: T, b: T): boolean { + if (a === b) return true; + if (a === undefined || b === undefined) return false; + if (a === null || b === null) return false; + if (a instanceof Date || b instanceof Date) { + if (!(a instanceof Date && b instanceof Date)) return false; + return a.getTime() === b.getTime(); + } + + if (a instanceof Set || b instanceof Set) { + if (!(a instanceof Set && b instanceof Set)) return false; + if (a.size !== b.size) return false; + for (const ai of a) if (!b.has(ai)) return false; + return true; + } + if (a instanceof Map || b instanceof Map) { + if (!(a instanceof Map && b instanceof Map)) return false; + if (a.size !== b.size) return false; + for (const [aKey, aValue] of a.entries()) { + if (!b.has(aKey) || !deepEqual(aValue, b.get(aKey))) return false; + } + return true; + } + + if (typeof a === 'object') { + if (typeof b !== 'object') return false; + const aObj = a as Record<string, unknown>; + const bObj = b as Record<string, unknown>; + const aKeys = Object.keys(aObj); + const bKeys = Object.keys(bObj); + if (aKeys.length !== bKeys.length) return false; + for (const key of aKeys) { + if (!deepEqual(aObj[key], bObj[key])) return false; + } + return true; + } + + return false; +} + +/** + * Queries for child element specified with a selector. Copied from Gerrit's common-util.ts. + */ +export function query<E extends Element = Element>( + el: Element | null | undefined, + selector: string +): E | undefined { + if (!el) return undefined; + if (el.shadowRoot) { + const r = el.shadowRoot.querySelector<E>(selector); + if (r) return r; + } + return el.querySelector<E>(selector) ?? undefined; +} + +/** + * Copied from Gerrit's path-list-util.ts. + */ +export function computeDisplayPath(path?: string) { + if (path === SpecialFilePath.COMMIT_MESSAGE) { + return 'Commit message'; + } else if (path === SpecialFilePath.MERGE_LIST) { + return 'Merge list'; + } + return path ?? ''; +} + +/** + * Separates a path into: + * - The part that matches another path, + * - The part that does not match the other path, + * - The file name + * + * For example: + * diffFilePaths('same/part/new/part/foo.js', 'same/part/different/foo.js'); + * yields: { + * matchingFolders: 'same/part/', + * newFolders: 'new/part/', + * fileName: 'foo.js', + * } + * + * Copied from Gerrit's string-util.ts. + */ +export function diffFilePaths(filePath: string, otherFilePath?: string) { + // Separate each string into an array of folder names + file name. + const displayPath = computeDisplayPath(filePath); + const previousFileDisplayPath = computeDisplayPath(otherFilePath); + const displayPathParts = displayPath.split('/'); + const previousFileDisplayPathParts = previousFileDisplayPath.split('/'); + + // Construct separate strings for matching folders, new folders, and file + // name. + const firstDifferencePartIndex = displayPathParts.findIndex( + (part, index) => previousFileDisplayPathParts[index] !== part + ); + const matchingSection = displayPathParts + .slice(0, firstDifferencePartIndex) + .join('/'); + const newFolderSection = displayPathParts + .slice(firstDifferencePartIndex, -1) + .join('/'); + const fileNameSection = displayPathParts[displayPathParts.length - 1]; + + // Note: folder sections need '/' appended back. + return { + matchingFolders: matchingSection.length > 0 ? `${matchingSection}/` : '', + newFolders: newFolderSection.length > 0 ? `${newFolderSection}/` : '', + fileName: fileNameSection, + }; +} + +/** + * Truncates URLs to display filename only + * Example + * // returns '.../text.html' + * util.truncatePath.('dir/text.html'); + * Example + * // returns 'text.html' + * util.truncatePath.('text.html'); + * + * Copied from Gerrit's path-list-util.ts. + */ +export function truncatePath(path: string, threshold = 1) { + const pathPieces = path.split('/'); + + if (pathPieces.length <= threshold) { + return path; + } + + const index = pathPieces.length - threshold; + // Character is an ellipsis. + return `\u2026/${pathPieces.slice(index).join('/')}`; +} + +/** + * Encodes *parts* of a URL. See inline comments below for the details. + * Note specifically that ? & = # are encoded. So this is very close to + * encodeURIComponent() with some tweaks. + * + * Copied from Gerrit's url-util.ts. + */ +export function encodeURL(url: string): string { + // gr-page decodes the entire URL, and then decodes once more the + // individual regex matching groups. It uses `decodeURIComponent()`, which + // will choke on singular `%` chars without two trailing digits. We prefer + // to not double encode *everything* (just for readaiblity and simplicity), + // but `%` *must* be double encoded. + let output = url.replaceAll('%', '%25'); // `replaceAll` function requires `es2021` hence the `lib` section was added to `tsconfig.json` + // `+` also requires double encoding, because `%2B` would be decoded to `+` + // and then replaced by ` `. + output = output.replaceAll('+', '%2B'); + + // This escapes ALL characters EXCEPT: + // A–Z a–z 0–9 - _ . ! ~ * ' ( ) + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent + output = encodeURIComponent(output); + + // If we would use `encodeURI()` instead of `encodeURIComponent()`, then we + // would also NOT encode: + // ; / ? : @ & = + $ , # + // + // That would be more readable, but for example ? and & have special meaning + // in the URL, so they must be encoded. Let's discuss all these chars and + // decide whether we have to encode them or not. + // + // ? & = # have to be encoded. Otherwise we might mess up the URL. + // + // : @ do not have to be encoded, because we are only dealing with path, + // query and fragment of the URL, not with scheme, user, host, port. + // For search queries it is much nicer to not encode those chars, think of + // searching for `owner:spearce@spearce.org`. + // + // / does not have to be encoded, because we don't care about individual path + // components. File path and repo names are so much nicer to read without / + // being encoded! + // + // + must be encoded, because we want to use it instead of %20 for spaces, see + // below. + // + // ; $ , probably don't have to be encoded, but we don't bother about them + // much, so we don't reverse the encoding here, but we don't think it would + // cause any harm, if we did. + output = output.replace(/%3A/g, ':'); + output = output.replace(/%40/g, '@'); + output = output.replace(/%2F/g, '/'); + + // gr-page replaces `+` by ` ` in addition to calling `decodeURIComponent()`. + // So we can use `+` to increase readability. + output = output.replace(/%20/g, '+'); + + return output; +} + +/** + * Function to obtain the base URL. + * + * Copied from Gerrit's url-util.ts. + */ +export function getBaseUrl(): string { + // window is not defined in service worker, therefore no CANONICAL_PATH + if (typeof window === 'undefined') return ''; + return self.CANONICAL_PATH || ''; +}