From 4352216293428ee2b3e3d492ec355f0c7f69aa75 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 11 Oct 2018 14:52:14 +0200 Subject: [PATCH] SONAR-11318 Ignore external issues on bulk change * Refactor BulkChangeActionTest by removing some code from before() method * SONAR-11318 Silently ignore external issues on bulk change * SONAR-11318 Move external issue check from DoTransitionAction to TransitionService --- .../sonar/server/issue/TransitionAction.java | 4 - .../sonar/server/issue/TransitionService.java | 6 + .../server/issue/ws/DoTransitionAction.java | 2 - .../server/issue/TransitionActionTest.java | 13 - .../server/issue/TransitionServiceTest.java | 71 ++- .../server/issue/ws/BulkChangeActionTest.java | 409 ++++++++++-------- .../issue/ws/DoTransitionActionTest.java | 114 +++-- 7 files changed, 339 insertions(+), 280 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java index 6bb6854525c..444b105017b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java @@ -21,7 +21,6 @@ package org.sonar.server.issue; import java.util.Collection; import java.util.Map; - import org.sonar.api.server.ServerSide; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.util.stream.MoreCollectors; @@ -63,9 +62,6 @@ public class TransitionAction extends Action { } private boolean canExecuteTransition(DefaultIssue issue, String transitionKey) { - - checkArgument(!issue.isFromExternalRuleEngine(), "No transition allowed on issue from externally define rule"); - return transitionService.listTransitions(issue) .stream() .map(Transition::key) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionService.java index cfb01d441b1..4be0ba37b7a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionService.java @@ -19,6 +19,7 @@ */ package org.sonar.server.issue; +import java.util.Collections; import java.util.List; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; @@ -27,6 +28,7 @@ import org.sonar.server.issue.workflow.IssueWorkflow; import org.sonar.server.issue.workflow.Transition; import org.sonar.server.user.UserSession; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; import static org.apache.commons.lang.StringUtils.isBlank; import static org.apache.commons.lang.StringUtils.isNotBlank; @@ -45,6 +47,9 @@ public class TransitionService { } public List listTransitions(DefaultIssue issue) { + if (issue.isFromExternalRuleEngine()){ + return Collections.emptyList(); + } String projectUuid = requireNonNull(issue.projectUuid()); return workflow.outTransitions(issue) .stream() @@ -54,6 +59,7 @@ public class TransitionService { } public boolean doTransition(DefaultIssue defaultIssue, IssueChangeContext issueChangeContext, String transitionKey) { + checkArgument(!defaultIssue.isFromExternalRuleEngine(), "Transition is not allowed on issues imported from external rule engines"); return workflow.doManualTransition(defaultIssue, transitionKey, issueChangeContext); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java index 37d5fda28f2..9d035ee4b3f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java @@ -38,7 +38,6 @@ import org.sonar.server.issue.IssueUpdater; import org.sonar.server.issue.TransitionService; import org.sonar.server.user.UserSession; -import static org.sonar.server.ws.WsUtils.checkRequest; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_DO_TRANSITION; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ISSUE; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_TRANSITION; @@ -95,7 +94,6 @@ public class DoTransitionAction implements IssuesWsAction { String issue = request.mandatoryParam(PARAM_ISSUE); try (DbSession dbSession = dbClient.openSession(false)) { IssueDto issueDto = issueFinder.getByKey(dbSession, issue); - checkRequest(!issueDto.isExternal(), "Transition is not allowed on issues imported from external rule engines"); SearchResponseData preloadedSearchResponseData = doTransition(dbSession, issueDto, request.mandatoryParam(PARAM_TRANSITION)); responseWriter.write(issue, preloadedSearchResponseData, request, response); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java index 42a32d89672..5ff2589b5cd 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java @@ -107,19 +107,6 @@ public class TransitionActionTest { action.verify(ImmutableMap.of("unknwown", "reopen"), Lists.newArrayList(), userSession); } - @Test - public void do_not_allow_transitions_for_issues_from_external_rule_engine() { - loginAndAddProjectPermission("john", ISSUE_ADMIN); - - context.issue() - .setIsFromExternalRuleEngine(true) - .setStatus(STATUS_CLOSED); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("No transition allowed on issue from externally define rule"); - action.execute(ImmutableMap.of("transition", "close"), context); - } - @Test public void should_support_all_issues() { assertThat(action.supports(new DefaultIssue().setResolution(null))).isTrue(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionServiceTest.java index a049b8638e6..be70934e222 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionServiceTest.java @@ -24,13 +24,13 @@ import java.util.List; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; +import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDto; -import org.sonar.db.component.ComponentTesting; import org.sonar.db.issue.IssueDto; -import org.sonar.db.organization.OrganizationTesting; -import org.sonar.db.rule.RuleDto; +import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.server.issue.workflow.FunctionExecutor; import org.sonar.server.issue.workflow.IssueWorkflow; import org.sonar.server.issue.workflow.Transition; @@ -41,13 +41,15 @@ import static org.sonar.api.issue.Issue.STATUS_CONFIRMED; import static org.sonar.api.issue.Issue.STATUS_OPEN; import static org.sonar.api.web.UserRole.ISSUE_ADMIN; import static org.sonar.db.component.ComponentTesting.newFileDto; -import static org.sonar.db.issue.IssueTesting.newDto; -import static org.sonar.db.rule.RuleTesting.newRuleDto; public class TransitionServiceTest { + @Rule + public DbTester db = DbTester.create(); @Rule public UserSessionRule userSession = UserSessionRule.standalone(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); private IssueFieldsSetter updater = new IssueFieldsSetter(); private IssueWorkflow workflow = new IssueWorkflow(new FunctionExecutor(updater), updater); @@ -61,18 +63,37 @@ public class TransitionServiceTest { @Test public void list_transitions() { - IssueDto issue = newIssue().setStatus(STATUS_OPEN).setResolution(null); - userSession.logIn("john").addProjectPermission(ISSUE_ADMIN, ComponentTesting.newPrivateProjectDto(OrganizationTesting.newOrganizationDto(), issue.getProjectUuid())); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + userSession.logIn().addProjectPermission(ISSUE_ADMIN, project); List result = underTest.listTransitions(issue.toDefaultIssue()); assertThat(result).extracting(Transition::key).containsOnly("confirm", "resolve", "falsepositive", "wontfix"); } + @Test + public void list_transitions_returns_empty_list_on_external_issue() { + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto externalRule = db.rules().insert(r -> r.setIsExternal(true)); + IssueDto externalIssue = db.issues().insert(externalRule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + userSession.logIn().addProjectPermission(ISSUE_ADMIN, project); + + List result = underTest.listTransitions(externalIssue.toDefaultIssue()); + + assertThat(result).isEmpty(); + } + @Test public void list_transitions_returns_only_transitions_that_do_not_requires_issue_admin_permission() { - userSession.logIn("john"); - IssueDto issue = newIssue().setStatus(STATUS_OPEN).setResolution(null); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + userSession.logIn(); List result = underTest.listTransitions(issue.toDefaultIssue()); @@ -81,7 +102,10 @@ public class TransitionServiceTest { @Test public void list_transitions_returns_nothing_when_not_logged() { - IssueDto issue = newIssue().setStatus(STATUS_OPEN).setResolution(null); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); List result = underTest.listTransitions(issue.toDefaultIssue()); @@ -90,18 +114,29 @@ public class TransitionServiceTest { @Test public void do_transition() { - DefaultIssue issue = newIssue().setStatus(STATUS_OPEN).setResolution(null).toDefaultIssue(); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); - boolean result = underTest.doTransition(issue, IssueChangeContext.createUser(new Date(), "user_uuid"), "confirm"); + DefaultIssue defaultIssue = issue.toDefaultIssue(); + boolean result = underTest.doTransition(defaultIssue, IssueChangeContext.createUser(new Date(), "user_uuid"), "confirm"); assertThat(result).isTrue(); - assertThat(issue.status()).isEqualTo(STATUS_CONFIRMED); + assertThat(defaultIssue.status()).isEqualTo(STATUS_CONFIRMED); } - private IssueDto newIssue() { - RuleDto rule = newRuleDto().setId(10); - ComponentDto project = ComponentTesting.newPrivateProjectDto(OrganizationTesting.newOrganizationDto()); - ComponentDto file = (newFileDto(project)); - return newDto(rule, file, project); + @Test + public void do_transition_fail_on_external_issue() { + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto externalRule = db.rules().insert(r -> r.setIsExternal(true)); + IssueDto externalIssue = db.issues().insert(externalRule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + DefaultIssue defaultIssue = externalIssue.toDefaultIssue(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Transition is not allowed on issues imported from external rule engines"); + + underTest.doTransition(defaultIssue, IssueChangeContext.createUser(new Date(), "user_uuid"), "confirm"); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java index ea19e99cda7..f1bdec55069 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java @@ -33,31 +33,27 @@ import org.mockito.ArgumentCaptor; import org.sonar.api.rules.RuleType; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.System2; +import org.sonar.api.utils.internal.TestSystem2; import org.sonar.db.DbClient; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueChangeDto; -import org.sonar.db.issue.IssueDbTester; import org.sonar.db.issue.IssueDto; -import org.sonar.db.organization.OrganizationDto; -import org.sonar.db.rule.RuleDbTester; import org.sonar.db.rule.RuleDefinitionDto; -import org.sonar.db.rule.RuleDto; import org.sonar.db.user.UserDto; import org.sonar.server.es.EsTester; import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.issue.Action; import org.sonar.server.issue.IssueFieldsSetter; -import org.sonar.server.issue.WebIssueStorage; import org.sonar.server.issue.TestIssueChangePostProcessor; import org.sonar.server.issue.TransitionService; +import org.sonar.server.issue.WebIssueStorage; import org.sonar.server.issue.index.IssueIndexer; import org.sonar.server.issue.index.IssueIteratorFactory; import org.sonar.server.issue.notification.IssueChangeNotification; import org.sonar.server.issue.workflow.FunctionExecutor; import org.sonar.server.issue.workflow.IssueWorkflow; import org.sonar.server.notification.NotificationManager; -import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.rule.DefaultRuleFinder; import org.sonar.server.tester.UserSessionRule; @@ -76,7 +72,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.issue.Issue.STATUS_CLOSED; import static org.sonar.api.issue.Issue.STATUS_OPEN; @@ -90,15 +85,12 @@ import static org.sonar.api.web.UserRole.USER; import static org.sonar.core.util.Protobuf.setNullable; import static org.sonar.db.component.ComponentTesting.newFileDto; import static org.sonar.db.issue.IssueChangeDto.TYPE_COMMENT; -import static org.sonar.db.issue.IssueTesting.newDto; -import static org.sonar.db.rule.RuleTesting.newRule; -import static org.sonar.db.rule.RuleTesting.newRuleDto; public class BulkChangeActionTest { private static long NOW = 2_000_000_000_000L; - private System2 system2 = mock(System2.class); + private System2 system2 = new TestSystem2().setNow(NOW); @Rule public ExpectedException expectedException = ExpectedException.none(); @@ -110,51 +102,42 @@ public class BulkChangeActionTest { public UserSessionRule userSession = UserSessionRule.standalone(); private DbClient dbClient = db.getDbClient(); - private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); private IssueFieldsSetter issueFieldsSetter = new IssueFieldsSetter(); private IssueWorkflow issueWorkflow = new IssueWorkflow(new FunctionExecutor(issueFieldsSetter), issueFieldsSetter); - private WebIssueStorage issueStorage = new WebIssueStorage(system2, dbClient, new DefaultRuleFinder(dbClient, defaultOrganizationProvider), + private WebIssueStorage issueStorage = new WebIssueStorage(system2, dbClient, + new DefaultRuleFinder(dbClient, TestDefaultOrganizationProvider.from(db)), new IssueIndexer(es.client(), dbClient, new IssueIteratorFactory(dbClient))); private NotificationManager notificationManager = mock(NotificationManager.class); private TestIssueChangePostProcessor issueChangePostProcessor = new TestIssueChangePostProcessor(); private List actions = new ArrayList<>(); - private RuleDbTester ruleDbTester = new RuleDbTester(db); - private IssueDbTester issueDbTester = new IssueDbTester(db); - - private RuleDto rule; - private OrganizationDto organization; - private ComponentDto project; - private ComponentDto file; - private UserDto user; - private WsActionTester tester = new WsActionTester(new BulkChangeAction(system2, userSession, dbClient, issueStorage, notificationManager, actions, issueChangePostProcessor)); @Before public void setUp() { issueWorkflow.start(); - rule = db.rules().insertRule(newRuleDto()); - organization = db.organizations().insert(); - project = db.components().insertMainBranch(organization); - file = db.components().insertComponent(newFileDto(project)); - user = db.users().insertUser("john"); - when(system2.now()).thenReturn(NOW); addActions(); } @Test public void set_type() { - setUserProjectPermissions(USER, ISSUE_ADMIN); - IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() - .setIssues(singletonList(issueDto.getKey())) + .setIssues(singletonList(issue.getKey())) .setSetType(RuleType.CODE_SMELL.name()) .build()); checkResponse(response, 1, 1, 0, 0); - IssueDto reloaded = getIssueByKeys(issueDto.getKey()).get(0); + IssueDto reloaded = getIssueByKeys(issue.getKey()).get(0); assertThat(reloaded.getType()).isEqualTo(RuleType.CODE_SMELL.getDbConstant()); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); @@ -163,16 +146,22 @@ public class BulkChangeActionTest { @Test public void set_severity() { - setUserProjectPermissions(USER, ISSUE_ADMIN); - IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setSeverity(MAJOR)); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setSeverity(MAJOR) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() - .setIssues(singletonList(issueDto.getKey())) + .setIssues(singletonList(issue.getKey())) .setSetSeverity(MINOR) .build()); checkResponse(response, 1, 1, 0, 0); - IssueDto reloaded = getIssueByKeys(issueDto.getKey()).get(0); + IssueDto reloaded = getIssueByKeys(issue.getKey()).get(0); assertThat(reloaded.getSeverity()).isEqualTo(MINOR); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); @@ -181,16 +170,22 @@ public class BulkChangeActionTest { @Test public void add_tags() { - setUserProjectPermissions(USER, ISSUE_ADMIN); - IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setTags(asList("tag1", "tag2"))); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setTags(asList("tag1", "tag2")) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() - .setIssues(singletonList(issueDto.getKey())) + .setIssues(singletonList(issue.getKey())) .setAddTags(singletonList("tag3")) .build()); checkResponse(response, 1, 1, 0, 0); - IssueDto reloaded = getIssueByKeys(issueDto.getKey()).get(0); + IssueDto reloaded = getIssueByKeys(issue.getKey()).get(0); assertThat(reloaded.getTags()).containsOnly("tag1", "tag2", "tag3"); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); @@ -200,16 +195,23 @@ public class BulkChangeActionTest { @Test public void remove_assignee() { - setUserProjectPermissions(USER); - IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setAssigneeUuid("arthur")); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + UserDto assignee = db.users().insertUser(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setAssigneeUuid(assignee.getUuid()) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() - .setIssues(singletonList(issueDto.getKey())) + .setIssues(singletonList(issue.getKey())) .setAssign("") .build()); checkResponse(response, 1, 1, 0, 0); - IssueDto reloaded = getIssueByKeys(issueDto.getKey()).get(0); + IssueDto reloaded = getIssueByKeys(issue.getKey()).get(0); assertThat(reloaded.getAssigneeUuid()).isNull(); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); @@ -219,17 +221,23 @@ public class BulkChangeActionTest { @Test public void bulk_change_with_comment() { - setUserProjectPermissions(USER); - IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() - .setIssues(singletonList(issueDto.getKey())) + .setIssues(singletonList(issue.getKey())) .setDoTransition("confirm") .setComment("type was badly defined") .build()); checkResponse(response, 1, 1, 0, 0); - IssueChangeDto issueComment = dbClient.issueChangeDao().selectByTypeAndIssueKeys(db.getSession(), singletonList(issueDto.getKey()), TYPE_COMMENT).get(0); + IssueChangeDto issueComment = dbClient.issueChangeDao().selectByTypeAndIssueKeys(db.getSession(), singletonList(issue.getKey()), TYPE_COMMENT).get(0); assertThat(issueComment.getUserUuid()).isEqualTo(user.getUuid()); assertThat(issueComment.getChangeData()).isEqualTo("type was badly defined"); @@ -238,13 +246,21 @@ public class BulkChangeActionTest { @Test public void bulk_change_many_issues() { - setUserProjectPermissions(USER, ISSUE_ADMIN); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + UserDto oldAssignee = db.users().insertUser(); UserDto userToAssign = db.users().insertUser(); - db.organizations().addMember(organization, user); - db.organizations().addMember(organization, userToAssign); - IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setAssigneeUuid(user.getUuid())).setType(BUG).setSeverity(MINOR); - IssueDto issue2 = db.issues().insertIssue(newUnresolvedIssue().setAssigneeUuid(userToAssign.getLogin())).setType(BUG).setSeverity(MAJOR); - IssueDto issue3 = db.issues().insertIssue(newUnresolvedIssue().setAssigneeUuid(null)).setType(VULNERABILITY).setSeverity(MAJOR); + db.organizations().addMember(db.getDefaultOrganization(), userToAssign); + IssueDto issue1 = db.issues().insert(rule, project, file, + i -> i.setAssigneeUuid(oldAssignee.getUuid())).setType(BUG).setSeverity(MINOR).setStatus(STATUS_OPEN).setResolution(null); + IssueDto issue2 = db.issues().insert(rule, project, file, + i -> i.setAssigneeUuid(userToAssign.getUuid())).setType(BUG).setSeverity(MAJOR).setStatus(STATUS_OPEN).setResolution(null); + IssueDto issue3 = db.issues().insert(rule, project, file, + i -> i.setAssigneeUuid(null)).setType(VULNERABILITY).setSeverity(MAJOR).setStatus(STATUS_OPEN).setResolution(null); BulkChangeWsResponse response = call(builder() .setIssues(asList(issue1.getKey(), issue2.getKey(), issue3.getKey())) @@ -266,20 +282,25 @@ public class BulkChangeActionTest { @Test public void send_notification() { - setUserProjectPermissions(USER); - IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() - .setIssues(singletonList(issueDto.getKey())) + .setIssues(singletonList(issue.getKey())) .setDoTransition("confirm") .setSendNotifications(true) .build()); checkResponse(response, 1, 1, 0, 0); - ArgumentCaptor issueChangeNotificationCaptor = ArgumentCaptor.forClass(IssueChangeNotification.class); verify(notificationManager).scheduleForSending(issueChangeNotificationCaptor.capture()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("key")).isEqualTo(issueDto.getKey()); + assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("key")).isEqualTo(issue.getKey()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("componentName")).isEqualTo(file.longName()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectName")).isEqualTo(project.name()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectKey")).isEqualTo(project.getDbKey()); @@ -289,58 +310,71 @@ public class BulkChangeActionTest { } @Test - public void no_notification_for_hotspots() { - setUserProjectPermissions(USER); - IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setType(SECURITY_HOTSPOT)); + public void hotspots_are_ignored_and_no_notification_is_sent() { + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setType(SECURITY_HOTSPOT) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() - .setIssues(singletonList(issueDto.getKey())) + .setIssues(singletonList(issue.getKey())) .setDoTransition("dismiss") .setSendNotifications(true) .build()); - checkResponse(response, 1, 1, 0, 0); - + checkResponse(response, 1, 0, 1, 0); verify(notificationManager, never()).scheduleForSending(any()); } @Test public void send_notification_on_branch() { - setUserProjectPermissions(USER); - - String branchName = "feature1"; - ComponentDto branch = db.components().insertProjectBranch(project, b -> b.setKey(branchName)); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertMainBranch(); + ComponentDto branch = db.components().insertProjectBranch(project, b -> b.setKey("feature")); ComponentDto fileOnBranch = db.components().insertComponent(newFileDto(branch)); - IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue(rule, fileOnBranch, branch).setType(BUG)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, branch, fileOnBranch, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() - .setIssues(singletonList(issueDto.getKey())) + .setIssues(singletonList(issue.getKey())) .setDoTransition("confirm") .setSendNotifications(true) .build()); checkResponse(response, 1, 1, 0, 0); - ArgumentCaptor issueChangeNotificationCaptor = ArgumentCaptor.forClass(IssueChangeNotification.class); verify(notificationManager).scheduleForSending(issueChangeNotificationCaptor.capture()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("key")).isEqualTo(issueDto.getKey()); + assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("key")).isEqualTo(issue.getKey()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("componentName")).isEqualTo(fileOnBranch.longName()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectName")).isEqualTo(project.name()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectKey")).isEqualTo(project.getDbKey()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("ruleName")).isEqualTo(rule.getName()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("changeAuthor")).isEqualTo(user.getLogin()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("branch")).isEqualTo(branchName); - + assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("branch")).isEqualTo("feature"); verifyPostProcessorCalled(fileOnBranch); } @Test public void send_notification_only_on_changed_issues() { - setUserProjectPermissions(USER, ISSUE_ADMIN); - IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); - IssueDto issue2 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); - IssueDto issue3 = db.issues().insertIssue(newUnresolvedIssue().setType(VULNERABILITY)); - ArgumentCaptor issueChangeNotificationCaptor = ArgumentCaptor.forClass(IssueChangeNotification.class); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertMainBranch(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue1 = db.issues().insert(rule, project, file, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); + IssueDto issue2 = db.issues().insert(rule, project, file, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); + IssueDto issue3 = db.issues().insert(rule, project, file, i -> i.setType(VULNERABILITY) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() .setIssues(asList(issue1.getKey(), issue2.getKey(), issue3.getKey())) @@ -349,21 +383,29 @@ public class BulkChangeActionTest { .build()); checkResponse(response, 3, 1, 2, 0); + ArgumentCaptor issueChangeNotificationCaptor = ArgumentCaptor.forClass(IssueChangeNotification.class); verify(notificationManager).scheduleForSending(issueChangeNotificationCaptor.capture()); assertThat(issueChangeNotificationCaptor.getAllValues()).hasSize(1); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("key")).isEqualTo(issue3.getKey()); - verifyPostProcessorCalled(file); } @Test public void ignore_the_issues_that_do_not_match_conditions() { - setUserProjectPermissions(USER, ISSUE_ADMIN); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertMainBranch(); + ComponentDto file1 = db.components().insertComponent(newFileDto(project)); ComponentDto file2 = db.components().insertComponent(newFileDto(project)); - IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue1 = db.issues().insert(rule, project, file1, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); // These 2 issues will be ignored as they are resolved, changing type is not possible - IssueDto issue2 = db.issues().insertIssue(newResolvedIssue().setType(BUG)); - IssueDto issue3 = db.issues().insertIssue(newResolvedIssue().setType(BUG).setComponent(file2)); + IssueDto issue2 = db.issues().insert(rule, project, file1, i -> i.setType(BUG) + .setStatus(STATUS_CLOSED).setResolution(RESOLUTION_FIXED)); + IssueDto issue3 = db.issues().insert(rule, project, file2, i -> i.setType(BUG) + .setStatus(STATUS_CLOSED).setResolution(RESOLUTION_FIXED)); BulkChangeWsResponse response = call(builder() .setIssues(asList(issue1.getKey(), issue2.getKey(), issue3.getKey())) @@ -375,21 +417,29 @@ public class BulkChangeActionTest { .extracting(IssueDto::getKey, IssueDto::getType, IssueDto::getUpdatedAt) .containsOnly( tuple(issue1.getKey(), VULNERABILITY.getDbConstant(), NOW), - tuple(issue3.getKey(), BUG.getDbConstant(), issue2.getUpdatedAt()), - tuple(issue2.getKey(), BUG.getDbConstant(), issue3.getUpdatedAt())); + tuple(issue2.getKey(), BUG.getDbConstant(), issue2.getUpdatedAt()), + tuple(issue3.getKey(), BUG.getDbConstant(), issue3.getUpdatedAt())); // file2 is not refreshed - verifyPostProcessorCalled(file); + verifyPostProcessorCalled(file1); } @Test public void ignore_issues_when_there_is_nothing_to_do() { - setUserProjectPermissions(USER, ISSUE_ADMIN); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertMainBranch(); + ComponentDto file1 = db.components().insertComponent(newFileDto(project)); ComponentDto file2 = db.components().insertComponent(newFileDto(project)); - IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG).setSeverity(MINOR)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue1 = db.issues().insert(rule, project, file1, i -> i.setType(BUG).setSeverity(MINOR) + .setStatus(STATUS_OPEN).setResolution(null)); // These 2 issues will be ignored as there's nothing to do - IssueDto issue2 = db.issues().insertIssue(newUnresolvedIssue().setType(VULNERABILITY)); - IssueDto issue3 = db.issues().insertIssue(newUnresolvedIssue().setType(VULNERABILITY).setComponent(file2)); + IssueDto issue2 = db.issues().insert(rule, project, file1, i -> i.setType(VULNERABILITY) + .setStatus(STATUS_OPEN).setResolution(null)); + IssueDto issue3 = db.issues().insert(rule, project, file2, i -> i.setType(VULNERABILITY) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() .setIssues(asList(issue1.getKey(), issue2.getKey(), issue3.getKey())) @@ -405,16 +455,25 @@ public class BulkChangeActionTest { tuple(issue3.getKey(), VULNERABILITY.getDbConstant(), issue3.getUpdatedAt())); // file2 is not refreshed - verifyPostProcessorCalled(file); + verifyPostProcessorCalled(file1); } @Test public void add_comment_only_on_changed_issues() { - setUserProjectPermissions(USER, ISSUE_ADMIN); - IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG).setSeverity(MINOR)); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertMainBranch(); + ComponentDto file1 = db.components().insertComponent(newFileDto(project)); + ComponentDto file2 = db.components().insertComponent(newFileDto(project)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue1 = db.issues().insert(rule, project, file1, i -> i.setType(BUG).setSeverity(MINOR) + .setStatus(STATUS_OPEN).setResolution(null)); // These 2 issues will be ignored as there's nothing to do - IssueDto issue2 = db.issues().insertIssue(newUnresolvedIssue().setType(VULNERABILITY)); - IssueDto issue3 = db.issues().insertIssue(newUnresolvedIssue().setType(VULNERABILITY)); + IssueDto issue2 = db.issues().insert(rule, project, file1, i -> i.setType(VULNERABILITY) + .setStatus(STATUS_OPEN).setResolution(null)); + IssueDto issue3 = db.issues().insert(rule, project, file2, i -> i.setType(VULNERABILITY) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() .setIssues(asList(issue1.getKey(), issue2.getKey(), issue3.getKey())) @@ -427,18 +486,43 @@ public class BulkChangeActionTest { assertThat(dbClient.issueChangeDao().selectByTypeAndIssueKeys(db.getSession(), singletonList(issue2.getKey()), TYPE_COMMENT)).isEmpty(); assertThat(dbClient.issueChangeDao().selectByTypeAndIssueKeys(db.getSession(), singletonList(issue3.getKey()), TYPE_COMMENT)).isEmpty(); - verifyPostProcessorCalled(file); + verifyPostProcessorCalled(file1); + } + + @Test + public void ignore_external_issue() { + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertPrivateProject(); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, project, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + RuleDefinitionDto externalRule = db.rules().insert(r -> r.setIsExternal(true)); + IssueDto externalIssue = db.issues().insert(externalRule, project, project, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + + BulkChangeWsResponse response = call(builder() + .setIssues(asList(issue.getKey(), externalIssue.getKey())) + .setDoTransition("confirm") + .build()); + + checkResponse(response, 2, 1, 1, 0); } @Test public void issues_on_which_user_has_not_browse_permission_are_ignored() { - setUserProjectPermissions(USER, ISSUE_ADMIN); - ComponentDto anotherProject = db.components().insertPrivateProject(); - ComponentDto anotherFile = db.components().insertComponent(newFileDto(anotherProject)); - IssueDto authorizedIssue = db.issues().insertIssue(newUnresolvedIssue(rule, file, project).setType(BUG)); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project1 = db.components().insertPrivateProject(); + addUserProjectPermissions(user, project1, USER, ISSUE_ADMIN); + ComponentDto project2 = db.components().insertPrivateProject(); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto authorizedIssue = db.issues().insert(rule, project1, project1, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); // User has not browse permission on these 2 issues - IssueDto notAuthorizedIssue1 = db.issues().insertIssue(newUnresolvedIssue(rule, anotherFile, anotherProject).setType(BUG)); - IssueDto notAuthorizedIssue2 = db.issues().insertIssue(newUnresolvedIssue(rule, anotherFile, anotherProject).setType(BUG)); + IssueDto notAuthorizedIssue1 = db.issues().insert(rule, project2, project2, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); + IssueDto notAuthorizedIssue2 = db.issues().insert(rule, project2, project2, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() .setIssues(asList(authorizedIssue.getKey(), notAuthorizedIssue1.getKey(), notAuthorizedIssue2.getKey())) @@ -453,20 +537,25 @@ public class BulkChangeActionTest { tuple(notAuthorizedIssue1.getKey(), BUG.getDbConstant(), notAuthorizedIssue1.getUpdatedAt()), tuple(notAuthorizedIssue2.getKey(), BUG.getDbConstant(), notAuthorizedIssue2.getUpdatedAt())); - verifyPostProcessorCalled(file); + verifyPostProcessorCalled(project1); } @Test public void does_not_update_type_when_no_issue_admin_permission() { - setUserProjectPermissions(USER, ISSUE_ADMIN); - ComponentDto anotherProject = db.components().insertPrivateProject(); - ComponentDto anotherFile = db.components().insertComponent(newFileDto(anotherProject)); - addUserProjectPermissions(anotherProject, USER); - - IssueDto authorizedIssue1 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project1 = db.components().insertPrivateProject(); + addUserProjectPermissions(user, project1, USER, ISSUE_ADMIN); + ComponentDto project2 = db.components().insertPrivateProject(); + addUserProjectPermissions(user, project2, USER); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto authorizedIssue1 = db.issues().insert(rule, project1, project1, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); // User has not issue admin permission on these 2 issues - IssueDto notAuthorizedIssue1 = db.issues().insertIssue(newUnresolvedIssue(rule, anotherFile, anotherProject).setType(BUG)); - IssueDto notAuthorizedIssue2 = db.issues().insertIssue(newUnresolvedIssue(rule, anotherFile, anotherProject).setType(BUG)); + IssueDto notAuthorizedIssue1 = db.issues().insert(rule, project2, project2, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); + IssueDto notAuthorizedIssue2 = db.issues().insert(rule, project2, project2, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() .setIssues(asList(authorizedIssue1.getKey(), notAuthorizedIssue1.getKey(), notAuthorizedIssue2.getKey())) @@ -480,20 +569,25 @@ public class BulkChangeActionTest { tuple(authorizedIssue1.getKey(), VULNERABILITY.getDbConstant(), NOW), tuple(notAuthorizedIssue1.getKey(), BUG.getDbConstant(), notAuthorizedIssue1.getUpdatedAt()), tuple(notAuthorizedIssue2.getKey(), BUG.getDbConstant(), notAuthorizedIssue2.getUpdatedAt())); - verifyPostProcessorCalled(file); + verifyPostProcessorCalled(project1); } @Test public void does_not_update_severity_when_no_issue_admin_permission() { - setUserProjectPermissions(USER, ISSUE_ADMIN); - ComponentDto anotherProject = db.components().insertPrivateProject(); - ComponentDto anotherFile = db.components().insertComponent(newFileDto(anotherProject)); - addUserProjectPermissions(anotherProject, USER); - - IssueDto authorizedIssue1 = db.issues().insertIssue(newUnresolvedIssue().setSeverity(MAJOR)); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project1 = db.components().insertPrivateProject(); + addUserProjectPermissions(user, project1, USER, ISSUE_ADMIN); + ComponentDto project2 = db.components().insertPrivateProject(); + addUserProjectPermissions(user, project2, USER); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto authorizedIssue1 = db.issues().insert(rule, project1, project1, i -> i.setSeverity(MAJOR) + .setStatus(STATUS_OPEN).setResolution(null)); // User has not issue admin permission on these 2 issues - IssueDto notAuthorizedIssue1 = db.issues().insertIssue(newUnresolvedIssue(rule, anotherFile, anotherProject).setSeverity(MAJOR)); - IssueDto notAuthorizedIssue2 = db.issues().insertIssue(newUnresolvedIssue(rule, anotherFile, anotherProject).setSeverity(MAJOR)); + IssueDto notAuthorizedIssue1 = db.issues().insert(rule, project2, project2, i -> i.setSeverity(MAJOR) + .setStatus(STATUS_OPEN).setResolution(null)); + IssueDto notAuthorizedIssue2 = db.issues().insert(rule, project2, project2, i -> i.setSeverity(MAJOR) + .setStatus(STATUS_OPEN).setResolution(null)); BulkChangeWsResponse response = call(builder() .setIssues(asList(authorizedIssue1.getKey(), notAuthorizedIssue1.getKey(), notAuthorizedIssue2.getKey())) @@ -507,55 +601,28 @@ public class BulkChangeActionTest { tuple(authorizedIssue1.getKey(), MINOR, NOW), tuple(notAuthorizedIssue1.getKey(), MAJOR, notAuthorizedIssue1.getUpdatedAt()), tuple(notAuthorizedIssue2.getKey(), MAJOR, notAuthorizedIssue2.getUpdatedAt())); - - verifyPostProcessorCalled(file); + verifyPostProcessorCalled(project1); } @Test public void fail_when_only_comment_action() { - setUserProjectPermissions(USER); - IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertPrivateProject(); + addUserProjectPermissions(user, project, USER); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, project, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); expectedException.expectMessage("At least one action must be provided"); expectedException.expect(IllegalArgumentException.class); call(builder() - .setIssues(singletonList(issueDto.getKey())) + .setIssues(singletonList(issue.getKey())) .setComment("type was badly defined") .build()); } - @Test - public void fail_when_requesting_transition_on_issue_from_external_rules_engine(){ - - setUserProjectPermissions(USER, ISSUE_ADMIN); - UserDto userToAssign = db.users().insertUser("arthur"); - db.organizations().addMember(organization, user); - db.organizations().addMember(organization, userToAssign); - - RuleDefinitionDto rule = ruleDbTester.insert(newRule() - .setIsExternal(true)); - IssueDto issue1 = issueDbTester.insertIssue(newIssue() - .setStatus(STATUS_OPEN) - .setResolution(null) - .setRuleId(rule.getId()) - .setRuleKey(rule.getRuleKey(), rule.getRepositoryKey()) - .setType(BUG) - .setSeverity(MINOR)); - - IssueDto issue2 = issueDbTester.insertIssue(newIssue() - .setStatus(STATUS_OPEN) - .setResolution(null) - .setRuleId(rule.getId()) - .setRuleKey(rule.getRuleKey(), rule.getRepositoryKey()) - .setType(BUG) - .setSeverity(MAJOR)); - - BulkChangeWsResponse confirm = call(builder().setIssues(asList(issue1.getKey(), issue2.getKey())).setDoTransition("confirm").build()); - - assertThat(confirm.getFailures()).isEqualTo(2); - } - @Test public void fail_when_number_of_issues_is_more_than_500() { userSession.logIn("john"); @@ -604,12 +671,7 @@ public class BulkChangeActionTest { return request.executeProtobuf(BulkChangeWsResponse.class); } - private void setUserProjectPermissions(String... permissions) { - userSession.logIn(user); - addUserProjectPermissions(project, permissions); - } - - private void addUserProjectPermissions(ComponentDto project, String... permissions) { + private void addUserProjectPermissions(UserDto user, ComponentDto project, String... permissions) { for (String permission : permissions) { db.users().insertProjectPermissionOnUser(user, permission, project); userSession.addProjectPermission(permission, project); @@ -620,7 +682,7 @@ public class BulkChangeActionTest { assertThat(response) .extracting(BulkChangeWsResponse::getTotal, BulkChangeWsResponse::getSuccess, BulkChangeWsResponse::getIgnored, BulkChangeWsResponse::getFailures) .as("Total, success, ignored, failure") - .containsOnly(total, success, ignored, failure); + .containsExactly(total, success, ignored, failure); } private List getIssueByKeys(String... issueKeys) { @@ -635,23 +697,6 @@ public class BulkChangeActionTest { assertThat(issueChangePostProcessor.wasCalled()).isFalse(); } - private IssueDto newUnresolvedIssue(RuleDto rule, ComponentDto file, ComponentDto project) { - return newDto(rule, file, project).setStatus(STATUS_OPEN).setResolution(null); - } - - private IssueDto newUnresolvedIssue() { - return newUnresolvedIssue(rule, file, project); - } - - private IssueDto newResolvedIssue() { - return newDto(rule, file, project).setStatus(STATUS_CLOSED).setResolution(RESOLUTION_FIXED); - } - - private IssueDto newIssue() { - RuleDto rule = ruleDbTester.insertRule(newRuleDto()); - return newDto(rule, file, project); - } - private void addActions() { actions.add(new org.sonar.server.issue.AssignAction(db.getDbClient(), issueFieldsSetter)); actions.add(new org.sonar.server.issue.SetSeverityAction(issueFieldsSetter, userSession)); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java index 5f28b486e1a..e9958c63fa4 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java @@ -28,26 +28,22 @@ import org.mockito.ArgumentCaptor; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.utils.System2; +import org.sonar.api.utils.internal.TestSystem2; import org.sonar.db.DbClient; import org.sonar.db.DbTester; -import org.sonar.db.component.ComponentDbTester; import org.sonar.db.component.ComponentDto; -import org.sonar.db.issue.IssueDbTester; import org.sonar.db.issue.IssueDto; -import org.sonar.db.rule.RuleDbTester; import org.sonar.db.rule.RuleDefinitionDto; -import org.sonar.db.rule.RuleDto; import org.sonar.server.es.EsTester; -import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.issue.IssueFieldsSetter; import org.sonar.server.issue.IssueFinder; -import org.sonar.server.issue.WebIssueStorage; import org.sonar.server.issue.IssueUpdater; import org.sonar.server.issue.TestIssueChangePostProcessor; import org.sonar.server.issue.TransitionService; +import org.sonar.server.issue.WebIssueStorage; import org.sonar.server.issue.index.IssueIndexer; import org.sonar.server.issue.index.IssueIteratorFactory; import org.sonar.server.issue.workflow.FunctionExecutor; @@ -67,26 +63,23 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.STATUS_CONFIRMED; import static org.sonar.api.issue.Issue.STATUS_OPEN; import static org.sonar.api.web.UserRole.CODEVIEWER; import static org.sonar.api.web.UserRole.USER; import static org.sonar.db.component.ComponentTesting.newFileDto; -import static org.sonar.db.issue.IssueTesting.newDto; -import static org.sonar.db.rule.RuleTesting.newRule; -import static org.sonar.db.rule.RuleTesting.newRuleDto; public class DoTransitionActionTest { private static final long NOW = 999_776_888L; - private System2 system2 = mock(System2.class); + + private System2 system2 = new TestSystem2().setNow(NOW); @Rule public ExpectedException expectedException = ExpectedException.none(); @Rule - public DbTester dbTester = DbTester.create(system2); + public DbTester db = DbTester.create(system2); @Rule public EsTester es = EsTester.create(); @@ -94,12 +87,8 @@ public class DoTransitionActionTest { @Rule public UserSessionRule userSession = UserSessionRule.standalone(); - private DbClient dbClient = dbTester.getDbClient(); - private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(dbTester); - - private RuleDbTester ruleDbTester = new RuleDbTester(dbTester); - private IssueDbTester issueDbTester = new IssueDbTester(dbTester); - private ComponentDbTester componentDbTester = new ComponentDbTester(dbTester); + private DbClient dbClient = db.getDbClient(); + private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); private IssueFieldsSetter updater = new IssueFieldsSetter(); private IssueWorkflow workflow = new IssueWorkflow(new FunctionExecutor(updater), updater); @@ -110,8 +99,6 @@ public class DoTransitionActionTest { private IssueUpdater issueUpdater = new IssueUpdater(dbClient, new WebIssueStorage(system2, dbClient, new DefaultRuleFinder(dbClient, defaultOrganizationProvider), issueIndexer), mock(NotificationManager.class), issueChangePostProcessor); - private ComponentDto project; - private ComponentDto file; private ArgumentCaptor preloadedSearchResponseDataCaptor = ArgumentCaptor.forClass(SearchResponseData.class); private WsAction underTest = new DoTransitionAction(dbClient, userSession, new IssueFinder(dbClient, userSession), issueUpdater, transitionService, responseWriter, system2); @@ -119,29 +106,43 @@ public class DoTransitionActionTest { @Before public void setUp() throws Exception { - project = componentDbTester.insertPrivateProject(); - file = componentDbTester.insertComponent(newFileDto(project)); workflow.start(); } @Test public void do_transition() { - when(system2.now()).thenReturn(NOW); - IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); - userSession.logIn("john").addProjectPermission(USER, project, file); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + userSession.logIn().addProjectPermission(USER, project, file); - call(issueDto.getKey(), "confirm"); + call(issue.getKey(), "confirm"); - verify(responseWriter).write(eq(issueDto.getKey()), preloadedSearchResponseDataCaptor.capture(), any(Request.class), any(Response.class)); - verifyContentOfPreloadedSearchResponseData(issueDto); - IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); + verify(responseWriter).write(eq(issue.getKey()), preloadedSearchResponseDataCaptor.capture(), any(Request.class), any(Response.class)); + verifyContentOfPreloadedSearchResponseData(issue); + IssueDto issueReloaded = db.getDbClient().issueDao().selectByKey(db.getSession(), issue.getKey()).get(); assertThat(issueReloaded.getStatus()).isEqualTo(STATUS_CONFIRMED); assertThat(issueChangePostProcessor.calledComponents()).containsExactlyInAnyOrder(file); } + @Test + public void fail_if_external_issue() { + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto externalRule = db.rules().insert(r -> r.setIsExternal(true)); + IssueDto externalIssue = db.issues().insert(externalRule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + userSession.logIn().addProjectPermission(USER, project, file); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Transition is not allowed on issues imported from external rule engines"); + + call(externalIssue.getKey(), "confirm"); + } + @Test public void fail_if_issue_does_not_exist() { - userSession.logIn("john"); + userSession.logIn(); expectedException.expect(NotFoundException.class); call("UNKNOWN", "confirm"); @@ -149,7 +150,7 @@ public class DoTransitionActionTest { @Test public void fail_if_no_issue_param() { - userSession.logIn("john"); + userSession.logIn(); expectedException.expect(IllegalArgumentException.class); call(null, "confirm"); @@ -157,52 +158,48 @@ public class DoTransitionActionTest { @Test public void fail_if_no_transition_param() { - IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); - userSession.logIn("john").addProjectPermission(USER, project, file); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + userSession.logIn().addProjectPermission(USER, project, file); expectedException.expect(IllegalArgumentException.class); - call(issueDto.getKey(), null); + call(issue.getKey(), null); } @Test public void fail_if_not_enough_permission_to_access_issue() { - IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); - userSession.logIn("john").addProjectPermission(CODEVIEWER, project, file); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + userSession.logIn().addProjectPermission(CODEVIEWER, project, file); expectedException.expect(ForbiddenException.class); - call(issueDto.getKey(), "confirm"); + + call(issue.getKey(), "confirm"); } @Test public void fail_if_not_enough_permission_to_apply_transition() { - IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); - userSession.logIn("john").addProjectPermission(USER, project, file); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); + userSession.logIn().addProjectPermission(USER, project, file); // False-positive transition is requiring issue admin permission expectedException.expect(ForbiddenException.class); - call(issueDto.getKey(), "falsepositive"); + + call(issue.getKey(), "falsepositive"); } @Test public void fail_if_not_authenticated() { expectedException.expect(UnauthorizedException.class); - call("ISSUE_KEY", "confirm"); - } - @Test - public void fail_if_external_issue() { - expectedException.expect(BadRequestException.class); - - RuleDefinitionDto rule = ruleDbTester.insert(newRule() - .setIsExternal(true)); - IssueDto issueDto = issueDbTester.insertIssue(newIssue() - .setStatus(STATUS_OPEN) - .setResolution(null) - .setRuleId(rule.getId()) - .setRuleKey(rule.getRuleKey(), rule.getRepositoryKey())); - userSession.logIn("john").addProjectPermission(USER, project, file); - - call(issueDto.getKey(), "confirm"); + call("ISSUE_KEY", "confirm"); } private TestResponse call(@Nullable String issueKey, @Nullable String transition) { @@ -216,11 +213,6 @@ public class DoTransitionActionTest { return request.execute(); } - private IssueDto newIssue() { - RuleDto rule = ruleDbTester.insertRule(newRuleDto()); - return newDto(rule, file, project); - } - private void verifyContentOfPreloadedSearchResponseData(IssueDto issue) { SearchResponseData preloadedSearchResponseData = preloadedSearchResponseDataCaptor.getValue(); assertThat(preloadedSearchResponseData.getIssues()) -- 2.39.5