Added configuration options for notifications.
Now the following individual notifications can be turned on or off
through configuration options:
- Patch set created
- Change merged
- Comment added
Also fixed tests so that they respect config changes.
Change-Id: Ia75be0f307a4b0199caa49ce1ed25542b7563ee5
diff --git a/README.md b/README.md
index 080c4e8..4b563ec 100644
--- a/README.md
+++ b/README.md
@@ -103,3 +103,12 @@
A "dotall" enabled regular expression pattern that, when matches
against a commit message, will prevent the publishing of patchset
created event messages (defaults to an empty string).
+ publish-on-patch-set-created - boolean (true/false)
+ Whether a Slack notification should be published when a new patch set
+ is created.
+ publish-on-change-merged - boolean (true/false)
+ Whether a Slack notification should be published when a change is
+ merged.
+ publish-on-comment-added - boolean (true/false)
+ Whether a Slack notification should be published when a comment is
+ added to a review.
diff --git a/src/main/java/com/cisco/gerrit/plugins/slack/config/ProjectConfig.java b/src/main/java/com/cisco/gerrit/plugins/slack/config/ProjectConfig.java
index 6f79f5b..717037e 100644
--- a/src/main/java/com/cisco/gerrit/plugins/slack/config/ProjectConfig.java
+++ b/src/main/java/com/cisco/gerrit/plugins/slack/config/ProjectConfig.java
@@ -47,6 +47,9 @@
private String channel;
private String username;
private String ignore;
+ private boolean publishOnPatchSetCreated;
+ private boolean publishOnChangeMerged;
+ private boolean publishOnCommentAdded;
/**
* Creates a new instance of the ProjectConfig class for the given project.
@@ -82,6 +85,21 @@
ignore = configFactory.getFromProjectConfigWithInheritance(
projectNameKey, CONFIG_NAME).getString(
"ignore", "");
+
+ publishOnPatchSetCreated =
+ configFactory.getFromProjectConfigWithInheritance(
+ projectNameKey, CONFIG_NAME).getBoolean(
+ "publish-on-patch-set-created", true);
+
+ publishOnChangeMerged =
+ configFactory.getFromProjectConfigWithInheritance(
+ projectNameKey, CONFIG_NAME).getBoolean(
+ "publish-on-change-merged", true);
+
+ publishOnCommentAdded =
+ configFactory.getFromProjectConfigWithInheritance(
+ projectNameKey, CONFIG_NAME).getBoolean(
+ "publish-on-comment-added", true);
}
catch (NoSuchProjectException e)
{
@@ -114,4 +132,19 @@
{
return ignore;
}
+
+ public boolean shouldPublishOnPatchSetCreated()
+ {
+ return publishOnPatchSetCreated;
+ }
+
+ public boolean shouldPublishOnChangeMerged()
+ {
+ return publishOnChangeMerged;
+ }
+
+ public boolean shouldPublishOnCommentAdded()
+ {
+ return publishOnCommentAdded;
+ }
}
diff --git a/src/main/java/com/cisco/gerrit/plugins/slack/message/ChangeMergedMessageGenerator.java b/src/main/java/com/cisco/gerrit/plugins/slack/message/ChangeMergedMessageGenerator.java
index 4ef465a..35d885e 100644
--- a/src/main/java/com/cisco/gerrit/plugins/slack/message/ChangeMergedMessageGenerator.java
+++ b/src/main/java/com/cisco/gerrit/plugins/slack/message/ChangeMergedMessageGenerator.java
@@ -61,7 +61,7 @@
@Override
public boolean shouldPublish()
{
- return config.isEnabled();
+ return config.isEnabled() && config.shouldPublishOnChangeMerged();
}
@Override
diff --git a/src/main/java/com/cisco/gerrit/plugins/slack/message/CommentAddedMessageGenerator.java b/src/main/java/com/cisco/gerrit/plugins/slack/message/CommentAddedMessageGenerator.java
index 90f9ff1..9c21abe 100644
--- a/src/main/java/com/cisco/gerrit/plugins/slack/message/CommentAddedMessageGenerator.java
+++ b/src/main/java/com/cisco/gerrit/plugins/slack/message/CommentAddedMessageGenerator.java
@@ -62,7 +62,7 @@
@Override
public boolean shouldPublish()
{
- return config.isEnabled();
+ return config.isEnabled() && config.shouldPublishOnCommentAdded();
}
@Override
diff --git a/src/main/java/com/cisco/gerrit/plugins/slack/message/PatchSetCreatedMessageGenerator.java b/src/main/java/com/cisco/gerrit/plugins/slack/message/PatchSetCreatedMessageGenerator.java
index 4147dd7..2d7ddf5 100644
--- a/src/main/java/com/cisco/gerrit/plugins/slack/message/PatchSetCreatedMessageGenerator.java
+++ b/src/main/java/com/cisco/gerrit/plugins/slack/message/PatchSetCreatedMessageGenerator.java
@@ -62,7 +62,10 @@
@Override
public boolean shouldPublish()
{
- if(!config.isEnabled()) return false;
+ if (!config.isEnabled() || !config.shouldPublishOnPatchSetCreated())
+ {
+ return false;
+ }
boolean result;
result = true;
diff --git a/src/test/java/com/cisco/gerrit/plugins/slack/config/ProjectConfigTest.java b/src/test/java/com/cisco/gerrit/plugins/slack/config/ProjectConfigTest.java
index 3a89b3b..48c92c8 100644
--- a/src/test/java/com/cisco/gerrit/plugins/slack/config/ProjectConfigTest.java
+++ b/src/test/java/com/cisco/gerrit/plugins/slack/config/ProjectConfigTest.java
@@ -78,6 +78,12 @@
.thenReturn("test-user");
when(mockPluginConfig.getString("ignore", ""))
.thenReturn("^WIP.*");
+ when(mockPluginConfig.getBoolean("publish-on-patch-set-created", true))
+ .thenReturn(true);
+ when(mockPluginConfig.getBoolean("publish-on-change-merged", true))
+ .thenReturn(true);
+ when(mockPluginConfig.getBoolean("publish-on-comment-added", true))
+ .thenReturn(true);
config = new ProjectConfig(mockConfigFactory, PROJECT_NAME);
}
@@ -105,4 +111,22 @@
{
assertThat(config.getUsername(), is(equalTo("test-user")));
}
+
+ @Test
+ public void testShouldPublishOnPatchSetCreated() throws Exception
+ {
+ assertThat(config.shouldPublishOnPatchSetCreated(), is(equalTo(true)));
+ }
+
+ @Test
+ public void testShouldPublishOnChangeMerged() throws Exception
+ {
+ assertThat(config.shouldPublishOnChangeMerged(), is(equalTo(true)));
+ }
+
+ @Test
+ public void testShouldPublishOnCommentAdded() throws Exception
+ {
+ assertThat(config.shouldPublishOnCommentAdded(), is(equalTo(true)));
+ }
}
diff --git a/src/test/java/com/cisco/gerrit/plugins/slack/message/ChangeMergedMessageGeneratorTest.java b/src/test/java/com/cisco/gerrit/plugins/slack/message/ChangeMergedMessageGeneratorTest.java
index a6851ca..8322b46 100644
--- a/src/test/java/com/cisco/gerrit/plugins/slack/message/ChangeMergedMessageGeneratorTest.java
+++ b/src/test/java/com/cisco/gerrit/plugins/slack/message/ChangeMergedMessageGeneratorTest.java
@@ -62,14 +62,15 @@
private AccountAttribute mockAccount = mock(AccountAttribute.class);
private ChangeAttribute mockChange = mock(ChangeAttribute.class);
- private ProjectConfig config;
-
@Before
public void setup() throws Exception
{
PowerMockito.mockStatic(Project.NameKey.class);
when(Project.NameKey.parse(PROJECT_NAME)).thenReturn(mockNameKey);
+ }
+ private ProjectConfig getConfig(boolean publishOnChangeMerged) throws Exception
+ {
Project.NameKey projectNameKey;
projectNameKey = Project.NameKey.parse(PROJECT_NAME);
@@ -88,13 +89,20 @@
.thenReturn("testuser");
when(mockPluginConfig.getString("ignore", ""))
.thenReturn("^WIP.*");
+ when(mockPluginConfig.getBoolean("publish-on-change-merged", true))
+ .thenReturn(publishOnChangeMerged);
- config = new ProjectConfig(mockConfigFactory, PROJECT_NAME);
+ return new ProjectConfig(mockConfigFactory, PROJECT_NAME);
+ }
+
+ private ProjectConfig getConfig() throws Exception {
+ return getConfig(true /* publishOnChangeMerged */);
}
@Test
public void factoryCreatesExpectedType() throws Exception
{
+ ProjectConfig config = getConfig();
MessageGenerator messageGenerator;
messageGenerator = MessageGeneratorFactory.newInstance(
mockEvent, config);
@@ -107,6 +115,7 @@
public void publishesWhenExpected() throws Exception
{
// Setup mocks
+ ProjectConfig config = getConfig();
mockEvent.change = Suppliers.ofInstance(mockChange);
mockChange.commitMessage = "This is a title\nAnd a the body.";
@@ -119,9 +128,10 @@
}
@Test
- public void doesNotPublishWhenExpected() throws Exception
+ public void publishesWhenMessageMatchesIgnore() throws Exception
{
// Setup mocks
+ ProjectConfig config = getConfig();
mockEvent.change = Suppliers.ofInstance(mockChange);
mockChange.commitMessage = "WIP:This is a title\nAnd a the body.";
@@ -134,8 +144,25 @@
}
@Test
+ public void doesNotPublishWhenTurnedOff() throws Exception
+ {
+ // Setup mocks
+ ProjectConfig config = getConfig(false /* publishOnChangeMerged */);
+ mockEvent.change = Suppliers.ofInstance(mockChange);
+ mockChange.commitMessage = "This is a title\nAnd a the body.";
+
+ // Test
+ MessageGenerator messageGenerator;
+ messageGenerator = MessageGeneratorFactory.newInstance(
+ mockEvent, config);
+
+ assertThat(messageGenerator.shouldPublish(), is(false));
+ }
+
+ @Test
public void handlesInvalidIgnorePatterns() throws Exception
{
+ ProjectConfig config = getConfig();
when(mockPluginConfig.getString("ignore", ""))
.thenReturn(null);
@@ -151,6 +178,7 @@
public void generatesExpectedMessage() throws Exception
{
// Setup mocks
+ ProjectConfig config = getConfig();
mockEvent.change = Suppliers.ofInstance(mockChange);
mockEvent.submitter = Suppliers.ofInstance(mockAccount);
diff --git a/src/test/java/com/cisco/gerrit/plugins/slack/message/CommentAddedMessageGeneratorTest.java b/src/test/java/com/cisco/gerrit/plugins/slack/message/CommentAddedMessageGeneratorTest.java
index c1cf569..a29d215 100644
--- a/src/test/java/com/cisco/gerrit/plugins/slack/message/CommentAddedMessageGeneratorTest.java
+++ b/src/test/java/com/cisco/gerrit/plugins/slack/message/CommentAddedMessageGeneratorTest.java
@@ -63,14 +63,15 @@
private ChangeAttribute mockChange = mock(ChangeAttribute.class);
private AccountAttribute mockOwner = mock(AccountAttribute.class);
- private ProjectConfig config;
-
@Before
public void setup() throws Exception
{
PowerMockito.mockStatic(Project.NameKey.class);
when(Project.NameKey.parse(PROJECT_NAME)).thenReturn(mockNameKey);
+ }
+ private ProjectConfig getConfig(boolean publishOnCommentAdded) throws Exception
+ {
Project.NameKey projectNameKey;
projectNameKey = Project.NameKey.parse(PROJECT_NAME);
@@ -89,13 +90,21 @@
.thenReturn("testuser");
when(mockPluginConfig.getString("ignore", ""))
.thenReturn("^WIP.*");
+ when(mockPluginConfig.getBoolean("publish-on-comment-added", true))
+ .thenReturn(publishOnCommentAdded);
- config = new ProjectConfig(mockConfigFactory, PROJECT_NAME);
+ return new ProjectConfig(mockConfigFactory, PROJECT_NAME);
+ }
+
+ private ProjectConfig getConfig() throws Exception
+ {
+ return getConfig(true /* publishOnCommentAdded */);
}
@Test
public void factoryCreatesExpectedType() throws Exception
{
+ ProjectConfig config = getConfig();
MessageGenerator messageGenerator;
messageGenerator = MessageGeneratorFactory.newInstance(
mockEvent, config);
@@ -108,6 +117,7 @@
public void publishesWhenExpected() throws Exception
{
// Setup mocks
+ ProjectConfig config = getConfig();
mockEvent.comment = "This is a title\nAnd a the body.";
// Test
@@ -119,9 +129,10 @@
}
@Test
- public void doesNotPublishWhenExpected() throws Exception
+ public void publishesWhenMessageMatchesIgnore() throws Exception
{
// Setup mocks
+ ProjectConfig config = getConfig();
mockEvent.comment = "WIP:This is a title\nAnd a the body.";
// Test
@@ -133,8 +144,24 @@
}
@Test
+ public void doesNotPublishWhenTurnedOff() throws Exception
+ {
+ // Setup mocks
+ ProjectConfig config = getConfig(false /* publishOnCommentAdded */);
+ mockEvent.comment = "This is a title\nAnd a the body.";
+
+ // Test
+ MessageGenerator messageGenerator;
+ messageGenerator = MessageGeneratorFactory.newInstance(
+ mockEvent, config);
+
+ assertThat(messageGenerator.shouldPublish(), is(false));
+ }
+
+ @Test
public void handlesInvalidIgnorePatterns() throws Exception
{
+ ProjectConfig config = getConfig();
when(mockPluginConfig.getString("ignore", ""))
.thenReturn(null);
@@ -150,6 +177,7 @@
public void generatesExpectedMessage() throws Exception
{
// Setup mocks
+ ProjectConfig config = getConfig();
mockEvent.change = Suppliers.ofInstance(mockChange);
mockEvent.author = Suppliers.ofInstance(mockAccount);
@@ -184,6 +212,7 @@
public void generatesExpectedMessageForLongComment() throws Exception
{
// Setup mocks
+ ProjectConfig config = getConfig();
mockEvent.change = Suppliers.ofInstance(mockChange);
mockEvent.author = Suppliers.ofInstance(mockAccount);
diff --git a/src/test/java/com/cisco/gerrit/plugins/slack/message/PatchSetCreatedMessageGeneratorTest.java b/src/test/java/com/cisco/gerrit/plugins/slack/message/PatchSetCreatedMessageGeneratorTest.java
index 9f4535c..1e32414 100644
--- a/src/test/java/com/cisco/gerrit/plugins/slack/message/PatchSetCreatedMessageGeneratorTest.java
+++ b/src/test/java/com/cisco/gerrit/plugins/slack/message/PatchSetCreatedMessageGeneratorTest.java
@@ -60,14 +60,18 @@
private AccountAttribute mockAccount = mock(AccountAttribute.class);
private ChangeAttribute mockChange = mock(ChangeAttribute.class);
- private ProjectConfig config;
-
@Before
public void setup() throws Exception
{
PowerMockito.mockStatic(Project.NameKey.class);
when(Project.NameKey.parse(PROJECT_NAME)).thenReturn(mockNameKey);
+ }
+ private ProjectConfig getConfig(
+ String ignore,
+ boolean publishOnPatchSetCreated)
+ throws Exception
+ {
Project.NameKey projectNameKey;
projectNameKey = Project.NameKey.parse(PROJECT_NAME);
@@ -85,14 +89,32 @@
when(mockPluginConfig.getString("username", "gerrit"))
.thenReturn("testuser");
when(mockPluginConfig.getString("ignore", ""))
- .thenReturn("^WIP.*");
+ .thenReturn(ignore);
+ when(mockPluginConfig.getBoolean("publish-on-patch-set-created", true))
+ .thenReturn(publishOnPatchSetCreated);
- config = new ProjectConfig(mockConfigFactory, PROJECT_NAME);
+ return new ProjectConfig(mockConfigFactory, PROJECT_NAME);
+ }
+
+ private ProjectConfig getConfig(String ignore) throws Exception
+ {
+ return getConfig(ignore, true /* publishOnPatchSetCreated */);
+ }
+
+ private ProjectConfig getConfig(boolean publishOnPatchSetCreated) throws Exception
+ {
+ return getConfig("^WIP.*", publishOnPatchSetCreated);
+ }
+
+ private ProjectConfig getConfig() throws Exception
+ {
+ return getConfig("^WIP.*", true /* publishOnPatchSetCreated */);
}
@Test
public void factoryCreatesExpectedType() throws Exception
{
+ ProjectConfig config = getConfig();
MessageGenerator messageGenerator;
messageGenerator = MessageGeneratorFactory.newInstance(
mockEvent, config);
@@ -105,6 +127,7 @@
public void publishesWhenExpected() throws Exception
{
// Setup mocks
+ ProjectConfig config = getConfig();
mockEvent.change = Suppliers.ofInstance(mockChange);
mockChange.commitMessage = "This is a title\nAnd a the body.";
@@ -117,9 +140,10 @@
}
@Test
- public void doesNotPublishWhenExpected() throws Exception
+ public void doesNotPublishWhenMessageMatchesIgnore() throws Exception
{
// Setup mocks
+ ProjectConfig config = getConfig();
mockEvent.change = Suppliers.ofInstance(mockChange);
mockChange.commitMessage = "WIP-This is a title\nAnd a the body.";
@@ -132,10 +156,25 @@
}
@Test
+ public void doesNotPublishWhenTurnedOff() throws Exception
+ {
+ // Setup mocks
+ ProjectConfig config = getConfig(false /* publishOnPatchSetCreated */);
+ mockEvent.change = Suppliers.ofInstance(mockChange);
+ mockChange.commitMessage = "This is a title\nAnd a the body.";
+
+ // Test
+ MessageGenerator messageGenerator;
+ messageGenerator = MessageGeneratorFactory.newInstance(
+ mockEvent, config);
+
+ assertThat(messageGenerator.shouldPublish(), is(false));
+ }
+
+ @Test
public void handlesInvalidIgnorePatterns() throws Exception
{
- when(mockPluginConfig.getString("ignore", ""))
- .thenReturn(null);
+ ProjectConfig config = getConfig(null /* ignore */);
// Test
MessageGenerator messageGenerator;
@@ -149,6 +188,7 @@
public void generatesExpectedMessage() throws Exception
{
// Setup mocks
+ ProjectConfig config = getConfig();
mockEvent.change = Suppliers.ofInstance(mockChange);
mockEvent.uploader = Suppliers.ofInstance(mockAccount);
