From b7a52c267be5197e270db57986fd1417409f4eb3 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 18 Jul 2013 11:22:25 +0200 Subject: [PATCH] Apply comment action only on modified issues --- .../resources/org/sonar/l10n/core.properties | 1 + .../server/issue/IssueBulkChangeQuery.java | 11 +++- .../server/issue/IssueBulkChangeService.java | 10 ++- .../views/issues/_bulk_change_form.html.erb | 3 +- .../issue/IssueBulkChangeQueryTest.java | 3 +- .../issue/IssueBulkChangeServiceTest.java | 62 ++++++++++++++++++- 6 files changed, 84 insertions(+), 6 deletions(-) diff --git a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties index 3660170fa62..400f3583b8f 100644 --- a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties +++ b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties @@ -565,6 +565,7 @@ issue_filter.manage.shared_filters=Shared Filters #------------------------------------------------------------------------------ issue_bulk_change.form.title=Change {0} issues +issue_bulk_change.comment.help=This comment will be applied only to issues that will effectively be modified issue_bulk_change.max_issues_reached=As too many issues have been selected, only the first {0} issues will be updated. issue_bulk_change.x_issues={0} issues issue_bulk_change.error.empty_issues=Issues must not be empty diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java index 6f20133a3c3..b1e5133d90a 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java @@ -46,6 +46,7 @@ public class IssueBulkChangeQuery { private List issues; private List actions; + private boolean hasComment; Map> propertiesByActions = new HashMap>(); @@ -72,7 +73,7 @@ public class IssueBulkChangeQuery { propertiesByActions.put(action, actionProperties); } if (!Strings.isNullOrEmpty(comment)) { - actions.add(CommentAction.KEY); + hasComment = true; Map commentMap = newHashMap(); commentMap.put(CommentAction.COMMENT_PROPERTY, comment); propertiesByActions.put(CommentAction.KEY, commentMap); @@ -95,10 +96,18 @@ public class IssueBulkChangeQuery { return issues; } + /** + * The list of actions to apply + * Note that even if a comment has been added, this list will NOT contains the comment action + */ public List actions() { return actions; } + public boolean hasComment(){ + return hasComment; + } + public Map properties(String action) { return propertiesByActions.get(action); } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java index 7793ad6630d..b6a0cff6ba2 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java @@ -82,7 +82,7 @@ public class IssueBulkChangeService { ActionContext actionContext = new ActionContext(issue, issueChangeContext); for (Action action : bulkActions) { try { - if (action.supports(issue) && action.execute(issueBulkChangeQuery.properties(action.key()), actionContext)) { + if (applyAction(action, actionContext, issueBulkChangeQuery)) { result.addIssueChanged(issue); } else { result.addIssueNotChanged(issue); @@ -93,6 +93,10 @@ public class IssueBulkChangeService { } } if (result.issuesChanged().contains(issue)) { + // Apply comment action only on changed issues + if (issueBulkChangeQuery.hasComment()) { + applyAction(getAction(CommentAction.KEY), actionContext, issueBulkChangeQuery); + } issueStorage.save((DefaultIssue) issue); issueNotifications.sendChanges((DefaultIssue) issue, issueChangeContext, issueQueryResult); } @@ -101,6 +105,10 @@ public class IssueBulkChangeService { return result; } + private boolean applyAction(Action action, ActionContext actionContext, IssueBulkChangeQuery issueBulkChangeQuery){ + return action.supports(actionContext.issue()) && action.execute(issueBulkChangeQuery.properties(action.key()), actionContext); + } + @CheckForNull private Action getAction(final String actionKey) { return Iterables.find(actions, new Predicate() { diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb index f6287c13517..8063e1d6464 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb @@ -90,7 +90,8 @@
diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java index 0176a342d86..e58a3fedb96 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java @@ -80,7 +80,8 @@ public class IssueBulkChangeQueryTest { params.put("assign.assignee", "arthur"); IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params, "My comment for bulk change"); - assertThat(issueBulkChangeQuery.actions()).containsOnly("assign", "comment"); + assertThat(issueBulkChangeQuery.hasComment()).isTrue(); + assertThat(issueBulkChangeQuery.actions()).containsOnly("assign"); assertThat(issueBulkChangeQuery.properties("comment").get("comment")).isEqualTo("My comment for bulk change"); } diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java index 4ebb6c998c3..70281632224 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java @@ -92,7 +92,65 @@ public class IssueBulkChangeServiceTest { } @Test - public void should_do_sve_once_per_issue() { + public void should_execute_bulk_change_with_comment() { + Map properties = newHashMap(); + properties.put("issues", "ABCD"); + properties.put("actions", "assign"); + properties.put("assign.assignee", "fred"); + + Action commentAction = mock(Action.class); + when(commentAction.key()).thenReturn("comment"); + when(commentAction.supports(any(Issue.class))).thenReturn(true); + when(commentAction.verify(anyMap(), anyListOf(Issue.class), any(UserSession.class))).thenReturn(true); + when(commentAction.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true); + actions.add(commentAction); + actions.add(new MockAction("assign")); + + IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, "my comment"); + IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession); + assertThat(result.issuesChanged()).hasSize(1); + assertThat(result.issuesNotChanged()).isEmpty(); + + verify(commentAction).execute(anyMap(), any(IssueBulkChangeService.ActionContext.class)); + verify(issueStorage).save(eq(issue)); + } + + @Test + public void should_execute_bulk_change_with_comment_only_on_changed_issues() { + when(issueQueryResult.issues()).thenReturn(newArrayList((Issue) new DefaultIssue().setKey("ABCD"), new DefaultIssue().setKey("EFGH"))); + + Map properties = newHashMap(); + properties.put("issues", "ABCD,EFGH"); + properties.put("actions", "assign"); + properties.put("assign.assignee", "fred"); + + Action commentAction = mock(Action.class); + when(commentAction.key()).thenReturn("comment"); + when(commentAction.supports(any(Issue.class))).thenReturn(true); + when(commentAction.verify(anyMap(), anyListOf(Issue.class), any(UserSession.class))).thenReturn(true); + when(commentAction.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true); + actions.add(commentAction); + + // This action will only be executed on the first issue, not the second + Action assignAction = mock(Action.class); + when(assignAction.key()).thenReturn("assign"); + when(assignAction.supports(any(Issue.class))).thenReturn(true).thenReturn(false); + when(assignAction.verify(anyMap(), anyListOf(Issue.class), any(UserSession.class))).thenReturn(true); + when(assignAction.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true).thenReturn(false); + actions.add(assignAction); + + IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, "my comment"); + IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession); + assertThat(result.issuesChanged()).hasSize(1); + assertThat(result.issuesNotChanged()).hasSize(1); + + // Only one issue will receive the comment + verify(assignAction, times(1)).execute(anyMap(), any(IssueBulkChangeService.ActionContext.class)); + verify(issueStorage).save(eq(issue)); + } + + @Test + public void should_save_once_per_issue() { Map properties = newHashMap(); properties.put("issues", "ABCD"); properties.put("actions", "assign,set_severity"); @@ -174,11 +232,11 @@ public class IssueBulkChangeServiceTest { properties.put("assign.assignee", "fred"); Action action = mock(Action.class); - actions.add(action); when(action.key()).thenReturn("assign"); when(action.supports(any(Issue.class))).thenReturn(true); when(action.verify(anyMap(), anyListOf(Issue.class), any(UserSession.class))).thenReturn(true); doThrow(new RuntimeException("Error")).when(action).execute(anyMap(), any(IssueBulkChangeService.ActionContext.class)); + actions.add(action); IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties); IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession); -- 2.39.5