Skip to content

Commit

Permalink
SONAR-11886 Allow to assign/bulk assign security hotspots
Browse files Browse the repository at this point in the history
  • Loading branch information
julienlancelot authored and SonarTech committed Apr 23, 2019
1 parent ff6994b commit 867949e
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 23 deletions.
Expand Up @@ -24,7 +24,6 @@
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import org.sonar.api.rules.RuleType;
import org.sonar.api.server.ServerSide; import org.sonar.api.server.ServerSide;
import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.DefaultIssue;
import org.sonar.db.DbClient; import org.sonar.db.DbClient;
Expand Down Expand Up @@ -52,7 +51,7 @@ public AssignAction(DbClient dbClient, IssueFieldsSetter issueFieldsSetter) {
super(ASSIGN_KEY); super(ASSIGN_KEY);
this.dbClient = dbClient; this.dbClient = dbClient;
this.issueFieldsSetter = issueFieldsSetter; this.issueFieldsSetter = issueFieldsSetter;
super.setConditions(new IsUnResolved(), issue -> ((DefaultIssue) issue).type() != RuleType.SECURITY_HOTSPOT); super.setConditions(new IsUnResolved());
} }


@Override @Override
Expand Down
Expand Up @@ -24,7 +24,6 @@
import java.util.Date; import java.util.Date;
import javax.annotation.CheckForNull; import javax.annotation.CheckForNull;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import org.sonar.api.rules.RuleType;
import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Change;
import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Request;
import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.Response;
Expand Down Expand Up @@ -109,7 +108,6 @@ private SearchResponseData assign(String issueKey, @Nullable String login) {
try (DbSession dbSession = dbClient.openSession(false)) { try (DbSession dbSession = dbClient.openSession(false)) {
IssueDto issueDto = issueFinder.getByKey(dbSession, issueKey); IssueDto issueDto = issueFinder.getByKey(dbSession, issueKey);
DefaultIssue issue = issueDto.toDefaultIssue(); DefaultIssue issue = issueDto.toDefaultIssue();
checkArgument(issue.type() != RuleType.SECURITY_HOTSPOT, "Assigning security hotspots is not allowed");
UserDto user = getUser(dbSession, login); UserDto user = getUser(dbSession, login);
if (user != null) { if (user != null) {
checkMembership(dbSession, issueDto, user); checkMembership(dbSession, issueDto, user);
Expand Down
Expand Up @@ -232,9 +232,7 @@ private Set<String> listAvailableActions(IssueDto issue, ComponentDto project) {
if (issue.getResolution() != null) { if (issue.getResolution() != null) {
return availableActions; return availableActions;
} }
if (ruleType != RuleType.SECURITY_HOTSPOT) { availableActions.add(ASSIGN_KEY);
availableActions.add(ASSIGN_KEY);
}
availableActions.add("set_tags"); availableActions.add("set_tags");
if (!issue.isFromHotspot() && userSession.hasComponentPermission(ISSUE_ADMIN, project)) { if (!issue.isFromHotspot() && userSession.hasComponentPermission(ISSUE_ADMIN, project)) {
availableActions.add(SET_TYPE_KEY); availableActions.add(SET_TYPE_KEY);
Expand Down
Expand Up @@ -24,7 +24,6 @@
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.ExpectedException; import org.junit.rules.ExpectedException;
import org.sonar.api.rules.RuleType;
import org.sonar.api.utils.internal.TestSystem2; import org.sonar.api.utils.internal.TestSystem2;
import org.sonar.db.DbClient; import org.sonar.db.DbClient;
import org.sonar.db.DbSession; import org.sonar.db.DbSession;
Expand All @@ -43,6 +42,7 @@
import org.sonar.server.issue.WebIssueStorage; import org.sonar.server.issue.WebIssueStorage;
import org.sonar.server.issue.index.IssueIndexer; import org.sonar.server.issue.index.IssueIndexer;
import org.sonar.server.issue.index.IssueIteratorFactory; import org.sonar.server.issue.index.IssueIteratorFactory;
import org.sonar.server.issue.notification.IssuesChangesNotification;
import org.sonar.server.issue.notification.IssuesChangesNotificationSerializer; import org.sonar.server.issue.notification.IssuesChangesNotificationSerializer;
import org.sonar.server.notification.NotificationManager; import org.sonar.server.notification.NotificationManager;
import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.DefaultOrganizationProvider;
Expand All @@ -53,7 +53,12 @@


import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.rules.ExpectedException.none; import static org.junit.rules.ExpectedException.none;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.sonar.api.rules.RuleType.CODE_SMELL;
import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT;
import static org.sonar.api.web.UserRole.CODEVIEWER; import static org.sonar.api.web.UserRole.CODEVIEWER;
import static org.sonar.api.web.UserRole.USER; import static org.sonar.api.web.UserRole.USER;
import static org.sonar.server.tester.UserSessionRule.standalone; import static org.sonar.server.tester.UserSessionRule.standalone;
Expand All @@ -79,6 +84,7 @@ public class AssignActionTest {
public DbTester db = DbTester.create(system2); public DbTester db = DbTester.create(system2);
public DbClient dbClient = db.getDbClient(); public DbClient dbClient = db.getDbClient();
private DbSession session = db.getSession(); private DbSession session = db.getSession();
private NotificationManager notificationManager = mock(NotificationManager.class);


private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db);
private IssueIndexer issueIndexer = new IssueIndexer(es.client(), dbClient, new IssueIteratorFactory(dbClient)); private IssueIndexer issueIndexer = new IssueIndexer(es.client(), dbClient, new IssueIteratorFactory(dbClient));
Expand All @@ -88,7 +94,7 @@ public class AssignActionTest {
private AssignAction underTest = new AssignAction(system2, userSession, dbClient, new IssueFinder(dbClient, userSession), new IssueFieldsSetter(), private AssignAction underTest = new AssignAction(system2, userSession, dbClient, new IssueFinder(dbClient, userSession), new IssueFieldsSetter(),
new IssueUpdater(dbClient, new IssueUpdater(dbClient,
new WebIssueStorage(system2, dbClient, new DefaultRuleFinder(dbClient, defaultOrganizationProvider), issueIndexer), new WebIssueStorage(system2, dbClient, new DefaultRuleFinder(dbClient, defaultOrganizationProvider), issueIndexer),
mock(NotificationManager.class), issueChangePostProcessor, issuesChangesSerializer), notificationManager, issueChangePostProcessor, issuesChangesSerializer),
responseWriter); responseWriter);
private WsActionTester ws = new WsActionTester(underTest); private WsActionTester ws = new WsActionTester(underTest);


Expand All @@ -99,7 +105,7 @@ public void assign_to_someone() {


ws.newRequest() ws.newRequest()
.setParam("issue", issue.getKey()) .setParam("issue", issue.getKey())
.setParam("assignee", "arthur") .setParam("assignee", arthur.getLogin())
.execute(); .execute();


checkIssueAssignee(issue.getKey(), arthur.getUuid()); checkIssueAssignee(issue.getKey(), arthur.getUuid());
Expand Down Expand Up @@ -174,29 +180,44 @@ public void nothing_to_do_when_new_assignee_is_same_as_old_one() {
} }


@Test @Test
public void fail_when_assignee_does_not_exist() { public void send_notification() {
IssueDto issue = newIssueWithBrowsePermission(); IssueDto issue = newIssueWithBrowsePermission();

UserDto arthur = insertUser("arthur");
expectedException.expect(NotFoundException.class);


ws.newRequest() ws.newRequest()
.setParam("issue", issue.getKey()) .setParam("issue", issue.getKey())
.setParam("assignee", "unknown") .setParam("assignee", arthur.getLogin())
.execute(); .execute();

verify(notificationManager).scheduleForSending(any(IssuesChangesNotification.class));
} }


@Test @Test
public void fail_when_trying_to_assign_hotspot() { public void do_not_send_notification_on_security_hotspot() {
IssueDto issueDto = db.issues().insertIssue(i -> i.setType(RuleType.SECURITY_HOTSPOT)); IssueDto issue = db.issues().insertIssue(
setUserWithBrowsePermission(issueDto); issueDto -> issueDto
.setAssigneeUuid(null)
.setType(SECURITY_HOTSPOT));
setUserWithBrowsePermission(issue);
UserDto arthur = insertUser("arthur"); UserDto arthur = insertUser("arthur");


expectedException.expect(IllegalArgumentException.class); ws.newRequest()
expectedException.expectMessage("Assigning security hotspots is not allowed"); .setParam("issue", issue.getKey())
.setParam("assignee", arthur.getLogin())
.execute();

verifyZeroInteractions(notificationManager);
}

@Test
public void fail_when_assignee_does_not_exist() {
IssueDto issue = newIssueWithBrowsePermission();

expectedException.expect(NotFoundException.class);


ws.newRequest() ws.newRequest()
.setParam("issue", issueDto.getKey()) .setParam("issue", issue.getKey())
.setParam("assignee", "arthur") .setParam("assignee", "unknown")
.execute(); .execute();
} }


Expand Down Expand Up @@ -242,7 +263,7 @@ public void fail_when_missing_browse_permission() {
@Test @Test
public void fail_when_assignee_is_not_member_of_organization_of_project_issue() { public void fail_when_assignee_is_not_member_of_organization_of_project_issue() {
OrganizationDto org = db.organizations().insert(organizationDto -> organizationDto.setKey("Organization key")); OrganizationDto org = db.organizations().insert(organizationDto -> organizationDto.setKey("Organization key"));
IssueDto issueDto = db.issues().insertIssue(org, i -> i.setType(RuleType.CODE_SMELL)); IssueDto issueDto = db.issues().insertIssue(org, i -> i.setType(CODE_SMELL));
setUserWithBrowsePermission(issueDto); setUserWithBrowsePermission(issueDto);
OrganizationDto otherOrganization = db.organizations().insert(); OrganizationDto otherOrganization = db.organizations().insert();
UserDto assignee = db.users().insertUser("arthur"); UserDto assignee = db.users().insertUser("arthur");
Expand All @@ -269,7 +290,7 @@ private IssueDto newIssue(String assignee) {
.setAssigneeUuid(assignee) .setAssigneeUuid(assignee)
.setCreatedAt(PAST).setIssueCreationTime(PAST) .setCreatedAt(PAST).setIssueCreationTime(PAST)
.setUpdatedAt(PAST).setIssueUpdateTime(PAST) .setUpdatedAt(PAST).setIssueUpdateTime(PAST)
.setType(RuleType.CODE_SMELL)); .setType(CODE_SMELL));
return issue; return issue;
} }


Expand Down

0 comments on commit 867949e

Please sign in to comment.