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 (cherry picked from commit 79e7c347083cfca1c7eb722086393221ec25d72b)
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);