Allow to configure a URL that explains how to get a code owner override
Sometimes a change needs to be submitted urgently and there is no time
to gather the required code owner approvals. In this case a code owner
override can be applied to make the change submittable. Usually the
permission to apply an override is limited to a small set of super
users that may not be known to the user needing a code owner override.
To allow users to find a person who can grant a code owner override we
allow to configure a URL to a documentation page where it's explained
how an override can be retrieved. This documentation page is specific
for a host or project. The frontend gets the URL via the
GetCodeOwnerProjectConfig REST endpoint and can display a link or icon
for it.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ice075c75f379491ded97d1f0dd8d06a942cd4188
diff --git a/java/com/google/gerrit/plugins/codeowners/api/GeneralInfo.java b/java/com/google/gerrit/plugins/codeowners/api/GeneralInfo.java
index c6cb2c2..8562057 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/GeneralInfo.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/GeneralInfo.java
@@ -37,4 +37,10 @@
* <p>Not set, if {@code false}.
*/
public Boolean implicitApprovals;
+
+ /**
+ * Optional URL for a page that provides project/host-specific information about how to request a
+ * code owner override.
+ */
+ public String overrideInfoUrl;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfiguration.java b/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfiguration.java
index 1c6f591..236b12e 100644
--- a/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfiguration.java
+++ b/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfiguration.java
@@ -154,6 +154,17 @@
}
/**
+ * Gets the override info URL that is configured for the given project.
+ *
+ * @param project the project for which the configured override info URL should be returned
+ * @return the override info URL that is configured for the given project
+ */
+ public Optional<String> getOverrideInfoUrl(Project.NameKey project) {
+ requireNonNull(project, "project");
+ return generalConfig.getOverrideInfoUrl(getPluginConfig(project));
+ }
+
+ /**
* Returns the email domains that are allowed to be used for code owners.
*
* @return the email domains that are allowed to be used for code owners, an empty set if all
diff --git a/java/com/google/gerrit/plugins/codeowners/config/GeneralConfig.java b/java/com/google/gerrit/plugins/codeowners/config/GeneralConfig.java
index f764e71..284413c 100644
--- a/java/com/google/gerrit/plugins/codeowners/config/GeneralConfig.java
+++ b/java/com/google/gerrit/plugins/codeowners/config/GeneralConfig.java
@@ -69,6 +69,8 @@
@VisibleForTesting
public static final String KEY_ENABLE_IMPLICIT_APPROVALS = "enableImplicitApprovals";
+ @VisibleForTesting public static final String KEY_OVERRIDE_INFO_URL = "overrideInfoUrl";
+
private final String pluginName;
private final PluginConfig pluginConfigFromGerritConfig;
@@ -275,4 +277,25 @@
.map(CodeOwnerReference::create)
.collect(toImmutableSet());
}
+
+ /**
+ * Gets an URL that leads to an information page about overrides.
+ *
+ * <p>The URL is retrieved from the given plugin config, with fallback to the {@code
+ * gerrit.config}.
+ *
+ * @param pluginConfig the plugin config from which the override info URL should be read.
+ * @return URL that leads to an information page about overrides, {@link Optional#empty()} if no
+ * such URL is configured
+ */
+ Optional<String> getOverrideInfoUrl(Config pluginConfig) {
+ requireNonNull(pluginConfig, "pluginConfig");
+
+ String fileExtension = pluginConfig.getString(SECTION_CODE_OWNERS, null, KEY_OVERRIDE_INFO_URL);
+ if (fileExtension != null) {
+ return Optional.of(fileExtension);
+ }
+
+ return Optional.ofNullable(pluginConfigFromGerritConfig.getString(KEY_OVERRIDE_INFO_URL));
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
index 4349d05..846b1c2 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
@@ -76,6 +76,8 @@
codeOwnersPluginConfiguration.getMergeCommitStrategy(projectName);
generalInfo.implicitApprovals =
codeOwnersPluginConfiguration.areImplicitApprovalsEnabled(projectName) ? true : null;
+ generalInfo.overrideInfoUrl =
+ codeOwnersPluginConfiguration.getOverrideInfoUrl(projectName).orElse(null);
return generalInfo;
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerProjectConfigIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerProjectConfigIT.java
index 8f9312b..5b879dc 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerProjectConfigIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerProjectConfigIT.java
@@ -87,6 +87,7 @@
assertThat(codeOwnerProjectConfigInfo.general.mergeCommitStrategy)
.isEqualTo(MergeCommitStrategy.ALL_CHANGED_FILES);
assertThat(codeOwnerProjectConfigInfo.general.implicitApprovals).isNull();
+ assertThat(codeOwnerProjectConfigInfo.general.overrideInfoUrl).isNull();
assertThat(codeOwnerProjectConfigInfo.status.disabled).isNull();
assertThat(codeOwnerProjectConfigInfo.status.disabledBranches).isNull();
assertThat(codeOwnerProjectConfigInfo.backend.idsByBranch).isNull();
@@ -109,6 +110,15 @@
}
@Test
+ public void getConfigWithConfiguredOverrideInfoUrl() throws Exception {
+ configureOverrideInfoUrl(project, "http://foo.example.com");
+ CodeOwnerProjectConfigInfo codeOwnerProjectConfigInfo =
+ projectCodeOwnersApiFactory.project(project).getConfig();
+ assertThat(codeOwnerProjectConfigInfo.general.overrideInfoUrl)
+ .isEqualTo("http://foo.example.com");
+ }
+
+ @Test
public void getConfigWithConfiguredMergeCommitStrategy() throws Exception {
configureMergeCommitStrategy(project, MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION);
CodeOwnerProjectConfigInfo codeOwnerProjectConfigInfo =
@@ -224,6 +234,11 @@
setConfig(project, null, GeneralConfig.KEY_FILE_EXTENSION, fileExtension);
}
+ private void configureOverrideInfoUrl(Project.NameKey project, String overrideInfoUrl)
+ throws Exception {
+ setConfig(project, null, GeneralConfig.KEY_OVERRIDE_INFO_URL, overrideInfoUrl);
+ }
+
private void configureMergeCommitStrategy(
Project.NameKey project, MergeCommitStrategy mergeCommitStrategy) throws Exception {
setConfig(project, null, GeneralConfig.KEY_MERGE_COMMIT_STRATEGY, mergeCommitStrategy.name());
diff --git a/javatests/com/google/gerrit/plugins/codeowners/config/GeneralConfigTest.java b/javatests/com/google/gerrit/plugins/codeowners/config/GeneralConfigTest.java
index 3037cd4..c4b8a8f 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/config/GeneralConfigTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/config/GeneralConfigTest.java
@@ -21,6 +21,7 @@
import static com.google.gerrit.plugins.codeowners.config.GeneralConfig.KEY_FILE_EXTENSION;
import static com.google.gerrit.plugins.codeowners.config.GeneralConfig.KEY_GLOBAL_CODE_OWNER;
import static com.google.gerrit.plugins.codeowners.config.GeneralConfig.KEY_MERGE_COMMIT_STRATEGY;
+import static com.google.gerrit.plugins.codeowners.config.GeneralConfig.KEY_OVERRIDE_INFO_URL;
import static com.google.gerrit.plugins.codeowners.config.GeneralConfig.KEY_READ_ONLY;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.truth.OptionalSubject.assertThat;
@@ -344,4 +345,34 @@
assertThat(generalConfig.getGlobalCodeOwners(cfg))
.containsExactly(CodeOwnerReference.create("bot3@example.com"));
}
+
+ @Test
+ public void cannotGetOverrideInfoUrlForNullPluginConfig() throws Exception {
+ NullPointerException npe =
+ assertThrows(NullPointerException.class, () -> generalConfig.getOverrideInfoUrl(null));
+ assertThat(npe).hasMessageThat().isEqualTo("pluginConfig");
+ }
+
+ @Test
+ public void noOverrideInfoUrlConfigured() throws Exception {
+ assertThat(generalConfig.getOverrideInfoUrl(new Config())).isEmpty();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideInfoUrl", value = "http://foo.example.com")
+ public void overrideInfoIsRetrievedFromGerritConfigIfNotSpecifiedOnProjectLevel()
+ throws Exception {
+ assertThat(generalConfig.getOverrideInfoUrl(new Config()))
+ .value()
+ .isEqualTo("http://foo.example.com");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideInfoUrl", value = "http://foo.example.com")
+ public void overrideInfoUrlInPluginConfigOverridesOverrideInfoUrlInGerritConfig()
+ throws Exception {
+ Config cfg = new Config();
+ cfg.setString(SECTION_CODE_OWNERS, null, KEY_OVERRIDE_INFO_URL, "http://bar.example.com");
+ assertThat(generalConfig.getOverrideInfoUrl(cfg)).value().isEqualTo("http://bar.example.com");
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
index 4313367..6300a99 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
@@ -144,6 +144,8 @@
when(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "stable-2.10")))
.thenReturn(protoBackend);
when(codeOwnersPluginConfiguration.getFileExtension(project)).thenReturn(Optional.of("foo"));
+ when(codeOwnersPluginConfiguration.getOverrideInfoUrl(project))
+ .thenReturn(Optional.of("http://foo.example.com"));
when(codeOwnersPluginConfiguration.getMergeCommitStrategy(project))
.thenReturn(MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION);
when(codeOwnersPluginConfiguration.areImplicitApprovalsEnabled(project)).thenReturn(true);
@@ -160,6 +162,8 @@
assertThat(codeOwnerProjectConfigInfo.status.disabled).isTrue();
assertThat(codeOwnerProjectConfigInfo.status.disabledBranches).isNull();
assertThat(codeOwnerProjectConfigInfo.general.fileExtension).isEqualTo("foo");
+ assertThat(codeOwnerProjectConfigInfo.general.overrideInfoUrl)
+ .isEqualTo("http://foo.example.com");
assertThat(codeOwnerProjectConfigInfo.general.mergeCommitStrategy)
.isEqualTo(MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION);
assertThat(codeOwnerProjectConfigInfo.general.implicitApprovals).isTrue();
diff --git a/resources/Documentation/config.md b/resources/Documentation/config.md
index 6b5db3b..905cadf 100644
--- a/resources/Documentation/config.md
+++ b/resources/Documentation/config.md
@@ -61,6 +61,16 @@
`@PLUGIN@.config`.\
By default unset (no file extension is used).
+<a id="pluginCodeOwnersOverrideInfoUrl">plugin.@PLUGIN@.overrideInfoUrl</a>
+: A URL for a page that provides host-specific information about how to
+ request a code owner override.\
+ The frontend displays a link to this page on the change screen so that
+ users can discover the override instructions easily.\
+ Can be overridden per project by setting
+ [codeOwners.overrideInfoUrl](#codeOwnersFileExtension) in
+ `@PLUGIN@.config`.\
+ By default unset (no override info URL).
+
<a id="pluginCodeOwnersEnableImplicitApprovals">plugin.@PLUGIN@.enableImplictApprovals</a>
: Whether an implicit code owner approval from the last uploader is
assumed.\
@@ -264,6 +274,18 @@
[plugin.@PLUGIN@.fileExtension](#pluginCodeOwnersFileExtension) in
`gerrit.config` is used.
+<a id="codeOwnersOverrideInfoUrl">codeOwners.overrideInfoUrl</a>
+: A URL for a page that provides project-specific information about how to
+ request a code owner override.\
+ The frontend displays a link to this page on the change screen so that
+ users can discover the override instructions easily.\
+ Overrides the global setting
+ [plugin.@PLUGIN@.overrideInfoUrl](#pluginCodeOwnersOverrideInfoUrl) in
+ `gerrit.config`.\
+ If not set, the global setting
+ [plugin.@PLUGIN@.overrideInfoUrl](#pluginCodeOwnersOverrideInfoUrl) in
+ `gerrit.config` is used.
+
<a id="codeOwnersEnableImplicitApprovals">codeOwners.enableImplicitApprovals</a>
: Whether an implicit code owner approval from the last uploader is
assumed.\
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index fdcd4da..335daae 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -545,6 +545,7 @@
| `file_extension` | optional | The file extension that is used for the code owner config files in this project. Not set if no file extension is used.
| `merge_commit_strategy` || Strategy that defines for merge commits which files require code owner approvals. Can be `ALL_CHANGED_FILES` or `FILES_WITH_CONFLICT_RESOLUTION` (see [mergeCommitStrategy](config.html#pluginCodeOwnersMergeCommitStrategy) for an explanation of these values).
| `implicit_approvals` | optional | Whether an implicit code owner approval from the last uploader is assumed (see [enableImplicitApprovals](config.html#pluginCodeOwnersEnableImplicitApprovals) for details). When unset, `false`.
+| `override_info_url` | optional | Optional URL for a page that provides project/host-specific information about how to request a code owner override.
### <a id="path-code-owner-status-info"> PathCodeOwnerStatusInfo
The `PathCodeOwnerStatusInfo` entity describes the code owner status for a path
